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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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");

?

> > +        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.


> > +        } else {
> 
> Coding style.
> 
> > +            printk(XENLOG_WARNING "dropping gnttab_max_nr_frames as 
> > superseding options are present\n");
> 
> "ignoring ..."
> 
> Also please make sure to issue at most one warning message.

OK


> > +        }
> > +    }
> > +
> > +    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.


> 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 

OK, I can do that.

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