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
