Stefano Stabellini
2010-Aug-27 11:18 UTC
[Xen-devel] [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target
libxl: introduce libxl_set_relative_memory_target Introduce libxl_set_relative_memory_target to modify the memory target of a domain by a relative amount of memory in a single xenstore transaction. Modify libxl_set_memory_target to use xenstore transactions. The first time we are reading/writing dom0 memory target, fill the informations in xenstore if they are missing. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> diff -r 990ff10b7b00 tools/libxl/libxl.c --- a/tools/libxl/libxl.c Wed Aug 25 20:59:35 2010 +0100 +++ b/tools/libxl/libxl.c Wed Aug 25 21:01:36 2010 +0100 @@ -2751,56 +2751,243 @@ out: return rc; } +static int fill_dom0_memory_info(libxl_gc *gc, uint32_t *target_memkb) +{ + int rc; + libxl_dominfo info; + char *target = NULL, *endptr = NULL; + char *target_path = "/local/domain/0/memory/target"; + char *max_path = "/local/domain/0/memory/static-max"; + xs_transaction_t t; + libxl_ctx *ctx = libxl_gc_owner(gc); + +retry_transaction: + t = xs_transaction_start(ctx->xsh); + + target = libxl_xs_read(gc, t, target_path); + if (target) { + *target_memkb = strtoul(target, &endptr, 10); + if (*endptr != ''\0'') { + XL_LOG_ERRNO(ctx, XL_LOG_ERROR, + "invalid memory target %s from %s\n", target, target_path); + rc = ERROR_FAIL; + goto out; + } + rc = 0; + goto out; + } + + rc = libxl_domain_info(ctx, &info, 0); + if (rc < 0) + return rc; + + libxl_xs_write(gc, t, target_path, "%"PRIu32, (uint32_t) info.target_memkb); + libxl_xs_write(gc, t, max_path, "%"PRIu32, (uint32_t) info.max_memkb); + + *target_memkb = (uint32_t) info.target_memkb; + rc = 0; + +out: + if (!xs_transaction_end(ctx->xsh, t, 0)) + if (errno == EAGAIN) + goto retry_transaction; + + + return rc; +} + int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, uint32_t target_memkb, int enforce) { libxl_gc gc = LIBXL_INIT_GC(ctx); - int rc = 1; - uint32_t memorykb = 0, videoram = 0; - char *memmax, *endptr, *videoram_s = NULL; + int rc = 1, abort = 0; + uint32_t videoram = 0; + char *videoram_s = NULL; char *dompath = libxl_xs_get_dompath(&gc, domid); xc_domaininfo_t info; libxl_dominfo ptr; char *uuid; - - if (domid) { - memmax = libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, "%s/memory/static-max", dompath)); - if (!memmax) { + xs_transaction_t t; + +retry_transaction: + t = xs_transaction_start(ctx->xsh); + + videoram_s = libxl_xs_read(&gc, t, libxl_sprintf(&gc, "%s/memory/videoram", dompath)); + videoram = videoram_s ? atoi(videoram_s) : 0; + + if (enforce) { + rc = xc_domain_setmaxmem(ctx->xch, domid, target_memkb + LIBXL_MAXMEM_CONSTANT); + if (rc != 0) { XL_LOG_ERRNO(ctx, XL_LOG_ERROR, - "cannot get memory info from %s/memory/static-max\n", dompath); + "xc_domain_setmaxmem domid=%d memkb=%d failed " + "rc=%d\n", domid, target_memkb + LIBXL_MAXMEM_CONSTANT, rc); + abort = 1; goto out; } - memorykb = strtoul(memmax, &endptr, 10); + libxl_xs_write(&gc, t, libxl_sprintf(&gc, "%s/memory/static-max", dompath), "%"PRIu32, target_memkb); + } + + rc = xc_domain_memory_set_pod_target(ctx->xch, domid, (target_memkb - videoram) / 4, NULL, NULL, NULL); + if (rc != 0) { + XL_LOG_ERRNO(ctx, XL_LOG_ERROR, + "xc_domain_memory_set_pod_target domid=%d, memkb=%d " + "failed rc=%d\n", domid, (target_memkb - videoram) / 4, + rc); + abort = 1; + goto out; + } + + libxl_xs_write(&gc, t, libxl_sprintf(&gc, "%s/memory/target", dompath), "%"PRIu32, target_memkb); + rc = xc_domain_getinfolist(ctx->xch, domid, 1, &info); + if (rc != 1 || info.domain != domid) { + abort = 1; + goto out; + } + xcinfo2xlinfo(&info, &ptr); + uuid = libxl_uuid2string(&gc, ptr.uuid); + libxl_xs_write(&gc, t, libxl_sprintf(&gc, "/vm/%s/memory", uuid), "%"PRIu32, target_memkb / 1024); + +out: + if (!xs_transaction_end(ctx->xsh, t, abort) && !abort) + if (errno == EAGAIN) + goto retry_transaction; + + libxl_free_all(&gc); + return rc; +} + +int libxl_set_relative_memory_target(libxl_ctx *ctx, uint32_t domid, int32_t relative_target_memkb, int enforce) +{ + libxl_gc gc = LIBXL_INIT_GC(ctx); + int rc = 1, abort = 0; + uint32_t memorykb = 0, videoram = 0, target_memkb = 0, new_target_memkb = 0; + char *memmax, *endptr, *videoram_s = NULL, *target = NULL; + char *dompath = libxl_xs_get_dompath(&gc, domid); + xc_domaininfo_t info; + libxl_dominfo ptr; + char *uuid; + xs_transaction_t t; + +retry_transaction: + t = xs_transaction_start(ctx->xsh); + + target = libxl_xs_read(&gc, t, libxl_sprintf(&gc, "%s/memory/target", dompath)); + if (!target && !domid) { + xs_transaction_end(ctx->xsh, t, 1); + rc = fill_dom0_memory_info(&gc, &target_memkb); + if (rc < 0) { + abort = 1; + goto out; + } + goto retry_transaction; + } else if (!target) { + XL_LOG_ERRNO(ctx, XL_LOG_ERROR, + "cannot get target memory info from %s/memory/target\n", dompath); + abort = 1; + goto out; + } else { + target_memkb = strtoul(target, &endptr, 10); if (*endptr != ''\0'') { XL_LOG_ERRNO(ctx, XL_LOG_ERROR, - "invalid max memory %s from %s/memory/static-max\n", memmax, dompath); - goto out; - } - - if (target_memkb > memorykb) { - XL_LOG(ctx, XL_LOG_ERROR, - "memory_dynamic_max must be less than or equal to memory_static_max\n"); + "invalid memory target %s from %s/memory/target\n", target, dompath); + abort = 1; goto out; } } - - videoram_s = libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, "%s/memory/videoram", dompath)); + memmax = libxl_xs_read(&gc, t, libxl_sprintf(&gc, "%s/memory/static-max", dompath)); + if (!memmax) { + XL_LOG_ERRNO(ctx, XL_LOG_ERROR, + "cannot get memory info from %s/memory/static-max\n", dompath); + abort = 1; + goto out; + } + memorykb = strtoul(memmax, &endptr, 10); + if (*endptr != ''\0'') { + XL_LOG_ERRNO(ctx, XL_LOG_ERROR, + "invalid max memory %s from %s/memory/static-max\n", memmax, dompath); + abort = 1; + goto out; + } + + new_target_memkb = target_memkb + relative_target_memkb; + if (new_target_memkb > memorykb) { + XL_LOG(ctx, XL_LOG_ERROR, + "memory_dynamic_max must be less than or equal to memory_static_max\n"); + abort = 1; + goto out; + } + + videoram_s = libxl_xs_read(&gc, t, libxl_sprintf(&gc, "%s/memory/videoram", dompath)); videoram = videoram_s ? atoi(videoram_s) : 0; - libxl_xs_write(&gc, XBT_NULL, libxl_sprintf(&gc, "%s/memory/target", dompath), "%"PRIu32, target_memkb); - + if (enforce) { + memorykb = new_target_memkb; + rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb + LIBXL_MAXMEM_CONSTANT); + if (rc != 0) { + XL_LOG_ERRNO(ctx, XL_LOG_ERROR, + "xc_domain_setmaxmem domid=%d memkb=%d failed " + "rc=%d\n", domid, memorykb + LIBXL_MAXMEM_CONSTANT, rc); + abort = 1; + goto out; + } + libxl_xs_write(&gc, t, libxl_sprintf(&gc, "%s/memory/static-max", dompath), "%"PRIu32, memorykb); + } + + rc = xc_domain_memory_set_pod_target(ctx->xch, domid, (new_target_memkb - videoram) / 4, NULL, NULL, NULL); + if (rc != 0) { + XL_LOG_ERRNO(ctx, XL_LOG_ERROR, + "xc_domain_memory_set_pod_target domid=%d, memkb=%d " + "failed rc=%d\n", domid, (new_target_memkb - videoram) / 4, + rc); + abort = 1; + goto out; + } + + libxl_xs_write(&gc, t, libxl_sprintf(&gc, "%s/memory/target", dompath), "%"PRIu32, new_target_memkb); rc = xc_domain_getinfolist(ctx->xch, domid, 1, &info); - if (rc != 1 || info.domain != domid) + if (rc != 1 || info.domain != domid) { + abort = 1; goto out; + } xcinfo2xlinfo(&info, &ptr); uuid = libxl_uuid2string(&gc, ptr.uuid); - libxl_xs_write(&gc, XBT_NULL, libxl_sprintf(&gc, "/vm/%s/memory", uuid), "%"PRIu32, target_memkb / 1024); - - if (enforce || !domid) - memorykb = target_memkb; - rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb + LIBXL_MAXMEM_CONSTANT); - if (rc != 0) + libxl_xs_write(&gc, t, libxl_sprintf(&gc, "/vm/%s/memory", uuid), "%"PRIu32, new_target_memkb / 1024); + +out: + if (!xs_transaction_end(ctx->xsh, t, abort) && !abort) + if (errno == EAGAIN) + goto retry_transaction; + + libxl_free_all(&gc); + return rc; +} + +int libxl_get_memory_target(libxl_ctx *ctx, uint32_t domid, uint32_t *out_target) +{ + libxl_gc gc = LIBXL_INIT_GC(ctx); + int rc = 1; + char *target = NULL, *endptr = NULL; + char *dompath = libxl_xs_get_dompath(&gc, domid); + uint32_t target_memkb; + + target = libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, "%s/memory/target", dompath)); + if (!target && !domid) { + rc = fill_dom0_memory_info(&gc, &target_memkb); + if (rc < 0) + goto out; + } else if (!target) { + XL_LOG_ERRNO(ctx, XL_LOG_ERROR, + "cannot get target memory info from %s/memory/target\n", dompath); goto out; - rc = xc_domain_memory_set_pod_target(ctx->xch, domid, (target_memkb - videoram) / 4, NULL, NULL, NULL); + } else { + target_memkb = strtoul(target, &endptr, 10); + if (*endptr != ''\0'') { + XL_LOG_ERRNO(ctx, XL_LOG_ERROR, + "invalid memory target %s from %s/memory/target\n", target, dompath); + goto out; + } + } + *out_target = target_memkb; + rc = 0; out: libxl_free_all(&gc); diff -r 990ff10b7b00 tools/libxl/libxl.h --- a/tools/libxl/libxl.h Wed Aug 25 20:59:35 2010 +0100 +++ b/tools/libxl/libxl.h Wed Aug 25 21:01:36 2010 +0100 @@ -322,6 +322,8 @@ int libxl_domain_core_dump(libxl_ctx *ct int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t target_memkb); int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, uint32_t target_memkb, int enforce); +int libxl_set_relative_memory_target(libxl_ctx *ctx, uint32_t domid, int32_t relative_target_memkb, int enforce); +int libxl_get_memory_target(libxl_ctx *ctx, uint32_t domid, uint32_t *out_target); int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid, int autopass); int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, libxl_console_constype type); _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2010-Aug-31 17:25 UTC
Re: [Xen-devel] [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target
Stefano Stabellini writes ("[Xen-devel] [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target"):> libxl: introduce libxl_set_relative_memory_target > > Introduce libxl_set_relative_memory_target to modify the memory target > of a domain by a relative amount of memory in a single xenstore > transaction. > Modify libxl_set_memory_target to use xenstore transactions. > The first time we are reading/writing dom0 memory target, fill the > informations in xenstore if they are missing.> int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, > uint32_t target_memkb, int enforce)See my earlier comments about memory targets. I don''t think it makes much sense to give a domain a memory target and then let it exceed it. So I think "enforce" should be abolished (as if it were always set). Also please can you try to keep your code to <75ish columns ? :-) (75 because there should be room for > and + quoting without wrap damage.)> int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, > uint32_t target_memkb, int enforce)...> +int libxl_set_relative_memory_target(libxl_ctx *ctx, uint32_t > + domid, int32_t relative_target_memkb, int enforce)These functions are really rather too similar for my taste. They seem to differ only in whether they read the existing target and add it on. Surely they should be combined. Also, I don''t really think this patch to introuce the relative setting function should involves adding a lot of code to the absolute setting function. It''s a shame that we have to set so many different copies of the same value, but if we do then that should be done in a separate patch first perhaps ? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Sep-01 10:55 UTC
Re: [Xen-devel] [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target
On Tue, 31 Aug 2010, Ian Jackson wrote:> Stefano Stabellini writes ("[Xen-devel] [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target"): > > libxl: introduce libxl_set_relative_memory_target > > > > Introduce libxl_set_relative_memory_target to modify the memory target > > of a domain by a relative amount of memory in a single xenstore > > transaction. > > Modify libxl_set_memory_target to use xenstore transactions. > > The first time we are reading/writing dom0 memory target, fill the > > informations in xenstore if they are missing. > > > int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, > > uint32_t target_memkb, int enforce) > > See my earlier comments about memory targets. I don''t think it makes > much sense to give a domain a memory target and then let it exceed it. > So I think "enforce" should be abolished (as if it were always set). >I can do that.> Also please can you try to keep your code to <75ish columns ? :-) > (75 because there should be room for > and + quoting without wrap > damage.) >Yes. We need a CODING_STILE, I''ll post a patch with it later on.> > int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, > > uint32_t target_memkb, int enforce) > ... > > +int libxl_set_relative_memory_target(libxl_ctx *ctx, uint32_t > > + domid, int32_t relative_target_memkb, int enforce) > > These functions are really rather too similar for my taste. They > seem to differ only in whether they read the existing target and add > it on. Surely they should be combined. > > Also, I don''t really think this patch to introuce the relative setting > function should involves adding a lot of code to the absolute setting > function. It''s a shame that we have to set so many different copies > of the same value, but if we do then that should be done in a separate > patch first perhaps ? >The separate patch is a good idea, but merging the two functions together will result in code harder to read in the implementation of a very important function. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2010-Sep-01 18:01 UTC
Re: [Xen-devel] [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target
On 08/31/2010 10:25 AM, Ian Jackson wrote:> Stefano Stabellini writes ("[Xen-devel] [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target"): >> libxl: introduce libxl_set_relative_memory_target >> >> Introduce libxl_set_relative_memory_target to modify the memory target >> of a domain by a relative amount of memory in a single xenstore >> transaction. >> Modify libxl_set_memory_target to use xenstore transactions. >> The first time we are reading/writing dom0 memory target, fill the >> informations in xenstore if they are missing. >> int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, >> uint32_t target_memkb, int enforce) > See my earlier comments about memory targets. I don''t think it makes > much sense to give a domain a memory target and then let it exceed it. > So I think "enforce" should be abolished (as if it were always set).There needs to be a way to set the maxmem for a domain without also setting the memory/target xenstore key, so that you can allow a domain to control its own ballooning up to a certain limit without also triggering its xenstore watch (which would trigger an external balloon target request). Ideally there should also be a xenstore key that allows a guest to determine what its max is, rather than just feeling for it until it gets hypercall failures. In general, guests can treat "memory/target" as a host recommendation of what size it should ideally be if it is being as cooperative as possible. A simple implementation - like the current balloon driver - just treats it literally. But there''s no reason why that''s the best thing to do. (If there is some kind of cost model where there''s an active incentive for a domain to reduce its own memory usage it would change the landscape a lot.) By extension, there''s no reason why the guest must be limited by "target" - its possible that its ideal size, based on its own internal memory pressure - is larger than target. It can stay larger than target if it is shrinking by stopping before it hits target, but I don''t see an inherent reason why the toolstack might not want to suggest a particular target but allow a domain to grow beyond that. There''s also the question of how tmem makes use of maxmem - I believe it allows a domain to use tmem pages up to its maxmem, without actually changing the current memory usage in terms of pages mapped into the domain. If libxl is supposed to be the general-purpose base of a toolstack, I don''t think it should be trying to make these kinds of policy decisions ("target == max") internally - or implicitly as part of the API design. In this case, I think it would be most sensible to have: int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, uint32_t target_memkb) (no enforce parameter) which simply sets the target xenstore key (nothing else), and int libxl_set_memory_limit(libxl_ctx *ctx, uint32_t domid, uint32_t target_memkb) which sets the domain''s Xen-enforced max size (and maybe a xenstore key so that the domain can tell that its max has been changed and what to). And that''s it. The target and limit are completely orthogonal concepts, and there definitely should not be any implementation of rules like "target must be smaller than limit". If a toolstack wants to implement more mechanism/policy on top of this, it is free to do so. But libxl should just provide plain basics. J _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Dan Magenheimer
2010-Sep-01 20:03 UTC
RE: [Xen-devel] [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target
Thanks for cc''ing me Jeremy... I saw the posting but missed the implications (thinking it just a vanilla reimplementation of "xm memset").> There needs to be a way to set the maxmem for a domain without also > setting the memory/target xenstore key, so that you can allow a domain > to control its own ballooning up to a certain limit without also > triggering its xenstore watch (which would trigger an external balloon > target request).Indeed, that''s what selfballooning does. The xenstore watch is irrelevant for selfballooning (though the watch also can be used asynchronously for backwards compatibility). IMHO, attempts to do memory load balancing externally (e.g. setting a memory target from tools in dom0) are doomed to failure. There was a discussion of memory "rightsizing" at the recent Linux MM summit; this is an almost impossible problem even within a single kernel, though there were heuristics discussed as to how to approach it... and a better understanding about why in-kernel tmem-ish functionalities like cleancache and frontswap are useful for mitigating the problems that occur when rightsizing is approximated.> There''s also the question of how tmem makes use of maxmem - I believe > it > allows a domain to use tmem pages up to its maxmem, without actually > changing the current memory usage in terms of pages mapped into the > domain.There are two classes of tmem memory: ephemeral and persistent. Ephemeral is clean pages that can be discarded at any time and tmem doesn''t count those against maxmem. Persistent pages must be retained until the guest that "owns" them flushes them or dies; these pages DO count against the guest''s maxmem but are not mapped into the domain. So, frankly, I think the "xm memset" functionality is largely useless, but agree that it should be maintained in xl for backwards compatibility. But trying to comingle the concepts of maxmem and target is a bad idea. Dan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2010-Sep-01 21:17 UTC
Re: [Xen-devel] [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target
On 09/01/2010 01:03 PM, Dan Magenheimer wrote:> Indeed, that''s what selfballooning does. The xenstore watch > is irrelevant for selfballooning (though the watch also can be used > asynchronously for backwards compatibility).There''s no mechanism to make the balloon driver ignore the target watch, so any updates to xenstore will update the driver''s target.> IMHO, attempts to do memory load balancing externally (e.g. setting > a memory target from tools in dom0) are doomed to failure. There > was a discussion of memory "rightsizing" at the recent Linux MM summit; > this is an almost impossible problem even within a single kernel, > though there were heuristics discussed as to how to approach it... > and a better understanding about why in-kernel tmem-ish functionalities > like cleancache and frontswap are useful for mitigating the problems > that occur when rightsizing is approximated. > [...] > So, frankly, I think the "xm memset" functionality is largely > useless, but agree that it should be maintained in xl for backwards > compatibility. But trying to comingle the concepts of maxmem > and target is a bad idea.In the general case I think you''re probably right (I can''t see it being useful in a VPS hosting service, for example), but there are definitely special cases where it is useful. Squashing down existing domains to make room for a new one, for example, or more app-specific uses. Giving domains some real incentive to be economical with memory would probably change the landscape a lot. But I don''t think there''s a real solution without knowing the specifics of that incentive. J _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Dan Magenheimer
2010-Sep-01 21:49 UTC
RE: [Xen-devel] [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target
> From: Jeremy Fitzhardinge [mailto:jeremy@goop.org] > > On 09/01/2010 01:03 PM, Dan Magenheimer wrote: > > Indeed, that''s what selfballooning does. The xenstore watch > > is irrelevant for selfballooning (though the watch also can be used > > asynchronously for backwards compatibility). > > There''s no mechanism to make the balloon driver ignore the target > watch, > so any updates to xenstore will update the driver''s target.The selfballooning patch currently applies to the balloon driver so it could easily disable the target watch, though it does not.> > So, frankly, I think the "xm memset" functionality is largely > > useless, but agree that it should be maintained in xl for backwards > > compatibility. But trying to comingle the concepts of maxmem > > and target is a bad idea. > > In the general case I think you''re probably right (I can''t see it being > useful in a VPS hosting service, for example), but there are definitely > special cases where it is useful. Squashing down existing domains to > make room for a new one, for example, or more app-specific uses.Agreed in general, though I suspect sysadmins would be rather peeved if/when a simple xm command in dom0 causes kernel OOMs and application kills... or, worse, guest kernel crashes (which are circumvented by minimum-target code in the linux-xen balloon driver but NOT in the pvops balloon driver). So "useless" is an overstatement, but "must be used extremely carefully with knowledge of current activity in the guest and/or willingness for the targeted guest or its applications to die for a greater cause" is not an overstatement.> Giving domains some real incentive to be economical with memory would > probably change the landscape a lot. But I don''t think there''s a real > solution without knowing the specifics of that incentive.Agreed. Lacking a clear incentive though, reducing the disincentive is a reasonable approach... which is what tmem+selfballooning does. My long term view of the incentive is something like a VPS hoster that offers service for $10/month, but offers it for $5/month if using a tmem(+selfballooning)-enabled kernel. This is essentially like the electric utility companies that give customers a discount if the customer allows them a remote-kill switch to turn off your air conditioning if system-wide conditions warrant. Dan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Sep-02 09:15 UTC
Re: [Xen-devel] [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target
On Wed, 1 Sep 2010, Jeremy Fitzhardinge wrote:> There needs to be a way to set the maxmem for a domain without also > setting the memory/target xenstore key, so that you can allow a domain > to control its own ballooning up to a certain limit without also > triggering its xenstore watch (which would trigger an external balloon > target request). > > Ideally there should also be a xenstore key that allows a guest to > determine what its max is, rather than just feeling for it until it gets > hypercall failures. > > In general, guests can treat "memory/target" as a host recommendation of > what size it should ideally be if it is being as cooperative as > possible. A simple implementation - like the current balloon driver - > just treats it literally. But there''s no reason why that''s the best > thing to do. (If there is some kind of cost model where there''s an > active incentive for a domain to reduce its own memory usage it would > change the landscape a lot.) > > By extension, there''s no reason why the guest must be limited by > "target" - its possible that its ideal size, based on its own internal > memory pressure - is larger than target. It can stay larger than target > if it is shrinking by stopping before it hits target, but I don''t see an > inherent reason why the toolstack might not want to suggest a particular > target but allow a domain to grow beyond that. > > There''s also the question of how tmem makes use of maxmem - I believe it > allows a domain to use tmem pages up to its maxmem, without actually > changing the current memory usage in terms of pages mapped into the domain. > > If libxl is supposed to be the general-purpose base of a toolstack, I > don''t think it should be trying to make these kinds of policy decisions > ("target == max") internally - or implicitly as part of the API design.I actually agree with you on this point, but yesterday morning I couldn''t come up with any practical use case to support this argument.> > In this case, I think it would be most sensible to have: > > int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, uint32_t > target_memkb) > > (no enforce parameter) which simply sets the target xenstore key > (nothing else), and > > int libxl_set_memory_limit(libxl_ctx *ctx, uint32_t domid, uint32_t > target_memkb) > > which sets the domain''s Xen-enforced max size (and maybe a xenstore key > so that the domain can tell that its max has been changed and what to). >Considering that there is already a libxl function to set memmax (libxl_domain_setmaxmem), I''ll just keep libxl_set_memory_target as it is because it might be very useful to set a new target and a new memmax in a single transaction. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel