Re: [Xen-devel] [PATCH for-4.13 2/2] Rationalize max_grant_frames and max_maptrack_frames handling

[George Dunlap <dunlapg@xxxxxxxxx>]

On Wed, Nov 27, 2019 at 4:33 AM Jürgen Groß <jgross@xxxxxxxx> wrote:
>
> On 26.11.19 18:17, George Dunlap wrote:
> > 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
>
> I'd rather use -1 for the "not specified" value. This allows to set e.g.
> the maptrack frames to 0 for non-driver domains.

I did wonder whether having 0 frames was something we might want.  I
can certainly change that.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel