From patchwork Tue Mar 29 12:14:33 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael S. Tsirkin" X-Patchwork-Id: 8686351 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 5578D9F44D for ; Tue, 29 Mar 2016 12:15:01 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 8FEDD201E4 for ; Tue, 29 Mar 2016 12:14:55 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 67AE02013D for ; Tue, 29 Mar 2016 12:14:54 +0000 (UTC) Received: from localhost ([::1]:46412 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aksXt-0000FF-Pt for patchwork-qemu-devel@patchwork.kernel.org; Tue, 29 Mar 2016 08:14:53 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34234) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aksXl-0000Aw-IG for qemu-devel@nongnu.org; Tue, 29 Mar 2016 08:14:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aksXj-0001qP-VE for qemu-devel@nongnu.org; Tue, 29 Mar 2016 08:14:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49337) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aksXd-0001pR-1Z; Tue, 29 Mar 2016 08:14:37 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id A8BA846201; Tue, 29 Mar 2016 12:14:36 +0000 (UTC) Received: from redhat.com (vpn1-6-67.ams2.redhat.com [10.36.6.67]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with SMTP id u2TCEY0l031562; Tue, 29 Mar 2016 08:14:34 -0400 Date: Tue, 29 Mar 2016 15:14:33 +0300 From: "Michael S. Tsirkin" To: qemu-devel@nongnu.org Message-ID: <1459253200-23978-1-git-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Disposition: inline X-Mutt-Fcc: =sent X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Kevin Wolf , Cornelia Huck , qemu-block@nongnu.org, Stefan Hajnoczi , Paolo Bonzini Subject: [Qemu-devel] [PATCH RFC] virtio blk dataplane: aio virtio handler X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP So I'm fairly unhappy about the need to rework host notifiers at this stage in release cycle. So I thought about the root cause, and I think the main issue is that dataplane is now reusing the regular virtio code but through aio. The following, then, is an attempt to work around that for 2.6 without making major changes affecting anyone except dataplace - it's ugly but not nearly as ugly as I thought it will turn out to be, and we can rip it out after 2.6. The idea is that scsi dataplane can do a similar hack. Note: compiled only, sending out for early flames/feedback. ---> Subject: [PATCH RFC] virtio blk dataplane: aio virtio handler In addition to handling IO in vcpu thread and in io thread, blk dataplane introduces yet another mode: handling it by aio. This reuses the same handler as previous modes, which triggers races as these were not designed to be reentrant. As a temporary fix, add a separate handler just for aio, and make regular handlers do nothing when dataplane is active. Signed-off-by: Michael S. Tsirkin --- include/hw/virtio/virtio-blk.h | 2 ++ include/hw/virtio/virtio.h | 4 ++++ hw/block/dataplane/virtio-blk.c | 16 ++++++++++++++++ hw/block/virtio-blk.c | 22 +++++++++++++++------- hw/virtio/virtio.c | 36 ++++++++++++++++++++++++++++++++---- 5 files changed, 69 insertions(+), 11 deletions(-) diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index ae84d92..df517ff 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -85,4 +85,6 @@ void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb); void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb); +void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq); + #endif diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 2b5b248..c032067 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -142,6 +142,9 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, void (*handle_output)(VirtIODevice *, VirtQueue *)); +void virtio_set_queue_aio(VirtQueue *vq, + void (*handle_output)(VirtIODevice *, VirtQueue *)); + void virtio_del_queue(VirtIODevice *vdev, int n); void *virtqueue_alloc_element(size_t sz, unsigned out_num, unsigned in_num); @@ -253,6 +256,7 @@ void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign, void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx, bool assign, bool set_handler); void virtio_queue_notify_vq(VirtQueue *vq); +void virtio_queue_notify_aio_vq(VirtQueue *vq); void virtio_irq(VirtQueue *vq); VirtQueue *virtio_vector_first_queue(VirtIODevice *vdev, uint16_t vector); VirtQueue *virtio_vector_next_queue(VirtQueue *vq); diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 36f3d2b..72dbce8 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -184,6 +184,20 @@ void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s) g_free(s); } +static void virtio_blk_data_plane_handle_output(VirtIODevice *vdev, + VirtQueue *vq) +{ + VirtIOBlock *s = VIRTIO_BLK(vdev); + + if (!s->dataplane) { + return; + } + + assert(s->dataplane_started); + + virtio_blk_handle_vq(s, vq); +} + /* Context: QEMU global mutex held */ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s) { @@ -226,6 +240,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s) /* Get this show started by hooking up our callbacks */ aio_context_acquire(s->ctx); + virtio_set_queue_aio(s->vq, virtio_blk_data_plane_handle_output); virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, true, true); aio_context_release(s->ctx); return; @@ -262,6 +277,7 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s) /* Stop notifications for new requests from guest */ virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, false, false); + virtio_set_queue_aio(s->vq, NULL); /* 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/block/virtio-blk.c b/hw/block/virtio-blk.c index cb710f1..5aa884d 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -577,17 +577,18 @@ void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) } } -static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq) +void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq) { - VirtIOBlock *s = VIRTIO_BLK(vdev); VirtIOBlockReq *req; MultiReqBuffer mrb = {}; - /* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start - * dataplane here instead of waiting for .set_status(). - */ - if (s->dataplane && !s->dataplane_started) { - virtio_blk_data_plane_start(s->dataplane); + if (s->dataplane) { + /* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start + * dataplane here instead of waiting for .set_status(). + */ + if (!s->dataplane_started) { + virtio_blk_data_plane_start(s->dataplane); + } return; } @@ -604,6 +605,13 @@ static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq) blk_io_unplug(s->blk); } +static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq) +{ + VirtIOBlock *s = VIRTIO_BLK(vdev); + + virtio_blk_handle_vq(s, vq); +} + static void virtio_blk_dma_restart_bh(void *opaque) { VirtIOBlock *s = opaque; diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 08275a9..182bc56 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -94,6 +94,7 @@ struct VirtQueue uint16_t vector; void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq); + void (*handle_aio_output)(VirtIODevice *vdev, VirtQueue *vq); VirtIODevice *vdev; EventNotifier guest_notifier; EventNotifier host_notifier; @@ -1086,6 +1087,16 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align) virtio_queue_update_rings(vdev, n); } +void virtio_queue_notify_aio_vq(VirtQueue *vq) +{ + if (vq->vring.desc && vq->handle_aio_output) { + VirtIODevice *vdev = vq->vdev; + + trace_virtio_queue_notify(vdev, vq - vdev->vq, vq); + vq->handle_aio_output(vdev, vq); + } +} + void virtio_queue_notify_vq(VirtQueue *vq) { if (vq->vring.desc && vq->handle_output) { @@ -1141,10 +1152,19 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, vdev->vq[i].vring.num_default = queue_size; vdev->vq[i].vring.align = VIRTIO_PCI_VRING_ALIGN; vdev->vq[i].handle_output = handle_output; + vdev->vq[i].handle_aio_output = NULL; return &vdev->vq[i]; } +void virtio_set_queue_aio(VirtQueue *vq, + void (*handle_output)(VirtIODevice *, VirtQueue *)) +{ + assert(vq->handle_output); + + vq->handle_aio_output = handle_output; +} + void virtio_del_queue(VirtIODevice *vdev, int n) { if (n < 0 || n >= VIRTIO_QUEUE_MAX) { @@ -1778,11 +1798,11 @@ EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq) return &vq->guest_notifier; } -static void virtio_queue_host_notifier_read(EventNotifier *n) +static void virtio_queue_host_notifier_aio_read(EventNotifier *n) { VirtQueue *vq = container_of(n, VirtQueue, host_notifier); if (event_notifier_test_and_clear(n)) { - virtio_queue_notify_vq(vq); + virtio_queue_notify_aio_vq(vq); } } @@ -1791,14 +1811,22 @@ void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx, { if (assign && set_handler) { aio_set_event_notifier(ctx, &vq->host_notifier, true, - virtio_queue_host_notifier_read); + virtio_queue_host_notifier_aio_read); } else { aio_set_event_notifier(ctx, &vq->host_notifier, true, NULL); } if (!assign) { /* Test and clear notifier before after disabling event, * in case poll callback didn't have time to run. */ - virtio_queue_host_notifier_read(&vq->host_notifier); + virtio_queue_host_notifier_aio_read(&vq->host_notifier); + } +} + +static void virtio_queue_host_notifier_read(EventNotifier *n) +{ + VirtQueue *vq = container_of(n, VirtQueue, host_notifier); + if (event_notifier_test_and_clear(n)) { + virtio_queue_notify_vq(vq); } }