Re: [Qemu-devel] [patch v4 05/16] memory: introduce ref, unref interface for MemoryRegionOps

[Paolo Bonzini]

Il 23/10/2012 16:49, Avi Kivity ha scritto:
> On 10/23/2012 02:32 PM, Paolo Bonzini wrote:
>> Il 23/10/2012 14:15, Avi Kivity ha scritto:
>>> On 10/23/2012 02:06 PM, Paolo Bonzini wrote:
>>>> Il 23/10/2012 14:02, Avi Kivity ha scritto:
>>>>> On 10/23/2012 01:57 PM, Paolo Bonzini wrote:
>>>>>> Il 23/10/2012 13:55, Avi Kivity ha scritto:
>>>>>>>>> So the stop_machine idea is thrown away?  
>>>>>>> IIRC I convinced myself that it's just as bad.
>>>>>>
>>>>>> It may be just as bad, but it is less code (and less pervasive), which
>>>>>> makes it less painful.
>>>>>
>>>>> It saves you the ->ref() and ->unref() calls, which are boilerplate, but
>>>>> not too onerous. All of the device model and subsystem threading work
>>>>> still needs to be done.
>>>>
>>>> I'm not worried about saving the ->ref() and ->unref() calls in the
>>>> devices.  I'm worried about saving it in timers, bottom halves and
>>>> whatnot.  And also I'm not sure whether all callbacks would have
>>>> something to ref/unref as they are implemented now.
>>>
>>> Hard to say without examples.
>>>
>>> Something that bothers be with stop_machine is the reliance on
>>> cancellation.  With timers it's easy, stop_machine, remove the timer,
>>> resume.  But if you have an aio operation in progress that is not
>>> cancellable, you have to wait for that operation to complete.  Refcounts
>>> handle that well, the object stays until completion, then disappears.
>>
>> Yes, that's the point of doing things asynchronously---you do not need
>> to do everything within stop_machine, you can start canceling AIO as
>> soon as the OS sends the hot-unplug request.  Then you only proceed with
>> stop_machine and freeing device memory when the first part.
> 
> You cannot always cancel I/O (for example threaded I/O already in progress).

Yep, but we try to do this anyway today and nothing changes really.  The
difference is between hotplug never completing and blocking
synchronously, vs. hotplug never completing and not invoking the
asynchronous callback.  I.e. really no difference at all.

>> In other words, isolate can complete asynchronously.
> 
> Can it?  I don't think so.
> 
> Here's how I see it:
> 
>  1. non-malicious guest stops driving device
>  2. isolate()
>  3. a malicious guest cannot drive the device at this point
>  4. some kind of barrier to let the device, or drive activity from a
> malicious guest, wind down
>  5. destroy()
> 
> If you need to report the completion of step 2, it cannot be done
> asynchronously.

In hardware everything is asynchronous anyway.  It will *look*
synchronous, because if CPU#0 is stuck in a synchronous isolate(), and
CPU#1 polls for the outcome, CPU#1 will lock on the BQL held by CPU#0.

But our interfaces had better support asynchronicity, and indeed they
do: after you write to the "eject" register, the "up" will show the
device as present until after destroy is done.  This can be changed to
show the device as present only until after step 4 is done.

> We may also want notification after step 4 (or 5); if the device holds
> some host resource someone may want to know that it is ready for reuse.

I think guest notification should be after (4), while management
notification should be after (5).

Paolo