Re: Hackbench pipes regression bisected to PSI

[Johannes Weiner]

On Mon, Nov 26, 2018 at 04:54:47PM +0000, Mel Gorman wrote:
> On Mon, Nov 26, 2018 at 11:07:24AM -0500, Johannes Weiner wrote:
> > @@ -509,6 +509,15 @@ config PSI
> >  
> >  	  Say N if unsure.
> >  
> > +config PSI_DEFAULT_DISABLED
> > +	bool "Require boot parameter to enable pressure stall information tracking"
> > +	default n
> > +	depends on PSI
> > +	help
> > +	  If set, pressure stall information tracking will be disabled
> > +	  per default but can be enabled through passing psi_enable=1
> > +	  on the kernel commandline during boot.
> > +
> >  endmenu # "CPU/Task time and stats accounting"
> >  
> 
> Should this default y on the basis that someone only wants the feature if
> they are aware of it? This is not that important as CONFIG_PSI is disabled
> by default and it's up to distribution maintainers to use their brain.

I went with the NUMA balancing example again here, which defaults to
enabling the feature at boot time. IMO that makes sense, as somebody
would presumably first read through the PSI help text, then decide y
on that before being asked the second question. A "yes, but
<stipulations>" for vendor kernels seems more appropriate than
requiring a double yes for other users to simply get the feature.

> > @@ -136,8 +136,18 @@
> >  
> >  static int psi_bug __read_mostly;
> >  
> > -bool psi_disabled __read_mostly;
> > -core_param(psi_disabled, psi_disabled, bool, 0644);
> > +DEFINE_STATIC_KEY_FALSE(psi_disabled);
> > +
> > +#ifdef CONFIG_PSI_DEFAULT_DISABLED
> > +bool psi_enable;
> > +#else
> > +bool psi_enable = true;
> > +#endif
> > +static int __init parse_psi_enable(char *str)
> > +{
> > +	return kstrtobool(str, &psi_enable) == 0;
> > +}
> > +__setup("psi_enable=", parse_psi_enable);
> >  
> 
> Bit late to notice but this switch should be in
> Documentation/admin-guide/kernel-parameters.txt. If you really want to
> match the automatic numa balancing switch then it also should be
> psi=[enable|disable] instead of psi_enable=[1|0]

Done and done, thanks. Updated patch:

---