Re: [PATCH v2 2/4] util/defer-call: move defer_call() to util/

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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 <i.maximets@ovn.org>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   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 0000000000..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é <philmd@linaro.org>