[PATCH] migration/postcopy: not necessary to discard all RAM at the beginning

[PATCH] migration/postcopy: not necessary to discard all RAM at the beginning

[Wei Yang]

ram_discard_range() unmap page for specific range. To be specific, this
clears related page table entries so that userfault would be triggered.
But this step is not necessary at the very beginning.

ram_postcopy_incoming_init() is called when destination gets ADVISE
command. ADVISE command is sent when migration thread just starts, which
implies destination is not running yet. This means no page fault
happened and memory region's page tables entries are empty.

This patch removes the discard at the beginning.

Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
 migration/postcopy-ram.c | 46 ----------------------------------------
 migration/postcopy-ram.h |  7 ------
 migration/ram.c          | 16 --------------
 migration/ram.h          |  1 -
 migration/savevm.c       |  4 ----
 5 files changed, 74 deletions(-)

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 5da6de8c8b..459be8e780 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -443,32 +443,6 @@ out:
     return ret;
 }
 
-/*
- * Setup an area of RAM so that it *can* be used for postcopy later; this
- * must be done right at the start prior to pre-copy.
- * opaque should be the MIS.
- */
-static int init_range(RAMBlock *rb, void *opaque)
-{
-    const char *block_name = qemu_ram_get_idstr(rb);
-    void *host_addr = qemu_ram_get_host_addr(rb);
-    ram_addr_t offset = qemu_ram_get_offset(rb);
-    ram_addr_t length = qemu_ram_get_used_length(rb);
-    trace_postcopy_init_range(block_name, host_addr, offset, length);
-
-    /*
-     * We need the whole of RAM to be truly empty for postcopy, so things
-     * like ROMs and any data tables built during init must be zero'd
-     * - we're going to get the copy from the source anyway.
-     * (Precopy will just overwrite this data, so doesn't need the discard)
-     */
-    if (ram_discard_range(block_name, 0, length)) {
-        return -1;
-    }
-
-    return 0;
-}
-
 /*
  * At the end of migration, undo the effects of init_range
  * opaque should be the MIS.
@@ -506,20 +480,6 @@ static int cleanup_range(RAMBlock *rb, void *opaque)
     return 0;
 }
 
-/*
- * Initialise postcopy-ram, setting the RAM to a state where we can go into
- * postcopy later; must be called prior to any precopy.
- * called from arch_init's similarly named ram_postcopy_incoming_init
- */
-int postcopy_ram_incoming_init(MigrationIncomingState *mis)
-{
-    if (foreach_not_ignored_block(init_range, NULL)) {
-        return -1;
-    }
-
-    return 0;
-}
-
 /*
  * Manage a single vote to the QEMU balloon inhibitor for all postcopy usage,
  * last caller wins.
@@ -1282,12 +1242,6 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
     return false;
 }
 
-int postcopy_ram_incoming_init(MigrationIncomingState *mis)
-{
-    error_report("postcopy_ram_incoming_init: No OS support");
-    return -1;
-}
-
 int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
 {
     assert(0);
diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
index c0ccf64a96..1c79c6e51f 100644
--- a/migration/postcopy-ram.h
+++ b/migration/postcopy-ram.h
@@ -22,13 +22,6 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis);
  */
 int postcopy_ram_incoming_setup(MigrationIncomingState *mis);
 
-/*
- * Initialise postcopy-ram, setting the RAM to a state where we can go into
- * postcopy later; must be called prior to any precopy.
- * called from ram.c's similarly named ram_postcopy_incoming_init
- */
-int postcopy_ram_incoming_init(MigrationIncomingState *mis);
-
 /*
  * At the end of a migration where postcopy_ram_incoming_init was called.
  */
diff --git a/migration/ram.c b/migration/ram.c
index dfc50d57d5..9a853703d8 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4015,22 +4015,6 @@ static int ram_load_cleanup(void *opaque)
     return 0;
 }
 
-/**
- * ram_postcopy_incoming_init: allocate postcopy data structures
- *
- * Returns 0 for success and negative if there was one error
- *
- * @mis: current migration incoming state
- *
- * Allocate data structures etc needed by incoming migration with
- * postcopy-ram. postcopy-ram's similarly names
- * postcopy_ram_incoming_init does the work.
- */
-int ram_postcopy_incoming_init(MigrationIncomingState *mis)
-{
-    return postcopy_ram_incoming_init(mis);
-}
-
 /**
  * ram_load_postcopy: load a page in postcopy case
  *
diff --git a/migration/ram.h b/migration/ram.h
index 44fe4753ad..66cbff1d52 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -58,7 +58,6 @@ void ram_postcopy_migrated_memory_release(MigrationState *ms);
 int ram_postcopy_send_discard_bitmap(MigrationState *ms);
 /* For incoming postcopy discard */
 int ram_discard_range(const char *block_name, uint64_t start, size_t length);
-int ram_postcopy_incoming_init(MigrationIncomingState *mis);
 bool postcopy_is_running(void);
 
 void ram_handle_compressed(void *host, uint8_t ch, uint64_t size);
diff --git a/migration/savevm.c b/migration/savevm.c
index 9dc191e0a0..d2a427a3bf 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1674,10 +1674,6 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis,
         return -1;
     }
 
-    if (ram_postcopy_incoming_init(mis)) {
-        return -1;
-    }
-
     return 0;
 }
 
-- 
2.17.1

Re: [PATCH] migration/postcopy: not necessary to discard all RAM at the beginning

[Wei Yang]

Oops, this one seems to be missed.

On Mon, Oct 07, 2019 at 05:10:08PM +0800, Wei Yang wrote:
>ram_discard_range() unmap page for specific range. To be specific, this
>clears related page table entries so that userfault would be triggered.
>But this step is not necessary at the very beginning.
>
>ram_postcopy_incoming_init() is called when destination gets ADVISE
>command. ADVISE command is sent when migration thread just starts, which
>implies destination is not running yet. This means no page fault
>happened and memory region's page tables entries are empty.
>
>This patch removes the discard at the beginning.
>
>Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>---
> migration/postcopy-ram.c | 46 ----------------------------------------
> migration/postcopy-ram.h |  7 ------
> migration/ram.c          | 16 --------------
> migration/ram.h          |  1 -
> migration/savevm.c       |  4 ----
> 5 files changed, 74 deletions(-)
>
>diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
>index 5da6de8c8b..459be8e780 100644
>--- a/migration/postcopy-ram.c
>+++ b/migration/postcopy-ram.c
>@@ -443,32 +443,6 @@ out:
>     return ret;
> }
> 
>-/*
>- * Setup an area of RAM so that it *can* be used for postcopy later; this
>- * must be done right at the start prior to pre-copy.
>- * opaque should be the MIS.
>- */
>-static int init_range(RAMBlock *rb, void *opaque)
>-{
>-    const char *block_name = qemu_ram_get_idstr(rb);
>-    void *host_addr = qemu_ram_get_host_addr(rb);
>-    ram_addr_t offset = qemu_ram_get_offset(rb);
>-    ram_addr_t length = qemu_ram_get_used_length(rb);
>-    trace_postcopy_init_range(block_name, host_addr, offset, length);
>-
>-    /*
>-     * We need the whole of RAM to be truly empty for postcopy, so things
>-     * like ROMs and any data tables built during init must be zero'd
>-     * - we're going to get the copy from the source anyway.
>-     * (Precopy will just overwrite this data, so doesn't need the discard)
>-     */
>-    if (ram_discard_range(block_name, 0, length)) {
>-        return -1;
>-    }
>-
>-    return 0;
>-}
>-
> /*
>  * At the end of migration, undo the effects of init_range
>  * opaque should be the MIS.
>@@ -506,20 +480,6 @@ static int cleanup_range(RAMBlock *rb, void *opaque)
>     return 0;
> }
> 
>-/*
>- * Initialise postcopy-ram, setting the RAM to a state where we can go into
>- * postcopy later; must be called prior to any precopy.
>- * called from arch_init's similarly named ram_postcopy_incoming_init
>- */
>-int postcopy_ram_incoming_init(MigrationIncomingState *mis)
>-{
>-    if (foreach_not_ignored_block(init_range, NULL)) {
>-        return -1;
>-    }
>-
>-    return 0;
>-}
>-
> /*
>  * Manage a single vote to the QEMU balloon inhibitor for all postcopy usage,
>  * last caller wins.
>@@ -1282,12 +1242,6 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
>     return false;
> }
> 
>-int postcopy_ram_incoming_init(MigrationIncomingState *mis)
>-{
>-    error_report("postcopy_ram_incoming_init: No OS support");
>-    return -1;
>-}
>-
> int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
> {
>     assert(0);
>diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
>index c0ccf64a96..1c79c6e51f 100644
>--- a/migration/postcopy-ram.h
>+++ b/migration/postcopy-ram.h
>@@ -22,13 +22,6 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis);
>  */
> int postcopy_ram_incoming_setup(MigrationIncomingState *mis);
> 
>-/*
>- * Initialise postcopy-ram, setting the RAM to a state where we can go into
>- * postcopy later; must be called prior to any precopy.
>- * called from ram.c's similarly named ram_postcopy_incoming_init
>- */
>-int postcopy_ram_incoming_init(MigrationIncomingState *mis);
>-
> /*
>  * At the end of a migration where postcopy_ram_incoming_init was called.
>  */
>diff --git a/migration/ram.c b/migration/ram.c
>index dfc50d57d5..9a853703d8 100644
>--- a/migration/ram.c
>+++ b/migration/ram.c
>@@ -4015,22 +4015,6 @@ static int ram_load_cleanup(void *opaque)
>     return 0;
> }
> 
>-/**
>- * ram_postcopy_incoming_init: allocate postcopy data structures
>- *
>- * Returns 0 for success and negative if there was one error
>- *
>- * @mis: current migration incoming state
>- *
>- * Allocate data structures etc needed by incoming migration with
>- * postcopy-ram. postcopy-ram's similarly names
>- * postcopy_ram_incoming_init does the work.
>- */
>-int ram_postcopy_incoming_init(MigrationIncomingState *mis)
>-{
>-    return postcopy_ram_incoming_init(mis);
>-}
>-
> /**
>  * ram_load_postcopy: load a page in postcopy case
>  *
>diff --git a/migration/ram.h b/migration/ram.h
>index 44fe4753ad..66cbff1d52 100644
>--- a/migration/ram.h
>+++ b/migration/ram.h
>@@ -58,7 +58,6 @@ void ram_postcopy_migrated_memory_release(MigrationState *ms);
> int ram_postcopy_send_discard_bitmap(MigrationState *ms);
> /* For incoming postcopy discard */
> int ram_discard_range(const char *block_name, uint64_t start, size_t length);
>-int ram_postcopy_incoming_init(MigrationIncomingState *mis);
> bool postcopy_is_running(void);
> 
> void ram_handle_compressed(void *host, uint8_t ch, uint64_t size);
>diff --git a/migration/savevm.c b/migration/savevm.c
>index 9dc191e0a0..d2a427a3bf 100644
>--- a/migration/savevm.c
>+++ b/migration/savevm.c
>@@ -1674,10 +1674,6 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis,
>         return -1;
>     }
> 
>-    if (ram_postcopy_incoming_init(mis)) {
>-        return -1;
>-    }
>-
>     return 0;
> }
> 
>-- 
>2.17.1

-- 
Wei Yang
Help you, Help me

Re: [PATCH] migration/postcopy: not necessary to discard all RAM at the beginning

[Wei Yang]

Hi, David and Juan

Does it look good to you?

On Mon, Oct 07, 2019 at 05:10:08PM +0800, Wei Yang wrote:
>ram_discard_range() unmap page for specific range. To be specific, this
>clears related page table entries so that userfault would be triggered.
>But this step is not necessary at the very beginning.
>
>ram_postcopy_incoming_init() is called when destination gets ADVISE
>command. ADVISE command is sent when migration thread just starts, which
>implies destination is not running yet. This means no page fault
>happened and memory region's page tables entries are empty.
>
>This patch removes the discard at the beginning.
>
>Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>---
> migration/postcopy-ram.c | 46 ----------------------------------------
> migration/postcopy-ram.h |  7 ------
> migration/ram.c          | 16 --------------
> migration/ram.h          |  1 -
> migration/savevm.c       |  4 ----
> 5 files changed, 74 deletions(-)
>
>diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
>index 5da6de8c8b..459be8e780 100644
>--- a/migration/postcopy-ram.c
>+++ b/migration/postcopy-ram.c
>@@ -443,32 +443,6 @@ out:
>     return ret;
> }
> 
>-/*
>- * Setup an area of RAM so that it *can* be used for postcopy later; this
>- * must be done right at the start prior to pre-copy.
>- * opaque should be the MIS.
>- */
>-static int init_range(RAMBlock *rb, void *opaque)
>-{
>-    const char *block_name = qemu_ram_get_idstr(rb);
>-    void *host_addr = qemu_ram_get_host_addr(rb);
>-    ram_addr_t offset = qemu_ram_get_offset(rb);
>-    ram_addr_t length = qemu_ram_get_used_length(rb);
>-    trace_postcopy_init_range(block_name, host_addr, offset, length);
>-
>-    /*
>-     * We need the whole of RAM to be truly empty for postcopy, so things
>-     * like ROMs and any data tables built during init must be zero'd
>-     * - we're going to get the copy from the source anyway.
>-     * (Precopy will just overwrite this data, so doesn't need the discard)
>-     */
>-    if (ram_discard_range(block_name, 0, length)) {
>-        return -1;
>-    }
>-
>-    return 0;
>-}
>-
> /*
>  * At the end of migration, undo the effects of init_range
>  * opaque should be the MIS.
>@@ -506,20 +480,6 @@ static int cleanup_range(RAMBlock *rb, void *opaque)
>     return 0;
> }
> 
>-/*
>- * Initialise postcopy-ram, setting the RAM to a state where we can go into
>- * postcopy later; must be called prior to any precopy.
>- * called from arch_init's similarly named ram_postcopy_incoming_init
>- */
>-int postcopy_ram_incoming_init(MigrationIncomingState *mis)
>-{
>-    if (foreach_not_ignored_block(init_range, NULL)) {
>-        return -1;
>-    }
>-
>-    return 0;
>-}
>-
> /*
>  * Manage a single vote to the QEMU balloon inhibitor for all postcopy usage,
>  * last caller wins.
>@@ -1282,12 +1242,6 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
>     return false;
> }
> 
>-int postcopy_ram_incoming_init(MigrationIncomingState *mis)
>-{
>-    error_report("postcopy_ram_incoming_init: No OS support");
>-    return -1;
>-}
>-
> int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
> {
>     assert(0);
>diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
>index c0ccf64a96..1c79c6e51f 100644
>--- a/migration/postcopy-ram.h
>+++ b/migration/postcopy-ram.h
>@@ -22,13 +22,6 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis);
>  */
> int postcopy_ram_incoming_setup(MigrationIncomingState *mis);
> 
>-/*
>- * Initialise postcopy-ram, setting the RAM to a state where we can go into
>- * postcopy later; must be called prior to any precopy.
>- * called from ram.c's similarly named ram_postcopy_incoming_init
>- */
>-int postcopy_ram_incoming_init(MigrationIncomingState *mis);
>-
> /*
>  * At the end of a migration where postcopy_ram_incoming_init was called.
>  */
>diff --git a/migration/ram.c b/migration/ram.c
>index dfc50d57d5..9a853703d8 100644
>--- a/migration/ram.c
>+++ b/migration/ram.c
>@@ -4015,22 +4015,6 @@ static int ram_load_cleanup(void *opaque)
>     return 0;
> }
> 
>-/**
>- * ram_postcopy_incoming_init: allocate postcopy data structures
>- *
>- * Returns 0 for success and negative if there was one error
>- *
>- * @mis: current migration incoming state
>- *
>- * Allocate data structures etc needed by incoming migration with
>- * postcopy-ram. postcopy-ram's similarly names
>- * postcopy_ram_incoming_init does the work.
>- */
>-int ram_postcopy_incoming_init(MigrationIncomingState *mis)
>-{
>-    return postcopy_ram_incoming_init(mis);
>-}
>-
> /**
>  * ram_load_postcopy: load a page in postcopy case
>  *
>diff --git a/migration/ram.h b/migration/ram.h
>index 44fe4753ad..66cbff1d52 100644
>--- a/migration/ram.h
>+++ b/migration/ram.h
>@@ -58,7 +58,6 @@ void ram_postcopy_migrated_memory_release(MigrationState *ms);
> int ram_postcopy_send_discard_bitmap(MigrationState *ms);
> /* For incoming postcopy discard */
> int ram_discard_range(const char *block_name, uint64_t start, size_t length);
>-int ram_postcopy_incoming_init(MigrationIncomingState *mis);
> bool postcopy_is_running(void);
> 
> void ram_handle_compressed(void *host, uint8_t ch, uint64_t size);
>diff --git a/migration/savevm.c b/migration/savevm.c
>index 9dc191e0a0..d2a427a3bf 100644
>--- a/migration/savevm.c
>+++ b/migration/savevm.c
>@@ -1674,10 +1674,6 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis,
>         return -1;
>     }
> 
>-    if (ram_postcopy_incoming_init(mis)) {
>-        return -1;
>-    }
>-
>     return 0;
> }
> 
>-- 
>2.17.1

-- 
Wei Yang
Help you, Help me

Re: [PATCH] migration/postcopy: not necessary to discard all RAM at the beginning

[Dr. David Alan Gilbert]

* Wei Yang (richardw.yang@linux.intel.com) wrote:
> ram_discard_range() unmap page for specific range. To be specific, this
> clears related page table entries so that userfault would be triggered.
> But this step is not necessary at the very beginning.
> 
> ram_postcopy_incoming_init() is called when destination gets ADVISE
> command. ADVISE command is sent when migration thread just starts, which
> implies destination is not running yet. This means no page fault
> happened and memory region's page tables entries are empty.
> 
> This patch removes the discard at the beginning.
> 
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> ---
>  migration/postcopy-ram.c | 46 ----------------------------------------
>  migration/postcopy-ram.h |  7 ------
>  migration/ram.c          | 16 --------------
>  migration/ram.h          |  1 -
>  migration/savevm.c       |  4 ----
>  5 files changed, 74 deletions(-)
> 
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 5da6de8c8b..459be8e780 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -443,32 +443,6 @@ out:
>      return ret;
>  }
>  
> -/*
> - * Setup an area of RAM so that it *can* be used for postcopy later; this
> - * must be done right at the start prior to pre-copy.
> - * opaque should be the MIS.
> - */
> -static int init_range(RAMBlock *rb, void *opaque)
> -{
> -    const char *block_name = qemu_ram_get_idstr(rb);
> -    void *host_addr = qemu_ram_get_host_addr(rb);
> -    ram_addr_t offset = qemu_ram_get_offset(rb);
> -    ram_addr_t length = qemu_ram_get_used_length(rb);
> -    trace_postcopy_init_range(block_name, host_addr, offset, length);
> -
> -    /*
> -     * We need the whole of RAM to be truly empty for postcopy, so things
> -     * like ROMs and any data tables built during init must be zero'd
> -     * - we're going to get the copy from the source anyway.
> -     * (Precopy will just overwrite this data, so doesn't need the discard)
> -     */

But this comment explains why we want to do the discard; we want to make
sure that any memory that's been populated by the destination during the
init process is discarded and replaced by content from the source.

Dave

> -    if (ram_discard_range(block_name, 0, length)) {
> -        return -1;
> -    }
> -
> -    return 0;
> -}
> -
>  /*
>   * At the end of migration, undo the effects of init_range
>   * opaque should be the MIS.
> @@ -506,20 +480,6 @@ static int cleanup_range(RAMBlock *rb, void *opaque)
>      return 0;
>  }
>  
> -/*
> - * Initialise postcopy-ram, setting the RAM to a state where we can go into
> - * postcopy later; must be called prior to any precopy.
> - * called from arch_init's similarly named ram_postcopy_incoming_init
> - */
> -int postcopy_ram_incoming_init(MigrationIncomingState *mis)
> -{
> -    if (foreach_not_ignored_block(init_range, NULL)) {
> -        return -1;
> -    }
> -
> -    return 0;
> -}
> -
>  /*
>   * Manage a single vote to the QEMU balloon inhibitor for all postcopy usage,
>   * last caller wins.
> @@ -1282,12 +1242,6 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
>      return false;
>  }
>  
> -int postcopy_ram_incoming_init(MigrationIncomingState *mis)
> -{
> -    error_report("postcopy_ram_incoming_init: No OS support");
> -    return -1;
> -}
> -
>  int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
>  {
>      assert(0);
> diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
> index c0ccf64a96..1c79c6e51f 100644
> --- a/migration/postcopy-ram.h
> +++ b/migration/postcopy-ram.h
> @@ -22,13 +22,6 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis);
>   */
>  int postcopy_ram_incoming_setup(MigrationIncomingState *mis);
>  
> -/*
> - * Initialise postcopy-ram, setting the RAM to a state where we can go into
> - * postcopy later; must be called prior to any precopy.
> - * called from ram.c's similarly named ram_postcopy_incoming_init
> - */
> -int postcopy_ram_incoming_init(MigrationIncomingState *mis);
> -
>  /*
>   * At the end of a migration where postcopy_ram_incoming_init was called.
>   */
> diff --git a/migration/ram.c b/migration/ram.c
> index dfc50d57d5..9a853703d8 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -4015,22 +4015,6 @@ static int ram_load_cleanup(void *opaque)
>      return 0;
>  }
>  
> -/**
> - * ram_postcopy_incoming_init: allocate postcopy data structures
> - *
> - * Returns 0 for success and negative if there was one error
> - *
> - * @mis: current migration incoming state
> - *
> - * Allocate data structures etc needed by incoming migration with
> - * postcopy-ram. postcopy-ram's similarly names
> - * postcopy_ram_incoming_init does the work.
> - */
> -int ram_postcopy_incoming_init(MigrationIncomingState *mis)
> -{
> -    return postcopy_ram_incoming_init(mis);
> -}
> -
>  /**
>   * ram_load_postcopy: load a page in postcopy case
>   *
> diff --git a/migration/ram.h b/migration/ram.h
> index 44fe4753ad..66cbff1d52 100644
> --- a/migration/ram.h
> +++ b/migration/ram.h
> @@ -58,7 +58,6 @@ void ram_postcopy_migrated_memory_release(MigrationState *ms);
>  int ram_postcopy_send_discard_bitmap(MigrationState *ms);
>  /* For incoming postcopy discard */
>  int ram_discard_range(const char *block_name, uint64_t start, size_t length);
> -int ram_postcopy_incoming_init(MigrationIncomingState *mis);
>  bool postcopy_is_running(void);
>  
>  void ram_handle_compressed(void *host, uint8_t ch, uint64_t size);
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 9dc191e0a0..d2a427a3bf 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1674,10 +1674,6 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis,
>          return -1;
>      }
>  
> -    if (ram_postcopy_incoming_init(mis)) {
> -        return -1;
> -    }
> -
>      return 0;
>  }
>  
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH] migration/postcopy: not necessary to discard all RAM at the beginning

[Wei Yang]

On Thu, Feb 13, 2020 at 10:17:04AM +0000, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.yang@linux.intel.com) wrote:
>> ram_discard_range() unmap page for specific range. To be specific, this
>> clears related page table entries so that userfault would be triggered.
>> But this step is not necessary at the very beginning.
>> 
>> ram_postcopy_incoming_init() is called when destination gets ADVISE
>> command. ADVISE command is sent when migration thread just starts, which
>> implies destination is not running yet. This means no page fault
>> happened and memory region's page tables entries are empty.
>> 
>> This patch removes the discard at the beginning.
>> 
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>> ---
>>  migration/postcopy-ram.c | 46 ----------------------------------------
>>  migration/postcopy-ram.h |  7 ------
>>  migration/ram.c          | 16 --------------
>>  migration/ram.h          |  1 -
>>  migration/savevm.c       |  4 ----
>>  5 files changed, 74 deletions(-)
>> 
>> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
>> index 5da6de8c8b..459be8e780 100644
>> --- a/migration/postcopy-ram.c
>> +++ b/migration/postcopy-ram.c
>> @@ -443,32 +443,6 @@ out:
>>      return ret;
>>  }
>>  
>> -/*
>> - * Setup an area of RAM so that it *can* be used for postcopy later; this
>> - * must be done right at the start prior to pre-copy.
>> - * opaque should be the MIS.
>> - */
>> -static int init_range(RAMBlock *rb, void *opaque)
>> -{
>> -    const char *block_name = qemu_ram_get_idstr(rb);
>> -    void *host_addr = qemu_ram_get_host_addr(rb);
>> -    ram_addr_t offset = qemu_ram_get_offset(rb);
>> -    ram_addr_t length = qemu_ram_get_used_length(rb);
>> -    trace_postcopy_init_range(block_name, host_addr, offset, length);
>> -
>> -    /*
>> -     * We need the whole of RAM to be truly empty for postcopy, so things
>> -     * like ROMs and any data tables built during init must be zero'd
>> -     * - we're going to get the copy from the source anyway.
>> -     * (Precopy will just overwrite this data, so doesn't need the discard)
>> -     */
>
>But this comment explains why we want to do the discard; we want to make
>sure that any memory that's been populated by the destination during the
>init process is discarded and replaced by content from the source.
>

OK, you are right. I missed the init stage.


>Dave
>

-- 
Wei Yang
Help you, Help me