All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: stefanha@redhat.com, marcandre.lureau@gmail.com,
	qemu-devel@nongnu.org, qemu-block@nongnu.org,
	dgilbert@redhat.com
Subject: Re: [PATCH v7 07/13] monitor: Make current monitor a per-coroutine property
Date: Mon, 28 Sep 2020 14:21:16 +0200	[thread overview]
Message-ID: <878scugkhv.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <20200928104214.GE5451@linux.fritz.box> (Kevin Wolf's message of "Mon, 28 Sep 2020 12:42:14 +0200")

Kevin Wolf <kwolf@redhat.com> writes:

> Am 28.09.2020 um 09:47 hat Markus Armbruster geschrieben:
>> Kevin Wolf <kwolf@redhat.com> writes:
>> 
>> > Am 14.09.2020 um 17:11 hat Markus Armbruster geschrieben:
>> >> 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 think with human-monitor-command, you can have three mappings:
>> >
>> > 1. The main thread's leader (it is a non-coroutine QMP command) to the
>> >    QMP monitor
>> 
>> This is (QMP).
>> 
>> > 2. With a coroutine HMP command, one mapping from the transient HMP
>> >    coroutine to the transient HMP monitor (with a non-coroutine HMP
>> >    command, we'd instead temporarily change the mapping from 1.)
>> 
>> This is (HMP-CO).
>> 
>> > 3. The OOB entry
>> 
>> This is (OOB).
>> 
>> To get 1. (QMP) and 2, (HMP-CO) at the same time, the in-band,
>> non-coroutine QMP command needs to execute interleaved with the in-band,
>> coroutine HMP command.
>> 
>> Such an interleaving contradicts "In-band execution is one command after
>> the other", which is a fundamental assumption in-band commands may make.
>> If the assumption is invalid, we got a problem.  Is it?
>
> Interleaving, or rather executing another command in the middle of its
> implementation is the very purpose of human-monitor-command (which is
> what I was talking about, so "the in-band non-coroutine QMP command" is
> a very specific one).

Got it now, thanks.

> It's the only command I can think of that is exceptional in this way
> and would lead to three mappings.
>
> Kevin



  reply	other threads:[~2020-09-28 12:22 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-09 15:11 [PATCH v7 00/13] monitor: Optionally run handlers in coroutines Kevin Wolf
2020-09-09 15:11 ` [PATCH v7 01/13] monitor: Add Monitor parameter to monitor_set_cpu() Kevin Wolf
2020-09-09 15:11 ` [PATCH v7 02/13] monitor: Add Monitor parameter to monitor_get_cpu_index() Kevin Wolf
2020-09-09 15:11 ` [PATCH v7 03/13] monitor: Use getter/setter functions for cur_mon Kevin Wolf
2020-10-02  7:51   ` Markus Armbruster
2020-09-09 15:11 ` [PATCH v7 04/13] hmp: Update current monitor only in handle_hmp_command() Kevin Wolf
2020-09-09 15:11 ` [PATCH v7 05/13] qmp: Assert that no other monitor is active Kevin Wolf
2020-09-09 15:11 ` [PATCH v7 06/13] qmp: Call monitor_set_cur() only in qmp_dispatch() Kevin Wolf
2020-09-14 15:10   ` Markus Armbruster
2020-09-25 15:13     ` Kevin Wolf
2020-09-28 11:42       ` Markus Armbruster
2020-09-28 14:30         ` Kevin Wolf
2020-09-30  9:26           ` Markus Armbruster
2020-09-30 11:29             ` Kevin Wolf
2020-09-30 13:14               ` Markus Armbruster
2020-09-30 14:00                 ` Kevin Wolf
2020-09-30 17:20                   ` Dr. David Alan Gilbert
2020-10-01 10:14                     ` Kevin Wolf
2020-10-01 16:00                       ` Markus Armbruster
2020-10-02  8:04                         ` Markus Armbruster
2020-09-09 15:11 ` [PATCH v7 07/13] monitor: Make current monitor a per-coroutine property Kevin Wolf
2020-09-14 15:11   ` Markus Armbruster
2020-09-25 15:23     ` Kevin Wolf
2020-09-28  7:47       ` Markus Armbruster
2020-09-28 10:42         ` Kevin Wolf
2020-09-28 12:21           ` Markus Armbruster [this message]
2020-10-02  7:53   ` Markus Armbruster
2020-09-09 15:11 ` [PATCH v7 08/13] qapi: Add a 'coroutine' flag for commands Kevin Wolf
2020-09-14 15:15   ` Markus Armbruster
2020-09-25 15:37     ` Kevin Wolf
2020-09-28  8:23       ` Markus Armbruster
2020-10-02  7:53   ` Markus Armbruster
2020-10-02  7:59   ` Markus Armbruster
2020-09-09 15:11 ` [PATCH v7 09/13] qmp: Move dispatcher to a coroutine Kevin Wolf
2020-09-14 15:30   ` Markus Armbruster
2020-09-25 15:38     ` Kevin Wolf
2020-09-28  8:24       ` Markus Armbruster
2020-10-02  8:01   ` Markus Armbruster
2020-09-09 15:11 ` [PATCH v7 10/13] hmp: Add support for coroutine command handlers Kevin Wolf
2020-09-16  9:46   ` Dr. David Alan Gilbert
2020-10-02  8:01   ` Markus Armbruster
2020-09-09 15:11 ` [PATCH v7 11/13] util/async: Add aio_co_reschedule_self() Kevin Wolf
2020-09-15 14:25   ` Stefan Hajnoczi
2020-10-02  8:01   ` Markus Armbruster
2020-09-09 15:11 ` [PATCH v7 12/13] block: Add bdrv_co_move_to_aio_context() Kevin Wolf
2020-09-15 14:31   ` Stefan Hajnoczi
2020-09-25 16:00     ` Kevin Wolf
2020-09-28  8:59       ` Stefan Hajnoczi
2020-09-28 10:21         ` Kevin Wolf
2020-09-09 15:11 ` [PATCH v7 13/13] block: Convert 'block_resize' to coroutine Kevin Wolf
2020-09-15 14:57   ` Stefan Hajnoczi
2020-09-25 16:07     ` Kevin Wolf
2020-09-28  9:05       ` Stefan Hajnoczi
2020-09-28 10:33         ` Kevin Wolf
2020-09-09 15:24 ` [PATCH v7 00/13] monitor: Optionally run handlers in coroutines no-reply
2020-09-10 13:24 ` Stefan Hajnoczi
2020-09-14 15:09   ` Markus Armbruster
2020-09-15 14:58     ` Stefan Hajnoczi
2020-09-25 17:15   ` Kevin Wolf
2020-09-28  8:46     ` Stefan Hajnoczi
2020-09-28  9:47       ` Kevin Wolf
2020-09-28  9:30     ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878scugkhv.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=marcandre.lureau@gmail.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.