Re: [PATCH] migration: Yield coroutine when receiving MIG_CMD_POSTCOPY_LISTEN

[Peter Xu]

On Wed, Apr 03, 2024 at 04:35:35PM +0800, Wang, Lei wrote:
> We should change the following line from
> 
>       while (!qemu_sem_timedwait(&mis->postcopy_qemufile_dst_done, 100)) {
> 
> to
> 
>       while (qemu_sem_timedwait(&mis->postcopy_qemufile_dst_done, 100)) {

Stupid me.. :(  Thanks for figuring this out.

> 
> After that fix, test passed and no segfault.
> 
> Given that the test shows a yield to the main loop won't introduce much 
> overhead
> (<1ms), how about first yield unconditionally, then we enter the while loop to
> wait for several ms and yield periodically?

Shouldn't the expectation be that this should return immediately without a
wait?  We're already processing LISTEN command, and on the source as you
said it was much after the connect().  It won't guarantee the ordering but
IIUC the majority should still have a direct hit?

What we can do though is reducing the 100ms timeout if you see that's
perhaps a risk of having too large a downtime when by accident.  We can
even do it in a tight loop here considering downtime is important, but to
provide an intermediate ground: how about 100ms -> 1ms poll?

If you agree (and also to Wei; please review this and comment if there's
any!), would you write up the commit log, fully test it in whatever way you
could, and resend as a formal patch (please do this before Friday if
possible)?  You can keep a "Suggested-by:" for me.  I want to queue it for
rc3 if it can catch it. It seems important if Wei can always reproduce it.

Thanks,

-- 
Peter Xu