Re: [PATCH 2/2] migration/multifd: Allow to sync with sender threads only

[Peter Xu <peterx@redhat.com>]

On Thu, Dec 05, 2024 at 06:50:23PM -0300, Fabiano Rosas wrote:
> I think I remember now, what's needed is to release p->sem and wait on
> p->sem_sync (one in each of these loops). We don't need to set the
> pending_sync flag if it's not going to be used:
> 
> multifd_send_sync_main:
>     for () {
>         ...
>         if (remote_sync) {
>             assert(qatomic_read(&p->pending_sync) == false);
>             qatomic_set(&p->pending_sync, true);
>         }
>         qemu_sem_post(&p->sem);
>     }
>     for () {
>         ...
>         qemu_sem_wait(&multifd_send_state->channels_ready);
>         qemu_sem_wait(&p->sem_sync);
>     }
> 
> in multifd_send_thread:
> 
>         if (qatomic_load_acquire(&p->pending_job)) {
>             ...
>             qatomic_store_release(&p->pending_job, false);
>         } else if (qatomic_read(&p->pending_sync)) {
>             ...
>             p->flags = MULTIFD_FLAG_SYNC;
>             qatomic_set(&p->pending_sync, false);
>             qemu_sem_post(&p->sem_sync);
>         } else {

How do you trigger this "else" path at all, if without setting pending_sync
first?

>             qemu_sem_post(&p->sem_sync);
>         }
> 
> Is this clearer? Then we avoid the enum altogether, a boolean would
> suffice.

So one of us missed something. :)

In case if I missed it, a runnable patch would work to clarify.

-- 
Peter Xu