Re: [RFC PATCH v15 1/7] sched: Add CONFIG_SCHED_PROXY_EXEC & boot argument to enable/disable

John Stultz
2025-03-14T00:49:10Z

On Thu, Mar 13, 2025 at 3:09 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed, 12 Mar 2025 15:11:31 -0700
> John Stultz <jstultz@google.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