[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] migration/postcopy: allocate tmp_page in setup stage
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [PATCH 1/2] migration/postcopy: allocate tmp_page in setup stage |
Date: |
Tue, 8 Oct 2019 18:18:54 +0100 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
* Wei Yang (address@hidden) wrote:
> During migration, a tmp page is allocated so that we could place a whole
> host page during postcopy.
>
> Currently the page is allocated during load stage, this is a little bit
> late. And more important, if we failed to allocate it, the error is not
> checked properly. Even it is NULL, we would still use it.
Oops, yes.
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
>
> This patch moves the allocation to setup stage and if failed error
> message would be printed and caller would notice it.
>
> Signed-off-by: Wei Yang <address@hidden>
> ---
> migration/postcopy-ram.c | 40 ++++++++++------------------------------
> migration/postcopy-ram.h | 7 -------
> migration/ram.c | 2 +-
> 3 files changed, 11 insertions(+), 38 deletions(-)
>
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 51dc164693..e554f93eec 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -1132,6 +1132,16 @@ int postcopy_ram_incoming_setup(MigrationIncomingState
> *mis)
> return -1;
> }
>
> + mis->postcopy_tmp_page = mmap(NULL, mis->largest_page_size,
> + PROT_READ | PROT_WRITE, MAP_PRIVATE |
> + MAP_ANONYMOUS, -1, 0);
> + if (mis->postcopy_tmp_page == MAP_FAILED) {
> + mis->postcopy_tmp_page = NULL;
> + error_report("%s: Failed to map postcopy_tmp_page %s",
> + __func__, strerror(errno));
> + return -1;
> + }
> +
> /*
> * Ballooning can mark pages as absent while we're postcopying
> * that would cause false userfaults.
> @@ -1258,30 +1268,6 @@ int postcopy_place_page_zero(MigrationIncomingState
> *mis, void *host,
> }
> }
>
> -/*
> - * Returns a target page of memory that can be mapped at a later point in
> time
> - * using postcopy_place_page
> - * The same address is used repeatedly, postcopy_place_page just takes the
> - * backing page away.
> - * Returns: Pointer to allocated page
> - *
> - */
> -void *postcopy_get_tmp_page(MigrationIncomingState *mis)
> -{
> - if (!mis->postcopy_tmp_page) {
> - mis->postcopy_tmp_page = mmap(NULL, mis->largest_page_size,
> - PROT_READ | PROT_WRITE, MAP_PRIVATE |
> - MAP_ANONYMOUS, -1, 0);
> - if (mis->postcopy_tmp_page == MAP_FAILED) {
> - mis->postcopy_tmp_page = NULL;
> - error_report("%s: %s", __func__, strerror(errno));
> - return NULL;
> - }
> - }
> -
> - return mis->postcopy_tmp_page;
> -}
> -
> #else
> /* No target OS support, stubs just fail */
> void fill_destination_postcopy_migration_info(MigrationInfo *info)
> @@ -1339,12 +1325,6 @@ int postcopy_place_page_zero(MigrationIncomingState
> *mis, void *host,
> return -1;
> }
>
> -void *postcopy_get_tmp_page(MigrationIncomingState *mis)
> -{
> - assert(0);
> - return NULL;
> -}
> -
> int postcopy_wake_shared(struct PostCopyFD *pcfd,
> uint64_t client_addr,
> RAMBlock *rb)
> diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
> index e3dde32155..c0ccf64a96 100644
> --- a/migration/postcopy-ram.h
> +++ b/migration/postcopy-ram.h
> @@ -100,13 +100,6 @@ typedef enum {
> POSTCOPY_INCOMING_END
> } PostcopyState;
>
> -/*
> - * Allocate a page of memory that can be mapped at a later point in time
> - * using postcopy_place_page
> - * Returns: Pointer to allocated page
> - */
> -void *postcopy_get_tmp_page(MigrationIncomingState *mis);
> -
> PostcopyState postcopy_state_get(void);
> /* Set the state and return the old state */
> PostcopyState postcopy_state_set(PostcopyState new_state,
> diff --git a/migration/ram.c b/migration/ram.c
> index 4c15162bd6..adbaf0b11a 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -4048,7 +4048,7 @@ static int ram_load_postcopy(QEMUFile *f)
> bool matches_target_page_size = false;
> MigrationIncomingState *mis = migration_incoming_get_current();
> /* Temporary page that is later 'placed' */
> - void *postcopy_host_page = postcopy_get_tmp_page(mis);
> + void *postcopy_host_page = mis->postcopy_tmp_page;
> void *last_host = NULL;
> bool all_zero = false;
>
> --
> 2.17.1
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK