Re: [Qemu-devel] [PATCH 2/2] virtio-blk: dataplane: notify guest as a batch

[Paolo Bonzini <pbonzini@redhat.com>]

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 <ming.lei@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) \
>