Re: [PATCH v2 2/4] util/defer-call: move defer_call() to util/
On Fri, Aug 18, 2023 at 10:31:40AM +0200, Philippe Mathieu-Daudé wrote: > Hi Stefan, > > On 17/8/23 17:58, Stefan Hajnoczi wrote: > > The networking subsystem may wish to use defer_call(), so move the code > > to util/ where it can be reused. > > > > As a reminder of what defer_call() does: > > > > This API defers a function call within a defer_call_begin()/defer_call_end() > > section, allowing multiple calls to batch up. This is a performance > > optimization that is used in the block layer to submit several I/O requests > > at once instead of individually: > > > >defer_call_begin(); <-- start of section > >... > >defer_call(my_func, my_obj); <-- deferred my_func(my_obj) call > >defer_call(my_func, my_obj); <-- another > >defer_call(my_func, my_obj); <-- another > >... > >defer_call_end(); <-- end of section, my_func(my_obj) is called once > > > > Suggested-by: Ilya Maximets > > Signed-off-by: Stefan Hajnoczi > > --- > > MAINTAINERS | 3 ++- > > include/qemu/defer-call.h | 15 +++ > > include/sysemu/block-backend-io.h | 4 > > block/blkio.c | 1 + > > block/io_uring.c | 1 + > > block/linux-aio.c | 1 + > > block/nvme.c | 1 + > > hw/block/dataplane/xen-block.c| 1 + > > hw/block/virtio-blk.c | 1 + > > hw/scsi/virtio-scsi.c | 1 + > > block/plug.c => util/defer-call.c | 2 +- > > block/meson.build | 1 - > > util/meson.build | 1 + > > 13 files changed, 26 insertions(+), 7 deletions(-) > > create mode 100644 include/qemu/defer-call.h > > rename block/plug.c => util/defer-call.c (99%) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 6111b6b4d9..7cd7132ffc 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -2676,12 +2676,13 @@ S: Supported > > F: util/async.c > > F: util/aio-*.c > > F: util/aio-*.h > > +F: util/defer-call.c > > If used by network/other backends, maybe worth adding a > brand new section instead, rather than "Block I/O path". Changes to defer-call.c will go through my block tree. We don't split out the event loop (async.c, aio-*.c, etc) either even though it's shared by other subsystems. The important thing is that scripts/get_maintainer.pl identifies the maintainers. I'd rather not create lots of micro-subsystems in MAINTAINERS that duplicate my email and block git repo URL. > > > F: util/fdmon-*.c > > F: block/io.c > > -F: block/plug.c > > F: migration/block* > > F: include/block/aio.h > > F: include/block/aio-wait.h > > +F: include/qemu/defer-call.h > > F: scripts/qemugdb/aio.py > > F: tests/unit/test-fdmon-epoll.c > > T: git https://github.com/stefanha/qemu.git block > > diff --git a/include/qemu/defer-call.h b/include/qemu/defer-call.h > > new file mode 100644 > > index 00..291f86c987 > > --- /dev/null > > +++ b/include/qemu/defer-call.h > > @@ -0,0 +1,15 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > +/* > > + * Deferred calls > > + * > > + * Copyright Red Hat. > > + */ > > + > > +#ifndef QEMU_DEFER_CALL_H > > +#define QEMU_DEFER_CALL_H > > + > > Please add smth like: > >/* See documentation in util/defer-call.c */ Sure, will fix. > > > +void defer_call_begin(void); > > +void defer_call_end(void); > > +void defer_call(void (*fn)(void *), void *opaque); > > + > > +#endif /* QEMU_DEFER_CALL_H */ > > Reviewed-by: Philippe Mathieu-Daudé > signature.asc Description: PGP signature
Re: [PATCH v2 2/4] util/defer-call: move defer_call() to util/
Hi Stefan, On 17/8/23 17:58, Stefan Hajnoczi wrote: The networking subsystem may wish to use defer_call(), so move the code to util/ where it can be reused. As a reminder of what defer_call() does: This API defers a function call within a defer_call_begin()/defer_call_end() section, allowing multiple calls to batch up. This is a performance optimization that is used in the block layer to submit several I/O requests at once instead of individually: defer_call_begin(); <-- start of section ... defer_call(my_func, my_obj); <-- deferred my_func(my_obj) call defer_call(my_func, my_obj); <-- another defer_call(my_func, my_obj); <-- another ... defer_call_end(); <-- end of section, my_func(my_obj) is called once Suggested-by: Ilya Maximets Signed-off-by: Stefan Hajnoczi --- MAINTAINERS | 3 ++- include/qemu/defer-call.h | 15 +++ include/sysemu/block-backend-io.h | 4 block/blkio.c | 1 + block/io_uring.c | 1 + block/linux-aio.c | 1 + block/nvme.c | 1 + hw/block/dataplane/xen-block.c| 1 + hw/block/virtio-blk.c | 1 + hw/scsi/virtio-scsi.c | 1 + block/plug.c => util/defer-call.c | 2 +- block/meson.build | 1 - util/meson.build | 1 + 13 files changed, 26 insertions(+), 7 deletions(-) create mode 100644 include/qemu/defer-call.h rename block/plug.c => util/defer-call.c (99%) diff --git a/MAINTAINERS b/MAINTAINERS index 6111b6b4d9..7cd7132ffc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2676,12 +2676,13 @@ S: Supported F: util/async.c F: util/aio-*.c F: util/aio-*.h +F: util/defer-call.c If used by network/other backends, maybe worth adding a brand new section instead, rather than "Block I/O path". F: util/fdmon-*.c F: block/io.c -F: block/plug.c F: migration/block* F: include/block/aio.h F: include/block/aio-wait.h +F: include/qemu/defer-call.h F: scripts/qemugdb/aio.py F: tests/unit/test-fdmon-epoll.c T: git https://github.com/stefanha/qemu.git block diff --git a/include/qemu/defer-call.h b/include/qemu/defer-call.h new file mode 100644 index 00..291f86c987 --- /dev/null +++ b/include/qemu/defer-call.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Deferred calls + * + * Copyright Red Hat. + */ + +#ifndef QEMU_DEFER_CALL_H +#define QEMU_DEFER_CALL_H + Please add smth like: /* See documentation in util/defer-call.c */ +void defer_call_begin(void); +void defer_call_end(void); +void defer_call(void (*fn)(void *), void *opaque); + +#endif /* QEMU_DEFER_CALL_H */ Reviewed-by: Philippe Mathieu-Daudé
[PATCH v2 2/4] util/defer-call: move defer_call() to util/
The networking subsystem may wish to use defer_call(), so move the code to util/ where it can be reused. As a reminder of what defer_call() does: This API defers a function call within a defer_call_begin()/defer_call_end() section, allowing multiple calls to batch up. This is a performance optimization that is used in the block layer to submit several I/O requests at once instead of individually: defer_call_begin(); <-- start of section ... defer_call(my_func, my_obj); <-- deferred my_func(my_obj) call defer_call(my_func, my_obj); <-- another defer_call(my_func, my_obj); <-- another ... defer_call_end(); <-- end of section, my_func(my_obj) is called once Suggested-by: Ilya Maximets Signed-off-by: Stefan Hajnoczi --- MAINTAINERS | 3 ++- include/qemu/defer-call.h | 15 +++ include/sysemu/block-backend-io.h | 4 block/blkio.c | 1 + block/io_uring.c | 1 + block/linux-aio.c | 1 + block/nvme.c | 1 + hw/block/dataplane/xen-block.c| 1 + hw/block/virtio-blk.c | 1 + hw/scsi/virtio-scsi.c | 1 + block/plug.c => util/defer-call.c | 2 +- block/meson.build | 1 - util/meson.build | 1 + 13 files changed, 26 insertions(+), 7 deletions(-) create mode 100644 include/qemu/defer-call.h rename block/plug.c => util/defer-call.c (99%) diff --git a/MAINTAINERS b/MAINTAINERS index 6111b6b4d9..7cd7132ffc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2676,12 +2676,13 @@ S: Supported F: util/async.c F: util/aio-*.c F: util/aio-*.h +F: util/defer-call.c F: util/fdmon-*.c F: block/io.c -F: block/plug.c F: migration/block* F: include/block/aio.h F: include/block/aio-wait.h +F: include/qemu/defer-call.h F: scripts/qemugdb/aio.py F: tests/unit/test-fdmon-epoll.c T: git https://github.com/stefanha/qemu.git block diff --git a/include/qemu/defer-call.h b/include/qemu/defer-call.h new file mode 100644 index 00..291f86c987 --- /dev/null +++ b/include/qemu/defer-call.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Deferred calls + * + * Copyright Red Hat. + */ + +#ifndef QEMU_DEFER_CALL_H +#define QEMU_DEFER_CALL_H + +void defer_call_begin(void); +void defer_call_end(void); +void defer_call(void (*fn)(void *), void *opaque); + +#endif /* QEMU_DEFER_CALL_H */ diff --git a/include/sysemu/block-backend-io.h b/include/sysemu/block-backend-io.h index cfcfd85c1d..d174275a5c 100644 --- a/include/sysemu/block-backend-io.h +++ b/include/sysemu/block-backend-io.h @@ -100,10 +100,6 @@ void blk_iostatus_set_err(BlockBackend *blk, int error); int blk_get_max_iov(BlockBackend *blk); int blk_get_max_hw_iov(BlockBackend *blk); -void defer_call_begin(void); -void defer_call_end(void); -void defer_call(void (*fn)(void *), void *opaque); - AioContext *blk_get_aio_context(BlockBackend *blk); BlockAcctStats *blk_get_stats(BlockBackend *blk); void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk, diff --git a/block/blkio.c b/block/blkio.c index 7cf6d61f47..0a0a6c0f5f 100644 --- a/block/blkio.c +++ b/block/blkio.c @@ -13,6 +13,7 @@ #include "block/block_int.h" #include "exec/memory.h" #include "exec/cpu-common.h" /* for qemu_ram_get_fd() */ +#include "qemu/defer-call.h" #include "qapi/error.h" #include "qemu/error-report.h" #include "qapi/qmp/qdict.h" diff --git a/block/io_uring.c b/block/io_uring.c index 8429f341be..3a1e1f45b3 100644 --- a/block/io_uring.c +++ b/block/io_uring.c @@ -15,6 +15,7 @@ #include "block/block.h" #include "block/raw-aio.h" #include "qemu/coroutine.h" +#include "qemu/defer-call.h" #include "qapi/error.h" #include "sysemu/block-backend.h" #include "trace.h" diff --git a/block/linux-aio.c b/block/linux-aio.c index 9a08219db0..62380593c8 100644 --- a/block/linux-aio.c +++ b/block/linux-aio.c @@ -14,6 +14,7 @@ #include "block/raw-aio.h" #include "qemu/event_notifier.h" #include "qemu/coroutine.h" +#include "qemu/defer-call.h" #include "qapi/error.h" #include "sysemu/block-backend.h" diff --git a/block/nvme.c b/block/nvme.c index dfbd1085fd..96b3f8f2fa 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -16,6 +16,7 @@ #include "qapi/error.h" #include "qapi/qmp/qdict.h" #include "qapi/qmp/qstring.h" +#include "qemu/defer-call.h" #include "qemu/error-report.h" #include "qemu/main-loop.h" #include "qemu/module.h" diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c index e9dd8f8a99..c4bb28c66f 100644 --- a/hw/block/dataplane/xen-block.c +++ b/hw/block/dataplane/xen-block.c @@ -19,6 +19,7 @@ */ #include "qemu/osdep.h" +#include "qemu/defer-call.h" #include "qemu/error-report.h" #include "qemu/main-loop.h" #include "qemu/memalign.h" diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 6a45033d15..a1f8e15522 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -12,6 +12,7 @@ */
