| Message ID | 20230817155847.3605115-4-stefanha@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | virtio-blk: use blk_io_plug_call() instead of notification BH | expand |
On Thu, Aug 17, 2023 at 11:58:46AM -0400, Stefan Hajnoczi wrote: > virtio-blk and virtio-scsi invoke virtio_irqfd_notify() to send Used > Buffer Notifications from an IOThread. This involves an eventfd > write(2) syscall. Calling this repeatedly when completing multiple I/O > requests in a row is wasteful. > > Use the defer_call() API to batch together virtio_irqfd_notify() calls > made during thread pool (aio=threads), Linux AIO (aio=native), and > io_uring (aio=io_uring) completion processing. > > Behavior is unchanged for emulated devices that do not use > defer_call_begin()/defer_call_end() since defer_call() immediately > invokes the callback when called outside a > defer_call_begin()/defer_call_end() region. > > fio rw=randread bs=4k iodepth=64 numjobs=8 IOPS increases by ~9% with a > single IOThread and 8 vCPUs. iodepth=1 decreases by ~1% but this could > be noise. Detailed performance data and configuration specifics are > available here: > https://gitlab.com/stefanha/virt-playbooks/-/tree/blk_io_plug-irqfd > > This duplicates the BH that virtio-blk uses for batching. The next > commit will remove it. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > block/io_uring.c | 6 ++++++ > block/linux-aio.c | 4 ++++ > hw/virtio/virtio.c | 11 ++++++++++- > util/thread-pool.c | 5 +++++ > 4 files changed, 25 insertions(+), 1 deletion(-) Reviewed-by: Eric Blake <eblake@redhat.com>
On 8/17/23 17:58, Stefan Hajnoczi wrote: > virtio-blk and virtio-scsi invoke virtio_irqfd_notify() to send Used > Buffer Notifications from an IOThread. This involves an eventfd > write(2) syscall. Calling this repeatedly when completing multiple I/O > requests in a row is wasteful. > > Use the defer_call() API to batch together virtio_irqfd_notify() calls > made during thread pool (aio=threads), Linux AIO (aio=native), and > io_uring (aio=io_uring) completion processing. > > Behavior is unchanged for emulated devices that do not use > defer_call_begin()/defer_call_end() since defer_call() immediately > invokes the callback when called outside a > defer_call_begin()/defer_call_end() region. > > fio rw=randread bs=4k iodepth=64 numjobs=8 IOPS increases by ~9% with a > single IOThread and 8 vCPUs. iodepth=1 decreases by ~1% but this could > be noise. Detailed performance data and configuration specifics are > available here: > https://gitlab.com/stefanha/virt-playbooks/-/tree/blk_io_plug-irqfd > > This duplicates the BH that virtio-blk uses for batching. The next > commit will remove it. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > block/io_uring.c | 6 ++++++ > block/linux-aio.c | 4 ++++ > hw/virtio/virtio.c | 11 ++++++++++- > util/thread-pool.c | 5 +++++ > 4 files changed, 25 insertions(+), 1 deletion(-) > > diff --git a/block/io_uring.c b/block/io_uring.c > index 3a1e1f45b3..7cdd00e9f1 100644 > --- a/block/io_uring.c > +++ b/block/io_uring.c > @@ -125,6 +125,9 @@ static void luring_process_completions(LuringState *s) > { > struct io_uring_cqe *cqes; > int total_bytes; > + > + defer_call_begin(); > + > /* > * Request completion callbacks can run the nested event loop. > * Schedule ourselves so the nested event loop will "see" remaining > @@ -217,7 +220,10 @@ end: > aio_co_wake(luringcb->co); > } > } > + > qemu_bh_cancel(s->completion_bh); > + > + defer_call_end(); > } > > static int ioq_submit(LuringState *s) > diff --git a/block/linux-aio.c b/block/linux-aio.c > index 62380593c8..ab607ade6a 100644 > --- a/block/linux-aio.c > +++ b/block/linux-aio.c > @@ -205,6 +205,8 @@ static void qemu_laio_process_completions(LinuxAioState *s) > { > struct io_event *events; > > + defer_call_begin(); > + > /* Reschedule so nested event loops see currently pending completions */ > qemu_bh_schedule(s->completion_bh); > > @@ -231,6 +233,8 @@ static void qemu_laio_process_completions(LinuxAioState *s) > * own `for` loop. If we are the last all counters droped to zero. */ > s->event_max = 0; > s->event_idx = 0; > + > + defer_call_end(); > } > > static void qemu_laio_process_completions_and_submit(LinuxAioState *s) > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 309038fd46..5eb1f91b41 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -15,6 +15,7 @@ > #include "qapi/error.h" > #include "qapi/qapi-commands-virtio.h" > #include "trace.h" > +#include "qemu/defer-call.h" > #include "qemu/error-report.h" > #include "qemu/log.h" > #include "qemu/main-loop.h" > @@ -28,6 +29,7 @@ > #include "hw/virtio/virtio-bus.h" > #include "hw/qdev-properties.h" > #include "hw/virtio/virtio-access.h" > +#include "sysemu/block-backend.h" An artifact from the previous version. > #include "sysemu/dma.h" > #include "sysemu/runstate.h" > #include "virtio-qmp.h" > @@ -2426,6 +2428,13 @@ static bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq) > } > } > > +/* Batch irqs while inside a defer_call_begin()/defer_call_end() section */ > +static void virtio_notify_irqfd_deferred_fn(void *opaque) > +{ > + EventNotifier *notifier = opaque; > + event_notifier_set(notifier); > +} > + > void virtio_notify_irqfd(VirtIODevice *vdev, VirtQueue *vq) > { > WITH_RCU_READ_LOCK_GUARD() { > @@ -2452,7 +2461,7 @@ void virtio_notify_irqfd(VirtIODevice *vdev, VirtQueue *vq) > * to an atomic operation. > */ > virtio_set_isr(vq->vdev, 0x1); > - event_notifier_set(&vq->guest_notifier); > + defer_call(virtio_notify_irqfd_deferred_fn, &vq->guest_notifier); Should we move the trace from this function to deferred one? Or maybe add a new trace? > } > > static void virtio_irq(VirtQueue *vq) > diff --git a/util/thread-pool.c b/util/thread-pool.c > index e3d8292d14..d84961779a 100644 > --- a/util/thread-pool.c > +++ b/util/thread-pool.c > @@ -15,6 +15,7 @@ > * GNU GPL, version 2 or (at your option) any later version. > */ > #include "qemu/osdep.h" > +#include "qemu/defer-call.h" > #include "qemu/queue.h" > #include "qemu/thread.h" > #include "qemu/coroutine.h" > @@ -175,6 +176,8 @@ static void thread_pool_completion_bh(void *opaque) > ThreadPool *pool = opaque; > ThreadPoolElement *elem, *next; > > + defer_call_begin(); /* cb() may use defer_call() to coalesce work */ > + > restart: > QLIST_FOREACH_SAFE(elem, &pool->head, all, next) { > if (elem->state != THREAD_DONE) { > @@ -208,6 +211,8 @@ restart: > qemu_aio_unref(elem); > } > } > + > + defer_call_end(); > } > > static void thread_pool_cancel(BlockAIOCB *acb)
diff --git a/block/io_uring.c b/block/io_uring.c index 3a1e1f45b3..7cdd00e9f1 100644 --- a/block/io_uring.c +++ b/block/io_uring.c @@ -125,6 +125,9 @@ static void luring_process_completions(LuringState *s) { struct io_uring_cqe *cqes; int total_bytes; + + defer_call_begin(); + /* * Request completion callbacks can run the nested event loop. * Schedule ourselves so the nested event loop will "see" remaining @@ -217,7 +220,10 @@ end: aio_co_wake(luringcb->co); } } + qemu_bh_cancel(s->completion_bh); + + defer_call_end(); } static int ioq_submit(LuringState *s) diff --git a/block/linux-aio.c b/block/linux-aio.c index 62380593c8..ab607ade6a 100644 --- a/block/linux-aio.c +++ b/block/linux-aio.c @@ -205,6 +205,8 @@ static void qemu_laio_process_completions(LinuxAioState *s) { struct io_event *events; + defer_call_begin(); + /* Reschedule so nested event loops see currently pending completions */ qemu_bh_schedule(s->completion_bh); @@ -231,6 +233,8 @@ static void qemu_laio_process_completions(LinuxAioState *s) * own `for` loop. If we are the last all counters droped to zero. */ s->event_max = 0; s->event_idx = 0; + + defer_call_end(); } static void qemu_laio_process_completions_and_submit(LinuxAioState *s) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 309038fd46..5eb1f91b41 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -15,6 +15,7 @@ #include "qapi/error.h" #include "qapi/qapi-commands-virtio.h" #include "trace.h" +#include "qemu/defer-call.h" #include "qemu/error-report.h" #include "qemu/log.h" #include "qemu/main-loop.h" @@ -28,6 +29,7 @@ #include "hw/virtio/virtio-bus.h" #include "hw/qdev-properties.h" #include "hw/virtio/virtio-access.h" +#include "sysemu/block-backend.h" #include "sysemu/dma.h" #include "sysemu/runstate.h" #include "virtio-qmp.h" @@ -2426,6 +2428,13 @@ static bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq) } } +/* Batch irqs while inside a defer_call_begin()/defer_call_end() section */ +static void virtio_notify_irqfd_deferred_fn(void *opaque) +{ + EventNotifier *notifier = opaque; + event_notifier_set(notifier); +} + void virtio_notify_irqfd(VirtIODevice *vdev, VirtQueue *vq) { WITH_RCU_READ_LOCK_GUARD() { @@ -2452,7 +2461,7 @@ void virtio_notify_irqfd(VirtIODevice *vdev, VirtQueue *vq) * to an atomic operation. */ virtio_set_isr(vq->vdev, 0x1); - event_notifier_set(&vq->guest_notifier); + defer_call(virtio_notify_irqfd_deferred_fn, &vq->guest_notifier); } static void virtio_irq(VirtQueue *vq) diff --git a/util/thread-pool.c b/util/thread-pool.c index e3d8292d14..d84961779a 100644 --- a/util/thread-pool.c +++ b/util/thread-pool.c @@ -15,6 +15,7 @@ * GNU GPL, version 2 or (at your option) any later version. */ #include "qemu/osdep.h" +#include "qemu/defer-call.h" #include "qemu/queue.h" #include "qemu/thread.h" #include "qemu/coroutine.h" @@ -175,6 +176,8 @@ static void thread_pool_completion_bh(void *opaque) ThreadPool *pool = opaque; ThreadPoolElement *elem, *next; + defer_call_begin(); /* cb() may use defer_call() to coalesce work */ + restart: QLIST_FOREACH_SAFE(elem, &pool->head, all, next) { if (elem->state != THREAD_DONE) { @@ -208,6 +211,8 @@ restart: qemu_aio_unref(elem); } } + + defer_call_end(); } static void thread_pool_cancel(BlockAIOCB *acb)
virtio-blk and virtio-scsi invoke virtio_irqfd_notify() to send Used Buffer Notifications from an IOThread. This involves an eventfd write(2) syscall. Calling this repeatedly when completing multiple I/O requests in a row is wasteful. Use the defer_call() API to batch together virtio_irqfd_notify() calls made during thread pool (aio=threads), Linux AIO (aio=native), and io_uring (aio=io_uring) completion processing. Behavior is unchanged for emulated devices that do not use defer_call_begin()/defer_call_end() since defer_call() immediately invokes the callback when called outside a defer_call_begin()/defer_call_end() region. fio rw=randread bs=4k iodepth=64 numjobs=8 IOPS increases by ~9% with a single IOThread and 8 vCPUs. iodepth=1 decreases by ~1% but this could be noise. Detailed performance data and configuration specifics are available here: https://gitlab.com/stefanha/virt-playbooks/-/tree/blk_io_plug-irqfd This duplicates the BH that virtio-blk uses for batching. The next commit will remove it. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- block/io_uring.c | 6 ++++++ block/linux-aio.c | 4 ++++ hw/virtio/virtio.c | 11 ++++++++++- util/thread-pool.c | 5 +++++ 4 files changed, 25 insertions(+), 1 deletion(-)