[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 5/6] hmp: convert handle_hmp_command() to AIO_WAIT_WHILE_UNLO
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH 5/6] hmp: convert handle_hmp_command() to AIO_WAIT_WHILE_UNLOCKED() |
Date: |
Thu, 2 Mar 2023 08:22:05 -0500 |
On Thu, Mar 02, 2023 at 08:17:43AM +0100, Markus Armbruster wrote:
> Stefan Hajnoczi <stefanha@redhat.com> writes:
>
> > The HMP monitor runs in the main loop thread. Calling
>
> Correct.
>
> > AIO_WAIT_WHILE(qemu_get_aio_context(), ...) from the main loop thread is
> > equivalent to AIO_WAIT_WHILE_UNLOCKED(NULL, ...) because neither unlocks
> > the AioContext and the latter's assertion that we're in the main loop
> > succeeds.
> >
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> > monitor/hmp.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/monitor/hmp.c b/monitor/hmp.c
> > index 2aa85d3982..5ecbdac802 100644
> > --- a/monitor/hmp.c
> > +++ b/monitor/hmp.c
> > @@ -1167,7 +1167,7 @@ void handle_hmp_command(MonitorHMP *mon, const char
> > *cmdline)
> > Coroutine *co = qemu_coroutine_create(handle_hmp_command_co,
> > &data);
> > monitor_set_cur(co, &mon->common);
> > aio_co_enter(qemu_get_aio_context(), co);
> > - AIO_WAIT_WHILE(qemu_get_aio_context(), !data.done);
> > + AIO_WAIT_WHILE_UNLOCKED(NULL, !data.done);
> > }
> >
> > qobject_unref(qdict);
>
> Acked-by: Markus Armbruster <armbru@redhat.com>
>
> For an R-by, I need to understand this in more detail. I'm not familiar
> with the innards of AIO_WAIT_WHILE() & friends, so I need to go real
> slow.
>
> We change
>
> ctx from qemu_get_aio_context() to NULL
> unlock from true to false
>
> in
>
> bool waited_ = false; \
> AioWait *wait_ = &global_aio_wait; \
> AioContext *ctx_ = (ctx); \
> /* Increment wait_->num_waiters before evaluating cond. */ \
> qatomic_inc(&wait_->num_waiters); \
> /* Paired with smp_mb in aio_wait_kick(). */ \
> smp_mb(); \
> if (ctx_ && in_aio_context_home_thread(ctx_)) { \
> while ((cond)) { \
> aio_poll(ctx_, true); \
> waited_ = true; \
> } \
> } else { \
> assert(qemu_get_current_aio_context() == \
> qemu_get_aio_context()); \
> while ((cond)) { \
> if (unlock && ctx_) { \
> aio_context_release(ctx_); \
> } \
> aio_poll(qemu_get_aio_context(), true); \
> if (unlock && ctx_) { \
> aio_context_acquire(ctx_); \
> } \
> waited_ = true; \
> } \
> } \
> qatomic_dec(&wait_->num_waiters); \
> waited_; })
>
> qemu_get_aio_context() is non-null here, correct?
qemu_get_aio_context() always returns the main loop thread's AioContext.
qemu_get_current_aio_context() returns the AioContext that was most
recently set in the my_aiocontext thread-local variable for IOThreads,
the main loop's AioContext for BQL threads, or NULL for threads
that don't use AioContext at all.
> What's the value of in_aio_context_home_thread(qemu_get_aio_context())?
This function checks whether the given AioContext is associated with
this thread. In a BQL thread it returns true if the context is the main
loop's AioContext. In an IOThread it returns true if the context is the
IOThread's AioContext. Otherwise it returns false.
signature.asc
Description: PGP signature
- Re: [PATCH 1/6] block: don't acquire AioContext lock in bdrv_drain_all(), (continued)
[PATCH 3/6] block: convert bdrv_graph_wrlock() to AIO_WAIT_WHILE_UNLOCKED(), Stefan Hajnoczi, 2023/03/01
[PATCH 4/6] block: convert bdrv_drain_all_begin() to AIO_WAIT_WHILE_UNLOCKED(), Stefan Hajnoczi, 2023/03/01
[PATCH 5/6] hmp: convert handle_hmp_command() to AIO_WAIT_WHILE_UNLOCKED(), Stefan Hajnoczi, 2023/03/01
Re: [PATCH 5/6] hmp: convert handle_hmp_command() to AIO_WAIT_WHILE_UNLOCKED(), Philippe Mathieu-Daudé, 2023/03/07
[PATCH 6/6] monitor: convert monitor_cleanup() to AIO_WAIT_WHILE_UNLOCKED(), Stefan Hajnoczi, 2023/03/01
Re: [PATCH 0/6] block: switch to AIO_WAIT_WHILE_UNLOCKED() where possible, Kevin Wolf, 2023/03/07