linux-kernel - Re: [RFC PATCH v15 1/7] sched: Add CONFIG_SCHED_PROXY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANDhNCr_c0y6v0F5y3H5ZRDxLO4Ah7gD0q25d29YiCvUrUPBXA@mail.gmail.com>
Date: Thu, 13 Mar 2025 17:48:56 -0700
From: John Stultz <jstultz@...gle.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Joel Fernandes <joelagnelf@...dia.com>, 
	Qais Yousef <qyousef@...alina.io>, Ingo Molnar <mingo@...hat.com>, 
	Peter Zijlstra <peterz@...radead.org>, Juri Lelli <juri.lelli@...hat.com>, 
	Vincent Guittot <vincent.guittot@...aro.org>, Dietmar Eggemann <dietmar.eggemann@....com>, 
	Valentin Schneider <vschneid@...hat.com>, Ben Segall <bsegall@...gle.com>, 
	Zimuzo Ezeozue <zezeozue@...gle.com>, Mel Gorman <mgorman@...e.de>, Will Deacon <will@...nel.org>, 
	Waiman Long <longman@...hat.com>, Boqun Feng <boqun.feng@...il.com>, 
	"Paul E. McKenney" <paulmck@...nel.org>, Metin Kaya <Metin.Kaya@....com>, 
	Xuewen Yan <xuewen.yan94@...il.com>, K Prateek Nayak <kprateek.nayak@....com>, 
	Thomas Gleixner <tglx@...utronix.de>, Daniel Lezcano <daniel.lezcano@...aro.org>, 
	Suleiman Souhlal <suleiman@...gle.com>, kernel-team@...roid.com
Subject: Re: [RFC PATCH v15 1/7] sched: Add CONFIG_SCHED_PROXY_EXEC & boot
 argument to enable/disable

On Thu, Mar 13, 2025 at 3:09 AM Steven Rostedt <rostedt@...dmis.org> wrote:
>
> On Wed, 12 Mar 2025 15:11:31 -0700
> John Stultz <jstultz@...gle.com> wrote:
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index fb8752b42ec85..dcc2443078d00 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -6262,6 +6262,11 @@
> >       sa1100ir        [NET]
> >                       See drivers/net/irda/sa1100_ir.c.
> >
> > +     sched_proxy_exec= [KNL]
> > +                     Enables or disables "proxy execution" style
> > +                     solution to mutex-based priority inversion.
> > +                     Format: <bool>
>
> To enable, does this require: sched_proxy_exec=true
>
> Could we just allow it to be:
>
>                 sched_proxy_exec
>
> Also mean true? That is, both of the above would be true, but to
> disable it, you would need: sched_proxy_exec=false.

Currently the flag defaults to true, so I'm not sure if
"sched_proxy_exec" on its own makes as much sense to me.

Though, in the android16-6.12 kernel, I have an additional change that
sets it default to false, which allows "sched_proxy_exec=true" to be
useful.  So I'm open to having the argument alone as an enablement
flag (in addition to the explicit setting), but I've personally always
found the mixed conventions there confusing, preferring the explicit
"=true" or "=1" on boot arguments.


> > diff --git a/init/Kconfig b/init/Kconfig
> > index d0d021b3fa3b3..b989ddc27444e 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -875,6 +875,16 @@ config UCLAMP_BUCKETS_COUNT
> >
> >         If in doubt, use the default value.
> >
> > +config SCHED_PROXY_EXEC
> > +     bool "Proxy Execution"
> > +     default n
>
> Nit, you don't need "default n" because "default n" is the default ;-)

Ah, thanks I'll drop that!

> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index 67189907214d3..3968c3967ec38 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -119,6 +119,35 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(sched_compute_energy_tp);
> >
> >  DEFINE_PER_CPU_SHARED_ALIGNED(struct rq, runqueues);
> >
> > +#ifdef CONFIG_SCHED_PROXY_EXEC
> > +DEFINE_STATIC_KEY_TRUE(__sched_proxy_exec);
> > +static int __init setup_proxy_exec(char *str)
> > +{
> > +     bool proxy_enable;
> > +
> > +     if (kstrtobool(str, &proxy_enable)) {
>
> To make it work without adding =true, the above could be:
>
>         bool proxy_enable = true;
>
>         if (*str && kstrtobool(str, &proxy_enable)) {
>

Ok, I'll give this a shot.

Thanks so much for the review and feedback! Really appreciate it!
-john