Re: [Qemu-devel] [RFC 1/2] migration: Stop postcopy fault thread before notifying

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Ilya Maximets (i.maximets@samsung.com) wrote:
> POSTCOPY_NOTIFY_INBOUND_END handlers will remove userfault fds
> from the postcopy_remote_fds array which could be still in
> use by the fault thread. Let's stop the thread before
> notification to avoid possible accessing wrong memory.

OK I think; since this is already in the cleanup we shouldn't
be getting faults anyway at that point.


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> Fixes: 46343570c06e ("vhost+postcopy: Wire up POSTCOPY_END notify")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>  migration/postcopy-ram.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 853d8b32ca..e5c02a32c5 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -533,6 +533,12 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
>      if (mis->have_fault_thread) {
>          Error *local_err = NULL;
>  
> +        /* Let the fault thread quit */
> +        atomic_set(&mis->fault_thread_quit, 1);
> +        postcopy_fault_thread_notify(mis);
> +        trace_postcopy_ram_incoming_cleanup_join();
> +        qemu_thread_join(&mis->fault_thread);
> +
>          if (postcopy_notify(POSTCOPY_NOTIFY_INBOUND_END, &local_err)) {
>              error_report_err(local_err);
>              return -1;
> @@ -541,11 +547,6 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
>          if (qemu_ram_foreach_migratable_block(cleanup_range, mis)) {
>              return -1;
>          }
> -        /* Let the fault thread quit */
> -        atomic_set(&mis->fault_thread_quit, 1);
> -        postcopy_fault_thread_notify(mis);
> -        trace_postcopy_ram_incoming_cleanup_join();
> -        qemu_thread_join(&mis->fault_thread);
>  
>          trace_postcopy_ram_incoming_cleanup_closeuf();
>          close(mis->userfault_fd);
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK