From: Olaf Hering Date: Fri, 23 Oct 2020 12:47:56 +0200 Subject: libxc sr save local_pages tools: save: preallocate local_pages array Remove repeated allocation from migration loop. There will never be more than MAX_BATCH_SIZE pages to process in a batch. Allocate the space once. Adjust the code to use the unmodified src page in case of HVM. In case of PV the page may need to be normalised, use a private memory area for this purpose. Signed-off-by: Olaf Hering --- tools/libs/guest/xg_sr_common.h | 22 ++++++++++--------- tools/libs/guest/xg_sr_save.c | 26 ++++------------------ tools/libs/guest/xg_sr_save_x86_hvm.c | 5 +++-- tools/libs/guest/xg_sr_save_x86_pv.c | 31 ++++++++++++++++++--------- 4 files changed, 40 insertions(+), 44 deletions(-) --- a/tools/libs/guest/xg_sr_common.h +++ b/tools/libs/guest/xg_sr_common.h @@ -33,16 +33,12 @@ struct xc_sr_save_ops * Optionally transform the contents of a page from being specific to the * sending environment, to being generic for the stream. * - * The page of data at the end of 'page' may be a read-only mapping of a - * running guest; it must not be modified. If no transformation is - * required, the callee should leave '*pages' untouched. + * The page of data '*src' may be a read-only mapping of a running guest; + * it must not be modified. If no transformation is required, the callee + * should leave '*src' untouched, and return it via '**ptr'. * - * If a transformation is required, the callee should allocate themselves - * a local page using malloc() and return it via '*page'. - * - * The caller shall free() '*page' in all cases. In the case that the - * callee encounters an error, it should *NOT* free() the memory it - * allocated for '*page'. + * If a transformation is required, the callee should provide the + * transformed page in a private buffer and return it via '**ptr'. * * It is valid to fail with EAGAIN if the transformation is not able to be * completed at this point. The page shall be retried later. @@ -50,7 +46,7 @@ struct xc_sr_save_ops * @returns 0 for success, -1 for failure, with errno appropriately set. */ int (*normalise_page)(struct xc_sr_context *ctx, xen_pfn_t type, - void **page); + void *src, unsigned int idx, void **ptr); /** * Set up local environment to save a domain. (Typically querying @@ -359,6 +355,12 @@ struct xc_sr_context { struct { + /* Used by write_batch for modified pages. */ + void *normalised_pages; + } save; + + struct + { /* State machine for the order of received records. */ bool seen_pv_info; --- a/tools/libs/guest/xg_sr_save.c +++ b/tools/libs/guest/xg_sr_save.c @@ -89,11 +89,10 @@ static int write_batch(struct xc_sr_cont { xc_interface *xch = ctx->xch; void *guest_mapping = NULL; - void **local_pages = NULL; int rc = -1; unsigned int i, p, nr_pages = 0, nr_pages_mapped = 0; unsigned int nr_pfns = ctx->save.nr_batch_pfns; - void *page, *orig_page; + void *src; int iovcnt = 0; struct xc_sr_rec_page_data_header hdr = { 0 }; struct xc_sr_record rec = { @@ -102,16 +101,6 @@ static int write_batch(struct xc_sr_cont assert(nr_pfns != 0); - /* Pointers to locally allocated pages. Need freeing. */ - local_pages = calloc(nr_pfns, sizeof(*local_pages)); - - if ( !local_pages ) - { - ERROR("Unable to allocate arrays for a batch of %u pages", - nr_pfns); - goto err; - } - for ( i = 0; i < nr_pfns; ++i ) { ctx->save.types[i] = ctx->save.mfns[i] = ctx->save.ops.pfn_to_gfn(ctx, @@ -175,11 +164,9 @@ static int write_batch(struct xc_sr_cont goto err; } - orig_page = page = guest_mapping + (p * PAGE_SIZE); - rc = ctx->save.ops.normalise_page(ctx, ctx->save.types[i], &page); - - if ( orig_page != page ) - local_pages[i] = page; + src = guest_mapping + (p * PAGE_SIZE); + rc = ctx->save.ops.normalise_page(ctx, ctx->save.types[i], src, i, + &ctx->save.guest_data[i]); if ( rc ) { @@ -194,8 +181,6 @@ static int write_batch(struct xc_sr_cont else goto err; } - else - ctx->save.guest_data[i] = page; rc = -1; ++p; @@ -256,9 +241,6 @@ static int write_batch(struct xc_sr_cont err: if ( guest_mapping ) xenforeignmemory_unmap(xch->fmem, guest_mapping, nr_pages_mapped); - for ( i = 0; local_pages && i < nr_pfns; ++i ) - free(local_pages[i]); - free(local_pages); return rc; } --- a/tools/libs/guest/xg_sr_save_x86_hvm.c +++ b/tools/libs/guest/xg_sr_save_x86_hvm.c @@ -129,9 +129,10 @@ static xen_pfn_t x86_hvm_pfn_to_gfn(cons return pfn; } -static int x86_hvm_normalise_page(struct xc_sr_context *ctx, - xen_pfn_t type, void **page) +static int x86_hvm_normalise_page(struct xc_sr_context *ctx, xen_pfn_t type, + void *src, unsigned int idx, void **ptr) { + *ptr = src; return 0; } --- a/tools/libs/guest/xg_sr_save_x86_pv.c +++ b/tools/libs/guest/xg_sr_save_x86_pv.c @@ -999,29 +999,31 @@ static xen_pfn_t x86_pv_pfn_to_gfn(const * save_ops function. Performs pagetable normalisation on appropriate pages. */ static int x86_pv_normalise_page(struct xc_sr_context *ctx, xen_pfn_t type, - void **page) + void *src, unsigned int idx, void **ptr) { xc_interface *xch = ctx->xch; - void *local_page; + void *dst; int rc; type &= XEN_DOMCTL_PFINFO_LTABTYPE_MASK; if ( type < XEN_DOMCTL_PFINFO_L1TAB || type > XEN_DOMCTL_PFINFO_L4TAB ) + { + *ptr = src; return 0; + } - local_page = malloc(PAGE_SIZE); - if ( !local_page ) + if ( idx >= MAX_BATCH_SIZE ) { - ERROR("Unable to allocate scratch page"); - rc = -1; - goto out; + ERROR("idx %u out of range", idx); + errno = ERANGE; + return -1; } - rc = normalise_pagetable(ctx, *page, local_page, type); - *page = local_page; + dst = ctx->x86.pv.save.normalised_pages + (idx * PAGE_SIZE); + rc = normalise_pagetable(ctx, src, dst, type); + *ptr = dst; - out: return rc; } @@ -1031,8 +1033,16 @@ static int x86_pv_normalise_page(struct */ static int x86_pv_setup(struct xc_sr_context *ctx) { + xc_interface *xch = ctx->xch; int rc; + ctx->x86.pv.save.normalised_pages = malloc(MAX_BATCH_SIZE * PAGE_SIZE); + if ( !ctx->x86.pv.save.normalised_pages ) + { + PERROR("Failed to allocate normalised_pages"); + return -1; + } + rc = x86_pv_domain_info(ctx); if ( rc ) return rc; @@ -1118,6 +1128,7 @@ static int x86_pv_check_vm_state(struct static int x86_pv_cleanup(struct xc_sr_context *ctx) { + free(ctx->x86.pv.save.normalised_pages); free(ctx->x86.pv.p2m_pfns); if ( ctx->x86.pv.p2m )