Re: [PATCH 6/8] migration/multifd: merge thread-join and cleanup loops in multifd_recv_cleanup

[Fabiano Rosas <farosas@suse.de>]

Bin Guo <guobin@linux.alibaba.com> writes:

> multifd_recv_cleanup() iterates over all receive channels twice:
> one loop to join threads and another to clean up each channel.

ok

> Unlike the send side where all threads must be signalled before any is
> joined (to avoid deadlock),

Not super relevant what the send side does. It's just a similar piece of
code, that's all.

> on the receive side the threads are already
> terminated by multifd_recv_terminate_threads().  Each join simply
> waits for an already-terminated thread, so the join and cleanup for
> channel i are independent of channel j, and the two loops can safely
> be merged into one.
>

We do really need to join all threads before proceeding. There's no
guarantee on the the order in which they'll terminate.

> This cuts the iteration count in half and improves locality: the
> thread's resources are freed immediately after its join.
>
> Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
> ---
>  migration/multifd.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/migration/multifd.c b/migration/multifd.c
> index b3eef875cc..67ee9bdf5e 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -1153,15 +1153,14 @@ void multifd_recv_cleanup(void)
>          return;
>      }
>      multifd_recv_terminate_threads(NULL);
> +
>      for (i = 0; i < migrate_multifd_channels(); i++) {
>          MultiFDRecvParams *p = &multifd_recv_state->params[i];
>  
>          if (p->thread_created) {
>              qemu_thread_join(&p->thread);
>          }
> -    }
> -    for (i = 0; i < migrate_multifd_channels(); i++) {
> -        multifd_recv_cleanup_channel(&multifd_recv_state->params[i]);
> +        multifd_recv_cleanup_channel(p);
>      }
>      multifd_recv_cleanup_state();
>  }