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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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?

Jan


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