From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48452) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1anONF-0006BW-36 for qemu-devel@nongnu.org; Tue, 05 Apr 2016 06:38:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1anONA-0007DW-4t for qemu-devel@nongnu.org; Tue, 05 Apr 2016 06:38:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52156) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1anON9-0007Cp-Tt for qemu-devel@nongnu.org; Tue, 05 Apr 2016 06:38:12 -0400 Date: Tue, 5 Apr 2016 13:38:07 +0300 From: "Michael S. Tsirkin" Message-ID: <20160405133641-mutt-send-email-mst@redhat.com> References: <1459516794-23629-1-git-send-email-pbonzini@redhat.com> <1459516794-23629-2-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1459516794-23629-2-git-send-email-pbonzini@redhat.com> Subject: Re: [Qemu-devel] [PATCH 1/9] virtio-dataplane: pass assign=true to virtio_queue_aio_set_host_notifier_handler List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: famz@redhat.com, tubo@linux.vnet.ibm.com, qemu-devel@nongnu.org, dgilbert@redhat.com, borntraeger@de.ibm.com, stefanha@redhat.com, cornelia.huck@de.ibm.com On Fri, Apr 01, 2016 at 03:19:46PM +0200, Paolo Bonzini wrote: > There is no need to run the handler one last time; the device is > being reset and it is okay to drop requests that are pending in > the virtqueue. Even in the case of migration, the requests would > be processed when ioeventfd is re-enabled on the destination > side: virtio_queue_set_host_notifier_fd_handler will call > virtio_queue_host_notifier_read, which will start dataplane; I didn't get this part: here's virtio_queue_host_notifier_read: VirtQueue *vq = container_of(n, VirtQueue, host_notifier); if (event_notifier_test_and_clear(n)) { virtio_queue_notify_vq(vq); } event notifier is initially clear on migration, how can we be sure virtio_queue_notify_vq is invoked? > the host > notifier is then connected to the I/O thread and event_notifier_set is > called to start processing it. > > By omitting this call, we dodge a possible cause of races between the > dataplane thread on one side and the main/vCPU threads on the other. > > The virtio_queue_aio_set_host_notifier_handler function is now > only ever called with assign=true, but for now this is left as is > because the function parameters will change soon anyway. > > Signed-off-by: Paolo Bonzini > --- > hw/block/dataplane/virtio-blk.c | 2 +- > hw/scsi/virtio-scsi-dataplane.c | 6 +++--- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c > index e666dd4..fddd3ab 100644 > --- a/hw/block/dataplane/virtio-blk.c > +++ b/hw/block/dataplane/virtio-blk.c > @@ -262,7 +262,7 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s) > aio_context_acquire(s->ctx); > > /* Stop notifications for new requests from guest */ > - virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, false, false); > + virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, true, false); > > /* Drain and switch bs back to the QEMU main loop */ > blk_set_aio_context(s->conf->conf.blk, qemu_get_aio_context()); > diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c > index b44ac5d..21d5bfd 100644 > --- a/hw/scsi/virtio-scsi-dataplane.c > +++ b/hw/scsi/virtio-scsi-dataplane.c > @@ -70,10 +70,10 @@ static void virtio_scsi_clear_aio(VirtIOSCSI *s) > VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s); > int i; > > - virtio_queue_aio_set_host_notifier_handler(vs->ctrl_vq, s->ctx, false, false); > - virtio_queue_aio_set_host_notifier_handler(vs->event_vq, s->ctx, false, false); > + virtio_queue_aio_set_host_notifier_handler(vs->ctrl_vq, s->ctx, true, false); > + virtio_queue_aio_set_host_notifier_handler(vs->event_vq, s->ctx, true, false); > for (i = 0; i < vs->conf.num_queues; i++) { > - virtio_queue_aio_set_host_notifier_handler(vs->cmd_vqs[i], s->ctx, false, false); > + virtio_queue_aio_set_host_notifier_handler(vs->cmd_vqs[i], s->ctx, true, false); > } > } > > -- > 1.8.3.1 >