[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v9 2/3] tools/libxc: add API for bitmap access for restore



On 01/09/17 17:08, Olaf Hering wrote:
> Extend API for managing bitmaps. Each bitmap is now represented by a
> generic struct xc_sr_bitmap.
> Switch the existing populated_pfns to this API.
>
> Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>
> Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
>  tools/libxc/xc_sr_common.c  | 41 ++++++++++++++++++++++++++
>  tools/libxc/xc_sr_common.h  | 72 
> +++++++++++++++++++++++++++++++++++++++++++--
>  tools/libxc/xc_sr_restore.c | 66 ++---------------------------------------
>  3 files changed, 114 insertions(+), 65 deletions(-)
>
> diff --git a/tools/libxc/xc_sr_common.c b/tools/libxc/xc_sr_common.c
> index 79b9c3e940..4d221ca90c 100644
> --- a/tools/libxc/xc_sr_common.c
> +++ b/tools/libxc/xc_sr_common.c
> @@ -155,6 +155,47 @@ static void __attribute__((unused)) 
> build_assertions(void)
>      BUILD_BUG_ON(sizeof(struct xc_sr_rec_hvm_params)        != 8);
>  }
>  
> +/*
> + * Expand the tracking structures as needed.
> + * To avoid realloc()ing too excessively, the size increased to the nearest 
> power
> + * of two large enough to contain the required number of bits.
> + */
> +bool _xc_sr_bitmap_resize(struct xc_sr_bitmap *bm, unsigned long bits)
> +{
> +    if (bits > bm->bits)

Libxc uses the hypervisor coding style, and xc_sr_* currently have a
consistent style.

> +    {
> +        size_t new_max;
> +        size_t old_sz, new_sz;
> +        void *p;
> +
> +        /* Round up to the nearest power of two larger than bit, less 1. */
> +        new_max = bits;
> +        new_max |= new_max >> 1;
> +        new_max |= new_max >> 2;
> +        new_max |= new_max >> 4;
> +        new_max |= new_max >> 8;
> +        new_max |= new_max >> 16;
> +#ifdef __x86_64__
> +        new_max |= new_max >> 32;
> +#endif
> +
> +        old_sz = bitmap_size(bm->bits + 1);
> +        new_sz = bitmap_size(new_max + 1);
> +        p = realloc(bm->p, new_sz);
> +        if (!p)
> +            return false;
> +
> +        if (bm->p)
> +            memset(p + old_sz, 0, new_sz - old_sz);
> +        else
> +            memset(p, 0, new_sz);
> +
> +        bm->p = p;
> +        bm->bits = new_max;
> +    }
> +    return true;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
> index a83f22af4e..734320947a 100644
> --- a/tools/libxc/xc_sr_common.h
> +++ b/tools/libxc/xc_sr_common.h
> @@ -172,6 +172,12 @@ struct xc_sr_x86_pv_restore_vcpu
>      size_t basicsz, extdsz, xsavesz, msrsz;
>  };
>  
> +struct xc_sr_bitmap
> +{
> +    void *p;
> +    unsigned long bits;
> +};
> +
>  struct xc_sr_context
>  {
>      xc_interface *xch;
> @@ -255,8 +261,7 @@ struct xc_sr_context
>              domid_t      xenstore_domid,  console_domid;
>  
>              /* Bitmap of currently populated PFNs during restore. */
> -            unsigned long *populated_pfns;
> -            xen_pfn_t max_populated_pfn;
> +            struct xc_sr_bitmap populated_pfns;
>  
>              /* Sender has invoked verify mode on the stream. */
>              bool verify;
> @@ -343,6 +348,69 @@ extern struct xc_sr_save_ops save_ops_x86_hvm;
>  extern struct xc_sr_restore_ops restore_ops_x86_pv;
>  extern struct xc_sr_restore_ops restore_ops_x86_hvm;
>  
> +extern bool _xc_sr_bitmap_resize(struct xc_sr_bitmap *bm, unsigned long 
> bits);

No need for extern on function prototypes.

> +
> +static inline bool xc_sr_bitmap_resize(struct xc_sr_bitmap *bm, unsigned 
> long bits)
> +{
> +    if (bits > bm->bits)
> +        return _xc_sr_bitmap_resize(bm, bits);
> +    return true;
> +}
> +
> +static inline void xc_sr_bitmap_free(struct xc_sr_bitmap *bm)
> +{
> +    free(bm->p);
> +    bm->p = NULL;

bm->bits = 0, or a subsequent test/set/clear will fall over a NULL pointer.

> +}
> +
> +static inline bool xc_sr_set_bit(unsigned long bit, struct xc_sr_bitmap *bm)
> +{
> +    if (!xc_sr_bitmap_resize(bm, bit))

There's a boundary condition here trying to test bit 0 of an empty bitmap.

> +        return false;
> +
> +    set_bit(bit, bm->p);
> +    return true;
> +}
> +
> +static inline bool xc_sr_test_bit(unsigned long bit, struct xc_sr_bitmap *bm)
> +{
> +    if (bit > bm->bits)
> +        return false;
> +    return !!test_bit(bit, bm->p);
> +}
> +
> +static inline bool xc_sr_test_and_clear_bit(unsigned long bit, struct 
> xc_sr_bitmap *bm)
> +{
> +    if (bit > bm->bits)
> +        return false;
> +    return !!test_and_clear_bit(bit, bm->p);
> +}
> +
> +static inline bool xc_sr_test_and_set_bit(unsigned long bit, struct 
> xc_sr_bitmap *bm)
> +{
> +    if (bit > bm->bits)
> +        return false;
> +    return !!test_and_set_bit(bit, bm->p);
> +}
> +
> +static inline bool pfn_is_populated(struct xc_sr_context *ctx, xen_pfn_t pfn)
> +{
> +    return xc_sr_test_bit(pfn, &ctx->restore.populated_pfns);
> +}
> +
> +static inline int pfn_set_populated(struct xc_sr_context *ctx, xen_pfn_t pfn)
> +{
> +    xc_interface *xch = ctx->xch;
> +
> +    if ( !xc_sr_set_bit(pfn, &ctx->restore.populated_pfns) )
> +    {
> +        ERROR("Failed to realloc populated_pfns bitmap");
> +        errno = ENOMEM;
> +        return -1;
> +    }
> +    return 0;
> +}

Why are these moved?  They are still restore specific.

~Andrew

> +
>  struct xc_sr_record
>  {
>      uint32_t type;
> diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c
> index a016678332..d53948e1a6 100644
> --- a/tools/libxc/xc_sr_restore.c
> +++ b/tools/libxc/xc_sr_restore.c
> @@ -68,64 +68,6 @@ static int read_headers(struct xc_sr_context *ctx)
>      return 0;
>  }
>  
> -/*
> - * Is a pfn populated?
> - */
> -static bool pfn_is_populated(const struct xc_sr_context *ctx, xen_pfn_t pfn)
> -{
> -    if ( pfn > ctx->restore.max_populated_pfn )
> -        return false;
> -    return test_bit(pfn, ctx->restore.populated_pfns);
> -}
> -
> -/*
> - * Set a pfn as populated, expanding the tracking structures if needed. To
> - * avoid realloc()ing too excessively, the size increased to the nearest 
> power
> - * of two large enough to contain the required pfn.
> - */
> -static int pfn_set_populated(struct xc_sr_context *ctx, xen_pfn_t pfn)
> -{
> -    xc_interface *xch = ctx->xch;
> -
> -    if ( pfn > ctx->restore.max_populated_pfn )
> -    {
> -        xen_pfn_t new_max;
> -        size_t old_sz, new_sz;
> -        unsigned long *p;
> -
> -        /* Round up to the nearest power of two larger than pfn, less 1. */
> -        new_max = pfn;
> -        new_max |= new_max >> 1;
> -        new_max |= new_max >> 2;
> -        new_max |= new_max >> 4;
> -        new_max |= new_max >> 8;
> -        new_max |= new_max >> 16;
> -#ifdef __x86_64__
> -        new_max |= new_max >> 32;
> -#endif
> -
> -        old_sz = bitmap_size(ctx->restore.max_populated_pfn + 1);
> -        new_sz = bitmap_size(new_max + 1);
> -        p = realloc(ctx->restore.populated_pfns, new_sz);
> -        if ( !p )
> -        {
> -            ERROR("Failed to realloc populated bitmap");
> -            errno = ENOMEM;
> -            return -1;
> -        }
> -
> -        memset((uint8_t *)p + old_sz, 0x00, new_sz - old_sz);
> -
> -        ctx->restore.populated_pfns    = p;
> -        ctx->restore.max_populated_pfn = new_max;
> -    }
> -
> -    assert(!test_bit(pfn, ctx->restore.populated_pfns));
> -    set_bit(pfn, ctx->restore.populated_pfns);
> -
> -    return 0;
> -}
> -
>  /*
>   * Given a set of pfns, obtain memory from Xen to fill the physmap for the
>   * unpopulated subset.  If types is NULL, no page type checking is performed
> @@ -684,10 +626,8 @@ static int setup(struct xc_sr_context *ctx)
>      if ( rc )
>          goto err;
>  
> -    ctx->restore.max_populated_pfn = (32 * 1024 / 4) - 1;
> -    ctx->restore.populated_pfns = bitmap_alloc(
> -        ctx->restore.max_populated_pfn + 1);
> -    if ( !ctx->restore.populated_pfns )
> +    rc = !xc_sr_bitmap_resize(&ctx->restore.populated_pfns, 32 * 1024 / 4);
> +    if ( rc )
>      {
>          ERROR("Unable to allocate memory for populated_pfns bitmap");
>          rc = -1;
> @@ -722,7 +662,7 @@ static void cleanup(struct xc_sr_context *ctx)
>          xc_hypercall_buffer_free_pages(xch, dirty_bitmap,
>                                     
> NRPAGES(bitmap_size(ctx->restore.p2m_size)));
>      free(ctx->restore.buffered_records);
> -    free(ctx->restore.populated_pfns);
> +    xc_sr_bitmap_free(&ctx->restore.populated_pfns);
>      if ( ctx->restore.ops.cleanup(ctx) )
>          PERROR("Failed to clean up");
>  }


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.