Re: [PATCH v2 06/15] migration: Yield bitmap_mutex properly when sending/sleeping

[Dr. David Alan Gilbert]

* Peter Xu (peterx@redhat.com) wrote:
> On Wed, Oct 12, 2022 at 01:51:07PM -0400, Peter Xu wrote:
> > On Wed, Oct 12, 2022 at 05:43:53PM +0100, Dr. David Alan Gilbert wrote:
> > > * Peter Xu (peterx@redhat.com) wrote:
> > > > Don't take the bitmap mutex when sending pages, or when being throttled 
> > > > by
> > > > migration_rate_limit() (which is a bit tricky to call it here in ram 
> > > > code,
> > > > but seems still helpful).
> > > > 
> > > > It prepares for the possibility of concurrently sending pages in >1 
> > > > threads
> > > > using the function ram_save_host_page() because all threads may need the
> > > > bitmap_mutex to operate on bitmaps, so that either sendmsg() or any 
> > > > kind of
> > > > qemu_sem_wait() blocking for one thread will not block the other from
> > > > progressing.
> > > > 
> > > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > 
> > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > 
> > > although a comment above the reclaration of ram_save_host_pages saying
> > > it can drop the lock would be veyr good.
> > 
> > Let me add that.  Thanks,
> 
> A fixup to this patch attached to touch up the comment for
> ram_save_host_page().

Yep, that's right (I don't think we have any formal annotation for
locks)

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

> -- 
> Peter Xu

> From dcc3adce062df7216851890d49f7d2b1fa2e84a4 Mon Sep 17 00:00:00 2001
> From: Peter Xu <peterx@redhat.com>
> Date: Thu, 13 Oct 2022 12:18:04 -0400
> Subject: [PATCH] fixup! migration: Yield bitmap_mutex properly when
>  sending/sleeping
> Content-type: text/plain
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/ram.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 538667b974..b311ece48c 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2407,9 +2407,14 @@ out:
>   * a host page in which case the remainder of the hostpage is sent.
>   * Only dirty target pages are sent. Note that the host page size may
>   * be a huge page for this block.
> + *
>   * The saving stops at the boundary of the used_length of the block
>   * if the RAMBlock isn't a multiple of the host page size.
>   *
> + * The caller must be with ram_state.bitmap_mutex held to call this
> + * function.  Note that this function can temporarily release the lock, but
> + * when the function is returned it'll make sure the lock is still held.
> + *
>   * Returns the number of pages written or negative on error
>   *
>   * @rs: current RAM state
> -- 
> 2.37.3
> 

-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK