From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49721) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agAt9-0005a7-Nr for qemu-devel@nongnu.org; Wed, 16 Mar 2016 08:49:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1agAt8-0006wu-RM for qemu-devel@nongnu.org; Wed, 16 Mar 2016 08:49:23 -0400 References: <1458123018-18651-1-git-send-email-famz@redhat.com> <56E9355A.5070700@redhat.com> <56E93A22.1080102@de.ibm.com> <56E93ECE.10103@redhat.com> <20160316123213.3dcf0abc.cornelia.huck@de.ibm.com> <56E94806.7060505@redhat.com> <20160316125623.38ab4c7e.cornelia.huck@de.ibm.com> <56E94AA9.8000803@redhat.com> <20160316132212.674f766f.cornelia.huck@de.ibm.com> <56E9527B.9010001@redhat.com> <20160316134257.72d7da56.cornelia.huck@de.ibm.com> From: Paolo Bonzini Message-ID: <56E95646.8020802@redhat.com> Date: Wed, 16 Mar 2016 13:49:10 +0100 MIME-Version: 1.0 In-Reply-To: <20160316134257.72d7da56.cornelia.huck@de.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 0/4] Tweaks around virtio-blk start/stop List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck Cc: Kevin Wolf , Fam Zheng , qemu-block@nongnu.org, "Michael S. Tsirkin" , qemu-devel@nongnu.org, Christian Borntraeger , tubo@linux.vnet.ibm.com, Stefan Hajnoczi On 16/03/2016 13:42, Cornelia Huck wrote: > On Wed, 16 Mar 2016 13:32:59 +0100 > Paolo Bonzini wrote: >=20 >> 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 notifier= s >>> 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=3Doff,vhost=3Don or >> ioeventfd=3Doff,dataplane=3Don are valid combinations; I think they ar= en't. >=20 > We should disallow that even temporary, then. >=20 > (This would imply that we need to drop the _stop_ioeventfd() call in > ->set_host_notifier(), correct?) No, it would not. ioeventfd=3Doff,vhost=3Don 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 usua= l >> ioeventfd handler, the vhost handler or the dataplane handler, but one >> will be there. >=20 > 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=3Dtrue/set_handler=3Dfalse ("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