* Peter Xu (pet...@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 <pet...@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
although a comment above the reclaration of ram_save_host_pages saying
it can drop the lock would be veyr good.
Dave
> ---
> migration/ram.c | 41 ++++++++++++++++++++++++++++++-----------
> 1 file changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index b9ac2d6921..578ad8d70a 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2462,6 +2462,7 @@ static void postcopy_preempt_reset_channel(RAMState *rs)
> */
> static int ram_save_host_page(RAMState *rs, PageSearchStatus *pss)
> {
> + bool page_dirty, preempt_active = postcopy_preempt_active();
> int tmppages, pages = 0;
> size_t pagesize_bits =
> qemu_ram_pagesize(pss->block) >> TARGET_PAGE_BITS;
> @@ -2485,22 +2486,40 @@ static int ram_save_host_page(RAMState *rs,
> PageSearchStatus *pss)
> break;
> }
>
> - /* Check the pages is dirty and if it is send it */
> - if (migration_bitmap_clear_dirty(rs, pss->block, pss->page)) {
> - tmppages = ram_save_target_page(rs, pss);
> - if (tmppages < 0) {
> - return tmppages;
> - }
> + page_dirty = migration_bitmap_clear_dirty(rs, pss->block, pss->page);
>
> - pages += tmppages;
> + /* Check the pages is dirty and if it is send it */
> + if (page_dirty) {
> /*
> - * Allow rate limiting to happen in the middle of huge pages if
> - * something is sent in the current iteration.
> + * Properly yield the lock only in postcopy preempt mode
> + * because both migration thread and rp-return thread can
> + * operate on the bitmaps.
> */
> - if (pagesize_bits > 1 && tmppages > 0) {
> - migration_rate_limit();
> + if (preempt_active) {
> + qemu_mutex_unlock(&rs->bitmap_mutex);
> + }
> + tmppages = ram_save_target_page(rs, pss);
> + if (tmppages >= 0) {
> + pages += tmppages;
> + /*
> + * Allow rate limiting to happen in the middle of huge pages
> if
> + * something is sent in the current iteration.
> + */
> + if (pagesize_bits > 1 && tmppages > 0) {
> + migration_rate_limit();
> + }
> }
> + if (preempt_active) {
> + qemu_mutex_lock(&rs->bitmap_mutex);
> + }
> + } else {
> + tmppages = 0;
> }
> +
> + if (tmppages < 0) {
> + return tmppages;
> + }
> +
> pss->page = migration_bitmap_find_dirty(rs, pss->block, pss->page);
> } while ((pss->page < hostpage_boundary) &&
> offset_in_ramblock(pss->block,
> --
> 2.37.3
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
