Re: [Xen-devel] [PATCH v6 1/8] xen: introduce gnttab_max_frames and gnttab_max_maptrack_frames command line options

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.10.14 at 18:56, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> On Thu, 16 Oct 2014, Jan Beulich wrote:
>> > @@ -2930,6 +2935,24 @@ static struct keyhandler 
>> > gnttab_usage_print_all_keyhandler = {
>> >  
>> >  static int __init gnttab_usage_init(void)
>> >  {
>> > +    if ( max_nr_grant_frames )
>> > +    {
>> > +        printk(XENLOG_WARNING "gnttab_max_nr_frames is deprecated, use 
> gnttab_max_frames instead\n");
>> 
>> Long line (only keep the string literal together).
> 
> Are you telling me that you actually prefer:
> 
> printk(XENLOG_WARNING
>        "gnttab_max_nr_frames is deprecated, use gnttab_max_frames 
> instead\n");
> 
> ?

Yes.

>> > +        if ( !max_grant_frames && !max_maptrack_frames )
>> > +        {
>> > +            max_grant_frames = max_nr_grant_frames;
>> > +            max_maptrack_frames = max_nr_grant_frames * 
>> > MAX_MAPTRACK_TO_GRANTS_RATIO;
>> 
>> Each unspecified field should be set independently when the
>> respective new option wasn't present, yet the deprecated one
>> was.
> 
> Sorry, I don't understand what you mean.

        if ( !max_grant_frames )
            max_grant_frames = max_nr_grant_frames;
        if ( !max_maptrack_frames )
            max_maptrack_frames = max_nr_grant_frames * 
MAX_MAPTRACK_TO_GRANTS_RATIO;

>> > +        }
>> > +    }
>> > +
>> > +    if ( !max_grant_frames )
>> > +        max_grant_frames = DEFAULT_MAX_NR_GRANT_FRAMES;
>> > +
>> > +    if ( !max_maptrack_frames )
>> > +        max_maptrack_frames = MAX_MAPTRACK_TO_GRANTS_RATIO * 
>> > max_grant_frames;
>> 
>> As said in the earlier reply, this is specifically what you shouldn't do
>> when you lower the current default.
> 
> I am not lowering the current default, not on this patch.
> This patch only introduces new options and keep the defaults as before.

But the patch that does the lowering doesn't take care of this either.

>> This has to remain at no lower
>> a default than what we currently have (and should also no longer
>> be tied to max_grant_frames).
>>
>> I.e. you'll need a separate new
>> default value here if you really want to reduce the other one (which,
>> as also said, I'm not certain is a good idea).
> 
> I understand now. You are saying that for the sake of the following
> patch lowering the default, I should turn MAX_MAPTRACK_TO_GRANTS_RATIO
> into
> 
> #define MAX_MAPTRACK_TO_GRANTS_RATIO 256 

No. The ratio should be left unchanged too, as it's being used above
(not sure about elsewhere). You should have a standalone new
default for the maptrack table size.

Jan


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