2021-11-05 12:12:41

by Valentin Schneider

[permalink] [raw]
Subject: [PATCH] sched: Tweak default dynamic preempt mode selection

Commit c597bfddc9e9 ("sched: Provide Kconfig support for default dynamic
preempt mode") changed the selectable config names for the preemption
model. This means a config file must now select

CONFIG_PREEMPT_BEHAVIOUR=y

rather than

CONFIG_PREEMPT=y

to get a preemptible kernel. This means all arch config files need to be
updated - right now arm64 defconfig selects CONFIG_PREEMPT=y but ends up
with CONFIG_PREEMPT_NONE_BEHAVIOUR=y.

Instead, have CONFIG_*PREEMPT be the selectable configs again, and make
them select their _BEHAVIOUR equivalent if CONFIG_PREEMPT_DYNAMIC is set.

Signed-off-by: Valentin Schneider <[email protected]>
---
kernel/Kconfig.preempt | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/kernel/Kconfig.preempt b/kernel/Kconfig.preempt
index 60f1bfc3c7b2..25e8d6a3d9fa 100644
--- a/kernel/Kconfig.preempt
+++ b/kernel/Kconfig.preempt
@@ -1,12 +1,21 @@
# SPDX-License-Identifier: GPL-2.0-only

+config PREEMPT_NONE_BEHAVIOUR
+ bool
+
+config PREEMPT_VOLUNTARY_BEHAVIOUR
+ bool
+
+config PREEMPT_BEHAVIOUR
+ bool
+
choice
prompt "Preemption Model"
- default PREEMPT_NONE_BEHAVIOUR
+ default PREEMPT_NONE

-config PREEMPT_NONE_BEHAVIOUR
+config PREEMPT_NONE
bool "No Forced Preemption (Server)"
- select PREEMPT_NONE if !PREEMPT_DYNAMIC
+ select PREEMPT_NONE_BEHAVIOUR if PREEMPT_DYNAMIC
help
This is the traditional Linux preemption model, geared towards
throughput. It will still provide good latencies most of the
@@ -18,10 +27,10 @@ config PREEMPT_NONE_BEHAVIOUR
raw processing power of the kernel, irrespective of scheduling
latencies.

-config PREEMPT_VOLUNTARY_BEHAVIOUR
+config PREEMPT_VOLUNTARY
bool "Voluntary Kernel Preemption (Desktop)"
depends on !ARCH_NO_PREEMPT
- select PREEMPT_VOLUNTARY if !PREEMPT_DYNAMIC
+ select PREEMPT_VOLUNTARY_BEHAVIOUR if PREEMPT_DYNAMIC
help
This option reduces the latency of the kernel by adding more
"explicit preemption points" to the kernel code. These new
@@ -37,10 +46,12 @@ config PREEMPT_VOLUNTARY_BEHAVIOUR

Select this if you are building a kernel for a desktop system.

-config PREEMPT_BEHAVIOUR
+config PREEMPT
bool "Preemptible Kernel (Low-Latency Desktop)"
depends on !ARCH_NO_PREEMPT
- select PREEMPT
+ select PREEMPTION
+ select UNINLINE_SPIN_UNLOCK if !ARCH_INLINE_SPIN_UNLOCK
+ select PREEMPT_BEHAVIOUR if PREEMPT_DYNAMIC
help
This option reduces the latency of the kernel by making
all kernel code (that is not executing in a critical section)
@@ -75,17 +86,6 @@ config PREEMPT_RT

endchoice

-config PREEMPT_NONE
- bool
-
-config PREEMPT_VOLUNTARY
- bool
-
-config PREEMPT
- bool
- select PREEMPTION
- select UNINLINE_SPIN_UNLOCK if !ARCH_INLINE_SPIN_UNLOCK
-
config PREEMPT_COUNT
bool

--
2.25.1


2021-11-06 11:08:17

by Mike Galbraith

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

On Fri, 2021-11-05 at 10:40 +0000, Valentin Schneider wrote:
> Commit c597bfddc9e9 ("sched: Provide Kconfig support for default dynamic
> preempt mode") changed the selectable config names for the preemption
> model. This means a config file must now select
>
>   CONFIG_PREEMPT_BEHAVIOUR=y
>
> rather than
>
>   CONFIG_PREEMPT=y
>
> to get a preemptible kernel. This means all arch config files need to be
> updated - right now arm64 defconfig selects CONFIG_PREEMPT=y but ends up
> with CONFIG_PREEMPT_NONE_BEHAVIOUR=y.
>
> Instead, have CONFIG_*PREEMPT be the selectable configs again, and make
> them select their _BEHAVIOUR equivalent if CONFIG_PREEMPT_DYNAMIC is set.


Is there any way to get to PREEMPT_RT in the first selection again as
well? I had created a behavior entry for RT (below) and inverted the
dependency to make it appear in the initial selection again, but that's
clearly not gonna fly.

Starting with a 5.15 config, to select RT you currently must first
select a model you don't want, then reject PREEMPT_DYNAMIC and you'll
be offered the full menu of models immediately. With your patch added,
that became worse. After rejecting PREEMPT_DYNAMIC, I had to go
through new 5.15+ options before finally being offered the full menu.

-Mike

--- a/kernel/Kconfig.preempt
+++ b/kernel/Kconfig.preempt
@@ -56,10 +56,10 @@ config PREEMPT_BEHAVIOUR
embedded system with latency requirements in the
milliseconds
range.

-config PREEMPT_RT
+config PREEMPT_RT_BEHAVIOR
bool "Fully Preemptible Kernel (Real-Time)"
- depends on EXPERT && ARCH_SUPPORTS_RT && !PREEMPT_DYNAMIC
- select PREEMPTION
+ depends on EXPERT && ARCH_SUPPORTS_RT
+ select PREEMPT_RT
help
This option turns the kernel into a real-time kernel by
replacing
various locking primitives (spinlocks, rwlocks, etc.) with
@@ -86,6 +86,10 @@ config PREEMPT
select PREEMPTION
select UNINLINE_SPIN_UNLOCK if !ARCH_INLINE_SPIN_UNLOCK

+config PREEMPT_RT
+ bool
+ select PREEMPTION
+
config PREEMPT_COUNT
bool

@@ -101,7 +105,7 @@ config PREEMPT_LAZY

config PREEMPT_DYNAMIC
bool "Preemption behaviour defined on boot"
- depends on HAVE_PREEMPT_DYNAMIC
+ depends on HAVE_PREEMPT_DYNAMIC && !PREEMPT_RT
select PREEMPT
default y
help



2021-11-06 12:18:01

by kernel test robot

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

Hi Valentin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tip/master]
[also build test WARNING on linus/master next-20211106]
[cannot apply to linux/master v5.15]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Valentin-Schneider/sched-Tweak-default-dynamic-preempt-mode-selection/20211105-184135
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git ea79c24a30aa27ccc4aac26be33f8b73f3f1f59c
config: x86_64-defconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/4731291127aa2100c984229a91533b671044a74b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Valentin-Schneider/sched-Tweak-default-dynamic-preempt-mode-selection/20211105-184135
git checkout 4731291127aa2100c984229a91533b671044a74b
# save the attached .config to linux build tree
make W=1 ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

kernel/sched/core.c:3439:6: warning: no previous prototype for 'sched_set_stop_task' [-Wmissing-prototypes]
3439 | void sched_set_stop_task(int cpu, struct task_struct *stop)
| ^~~~~~~~~~~~~~~~~~~
In file included from <command-line>:
kernel/sched/core.c: In function 'sched_dynamic_update':
include/linux/static_call_types.h:15:34: error: '__SCT__might_resched' undeclared (first use in this function); did you mean '__SCT__cond_resched'?
15 | #define STATIC_CALL_TRAMP_PREFIX __SCT__
| ^~~~~~~
include/linux/compiler_types.h:59:23: note: in definition of macro '___PASTE'
59 | #define ___PASTE(a,b) a##b
| ^
include/linux/static_call_types.h:18:34: note: in expansion of macro '__PASTE'
18 | #define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
| ^~~~~~~
include/linux/static_call_types.h:18:42: note: in expansion of macro 'STATIC_CALL_TRAMP_PREFIX'
18 | #define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/linux/static_call.h:154:10: note: in expansion of macro 'STATIC_CALL_TRAMP'
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^~~~~~~~~~~~~~~~~
kernel/sched/core.c:6575:2: note: in expansion of macro 'static_call_update'
6575 | static_call_update(might_resched, __cond_resched);
| ^~~~~~~~~~~~~~~~~~
include/linux/static_call_types.h:15:34: note: each undeclared identifier is reported only once for each function it appears in
15 | #define STATIC_CALL_TRAMP_PREFIX __SCT__
| ^~~~~~~
include/linux/compiler_types.h:59:23: note: in definition of macro '___PASTE'
59 | #define ___PASTE(a,b) a##b
| ^
include/linux/static_call_types.h:18:34: note: in expansion of macro '__PASTE'
18 | #define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
| ^~~~~~~
include/linux/static_call_types.h:18:42: note: in expansion of macro 'STATIC_CALL_TRAMP_PREFIX'
18 | #define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/linux/static_call.h:154:10: note: in expansion of macro 'STATIC_CALL_TRAMP'
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^~~~~~~~~~~~~~~~~
kernel/sched/core.c:6575:2: note: in expansion of macro 'static_call_update'
6575 | static_call_update(might_resched, __cond_resched);
| ^~~~~~~~~~~~~~~~~~
In file included from include/linux/tracepoint.h:22,
from include/trace/events/sched.h:10,
from kernel/sched/core.c:10:
>> include/linux/static_call.h:154:41: warning: initialization of 'int' from 'int (*)(void)' makes integer from pointer without a cast [-Wint-conversion]
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^
kernel/sched/core.c:6575:2: note: in expansion of macro 'static_call_update'
6575 | static_call_update(might_resched, __cond_resched);
| ^~~~~~~~~~~~~~~~~~
In file included from <command-line>:
include/linux/static_call_types.h:9:33: error: '__SCK__might_resched' undeclared (first use in this function); did you mean '__SCK__cond_resched'?
9 | #define STATIC_CALL_KEY_PREFIX __SCK__
| ^~~~~~~
include/linux/compiler_types.h:59:23: note: in definition of macro '___PASTE'
59 | #define ___PASTE(a,b) a##b
| ^
include/linux/static_call_types.h:12:32: note: in expansion of macro '__PASTE'
12 | #define STATIC_CALL_KEY(name) __PASTE(STATIC_CALL_KEY_PREFIX, name)
| ^~~~~~~
include/linux/static_call_types.h:12:40: note: in expansion of macro 'STATIC_CALL_KEY_PREFIX'
12 | #define STATIC_CALL_KEY(name) __PASTE(STATIC_CALL_KEY_PREFIX, name)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/static_call.h:155:24: note: in expansion of macro 'STATIC_CALL_KEY'
155 | __static_call_update(&STATIC_CALL_KEY(name), \
| ^~~~~~~~~~~~~~~
kernel/sched/core.c:6575:2: note: in expansion of macro 'static_call_update'
6575 | static_call_update(might_resched, __cond_resched);
| ^~~~~~~~~~~~~~~~~~
In file included from include/linux/tracepoint.h:22,
from include/trace/events/sched.h:10,
from kernel/sched/core.c:10:
>> include/linux/static_call.h:156:39: warning: passing argument 3 of '__static_call_update' makes pointer from integer without a cast [-Wint-conversion]
156 | STATIC_CALL_TRAMP_ADDR(name), __F); \
| ^~~
| |
| int
kernel/sched/core.c:6575:2: note: in expansion of macro 'static_call_update'
6575 | static_call_update(might_resched, __cond_resched);
| ^~~~~~~~~~~~~~~~~~
include/linux/static_call.h:177:82: note: expected 'void *' but argument is of type 'int'
177 | extern void __static_call_update(struct static_call_key *key, void *tramp, void *func);
| ~~~~~~^~~~
In file included from <command-line>:
include/linux/static_call_types.h:15:34: error: '__SCT__preempt_schedule' undeclared (first use in this function)
15 | #define STATIC_CALL_TRAMP_PREFIX __SCT__
| ^~~~~~~
include/linux/compiler_types.h:59:23: note: in definition of macro '___PASTE'
59 | #define ___PASTE(a,b) a##b
| ^
include/linux/static_call_types.h:18:34: note: in expansion of macro '__PASTE'
18 | #define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
| ^~~~~~~
include/linux/static_call_types.h:18:42: note: in expansion of macro 'STATIC_CALL_TRAMP_PREFIX'
18 | #define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/linux/static_call.h:154:10: note: in expansion of macro 'STATIC_CALL_TRAMP'
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^~~~~~~~~~~~~~~~~
kernel/sched/core.c:6576:2: note: in expansion of macro 'static_call_update'
6576 | static_call_update(preempt_schedule, __preempt_schedule_func);
| ^~~~~~~~~~~~~~~~~~
In file included from include/linux/tracepoint.h:22,
from include/trace/events/sched.h:10,
from kernel/sched/core.c:10:
kernel/sched/core.c:6576:39: error: '__preempt_schedule_func' undeclared (first use in this function); did you mean 'preempt_schedule_irq'?
6576 | static_call_update(preempt_schedule, __preempt_schedule_func);
| ^~~~~~~~~~~~~~~~~~~~~~~
include/linux/static_call.h:154:42: note: in definition of macro 'static_call_update'
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^~~~
In file included from <command-line>:
include/linux/static_call_types.h:9:33: error: '__SCK__preempt_schedule' undeclared (first use in this function)
9 | #define STATIC_CALL_KEY_PREFIX __SCK__
| ^~~~~~~
include/linux/compiler_types.h:59:23: note: in definition of macro '___PASTE'
59 | #define ___PASTE(a,b) a##b
| ^
include/linux/static_call_types.h:12:32: note: in expansion of macro '__PASTE'
12 | #define STATIC_CALL_KEY(name) __PASTE(STATIC_CALL_KEY_PREFIX, name)
| ^~~~~~~
include/linux/static_call_types.h:12:40: note: in expansion of macro 'STATIC_CALL_KEY_PREFIX'
12 | #define STATIC_CALL_KEY(name) __PASTE(STATIC_CALL_KEY_PREFIX, name)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/static_call.h:155:24: note: in expansion of macro 'STATIC_CALL_KEY'
155 | __static_call_update(&STATIC_CALL_KEY(name), \
| ^~~~~~~~~~~~~~~
kernel/sched/core.c:6576:2: note: in expansion of macro 'static_call_update'
6576 | static_call_update(preempt_schedule, __preempt_schedule_func);
| ^~~~~~~~~~~~~~~~~~
In file included from include/linux/tracepoint.h:22,
from include/trace/events/sched.h:10,
from kernel/sched/core.c:10:
>> include/linux/static_call.h:156:39: warning: passing argument 3 of '__static_call_update' makes pointer from integer without a cast [-Wint-conversion]
156 | STATIC_CALL_TRAMP_ADDR(name), __F); \
| ^~~
| |
| int
kernel/sched/core.c:6576:2: note: in expansion of macro 'static_call_update'
6576 | static_call_update(preempt_schedule, __preempt_schedule_func);
| ^~~~~~~~~~~~~~~~~~
include/linux/static_call.h:177:82: note: expected 'void *' but argument is of type 'int'
177 | extern void __static_call_update(struct static_call_key *key, void *tramp, void *func);
| ~~~~~~^~~~
In file included from <command-line>:
include/linux/static_call_types.h:15:34: error: '__SCT__preempt_schedule_notrace' undeclared (first use in this function)
15 | #define STATIC_CALL_TRAMP_PREFIX __SCT__
| ^~~~~~~
include/linux/compiler_types.h:59:23: note: in definition of macro '___PASTE'
59 | #define ___PASTE(a,b) a##b
| ^
include/linux/static_call_types.h:18:34: note: in expansion of macro '__PASTE'
18 | #define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
| ^~~~~~~
include/linux/static_call_types.h:18:42: note: in expansion of macro 'STATIC_CALL_TRAMP_PREFIX'
18 | #define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/linux/static_call.h:154:10: note: in expansion of macro 'STATIC_CALL_TRAMP'
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^~~~~~~~~~~~~~~~~
kernel/sched/core.c:6577:2: note: in expansion of macro 'static_call_update'
6577 | static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
| ^~~~~~~~~~~~~~~~~~
In file included from include/linux/tracepoint.h:22,
from include/trace/events/sched.h:10,
from kernel/sched/core.c:10:
kernel/sched/core.c:6577:47: error: '__preempt_schedule_notrace_func' undeclared (first use in this function)
6577 | static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/static_call.h:154:42: note: in definition of macro 'static_call_update'
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^~~~
In file included from <command-line>:
include/linux/static_call_types.h:9:33: error: '__SCK__preempt_schedule_notrace' undeclared (first use in this function)
9 | #define STATIC_CALL_KEY_PREFIX __SCK__
| ^~~~~~~
include/linux/compiler_types.h:59:23: note: in definition of macro '___PASTE'
59 | #define ___PASTE(a,b) a##b
| ^
include/linux/static_call_types.h:12:32: note: in expansion of macro '__PASTE'
12 | #define STATIC_CALL_KEY(name) __PASTE(STATIC_CALL_KEY_PREFIX, name)
| ^~~~~~~
include/linux/static_call_types.h:12:40: note: in expansion of macro 'STATIC_CALL_KEY_PREFIX'
12 | #define STATIC_CALL_KEY(name) __PASTE(STATIC_CALL_KEY_PREFIX, name)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/static_call.h:155:24: note: in expansion of macro 'STATIC_CALL_KEY'
155 | __static_call_update(&STATIC_CALL_KEY(name), \
| ^~~~~~~~~~~~~~~
kernel/sched/core.c:6577:2: note: in expansion of macro 'static_call_update'
6577 | static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
| ^~~~~~~~~~~~~~~~~~
In file included from include/linux/tracepoint.h:22,
from include/trace/events/sched.h:10,
from kernel/sched/core.c:10:
>> include/linux/static_call.h:156:39: warning: passing argument 3 of '__static_call_update' makes pointer from integer without a cast [-Wint-conversion]
156 | STATIC_CALL_TRAMP_ADDR(name), __F); \
| ^~~
| |
| int
kernel/sched/core.c:6577:2: note: in expansion of macro 'static_call_update'
6577 | static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
| ^~~~~~~~~~~~~~~~~~
include/linux/static_call.h:177:82: note: expected 'void *' but argument is of type 'int'
177 | extern void __static_call_update(struct static_call_key *key, void *tramp, void *func);
| ~~~~~~^~~~
>> include/linux/static_call.h:154:41: warning: initialization of 'int' from 'void *' makes integer from pointer without a cast [-Wint-conversion]
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^
kernel/sched/core.c:6583:3: note: in expansion of macro 'static_call_update'
6583 | static_call_update(might_resched, (void *)&__static_call_return0);
| ^~~~~~~~~~~~~~~~~~
>> include/linux/static_call.h:156:39: warning: passing argument 3 of '__static_call_update' makes pointer from integer without a cast [-Wint-conversion]
156 | STATIC_CALL_TRAMP_ADDR(name), __F); \
| ^~~
| |
| int
kernel/sched/core.c:6583:3: note: in expansion of macro 'static_call_update'
6583 | static_call_update(might_resched, (void *)&__static_call_return0);
| ^~~~~~~~~~~~~~~~~~
include/linux/static_call.h:177:82: note: expected 'void *' but argument is of type 'int'
177 | extern void __static_call_update(struct static_call_key *key, void *tramp, void *func);
| ~~~~~~^~~~
>> include/linux/static_call.h:154:41: warning: initialization of 'int' from 'void *' makes integer from pointer without a cast [-Wint-conversion]
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^
kernel/sched/core.c:6584:3: note: in expansion of macro 'static_call_update'
6584 | static_call_update(preempt_schedule, NULL);
| ^~~~~~~~~~~~~~~~~~
>> include/linux/static_call.h:156:39: warning: passing argument 3 of '__static_call_update' makes pointer from integer without a cast [-Wint-conversion]
156 | STATIC_CALL_TRAMP_ADDR(name), __F); \
| ^~~
| |
| int
kernel/sched/core.c:6584:3: note: in expansion of macro 'static_call_update'
6584 | static_call_update(preempt_schedule, NULL);
| ^~~~~~~~~~~~~~~~~~
include/linux/static_call.h:177:82: note: expected 'void *' but argument is of type 'int'
177 | extern void __static_call_update(struct static_call_key *key, void *tramp, void *func);
| ~~~~~~^~~~
>> include/linux/static_call.h:154:41: warning: initialization of 'int' from 'void *' makes integer from pointer without a cast [-Wint-conversion]
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^
kernel/sched/core.c:6585:3: note: in expansion of macro 'static_call_update'
6585 | static_call_update(preempt_schedule_notrace, NULL);
| ^~~~~~~~~~~~~~~~~~
>> include/linux/static_call.h:156:39: warning: passing argument 3 of '__static_call_update' makes pointer from integer without a cast [-Wint-conversion]
156 | STATIC_CALL_TRAMP_ADDR(name), __F); \
| ^~~
| |
| int
kernel/sched/core.c:6585:3: note: in expansion of macro 'static_call_update'
6585 | static_call_update(preempt_schedule_notrace, NULL);
| ^~~~~~~~~~~~~~~~~~
include/linux/static_call.h:177:82: note: expected 'void *' but argument is of type 'int'
177 | extern void __static_call_update(struct static_call_key *key, void *tramp, void *func);
| ~~~~~~^~~~
>> include/linux/static_call.h:154:41: warning: initialization of 'int' from 'int (*)(void)' makes integer from pointer without a cast [-Wint-conversion]
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^
kernel/sched/core.c:6592:3: note: in expansion of macro 'static_call_update'
6592 | static_call_update(might_resched, __cond_resched);
| ^~~~~~~~~~~~~~~~~~
>> include/linux/static_call.h:156:39: warning: passing argument 3 of '__static_call_update' makes pointer from integer without a cast [-Wint-conversion]
156 | STATIC_CALL_TRAMP_ADDR(name), __F); \
| ^~~
| |
| int
kernel/sched/core.c:6592:3: note: in expansion of macro 'static_call_update'
6592 | static_call_update(might_resched, __cond_resched);
| ^~~~~~~~~~~~~~~~~~
include/linux/static_call.h:177:82: note: expected 'void *' but argument is of type 'int'
177 | extern void __static_call_update(struct static_call_key *key, void *tramp, void *func);
| ~~~~~~^~~~
>> include/linux/static_call.h:154:41: warning: initialization of 'int' from 'void *' makes integer from pointer without a cast [-Wint-conversion]
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^
kernel/sched/core.c:6593:3: note: in expansion of macro 'static_call_update'
6593 | static_call_update(preempt_schedule, NULL);
| ^~~~~~~~~~~~~~~~~~
>> include/linux/static_call.h:156:39: warning: passing argument 3 of '__static_call_update' makes pointer from integer without a cast [-Wint-conversion]
156 | STATIC_CALL_TRAMP_ADDR(name), __F); \
| ^~~
| |
| int
kernel/sched/core.c:6593:3: note: in expansion of macro 'static_call_update'
6593 | static_call_update(preempt_schedule, NULL);
| ^~~~~~~~~~~~~~~~~~
include/linux/static_call.h:177:82: note: expected 'void *' but argument is of type 'int'
177 | extern void __static_call_update(struct static_call_key *key, void *tramp, void *func);
| ~~~~~~^~~~
>> include/linux/static_call.h:154:41: warning: initialization of 'int' from 'void *' makes integer from pointer without a cast [-Wint-conversion]
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^
kernel/sched/core.c:6594:3: note: in expansion of macro 'static_call_update'
6594 | static_call_update(preempt_schedule_notrace, NULL);
| ^~~~~~~~~~~~~~~~~~
>> include/linux/static_call.h:156:39: warning: passing argument 3 of '__static_call_update' makes pointer from integer without a cast [-Wint-conversion]
156 | STATIC_CALL_TRAMP_ADDR(name), __F); \
| ^~~
| |
| int
kernel/sched/core.c:6594:3: note: in expansion of macro 'static_call_update'
6594 | static_call_update(preempt_schedule_notrace, NULL);
| ^~~~~~~~~~~~~~~~~~
include/linux/static_call.h:177:82: note: expected 'void *' but argument is of type 'int'
177 | extern void __static_call_update(struct static_call_key *key, void *tramp, void *func);
| ~~~~~~^~~~
>> include/linux/static_call.h:154:41: warning: initialization of 'int' from 'void *' makes integer from pointer without a cast [-Wint-conversion]
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^
kernel/sched/core.c:6601:3: note: in expansion of macro 'static_call_update'
6601 | static_call_update(might_resched, (void *)&__static_call_return0);
| ^~~~~~~~~~~~~~~~~~
>> include/linux/static_call.h:156:39: warning: passing argument 3 of '__static_call_update' makes pointer from integer without a cast [-Wint-conversion]
156 | STATIC_CALL_TRAMP_ADDR(name), __F); \
| ^~~
| |
| int
kernel/sched/core.c:6601:3: note: in expansion of macro 'static_call_update'
6601 | static_call_update(might_resched, (void *)&__static_call_return0);
| ^~~~~~~~~~~~~~~~~~
include/linux/static_call.h:177:82: note: expected 'void *' but argument is of type 'int'
177 | extern void __static_call_update(struct static_call_key *key, void *tramp, void *func);
| ~~~~~~^~~~
>> include/linux/static_call.h:156:39: warning: passing argument 3 of '__static_call_update' makes pointer from integer without a cast [-Wint-conversion]
156 | STATIC_CALL_TRAMP_ADDR(name), __F); \
| ^~~
| |
| int
kernel/sched/core.c:6602:3: note: in expansion of macro 'static_call_update'
6602 | static_call_update(preempt_schedule, __preempt_schedule_func);
| ^~~~~~~~~~~~~~~~~~
include/linux/static_call.h:177:82: note: expected 'void *' but argument is of type 'int'
177 | extern void __static_call_update(struct static_call_key *key, void *tramp, void *func);
| ~~~~~~^~~~
>> include/linux/static_call.h:156:39: warning: passing argument 3 of '__static_call_update' makes pointer from integer without a cast [-Wint-conversion]
156 | STATIC_CALL_TRAMP_ADDR(name), __F); \
| ^~~
| |
| int
kernel/sched/core.c:6603:3: note: in expansion of macro 'static_call_update'
6603 | static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
| ^~~~~~~~~~~~~~~~~~
include/linux/static_call.h:177:82: note: expected 'void *' but argument is of type 'int'
177 | extern void __static_call_update(struct static_call_key *key, void *tramp, void *func);
| ~~~~~~^~~~


vim +154 include/linux/static_call.h

115284d89a436e Josh Poimboeuf 2020-08-18 151
115284d89a436e Josh Poimboeuf 2020-08-18 152 #define static_call_update(name, func) \
115284d89a436e Josh Poimboeuf 2020-08-18 153 ({ \
9432bbd969c667 Peter Zijlstra 2021-03-23 @154 typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
115284d89a436e Josh Poimboeuf 2020-08-18 155 __static_call_update(&STATIC_CALL_KEY(name), \
9432bbd969c667 Peter Zijlstra 2021-03-23 @156 STATIC_CALL_TRAMP_ADDR(name), __F); \
115284d89a436e Josh Poimboeuf 2020-08-18 157 })
115284d89a436e Josh Poimboeuf 2020-08-18 158

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (26.62 kB)
.config.gz (29.24 kB)
Download all attachments

2021-11-06 12:52:15

by kernel test robot

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

Hi Valentin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/master]
[also build test ERROR on linus/master next-20211105]
[cannot apply to linux/master v5.15]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Valentin-Schneider/sched-Tweak-default-dynamic-preempt-mode-selection/20211105-184135
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git ea79c24a30aa27ccc4aac26be33f8b73f3f1f59c
config: x86_64-buildonly-randconfig-r004-20211105 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 847a6807332b13f43704327c2d30103ec0347c77)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/4731291127aa2100c984229a91533b671044a74b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Valentin-Schneider/sched-Tweak-default-dynamic-preempt-mode-selection/20211105-184135
git checkout 4731291127aa2100c984229a91533b671044a74b
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

kernel/sched/core.c:3439:6: error: no previous prototype for function 'sched_set_stop_task' [-Werror,-Wmissing-prototypes]
void sched_set_stop_task(int cpu, struct task_struct *stop)
^
kernel/sched/core.c:3439:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void sched_set_stop_task(int cpu, struct task_struct *stop)
^
static
>> kernel/sched/core.c:6576:2: error: use of undeclared identifier '__SCT__preempt_schedule'
static_call_update(preempt_schedule, __preempt_schedule_func);
^
include/linux/static_call.h:154:10: note: expanded from macro 'static_call_update'
typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
^
include/linux/static_call_types.h:18:34: note: expanded from macro 'STATIC_CALL_TRAMP'
#define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
^
include/linux/compiler_types.h:60:22: note: expanded from macro '__PASTE'
#define __PASTE(a,b) ___PASTE(a,b)
^
include/linux/compiler_types.h:59:23: note: expanded from macro '___PASTE'
#define ___PASTE(a,b) a##b
^
<scratch space>:252:1: note: expanded from here
__SCT__preempt_schedule
^
>> kernel/sched/core.c:6576:39: error: use of undeclared identifier '__preempt_schedule_func'; did you mean 'preempt_schedule_irq'?
static_call_update(preempt_schedule, __preempt_schedule_func);
^~~~~~~~~~~~~~~~~~~~~~~
preempt_schedule_irq
include/linux/static_call.h:154:42: note: expanded from macro 'static_call_update'
typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
^
include/linux/sched.h:291:17: note: 'preempt_schedule_irq' declared here
asmlinkage void preempt_schedule_irq(void);
^
>> kernel/sched/core.c:6576:2: error: use of undeclared identifier '__SCK__preempt_schedule'
static_call_update(preempt_schedule, __preempt_schedule_func);
^
include/linux/static_call.h:155:24: note: expanded from macro 'static_call_update'
__static_call_update(&STATIC_CALL_KEY(name), \
^
include/linux/static_call_types.h:12:32: note: expanded from macro 'STATIC_CALL_KEY'
#define STATIC_CALL_KEY(name) __PASTE(STATIC_CALL_KEY_PREFIX, name)
^
include/linux/compiler_types.h:60:22: note: expanded from macro '__PASTE'
#define __PASTE(a,b) ___PASTE(a,b)
^
include/linux/compiler_types.h:59:23: note: expanded from macro '___PASTE'
#define ___PASTE(a,b) a##b
^
<scratch space>:253:1: note: expanded from here
__SCK__preempt_schedule
^
>> kernel/sched/core.c:6576:2: error: use of undeclared identifier '__SCT__preempt_schedule'
include/linux/static_call.h:156:9: note: expanded from macro 'static_call_update'
STATIC_CALL_TRAMP_ADDR(name), __F); \
^
include/linux/static_call.h:146:39: note: expanded from macro 'STATIC_CALL_TRAMP_ADDR'
#define STATIC_CALL_TRAMP_ADDR(name) &STATIC_CALL_TRAMP(name)
^
include/linux/static_call_types.h:18:34: note: expanded from macro 'STATIC_CALL_TRAMP'
#define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
^
include/linux/compiler_types.h:60:22: note: expanded from macro '__PASTE'
#define __PASTE(a,b) ___PASTE(a,b)
^
include/linux/compiler_types.h:59:23: note: expanded from macro '___PASTE'
#define ___PASTE(a,b) a##b
^
<scratch space>:254:1: note: expanded from here
__SCT__preempt_schedule
^
>> kernel/sched/core.c:6577:2: error: use of undeclared identifier '__SCT__preempt_schedule_notrace'
static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
^
include/linux/static_call.h:154:10: note: expanded from macro 'static_call_update'
typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
^
include/linux/static_call_types.h:18:34: note: expanded from macro 'STATIC_CALL_TRAMP'
#define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
^
include/linux/compiler_types.h:60:22: note: expanded from macro '__PASTE'
#define __PASTE(a,b) ___PASTE(a,b)
^
include/linux/compiler_types.h:59:23: note: expanded from macro '___PASTE'
#define ___PASTE(a,b) a##b
^
<scratch space>:255:1: note: expanded from here
__SCT__preempt_schedule_notrace
^
>> kernel/sched/core.c:6577:47: error: use of undeclared identifier '__preempt_schedule_notrace_func'
static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
^
>> kernel/sched/core.c:6577:2: error: use of undeclared identifier '__SCK__preempt_schedule_notrace'
static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
^
include/linux/static_call.h:155:24: note: expanded from macro 'static_call_update'
__static_call_update(&STATIC_CALL_KEY(name), \
^
include/linux/static_call_types.h:12:32: note: expanded from macro 'STATIC_CALL_KEY'
#define STATIC_CALL_KEY(name) __PASTE(STATIC_CALL_KEY_PREFIX, name)
^
include/linux/compiler_types.h:60:22: note: expanded from macro '__PASTE'
#define __PASTE(a,b) ___PASTE(a,b)
^
include/linux/compiler_types.h:59:23: note: expanded from macro '___PASTE'
#define ___PASTE(a,b) a##b
^
<scratch space>:256:1: note: expanded from here
__SCK__preempt_schedule_notrace
^
>> kernel/sched/core.c:6577:2: error: use of undeclared identifier '__SCT__preempt_schedule_notrace'
include/linux/static_call.h:156:9: note: expanded from macro 'static_call_update'
STATIC_CALL_TRAMP_ADDR(name), __F); \
^
include/linux/static_call.h:146:39: note: expanded from macro 'STATIC_CALL_TRAMP_ADDR'
#define STATIC_CALL_TRAMP_ADDR(name) &STATIC_CALL_TRAMP(name)
^
include/linux/static_call_types.h:18:34: note: expanded from macro 'STATIC_CALL_TRAMP'
#define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
^
include/linux/compiler_types.h:60:22: note: expanded from macro '__PASTE'
#define __PASTE(a,b) ___PASTE(a,b)
^
include/linux/compiler_types.h:59:23: note: expanded from macro '___PASTE'
#define ___PASTE(a,b) a##b
^
<scratch space>:257:1: note: expanded from here
__SCT__preempt_schedule_notrace
^
kernel/sched/core.c:6584:3: error: use of undeclared identifier '__SCT__preempt_schedule'
static_call_update(preempt_schedule, NULL);
^
include/linux/static_call.h:154:10: note: expanded from macro 'static_call_update'
typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
^
include/linux/static_call_types.h:18:34: note: expanded from macro 'STATIC_CALL_TRAMP'
#define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
^
include/linux/compiler_types.h:60:22: note: expanded from macro '__PASTE'
#define __PASTE(a,b) ___PASTE(a,b)
^
include/linux/compiler_types.h:59:23: note: expanded from macro '___PASTE'
#define ___PASTE(a,b) a##b
^
<scratch space>:11:1: note: expanded from here
__SCT__preempt_schedule
^
kernel/sched/core.c:6584:3: error: use of undeclared identifier '__SCK__preempt_schedule'
include/linux/static_call.h:155:24: note: expanded from macro 'static_call_update'
__static_call_update(&STATIC_CALL_KEY(name), \
^
include/linux/static_call_types.h:12:32: note: expanded from macro 'STATIC_CALL_KEY'
#define STATIC_CALL_KEY(name) __PASTE(STATIC_CALL_KEY_PREFIX, name)
^
include/linux/compiler_types.h:60:22: note: expanded from macro '__PASTE'
#define __PASTE(a,b) ___PASTE(a,b)
^
include/linux/compiler_types.h:59:23: note: expanded from macro '___PASTE'
#define ___PASTE(a,b) a##b
^
<scratch space>:12:1: note: expanded from here
__SCK__preempt_schedule
^
kernel/sched/core.c:6584:3: error: use of undeclared identifier '__SCT__preempt_schedule'
include/linux/static_call.h:156:9: note: expanded from macro 'static_call_update'
STATIC_CALL_TRAMP_ADDR(name), __F); \
^
include/linux/static_call.h:146:39: note: expanded from macro 'STATIC_CALL_TRAMP_ADDR'
#define STATIC_CALL_TRAMP_ADDR(name) &STATIC_CALL_TRAMP(name)
^
include/linux/static_call_types.h:18:34: note: expanded from macro 'STATIC_CALL_TRAMP'
#define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
^
include/linux/compiler_types.h:60:22: note: expanded from macro '__PASTE'
#define __PASTE(a,b) ___PASTE(a,b)
^
include/linux/compiler_types.h:59:23: note: expanded from macro '___PASTE'
#define ___PASTE(a,b) a##b
^
<scratch space>:13:1: note: expanded from here
__SCT__preempt_schedule
^
kernel/sched/core.c:6585:3: error: use of undeclared identifier '__SCT__preempt_schedule_notrace'
static_call_update(preempt_schedule_notrace, NULL);
^
include/linux/static_call.h:154:10: note: expanded from macro 'static_call_update'
typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
^
include/linux/static_call_types.h:18:34: note: expanded from macro 'STATIC_CALL_TRAMP'
#define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
^
include/linux/compiler_types.h:60:22: note: expanded from macro '__PASTE'
#define __PASTE(a,b) ___PASTE(a,b)
^
include/linux/compiler_types.h:59:23: note: expanded from macro '___PASTE'
#define ___PASTE(a,b) a##b
^
<scratch space>:14:1: note: expanded from here
__SCT__preempt_schedule_notrace
^
kernel/sched/core.c:6585:3: error: use of undeclared identifier '__SCK__preempt_schedule_notrace'
include/linux/static_call.h:155:24: note: expanded from macro 'static_call_update'
__static_call_update(&STATIC_CALL_KEY(name), \
^
include/linux/static_call_types.h:12:32: note: expanded from macro 'STATIC_CALL_KEY'
#define STATIC_CALL_KEY(name) __PASTE(STATIC_CALL_KEY_PREFIX, name)
^
include/linux/compiler_types.h:60:22: note: expanded from macro '__PASTE'
#define __PASTE(a,b) ___PASTE(a,b)
^
include/linux/compiler_types.h:59:23: note: expanded from macro '___PASTE'


vim +/__SCT__preempt_schedule +6576 kernel/sched/core.c

e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6567
1011dcce99f802 Peter Zijlstra 2021-03-25 6568 void sched_dynamic_update(int mode)
826bfeb37bb430 Peter Zijlstra (Intel 2021-01-18 6569) {
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6570 /*
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6571 * Avoid {NONE,VOLUNTARY} -> FULL transitions from ever ending up in
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6572 * the ZERO state, which is invalid.
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6573 */
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6574 static_call_update(cond_resched, __cond_resched);
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6575 static_call_update(might_resched, __cond_resched);
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 @6576 static_call_update(preempt_schedule, __preempt_schedule_func);
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 @6577 static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6578 static_call_update(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6579
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6580 switch (mode) {
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6581 case preempt_dynamic_none:
826bfeb37bb430 Peter Zijlstra (Intel 2021-01-18 6582) static_call_update(cond_resched, __cond_resched);
9432bbd969c667 Peter Zijlstra 2021-03-23 6583 static_call_update(might_resched, (void *)&__static_call_return0);
9432bbd969c667 Peter Zijlstra 2021-03-23 6584 static_call_update(preempt_schedule, NULL);
9432bbd969c667 Peter Zijlstra 2021-03-23 6585 static_call_update(preempt_schedule_notrace, NULL);
9432bbd969c667 Peter Zijlstra 2021-03-23 6586 static_call_update(irqentry_exit_cond_resched, NULL);
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6587 pr_info("Dynamic Preempt: none\n");
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6588 break;
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6589
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6590 case preempt_dynamic_voluntary:
826bfeb37bb430 Peter Zijlstra (Intel 2021-01-18 6591) static_call_update(cond_resched, __cond_resched);
826bfeb37bb430 Peter Zijlstra (Intel 2021-01-18 6592) static_call_update(might_resched, __cond_resched);
9432bbd969c667 Peter Zijlstra 2021-03-23 6593 static_call_update(preempt_schedule, NULL);
9432bbd969c667 Peter Zijlstra 2021-03-23 6594 static_call_update(preempt_schedule_notrace, NULL);
9432bbd969c667 Peter Zijlstra 2021-03-23 6595 static_call_update(irqentry_exit_cond_resched, NULL);
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6596 pr_info("Dynamic Preempt: voluntary\n");
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6597 break;
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6598
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6599 case preempt_dynamic_full:
9432bbd969c667 Peter Zijlstra 2021-03-23 6600 static_call_update(cond_resched, (void *)&__static_call_return0);
9432bbd969c667 Peter Zijlstra 2021-03-23 6601 static_call_update(might_resched, (void *)&__static_call_return0);
826bfeb37bb430 Peter Zijlstra (Intel 2021-01-18 6602) static_call_update(preempt_schedule, __preempt_schedule_func);
826bfeb37bb430 Peter Zijlstra (Intel 2021-01-18 6603) static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
826bfeb37bb430 Peter Zijlstra (Intel 2021-01-18 6604) static_call_update(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6605 pr_info("Dynamic Preempt: full\n");
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6606 break;
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6607 }
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6608
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6609 preempt_dynamic_mode = mode;
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6610 }
e59e10f8ef63d4 Peter Zijlstra 2021-01-22 6611

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (17.82 kB)
.config.gz (31.75 kB)
Download all attachments

2021-11-06 14:59:56

by kernel test robot

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

Hi Valentin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/master]
[also build test ERROR on linus/master next-20211106]
[cannot apply to linux/master v5.15]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Valentin-Schneider/sched-Tweak-default-dynamic-preempt-mode-selection/20211105-184135
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git ea79c24a30aa27ccc4aac26be33f8b73f3f1f59c
config: x86_64-defconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/4731291127aa2100c984229a91533b671044a74b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Valentin-Schneider/sched-Tweak-default-dynamic-preempt-mode-selection/20211105-184135
git checkout 4731291127aa2100c984229a91533b671044a74b
# save the attached .config to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

kernel/sched/core.c:3439:6: warning: no previous prototype for 'sched_set_stop_task' [-Wmissing-prototypes]
3439 | void sched_set_stop_task(int cpu, struct task_struct *stop)
| ^~~~~~~~~~~~~~~~~~~
In file included from <command-line>:
kernel/sched/core.c: In function 'sched_dynamic_update':
>> include/linux/static_call_types.h:15:34: error: '__SCT__might_resched' undeclared (first use in this function); did you mean '__SCT__cond_resched'?
15 | #define STATIC_CALL_TRAMP_PREFIX __SCT__
| ^~~~~~~
include/linux/compiler_types.h:59:23: note: in definition of macro '___PASTE'
59 | #define ___PASTE(a,b) a##b
| ^
include/linux/static_call_types.h:18:34: note: in expansion of macro '__PASTE'
18 | #define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
| ^~~~~~~
include/linux/static_call_types.h:18:42: note: in expansion of macro 'STATIC_CALL_TRAMP_PREFIX'
18 | #define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/linux/static_call.h:154:10: note: in expansion of macro 'STATIC_CALL_TRAMP'
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^~~~~~~~~~~~~~~~~
kernel/sched/core.c:6575:2: note: in expansion of macro 'static_call_update'
6575 | static_call_update(might_resched, __cond_resched);
| ^~~~~~~~~~~~~~~~~~
include/linux/static_call_types.h:15:34: note: each undeclared identifier is reported only once for each function it appears in
15 | #define STATIC_CALL_TRAMP_PREFIX __SCT__
| ^~~~~~~
include/linux/compiler_types.h:59:23: note: in definition of macro '___PASTE'
59 | #define ___PASTE(a,b) a##b
| ^
include/linux/static_call_types.h:18:34: note: in expansion of macro '__PASTE'
18 | #define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
| ^~~~~~~
include/linux/static_call_types.h:18:42: note: in expansion of macro 'STATIC_CALL_TRAMP_PREFIX'
18 | #define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/linux/static_call.h:154:10: note: in expansion of macro 'STATIC_CALL_TRAMP'
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^~~~~~~~~~~~~~~~~
kernel/sched/core.c:6575:2: note: in expansion of macro 'static_call_update'
6575 | static_call_update(might_resched, __cond_resched);
| ^~~~~~~~~~~~~~~~~~
In file included from include/linux/tracepoint.h:22,
from include/trace/events/sched.h:10,
from kernel/sched/core.c:10:
include/linux/static_call.h:154:41: warning: initialization of 'int' from 'int (*)(void)' makes integer from pointer without a cast [-Wint-conversion]
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^
kernel/sched/core.c:6575:2: note: in expansion of macro 'static_call_update'
6575 | static_call_update(might_resched, __cond_resched);
| ^~~~~~~~~~~~~~~~~~
In file included from <command-line>:
>> include/linux/static_call_types.h:9:33: error: '__SCK__might_resched' undeclared (first use in this function); did you mean '__SCK__cond_resched'?
9 | #define STATIC_CALL_KEY_PREFIX __SCK__
| ^~~~~~~
include/linux/compiler_types.h:59:23: note: in definition of macro '___PASTE'
59 | #define ___PASTE(a,b) a##b
| ^
include/linux/static_call_types.h:12:32: note: in expansion of macro '__PASTE'
12 | #define STATIC_CALL_KEY(name) __PASTE(STATIC_CALL_KEY_PREFIX, name)
| ^~~~~~~
include/linux/static_call_types.h:12:40: note: in expansion of macro 'STATIC_CALL_KEY_PREFIX'
12 | #define STATIC_CALL_KEY(name) __PASTE(STATIC_CALL_KEY_PREFIX, name)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/static_call.h:155:24: note: in expansion of macro 'STATIC_CALL_KEY'
155 | __static_call_update(&STATIC_CALL_KEY(name), \
| ^~~~~~~~~~~~~~~
kernel/sched/core.c:6575:2: note: in expansion of macro 'static_call_update'
6575 | static_call_update(might_resched, __cond_resched);
| ^~~~~~~~~~~~~~~~~~
In file included from include/linux/tracepoint.h:22,
from include/trace/events/sched.h:10,
from kernel/sched/core.c:10:
include/linux/static_call.h:156:39: warning: passing argument 3 of '__static_call_update' makes pointer from integer without a cast [-Wint-conversion]
156 | STATIC_CALL_TRAMP_ADDR(name), __F); \
| ^~~
| |
| int
kernel/sched/core.c:6575:2: note: in expansion of macro 'static_call_update'
6575 | static_call_update(might_resched, __cond_resched);
| ^~~~~~~~~~~~~~~~~~
include/linux/static_call.h:177:82: note: expected 'void *' but argument is of type 'int'
177 | extern void __static_call_update(struct static_call_key *key, void *tramp, void *func);
| ~~~~~~^~~~
In file included from <command-line>:
include/linux/static_call_types.h:15:34: error: '__SCT__preempt_schedule' undeclared (first use in this function)
15 | #define STATIC_CALL_TRAMP_PREFIX __SCT__
| ^~~~~~~
include/linux/compiler_types.h:59:23: note: in definition of macro '___PASTE'
59 | #define ___PASTE(a,b) a##b
| ^
include/linux/static_call_types.h:18:34: note: in expansion of macro '__PASTE'
18 | #define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
| ^~~~~~~
include/linux/static_call_types.h:18:42: note: in expansion of macro 'STATIC_CALL_TRAMP_PREFIX'
18 | #define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/linux/static_call.h:154:10: note: in expansion of macro 'STATIC_CALL_TRAMP'
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^~~~~~~~~~~~~~~~~
kernel/sched/core.c:6576:2: note: in expansion of macro 'static_call_update'
6576 | static_call_update(preempt_schedule, __preempt_schedule_func);
| ^~~~~~~~~~~~~~~~~~
In file included from include/linux/tracepoint.h:22,
from include/trace/events/sched.h:10,
from kernel/sched/core.c:10:
kernel/sched/core.c:6576:39: error: '__preempt_schedule_func' undeclared (first use in this function); did you mean 'preempt_schedule_irq'?
6576 | static_call_update(preempt_schedule, __preempt_schedule_func);
| ^~~~~~~~~~~~~~~~~~~~~~~
include/linux/static_call.h:154:42: note: in definition of macro 'static_call_update'
154 | typeof(&STATIC_CALL_TRAMP(name)) __F = (func); \
| ^~~~
In file included from <command-line>:
include/linux/static_call_types.h:9:33: error: '__SCK__preempt_schedule' undeclared (first use in this function)
9 | #define STATIC_CALL_KEY_PREFIX __SCK__
| ^~~~~~~
include/linux/compiler_types.h:59:23: note: in definition of macro '___PASTE'
59 | #define ___PASTE(a,b) a##b
| ^
include/linux/static_call_types.h:12:32: note: in expansion of macro '__PASTE'
12 | #define STATIC_CALL_KEY(name) __PASTE(STATIC_CALL_KEY_PREFIX, name)
| ^~~~~~~
include/linux/static_call_types.h:12:40: note: in expansion of macro 'STATIC_CALL_KEY_PREFIX'
12 | #define STATIC_CALL_KEY(name) __PASTE(STATIC_CALL_KEY_PREFIX, name)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/static_call.h:155:24: note: in expansion of macro 'STATIC_CALL_KEY'
155 | __static_call_update(&STATIC_CALL_KEY(name), \
| ^~~~~~~~~~~~~~~
kernel/sched/core.c:6576:2: note: in expansion of macro 'static_call_update'
6576 | static_call_update(preempt_schedule, __preempt_schedule_func);
| ^~~~~~~~~~~~~~~~~~
In file included from include/linux/tracepoint.h:22,
from include/trace/events/sched.h:10,
from kernel/sched/core.c:10:
include/linux/static_call.h:156:39: warning: passing argument 3 of '__static_call_update' makes pointer from integer without a cast [-Wint-conversion]
156 | STATIC_CALL_TRAMP_ADDR(name), __F); \
| ^~~
| |
| int
kernel/sched/core.c:6576:2: note: in expansion of macro 'static_call_update'
6576 | static_call_update(preempt_schedule, __preempt_schedule_func);
| ^~~~~~~~~~~~~~~~~~
include/linux/static_call.h:177:82: note: expected 'void *' but argument is of type 'int'
177 | extern void __static_call_update(struct static_call_key *key, void *tramp, void *func);
| ~~~~~~^~~~
In file included from <command-line>:
include/linux/static_call_types.h:15:34: error: '__SCT__preempt_schedule_notrace' undeclared (first use in this function)
15 | #define STATIC_CALL_TRAMP_PREFIX __SCT__
| ^~~~~~~
include/linux/compiler_types.h:59:23: note: in definition of macro '___PASTE'
59 | #define ___PASTE(a,b) a##b
| ^
include/linux/static_call_types.h:18:34: note: in expansion of macro '__PASTE'


vim +15 include/linux/static_call_types.h

115284d89a436e Josh Poimboeuf 2020-08-18 8
115284d89a436e Josh Poimboeuf 2020-08-18 @9 #define STATIC_CALL_KEY_PREFIX __SCK__
9183c3f9ed710a Josh Poimboeuf 2020-08-18 10 #define STATIC_CALL_KEY_PREFIX_STR __stringify(STATIC_CALL_KEY_PREFIX)
9183c3f9ed710a Josh Poimboeuf 2020-08-18 11 #define STATIC_CALL_KEY_PREFIX_LEN (sizeof(STATIC_CALL_KEY_PREFIX_STR) - 1)
115284d89a436e Josh Poimboeuf 2020-08-18 12 #define STATIC_CALL_KEY(name) __PASTE(STATIC_CALL_KEY_PREFIX, name)
73f44fe19d3596 Josh Poimboeuf 2021-01-27 13 #define STATIC_CALL_KEY_STR(name) __stringify(STATIC_CALL_KEY(name))
115284d89a436e Josh Poimboeuf 2020-08-18 14
115284d89a436e Josh Poimboeuf 2020-08-18 @15 #define STATIC_CALL_TRAMP_PREFIX __SCT__
115284d89a436e Josh Poimboeuf 2020-08-18 16 #define STATIC_CALL_TRAMP_PREFIX_STR __stringify(STATIC_CALL_TRAMP_PREFIX)
9183c3f9ed710a Josh Poimboeuf 2020-08-18 17 #define STATIC_CALL_TRAMP_PREFIX_LEN (sizeof(STATIC_CALL_TRAMP_PREFIX_STR) - 1)
115284d89a436e Josh Poimboeuf 2020-08-18 18 #define STATIC_CALL_TRAMP(name) __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
115284d89a436e Josh Poimboeuf 2020-08-18 19 #define STATIC_CALL_TRAMP_STR(name) __stringify(STATIC_CALL_TRAMP(name))
115284d89a436e Josh Poimboeuf 2020-08-18 20

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (13.07 kB)
.config.gz (29.24 kB)
Download all attachments

2021-11-08 14:20:15

by Valentin Schneider

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

On 06/11/21 05:40, Mike Galbraith wrote:
> On Fri, 2021-11-05 at 10:40 +0000, Valentin Schneider wrote:
>> Commit c597bfddc9e9 ("sched: Provide Kconfig support for default dynamic
>> preempt mode") changed the selectable config names for the preemption
>> model. This means a config file must now select
>>
>>   CONFIG_PREEMPT_BEHAVIOUR=y
>>
>> rather than
>>
>>   CONFIG_PREEMPT=y
>>
>> to get a preemptible kernel. This means all arch config files need to be
>> updated - right now arm64 defconfig selects CONFIG_PREEMPT=y but ends up
>> with CONFIG_PREEMPT_NONE_BEHAVIOUR=y.
>>
>> Instead, have CONFIG_*PREEMPT be the selectable configs again, and make
>> them select their _BEHAVIOUR equivalent if CONFIG_PREEMPT_DYNAMIC is set.
>
>
> Is there any way to get to PREEMPT_RT in the first selection again as
> well? I had created a behavior entry for RT (below) and inverted the
> dependency to make it appear in the initial selection again, but that's
> clearly not gonna fly.
>
> Starting with a 5.15 config, to select RT you currently must first
> select a model you don't want, then reject PREEMPT_DYNAMIC and you'll
> be offered the full menu of models immediately. With your patch added,
> that became worse. After rejecting PREEMPT_DYNAMIC, I had to go
> through new 5.15+ options before finally being offered the full menu.
>

Do you mean at the syncconfig step? I've only really played with upstream
arm64 / x86 defconfigs and didn't have to fight with any prompts, though
yes for x86 the default-y PREEMPT_DYNAMIC makes it a bit annoying to select
PREEMPT_RT.

2021-11-08 17:44:01

by Valentin Schneider

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

On 08/11/21 13:27, Mike Galbraith wrote:
> On Mon, 2021-11-08 at 11:17 +0000, Valentin Schneider wrote:
>> On 06/11/21 05:40, Mike Galbraith wrote:
>> >
>> > Starting with a 5.15 config, to select RT you currently must first
>> > select a model you don't want, then reject PREEMPT_DYNAMIC and you'll
>> > be offered the full menu of models immediately. With your patch added,
>> > that became worse.  After rejecting PREEMPT_DYNAMIC, I had to go
>> > through new 5.15+ options before finally being offered the full menu.
>> >
>>
>> Do you mean at the syncconfig step?
>
> Um, not sure what that is, but it sounds about right.
>
>> I've only really played with upstream
>> arm64 / x86 defconfigs and didn't have to fight with any prompts, though
>> yes for x86 the default-y PREEMPT_DYNAMIC makes it a bit annoying to select
>> PREEMPT_RT.
>
> As long as RT depends on EXPERT it'll be a bit annoying regardless. I
> just thought it worth mention that what you want now and what RT will
> presumably want upon merge completion appear to be mutually exclusive.
>

Hmm actually I think your approach should work, i.e. have

config PREEMPT_DYNAMIC
depends on [...] && !PREEMPT_RT

rather than

config PREEMPT_RT
depends on [...] && !PREEMPT_DYNAMIC

This essentially gives priority to the preemption model type over the
preemption model dynamicness, which I think makes sense. I can fold that in
v2.

> -Mike

2021-11-08 17:57:13

by Valentin Schneider

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

On 06/11/21 13:05, kernel test robot wrote:
> kernel/sched/core.c:3439:6: error: no previous prototype for function 'sched_set_stop_task' [-Werror,-Wmissing-prototypes]
> void sched_set_stop_task(int cpu, struct task_struct *stop)
> ^
> kernel/sched/core.c:3439:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> void sched_set_stop_task(int cpu, struct task_struct *stop)
> ^
> static
>>> kernel/sched/core.c:6576:2: error: use of undeclared identifier '__SCT__preempt_schedule'
> static_call_update(preempt_schedule, __preempt_schedule_func);
> ^

Doh, I broke the

PREEMPT_DYNAMIC
select PREEMPT

thing - the above happens with PREEMPT_DYNAMIC && !PREEMPT. Lemme fix.

2021-11-08 18:12:48

by Mike Galbraith

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

On Mon, 2021-11-08 at 11:17 +0000, Valentin Schneider wrote:
> On 06/11/21 05:40, Mike Galbraith wrote:
> >
> > Starting with a 5.15 config, to select RT you currently must first
> > select a model you don't want, then reject PREEMPT_DYNAMIC and you'll
> > be offered the full menu of models immediately. With your patch added,
> > that became worse.  After rejecting PREEMPT_DYNAMIC, I had to go
> > through new 5.15+ options before finally being offered the full menu.
> >
>
> Do you mean at the syncconfig step?

Um, not sure what that is, but it sounds about right.

> I've only really played with upstream
> arm64 / x86 defconfigs and didn't have to fight with any prompts, though
> yes for x86 the default-y PREEMPT_DYNAMIC makes it a bit annoying to select
> PREEMPT_RT.

As long as RT depends on EXPERT it'll be a bit annoying regardless. I
just thought it worth mention that what you want now and what RT will
presumably want upon merge completion appear to be mutually exclusive.

-Mike

2021-11-09 14:09:35

by Mike Galbraith

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

On Mon, 2021-11-08 at 15:21 +0000, Valentin Schneider wrote:
> On 08/11/21 13:27, Mike Galbraith wrote:
> >
> > As long as RT depends on EXPERT it'll be a bit annoying regardless.  I
> > just thought it worth mention that what you want now and what RT will
> > presumably want upon merge completion appear to be mutually exclusive.
> >
>
> Hmm actually I think your approach should work, i.e. have
>
>   config PREEMPT_DYNAMIC
>     depends on [...] && !PREEMPT_RT
>
> rather than
>
>   config PREEMPT_RT
>     depends on [...] && !PREEMPT_DYNAMIC
>
> This essentially gives priority to the preemption model type over the
> preemption model dynamicness, which I think makes sense. I can fold that in
> v2.

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.

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.

Signed-off-by: Mike Galbraith <[email protected]>
---
kernel/Kconfig.preempt | 46 +++++++++++++++++++++++++---------------
------
1 file changed, 25 insertions(+), 21 deletions(-)

--- a/kernel/Kconfig.preempt
+++ b/kernel/Kconfig.preempt
@@ -2,11 +2,10 @@

choice
prompt "Preemption Model"
- default PREEMPT_NONE_BEHAVIOUR
+ default PREEMPT_NONE

-config PREEMPT_NONE_BEHAVIOUR
+config PREEMPT_NONE
bool "No Forced Preemption (Server)"
- select PREEMPT_NONE if !PREEMPT_DYNAMIC
help
This is the traditional Linux preemption model, geared
towards
throughput. It will still provide good latencies most of the
@@ -18,10 +17,9 @@ config PREEMPT_NONE_BEHAVIOUR
raw processing power of the kernel, irrespective of
scheduling
latencies.

-config PREEMPT_VOLUNTARY_BEHAVIOUR
+config PREEMPT_VOLUNTARY
bool "Voluntary Kernel Preemption (Desktop)"
depends on !ARCH_NO_PREEMPT
- select PREEMPT_VOLUNTARY if !PREEMPT_DYNAMIC
help
This option reduces the latency of the kernel by adding more
"explicit preemption points" to the kernel code. These new
@@ -37,10 +35,11 @@ config PREEMPT_VOLUNTARY_BEHAVIOUR

Select this if you are building a kernel for a desktop
system.

-config PREEMPT_BEHAVIOUR
+config PREEMPT
bool "Preemptible Kernel (Low-Latency Desktop)"
depends on !ARCH_NO_PREEMPT
- select PREEMPT
+ select PREEMPTION
+ select UNINLINE_SPIN_UNLOCK if !ARCH_INLINE_SPIN_UNLOCK
help
This option reduces the latency of the kernel by making
all kernel code (that is not executing in a critical
section)
@@ -58,7 +57,7 @@ config PREEMPT_BEHAVIOUR

config PREEMPT_RT
bool "Fully Preemptible Kernel (Real-Time)"
- depends on EXPERT && ARCH_SUPPORTS_RT && !PREEMPT_DYNAMIC
+ depends on EXPERT && ARCH_SUPPORTS_RT
select PREEMPTION
help
This option turns the kernel into a real-time kernel by
replacing
@@ -75,17 +74,6 @@ config PREEMPT_RT

endchoice

-config PREEMPT_NONE
- bool
-
-config PREEMPT_VOLUNTARY
- bool
-
-config PREEMPT
- bool
- select PREEMPTION
- select UNINLINE_SPIN_UNLOCK if !ARCH_INLINE_SPIN_UNLOCK
-
config PREEMPT_COUNT
bool

@@ -95,8 +83,7 @@ config PREEMPTION

config PREEMPT_DYNAMIC
bool "Preemption behaviour defined on boot"
- depends on HAVE_PREEMPT_DYNAMIC
- select PREEMPT
+ depends on HAVE_PREEMPT_DYNAMIC && PREEMPT
default y
help
This option allows to define the preemption model on the
kernel
@@ -114,6 +101,23 @@ config PREEMPT_DYNAMIC
Interesting if you want the same pre-built kernel should be
used for
both Server and Desktop workloads.

+if PREEMPT_DYNAMIC
+choice
+ prompt "Boot Time Preemption Model"
+ default PREEMPT_NONE_BEHAVIOR
+
+config PREEMPT_NONE_BEHAVIOR
+ bool "No Forced Preemption (Server)"
+
+config PREEMPT_VOLUNTARY_BEHAVIOR
+ bool "Voluntary Kernel Preemption (Desktop)"
+
+config PREEMPT_BEHAVIOR
+ bool "Preemptible Kernel (Low-Latency Desktop)"
+
+endchoice
+endif # PREEMPT_DYNAMIC
+
config SCHED_CORE
bool "Core Scheduling for SMT"
depends on SCHED_SMT

2021-11-09 20:06:17

by Valentin Schneider

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

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.

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

Reviewed-by: Valentin Schneider <[email protected]>

> Signed-off-by: Mike Galbraith <[email protected]>
> ---
> kernel/Kconfig.preempt | 46 +++++++++++++++++++++++++---------------
> ------
> 1 file changed, 25 insertions(+), 21 deletions(-)
>
> --- a/kernel/Kconfig.preempt
> +++ b/kernel/Kconfig.preempt
> @@ -2,11 +2,10 @@
>
> choice
> prompt "Preemption Model"
> - default PREEMPT_NONE_BEHAVIOUR
> + default PREEMPT_NONE
>
> -config PREEMPT_NONE_BEHAVIOUR
> +config PREEMPT_NONE
> bool "No Forced Preemption (Server)"
> - select PREEMPT_NONE if !PREEMPT_DYNAMIC
> help
> This is the traditional Linux preemption model, geared
> towards
> throughput. It will still provide good latencies most of the
> @@ -18,10 +17,9 @@ config PREEMPT_NONE_BEHAVIOUR
> raw processing power of the kernel, irrespective of
> scheduling
> latencies.
>
> -config PREEMPT_VOLUNTARY_BEHAVIOUR
> +config PREEMPT_VOLUNTARY
> bool "Voluntary Kernel Preemption (Desktop)"
> depends on !ARCH_NO_PREEMPT
> - select PREEMPT_VOLUNTARY if !PREEMPT_DYNAMIC
> help
> This option reduces the latency of the kernel by adding more
> "explicit preemption points" to the kernel code. These new
> @@ -37,10 +35,11 @@ config PREEMPT_VOLUNTARY_BEHAVIOUR
>
> Select this if you are building a kernel for a desktop
> system.
>
> -config PREEMPT_BEHAVIOUR
> +config PREEMPT
> bool "Preemptible Kernel (Low-Latency Desktop)"
> depends on !ARCH_NO_PREEMPT
> - select PREEMPT
> + select PREEMPTION
> + select UNINLINE_SPIN_UNLOCK if !ARCH_INLINE_SPIN_UNLOCK
> help
> This option reduces the latency of the kernel by making
> all kernel code (that is not executing in a critical
> section)
> @@ -58,7 +57,7 @@ config PREEMPT_BEHAVIOUR
>
> config PREEMPT_RT
> bool "Fully Preemptible Kernel (Real-Time)"
> - depends on EXPERT && ARCH_SUPPORTS_RT && !PREEMPT_DYNAMIC
> + depends on EXPERT && ARCH_SUPPORTS_RT
> select PREEMPTION
> help
> This option turns the kernel into a real-time kernel by
> replacing

This we can't get away from IMO - it's pretty much the same issue as

b8d3349803ba ("sched/rt, Kconfig: Unbreak def/oldconfig with CONFIG_PREEMPT=y")

IOW we can't really touch those preempt configs, but rather add stuff
around them.

> @@ -75,17 +74,6 @@ config PREEMPT_RT
>
> endchoice
>
> -config PREEMPT_NONE
> - bool
> -
> -config PREEMPT_VOLUNTARY
> - bool
> -
> -config PREEMPT
> - bool
> - select PREEMPTION
> - select UNINLINE_SPIN_UNLOCK if !ARCH_INLINE_SPIN_UNLOCK
> -
> config PREEMPT_COUNT
> bool
>
> @@ -95,8 +83,7 @@ config PREEMPTION
>
> config PREEMPT_DYNAMIC
> bool "Preemption behaviour defined on boot"
> - depends on HAVE_PREEMPT_DYNAMIC
> - select PREEMPT
> + depends on HAVE_PREEMPT_DYNAMIC && PREEMPT

I got there too. If we look at x86 defconfig, it selects
PREEMPT_VOLUNTARY. I initially tried making it so PREEMPT_VOLUNTARY means
both:

- VOLUNTARY model if !PREEMPT_DYNAMIC
- VOLUNTARY model as default boot-time mode if PREEMPT_DYNAMIC
(& similar for other preemption modes)

which, now that I've played with it, is a sad wreck. Having PREEMPT_DYNAMIC
depend on PREEMPT at least makes things clear, and defconfig choices remain
respected (they just won't get PREEMPT_DYNAMIC if they didn't pick
PREEMPT).

> default y
> help
> This option allows to define the preemption model on the
> kernel
> @@ -114,6 +101,23 @@ config PREEMPT_DYNAMIC
> Interesting if you want the same pre-built kernel should be
> used for
> both Server and Desktop workloads.
>
> +if PREEMPT_DYNAMIC

AFAICT that should be

depends on PREEMPT_DYNAMIC

within the choice itself. See e.g. "Kernel compression mode" choice in
init/Kconfig.

> +choice
> + prompt "Boot Time Preemption Model"

Default boot-time [...]? This is only used if the cmdline parameter is
missing. Also a help-text wouldn't hurt. I had:

This option defines the default preemption model of the kernel
if it hasn't been specified by the command line parameter.

> + default PREEMPT_NONE_BEHAVIOR
> +
> +config PREEMPT_NONE_BEHAVIOR
> + bool "No Forced Preemption (Server)"
> +
> +config PREEMPT_VOLUNTARY_BEHAVIOR
> + bool "Voluntary Kernel Preemption (Desktop)"
> +
> +config PREEMPT_BEHAVIOR
> + bool "Preemptible Kernel (Low-Latency Desktop)"
> +
> +endchoice
> +endif # PREEMPT_DYNAMIC
> +
> config SCHED_CORE
> bool "Core Scheduling for SMT"
> depends on SCHED_SMT

2021-11-09 20:22:18

by Frederic Weisbecker

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

On Fri, Nov 05, 2021 at 10:40:35AM +0000, Valentin Schneider wrote:
> Commit c597bfddc9e9 ("sched: Provide Kconfig support for default dynamic
> preempt mode") changed the selectable config names for the preemption
> model. This means a config file must now select
>
> CONFIG_PREEMPT_BEHAVIOUR=y
>
> rather than
>
> CONFIG_PREEMPT=y
>
> to get a preemptible kernel. This means all arch config files need to be
> updated - right now arm64 defconfig selects CONFIG_PREEMPT=y but ends up
> with CONFIG_PREEMPT_NONE_BEHAVIOUR=y.
>
> Instead, have CONFIG_*PREEMPT be the selectable configs again, and make
> them select their _BEHAVIOUR equivalent if CONFIG_PREEMPT_DYNAMIC is set.
>
> Signed-off-by: Valentin Schneider <[email protected]>
> ---
> kernel/Kconfig.preempt | 36 ++++++++++++++++++------------------
> 1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/kernel/Kconfig.preempt b/kernel/Kconfig.preempt
> index 60f1bfc3c7b2..25e8d6a3d9fa 100644
> --- a/kernel/Kconfig.preempt
> +++ b/kernel/Kconfig.preempt
> @@ -1,12 +1,21 @@
> # SPDX-License-Identifier: GPL-2.0-only
>
> +config PREEMPT_NONE_BEHAVIOUR
> + bool
> +
> +config PREEMPT_VOLUNTARY_BEHAVIOUR
> + bool
> +
> +config PREEMPT_BEHAVIOUR
> + bool
> +
> choice
> prompt "Preemption Model"
> - default PREEMPT_NONE_BEHAVIOUR
> + default PREEMPT_NONE
>
> -config PREEMPT_NONE_BEHAVIOUR
> +config PREEMPT_NONE
> bool "No Forced Preemption (Server)"
> - select PREEMPT_NONE if !PREEMPT_DYNAMIC
> + select PREEMPT_NONE_BEHAVIOUR if PREEMPT_DYNAMIC
> help
> This is the traditional Linux preemption model, geared towards
> throughput. It will still provide good latencies most of the
> @@ -18,10 +27,10 @@ config PREEMPT_NONE_BEHAVIOUR
> raw processing power of the kernel, irrespective of scheduling
> latencies.
>
> -config PREEMPT_VOLUNTARY_BEHAVIOUR
> +config PREEMPT_VOLUNTARY
> bool "Voluntary Kernel Preemption (Desktop)"
> depends on !ARCH_NO_PREEMPT
> - select PREEMPT_VOLUNTARY if !PREEMPT_DYNAMIC
> + select PREEMPT_VOLUNTARY_BEHAVIOUR if PREEMPT_DYNAMIC
> help
> This option reduces the latency of the kernel by adding more
> "explicit preemption points" to the kernel code. These new
> @@ -37,10 +46,12 @@ config PREEMPT_VOLUNTARY_BEHAVIOUR
>
> Select this if you are building a kernel for a desktop system.
>
> -config PREEMPT_BEHAVIOUR
> +config PREEMPT
> bool "Preemptible Kernel (Low-Latency Desktop)"
> depends on !ARCH_NO_PREEMPT
> - select PREEMPT
> + select PREEMPTION
> + select UNINLINE_SPIN_UNLOCK if !ARCH_INLINE_SPIN_UNLOCK
> + select PREEMPT_BEHAVIOUR if PREEMPT_DYNAMIC
> help
> This option reduces the latency of the kernel by making
> all kernel code (that is not executing in a critical section)
> @@ -75,17 +86,6 @@ config PREEMPT_RT
>
> endchoice
>
> -config PREEMPT_NONE
> - bool
> -
> -config PREEMPT_VOLUNTARY
> - bool
> -
> -config PREEMPT
> - bool
> - select PREEMPTION
> - select UNINLINE_SPIN_UNLOCK if !ARCH_INLINE_SPIN_UNLOCK
> -
> config PREEMPT_COUNT
> bool

This must be breaking cond_resched() and might_resched() definitions.

Since CONFIG_PREEMPT_NONE, CONFIG_PREEMPT_VOLUNTARY and CONFIG_PREEMPT aren't too widely
spread around within ifdefferies, you can:

1) Rename CONFIG_PREEMPT_NONE to CONFIG_PREEMPT_NONE_STATIC
Rename CONFIG_PREEMPT_VOLUNTARY to CONFIG_PREEMPT_VOLUNTARY_STATIC
Rename CONFIG_PREEMPT to CONFIG_PREEMPT_STATIC

2) Keep the old CONFIG_PREEMPT_NONE, CONFIG_PREEMPT_VOLUNTARY,
CONFIG_PREEMPT around for compatibility and make them select their
corresponding BEHAVIOUR entries.

Thanks.

2021-11-09 21:24:48

by Mike Galbraith

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

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.

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

I'm looking at two straight up choice sets, one for those who have and
want PREEMPT_DYNAMIC, and another for the rest of us. Unfortunately,
there's no "else", so I end up with this mess, which would likely be
better served by "source foo/bar".

---
kernel/Kconfig.preempt | 90 +++++++++++++++++++++++++++++--------------------
1 file changed, 54 insertions(+), 36 deletions(-)

--- a/kernel/Kconfig.preempt
+++ b/kernel/Kconfig.preempt
@@ -2,11 +2,41 @@

choice
prompt "Preemption Model"
- default PREEMPT_NONE_BEHAVIOUR
+ default PREEMPT_STATIC

-config PREEMPT_NONE_BEHAVIOUR
+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
+ model defined during compile time.
+
+ The feature is primarily interesting for Linux distributions which
+ provide a pre-built kernel binary to reduce the number of kernel
+ flavors they offer while still offering different usecases.
+
+ The runtime overhead is negligible with HAVE_STATIC_CALL_INLINE enabled
+ but if runtime patching is not available for the specific architecture
+ then the potential overhead should be considered.
+
+ 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"
+ default PREEMPT_NONE
+
+config PREEMPT_NONE
bool "No Forced Preemption (Server)"
- select PREEMPT_NONE if !PREEMPT_DYNAMIC
help
This is the traditional Linux preemption model, geared towards
throughput. It will still provide good latencies most of the
@@ -18,10 +48,9 @@ config PREEMPT_NONE_BEHAVIOUR
raw processing power of the kernel, irrespective of scheduling
latencies.

-config PREEMPT_VOLUNTARY_BEHAVIOUR
+config PREEMPT_VOLUNTARY
bool "Voluntary Kernel Preemption (Desktop)"
depends on !ARCH_NO_PREEMPT
- select PREEMPT_VOLUNTARY if !PREEMPT_DYNAMIC
help
This option reduces the latency of the kernel by adding more
"explicit preemption points" to the kernel code. These new
@@ -37,10 +66,11 @@ config PREEMPT_VOLUNTARY_BEHAVIOUR

Select this if you are building a kernel for a desktop system.

-config PREEMPT_BEHAVIOUR
+config PREEMPT
bool "Preemptible Kernel (Low-Latency Desktop)"
depends on !ARCH_NO_PREEMPT
- select PREEMPT
+ select PREEMPTION
+ select UNINLINE_SPIN_UNLOCK if !ARCH_INLINE_SPIN_UNLOCK
help
This option reduces the latency of the kernel by making
all kernel code (that is not executing in a critical section)
@@ -58,7 +88,7 @@ config PREEMPT_BEHAVIOUR

config PREEMPT_RT
bool "Fully Preemptible Kernel (Real-Time)"
- depends on EXPERT && ARCH_SUPPORTS_RT && !PREEMPT_DYNAMIC
+ depends on EXPERT && ARCH_SUPPORTS_RT
select PREEMPTION
help
This option turns the kernel into a real-time kernel by replacing
@@ -74,17 +104,26 @@ config PREEMPT_RT
require real-time guarantees.

endchoice
+endif # PREEMPT_STATIC

-config PREEMPT_NONE
+if PREEMPT_DYNAMIC
+config PREEMPT
bool

-config PREEMPT_VOLUNTARY
- bool
+choice
+ prompt "Boot Time Preemption Flavor"
+ default PREEMPT_NONE_BEHAVIOR

-config PREEMPT
- bool
- select PREEMPTION
- select UNINLINE_SPIN_UNLOCK if !ARCH_INLINE_SPIN_UNLOCK
+config PREEMPT_NONE_BEHAVIOR
+ bool "No Forced Preemption (Server)"
+
+config PREEMPT_VOLUNTARY_BEHAVIOR
+ bool "Voluntary Kernel Preemption (Desktop)"
+
+config PREEMPT_BEHAVIOR
+ bool "Preemptible Kernel (Low-Latency Desktop)"
+endchoice
+endif # PREEMPT_DYNAMIC

config PREEMPT_COUNT
bool
@@ -93,27 +132,6 @@ config PREEMPTION
bool
select PREEMPT_COUNT

-config PREEMPT_DYNAMIC
- bool "Preemption behaviour defined on boot"
- depends on HAVE_PREEMPT_DYNAMIC
- select PREEMPT
- default y
- help
- This option allows to define the preemption model on the kernel
- command line parameter and thus override the default preemption
- model defined during compile time.
-
- The feature is primarily interesting for Linux distributions which
- provide a pre-built kernel binary to reduce the number of kernel
- flavors they offer while still offering different usecases.
-
- The runtime overhead is negligible with HAVE_STATIC_CALL_INLINE enabled
- but if runtime patching is not available for the specific architecture
- then the potential overhead should be considered.
-
- Interesting if you want the same pre-built kernel should be used for
- both Server and Desktop workloads.
-
config SCHED_CORE
bool "Core Scheduling for SMT"
depends on SCHED_SMT




2021-11-09 23:38:34

by Valentin Schneider

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

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

2021-11-09 23:51:18

by Mike Galbraith

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

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

2021-11-10 00:51:55

by Valentin Schneider

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

On 09/11/21 11:25, Frederic Weisbecker wrote:
> On Fri, Nov 05, 2021 at 10:40:35AM +0000, Valentin Schneider wrote:
>> @@ -75,17 +86,6 @@ config PREEMPT_RT
>>
>> endchoice
>>
>> -config PREEMPT_NONE
>> - bool
>> -
>> -config PREEMPT_VOLUNTARY
>> - bool
>> -
>> -config PREEMPT
>> - bool
>> - select PREEMPTION
>> - select UNINLINE_SPIN_UNLOCK if !ARCH_INLINE_SPIN_UNLOCK
>> -
>> config PREEMPT_COUNT
>> bool
>
> This must be breaking cond_resched() and might_resched() definitions.
>
> Since CONFIG_PREEMPT_NONE, CONFIG_PREEMPT_VOLUNTARY and CONFIG_PREEMPT aren't too widely
> spread around within ifdefferies, you can:
>
> 1) Rename CONFIG_PREEMPT_NONE to CONFIG_PREEMPT_NONE_STATIC
> Rename CONFIG_PREEMPT_VOLUNTARY to CONFIG_PREEMPT_VOLUNTARY_STATIC
> Rename CONFIG_PREEMPT to CONFIG_PREEMPT_STATIC
>
> 2) Keep the old CONFIG_PREEMPT_NONE, CONFIG_PREEMPT_VOLUNTARY,
> CONFIG_PREEMPT around for compatibility and make them select their
> corresponding BEHAVIOUR entries.
>

Damn, I was too zealous on my inbox clearing and skipped that one, sorry!
(and ofc in the meantime Mike and I went down separate rabbit holes).

The old CONFIG_PREEMPT* would have to remain somewhere in the menu entry
(e.g. "legacy" preemption mode selection?), but other than that it seems to
work with this kind of structure:

choice
prompt "Preemption Model"
default PREEMPT_BEHAVIOUR if PREEMPT
default PREEMPT_VOLUNTARY_BEHAVIOUR if PREEMPT_VOLUNTARY
default PREEMPT_NONE_BEHAVIOUR

This however doesn't solve Mike's concern wrt deselecting PREEMPT_RT to
flip PREEMPT_DYNAMIC and vice versa...


> Thanks.
>

2021-11-10 05:48:09

by Mike Galbraith

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

On Wed, 2021-11-10 at 00:03 +0000, Valentin Schneider wrote:
> On 09/11/21 11:25, Frederic Weisbecker wrote:
> > On Fri, Nov 05, 2021 at 10:40:35AM +0000, Valentin Schneider wrote:
> > > @@ -75,17 +86,6 @@ config PREEMPT_RT
> > >
> > >  endchoice
> > >
> > > -config PREEMPT_NONE
> > > -       bool
> > > -
> > > -config PREEMPT_VOLUNTARY
> > > -       bool
> > > -
> > > -config PREEMPT
> > > -       bool
> > > -       select PREEMPTION
> > > -       select UNINLINE_SPIN_UNLOCK if !ARCH_INLINE_SPIN_UNLOCK
> > > -
> > >  config PREEMPT_COUNT
> > >         bool
> >
> > This must be breaking cond_resched() and might_resched() definitions.
> >
> > Since CONFIG_PREEMPT_NONE, CONFIG_PREEMPT_VOLUNTARY and CONFIG_PREEMPT aren't too widely
> > spread around within ifdefferies, you can:
> >
> > 1) Rename CONFIG_PREEMPT_NONE to CONFIG_PREEMPT_NONE_STATIC
> >    Rename CONFIG_PREEMPT_VOLUNTARY to CONFIG_PREEMPT_VOLUNTARY_STATIC
> >    Rename CONFIG_PREEMPT to CONFIG_PREEMPT_STATIC
> >
> > 2) Keep the old CONFIG_PREEMPT_NONE, CONFIG_PREEMPT_VOLUNTARY,
> >    CONFIG_PREEMPT around for compatibility and make them select their
> >    corresponding BEHAVIOUR entries.
> >
>
> Damn, I was too zealous on my inbox clearing and skipped that one, sorry!
> (and ofc in the meantime Mike and I went down separate rabbit holes).
>
> The old CONFIG_PREEMPT* would have to remain somewhere in the menu entry
> (e.g. "legacy" preemption mode selection?), but other than that it seems to
> work with this kind of structure:
>
> choice
>         prompt "Preemption Model"
>         default PREEMPT_BEHAVIOUR if PREEMPT
>         default PREEMPT_VOLUNTARY_BEHAVIOUR if PREEMPT_VOLUNTARY
>         default PREEMPT_NONE_BEHAVIOUR
>
> This however doesn't solve Mike's concern wrt deselecting PREEMPT_RT to
> flip PREEMPT_DYNAMIC and vice versa...

OTOH it's not quite here/now relevant, whereas your config woes are...

-Mike