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, Stefano Stabellini 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");
> 
> ?
> 
> > > +        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.

With a changed name of course, maybe MAX_MAPTRACK_FRAMES.


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