qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 0/4] Tweaks around virtio-blk start/stop


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH 0/4] Tweaks around virtio-blk start/stop
Date: Wed, 16 Mar 2016 13:49:10 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0


On 16/03/2016 13:42, Cornelia Huck wrote:
> On Wed, 16 Mar 2016 13:32:59 +0100
> Paolo Bonzini <address@hidden> wrote:
> 
>> On 16/03/2016 13:22, Cornelia Huck wrote:
>>>>> Yeah, it doesn't help that the functions are underdocumented (as in the
>>>>> "assign" parameter above).
>>> My understanding is:
>>>
>>> - 'assign': set up a new notifier (true) or disable it (false)
>>> - 'set_handler': use our handler (true) or have it handled elsewhere
>>>   (false)
>>
>> Right.  So if we're setting up a new notifier in
>> virtio_queue_aio_set_host_notifier_handler, virtio_pci_stop_ioeventfd should

... not call virtio_queue_host_notifier_read.

>>>>> I don't think the ->set_host_notifiers() api really allows for this.
>>>>
>>>> I think it does, assign is the last argument to k->set_host_notifier().
>>>
>>> This depends on whether we want 'assign' to clean up any old notifiers
>>> before setting up new ones. I think we want different behaviour for
>>> dataplane and vhost.
>>
>> I think dataplane and vhost are the same.
>>
>> The question is whether ioeventfd=off,vhost=on or
>> ioeventfd=off,dataplane=on are valid combinations; I think they aren't.
> 
> We should disallow that even temporary, then.
> 
> (This would imply that we need to drop the _stop_ioeventfd() call in
> ->set_host_notifier(), correct?)

No, it would not.  ioeventfd=off,vhost=on would mean: "when vhost is
off, use vCPU thread notification".

When turning on vhost you'd still stop ioeventfd (i.e. stop processing
the virtqueue in QEMU's main iothread), but you don't need to do
anything to the event notifier.  vhost will pick it up and work on the
virtqueue if necessary.  Likewise for dataplane.

>> If they aren't, it should be okay to remove the
>> virtio_queue_host_notifier_read call in
>> virtio_queue_set_host_notifier_fd_handler and
>> virtio_queue_aio_set_host_notifier_handler.  That's because a handler
>> for the notifier will always be set _somewhere_.  It could be the usual
>> ioeventfd handler, the vhost handler or the dataplane handler, but one
>> will be there.
> 
> It should; but we probably need to do a final read when we stop the
> ioeventfd.

I was thinking of handing the final read directly to the next guy who
polls the event notifier instead.  So, when called from vhost or
dataplane, virtio_pci_stop_ioeventfd would use
assign=true/set_handler=false ("a new notifier is going to be set up by
the caller").

The host notifier API unfortunately is full of indirections.  I'm not
sure how many of them are actually necessary.

Paolo

Paolo



reply via email to

[Prev in Thread] Current Thread [Next in Thread]