[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 07/13] monitor: Make current monitor a per-coroutine prope
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v7 07/13] monitor: Make current monitor a per-coroutine property |
Date: |
Mon, 14 Sep 2020 17:11:48 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
Kevin Wolf <kwolf@redhat.com> writes:
> This way, a monitor command handler will still be able to access the
> current monitor, but when it yields, all other code code will correctly
> get NULL from monitor_cur().
>
> This uses a hash table to map the coroutine pointer to the current
> monitor of that coroutine. Outside of coroutine context, we associate
> the current monitor with the leader coroutine of the current thread.
In qemu-system-FOO, the hash table can have only these entries:
* (OOB) One mapping @mon_iothread's thread leader to a QMP monitor, while
executing a QMP command out-of-band.
* (QMP-CO) One mapping @qmp_dispatcher_co (a coroutine in the main
thread) to a QMP monitor, while executing a QMP command in-band and in
coroutine context.
* (QMP) One mapping the main thread's leader to a QMP monitor, while
executing a QMP command in-band and out of coroutine context, in a
bottom half.
* (HMP) One mapping the main thread's leader to an HMP monitor, while
executing an HMP command out of coroutine context.
* (HMP-CO) One mapping a transient coroutine in the main thread to an
HMP monitor, while executing an HMP command in coroutine context.
In-band execution is one command after the other.
Therefore, at most one monitor command can be executing in-band at any
time.
Therefore, the hash table has at most *two* entries: one (OOB), and one
of the other four.
Can you shoot any holes into my argument?
I suspect there's a simpler solution struggling to get out. But this
solution works, so in it goes. Should the simpler one succeed at
getting out, it can go in on top. If not, I'll probably add even more
comments to remind myself of these facts.
> Approaches to implement some form of coroutine local storage directly in
> the coroutine core code have been considered and discarded because they
> didn't end up being much more generic than the hash table and their
> performance impact on coroutines not using coroutine local storage was
> unclear. As the block layer uses a coroutine per I/O request, this is a
> fast path and we have to be careful. It's safest to just stay out of
> this path with code only used by the monitor.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
- [PATCH v7 00/13] monitor: Optionally run handlers in coroutines, Kevin Wolf, 2020/09/09
- [PATCH v7 01/13] monitor: Add Monitor parameter to monitor_set_cpu(), Kevin Wolf, 2020/09/09
- [PATCH v7 02/13] monitor: Add Monitor parameter to monitor_get_cpu_index(), Kevin Wolf, 2020/09/09
- [PATCH v7 03/13] monitor: Use getter/setter functions for cur_mon, Kevin Wolf, 2020/09/09
- [PATCH v7 04/13] hmp: Update current monitor only in handle_hmp_command(), Kevin Wolf, 2020/09/09
- [PATCH v7 05/13] qmp: Assert that no other monitor is active, Kevin Wolf, 2020/09/09
- [PATCH v7 07/13] monitor: Make current monitor a per-coroutine property, Kevin Wolf, 2020/09/09
- Re: [PATCH v7 07/13] monitor: Make current monitor a per-coroutine property,
Markus Armbruster <=
[PATCH v7 06/13] qmp: Call monitor_set_cur() only in qmp_dispatch(), Kevin Wolf, 2020/09/09