Re: [Qemu-devel] [PATCH for-2.11 3/4] coroutine: Cancel aio_co_schedule() on direct entry

[Paolo Bonzini]

On 28/11/2017 17:28, Kevin Wolf wrote:
> To be honest, I just wasn't sure what to do with this case anyway. It
> means that the coroutine is already running when someone else schedules
> it. We don't really know whether we have to enter it a second time or
> not.
>
> So if it can indeed happen in practice, we need to think a bit more
> about this.

>>> This means that the coroutine just needs to
>>> +     * prevent other callers from calling aio_co_schedule() before it 
>>> yields
>>> +     * (e.g. block job coroutines by setting job->busy = true).
>>> +     *
>>> +     * We still want to ensure that the second case doesn't happen, so 
>>> reset
>>> +     * co->scheduled only after setting co->caller to make the above check
>>> +     * effective for the co_schedule_bh_cb() case. */
>>> +    atomic_set(&co->scheduled, NULL);
>>
>> This doesn't work.  The coroutine is still in the list, and if someone
>> calls aio_co_schedule again now, any coroutines linked from "co" via
>> co_scheduled_next are lost.
> 
> Why would they? We still iterate the whole list in co_schedule_bh_cb(),
> we just skip the single qemu_coroutine_enter().

Because you modify co_scheduled_next (in QSLIST_INSERT_HEAD_ATOMIC)
before it has been consumed.

>> (Also, the AioContext lock is by design not protecting any state in
>> AioContext itself; the AioContext lock is only protecting things that
>> run in an AioContext but do not have their own lock).
> 
> Such as the coroutine we want to enter, no?

If you mean the Coroutine object then no, coroutines are entirely
thread-safe.  All you need to do is: 1) re-enter them with aio_co_wake
or aio_co_schedule after the first time you've entered them; 2) make
sure you don't enter/schedule them twice.

Paolo