Re: [PATCH 4/5] migration: Maintain postcopy faulted addresses

[Peter Xu]

On Tue, Sep 08, 2020 at 12:00:39PM +0100, Dr. David Alan Gilbert wrote:
> > @@ -238,6 +247,11 @@ void migration_incoming_state_destroy(void)
> >          mis->postcopy_remote_fds = NULL;
> >      }
> >  
> > +    if (mis->page_requested) {
> > +        g_tree_destroy(mis->page_requested);
> > +        mis->page_requested = NULL;
> > +    }
> > +
> 
> I think you want a:
>        qemu_mutex_destroy(&current_incoming->page_request_mutex);

I explicitly didn't do that because I saw that we've got quite a few things
that were not destroyed here, just in case I introduce some bug on multi-free
of the mutex.  However... after a closer look, I don't see a reason to not free
them at all...  Namely:

    - postcopy_pause_sem_dst
    - postcopy_pause_sem_fault
    - rp_mutex
    - main_thread_load_event (instead of _reset it in this function, we might
      want to use _destroy)

I'll prepare another standalone patch for that.

> 
> >      qemu_event_reset(&mis->main_thread_load_event);
> >  
> >      if (mis->socket_address_list) {
> > @@ -354,8 +368,33 @@ int 
> > migrate_send_rp_message_req_pages(MigrationIncomingState *mis,
> >  }
> >  
> >  int migrate_send_rp_req_pages(MigrationIncomingState *mis,
> > -                              RAMBlock *rb, ram_addr_t start)
> > +                              RAMBlock *rb, ram_addr_t start, uint64_t 
> > haddr)
> >  {
> > +    uint64_t aligned = haddr & (-qemu_target_page_size());
> > +    bool received;
> > +
> > +    qemu_mutex_lock(&mis->page_request_mutex);
> 
> You could use WITH_QEMU_LOCK_GUARD

Sure.

Thanks,

-- 
Peter Xu