On Fri, Jul 4, 2014 at 8:55 PM, Paolo Bonzini <pbonz...@redhat.com> wrote: > 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.
Yes, you are right. What I want to do is to keep old dataplane's behavior: only call notify_guest() one time for one IO completion. Switching to QEMU block makes it a bit difficult to implement, especially with io plug & unplug. One approach I thought of is to introduce a notifier in bs->file, and write it inside linux aio completion handler, which looks a bit ugly, but shouldn't have performance effect because io completion becomes less frequent with io plug. > > 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). I think we can do better than above because one aio IO completes lots of requests, and we just need to notify guest for all these requests. Thanks, > > Paolo > > >> Signed-off-by: Ming Lei <ming....@canonical.com> >> --- >> 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) \ >> > >