Re: [Xen-devel] [PATCH v4 1/7] xen: introduce gnttab_max_nr_maptrack_frames command line option

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 13 Oct 2014, Jan Beulich wrote:
> >>> On 13.10.14 at 11:59, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> > On Fri, 10 Oct 2014, Jan Beulich wrote:
> >> >>> On 10.10.14 at 13:43, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> >> > --- a/xen/common/grant_table.c
> >> > +++ b/xen/common/grant_table.c
> >> > @@ -102,10 +102,9 @@ nr_maptrack_frames(struct grant_table *t)
> >> >      return t->maptrack_limit / MAPTRACK_PER_PAGE;
> >> >  }
> >> >  
> >> > -static unsigned inline int max_nr_maptrack_frames(void)
> >> > -{
> >> > -    return (max_nr_grant_frames * MAX_MAPTRACK_TO_GRANTS_RATIO);
> >> > -}
> >> > +static unsigned int max_nr_maptrack_frames = 
> >> > DEFAULT_MAX_NR_GRANT_FRAMES *
> >> > +                                             
> >> > MAX_MAPTRACK_TO_GRANTS_RATIO;
> >> > +integer_param("gnttab_max_nr_maptrack_frames", max_nr_maptrack_frames);
> >> 
> >> I'm not sure: As said before, the primary goal must be that existing
> >> setups don't suddenly start failing. I.e. the other options no longer
> >> controlling the maptrack table size may badly affect Dom0. One
> >> possibility would be to honor the other option in the original way if
> >> the new option wasn't made use of (and perhaps issue a warning
> >> to that effect).
> > 
> > Honestly I think that it would be a bad idea to try to second guess what
> > the user really meant with the options she passed. Also I think it is a
> > bad idea to have options do different things depending on the presence
> > or absence of other options.
> 
> I agree this is not optimal, but I suppose you also agree that it is
> not really acceptable to break working systems.
> 
> > I think we should either:
> > 
> > - leave things as they are
> 
> Only as long as this doesn't result in (perceived) regressions. I.e.
> not an option here afaict.
> 
> > - add this new option and as a consequence also change the effect of
> > gnttab_max_nr_frames
> 
> We could deprecate the old option altogether (and have it control
> both values unless one of the new options got used), and introduce
> a new option "gnttab_max_frames" controlling just the one table's
> size. Does that sound any better?
 
Yes, that's better.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel