[PATCH v3 0/4] virtio-blk: use blk_io_plug_call() instead of notification BH

Stefan Hajnoczi posted 4 patches 7 months, 1 week ago
Failed in applying to current master (apply log)
There is a newer version of this series
MAINTAINERS                       |   3 +-
include/qemu/defer-call.h         |  16 +++
include/sysemu/block-backend-io.h |   4 -
block/blkio.c                     |   9 +-
block/io_uring.c                  |  11 ++-
block/linux-aio.c                 |   9 +-
block/nvme.c                      |   5 +-
block/plug.c                      | 159 ------------------------------
hw/block/dataplane/virtio-blk.c   |  48 +--------
hw/block/dataplane/xen-block.c    |  11 ++-
hw/block/virtio-blk.c             |   5 +-
hw/scsi/virtio-scsi.c             |   7 +-
hw/virtio/virtio.c                |  13 ++-
util/defer-call.c                 | 156 +++++++++++++++++++++++++++++
util/thread-pool.c                |   5 +
block/meson.build                 |   1 -
hw/virtio/trace-events            |   1 +
util/meson.build                  |   1 +
18 files changed, 231 insertions(+), 233 deletions(-)
create mode 100644 include/qemu/defer-call.h
delete mode 100644 block/plug.c
create mode 100644 util/defer-call.c
[PATCH v3 0/4] virtio-blk: use blk_io_plug_call() instead of notification BH
Posted by Stefan Hajnoczi 7 months, 1 week ago
v3:
- Add comment pointing to API documentation in .c file [Philippe]
- Add virtio_notify_irqfd_deferred_fn trace event [Ilya]
- Remove outdated #include [Ilya]
v2:
- Rename blk_io_plug() to defer_call() and move it to util/ so the net
  subsystem can use it [Ilya]
- Add defer_call_begin()/end() to thread_pool_completion_bh() to match Linux
  AIO and io_uring completion batching

Replace the seldom-used virtio-blk notification BH mechanism with
blk_io_plug(). This is part of an effort to enable the multi-queue block layer
in virtio-blk. The notification BH was not multi-queue friendly.

The blk_io_plug() mechanism improves fio rw=randread bs=4k iodepth=64 numjobs=8
IOPS by ~9% with a single IOThread and 8 vCPUs (this is not even a multi-queue
block layer configuration) compared to no completion batching. iodepth=1
decreases by ~1% but this could be noise. Benchmark details are available here:
https://gitlab.com/stefanha/virt-playbooks/-/tree/blk_io_plug-irqfd

Stefan Hajnoczi (4):
  block: rename blk_io_plug_call() API to defer_call()
  util/defer-call: move defer_call() to util/
  virtio: use defer_call() in virtio_irqfd_notify()
  virtio-blk: remove batch notification BH

 MAINTAINERS                       |   3 +-
 include/qemu/defer-call.h         |  16 +++
 include/sysemu/block-backend-io.h |   4 -
 block/blkio.c                     |   9 +-
 block/io_uring.c                  |  11 ++-
 block/linux-aio.c                 |   9 +-
 block/nvme.c                      |   5 +-
 block/plug.c                      | 159 ------------------------------
 hw/block/dataplane/virtio-blk.c   |  48 +--------
 hw/block/dataplane/xen-block.c    |  11 ++-
 hw/block/virtio-blk.c             |   5 +-
 hw/scsi/virtio-scsi.c             |   7 +-
 hw/virtio/virtio.c                |  13 ++-
 util/defer-call.c                 | 156 +++++++++++++++++++++++++++++
 util/thread-pool.c                |   5 +
 block/meson.build                 |   1 -
 hw/virtio/trace-events            |   1 +
 util/meson.build                  |   1 +
 18 files changed, 231 insertions(+), 233 deletions(-)
 create mode 100644 include/qemu/defer-call.h
 delete mode 100644 block/plug.c
 create mode 100644 util/defer-call.c

-- 
2.41.0
Re: [PATCH v3 0/4] virtio-blk: use blk_io_plug_call() instead of notification BH
Posted by Kevin Wolf 5 months, 2 weeks ago
Am 13.09.2023 um 22:00 hat Stefan Hajnoczi geschrieben:
> v3:
> - Add comment pointing to API documentation in .c file [Philippe]
> - Add virtio_notify_irqfd_deferred_fn trace event [Ilya]
> - Remove outdated #include [Ilya]
> v2:
> - Rename blk_io_plug() to defer_call() and move it to util/ so the net
>   subsystem can use it [Ilya]
> - Add defer_call_begin()/end() to thread_pool_completion_bh() to match Linux
>   AIO and io_uring completion batching
> 
> Replace the seldom-used virtio-blk notification BH mechanism with
> blk_io_plug(). This is part of an effort to enable the multi-queue block layer
> in virtio-blk. The notification BH was not multi-queue friendly.
> 
> The blk_io_plug() mechanism improves fio rw=randread bs=4k iodepth=64 numjobs=8
> IOPS by ~9% with a single IOThread and 8 vCPUs (this is not even a multi-queue
> block layer configuration) compared to no completion batching. iodepth=1
> decreases by ~1% but this could be noise. Benchmark details are available here:
> https://gitlab.com/stefanha/virt-playbooks/-/tree/blk_io_plug-irqfd

Thanks, applied to the block branch.

Kevin
Re: [PATCH v3 0/4] virtio-blk: use blk_io_plug_call() instead of notification BH
Posted by Michael S. Tsirkin 7 months, 1 week ago
On Wed, Sep 13, 2023 at 04:00:41PM -0400, Stefan Hajnoczi wrote:
> v3:
> - Add comment pointing to API documentation in .c file [Philippe]
> - Add virtio_notify_irqfd_deferred_fn trace event [Ilya]
> - Remove outdated #include [Ilya]
> v2:
> - Rename blk_io_plug() to defer_call() and move it to util/ so the net
>   subsystem can use it [Ilya]
> - Add defer_call_begin()/end() to thread_pool_completion_bh() to match Linux
>   AIO and io_uring completion batching
> 
> Replace the seldom-used virtio-blk notification BH mechanism with
> blk_io_plug(). This is part of an effort to enable the multi-queue block layer
> in virtio-blk. The notification BH was not multi-queue friendly.
> 
> The blk_io_plug() mechanism improves fio rw=randread bs=4k iodepth=64 numjobs=8
> IOPS by ~9% with a single IOThread and 8 vCPUs (this is not even a multi-queue
> block layer configuration) compared to no completion batching. iodepth=1
> decreases by ~1% but this could be noise. Benchmark details are available here:
> https://gitlab.com/stefanha/virt-playbooks/-/tree/blk_io_plug-irqfd


virtio things:

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>


regression is a bit worrysome if real though - just do a bit
longer testing to make sure?



> Stefan Hajnoczi (4):
>   block: rename blk_io_plug_call() API to defer_call()
>   util/defer-call: move defer_call() to util/
>   virtio: use defer_call() in virtio_irqfd_notify()
>   virtio-blk: remove batch notification BH
> 
>  MAINTAINERS                       |   3 +-
>  include/qemu/defer-call.h         |  16 +++
>  include/sysemu/block-backend-io.h |   4 -
>  block/blkio.c                     |   9 +-
>  block/io_uring.c                  |  11 ++-
>  block/linux-aio.c                 |   9 +-
>  block/nvme.c                      |   5 +-
>  block/plug.c                      | 159 ------------------------------
>  hw/block/dataplane/virtio-blk.c   |  48 +--------
>  hw/block/dataplane/xen-block.c    |  11 ++-
>  hw/block/virtio-blk.c             |   5 +-
>  hw/scsi/virtio-scsi.c             |   7 +-
>  hw/virtio/virtio.c                |  13 ++-
>  util/defer-call.c                 | 156 +++++++++++++++++++++++++++++
>  util/thread-pool.c                |   5 +
>  block/meson.build                 |   1 -
>  hw/virtio/trace-events            |   1 +
>  util/meson.build                  |   1 +
>  18 files changed, 231 insertions(+), 233 deletions(-)
>  create mode 100644 include/qemu/defer-call.h
>  delete mode 100644 block/plug.c
>  create mode 100644 util/defer-call.c
> 
> -- 
> 2.41.0