monitor: do not use mb_read/mb_set

Instead of relying on magic memory barriers, document the pattern that
is being used.  It is the one based on Dekker's algorithm, and in this
case it is embodied as follows:

    enqueue request;              sleeping = true;
    smp_mb();                     smp_mb();
    if (sleeping) kick();         if (!have a request) yield();

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
Paolo Bonzini 2023-03-15 12:34:01 +01:00
parent 60f4f62efe
commit eea7cd3fc5

View file

@ -39,13 +39,16 @@
* coroutine never gets scheduled a second time when it's already * coroutine never gets scheduled a second time when it's already
* scheduled (scheduling the same coroutine twice is forbidden). * scheduled (scheduling the same coroutine twice is forbidden).
* *
* It is true if the coroutine is active and processing requests. * It is true if the coroutine will process at least one more request
* Additional requests may then be pushed onto mon->qmp_requests, * before going to sleep. Either it has been kicked already, or it
* and @qmp_dispatcher_co_shutdown may be set without further ado. * is active and processing requests. Additional requests may therefore
* @qmp_dispatcher_co_busy must not be woken up in this case. * be pushed onto mon->qmp_requests, and @qmp_dispatcher_co_shutdown may
* be set without further ado. @qmp_dispatcher_co must not be woken up
* in this case.
* *
* If false, you also have to set @qmp_dispatcher_co_busy to true and * If false, you have to wake up @qmp_dispatcher_co after pushing new
* wake up @qmp_dispatcher_co after pushing the new requests. * requests. You also have to set @qmp_dispatcher_co_busy to true
* before waking up the coroutine.
* *
* The coroutine will automatically change this variable back to false * The coroutine will automatically change this variable back to false
* before it yields. Nobody else may set the variable to false. * before it yields. Nobody else may set the variable to false.
@ -230,15 +233,18 @@ static QMPRequest *monitor_qmp_dispatcher_pop_any(void)
{ {
while (true) { while (true) {
/* /*
* busy must be set to true again by whoever * To avoid double scheduling, busy is true on entry to
* rescheduled us to avoid double scheduling * monitor_qmp_dispatcher_co(), and must be set again before
* aio_co_wake()-ing it.
*/ */
assert(qatomic_mb_read(&qmp_dispatcher_co_busy) == true); assert(qatomic_read(&qmp_dispatcher_co_busy) == true);
/* /*
* Mark the dispatcher as not busy already here so that we * Mark the dispatcher as not busy already here so that we
* don't miss any new requests coming in the middle of our * don't miss any new requests coming in the middle of our
* processing. * processing.
*
* Clear qmp_dispatcher_co_busy before reading request.
*/ */
qatomic_mb_set(&qmp_dispatcher_co_busy, false); qatomic_mb_set(&qmp_dispatcher_co_busy, false);
@ -364,6 +370,9 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
void qmp_dispatcher_co_wake(void) void qmp_dispatcher_co_wake(void)
{ {
/* Write request before reading qmp_dispatcher_co_busy. */
smp_mb__before_rmw();
if (!qatomic_xchg(&qmp_dispatcher_co_busy, true)) { if (!qatomic_xchg(&qmp_dispatcher_co_busy, true)) {
aio_co_wake(qmp_dispatcher_co); aio_co_wake(qmp_dispatcher_co);
} }