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

From: Valentin Schneider <valentin.schneider@arm.com>
To: Mike Galbraith <efault@gmx.de>, 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 12:19:30 +0000	[thread overview]
Message-ID: <87pmr9ser1.mognet@arm.com> (raw)
In-Reply-To: <1905cf613576d04f585d752d85ce21a3504a40d6.camel@gmx.de>

On 09/11/21 12:00, Mike Galbraith wrote:
> On Tue, 2021-11-09 at 09:52 +0000, Valentin Schneider wrote:
>> On 09/11/21 06:30, Mike Galbraith wrote:
>> > Not seeing your v2 land yet, I grabbed my mallet and had a go at goal
>> > reconciliation over morning java.  Non-lovely result seems to work.
>> >
>>
>> Yeah so I went down a debatable path, gave up on that and started something
>> different, and gave up on that because it was late :-)
>>
>> Now interestingly my second attempt is pretty close to what you have
>> below.
>
> Well that's a shame, because while it seems to function, it also puts
> PREEMPT_DYNAMIC in a pretty darn similar spot to the one PREEMPT_RT was
> in.  Drat.
>

Yep...

>> > sched, Kconfig: Fix preemption model selection
>> >
>> > Switch PREEMPT_DYNAMIC/PREEMPT_RT dependency around so PREEMPT_RT
>> > can be selected during the initial preemption model selection.
>> > Further, since PREEMPT_DYNAMIC requires PREEMPT, make it depend
>> > upon it instead of selecting it, and add a menu to allow selection
>> > of the boot time behavior, this to allow arches that do not support
>> > PREEMPT_DYNAMIC to retain their various configs untouched.
>> >
>>
>> Have some nits below, but otherwise where I stand right now I think it's
>> the least ugly way of tackling this :)
>
> We're supposed to be going for _least_ ugly?  Oh ;)  This is really a
> job for someone who knows their way around Kconfig-land, but since I'm
> not hearing "Yawn, here ya go", it gets whacked with a mallet until one
> of us gives up.
>

:)

> -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).

AIUI this is because the 'if <expr>' is equivalent to appending
  depends on <expr>
to every choice item.

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...

---

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.
-
-
