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>
---
 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

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>

Reviewed-by: Juan Quintela <quintela@redhat.com>

queued

* 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