[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH for-2.11 1/4] Revert "coroutine: abort if we try

From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH for-2.11 1/4] Revert "coroutine: abort if we try to schedule or enter a pending coroutine"
Date: Tue, 28 Nov 2017 18:01:32 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0

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

          '---> 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:
          '---> 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...


reply via email to

[Prev in Thread] Current Thread [Next in Thread]