Re: [PATCH] migrate/multifd: fix coredump when the multifd thread cleanup

[Peter Xu]

On Wed, Jun 21, 2023 at 04:18:26PM +0800, z00619469 via wrote:
> From: c00454449 <chenyuhui5@huawei.com>
> 
> There is a coredump while trying to destroy mutex when
> p->running is false but p->mutex is not unlock.
> Make sure all mutexes has been released before destroy them.

It'll be nice to add a backtrace of the coredump here, and also copy
maintainer (Juan Quintela, copied now).

> 
> Signed-off-by: c00454449 <chenyuhui5@huawei.com>
> ---
>  migration/multifd.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/migration/multifd.c b/migration/multifd.c
> index b7ad7002e0..7dcdb2d3a0 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -523,9 +523,7 @@ void multifd_save_cleanup(void)
>      for (i = 0; i < migrate_multifd_channels(); i++) {
>          MultiFDSendParams *p = &multifd_send_state->params[i];
>  
> -        if (p->running) {
> -            qemu_thread_join(&p->thread);
> -        }
> +        qemu_thread_join(&p->thread);

I'm not sure whether this will always work, e.g. when migration fails early
before creating multifd threads?

>      }
>      for (i = 0; i < migrate_multifd_channels(); i++) {
>          MultiFDSendParams *p = &multifd_send_state->params[i];
> @@ -1040,8 +1038,8 @@ int multifd_load_cleanup(Error **errp)
>               * however try to wakeup it without harm in cleanup phase.
>               */
>              qemu_sem_post(&p->sem_sync);
> -            qemu_thread_join(&p->thread);
>          }
> +        qemu_thread_join(&p->thread);
>      }
>      for (i = 0; i < migrate_multifd_channels(); i++) {
>          MultiFDRecvParams *p = &multifd_recv_state->params[i];
> -- 
> 2.21.0.windows.1
> 
> 

-- 
Peter Xu