Re: [Xen-devel] [PATCH v1] tools/libxc: use superpages during restore of HVM guest
On Fri, Aug 04, Wei Liu wrote: > Can you split this patch into several: > 1. code movement > 2. refactoring / introduction of new hooks > 3. implementing the new algorithm I tried that, it did not work well. But, I can try again if required. Olaf signature.asc Description: PGP signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1] tools/libxc: use superpages during restore of HVM guest
On Fri, Aug 04, 2017 at 07:43:47AM +0200, Olaf Hering wrote: > On Wed, Aug 02, Olaf Hering wrote: > > > +++ b/tools/libxc/xc_sr_restore_x86_hvm.c > > > +#define SUPERPAGE_2MB_SHIFT 9 > > +#define SUPERPAGE_2MB_NR_PFNS (1UL << SUPERPAGE_2MB_SHIFT) > > +#define SUPERPAGE_1GB_SHIFT 18 > > +#define SUPERPAGE_1GB_NR_PFNS (1UL << SUPERPAGE_1GB_SHIFT) > > I think these can be moved to a header file. xc_dom_x86.c and > xc_sr_restore_x86_hvm.c use xc_dom.h. > > > +static int x86_hvm_populate_pfns(struct xc_sr_context *ctx, unsigned count, > > + const xen_pfn_t *original_pfns, > > + const uint32_t *types) > > +{ > > +xc_interface *xch = ctx->xch; > > +xen_pfn_t min_pfn = original_pfns[0], max_pfn = original_pfns[0]; > > +unsigned i; > > +int rc = -1; > > + > > +for ( i = 0; i < count; ++i ) > > +{ > > +if (original_pfns[i] < min_pfn) > > +min_pfn = original_pfns[i]; > > +if (original_pfns[i] > max_pfn) > > +max_pfn = original_pfns[i]; > > +if ( (types[i] != XEN_DOMCTL_PFINFO_XTAB && > > + types[i] != XEN_DOMCTL_PFINFO_BROKEN) && > > + !pfn_is_populated(ctx, original_pfns[i]) ) > > Are these types used at all for a HVM domU? Otherwise this condition can > be simplified to just check the populated state. I *think* XTAB is PV only but BROKEN applies to both PV and HVM. Maybe someone more familiar with the bit can chime in to provide more information. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1] tools/libxc: use superpages during restore of HVM guest
On Wed, Aug 02, 2017 at 03:45:25PM +0200, Olaf Hering wrote: > During creating of a HVM domU meminit_hvm() tries to map superpages. > After save/restore or migration this mapping is lost, everything is > allocated in single pages. This causes a performance degradition after > migration. > > Add neccessary code to preallocate a superpage for the chunk of pfns > that is received. In case a pfn was not populated on the sending side it > must be freed on the receiving side to avoid over-allocation. > > The existing code for x86_pv is moved unmodified into its own file. > Can you split this patch into several: 1. code movement 2. refactoring / introduction of new hooks 3. implementing the new algorithm Thanks ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1] tools/libxc: use superpages during restore of HVM guest
On Wed, Aug 02, Olaf Hering wrote: > +++ b/tools/libxc/xc_sr_restore_x86_hvm.c > +#define SUPERPAGE_2MB_SHIFT 9 > +#define SUPERPAGE_2MB_NR_PFNS (1UL << SUPERPAGE_2MB_SHIFT) > +#define SUPERPAGE_1GB_SHIFT 18 > +#define SUPERPAGE_1GB_NR_PFNS (1UL << SUPERPAGE_1GB_SHIFT) I think these can be moved to a header file. xc_dom_x86.c and xc_sr_restore_x86_hvm.c use xc_dom.h. > +static int x86_hvm_populate_pfns(struct xc_sr_context *ctx, unsigned count, > + const xen_pfn_t *original_pfns, > + const uint32_t *types) > +{ > +xc_interface *xch = ctx->xch; > +xen_pfn_t min_pfn = original_pfns[0], max_pfn = original_pfns[0]; > +unsigned i; > +int rc = -1; > + > +for ( i = 0; i < count; ++i ) > +{ > +if (original_pfns[i] < min_pfn) > +min_pfn = original_pfns[i]; > +if (original_pfns[i] > max_pfn) > +max_pfn = original_pfns[i]; > +if ( (types[i] != XEN_DOMCTL_PFINFO_XTAB && > + types[i] != XEN_DOMCTL_PFINFO_BROKEN) && > + !pfn_is_populated(ctx, original_pfns[i]) ) Are these types used at all for a HVM domU? Otherwise this condition can be simplified to just check the populated state. Olaf signature.asc Description: PGP signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v1] tools/libxc: use superpages during restore of HVM guest
During creating of a HVM domU meminit_hvm() tries to map superpages. After save/restore or migration this mapping is lost, everything is allocated in single pages. This causes a performance degradition after migration. Add neccessary code to preallocate a superpage for the chunk of pfns that is received. In case a pfn was not populated on the sending side it must be freed on the receiving side to avoid over-allocation. The existing code for x86_pv is moved unmodified into its own file. Signed-off-by: Olaf Hering --- based on RELEASE-4.9.0 tools/libxc/xc_sr_common.c | 41 tools/libxc/xc_sr_common.h | 79 +++- tools/libxc/xc_sr_restore.c | 135 +- tools/libxc/xc_sr_restore_x86_hvm.c | 183 tools/libxc/xc_sr_restore_x86_pv.c | 72 +- 5 files changed, 376 insertions(+), 134 deletions(-) diff --git a/tools/libxc/xc_sr_common.c b/tools/libxc/xc_sr_common.c index 48fa676f4e..9b68a064eb 100644 --- a/tools/libxc/xc_sr_common.c +++ b/tools/libxc/xc_sr_common.c @@ -156,6 +156,47 @@ static void __attribute__((unused)) build_assertions(void) } /* + * 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) +{ +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 * c-file-style: "BSD" diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h index a83f22af4e..ad1a2e6e02 100644 --- a/tools/libxc/xc_sr_common.h +++ b/tools/libxc/xc_sr_common.h @@ -140,6 +140,13 @@ struct xc_sr_restore_ops int (*setup)(struct xc_sr_context *ctx); /** + * Populate PFNs + * + */ +int (*populate_pfns)(struct xc_sr_context *ctx, unsigned count, + const xen_pfn_t *original_pfns, const uint32_t *types); + +/** * Process an individual record from the stream. The caller shall take * care of processing common records (e.g. END, PAGE_DATA). * @@ -172,6 +179,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 +268,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; @@ -331,6 +343,11 @@ struct xc_sr_context /* HVM context blob. */ void *context; size_t contextsz; + +/* Bitmap of currently allocated PFNs during restore. */ +struct xc_sr_bitmap attempted_1g; +struct xc_sr_bitmap attempted_2m; +struct xc_sr_bitmap allocated_pfns; } restore; }; } x86_hvm; @@ -343,6 +360,64 @@ 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); + +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); +} + +static inline bool xc_sr_set_bit(unsigned long bit, struct xc_sr_bitmap *bm) +{ +if (!xc_sr_bitmap_resize(bm, bit)) +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 int xc_sr_test_and_