From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55498) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X331N-0003zL-D5 for qemu-devel@nongnu.org; Fri, 04 Jul 2014 08:55:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X331H-0001sr-Ua for qemu-devel@nongnu.org; Fri, 04 Jul 2014 08:55:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36681) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X331H-0001sX-MW for qemu-devel@nongnu.org; Fri, 04 Jul 2014 08:55:15 -0400 Message-ID: <53B6A428.7030800@redhat.com> Date: Fri, 04 Jul 2014 14:55:04 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1404476854-23773-1-git-send-email-ming.lei@canonical.com> <1404476854-23773-3-git-send-email-ming.lei@canonical.com> In-Reply-To: <1404476854-23773-3-git-send-email-ming.lei@canonical.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] virtio-blk: dataplane: notify guest as a batch List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ming Lei , Peter Maydell , qemu-devel@nongnu.org, Stefan Hajnoczi Cc: Kevin Wolf , Fam Zheng , "Michael S. Tsirkin" Il 04/07/2014 14:27, Ming Lei ha scritto: > Now requests are submitted as a batch, so it is natural > to notify guest as a batch too. > > This may supress interrupt notification to VM: > > - in my test, decreased by ~13K/sec I don't think this can work. Requests are not completed FIFO. What you can do is change notify_guest to something like qemu_bh_schedule(req->dev->dataplane->notify_guest_bh); and do the actual notification in the bottom half. This should ensure that multiple notifications are coalesced, but it may also introduce new aio_notify calls even with my patch (a BH scheduled from a BH currently does an aio_notify; this can be fixed). Paolo > Signed-off-by: Ming Lei > --- > hw/block/dataplane/virtio-blk.c | 13 ++++++++++++- > hw/block/virtio-blk.c | 1 + > include/hw/virtio/virtio-blk.h | 1 + > 3 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c > index e88862d..9d36659 100644 > --- a/hw/block/dataplane/virtio-blk.c > +++ b/hw/block/dataplane/virtio-blk.c > @@ -45,6 +45,8 @@ struct VirtIOBlockDataPlane { > AioContext *ctx; > EventNotifier host_notifier; /* doorbell */ > > + VirtIOBlockReq *last_submit; > + > /* Operation blocker on BDS */ > Error *blocker; > void (*saved_complete_request)(struct VirtIOBlockReq *req, > @@ -67,7 +69,10 @@ static void complete_request_vring(VirtIOBlockReq *req, unsigned char status) > > vring_push(&req->dev->dataplane->vring, &req->elem, > req->qiov.size + sizeof(*req->in)); > - notify_guest(req->dev->dataplane); > + > + if (req->last) { > + notify_guest(req->dev->dataplane); > + } > } > > static void handle_notify(EventNotifier *e) > @@ -101,6 +106,8 @@ static void handle_notify(EventNotifier *e) > req->elem.index); > > virtio_blk_handle_request(req, &mrb); > + > + s->last_submit = req; > } > > virtio_submit_multiwrite(s->blk->conf.bs, &mrb); > @@ -116,6 +123,10 @@ static void handle_notify(EventNotifier *e) > break; > } > } > + > + if (s->last_submit) { > + s->last_submit->last = true; > + } > bdrv_io_unplug(s->blk->conf.bs); > } > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index 02cd6b0..86b37f7 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -35,6 +35,7 @@ VirtIOBlockReq *virtio_blk_alloc_request(VirtIOBlock *s) > req->dev = s; > req->qiov.size = 0; > req->next = NULL; > + req->last = false; > return req; > } > > diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h > index 992da26..07659c3 100644 > --- a/include/hw/virtio/virtio-blk.h > +++ b/include/hw/virtio/virtio-blk.h > @@ -150,6 +150,7 @@ typedef struct VirtIOBlockReq { > QEMUIOVector qiov; > struct VirtIOBlockReq *next; > BlockAcctCookie acct; > + bool last; > } VirtIOBlockReq; > > #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \ >