Re: [Qemu-devel] [PATCH for-2.11 1/4] Revert "coroutine: abort if we try to schedule or enter a pending coroutine"

[Paolo Bonzini]

On 28/11/2017 17:37, Kevin Wolf wrote:
>>
>> It can also conflict badly with another aio_co_schedule().  Your patch
>> here removes the assertion in this case, and patch 3 makes it easier to
>> get into the situation where two aio_co_schedule()s conflict with each
>> other.
> I don't see how they conflict. If the second aio_co_schedule() comes
> before the coroutine is actually entered, they are effectively simply
> merged into a single one. Which is exactly what was intended.

Suppose you have

        ctx->scheduled_coroutines
          |
          '---> co
                |
                '---> NULL

Then aio_co_schedule(ctx, co) does QSLIST_INSERT_HEAD_ATOMIC, which is

        /* On entry, ctx->scheduled_coroutines == co.  */
        co->next = ctx->scheduled_coroutines
        change ctx->scheduled_coroutines from co->next to co

This results in a loop:
        
        ctx->scheduled_coroutines
          |
          '---> co <-.
                |    |
                '----'

This is an obvious hang due to list corruption.  In other more
complicated cases it can skip a wakeup, which is a more subtle kind of
hang.  You can also get memory corruption if the coroutine terminates
(and is freed) before aio_co_schedule executes.

Basically, once you do aio_co_schedule or aio_co_wake the coroutine is
not any more yours.  It's owned by the context that will run it and you
should not do anything with it.

The same is true for co_aio_sleep_ns.  Just because in practice it works
(and it seemed like a clever idea at the time) it doesn't mean it *is* a
good idea...

Paolo