From: Wei Liu <wei.liu2@citrix.com>
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>,
Xen-devel <xen-devel@lists.xenproject.org>,
Vitaly Kuznetsov <vkuznets@redhat.com>,
Wei Liu <wei.liu2@citrix.com>
Subject: Re: [PATCH for-4.7] libxl: keep PoD target adjustment by memory fudge after reload_domain_config()
Date: Wed, 1 Jun 2016 16:56:30 +0100 [thread overview]
Message-ID: <20160601155630.GN5160@citrix.com> (raw)
In-Reply-To: <22351.991.967255.512153@mariner.uk.xensource.com>
On Wed, Jun 01, 2016 at 04:48:47PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[Xen-devel] [PATCH for-4.7] libxl: keep PoD target adjustment by memory fudge after reload_domain_config()"):
> > From: Vitaly Kuznetsov <vkuznets@redhat.com>
> >
> > Commit 56fb5fd623 ("libxl: adjust PoD target by memory fudge, too")
> > introduced target_memkb adjustment for HVM PoD domains on create. The
> > adjustment is however being reset on reload_domain_config() (e.g. when
> > we reboot the guest). For example:
>
> With George's revised commit message this patch makes sense for 4.7.
>
> I would like to see this function retain the name "fudge" though:
>
> > - ents[3] = GCSPRINTF("%"PRId64, info->target_memkb - info->video_memkb
> > - - mem_target_fudge);
> > + ents[3] = GCSPRINTF("%"PRId64, info->target_memkb -
> > + libxl__get_targetmem_difference(gc, info));
>
> ie:
>
> + ents[3] = GCSPRINTF("%"PRId64, info->target_memkb -
> + libxl__get_targetmem_fudge(gc, info));
>
> This makes it clear that there is still a problem here (and it will
> help things like "git grep -G").
>
> With that name changed, and George's commit message, you may put my
> ack on this.
>
>
Thanks everyone.
Updated patch here and I will push it shortly.
---8<---
From b9d63c2da58471a767852ab68111d245ee8d195f Mon Sep 17 00:00:00 2001
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Date: Wed, 3 Feb 2016 16:53:03 +0100
Subject: [PATCH] libxl: keep PoD target adjustment by memory fudge after
reload_domain_config()
Commit 56fb5fd623 ("libxl: adjust PoD target by memory fudge, too")
introduced target_memkb adjustment for HVM PoD domains on create,
wherein the value it wrote to target is always 1MiB lower than the
actual target_memkb. Unfortunately, on reboot, it is this value which
is read *unmodified* to feed into the next domain creation; from which
1MiB is subtracted *again*. This means that any guest which reboots
with memory < maxmem will have its memory target decreased by 1MiB on
every boot.
This patch makes it so that when reading target on reboot, we adjust the
value we read *up* by 1MiB, so that the domain will be build with the
appropriate amount of memory and the target will remain the same after
reboot.
This is still not quite a complete fix, as the 1MiB offset is only
subtracted when creating or rebooting; it is not subtracted when 'xl
set-memory' is called. But it will prevent any situations where memory
is continually increased or decreased. A better fix will have to wait
until after the release.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxl/libxl.c | 8 ++++----
tools/libxl/libxl_dom.c | 10 ++--------
tools/libxl/libxl_internal.h | 15 +++++++++++++++
3 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index b9d855b..306984b 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -7229,12 +7229,12 @@ int libxl_retrieve_domain_configuration(libxl_ctx *ctx, uint32_t domid,
LOG(ERROR, "fail to get memory target for domain %d", domid);
goto out;
}
- /* Target memory in xenstore is different from what user has
- * asked for. The difference is video_memkb. See
- * libxl_set_memory_target.
+
+ /* libxl__get_targetmem_fudge() calculates the difference from
+ * what is in xenstore to what we have in the domain build info.
*/
d_config->b_info.target_memkb = target_memkb +
- d_config->b_info.video_memkb;
+ libxl__get_targetmem_fudge(gc, &d_config->b_info);
d_config->b_info.max_memkb = max_memkb;
}
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 9b20cf5..805774f 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -490,7 +490,6 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid,
xs_transaction_t t;
char **ents;
int i, rc;
- int64_t mem_target_fudge;
if (info->num_vnuma_nodes && !info->num_vcpu_soft_affinity) {
rc = set_vnuma_affinity(gc, domid, info);
@@ -523,17 +522,12 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid,
}
}
- mem_target_fudge =
- (info->type == LIBXL_DOMAIN_TYPE_HVM &&
- info->max_memkb > info->target_memkb)
- ? LIBXL_MAXMEM_CONSTANT : 0;
-
ents = libxl__calloc(gc, 12 + (info->max_vcpus * 2) + 2, sizeof(char *));
ents[0] = "memory/static-max";
ents[1] = GCSPRINTF("%"PRId64, info->max_memkb);
ents[2] = "memory/target";
- ents[3] = GCSPRINTF("%"PRId64, info->target_memkb - info->video_memkb
- - mem_target_fudge);
+ ents[3] = GCSPRINTF("%"PRId64, info->target_memkb -
+ libxl__get_targetmem_fudge(gc, info));
ents[4] = "memory/videoram";
ents[5] = GCSPRINTF("%"PRId64, info->video_memkb);
ents[6] = "domid";
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index fac5751..3bdeb3c 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -4107,6 +4107,21 @@ static inline void libxl__update_config_vtpm(libxl__gc *gc,
libxl_uuid_copy(CTX, &dst->uuid, &src->uuid);
}
+/* Target memory in xenstore is different from what user has
+ * asked for. The difference is video_memkb + (possible) fudge.
+ * See libxl_set_memory_target.
+ */
+static inline
+uint64_t libxl__get_targetmem_fudge(libxl__gc *gc,
+ const libxl_domain_build_info *info)
+{
+ int64_t mem_target_fudge = (info->type == LIBXL_DOMAIN_TYPE_HVM &&
+ info->max_memkb > info->target_memkb)
+ ? LIBXL_MAXMEM_CONSTANT : 0;
+
+ return info->video_memkb + mem_target_fudge;
+}
+
/* Macros used to compare device identifier. Returns true if the two
* devices have same identifier. */
#define COMPARE_DEVID(a, b) ((a)->devid == (b)->devid)
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
prev parent reply other threads:[~2016-06-01 15:56 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-31 17:42 [PATCH for-4.7] libxl: keep PoD target adjustment by memory fudge after reload_domain_config() Wei Liu
2016-05-31 18:36 ` George Dunlap
2016-06-01 8:29 ` Vitaly Kuznetsov
2016-06-01 8:41 ` Wei Liu
2016-06-01 15:48 ` Ian Jackson
2016-06-01 15:56 ` Wei Liu [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160601155630.GN5160@citrix.com \
--to=wei.liu2@citrix.com \
--cc=Ian.Jackson@eu.citrix.com \
--cc=george.dunlap@eu.citrix.com \
--cc=vkuznets@redhat.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).