[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH for-4.13 2/2] Rationalize max_grant_frames and max_maptrack_frames handling
> -----Original Message----- > From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of > George Dunlap > Sent: 26 November 2019 17:18 > To: xen-devel@xxxxxxxxxxxxxxxxxxxx > Cc: Juergen Gross <jgross@xxxxxxxx>; Stefano Stabellini > <sstabellini@xxxxxxxxxx>; Julien Grall <julien@xxxxxxx>; Wei Liu > <wl@xxxxxxx>; Paul Durrant <paul@xxxxxxx>; Andrew Cooper > <andrew.cooper3@xxxxxxxxxx>; Konrad Rzeszutek Wilk > <konrad.wilk@xxxxxxxxxx>; George Dunlap <george.dunlap@xxxxxxxxxx>; Marek > Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>; Jan Beulich > <jbeulich@xxxxxxxx>; Ian Jackson <ian.jackson@xxxxxxxxxx> > Subject: [Xen-devel] [PATCH for-4.13 2/2] Rationalize max_grant_frames and > max_maptrack_frames handling > > Xen used to have single, system-wide limits for the number of grant > frames and maptrack frames a guest was allowed to create. Increasing > or decreasing this single limit on the Xen command-line would change > the limit for all guests on the system. > > Later, per-domain limits for these values was created. The > system-wide limits became strict limits: domains could not be created > with higher limits, but could be created with lower limits. > > However, the change also introduced a range of different "default" > values into various places in the toolstack: > > - The python libxc bindings hard-coded these values to 32 and 1024, > respectively > > - The libxl default values are 32 and 1024 respectively. > > - xl will use the libxl default for maptrack, but does its own default > calculation for grant frames: either 32 or 64, based on the max > possible mfn. > > These defaults interact poorly with the hypervisor command-line limit: > > - The hypervisor command-line limit cannot be used to raise the limit > for all guests anymore, as the default in the toolstack will > effectively override this. > > - If you use the hypervisor command-line limit to *reduce* the limit, > then the "default" values generated by the toolstack are too high, > and all guest creations will fail. > > In other words, the toolstack defaults require any change to be > effected by having the admin explicitly specify a new value in every > guest. > > In order to address this, have grant_table_init treat '0' values for > max_grant_frames and max_maptrack_frames as instructions to use the > system-wide default. Have all the above toolstacks default to passing > 0 unless a different value is explicitly given. > > This restores the old behavior, that changing the hypervisor > command-line option can change the behavior for all guests, while > retaining the ability to set per-guest values. It also removes the > bug that *reducing* the system-wide max will cause all domains without > explicit limits to fail. > > (The ocaml bindings require the caller to always specify a value, and > the code to start a xenstored stubdomain hard-codes these to 4 and 128 > respectively; these will not be addressed here.) > > Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx> > --- > Release justification: This is an observed regression (albeit one that > has spanned several releases now). > > Compile-tested only. > > NB this patch could be applied without the whitespace fixes (perhaps > with some fix-ups); it's just easier since my editor strips trailing > whitespace out automatically. > > CC: Ian Jackson <ian.jackson@xxxxxxxxxx> > CC: Wei Liu <wl@xxxxxxx> > CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> > CC: Jan Beulich <jbeulich@xxxxxxxx> > CC: Paul Durrant <paul@xxxxxxx> > CC: Julien Grall <julien@xxxxxxx> > CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> > CC: Stefano Stabellini <sstabellini@xxxxxxxxxx> > CC: Juergen Gross <jgross@xxxxxxxx> > CC: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx> > --- > tools/libxl/libxl.h | 4 ++-- > tools/python/xen/lowlevel/xc/xc.c | 2 -- > tools/xl/xl.c | 12 ++---------- > xen/common/grant_table.c | 7 +++++++ > xen/include/public/domctl.h | 6 ++++-- > 5 files changed, 15 insertions(+), 16 deletions(-) > > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h > index 49b56fa1a3..1648d337e7 100644 > --- a/tools/libxl/libxl.h > +++ b/tools/libxl/libxl.h > @@ -364,8 +364,8 @@ > */ > #define LIBXL_HAVE_BUILDINFO_GRANT_LIMITS 1 > > -#define LIBXL_MAX_GRANT_FRAMES_DEFAULT 32 > -#define LIBXL_MAX_MAPTRACK_FRAMES_DEFAULT 1024 > +#define LIBXL_MAX_GRANT_FRAMES_DEFAULT 0 > +#define LIBXL_MAX_MAPTRACK_FRAMES_DEFAULT 0 > > /* > * LIBXL_HAVE_BUILDINFO_* indicates that libxl_domain_build_info has > diff --git a/tools/python/xen/lowlevel/xc/xc.c > b/tools/python/xen/lowlevel/xc/xc.c > index 6d2afd5695..0f861872ce 100644 > --- a/tools/python/xen/lowlevel/xc/xc.c > +++ b/tools/python/xen/lowlevel/xc/xc.c > @@ -127,8 +127,6 @@ static PyObject *pyxc_domain_create(XcObject *self, > }, > .max_vcpus = 1, > .max_evtchn_port = -1, /* No limit. */ > - .max_grant_frames = 32, > - .max_maptrack_frames = 1024, > }; > > static char *kwd_list[] = { "domid", "ssidref", "handle", "flags", > diff --git a/tools/xl/xl.c b/tools/xl/xl.c > index ddd29b3f1b..b6e220184d 100644 > --- a/tools/xl/xl.c > +++ b/tools/xl/xl.c > @@ -51,8 +51,8 @@ libxl_bitmap global_pv_affinity_mask; > enum output_format default_output_format = OUTPUT_FORMAT_JSON; > int claim_mode = 1; > bool progress_use_cr = 0; > -int max_grant_frames = -1; > -int max_maptrack_frames = -1; > +int max_grant_frames = 0; > +int max_maptrack_frames = 0; > > xentoollog_level minmsglevel = minmsglevel_default; > > @@ -96,7 +96,6 @@ static void parse_global_config(const char *configfile, > XLU_Config *config; > int e; > const char *buf; > - libxl_physinfo physinfo; > > config = xlu_cfg_init(stderr, configfile); > if (!config) { > @@ -199,13 +198,6 @@ static void parse_global_config(const char > *configfile, > > if (!xlu_cfg_get_long (config, "max_grant_frames", &l, 0)) > max_grant_frames = l; > - else { > - libxl_physinfo_init(&physinfo); > - max_grant_frames = (libxl_get_physinfo(ctx, &physinfo) != 0 || > - !(physinfo.max_possible_mfn >> 32)) > - ? 32 : 64; > - libxl_physinfo_dispose(&physinfo); > - } > if (!xlu_cfg_get_long (config, "max_maptrack_frames", &l, 0)) > max_maptrack_frames = l; > > diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c > index b34d520f6d..cd24029e33 100644 > --- a/xen/common/grant_table.c > +++ b/xen/common/grant_table.c > @@ -1843,6 +1843,13 @@ int grant_table_init(struct domain *d, unsigned int > max_grant_frames, > struct grant_table *gt; > int ret = -ENOMEM; > > + /* Default to maximum values if no lower ones are specified */ > + if ( !max_grant_frames ) > + max_grant_frames = opt_max_grant_frames; > + > + if ( !max_maptrack_frames ) > + max_maptrack_frames = opt_max_maptrack_frames; > + This means should also be able to drop the field setting in dom0_cfg in __start_xen() too :-) Paul > if ( max_grant_frames < INITIAL_NR_GRANT_FRAMES || > max_grant_frames > opt_max_grant_frames || > max_maptrack_frames > opt_max_maptrack_frames ) > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h > index 9f2cfd602c..27d04f67aa 100644 > --- a/xen/include/public/domctl.h > +++ b/xen/include/public/domctl.h > @@ -82,8 +82,10 @@ struct xen_domctl_createdomain { > uint32_t iommu_opts; > > /* > - * Various domain limits, which impact the quantity of resources > (global > - * mapping space, xenheap, etc) a guest may consume. > + * Various domain limits, which impact the quantity of resources > + * (global mapping space, xenheap, etc) a guest may consume. For > + * max_grant_frames and max_maptrack_frames, "0" means "use the > + * default maximum value". > */ > uint32_t max_vcpus; > uint32_t max_evtchn_port; > -- > 2.24.0 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@xxxxxxxxxxxxxxxxxxxx > https://lists.xenproject.org/mailman/listinfo/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |