Re: [Xen-devel] [PATCH v1] tools/libxc: use superpages during restore of HVM guest

2017-08-04 Thread Olaf Hering
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

2017-08-04 Thread Wei Liu
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

2017-08-04 Thread Wei Liu
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

2017-08-03 Thread Olaf Hering
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

2017-08-02 Thread Olaf Hering
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_