Re: [Qemu-devel] [PATCH] virtio-blk: notify guest directly

[sochin . jiang]

In fact, I firstly found a performance loss  before and after

commit 9ffe337 using fio tools in suse11-sp3 guest(vitio-blk), especially

when testing 4k single IO models(say, write, randwrite, read and

randread, with iodepth set to 1), the result is 15%-20% performance loss

since commit 9ffe337, difference is an extra notification bh in dataplane as

I posted.

Then, I tested more IO models. Indeed, an extra BH can decrease interrupts

in guest, but not much more like ~13K/sec now, and only when testing

large IO(bs set to 64k,128k, 1024k...) or large iodepths(64, 128...) models, 
~5K/sec

decreased.

I did not test all IO models of course, and different models with different 
interrupts.

But on the whole, a performance loss is certain with an extra notification BH.


Sochin






On 2017/12/20 8:57, Paolo Bonzini wrote:
> On 19/12/2017 14:33, sochin.jiang wrote:
>> From: "sochin.jiang" <address@hidden>
>>
>>  Till now, we've already notify guest as a batch mostly, an
>>  extra BH won't decrease guest interrupts much, but cause a
>>  significant notification loss. Generally, we could have 15%
>>  or so performance lost in single queue IO models, as I tested.
> Interesting, this was indeed done to decrease interrupt overhead:
>
>     commit 5b2ffbe4d99843fd8305c573a100047a8c962327
>     Author: Ming Lei <address@hidden>
>     Date:   Sat Jul 12 12:08:53 2014 +0800
>
>     virtio-blk: dataplane: notify guest as a batch
>
>     Now requests are submitted as a batch, so it is natural
>     to notify guest as a batch too.
>
>     This may suppress interrupt notification to VM a lot:
>
>             - in my test, decreased by ~13K/sec
>
>     Signed-off-by: Ming Lei <address@hidden>
>     Signed-off-by: Stefan Hajnoczi <address@hidden>
>
> Can you explain your benchmark setup?
>
> Paolo
>
>
>> Signed-off-by: sochin.jiang <address@hidden>
>> ---
>>  hw/block/dataplane/virtio-blk.c | 19 +++++++------------
>>  1 file changed, 7 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/block/dataplane/virtio-blk.c 
>> b/hw/block/dataplane/virtio-blk.c
>> index 5556f0e..a261a1d 100644
>> --- a/hw/block/dataplane/virtio-blk.c
>> +++ b/hw/block/dataplane/virtio-blk.c
>> @@ -32,7 +32,6 @@ struct VirtIOBlockDataPlane {
>>  
>>      VirtIOBlkConf *conf;
>>      VirtIODevice *vdev;
>> -    QEMUBH *bh;                     /* bh for guest notification */
>>      unsigned long *batch_notify_vqs;
>>  
>>      /* Note that these EventNotifiers are assigned by value.  This is
>> @@ -44,14 +43,7 @@ struct VirtIOBlockDataPlane {
>>      AioContext *ctx;
>>  };
>>  
>> -/* Raise an interrupt to signal guest, if necessary */
>> -void virtio_blk_data_plane_notify(VirtIOBlockDataPlane *s, VirtQueue *vq)
>> -{
>> -    set_bit(virtio_get_queue_index(vq), s->batch_notify_vqs);
>> -    qemu_bh_schedule(s->bh);
>> -}
>> -
>> -static void notify_guest_bh(void *opaque)
>> +static void notify_guest(void *opaque)
>>  {
>>      VirtIOBlockDataPlane *s = opaque;
>>      unsigned nvqs = s->conf->num_queues;
>> @@ -75,7 +67,12 @@ static void notify_guest_bh(void *opaque)
>>      }
>>  }
>>  
>> -/* Context: QEMU global mutex held */
>> +/* Raise an interrupt to signal guest, if necessary */
>> +void virtio_blk_data_plane_notify(VirtIOBlockDataPlane *s, VirtQueue *vq)
>> +{
>> +    set_bit(virtio_get_queue_index(vq), s->batch_notify_vqs);
>> +    notify_guest(s);
>> +}
>>  void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
>>                                    VirtIOBlockDataPlane **dataplane,
>>                                    Error **errp)
>> @@ -122,7 +119,6 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, 
>> VirtIOBlkConf *conf,
>>      } else {
>>          s->ctx = qemu_get_aio_context();
>>      }
>> -    s->bh = aio_bh_new(s->ctx, notify_guest_bh, s);
>>      s->batch_notify_vqs = bitmap_new(conf->num_queues);
>>  
>>      *dataplane = s;
>> @@ -140,7 +136,6 @@ void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane 
>> *s)
>>      vblk = VIRTIO_BLK(s->vdev);
>>      assert(!vblk->dataplane_started);
>>      g_free(s->batch_notify_vqs);
>> -    qemu_bh_delete(s->bh);
>>      if (s->iothread) {
>>          object_unref(OBJECT(s->iothread));
>>      }
>>
>
> .
>