Re: [PATCH 3/7] migration: Drop postcopy_chunk_hostpages()

[Dr. David Alan Gilbert]

* Peter Xu (peterx@redhat.com) wrote:
> This function calls three functions:
> 
>   - postcopy_discard_send_init(ms, block->idstr);
>   - postcopy_chunk_hostpages_pass(ms, block);
>   - postcopy_discard_send_finish(ms);
> 
> However only the 2nd function call is meaningful.  It's major role is to make
> sure dirty bits are applied in host-page-size granule, so there will be no
> partial dirty bits set for a whole host page if huge pages are used.
> 
> The 1st/3rd call are for latter when we want to send the disgard ranges.
> They're mostly no-op here besides some tracepoints (which are misleading!).
> 
> Drop them, then we can directly drop postcopy_chunk_hostpages() as a whole
> because we can call postcopy_chunk_hostpages_pass() directly.
> 
> There're still some nice comments above postcopy_chunk_hostpages() that 
> explain
> what it does.  Copy it over to the caller's site.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

Yeh, I think originally the idea was to send some of the messages during
the chunking

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

> ---
>  migration/ram.c | 33 +++++++--------------------------
>  1 file changed, 7 insertions(+), 26 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index fb8c1a887e..e3876181ab 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2576,30 +2576,6 @@ static void 
> postcopy_chunk_hostpages_pass(MigrationState *ms, RAMBlock *block)
>      }
>  }
>  
> -/**
> - * postcopy_chunk_hostpages: discard any partially sent host page
> - *
> - * Utility for the outgoing postcopy code.
> - *
> - * Discard any partially sent host-page size chunks, mark any partially
> - * dirty host-page size chunks as all dirty.  In this case the host-page
> - * is the host-page for the particular RAMBlock, i.e. it might be a huge page
> - *
> - * @ms: current migration state
> - * @block: block we want to work with
> - */
> -static void postcopy_chunk_hostpages(MigrationState *ms, RAMBlock *block)
> -{
> -    postcopy_discard_send_init(ms, block->idstr);
> -
> -    /*
> -     * Ensure that all partially dirty host pages are made fully dirty.
> -     */
> -    postcopy_chunk_hostpages_pass(ms, block);
> -
> -    postcopy_discard_send_finish(ms);
> -}
> -
>  /**
>   * ram_postcopy_send_discard_bitmap: transmit the discard bitmap
>   *
> @@ -2631,8 +2607,13 @@ int ram_postcopy_send_discard_bitmap(MigrationState 
> *ms)
>      rs->last_page = 0;
>  
>      RAMBLOCK_FOREACH_NOT_IGNORED(block) {
> -        /* Deal with TPS != HPS and huge pages */
> -        postcopy_chunk_hostpages(ms, block);
> +        /*
> +         * Deal with TPS != HPS and huge pages.  It discard any partially 
> sent
> +         * host-page size chunks, mark any partially dirty host-page size
> +         * chunks as all dirty.  In this case the host-page is the host-page
> +         * for the particular RAMBlock, i.e. it might be a huge page.
> +         */
> +        postcopy_chunk_hostpages_pass(ms, block);
>      }
>      trace_ram_postcopy_send_discard_bitmap();
>  
> -- 
> 2.32.0
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK