ACK/Cmnt: [SRU][F][PATCH 1/1] Ubuntu: [Config] CONFIG_PSI is enabled by default, but creates additional overhead on s390x, hence should be disabled by default on s390x only.

[Seth Forshee]

On Wed, May 13, 2020 at 11:17:07AM +0200, Stefan Bader wrote:
> On 05.05.20 21:20, frank.heimes at canonical.com wrote:
> > From: Frank Heimes <frank.heimes at canonical.com>
> > 
> > BugLink: https://bugs.launchpad.net/bugs/1876044
> > 
> > PSI is always enabled in Ubuntu due to 'CONFIG_PSI_DEFAULT_DISABLED=n' / 'not set'.
> > This causes a performance degradation for popular workloads like web serving (nginx) due to additional overhead.
> > Hence IBM is requesting to change this option to yes for s390x: 'CONFIG_PSI_DEFAULT_DISABLED=y'.
> > CONFIG_PSI is already set to yes, and should stay.
> > 
> > Signed-off-by: Frank Heimes <frank.heimes at canonical.com>
> Acked-by: Stefan Bader <stefan.bader at canonical.com>
> > ---
> 
> I would not know of any reason and nobody spoke up so far. I guess this option
> is only relevant for s390x and therefor enforcing the change there makes the
> config change go into the common file. Andy, is there a better way to record a
> don't care for other arches in the annotations file? Like 'amd64': '-'.

Do you mean by this that you want something that would enforce the value
for only s390x without enforcing it for other architectures? I don't
think '-' would work for this, because iirc that says that the option is
not selectable, and the enforcer will not treat it as equal to 'n'.

> 
> -Stefan
> 
> >  debian.master/config/annotations          | 4 +++-
> >  debian.master/config/config.common.ubuntu | 2 +-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/debian.master/config/annotations b/debian.master/config/annotations
> > index dd6481141e9f..20fd9e21bc72 100644
> > --- a/debian.master/config/annotations
> > +++ b/debian.master/config/annotations
> > @@ -10316,7 +10316,9 @@ CONFIG_TASK_DELAY_ACCT                          policy<{'amd64': 'y', 'arm64': '
> >  CONFIG_TASK_XACCT                               policy<{'amd64': 'y', 'arm64': 'y', 'armhf': 'y', 'i386': 'y', 'ppc64el': 'y', 's390x': 'y'}>
> >  CONFIG_TASK_IO_ACCOUNTING                       policy<{'amd64': 'y', 'arm64': 'y', 'armhf': 'y', 'i386': 'y', 'ppc64el': 'y', 's390x': 'y'}>
> >  CONFIG_PSI                                      policy<{'amd64': 'y', 'arm64': 'y', 'armhf': 'y', 'i386': 'y', 'ppc64el': 'y', 's390x': 'y'}>
> > -CONFIG_PSI_DEFAULT_DISABLED                     policy<{'amd64': 'n', 'arm64': 'n', 'armhf': 'n', 'i386': 'n', 'ppc64el': 'n', 's390x': 'n'}>
> > +CONFIG_PSI_DEFAULT_DISABLED                     policy<{'amd64': 'n', 'arm64': 'n', 'armhf': 'n', 'i386': 'n', 'ppc64el': 'n', 's390x': 'y'}>
> > +
> > +CONFIG_PSI_DEFAULT_DISABLED                     mark<LP:1876044>

This is wrong, but numbers should be recorded in note<> annotations. I
would think we would also want mark<ENFORCE> here.

> >  
> >  # Menu: General setup >> CPU/Task time and stats accounting >> Cputime accounting
> >  CONFIG_TICK_CPU_ACCOUNTING                      policy<{'amd64': 'y', 'arm64': 'y', 'armhf': 'y', 'i386': 'y', 'ppc64el': 'y', }>
> > diff --git a/debian.master/config/config.common.ubuntu b/debian.master/config/config.common.ubuntu
> > index f502f2bdfa07..115590e89d73 100644
> > --- a/debian.master/config/config.common.ubuntu
> > +++ b/debian.master/config/config.common.ubuntu
> > @@ -7567,7 +7567,7 @@ CONFIG_PSAMPLE=m
> >  CONFIG_PSERIES_CPUIDLE=y
> >  CONFIG_PSERIES_ENERGY=m
> >  CONFIG_PSI=y
> > -# CONFIG_PSI_DEFAULT_DISABLED is not set
> > +CONFIG_PSI_DEFAULT_DISABLED=y
> >  # CONFIG_PSTORE_842_COMPRESS is not set
> >  CONFIG_PSTORE_COMPRESS=y
> >  CONFIG_PSTORE_COMPRESS_DEFAULT="deflate"
> > 
> 
> 




> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team