Zhang Chen <[email protected]> writes:
> Introduce iothread_ref_and_get_aio_context() with a holder argument
> and its counterpart iothread_put_aio_context().
>
> Previously, users of an IOThread AioContext did not explicitly record
> their identity, making it difficult to debug which devices or
> subsystems were pinning an IOThread.
>
> Registering a holder takes an IOThread object reference so that the
> IOThread and its AioContext stay alive until the matching put
> operation. Document the ownership and BQL requirements.
>
> Signed-off-by: Zhang Chen <[email protected]>
> ---
> include/system/iothread.h | 21 +++++++++++++++++++++
> iothread.c | 23 ++++++++++++++++++++++-
> 2 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/include/system/iothread.h b/include/system/iothread.h
> index b8aeb32b0e..b6664e7a38 100644
> --- a/include/system/iothread.h
> +++ b/include/system/iothread.h
> @@ -67,6 +67,27 @@ DECLARE_INSTANCE_CHECKER(IOThread, IOTHREAD,
> char *iothread_get_id(IOThread *iothread);
> IOThread *iothread_by_id(const char *id);
> AioContext *iothread_get_aio_context(IOThread *iothread);
> +
> +/*
> + * Register @holder and return @iothread's AioContext. The holder is copied,
> + * and a reference is taken on @iothread so that both the IOThread and its
> + * AioContext remain alive.
> + *
> + * The caller must eventually call iothread_put_aio_context() with an
> + * equivalent holder. This function is not thread-safe and must be called
> + * under the Big QEMU Lock (BQL).
> + */
> +AioContext *iothread_ref_and_get_aio_context(IOThread *iothread,
> + const IOThreadHolder *holder);
> +
> +/*
> + * Unregister @holder and release the corresponding reference on @iothread.
> + * Calling this function without a matching
> + * iothread_ref_and_get_aio_context() call is a programming error.
> + *
> + * This function is not thread-safe and must be called under the BQL.
> + */
> +void iothread_put_aio_context(IOThread *iothread, const IOThreadHolder
> *holder);
> GMainContext *iothread_get_g_main_context(IOThread *iothread);
>
> /*
> diff --git a/iothread.c b/iothread.c
> index 2a4c92e08b..0cc8344ee6 100644
> --- a/iothread.c
> +++ b/iothread.c
> @@ -33,6 +33,11 @@ void iothread_ref(IOThread *iothread, const IOThreadHolder
> *holder)
> assert(holder);
>
> QAPI_LIST_PREPEND(iothread->holders, QAPI_CLONE(IOThreadHolder, holder));
> + /*
> + * This guarantees that the IOThread and its AioContext remain alive
> + * as long as there is a holder.
> + */
> + object_ref(OBJECT(iothread));
> }
>
> static int iothread_holder_compare(const IOThreadHolder *holder_a,
> @@ -78,6 +83,7 @@ void iothread_unref(IOThread *iothread, const
> IOThreadHolder *holder)
> *prev = curr->next;
> curr->next = NULL;
> qapi_free_IOThreadHolderList(curr);
> + object_unref(OBJECT(iothread));
> return;
> }
> prev = &curr->next;
> @@ -199,7 +205,7 @@ static void iothread_init_gcontext(IOThread *iothread,
> const char *thread_name)
> g_autofree char *name = g_strdup_printf("%s aio-context", thread_name);
>
> iothread->worker_context = g_main_context_new();
> - source = aio_get_g_source(iothread_get_aio_context(iothread));
> + source = aio_get_g_source(iothread->ctx);
> g_source_set_name(source, name);
> g_source_attach(source, iothread->worker_context);
> g_source_unref(source);
> @@ -421,6 +427,21 @@ AioContext *iothread_get_aio_context(IOThread *iothread)
> return iothread->ctx;
> }
>
> +AioContext *iothread_ref_and_get_aio_context(IOThread *iothread,
> + const IOThreadHolder *holder)
> +{
> + /* Add IOThreadHolder to the list */
> + iothread_ref(iothread, holder);
> +
> + return iothread->ctx;
> +}
> +
> +void iothread_put_aio_context(IOThread *iothread, const IOThreadHolder
> *holder)
Should we name this "unref" rather than "put"? Perhaps Stefan has an
opinion.
> +{
> + /* Delete IOThreadHolder from the list */
> + iothread_unref(iothread, holder);
> +}
> +
> static int query_one_iothread(Object *object, void *opaque)
> {
> IOThreadInfoList ***tail = opaque;
