On Wed, Jun 3, 2026 at 1:20 AM Markus Armbruster <[email protected]> wrote: > > Zhang Chen <[email protected]> writes: > > > Based on monitor ID tracking iothread users with holder. > > Introduce the AioContext in the Monitor struct to avoid repeated calls to > > iothread_get_aio_context() > > iothread_get_aio_context(iothread) returns iothread->ctx. I doubt > avoiding the call is worth the complexity yet another variable adds. > > > and ensure symmetrical ref/unref during > > monitor lifecycle. > > What do you mean by that?
As we discussed with Stefan before, the iothread_ref/unref need to call in pairs, for example the virtio-blk. But the monitor implementation is special, it have the monitor_suspend/resume with iothread. So I try to introduce the mon->ctx to simplify it. > > > Signed-off-by: Zhang Chen <[email protected]> > > --- > > monitor/monitor-internal.h | 3 +++ > > monitor/monitor.c | 20 ++++++++++++++++++-- > > monitor/qmp.c | 3 ++- > > 3 files changed, 23 insertions(+), 3 deletions(-) > > > > diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h > > index 2060311b03..92d7630921 100644 > > --- a/monitor/monitor-internal.h > > +++ b/monitor/monitor-internal.h > > @@ -127,6 +127,9 @@ struct Monitor { > > guint out_watch; > > int mux_out; > > int reset_seen; > > + > > + /* iothread context */ > > + AioContext *ctx; > > }; > > > > struct MonitorHMP { > > diff --git a/monitor/monitor.c b/monitor/monitor.c > > index dafb4ad8b0..7eec6f967d 100644 > > --- a/monitor/monitor.c > > +++ b/monitor/monitor.c > > @@ -529,7 +529,7 @@ int monitor_suspend(Monitor *mon) > > * Kick I/O thread to make sure this takes effect. It'll be > > * evaluated again in prepare() of the watch object. > > */ > > - aio_notify(iothread_get_aio_context(mon_iothread)); > > + aio_notify(mon->ctx); > > } > > > > trace_monitor_suspend(mon, 1); > > @@ -564,7 +564,7 @@ void monitor_resume(Monitor *mon) > > AioContext *ctx; > > > > if (mon->use_io_thread) { > > - ctx = iothread_get_aio_context(mon_iothread); > > + ctx = mon->ctx; > > } else { > > ctx = qemu_get_aio_context(); > > } > > @@ -612,6 +612,12 @@ void monitor_data_init(Monitor *mon, bool is_qmp, bool > > skip_flush, > > { > > if (use_io_thread && !mon_iothread) { > > monitor_iothread_init(); > > + > > + IOThreadHolder holder = { > > + .type = IO_THREAD_HOLDER_KIND_MONITOR_NAME, > > + .u.monitor_name.monitor_name = (char *)mon->id, > > + }; > > + mon->ctx = iothread_ref_and_get_aio_context(mon_iothread, &holder); > > } > > qemu_mutex_init(&mon->mon_lock); > > mon->is_qmp = is_qmp; > > @@ -631,6 +637,16 @@ void monitor_data_destroy(Monitor *mon) > > } > > g_string_free(mon->outbuf, true); > > qemu_mutex_destroy(&mon->mon_lock); > > + > > + if (mon->ctx && mon_iothread) { > > + IOThreadHolder holder = { > > + .type = IO_THREAD_HOLDER_KIND_MONITOR_NAME, > > + .u.monitor_name.monitor_name = (char *)mon->id, > > + }; > > + > > + iothread_put_aio_context(mon_iothread, &holder); > > + mon->ctx = NULL; > > + } > > } > > > > void monitor_cleanup(void) > > diff --git a/monitor/qmp.c b/monitor/qmp.c > > index b210850a15..b67674aef3 100644 > > --- a/monitor/qmp.c > > +++ b/monitor/qmp.c > > @@ -565,7 +565,8 @@ void monitor_init_qmp(Chardev *chr, bool pretty, const > > char *id, Error **errp) > > * since chardev might be running in the monitor I/O > > * thread. Schedule a bottom half. > > */ > > - aio_bh_schedule_oneshot(iothread_get_aio_context(mon_iothread), > > + Monitor *mon_p = &mon->common; > > + aio_bh_schedule_oneshot(mon_p->ctx, > > monitor_qmp_setup_handlers_bh, mon); > > /* The bottom half will add @mon to @mon_list */ > > } else { > > The patch does two things: > > (1) Replace iothread_get_aio_context(mon_iothread) by mon->ctx. > > (2) Call iothread_ref_and_get_aio_context() / iothread_put_aio_context(). > > I challenged (1). If we decide we want it, I'll ask you to split the > patch. It's OK for me~ Thanks Chen >
