Re: [PATCH] sched: Tweak default dynamic preempt mode selection

From: Mike Galbraith <efault@gmx.de>
To: Valentin Schneider <valentin.schneider@arm.com>,
	linux-kernel@vger.kernel.org
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	Frederic Weisbecker <frederic@kernel.org>
Subject: Re: [PATCH] sched: Tweak default dynamic preempt mode selection
Date: Tue, 09 Nov 2021 14:31:03 +0100	[thread overview]
Message-ID: <44e93989570764f074849deaef4bbcc57d1c6891.camel@gmx.de> (raw)
In-Reply-To: <87pmr9ser1.mognet@arm.com>

On Tue, 2021-11-09 at 12:19 +0000, Valentin Schneider wrote:
> On 09/11/21 12:00, Mike Galbraith wrote:
> 
> 
> > -config PREEMPT_NONE
> > +if PREEMPT_DYNAMIC
> > +config PREEMPT
> >       bool
> > 
> 
> On my end this doesn't let PREEMPT_DYNAMIC select PREEMPT, which I came to
> realize we need (places like vermagic.h and ftrace's print_trace_header();
> also it avoids a lot of headaches).

Oops.

> 
> The below lets me have PREEMPT w/ PREEMPT_DYNAMIC. The one annoying thing
> is the Preemption Model prompt remains visible in menuconfig when
> PREEMPT_DYNAMIC is selected, but eh...

Works for me, and looks a hell of a lot better.  Ship it.

> 
> ---
> diff --git a/kernel/Kconfig.preempt b/kernel/Kconfig.preempt
> index 12ac42a3415f..e01588f9de1f 100644
> --- a/kernel/Kconfig.preempt
> +++ b/kernel/Kconfig.preempt
> @@ -1,18 +1,9 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  
> -choice
> -       prompt "Preemption Model"
> -       default PREEMPT_STATIC
> -
> -config PREEMPT_STATIC
> -       bool "Preemption behaviour defined at build"
> -
>  config PREEMPT_DYNAMIC
>         bool "Preemption behaviour defined on boot"
>         depends on HAVE_PREEMPT_DYNAMIC && !ARCH_NO_PREEMPT
>         select PREEMPT
> -       select PREEMPTION
> -       select UNINLINE_SPIN_UNLOCK if !ARCH_INLINE_SPIN_UNLOCK
>         help
>           This option allows to define the preemption model on the kernel
>           command line parameter and thus override the default preemption
> @@ -28,15 +19,14 @@ config PREEMPT_DYNAMIC
>  
>           Interesting if you want the same pre-built kernel should be used for
>           both Server and Desktop workloads.
> -endchoice
>  
> -if PREEMPT_STATIC
>  choice
> -       prompt "Preemption Flavor"
> +       prompt "Preemption Model"
>         default PREEMPT_NONE
>  
>  config PREEMPT_NONE
>         bool "No Forced Preemption (Server)"
> +       depends on !PREEMPT_DYNAMIC
>         help
>           This is the traditional Linux preemption model, geared towards
>           throughput. It will still provide good latencies most of the
> @@ -50,7 +40,7 @@ config PREEMPT_NONE
>  
>  config PREEMPT_VOLUNTARY
>         bool "Voluntary Kernel Preemption (Desktop)"
> -       depends on !ARCH_NO_PREEMPT
> +       depends on !ARCH_NO_PREEMPT && !PREEMPT_DYNAMIC
>         help
>           This option reduces the latency of the kernel by adding more
>           "explicit preemption points" to the kernel code. These new
> @@ -88,7 +78,7 @@ config PREEMPT
>  
>  config PREEMPT_RT
>         bool "Fully Preemptible Kernel (Real-Time)"
> -       depends on EXPERT && ARCH_SUPPORTS_RT
> +       depends on EXPERT && ARCH_SUPPORTS_RT && !PREEMPT_DYNAMIC
>         select PREEMPTION
>         help
>           This option turns the kernel into a real-time kernel by replacing
> @@ -104,14 +94,10 @@ config PREEMPT_RT
>           require real-time guarantees.
>  
>  endchoice
> -endif # PREEMPT_STATIC
> -
> -if PREEMPT_DYNAMIC
> -config PREEMPT
> -       bool
>  
>  choice
> -       prompt "Boot Time Preemption Flavor"
> +       prompt "Boot Time Preemption Model"
> +       depends on PREEMPT_DYNAMIC
>         default PREEMPT_NONE_BEHAVIOR
>  
>  config PREEMPT_NONE_BEHAVIOR
> @@ -123,7 +109,6 @@ config PREEMPT_VOLUNTARY_BEHAVIOR
>  config PREEMPT_BEHAVIOR
>         bool "Preemptible Kernel (Low-Latency Desktop)"
>  endchoice
> -endif # PREEMPT_DYNAMIC
>  
>  config PREEMPT_COUNT
>         bool
> @@ -149,5 +134,3 @@ config SCHED_CORE
>           SCHED_CORE is default disabled. When it is enabled and unused,
>           which is the likely usage by Linux distributions, there should
>           be no measurable impact on performance.
> -
> -