Re: [PATCH 2/2] aio-posix: disable fdmon-io_uring when GSource is used

[Oleksandr Natalenko]

On Mon, May 11, 2020 at 07:36:30PM +0100, Stefan Hajnoczi wrote:
> The glib event loop does not call fdmon_io_uring_wait() so fd handlers
> waiting to be submitted build up in the list. There is no benefit is
> using io_uring when the glib GSource is being used, so disable it
> instead of implementing a more complex fix.
> 
> This fixes a memory leak where AioHandlers would build up and increasing
> amounts of CPU time were spent iterating them in aio_pending(). The
> symptom is that guests become slow when QEMU is built with io_uring
> support.
> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1877716
> Fixes: 73fd282e7b6dd4e4ea1c3bbb3d302c8db51e4ccf ("aio-posix: add io_uring fd 
> monitoring implementation")
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
>  include/block/aio.h |  3 +++
>  util/aio-posix.c    | 12 ++++++++++++
>  util/aio-win32.c    |  4 ++++
>  util/async.c        |  1 +
>  4 files changed, 20 insertions(+)
> 
> diff --git a/include/block/aio.h b/include/block/aio.h
> index 62ed954344..b2f703fa3f 100644
> --- a/include/block/aio.h
> +++ b/include/block/aio.h
> @@ -701,6 +701,9 @@ void aio_context_setup(AioContext *ctx);
>   */
>  void aio_context_destroy(AioContext *ctx);
>  
> +/* Used internally, do not call outside AioContext code */
> +void aio_context_use_g_source(AioContext *ctx);
> +
>  /**
>   * aio_context_set_poll_params:
>   * @ctx: the aio context
> diff --git a/util/aio-posix.c b/util/aio-posix.c
> index 8af334ab19..1b2a3af65b 100644
> --- a/util/aio-posix.c
> +++ b/util/aio-posix.c
> @@ -682,6 +682,18 @@ void aio_context_destroy(AioContext *ctx)
>      aio_free_deleted_handlers(ctx);
>  }
>  
> +void aio_context_use_g_source(AioContext *ctx)
> +{
> +    /*
> +     * Disable io_uring when the glib main loop is used because it doesn't
> +     * support mixed glib/aio_poll() usage. It relies on aio_poll() being
> +     * called regularly so that changes to the monitored file descriptors are
> +     * submitted, otherwise a list of pending fd handlers builds up.
> +     */
> +    fdmon_io_uring_destroy(ctx);
> +    aio_free_deleted_handlers(ctx);
> +}
> +
>  void aio_context_set_poll_params(AioContext *ctx, int64_t max_ns,
>                                   int64_t grow, int64_t shrink, Error **errp)
>  {
> diff --git a/util/aio-win32.c b/util/aio-win32.c
> index 729d533faf..953c56ab48 100644
> --- a/util/aio-win32.c
> +++ b/util/aio-win32.c
> @@ -414,6 +414,10 @@ void aio_context_destroy(AioContext *ctx)
>  {
>  }
>  
> +void aio_context_use_g_source(AioContext *ctx)
> +{
> +}
> +
>  void aio_context_set_poll_params(AioContext *ctx, int64_t max_ns,
>                                   int64_t grow, int64_t shrink, Error **errp)
>  {
> diff --git a/util/async.c b/util/async.c
> index 3165a28f2f..1319eee3bc 100644
> --- a/util/async.c
> +++ b/util/async.c
> @@ -362,6 +362,7 @@ static GSourceFuncs aio_source_funcs = {
>  
>  GSource *aio_get_g_source(AioContext *ctx)
>  {
> +    aio_context_use_g_source(ctx);
>      g_source_ref(&ctx->source);
>      return &ctx->source;
>  }

Tested-by: Oleksandr Natalenko <address@hidden>

(run Windows 10 VM with storage accessible via io_uring on qemu v5.0.0
with these 2 patches)

Thank you.

-- 
  Best regards,
    Oleksandr Natalenko (post-factum)
    Principal Software Maintenance Engineer