Re: [Xen-devel] [PATCH] domain_create: honour global grant/maptrack frame limits...

["Durrant, Paul" <pdurrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jürgen Groß <jgross@xxxxxxxx>
> Sent: 26 November 2019 11:37
> To: Paul Durrant <pdurrant@xxxxxxxxx>; Durrant, Paul <pdurrant@xxxxxxxxxx>
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>; Julien Grall
> <julien@xxxxxxx>; Wei Liu <wl@xxxxxxx>; Konrad Rzeszutek Wilk
> <konrad.wilk@xxxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxxxxx>;
> Andrew Cooper <andrew.cooper3@xxxxxxxxxx>; Ian Jackson
> <ian.jackson@xxxxxxxxxxxxx>; Jan Beulich <jbeulich@xxxxxxxx>; xen-devel
> <xen-devel@xxxxxxxxxxxxxxxxxxxx>
> Subject: Re: [Xen-devel] [PATCH] domain_create: honour global
> grant/maptrack frame limits...
> 
> On 26.11.19 12:30, Paul Durrant wrote:
> > On Wed, 13 Nov 2019 at 13:55, Paul Durrant <pdurrant@xxxxxxxxxx> wrote:
> >>
> >> ...when their values are larger than the per-domain configured limits.
> >>
> >> Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx>
> >> ---
> >> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> >> Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> >> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> >> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> >> Cc: Julien Grall <julien@xxxxxxx>
> >> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> >> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> >> Cc: Wei Liu <wl@xxxxxxx>
> >>
> >> After mining through commits it is still unclear to me exactly when Xen
> >> stopped honouring the global values, but I really think this commit
> should
> >> be back-ported to stable trees as it was a behavioural change that can
> >> cause domUs to fail in non-obvious ways.
> >
> > Any other opinions on this? AFAICT questions is still open:
> >
> > - Do we consider not honouring the command line values to be a
> > regression (since domUs that would have worked before will no longer
> > work after a basic upgrade of Xen)?
> >
> >    Paul
> >
> >> ---
> >>   xen/common/domain.c | 14 ++++++++++++--
> >>   1 file changed, 12 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/xen/common/domain.c b/xen/common/domain.c
> >> index 611116c7fc..aad6d55b82 100644
> >> --- a/xen/common/domain.c
> >> +++ b/xen/common/domain.c
> >> @@ -335,6 +335,7 @@ struct domain *domain_create(domid_t domid,
> >>       enum { INIT_watchdog = 1u<<1,
> >>              INIT_evtchn = 1u<<3, INIT_gnttab = 1u<<4, INIT_arch =
> 1u<<5 };
> >>       int err, init_status = 0;
> >> +    unsigned int max_grant_frames, max_maptrack_frames;
> >>
> >>       if ( config && (err = sanitise_domain_config(config)) )
> >>           return ERR_PTR(err);
> >> @@ -456,8 +457,17 @@ struct domain *domain_create(domid_t domid,
> >>               goto fail;
> >>           init_status |= INIT_evtchn;
> >>
> >> -        if ( (err = grant_table_init(d, config->max_grant_frames,
> >> -                                     config->max_maptrack_frames)) !=
> 0 )
> >> +        /*
> >> +         * Make sure that the configured values don't reduce any
> >> +         * global command line override.
> >> +         */
> >> +        max_grant_frames = max(config->max_grant_frames,
> >> +                               opt_max_grant_frames);
> >> +        max_maptrack_frames = max(config->max_maptrack_frames,
> >> +                                  opt_max_maptrack_frames);
> >> +
> >> +        if ( (err = grant_table_init(d, max_grant_frames,
> >> +                                     max_maptrack_frames)) != 0 )
> 
> So basically the per-domain settings are ignored.
> 

Basically, yes.

> They are not allowed to be smaller than the global limits (due to
> using max()).
> 
> They are not allowed to be larger than the global limits (due to the
> test in grant_table_init().
> 
> That is _not_ the purpose of being able to control the settings per
> domain.
> 

Ok, if a straight-up return to old behaviour is out then I guess 4.13 will 
carry the regression.

  Paul

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