Re: [Qemu-devel] [PATCH v6 5/7] qemu-coroutine-sleep: introduce qemu_co_sleep_wake

[Vladimir Sementsov-Ogievskiy]

07.06.2019 16:02, Kevin Wolf wrote:
> Am 07.06.2019 um 13:18 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> 07.06.2019 10:57, Kevin Wolf wrote:
>>> Am 11.04.2019 um 19:27 hat Vladimir Sementsov-Ogievskiy geschrieben:
>>>> Introduce a function to gracefully wake-up a coroutine, sleeping in
>>>> qemu_co_sleep_ns() sleep.
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>>>
>>> You can simply reenter the coroutine while it has yielded in
>>> qemu_co_sleep_ns(). This is supported.
>>
>> No it doesn't. qemu_aio_coroutine_enter checks for scheduled field,
>> and aborts if it is set.
> 
> Ah, yes, it has been broken since commit
> 
> I actually tried to fix it once, but it turned out more complicated and
> I think we found a different solution for the problem at hand:
> 
>      Subject: [PATCH for-2.11 0/4] Fix qemu-iotests failures
>      Message-Id: <address@hidden>
> 
> In this case, I guess your approach with a new function to interrupt
> qemu_co_sleep_ns() is okay.
> 
> Do we need to timer_del() when taking the shortcut? We don't necessarily
> reenter the coroutine immediately, but might only be scheduling it. In
> this case, the timer could fire before qemu_co_sleep_ns() has run and
> schedule the coroutine a second time

No it will not, as we do cmpxchg, scheduled to NULL, so second call will do
nothing..

But it seems unsafe, as even coroutine pointer may be stale when we call
qemu_co_sleep_wake second time. So, we possibly should remove timer, but ..

  (ignoring co->scheduled again -
> maybe we should actually not do that in the timer callback path, but
> instead let it run into the assertion because it would be a bug for the
> timer callback to end up in this situation).
> 
> Kevin
> 

Interesting, could there be a race condition, when we call qemu_co_sleep_wake,
but co_sleep_cb already scheduled in some queue and will run soon? Then removing
the timer will not help.


-- 
Best regards,
Vladimir