Re: [PATCH V11 03/15] iothread: track users with holder name

On Wed, Aug 12, 2026 at 3:19 AM Stefan Hajnoczi <[email protected]> wrote:
>
> On Tue, Aug 11, 2026 at 04:51:55PM +0800, Zhang Chen wrote:
> > On Fri, Aug 7, 2026 at 9:41 PM Markus Armbruster <[email protected]> wrote:
> > >
> > > 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.
> >
> > It's OK for me. The original code was designed to match the
> > "iothread_get_aio_context",
> > If Stefan has no objection， I will change the name to
> > "iothread_unref_aio_context".
>
> iothread_unref_and_put_aio_context() would be consistent with
> iothread_ref_and_get_aio_context() :)

Sure, it looks much better.

Thanks
Chen

>
> Stefan
>
> >
> > Thanks
> >
> > Chen
> >
> > >
> > > > +{
> > > > +    /* Delete IOThreadHolder from the list */
> > > > +    iothread_unref(iothread, holder);
> > > > +}
> > > > +
> > > >  static int query_one_iothread(Object *object, void *opaque)
> > > >  {
> > > >      IOThreadInfoList ***tail = opaque;
> > >
> >