[PATCH 0/6] arm64 / sched/preempt: support PREEMPT_DYNAMIC with static keys

[PATCH 0/6] arm64 / sched/preempt: support PREEMPT_DYNAMIC with static keys

[Mark Rutland]

This series enables PREEMPT_DYNAMIC on arm64, offering an alternative approach
to other efforts which rely on enabling static_calls, e.g. as Ard is currently
attempting:

  https://lore.kernel.org/linux-arm-kernel/20211105145917.2828911-1-ardb@kernel.org/

For a number of reasons (laid out in grauitous detail in patch 5), static calls
are somewhat painful on arm64. For PREEMPT_DYNAMIC specifically where we're
only enabling/disabling functions rather than targetting multiple distinct
callees, (non-inline) static calls don't buy us much over placing early returns
in the preemption functions, which this series implements using static keys.

The first 4 patches are largely cleanup, and I think might make sense on their
own. Patch 3 specifically change the behaviour on x86 where I believe there's
simply an oversight, called out in the commit message.

I think the diffstate can be reduced a bit by with some helper macros to reduce
amount of boilerplate needed for the callees. There's also some room for
cleanup of the existing preempt logic to require less arch code (other than
where x86 has to override that today).

I've given this very light build+boot testing so far.

Thanks,
Mark.

Mark Rutland (6):
  sched/preempt: move PREEMPT_DYNAMIC logic later
  sched/preempt: refactor sched_dynamic_update()
  sched/preempt: simplify irqentry_exit_cond_resched() callers
  sched/preempt: decouple HAVE_PREEMPT_DYNAMIC from GENERIC_ENTRY
  sched/preempt: add PREEMPT_DYNAMIC using static keys
  arm64: support PREEMPT_DYNAMIC

 arch/Kconfig                     |  14 +-
 arch/arm64/Kconfig               |   1 +
 arch/arm64/include/asm/preempt.h |  16 +-
 arch/arm64/kernel/entry-common.c |   9 +
 arch/x86/Kconfig                 |   2 +-
 arch/x86/include/asm/preempt.h   |  10 +-
 include/linux/entry-common.h     |  15 +-
 include/linux/kernel.h           |   7 +-
 include/linux/sched.h            |  10 +-
 kernel/entry/common.c            |  22 ++-
 kernel/sched/core.c              | 347 +++++++++++++++++++++++----------------
 11 files changed, 293 insertions(+), 160 deletions(-)

-- 
2.11.0

[PATCH 1/6] sched/preempt: move PREEMPT_DYNAMIC logic later

[Mark Rutland]

The PREEMPT_DYNAMIC logic in kernel/sched/core.c patches static calls
for a bunch of preemption functions. While most are defined prior to
this, the definition of cond_resched() is later in the file, and so we
only have its declarations from include/linux/sched.h.

In subsequent patches we'd like to define some macros alongside the
definition of each of the preemption functions, which we can use within
sched_dynamic_update(). For this to be possible, the PREEMPT_DYNAMIC
logic needs to be placed after the various preemption functions.

As a preparatory step, this patch moves the PREEMPT_DYNAMIC logic after
the various preemption functions, with no other changes -- this is
purely a move.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Frederic Weisbecker <frederic@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/sched/core.c | 273 ++++++++++++++++++++++++++--------------------------
 1 file changed, 137 insertions(+), 136 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 523fd602ea90..ece89f3e3d93 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6508,142 +6508,6 @@ EXPORT_STATIC_CALL_TRAMP(preempt_schedule_notrace);
 
 #endif /* CONFIG_PREEMPTION */
 
-#ifdef CONFIG_PREEMPT_DYNAMIC
-
-#include <linux/entry-common.h>
-
-/*
- * SC:cond_resched
- * SC:might_resched
- * SC:preempt_schedule
- * SC:preempt_schedule_notrace
- * SC:irqentry_exit_cond_resched
- *
- *
- * NONE:
- *   cond_resched               <- __cond_resched
- *   might_resched              <- RET0
- *   preempt_schedule           <- NOP
- *   preempt_schedule_notrace   <- NOP
- *   irqentry_exit_cond_resched <- NOP
- *
- * VOLUNTARY:
- *   cond_resched               <- __cond_resched
- *   might_resched              <- __cond_resched
- *   preempt_schedule           <- NOP
- *   preempt_schedule_notrace   <- NOP
- *   irqentry_exit_cond_resched <- NOP
- *
- * FULL:
- *   cond_resched               <- RET0
- *   might_resched              <- RET0
- *   preempt_schedule           <- preempt_schedule
- *   preempt_schedule_notrace   <- preempt_schedule_notrace
- *   irqentry_exit_cond_resched <- irqentry_exit_cond_resched
- */
-
-enum {
-	preempt_dynamic_undefined = -1,
-	preempt_dynamic_none,
-	preempt_dynamic_voluntary,
-	preempt_dynamic_full,
-};
-
-int preempt_dynamic_mode = preempt_dynamic_undefined;
-
-int sched_dynamic_mode(const char *str)
-{
-	if (!strcmp(str, "none"))
-		return preempt_dynamic_none;
-
-	if (!strcmp(str, "voluntary"))
-		return preempt_dynamic_voluntary;
-
-	if (!strcmp(str, "full"))
-		return preempt_dynamic_full;
-
-	return -EINVAL;
-}
-
-void sched_dynamic_update(int mode)
-{
-	/*
-	 * Avoid {NONE,VOLUNTARY} -> FULL transitions from ever ending up in
-	 * the ZERO state, which is invalid.
-	 */
-	static_call_update(cond_resched, __cond_resched);
-	static_call_update(might_resched, __cond_resched);
-	static_call_update(preempt_schedule, __preempt_schedule_func);
-	static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
-	static_call_update(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
-
-	switch (mode) {
-	case preempt_dynamic_none:
-		static_call_update(cond_resched, __cond_resched);
-		static_call_update(might_resched, (void *)&__static_call_return0);
-		static_call_update(preempt_schedule, NULL);
-		static_call_update(preempt_schedule_notrace, NULL);
-		static_call_update(irqentry_exit_cond_resched, NULL);
-		pr_info("Dynamic Preempt: none\n");
-		break;
-
-	case preempt_dynamic_voluntary:
-		static_call_update(cond_resched, __cond_resched);
-		static_call_update(might_resched, __cond_resched);
-		static_call_update(preempt_schedule, NULL);
-		static_call_update(preempt_schedule_notrace, NULL);
-		static_call_update(irqentry_exit_cond_resched, NULL);
-		pr_info("Dynamic Preempt: voluntary\n");
-		break;
-
-	case preempt_dynamic_full:
-		static_call_update(cond_resched, (void *)&__static_call_return0);
-		static_call_update(might_resched, (void *)&__static_call_return0);
-		static_call_update(preempt_schedule, __preempt_schedule_func);
-		static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
-		static_call_update(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
-		pr_info("Dynamic Preempt: full\n");
-		break;
-	}
-
-	preempt_dynamic_mode = mode;
-}
-
-static int __init setup_preempt_mode(char *str)
-{
-	int mode = sched_dynamic_mode(str);
-	if (mode < 0) {
-		pr_warn("Dynamic Preempt: unsupported mode: %s\n", str);
-		return 1;
-	}
-
-	sched_dynamic_update(mode);
-	return 0;
-}
-__setup("preempt=", setup_preempt_mode);
-
-static void __init preempt_dynamic_init(void)
-{
-	if (preempt_dynamic_mode == preempt_dynamic_undefined) {
-		if (IS_ENABLED(CONFIG_PREEMPT_NONE_BEHAVIOUR)) {
-			sched_dynamic_update(preempt_dynamic_none);
-		} else if (IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR)) {
-			sched_dynamic_update(preempt_dynamic_voluntary);
-		} else {
-			/* Default static call setting, nothing to do */
-			WARN_ON_ONCE(!IS_ENABLED(CONFIG_PREEMPT_BEHAVIOUR));
-			preempt_dynamic_mode = preempt_dynamic_full;
-			pr_info("Dynamic Preempt: full\n");
-		}
-	}
-}
-
-#else /* !CONFIG_PREEMPT_DYNAMIC */
-
-static inline void preempt_dynamic_init(void) { }
-
-#endif /* #ifdef CONFIG_PREEMPT_DYNAMIC */
-
 /*
  * This is the entry point to schedule() from kernel preemption
  * off of irq context.
@@ -8224,6 +8088,143 @@ int __cond_resched_rwlock_write(rwlock_t *lock)
 }
 EXPORT_SYMBOL(__cond_resched_rwlock_write);
 
+#ifdef CONFIG_PREEMPT_DYNAMIC
+
+#include <linux/entry-common.h>
+
+/*
+ * SC:cond_resched
+ * SC:might_resched
+ * SC:preempt_schedule
+ * SC:preempt_schedule_notrace
+ * SC:irqentry_exit_cond_resched
+ *
+ *
+ * NONE:
+ *   cond_resched               <- __cond_resched
+ *   might_resched              <- RET0
+ *   preempt_schedule           <- NOP
+ *   preempt_schedule_notrace   <- NOP
+ *   irqentry_exit_cond_resched <- NOP
+ *
+ * VOLUNTARY:
+ *   cond_resched               <- __cond_resched
+ *   might_resched              <- __cond_resched
+ *   preempt_schedule           <- NOP
+ *   preempt_schedule_notrace   <- NOP
+ *   irqentry_exit_cond_resched <- NOP
+ *
+ * FULL:
+ *   cond_resched               <- RET0
+ *   might_resched              <- RET0
+ *   preempt_schedule           <- preempt_schedule
+ *   preempt_schedule_notrace   <- preempt_schedule_notrace
+ *   irqentry_exit_cond_resched <- irqentry_exit_cond_resched
+ */
+
+enum {
+	preempt_dynamic_undefined = -1,
+	preempt_dynamic_none,
+	preempt_dynamic_voluntary,
+	preempt_dynamic_full,
+};
+
+int preempt_dynamic_mode = preempt_dynamic_undefined;
+
+int sched_dynamic_mode(const char *str)
+{
+	if (!strcmp(str, "none"))
+		return preempt_dynamic_none;
+
+	if (!strcmp(str, "voluntary"))
+		return preempt_dynamic_voluntary;
+
+	if (!strcmp(str, "full"))
+		return preempt_dynamic_full;
+
+	return -EINVAL;
+}
+
+void sched_dynamic_update(int mode)
+{
+	/*
+	 * Avoid {NONE,VOLUNTARY} -> FULL transitions from ever ending up in
+	 * the ZERO state, which is invalid.
+	 */
+	static_call_update(cond_resched, __cond_resched);
+	static_call_update(might_resched, __cond_resched);
+	static_call_update(preempt_schedule, __preempt_schedule_func);
+	static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
+	static_call_update(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
+
+	switch (mode) {
+	case preempt_dynamic_none:
+		static_call_update(cond_resched, __cond_resched);
+		static_call_update(might_resched, (void *)&__static_call_return0);
+		static_call_update(preempt_schedule, NULL);
+		static_call_update(preempt_schedule_notrace, NULL);
+		static_call_update(irqentry_exit_cond_resched, NULL);
+		pr_info("Dynamic Preempt: none\n");
+		break;
+
+	case preempt_dynamic_voluntary:
+		static_call_update(cond_resched, __cond_resched);
+		static_call_update(might_resched, __cond_resched);
+		static_call_update(preempt_schedule, NULL);
+		static_call_update(preempt_schedule_notrace, NULL);
+		static_call_update(irqentry_exit_cond_resched, NULL);
+		pr_info("Dynamic Preempt: voluntary\n");
+		break;
+
+	case preempt_dynamic_full:
+		static_call_update(cond_resched, (void *)&__static_call_return0);
+		static_call_update(might_resched, (void *)&__static_call_return0);
+		static_call_update(preempt_schedule, __preempt_schedule_func);
+		static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
+		static_call_update(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
+		pr_info("Dynamic Preempt: full\n");
+		break;
+	}
+
+	preempt_dynamic_mode = mode;
+}
+
+static int __init setup_preempt_mode(char *str)
+{
+	int mode = sched_dynamic_mode(str);
+	if (mode < 0) {
+		pr_warn("Dynamic Preempt: unsupported mode: %s\n", str);
+		return 1;
+	}
+
+	sched_dynamic_update(mode);
+	return 0;
+}
+__setup("preempt=", setup_preempt_mode);
+
+static void __init preempt_dynamic_init(void)
+{
+	if (preempt_dynamic_mode == preempt_dynamic_undefined) {
+		if (IS_ENABLED(CONFIG_PREEMPT_NONE_BEHAVIOUR)) {
+			sched_dynamic_update(preempt_dynamic_none);
+		} else if (IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR)) {
+			sched_dynamic_update(preempt_dynamic_voluntary);
+		} else {
+			/* Default static call setting, nothing to do */
+			WARN_ON_ONCE(!IS_ENABLED(CONFIG_PREEMPT_BEHAVIOUR));
+			preempt_dynamic_mode = preempt_dynamic_full;
+			pr_info("Dynamic Preempt: full\n");
+		}
+	}
+}
+
+#else /* !CONFIG_PREEMPT_DYNAMIC */
+
+static inline void preempt_dynamic_init(void) { }
+
+#endif /* #ifdef CONFIG_PREEMPT_DYNAMIC */
+
+
 /**
  * yield - yield the current processor to other threads.
  *
-- 
2.11.0

[PATCH 2/6] sched/preempt: refactor sched_dynamic_update()

[Mark Rutland]

Currently sched_dynamic_update needs to open-code the enabled/disabled
function names for each preemption model it supoprts, when in practice
this is a boolean enabled/disabled state for each function.

Make this clearer and avoid repetition by defining the enabled/disabled
states at the function definition, and using helper macros to peform the
static_call_update(). Where x86 currently overrides the enabled
function, it is made to provide both the enabled and disabled states for
consistency, with defaults provided by the core code otherwise.

In subsequent patches this will allow us to support PREEMPT_DYNAMIC
without static calls.

There shoud be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Frederic Weisbecker <frederic@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 arch/x86/include/asm/preempt.h | 10 ++++---
 include/linux/entry-common.h   |  2 ++
 kernel/sched/core.c            | 59 ++++++++++++++++++++++++++----------------
 3 files changed, 45 insertions(+), 26 deletions(-)

diff --git a/arch/x86/include/asm/preempt.h b/arch/x86/include/asm/preempt.h
index fe5efbcba824..5f6daea1ee24 100644
--- a/arch/x86/include/asm/preempt.h
+++ b/arch/x86/include/asm/preempt.h
@@ -108,16 +108,18 @@ static __always_inline bool should_resched(int preempt_offset)
 extern asmlinkage void preempt_schedule(void);
 extern asmlinkage void preempt_schedule_thunk(void);
 
-#define __preempt_schedule_func preempt_schedule_thunk
+#define preempt_schedule_dynamic_enabled	preempt_schedule_thunk
+#define preempt_schedule_dynamic_disabled	NULL
 
 extern asmlinkage void preempt_schedule_notrace(void);
 extern asmlinkage void preempt_schedule_notrace_thunk(void);
 
-#define __preempt_schedule_notrace_func preempt_schedule_notrace_thunk
+#define preempt_schedule_notrace_dynamic_enabled	preempt_schedule_notrace_thunk
+#define preempt_schedule_notrace_dynamic_disabled	NULL
 
 #ifdef CONFIG_PREEMPT_DYNAMIC
 
-DECLARE_STATIC_CALL(preempt_schedule, __preempt_schedule_func);
+DECLARE_STATIC_CALL(preempt_schedule, preempt_schedule_dynamic_enabled);
 
 #define __preempt_schedule() \
 do { \
@@ -125,7 +127,7 @@ do { \
 	asm volatile ("call " STATIC_CALL_TRAMP_STR(preempt_schedule) : ASM_CALL_CONSTRAINT); \
 } while (0)
 
-DECLARE_STATIC_CALL(preempt_schedule_notrace, __preempt_schedule_notrace_func);
+DECLARE_STATIC_CALL(preempt_schedule_notrace, preempt_schedule_notrace_dynamic_enabled);
 
 #define __preempt_schedule_notrace() \
 do { \
diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h
index 2e2b8d6140ed..a01ac1a0a292 100644
--- a/include/linux/entry-common.h
+++ b/include/linux/entry-common.h
@@ -456,6 +456,8 @@ irqentry_state_t noinstr irqentry_enter(struct pt_regs *regs);
  */
 void irqentry_exit_cond_resched(void);
 #ifdef CONFIG_PREEMPT_DYNAMIC
+#define irqentry_exit_cond_resched_dynamic_enabled	irqentry_exit_cond_resched
+#define irqentry_exit_cond_resched_dynamic_disabled	NULL
 DECLARE_STATIC_CALL(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
 #endif
 
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index ece89f3e3d93..3a1caa9a095a 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6444,7 +6444,11 @@ NOKPROBE_SYMBOL(preempt_schedule);
 EXPORT_SYMBOL(preempt_schedule);
 
 #ifdef CONFIG_PREEMPT_DYNAMIC
-DEFINE_STATIC_CALL(preempt_schedule, __preempt_schedule_func);
+#ifndef preempt_schedule_dynamic_enabled
+#define preempt_schedule_dynamic_enabled	preempt_schedule
+#define preempt_schedule_dynamic_disabled	NULL
+#endif
+DEFINE_STATIC_CALL(preempt_schedule, preempt_schedule_dynamic_enabled);
 EXPORT_STATIC_CALL_TRAMP(preempt_schedule);
 #endif
 
@@ -6502,7 +6506,11 @@ asmlinkage __visible void __sched notrace preempt_schedule_notrace(void)
 EXPORT_SYMBOL_GPL(preempt_schedule_notrace);
 
 #ifdef CONFIG_PREEMPT_DYNAMIC
-DEFINE_STATIC_CALL(preempt_schedule_notrace, __preempt_schedule_notrace_func);
+#ifndef preempt_schedule_notrace_dynamic_enabled
+#define preempt_schedule_notrace_dynamic_enabled	preempt_schedule_notrace
+#define preempt_schedule_notrace_dynamic_disabled	NULL
+#endif
+DEFINE_STATIC_CALL(preempt_schedule_notrace, preempt_schedule_notrace_dynamic_enabled);
 EXPORT_STATIC_CALL_TRAMP(preempt_schedule_notrace);
 #endif
 
@@ -8013,9 +8021,13 @@ EXPORT_SYMBOL(__cond_resched);
 #endif
 
 #ifdef CONFIG_PREEMPT_DYNAMIC
+#define cond_resched_dynamic_enabled	__cond_resched
+#define cond_resched_dynamic_disabled	((void *)&__static_call_return0)
 DEFINE_STATIC_CALL_RET0(cond_resched, __cond_resched);
 EXPORT_STATIC_CALL_TRAMP(cond_resched);
 
+#define might_resched_dynamic_enabled	__cond_resched
+#define might_resched_dynamic_disabled	((void *)&__static_call_return0)
 DEFINE_STATIC_CALL_RET0(might_resched, __cond_resched);
 EXPORT_STATIC_CALL_TRAMP(might_resched);
 #endif
@@ -8145,43 +8157,46 @@ int sched_dynamic_mode(const char *str)
 	return -EINVAL;
 }
 
+#define preempt_dynamic_enable(f)	static_call_update(f, f##_dynamic_enabled)
+#define preempt_dynamic_disable(f)	static_call_update(f, f##_dynamic_disabled)
+
 void sched_dynamic_update(int mode)
 {
 	/*
 	 * Avoid {NONE,VOLUNTARY} -> FULL transitions from ever ending up in
 	 * the ZERO state, which is invalid.
 	 */
-	static_call_update(cond_resched, __cond_resched);
-	static_call_update(might_resched, __cond_resched);
-	static_call_update(preempt_schedule, __preempt_schedule_func);
-	static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
-	static_call_update(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
+	preempt_dynamic_enable(cond_resched);
+	preempt_dynamic_enable(might_resched);
+	preempt_dynamic_enable(preempt_schedule);
+	preempt_dynamic_enable(preempt_schedule_notrace);
+	preempt_dynamic_enable(irqentry_exit_cond_resched);
 
 	switch (mode) {
 	case preempt_dynamic_none:
-		static_call_update(cond_resched, __cond_resched);
-		static_call_update(might_resched, (void *)&__static_call_return0);
-		static_call_update(preempt_schedule, NULL);
-		static_call_update(preempt_schedule_notrace, NULL);
-		static_call_update(irqentry_exit_cond_resched, NULL);
+		preempt_dynamic_enable(cond_resched);
+		preempt_dynamic_disable(might_resched);
+		preempt_dynamic_disable(preempt_schedule);
+		preempt_dynamic_disable(preempt_schedule_notrace);
+		preempt_dynamic_disable(irqentry_exit_cond_resched);
 		pr_info("Dynamic Preempt: none\n");
 		break;
 
 	case preempt_dynamic_voluntary:
-		static_call_update(cond_resched, __cond_resched);
-		static_call_update(might_resched, __cond_resched);
-		static_call_update(preempt_schedule, NULL);
-		static_call_update(preempt_schedule_notrace, NULL);
-		static_call_update(irqentry_exit_cond_resched, NULL);
+		preempt_dynamic_enable(cond_resched);
+		preempt_dynamic_enable(might_resched);
+		preempt_dynamic_disable(preempt_schedule);
+		preempt_dynamic_disable(preempt_schedule_notrace);
+		preempt_dynamic_disable(irqentry_exit_cond_resched);
 		pr_info("Dynamic Preempt: voluntary\n");
 		break;
 
 	case preempt_dynamic_full:
-		static_call_update(cond_resched, (void *)&__static_call_return0);
-		static_call_update(might_resched, (void *)&__static_call_return0);
-		static_call_update(preempt_schedule, __preempt_schedule_func);
-		static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
-		static_call_update(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
+		preempt_dynamic_disable(cond_resched);
+		preempt_dynamic_disable(might_resched);
+		preempt_dynamic_enable(preempt_schedule);
+		preempt_dynamic_enable(preempt_schedule_notrace);
+		preempt_dynamic_enable(irqentry_exit_cond_resched);
 		pr_info("Dynamic Preempt: full\n");
 		break;
 	}
-- 
2.11.0

Re: [PATCH 2/6] sched/preempt: refactor sched_dynamic_update()

[Frederic Weisbecker]

On Tue, Nov 09, 2021 at 05:24:04PM +0000, Mark Rutland wrote:
> Currently sched_dynamic_update needs to open-code the enabled/disabled
> function names for each preemption model it supoprts, when in practice
> this is a boolean enabled/disabled state for each function.
> 
> Make this clearer and avoid repetition by defining the enabled/disabled
> states at the function definition, and using helper macros to peform the
> static_call_update(). Where x86 currently overrides the enabled
> function, it is made to provide both the enabled and disabled states for
> consistency, with defaults provided by the core code otherwise.
> 
> In subsequent patches this will allow us to support PREEMPT_DYNAMIC
> without static calls.
> 
> There shoud be no functional change as a result of this patch.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Frederic Weisbecker <frederic@kernel.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Juri Lelli <juri.lelli@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> ---
>  arch/x86/include/asm/preempt.h | 10 ++++---
>  include/linux/entry-common.h   |  2 ++
>  kernel/sched/core.c            | 59 ++++++++++++++++++++++++++----------------
>  3 files changed, 45 insertions(+), 26 deletions(-)
> 
> diff --git a/arch/x86/include/asm/preempt.h b/arch/x86/include/asm/preempt.h
> index fe5efbcba824..5f6daea1ee24 100644
> --- a/arch/x86/include/asm/preempt.h
> +++ b/arch/x86/include/asm/preempt.h
> @@ -108,16 +108,18 @@ static __always_inline bool should_resched(int preempt_offset)
>  extern asmlinkage void preempt_schedule(void);
>  extern asmlinkage void preempt_schedule_thunk(void);
>  
> -#define __preempt_schedule_func preempt_schedule_thunk
> +#define preempt_schedule_dynamic_enabled	preempt_schedule_thunk
> +#define preempt_schedule_dynamic_disabled	NULL
>  
>  extern asmlinkage void preempt_schedule_notrace(void);
>  extern asmlinkage void preempt_schedule_notrace_thunk(void);
>  
> -#define __preempt_schedule_notrace_func preempt_schedule_notrace_thunk
> +#define preempt_schedule_notrace_dynamic_enabled	preempt_schedule_notrace_thunk
> +#define preempt_schedule_notrace_dynamic_disabled	NULL

I'm worried about un-greppable macro definitions like this. Also this
enable/disable switch look like a common pattern on static call so how
about moving that logic to static call itself? As in below (only build-tested):

diff --git a/arch/x86/include/asm/preempt.h b/arch/x86/include/asm/preempt.h
index fe5efbcba824..64320c0a00a6 100644
--- a/arch/x86/include/asm/preempt.h
+++ b/arch/x86/include/asm/preempt.h
@@ -117,7 +117,7 @@ extern asmlinkage void preempt_schedule_notrace_thunk(void);
 
 #ifdef CONFIG_PREEMPT_DYNAMIC
 
-DECLARE_STATIC_CALL(preempt_schedule, __preempt_schedule_func);
+DECLARE_STATIC_CALL_TOGGLE(preempt_schedule, __preempt_schedule_func);
 
 #define __preempt_schedule() \
 do { \
@@ -125,7 +125,7 @@ do { \
 	asm volatile ("call " STATIC_CALL_TRAMP_STR(preempt_schedule) : ASM_CALL_CONSTRAINT); \
 } while (0)
 
-DECLARE_STATIC_CALL(preempt_schedule_notrace, __preempt_schedule_notrace_func);
+DECLARE_STATIC_CALL_TOGGLE(preempt_schedule_notrace, __preempt_schedule_notrace_func);
 
 #define __preempt_schedule_notrace() \
 do { \
diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h
index 2e2b8d6140ed..a3d99ffc32ca 100644
--- a/include/linux/entry-common.h
+++ b/include/linux/entry-common.h
@@ -456,7 +456,7 @@ irqentry_state_t noinstr irqentry_enter(struct pt_regs *regs);
  */
 void irqentry_exit_cond_resched(void);
 #ifdef CONFIG_PREEMPT_DYNAMIC
-DECLARE_STATIC_CALL(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
+DECLARE_STATIC_CALL_TOGGLE(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
 #endif
 
 /**
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 77755ac3e189..f4f5f4f3b496 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -94,7 +94,7 @@ extern int __cond_resched(void);
 
 extern int __cond_resched(void);
 
-DECLARE_STATIC_CALL(might_resched, __cond_resched);
+DECLARE_STATIC_CALL_TOGGLE(might_resched, __cond_resched);
 
 static __always_inline void might_resched(void)
 {
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 78c351e35fec..01a9e8590ea0 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2010,7 +2010,7 @@ extern int __cond_resched(void);
 
 #ifdef CONFIG_PREEMPT_DYNAMIC
 
-DECLARE_STATIC_CALL(cond_resched, __cond_resched);
+DECLARE_STATIC_CALL_TOGGLE(cond_resched, __cond_resched);
 
 static __always_inline int _cond_resched(void)
 {
diff --git a/include/linux/static_call.h b/include/linux/static_call.h
index 3e56a9751c06..351ac27fa9d1 100644
--- a/include/linux/static_call.h
+++ b/include/linux/static_call.h
@@ -156,6 +156,19 @@ extern void arch_static_call_transform(void *site, void *tramp, void *func, bool
 			     STATIC_CALL_TRAMP_ADDR(name), __F);	\
 })
 
+#define static_call_toggle(name, enable)				\
+({									\
+	struct static_call_toggle *__G = &STATIC_CALL_TOGGLE(name);	\
+	if (enable)							\
+		static_call_update(name, __G->func_enabled);		\
+	else								\
+		static_call_update(name, __G->func_disabled);		\
+})
+
+#define static_call_enable(name) static_call_toggle(name, 1);
+#define static_call_disable(name) static_call_toggle(name, 0);
+
+
 #define static_call_query(name) (READ_ONCE(STATIC_CALL_KEY(name).func))
 
 #ifdef CONFIG_HAVE_STATIC_CALL_INLINE
@@ -333,4 +346,27 @@ static inline int static_call_text_reserved(void *start, void *end)
 #define DEFINE_STATIC_CALL_RET0(name, _func)				\
 	__DEFINE_STATIC_CALL(name, _func, __static_call_return0)
 
+#define __DEFINE_STATIC_CALL_TOGGLE(name, _func_enabled, _func_disabled) \
+	struct static_call_toggle STATIC_CALL_TOGGLE(name) = {			\
+		.key = &STATIC_CALL_KEY(name),					\
+		.func_enabled = _func_enabled,					\
+		.func_disabled = _func_disabled,				\
+	}
+
+#define DEFINE_STATIC_CALL_ENABLED(name, _func)					\
+	DEFINE_STATIC_CALL(name, _func);					\
+	__DEFINE_STATIC_CALL_TOGGLE(name, _func, NULL)
+
+#define DEFINE_STATIC_CALL_DISABLED(name, _func)				\
+	DEFINE_STATIC_CALL_NULL(name, _func);					\
+	__DEFINE_STATIC_CALL_TOGGLE(name, _func, NULL)
+
+#define DEFINE_STATIC_CALL_ENABLED_RET0(name, _func)				\
+	DEFINE_STATIC_CALL(name, _func);					\
+	__DEFINE_STATIC_CALL_TOGGLE(name, _func, __static_call_return0)
+
+#define DEFINE_STATIC_CALL_DISABLED_RET0(name, _func)				\
+	DEFINE_STATIC_CALL_RET0(name, _func);					\
+	__DEFINE_STATIC_CALL_TOGGLE(name, _func, __static_call_return0)
+
 #endif /* _LINUX_STATIC_CALL_H */
diff --git a/include/linux/static_call_types.h b/include/linux/static_call_types.h
index 5a00b8b2cf9f..e150f29aee2d 100644
--- a/include/linux/static_call_types.h
+++ b/include/linux/static_call_types.h
@@ -18,6 +18,9 @@
 #define STATIC_CALL_TRAMP(name)		__PASTE(STATIC_CALL_TRAMP_PREFIX, name)
 #define STATIC_CALL_TRAMP_STR(name)	__stringify(STATIC_CALL_TRAMP(name))
 
+#define STATIC_CALL_TOGGLE_PREFIX	__SCG_
+#define STATIC_CALL_TOGGLE(name)	__PASTE(STATIC_CALL_TOGGLE_PREFIX, name)
+
 /*
  * Flags in the low bits of static_call_site::key.
  */
@@ -38,6 +41,10 @@ struct static_call_site {
 	extern struct static_call_key STATIC_CALL_KEY(name);		\
 	extern typeof(func) STATIC_CALL_TRAMP(name);
 
+#define DECLARE_STATIC_CALL_TOGGLE(name, func)				\
+	DECLARE_STATIC_CALL(name, func);				\
+	extern struct static_call_toggle STATIC_CALL_TOGGLE(name);
+
 #ifdef CONFIG_HAVE_STATIC_CALL
 
 #define __raw_static_call(name)	(&STATIC_CALL_TRAMP(name))
@@ -68,6 +75,13 @@ struct static_call_key {
 	};
 };
 
+struct static_call_toggle {
+	struct static_call_key *key;
+	void *func_enabled;
+	void *func_disabled;
+};
+
+
 #else /* !CONFIG_HAVE_STATIC_CALL_INLINE */
 
 #define __STATIC_CALL_ADDRESSABLE(name)
diff --git a/kernel/entry/common.c b/kernel/entry/common.c
index d5a61d565ad5..d48b91579475 100644
--- a/kernel/entry/common.c
+++ b/kernel/entry/common.c
@@ -392,7 +392,7 @@ void irqentry_exit_cond_resched(void)
 	}
 }
 #ifdef CONFIG_PREEMPT_DYNAMIC
-DEFINE_STATIC_CALL(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
+DEFINE_STATIC_CALL_ENABLED(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
 #endif
 
 noinstr void irqentry_exit(struct pt_regs *regs, irqentry_state_t state)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 27c795fbcaaf..389fcc412e6b 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6447,7 +6447,7 @@ NOKPROBE_SYMBOL(preempt_schedule);
 EXPORT_SYMBOL(preempt_schedule);
 
 #ifdef CONFIG_PREEMPT_DYNAMIC
-DEFINE_STATIC_CALL(preempt_schedule, __preempt_schedule_func);
+DEFINE_STATIC_CALL_ENABLED(preempt_schedule, __preempt_schedule_func);
 EXPORT_STATIC_CALL_TRAMP(preempt_schedule);
 #endif
 
@@ -6505,7 +6505,7 @@ asmlinkage __visible void __sched notrace preempt_schedule_notrace(void)
 EXPORT_SYMBOL_GPL(preempt_schedule_notrace);
 
 #ifdef CONFIG_PREEMPT_DYNAMIC
-DEFINE_STATIC_CALL(preempt_schedule_notrace, __preempt_schedule_notrace_func);
+DEFINE_STATIC_CALL_ENABLED(preempt_schedule_notrace, __preempt_schedule_notrace_func);
 EXPORT_STATIC_CALL_TRAMP(preempt_schedule_notrace);
 #endif
 
@@ -8016,10 +8016,9 @@ EXPORT_SYMBOL(__cond_resched);
 #endif
 
 #ifdef CONFIG_PREEMPT_DYNAMIC
-DEFINE_STATIC_CALL_RET0(cond_resched, __cond_resched);
+DEFINE_STATIC_CALL_DISABLED_RET0(cond_resched, __cond_resched);
 EXPORT_STATIC_CALL_TRAMP(cond_resched);
-
-DEFINE_STATIC_CALL_RET0(might_resched, __cond_resched);
+DEFINE_STATIC_CALL_DISABLED_RET0(might_resched, __cond_resched);
 EXPORT_STATIC_CALL_TRAMP(might_resched);
 #endif
 
@@ -8154,37 +8153,37 @@ void sched_dynamic_update(int mode)
 	 * Avoid {NONE,VOLUNTARY} -> FULL transitions from ever ending up in
 	 * the ZERO state, which is invalid.
 	 */
-	static_call_update(cond_resched, __cond_resched);
-	static_call_update(might_resched, __cond_resched);
-	static_call_update(preempt_schedule, __preempt_schedule_func);
-	static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
-	static_call_update(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
+	static_call_enable(cond_resched);
+	static_call_enable(might_resched);
+	static_call_enable(preempt_schedule);
+	static_call_enable(preempt_schedule_notrace);
+	static_call_enable(irqentry_exit_cond_resched);
 
 	switch (mode) {
 	case preempt_dynamic_none:
-		static_call_update(cond_resched, __cond_resched);
-		static_call_update(might_resched, (void *)&__static_call_return0);
-		static_call_update(preempt_schedule, NULL);
-		static_call_update(preempt_schedule_notrace, NULL);
-		static_call_update(irqentry_exit_cond_resched, NULL);
+		static_call_enable(cond_resched);
+		static_call_disable(might_resched);
+		static_call_disable(preempt_schedule);
+		static_call_disable(preempt_schedule_notrace);
+		static_call_disable(irqentry_exit_cond_resched);
 		pr_info("Dynamic Preempt: none\n");
 		break;
 
 	case preempt_dynamic_voluntary:
-		static_call_update(cond_resched, __cond_resched);
-		static_call_update(might_resched, __cond_resched);
-		static_call_update(preempt_schedule, NULL);
-		static_call_update(preempt_schedule_notrace, NULL);
-		static_call_update(irqentry_exit_cond_resched, NULL);
+		static_call_enable(cond_resched);
+		static_call_enable(might_resched);
+		static_call_disable(preempt_schedule);
+		static_call_disable(preempt_schedule_notrace);
+		static_call_disable(irqentry_exit_cond_resched);
 		pr_info("Dynamic Preempt: voluntary\n");
 		break;
 
 	case preempt_dynamic_full:
-		static_call_update(cond_resched, (void *)&__static_call_return0);
-		static_call_update(might_resched, (void *)&__static_call_return0);
-		static_call_update(preempt_schedule, __preempt_schedule_func);
-		static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
-		static_call_update(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
+		static_call_disable(cond_resched);
+		static_call_disable(might_resched);
+		static_call_enable(preempt_schedule);
+		static_call_enable(preempt_schedule_notrace);
+		static_call_enable(irqentry_exit_cond_resched);
 		pr_info("Dynamic Preempt: full\n");
 		break;
 	}

Re: [PATCH 2/6] sched/preempt: refactor sched_dynamic_update()

[Mark Rutland]

Hi,

I'm looking at what I need to do to rebase/repost this atop v5.17-rc2, and I
realised I need your S-o-B to take your suggestion below.

On Fri, Dec 10, 2021 at 04:13:43PM +0100, Frederic Weisbecker wrote:
> On Tue, Nov 09, 2021 at 05:24:04PM +0000, Mark Rutland wrote:
> > Currently sched_dynamic_update needs to open-code the enabled/disabled
> > function names for each preemption model it supoprts, when in practice
> > this is a boolean enabled/disabled state for each function.
> > 
> > Make this clearer and avoid repetition by defining the enabled/disabled
> > states at the function definition, and using helper macros to peform the
> > static_call_update(). Where x86 currently overrides the enabled
> > function, it is made to provide both the enabled and disabled states for
> > consistency, with defaults provided by the core code otherwise.

> > -#define __preempt_schedule_notrace_func preempt_schedule_notrace_thunk
> > +#define preempt_schedule_notrace_dynamic_enabled	preempt_schedule_notrace_thunk
> > +#define preempt_schedule_notrace_dynamic_disabled	NULL
> 
> I'm worried about un-greppable macro definitions like this.
I assume you mean that it's hard to go from:

  preempt_dynamic_enable(preempt_schedule_notrace);

... to this, because the `_dynamic_enabled` or `_dynamic_disabled` part gets
token-pasted on?

The above will show up in a grep for `preempt_schedule_notrace`, but I agree
it's not necessarily ideal, especially if grepping for an exact match.

> Also this enable/disable switch look like a common pattern on static call so
> how about moving that logic to static call itself? As in below (only
> build-tested):

Sure; if others also prefer that I'm more than happy to build atop.

Can I have your Signed-off-by for that, or can you post that as its own patch?

Thanks,
Mark.

> 
> diff --git a/arch/x86/include/asm/preempt.h b/arch/x86/include/asm/preempt.h
> index fe5efbcba824..64320c0a00a6 100644
> --- a/arch/x86/include/asm/preempt.h
> +++ b/arch/x86/include/asm/preempt.h
> @@ -117,7 +117,7 @@ extern asmlinkage void preempt_schedule_notrace_thunk(void);
>  
>  #ifdef CONFIG_PREEMPT_DYNAMIC
>  
> -DECLARE_STATIC_CALL(preempt_schedule, __preempt_schedule_func);
> +DECLARE_STATIC_CALL_TOGGLE(preempt_schedule, __preempt_schedule_func);
>  
>  #define __preempt_schedule() \
>  do { \
> @@ -125,7 +125,7 @@ do { \
>  	asm volatile ("call " STATIC_CALL_TRAMP_STR(preempt_schedule) : ASM_CALL_CONSTRAINT); \
>  } while (0)
>  
> -DECLARE_STATIC_CALL(preempt_schedule_notrace, __preempt_schedule_notrace_func);
> +DECLARE_STATIC_CALL_TOGGLE(preempt_schedule_notrace, __preempt_schedule_notrace_func);
>  
>  #define __preempt_schedule_notrace() \
>  do { \
> diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h
> index 2e2b8d6140ed..a3d99ffc32ca 100644
> --- a/include/linux/entry-common.h
> +++ b/include/linux/entry-common.h
> @@ -456,7 +456,7 @@ irqentry_state_t noinstr irqentry_enter(struct pt_regs *regs);
>   */
>  void irqentry_exit_cond_resched(void);
>  #ifdef CONFIG_PREEMPT_DYNAMIC
> -DECLARE_STATIC_CALL(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
> +DECLARE_STATIC_CALL_TOGGLE(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
>  #endif
>  
>  /**
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 77755ac3e189..f4f5f4f3b496 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -94,7 +94,7 @@ extern int __cond_resched(void);
>  
>  extern int __cond_resched(void);
>  
> -DECLARE_STATIC_CALL(might_resched, __cond_resched);
> +DECLARE_STATIC_CALL_TOGGLE(might_resched, __cond_resched);
>  
>  static __always_inline void might_resched(void)
>  {
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 78c351e35fec..01a9e8590ea0 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -2010,7 +2010,7 @@ extern int __cond_resched(void);
>  
>  #ifdef CONFIG_PREEMPT_DYNAMIC
>  
> -DECLARE_STATIC_CALL(cond_resched, __cond_resched);
> +DECLARE_STATIC_CALL_TOGGLE(cond_resched, __cond_resched);
>  
>  static __always_inline int _cond_resched(void)
>  {
> diff --git a/include/linux/static_call.h b/include/linux/static_call.h
> index 3e56a9751c06..351ac27fa9d1 100644
> --- a/include/linux/static_call.h
> +++ b/include/linux/static_call.h
> @@ -156,6 +156,19 @@ extern void arch_static_call_transform(void *site, void *tramp, void *func, bool
>  			     STATIC_CALL_TRAMP_ADDR(name), __F);	\
>  })
>  
> +#define static_call_toggle(name, enable)				\
> +({									\
> +	struct static_call_toggle *__G = &STATIC_CALL_TOGGLE(name);	\
> +	if (enable)							\
> +		static_call_update(name, __G->func_enabled);		\
> +	else								\
> +		static_call_update(name, __G->func_disabled);		\
> +})
> +
> +#define static_call_enable(name) static_call_toggle(name, 1);
> +#define static_call_disable(name) static_call_toggle(name, 0);
> +
> +
>  #define static_call_query(name) (READ_ONCE(STATIC_CALL_KEY(name).func))
>  
>  #ifdef CONFIG_HAVE_STATIC_CALL_INLINE
> @@ -333,4 +346,27 @@ static inline int static_call_text_reserved(void *start, void *end)
>  #define DEFINE_STATIC_CALL_RET0(name, _func)				\
>  	__DEFINE_STATIC_CALL(name, _func, __static_call_return0)
>  
> +#define __DEFINE_STATIC_CALL_TOGGLE(name, _func_enabled, _func_disabled) \
> +	struct static_call_toggle STATIC_CALL_TOGGLE(name) = {			\
> +		.key = &STATIC_CALL_KEY(name),					\
> +		.func_enabled = _func_enabled,					\
> +		.func_disabled = _func_disabled,				\
> +	}
> +
> +#define DEFINE_STATIC_CALL_ENABLED(name, _func)					\
> +	DEFINE_STATIC_CALL(name, _func);					\
> +	__DEFINE_STATIC_CALL_TOGGLE(name, _func, NULL)
> +
> +#define DEFINE_STATIC_CALL_DISABLED(name, _func)				\
> +	DEFINE_STATIC_CALL_NULL(name, _func);					\
> +	__DEFINE_STATIC_CALL_TOGGLE(name, _func, NULL)
> +
> +#define DEFINE_STATIC_CALL_ENABLED_RET0(name, _func)				\
> +	DEFINE_STATIC_CALL(name, _func);					\
> +	__DEFINE_STATIC_CALL_TOGGLE(name, _func, __static_call_return0)
> +
> +#define DEFINE_STATIC_CALL_DISABLED_RET0(name, _func)				\
> +	DEFINE_STATIC_CALL_RET0(name, _func);					\
> +	__DEFINE_STATIC_CALL_TOGGLE(name, _func, __static_call_return0)
> +
>  #endif /* _LINUX_STATIC_CALL_H */
> diff --git a/include/linux/static_call_types.h b/include/linux/static_call_types.h
> index 5a00b8b2cf9f..e150f29aee2d 100644
> --- a/include/linux/static_call_types.h
> +++ b/include/linux/static_call_types.h
> @@ -18,6 +18,9 @@
>  #define STATIC_CALL_TRAMP(name)		__PASTE(STATIC_CALL_TRAMP_PREFIX, name)
>  #define STATIC_CALL_TRAMP_STR(name)	__stringify(STATIC_CALL_TRAMP(name))
>  
> +#define STATIC_CALL_TOGGLE_PREFIX	__SCG_
> +#define STATIC_CALL_TOGGLE(name)	__PASTE(STATIC_CALL_TOGGLE_PREFIX, name)
> +
>  /*
>   * Flags in the low bits of static_call_site::key.
>   */
> @@ -38,6 +41,10 @@ struct static_call_site {
>  	extern struct static_call_key STATIC_CALL_KEY(name);		\
>  	extern typeof(func) STATIC_CALL_TRAMP(name);
>  
> +#define DECLARE_STATIC_CALL_TOGGLE(name, func)				\
> +	DECLARE_STATIC_CALL(name, func);				\
> +	extern struct static_call_toggle STATIC_CALL_TOGGLE(name);
> +
>  #ifdef CONFIG_HAVE_STATIC_CALL
>  
>  #define __raw_static_call(name)	(&STATIC_CALL_TRAMP(name))
> @@ -68,6 +75,13 @@ struct static_call_key {
>  	};
>  };
>  
> +struct static_call_toggle {
> +	struct static_call_key *key;
> +	void *func_enabled;
> +	void *func_disabled;
> +};
> +
> +
>  #else /* !CONFIG_HAVE_STATIC_CALL_INLINE */
>  
>  #define __STATIC_CALL_ADDRESSABLE(name)
> diff --git a/kernel/entry/common.c b/kernel/entry/common.c
> index d5a61d565ad5..d48b91579475 100644
> --- a/kernel/entry/common.c
> +++ b/kernel/entry/common.c
> @@ -392,7 +392,7 @@ void irqentry_exit_cond_resched(void)
>  	}
>  }
>  #ifdef CONFIG_PREEMPT_DYNAMIC
> -DEFINE_STATIC_CALL(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
> +DEFINE_STATIC_CALL_ENABLED(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
>  #endif
>  
>  noinstr void irqentry_exit(struct pt_regs *regs, irqentry_state_t state)
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 27c795fbcaaf..389fcc412e6b 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -6447,7 +6447,7 @@ NOKPROBE_SYMBOL(preempt_schedule);
>  EXPORT_SYMBOL(preempt_schedule);
>  
>  #ifdef CONFIG_PREEMPT_DYNAMIC
> -DEFINE_STATIC_CALL(preempt_schedule, __preempt_schedule_func);
> +DEFINE_STATIC_CALL_ENABLED(preempt_schedule, __preempt_schedule_func);
>  EXPORT_STATIC_CALL_TRAMP(preempt_schedule);
>  #endif
>  
> @@ -6505,7 +6505,7 @@ asmlinkage __visible void __sched notrace preempt_schedule_notrace(void)
>  EXPORT_SYMBOL_GPL(preempt_schedule_notrace);
>  
>  #ifdef CONFIG_PREEMPT_DYNAMIC
> -DEFINE_STATIC_CALL(preempt_schedule_notrace, __preempt_schedule_notrace_func);
> +DEFINE_STATIC_CALL_ENABLED(preempt_schedule_notrace, __preempt_schedule_notrace_func);
>  EXPORT_STATIC_CALL_TRAMP(preempt_schedule_notrace);
>  #endif
>  
> @@ -8016,10 +8016,9 @@ EXPORT_SYMBOL(__cond_resched);
>  #endif
>  
>  #ifdef CONFIG_PREEMPT_DYNAMIC
> -DEFINE_STATIC_CALL_RET0(cond_resched, __cond_resched);
> +DEFINE_STATIC_CALL_DISABLED_RET0(cond_resched, __cond_resched);
>  EXPORT_STATIC_CALL_TRAMP(cond_resched);
> -
> -DEFINE_STATIC_CALL_RET0(might_resched, __cond_resched);
> +DEFINE_STATIC_CALL_DISABLED_RET0(might_resched, __cond_resched);
>  EXPORT_STATIC_CALL_TRAMP(might_resched);
>  #endif
>  
> @@ -8154,37 +8153,37 @@ void sched_dynamic_update(int mode)
>  	 * Avoid {NONE,VOLUNTARY} -> FULL transitions from ever ending up in
>  	 * the ZERO state, which is invalid.
>  	 */
> -	static_call_update(cond_resched, __cond_resched);
> -	static_call_update(might_resched, __cond_resched);
> -	static_call_update(preempt_schedule, __preempt_schedule_func);
> -	static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
> -	static_call_update(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
> +	static_call_enable(cond_resched);
> +	static_call_enable(might_resched);
> +	static_call_enable(preempt_schedule);
> +	static_call_enable(preempt_schedule_notrace);
> +	static_call_enable(irqentry_exit_cond_resched);
>  
>  	switch (mode) {
>  	case preempt_dynamic_none:
> -		static_call_update(cond_resched, __cond_resched);
> -		static_call_update(might_resched, (void *)&__static_call_return0);
> -		static_call_update(preempt_schedule, NULL);
> -		static_call_update(preempt_schedule_notrace, NULL);
> -		static_call_update(irqentry_exit_cond_resched, NULL);
> +		static_call_enable(cond_resched);
> +		static_call_disable(might_resched);
> +		static_call_disable(preempt_schedule);
> +		static_call_disable(preempt_schedule_notrace);
> +		static_call_disable(irqentry_exit_cond_resched);
>  		pr_info("Dynamic Preempt: none\n");
>  		break;
>  
>  	case preempt_dynamic_voluntary:
> -		static_call_update(cond_resched, __cond_resched);
> -		static_call_update(might_resched, __cond_resched);
> -		static_call_update(preempt_schedule, NULL);
> -		static_call_update(preempt_schedule_notrace, NULL);
> -		static_call_update(irqentry_exit_cond_resched, NULL);
> +		static_call_enable(cond_resched);
> +		static_call_enable(might_resched);
> +		static_call_disable(preempt_schedule);
> +		static_call_disable(preempt_schedule_notrace);
> +		static_call_disable(irqentry_exit_cond_resched);
>  		pr_info("Dynamic Preempt: voluntary\n");
>  		break;
>  
>  	case preempt_dynamic_full:
> -		static_call_update(cond_resched, (void *)&__static_call_return0);
> -		static_call_update(might_resched, (void *)&__static_call_return0);
> -		static_call_update(preempt_schedule, __preempt_schedule_func);
> -		static_call_update(preempt_schedule_notrace, __preempt_schedule_notrace_func);
> -		static_call_update(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
> +		static_call_disable(cond_resched);
> +		static_call_disable(might_resched);
> +		static_call_enable(preempt_schedule);
> +		static_call_enable(preempt_schedule_notrace);
> +		static_call_enable(irqentry_exit_cond_resched);
>  		pr_info("Dynamic Preempt: full\n");
>  		break;
>  	}

Re: [PATCH 2/6] sched/preempt: refactor sched_dynamic_update()

[Frederic Weisbecker]

On Wed, Feb 02, 2022 at 03:13:57PM +0000, Mark Rutland wrote:
> Hi,
> 
> I'm looking at what I need to do to rebase/repost this atop v5.17-rc2, and I
> realised I need your S-o-B to take your suggestion below.
> 
> On Fri, Dec 10, 2021 at 04:13:43PM +0100, Frederic Weisbecker wrote:
> > On Tue, Nov 09, 2021 at 05:24:04PM +0000, Mark Rutland wrote:
> > > Currently sched_dynamic_update needs to open-code the enabled/disabled
> > > function names for each preemption model it supoprts, when in practice
> > > this is a boolean enabled/disabled state for each function.
> > > 
> > > Make this clearer and avoid repetition by defining the enabled/disabled
> > > states at the function definition, and using helper macros to peform the
> > > static_call_update(). Where x86 currently overrides the enabled
> > > function, it is made to provide both the enabled and disabled states for
> > > consistency, with defaults provided by the core code otherwise.
> 
> > > -#define __preempt_schedule_notrace_func preempt_schedule_notrace_thunk
> > > +#define preempt_schedule_notrace_dynamic_enabled	preempt_schedule_notrace_thunk
> > > +#define preempt_schedule_notrace_dynamic_disabled	NULL
> > 
> > I'm worried about un-greppable macro definitions like this.
> I assume you mean that it's hard to go from:
> 
>   preempt_dynamic_enable(preempt_schedule_notrace);
> 
> ... to this, because the `_dynamic_enabled` or `_dynamic_disabled` part gets
> token-pasted on?

Right.

> 
> The above will show up in a grep for `preempt_schedule_notrace`, but I agree
> it's not necessarily ideal, especially if grepping for an exact match.
> 
> > Also this enable/disable switch look like a common pattern on static call so
> > how about moving that logic to static call itself? As in below (only
> > build-tested):
> 
> Sure; if others also prefer that I'm more than happy to build atop.
> 
> Can I have your Signed-off-by for that, or can you post that as its own patch?

Sure, here is a better split and tested version here:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
	static_call/toggle

I was hoping to make a default backend based on static keys to implement these
toggeable static calls, but I had some issues on the way, although I can't
remember exactly which.

So eventually I don't know if this stuff will be useful for you....

Well, I guess this can still ease a wrapper like:

preempt_dynamic_enable(sym)
	---> CONFIG_STATIC_CALL=y? -----> static_call_enable(sym)
	else
	---> CONFIG_STATIC_KEY=y? -----> static_key_enable(sym)

Thanks.

Re: [PATCH 2/6] sched/preempt: refactor sched_dynamic_update()

[Mark Rutland]

On Wed, Feb 02, 2022 at 05:01:44PM +0100, Frederic Weisbecker wrote:
> On Wed, Feb 02, 2022 at 03:13:57PM +0000, Mark Rutland wrote:
> > Hi,
> > 
> > I'm looking at what I need to do to rebase/repost this atop v5.17-rc2, and I
> > realised I need your S-o-B to take your suggestion below.
> > 
> > On Fri, Dec 10, 2021 at 04:13:43PM +0100, Frederic Weisbecker wrote:
> > > On Tue, Nov 09, 2021 at 05:24:04PM +0000, Mark Rutland wrote:
> > > > Currently sched_dynamic_update needs to open-code the enabled/disabled
> > > > function names for each preemption model it supoprts, when in practice
> > > > this is a boolean enabled/disabled state for each function.
> > > > 
> > > > Make this clearer and avoid repetition by defining the enabled/disabled
> > > > states at the function definition, and using helper macros to peform the
> > > > static_call_update(). Where x86 currently overrides the enabled
> > > > function, it is made to provide both the enabled and disabled states for
> > > > consistency, with defaults provided by the core code otherwise.
> > 
> > > > -#define __preempt_schedule_notrace_func preempt_schedule_notrace_thunk
> > > > +#define preempt_schedule_notrace_dynamic_enabled	preempt_schedule_notrace_thunk
> > > > +#define preempt_schedule_notrace_dynamic_disabled	NULL
> > > 
> > > I'm worried about un-greppable macro definitions like this.
> > I assume you mean that it's hard to go from:
> > 
> >   preempt_dynamic_enable(preempt_schedule_notrace);
> > 
> > ... to this, because the `_dynamic_enabled` or `_dynamic_disabled` part gets
> > token-pasted on?
> 
> Right.

Looking at this some more, I'm probably going to need to do token-pasting at
some level no matter what we do, so how big of a concern is this? Searching
for 'foo_function' should also find 'foo_function_dynamic_enabled' and
'foo_function_dynamic_disabled', and searching for either of those will find
their original definition.

If others aren't concerned, could we just live with that for now?

> > The above will show up in a grep for `preempt_schedule_notrace`, but I agree
> > it's not necessarily ideal, especially if grepping for an exact match.
> > 
> > > Also this enable/disable switch look like a common pattern on static call so
> > > how about moving that logic to static call itself? As in below (only
> > > build-tested):
> > 
> > Sure; if others also prefer that I'm more than happy to build atop.
> > 
> > Can I have your Signed-off-by for that, or can you post that as its own patch?
> 
> Sure, here is a better split and tested version here:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> 	static_call/toggle

Thanks!

> I was hoping to make a default backend based on static keys to implement these
> toggeable static calls, but I had some issues on the way, although I can't
> remember exactly which.
> 
> So eventually I don't know if this stuff will be useful for you....

Having had a play with this, since you need to generate a wrapper for the
static_key case, you either need to match the prototype or have a generic
macro (and you likely end up back in token-pasting hell again anyhow).

So as above, how much does this matter for now?

> Well, I guess this can still ease a wrapper like:
> 
> preempt_dynamic_enable(sym)
> 	---> CONFIG_STATIC_CALL=y? -----> static_call_enable(sym)
> 	else
> 	---> CONFIG_STATIC_KEY=y? -----> static_key_enable(sym)

In this series I just define preempt_dynamic_enable() dependent on
CONFIG_STATIC_CALL or CONFIG_STATIC_KEY, which is functionally equivalent.

Thanks,
Mark.

Re: [PATCH 2/6] sched/preempt: refactor sched_dynamic_update()

[Frederic Weisbecker]

On Wed, Feb 02, 2022 at 06:08:27PM +0000, Mark Rutland wrote:
> On Wed, Feb 02, 2022 at 05:01:44PM +0100, Frederic Weisbecker wrote:
> > On Wed, Feb 02, 2022 at 03:13:57PM +0000, Mark Rutland wrote:
> > > > > +#define preempt_schedule_notrace_dynamic_enabled	preempt_schedule_notrace_thunk
> > > > > +#define preempt_schedule_notrace_dynamic_disabled	NULL
> > > > 
> > > > I'm worried about un-greppable macro definitions like this.
> > > I assume you mean that it's hard to go from:
> > > 
> > >   preempt_dynamic_enable(preempt_schedule_notrace);
> > > 
> > > ... to this, because the `_dynamic_enabled` or `_dynamic_disabled` part gets
> > > token-pasted on?
> > 
> > Right.
> 
> Looking at this some more, I'm probably going to need to do token-pasting at
> some level no matter what we do, so how big of a concern is this? Searching
> for 'foo_function' should also find 'foo_function_dynamic_enabled' and
> 'foo_function_dynamic_disabled', and searching for either of those will find
> their original definition.
> 
> If others aren't concerned, could we just live with that for now?

Sure, I don't have a better idea right now. I'll try to think of something
after the next iteration.

> > I was hoping to make a default backend based on static keys to implement these
> > toggeable static calls, but I had some issues on the way, although I can't
> > remember exactly which.
> > 
> > So eventually I don't know if this stuff will be useful for you....
> 
> Having had a play with this, since you need to generate a wrapper for the
> static_key case, you either need to match the prototype or have a generic
> macro (and you likely end up back in token-pasting hell again anyhow).
> 
> So as above, how much does this matter for now?
> 
> > Well, I guess this can still ease a wrapper like:
> > 
> > preempt_dynamic_enable(sym)
> > 	---> CONFIG_STATIC_CALL=y? -----> static_call_enable(sym)
> > 	else
> > 	---> CONFIG_STATIC_KEY=y? -----> static_key_enable(sym)
> 
> In this series I just define preempt_dynamic_enable() dependent on
> CONFIG_STATIC_CALL or CONFIG_STATIC_KEY, which is functionally equivalent.

You're right.

It's just that instead of:

#if defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
 #ifndef preempt_schedule_notrace_dynamic_enabled
 #define preempt_schedule_notrace_dynamic_enabled       preempt_schedule_notrace
 #define preempt_schedule_notrace_dynamic_disabled      NULL
 #endif

#if defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
 #define preempt_dynamic_enable(f)      static_call_update(f, f##_dynamic_enabled)
 #define preempt_dynamic_disable(f)     static_call_update(f, #f##_dynamic_disabled)

You have:

DECLARE_STATIC_CALL_TOGGLE(preempt_schedule_notrace, __preempt_schedule_notrace_func);

#define preempt_dynamic_enable(f)      static_call_enable(f)
#define preempt_dynamic_disable(f)     static_call_disable(f)

Thanks.

[PATCH 3/6] sched/preempt: simplify irqentry_exit_cond_resched() callers

[Mark Rutland]

Currently callers of irqentry_exit_cond_resched() need to be aware of
whether the function should be indirected via a static call, leading to
ugly ifdeffery in callers.

Save them the hassle with a static inline wrapper that does the right
thing. The raw_irqentry_exit_cond_resched() will also be useful in
subsequent patches which will add conditional wrappers for preemption
functions.

Note: in arch/x86/entry/common.c, xen_pv_evtchn_do_upcall() always calls
irqentry_exit_cond_resched() directly, even when PREEMPT_DYNAMIC is in
use. I believe this is a latent bug (which this patch corrects), but I'm
not entirely certain this wasn't deliberate.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Frederic Weisbecker <frederic@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 include/linux/entry-common.h |  9 ++++++---
 kernel/entry/common.c        | 12 ++++--------
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h
index a01ac1a0a292..dfd84c59b144 100644
--- a/include/linux/entry-common.h
+++ b/include/linux/entry-common.h
@@ -454,11 +454,14 @@ irqentry_state_t noinstr irqentry_enter(struct pt_regs *regs);
  *
  * Conditional reschedule with additional sanity checks.
  */
-void irqentry_exit_cond_resched(void);
+void raw_irqentry_exit_cond_resched(void);
 #ifdef CONFIG_PREEMPT_DYNAMIC
-#define irqentry_exit_cond_resched_dynamic_enabled	irqentry_exit_cond_resched
+#define irqentry_exit_cond_resched_dynamic_enabled	raw_irqentry_exit_cond_resched
 #define irqentry_exit_cond_resched_dynamic_disabled	NULL
-DECLARE_STATIC_CALL(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
+DECLARE_STATIC_CALL(irqentry_exit_cond_resched, raw_irqentry_exit_cond_resched);
+#define irqentry_exit_cond_resched()	static_call(irqentry_exit_cond_resched)()
+#else
+#define irqentry_exit_cond_resched()	raw_irqentry_exit_cond_resched()
 #endif
 
 /**
diff --git a/kernel/entry/common.c b/kernel/entry/common.c
index d5a61d565ad5..3be0526e003d 100644
--- a/kernel/entry/common.c
+++ b/kernel/entry/common.c
@@ -380,7 +380,7 @@ noinstr irqentry_state_t irqentry_enter(struct pt_regs *regs)
 	return ret;
 }
 
-void irqentry_exit_cond_resched(void)
+void raw_irqentry_exit_cond_resched(void)
 {
 	if (!preempt_count()) {
 		/* Sanity check RCU and thread stack */
@@ -392,7 +392,7 @@ void irqentry_exit_cond_resched(void)
 	}
 }
 #ifdef CONFIG_PREEMPT_DYNAMIC
-DEFINE_STATIC_CALL(irqentry_exit_cond_resched, irqentry_exit_cond_resched);
+DEFINE_STATIC_CALL(irqentry_exit_cond_resched, raw_irqentry_exit_cond_resched);
 #endif
 
 noinstr void irqentry_exit(struct pt_regs *regs, irqentry_state_t state)
@@ -420,13 +420,9 @@ noinstr void irqentry_exit(struct pt_regs *regs, irqentry_state_t state)
 		}
 
 		instrumentation_begin();
-		if (IS_ENABLED(CONFIG_PREEMPTION)) {
-#ifdef CONFIG_PREEMPT_DYNAMIC
-			static_call(irqentry_exit_cond_resched)();
-#else
+		if (IS_ENABLED(CONFIG_PREEMPTION))
 			irqentry_exit_cond_resched();
-#endif
-		}
+
 		/* Covers both tracing and lockdep */
 		trace_hardirqs_on();
 		instrumentation_end();
-- 
2.11.0

[PATCH 4/6] sched/preempt: decouple HAVE_PREEMPT_DYNAMIC from GENERIC_ENTRY

[Mark Rutland]

Now that the enabled/disabled states for the preemption functions are
declared alongside their definitions, the core PREEMPT_DYNAMIC logic is
no longer tied to GENERIC_ENTRY, and can safely be selected so long as
an architecture provides enabled/disabled states for
irqentry_exit_cond_resched().

Make it possible to select HAVE_PREEMPT_DYNAMIC without GENERIC_ENTRY.

For existing users of HAVE_PREEMPT_DYNAMIC there should be no functional
change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Frederic Weisbecker <frederic@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 arch/Kconfig        | 1 -
 kernel/sched/core.c | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 26b8ed11639d..f3fb543d5da0 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1266,7 +1266,6 @@ config HAVE_STATIC_CALL_INLINE
 config HAVE_PREEMPT_DYNAMIC
 	bool
 	depends on HAVE_STATIC_CALL
-	depends on GENERIC_ENTRY
 	help
 	   Select this if the architecture support boot time preempt setting
 	   on top of static calls. It is strongly advised to support inline
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 3a1caa9a095a..ff28f4bd7192 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -8102,7 +8102,9 @@ EXPORT_SYMBOL(__cond_resched_rwlock_write);
 
 #ifdef CONFIG_PREEMPT_DYNAMIC
 
+#ifdef CONFIG_GENERIC_ENTRY
 #include <linux/entry-common.h>
+#endif
 
 /*
  * SC:cond_resched
-- 
2.11.0

[PATCH 5/6] sched/preempt: add PREEMPT_DYNAMIC using static keys

[Mark Rutland]

Where an architecture selects HAVE_STATIC_CALL but not
HAVE_STATIC_CALL_INLINE, each static call has an out-of-line trampoline
which will either branch to a callee or return to the caller.

On such architectures, a number of constraints can conspire to make
those trampolines more complicated and potentially less useful than we'd
like. For example:

* Hardware and software control flow integrity schemes can require the
  additition of "landing pad" instructions (e.g. `BTI` for arm64), which
  will also be present at the "real" callee.

* Limited branch ranges can require that trampolines generate or load an
  address into a registter and perform an indirect brach (or at least
  have a slow path that does so). This loses some of the benefits of
  having a direct branch.

* Interaction with SW CFI schemes can be complicated and fragile, e.g.
  requiring that we can recognise idiomatic codegen and remove
  indirections understand, at least until clang proves more helpful
  mechanisms for dealing with this.

For PREEMPT_DYNAMIC, we don't need the full power of static calls, as we
really only need to enable/disable specific preemption functions. We can
achieve the same effect without a number of the pain points above by
using static keys to fold early return cases into the preemption
functions themselves rather than in an out-of-line trampoline,
effectively inlining the trampoline into the start of the function.

For arm64, this results in good code generation, e.g. the
dynamic_cond_resched() wrapper looks as follows (with the first `B` being
replaced with a `NOP` when the function is disabled):

| <dynamic_cond_resched>:
|        bti     c
|        b       <dynamic_cond_resched+0x10>
|        mov     w0, #0x0                        // #0
|        ret
|        mrs     x0, sp_el0
|        ldr     x0, [x0, #8]
|        cbnz    x0, <dynamic_cond_resched+0x8>
|        paciasp
|        stp     x29, x30, [sp, #-16]!
|        mov     x29, sp
|        bl      <preempt_schedule_common>
|        mov     w0, #0x1                        // #1
|        ldp     x29, x30, [sp], #16
|        autiasp
|        ret

... compared to the regular form of the function:

| <__cond_resched>:
|        bti     c
|        mrs     x0, sp_el0
|        ldr     x1, [x0, #8]
|        cbz     x1, <__cond_resched+0x18>
|        mov     w0, #0x0                        // #0
|        ret
|        paciasp
|        stp     x29, x30, [sp, #-16]!
|        mov     x29, sp
|        bl      <preempt_schedule_common>
|        mov     w0, #0x1                        // #1
|        ldp     x29, x30, [sp], #16
|        autiasp
|        ret

Any architecture which implements static keys should be able to use this
to implement PREEMPT_DYNAMIC with similar cost to non-inlined static
calls.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Frederic Weisbecker <frederic@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 arch/Kconfig                 | 13 +++++++++++
 arch/x86/Kconfig             |  2 +-
 include/linux/entry-common.h | 10 +++++++--
 include/linux/kernel.h       |  7 +++++-
 include/linux/sched.h        | 10 ++++++++-
 kernel/entry/common.c        | 10 +++++++++
 kernel/sched/core.c          | 53 ++++++++++++++++++++++++++++++++++++++++++--
 7 files changed, 98 insertions(+), 7 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index f3fb543d5da0..907f2e09512c 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1265,12 +1265,25 @@ config HAVE_STATIC_CALL_INLINE
 
 config HAVE_PREEMPT_DYNAMIC
 	bool
+
+config HAVE_PREEMPT_DYNAMIC_CALL
+	bool
 	depends on HAVE_STATIC_CALL
+	select HAVE_PREEMPT_DYNAMIC
 	help
 	   Select this if the architecture support boot time preempt setting
 	   on top of static calls. It is strongly advised to support inline
 	   static call to avoid any overhead.
 
+config HAVE_PREEMPT_DYNAMIC_KEY
+	bool
+	depends on JUMP_LABEL
+	select HAVE_PREEMPT_DYNAMIC
+	help
+	   Select this if the architecture support boot time preempt setting
+	   on top of static keys. This will likely have more overhead than
+	   using static calls.
+
 config ARCH_WANT_LD_ORPHAN_WARN
 	bool
 	help
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 95dd1ee01546..750394198968 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -244,7 +244,7 @@ config X86
 	select HAVE_STACK_VALIDATION		if X86_64
 	select HAVE_STATIC_CALL
 	select HAVE_STATIC_CALL_INLINE		if HAVE_STACK_VALIDATION
-	select HAVE_PREEMPT_DYNAMIC
+	select HAVE_PREEMPT_DYNAMIC_CALL
 	select HAVE_RSEQ
 	select HAVE_SYSCALL_TRACEPOINTS
 	select HAVE_UNSTABLE_SCHED_CLOCK
diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h
index dfd84c59b144..141952f4fee8 100644
--- a/include/linux/entry-common.h
+++ b/include/linux/entry-common.h
@@ -456,13 +456,19 @@ irqentry_state_t noinstr irqentry_enter(struct pt_regs *regs);
  */
 void raw_irqentry_exit_cond_resched(void);
 #ifdef CONFIG_PREEMPT_DYNAMIC
+#if defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
 #define irqentry_exit_cond_resched_dynamic_enabled	raw_irqentry_exit_cond_resched
 #define irqentry_exit_cond_resched_dynamic_disabled	NULL
 DECLARE_STATIC_CALL(irqentry_exit_cond_resched, raw_irqentry_exit_cond_resched);
 #define irqentry_exit_cond_resched()	static_call(irqentry_exit_cond_resched)()
-#else
-#define irqentry_exit_cond_resched()	raw_irqentry_exit_cond_resched()
+#elif defined(CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)
+DECLARE_STATIC_KEY_TRUE(sk_dynamic_irqentry_exit_cond_resched);
+void dynamic_irqentry_exit_cond_resched(void);
+#define irqentry_exit_cond_resched()	dynamic_irqentry_exit_cond_resched()
 #endif
+#else /* CONFIG_PREEMPT_DYNAMIC */
+#define irqentry_exit_cond_resched()	raw_irqentry_exit_cond_resched()
+#endif /* CONFIG_PREEMPT_DYNAMIC */
 
 /**
  * irqentry_exit - Handle return from exception that used irqentry_enter()
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index e5359b09de1d..8a94ccfc7dc8 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -93,7 +93,7 @@ struct user;
 extern int __cond_resched(void);
 # define might_resched() __cond_resched()
 
-#elif defined(CONFIG_PREEMPT_DYNAMIC)
+#elif defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
 
 extern int __cond_resched(void);
 
@@ -104,6 +104,11 @@ static __always_inline void might_resched(void)
 	static_call_mod(might_resched)();
 }
 
+#elif defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)
+
+extern int dynamic_might_resched(void);
+# define might_resched() dynamic_might_resched()
+
 #else
 
 # define might_resched() do { } while (0)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 78c351e35fec..7710b6593c72 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2008,7 +2008,7 @@ static inline int test_tsk_need_resched(struct task_struct *tsk)
 #if !defined(CONFIG_PREEMPTION) || defined(CONFIG_PREEMPT_DYNAMIC)
 extern int __cond_resched(void);
 
-#ifdef CONFIG_PREEMPT_DYNAMIC
+#if defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
 
 DECLARE_STATIC_CALL(cond_resched, __cond_resched);
 
@@ -2017,6 +2017,14 @@ static __always_inline int _cond_resched(void)
 	return static_call_mod(cond_resched)();
 }
 
+#elif defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)
+extern int dynamic_cond_resched(void);
+
+static __always_inline int _cond_resched(void)
+{
+	return dynamic_cond_resched();
+}
+
 #else
 
 static inline int _cond_resched(void)
diff --git a/kernel/entry/common.c b/kernel/entry/common.c
index 3be0526e003d..294b5ce2e198 100644
--- a/kernel/entry/common.c
+++ b/kernel/entry/common.c
@@ -392,7 +392,17 @@ void raw_irqentry_exit_cond_resched(void)
 	}
 }
 #ifdef CONFIG_PREEMPT_DYNAMIC
+#if defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
 DEFINE_STATIC_CALL(irqentry_exit_cond_resched, raw_irqentry_exit_cond_resched);
+#elif defined(CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)
+DEFINE_STATIC_KEY_TRUE(sk_dynamic_irqentry_exit_cond_resched);
+void dynamic_irqentry_exit_cond_resched(void)
+{
+	if (!static_key_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
+		return;
+	raw_irqentry_exit_cond_resched();
+}
+#endif
 #endif
 
 noinstr void irqentry_exit(struct pt_regs *regs, irqentry_state_t state)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index ff28f4bd7192..39a7df37657a 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6437,21 +6437,31 @@ asmlinkage __visible void __sched notrace preempt_schedule(void)
 	 */
 	if (likely(!preemptible()))
 		return;
-
 	preempt_schedule_common();
 }
 NOKPROBE_SYMBOL(preempt_schedule);
 EXPORT_SYMBOL(preempt_schedule);
 
 #ifdef CONFIG_PREEMPT_DYNAMIC
+#if defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
 #ifndef preempt_schedule_dynamic_enabled
 #define preempt_schedule_dynamic_enabled	preempt_schedule
 #define preempt_schedule_dynamic_disabled	NULL
 #endif
 DEFINE_STATIC_CALL(preempt_schedule, preempt_schedule_dynamic_enabled);
 EXPORT_STATIC_CALL_TRAMP(preempt_schedule);
+#elif defined(CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)
+static DEFINE_STATIC_KEY_TRUE(sk_dynamic_preempt_schedule);
+void __sched notrace dynamic_preempt_schedule(void)
+{
+	if (!static_branch_unlikely(&sk_dynamic_preempt_schedule))
+		return;
+	preempt_schedule();
+}
+NOKPROBE_SYMBOL(dynamic_preempt_schedule);
+EXPORT_SYMBOL(dynamic_preempt_schedule);
+#endif
 #endif
-
 
 /**
  * preempt_schedule_notrace - preempt_schedule called by tracing
@@ -6506,12 +6516,24 @@ asmlinkage __visible void __sched notrace preempt_schedule_notrace(void)
 EXPORT_SYMBOL_GPL(preempt_schedule_notrace);
 
 #ifdef CONFIG_PREEMPT_DYNAMIC
+#if defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
 #ifndef preempt_schedule_notrace_dynamic_enabled
 #define preempt_schedule_notrace_dynamic_enabled	preempt_schedule_notrace
 #define preempt_schedule_notrace_dynamic_disabled	NULL
 #endif
 DEFINE_STATIC_CALL(preempt_schedule_notrace, preempt_schedule_notrace_dynamic_enabled);
 EXPORT_STATIC_CALL_TRAMP(preempt_schedule_notrace);
+#elif defined(CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)
+static DEFINE_STATIC_KEY_TRUE(sk_dynamic_preempt_schedule_notrace);
+void __sched notrace dynamic_preempt_schedule_notrace(void)
+{
+	if (!static_branch_unlikely(&sk_dynamic_preempt_schedule_notrace))
+		return;
+	preempt_schedule_notrace();
+}
+NOKPROBE_SYMBOL(dynamic_preempt_schedule_notrace);
+EXPORT_SYMBOL(dynamic_preempt_schedule_notrace);
+#endif
 #endif
 
 #endif /* CONFIG_PREEMPTION */
@@ -8021,6 +8043,7 @@ EXPORT_SYMBOL(__cond_resched);
 #endif
 
 #ifdef CONFIG_PREEMPT_DYNAMIC
+#if defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
 #define cond_resched_dynamic_enabled	__cond_resched
 #define cond_resched_dynamic_disabled	((void *)&__static_call_return0)
 DEFINE_STATIC_CALL_RET0(cond_resched, __cond_resched);
@@ -8030,6 +8053,25 @@ EXPORT_STATIC_CALL_TRAMP(cond_resched);
 #define might_resched_dynamic_disabled	((void *)&__static_call_return0)
 DEFINE_STATIC_CALL_RET0(might_resched, __cond_resched);
 EXPORT_STATIC_CALL_TRAMP(might_resched);
+#elif defined(CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)
+static DEFINE_STATIC_KEY_TRUE(sk_dynamic_cond_resched);
+int __sched dynamic_cond_resched(void)
+{
+	if (!static_branch_unlikely(&sk_dynamic_cond_resched))
+		return 0;
+	return __cond_resched();
+}
+EXPORT_SYMBOL(dynamic_cond_resched);
+
+static DEFINE_STATIC_KEY_TRUE(sk_dynamic_might_resched);
+int __sched dynamic_might_resched(void)
+{
+	if (!static_branch_unlikely(&sk_dynamic_might_resched))
+		return 0;
+	return __cond_resched();
+}
+EXPORT_SYMBOL(dynamic_might_resched);
+#endif
 #endif
 
 /*
@@ -8159,8 +8201,15 @@ int sched_dynamic_mode(const char *str)
 	return -EINVAL;
 }
 
+#if defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
 #define preempt_dynamic_enable(f)	static_call_update(f, f##_dynamic_enabled)
 #define preempt_dynamic_disable(f)	static_call_update(f, f##_dynamic_disabled)
+#elif defined(CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)
+#define preempt_dynamic_enable(f)	static_key_enable(&sk_dynamic_##f.key)
+#define preempt_dynamic_disable(f)	static_key_disable(&sk_dynamic_##f.key)
+#else
+#error "Unsupported PREEMPT_DYNAMIC mechanism"
+#endif
 
 void sched_dynamic_update(int mode)
 {
-- 
2.11.0

Re: [PATCH 5/6] sched/preempt: add PREEMPT_DYNAMIC using static keys

[Frederic Weisbecker]

On Tue, Nov 09, 2021 at 05:24:07PM +0000, Mark Rutland wrote:
> Where an architecture selects HAVE_STATIC_CALL but not
> HAVE_STATIC_CALL_INLINE, each static call has an out-of-line trampoline
> which will either branch to a callee or return to the caller.
> 
> On such architectures, a number of constraints can conspire to make
> those trampolines more complicated and potentially less useful than we'd
> like. For example:
> 
> * Hardware and software control flow integrity schemes can require the
>   additition of "landing pad" instructions (e.g. `BTI` for arm64), which
>   will also be present at the "real" callee.
> 
> * Limited branch ranges can require that trampolines generate or load an
>   address into a registter and perform an indirect brach (or at least
>   have a slow path that does so). This loses some of the benefits of
>   having a direct branch.
> 
> * Interaction with SW CFI schemes can be complicated and fragile, e.g.
>   requiring that we can recognise idiomatic codegen and remove
>   indirections understand, at least until clang proves more helpful
>   mechanisms for dealing with this.
> 
> For PREEMPT_DYNAMIC, we don't need the full power of static calls, as we
> really only need to enable/disable specific preemption functions. We can
> achieve the same effect without a number of the pain points above by
> using static keys to fold early return cases into the preemption
> functions themselves rather than in an out-of-line trampoline,
> effectively inlining the trampoline into the start of the function.
> 
> For arm64, this results in good code generation, e.g. the
> dynamic_cond_resched() wrapper looks as follows (with the first `B` being
> replaced with a `NOP` when the function is disabled):
> 
> | <dynamic_cond_resched>:
> |        bti     c
> |        b       <dynamic_cond_resched+0x10>
> |        mov     w0, #0x0                        // #0
> |        ret
> |        mrs     x0, sp_el0
> |        ldr     x0, [x0, #8]
> |        cbnz    x0, <dynamic_cond_resched+0x8>
> |        paciasp
> |        stp     x29, x30, [sp, #-16]!
> |        mov     x29, sp
> |        bl      <preempt_schedule_common>
> |        mov     w0, #0x1                        // #1
> |        ldp     x29, x30, [sp], #16
> |        autiasp
> |        ret
> 
> ... compared to the regular form of the function:
> 
> | <__cond_resched>:
> |        bti     c
> |        mrs     x0, sp_el0
> |        ldr     x1, [x0, #8]
> |        cbz     x1, <__cond_resched+0x18>
> |        mov     w0, #0x0                        // #0
> |        ret
> |        paciasp
> |        stp     x29, x30, [sp, #-16]!
> |        mov     x29, sp
> |        bl      <preempt_schedule_common>
> |        mov     w0, #0x1                        // #1
> |        ldp     x29, x30, [sp], #16
> |        autiasp
> |        ret
> 
> Any architecture which implements static keys should be able to use this
> to implement PREEMPT_DYNAMIC with similar cost to non-inlined static
> calls.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Frederic Weisbecker <frederic@kernel.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Juri Lelli <juri.lelli@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>

Anyone has an opinion on that? Can we do better on the arm64 static call side
or should we resign ourself to using that static keys direction?

Also I assume that, sooner or later, arm64 will eventually need a static call
implementation....

Thanks.

Re: [PATCH 5/6] sched/preempt: add PREEMPT_DYNAMIC using static keys

[Mark Rutland]

On Mon, Dec 13, 2021 at 11:05:01PM +0100, Frederic Weisbecker wrote:
> On Tue, Nov 09, 2021 at 05:24:07PM +0000, Mark Rutland wrote:
> > Where an architecture selects HAVE_STATIC_CALL but not
> > HAVE_STATIC_CALL_INLINE, each static call has an out-of-line trampoline
> > which will either branch to a callee or return to the caller.
> > 
> > On such architectures, a number of constraints can conspire to make
> > those trampolines more complicated and potentially less useful than we'd
> > like. For example:
> > 
> > * Hardware and software control flow integrity schemes can require the
> >   additition of "landing pad" instructions (e.g. `BTI` for arm64), which
> >   will also be present at the "real" callee.
> > 
> > * Limited branch ranges can require that trampolines generate or load an
> >   address into a registter and perform an indirect brach (or at least
> >   have a slow path that does so). This loses some of the benefits of
> >   having a direct branch.
> > 
> > * Interaction with SW CFI schemes can be complicated and fragile, e.g.
> >   requiring that we can recognise idiomatic codegen and remove
> >   indirections understand, at least until clang proves more helpful
> >   mechanisms for dealing with this.
> > 
> > For PREEMPT_DYNAMIC, we don't need the full power of static calls, as we
> > really only need to enable/disable specific preemption functions. We can
> > achieve the same effect without a number of the pain points above by
> > using static keys to fold early return cases into the preemption
> > functions themselves rather than in an out-of-line trampoline,
> > effectively inlining the trampoline into the start of the function.
> > 
> > For arm64, this results in good code generation, e.g. the
> > dynamic_cond_resched() wrapper looks as follows (with the first `B` being
> > replaced with a `NOP` when the function is disabled):
> > 
> > | <dynamic_cond_resched>:
> > |        bti     c
> > |        b       <dynamic_cond_resched+0x10>
> > |        mov     w0, #0x0                        // #0
> > |        ret
> > |        mrs     x0, sp_el0
> > |        ldr     x0, [x0, #8]
> > |        cbnz    x0, <dynamic_cond_resched+0x8>
> > |        paciasp
> > |        stp     x29, x30, [sp, #-16]!
> > |        mov     x29, sp
> > |        bl      <preempt_schedule_common>
> > |        mov     w0, #0x1                        // #1
> > |        ldp     x29, x30, [sp], #16
> > |        autiasp
> > |        ret
> > 
> > ... compared to the regular form of the function:
> > 
> > | <__cond_resched>:
> > |        bti     c
> > |        mrs     x0, sp_el0
> > |        ldr     x1, [x0, #8]
> > |        cbz     x1, <__cond_resched+0x18>
> > |        mov     w0, #0x0                        // #0
> > |        ret
> > |        paciasp
> > |        stp     x29, x30, [sp, #-16]!
> > |        mov     x29, sp
> > |        bl      <preempt_schedule_common>
> > |        mov     w0, #0x1                        // #1
> > |        ldp     x29, x30, [sp], #16
> > |        autiasp
> > |        ret
> > 
> > Any architecture which implements static keys should be able to use this
> > to implement PREEMPT_DYNAMIC with similar cost to non-inlined static
> > calls.
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Ard Biesheuvel <ardb@kernel.org>
> > Cc: Frederic Weisbecker <frederic@kernel.org>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Juri Lelli <juri.lelli@redhat.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> 
> Anyone has an opinion on that? Can we do better on the arm64 static call side
> or should we resign ourself to using that static keys direction?

From speaking with other arm64 folk, I think we're agreed that this is
preferable to implementing static calls (especially givne the pain points with
interaction with CFI).

I don't think it's fair to say we're "resigning outselves" to using static keys
-- this is vastly simpler to implement and maintain the static call approach,
should perform no worse than the form of static call trampolines that we'd have
to implement for static calls, and makes it easier for architectures to enable
PREEMPT_DYNAMIC, so it seems like an all-round win.

> Also I assume that, sooner or later, arm64 will eventually need a static call
> implementation....

I really hope not, becuase the current design of static calls (with arbitrary
targets) is not a great fit for arm64.

The only other major use for static keys on the arm64 side is for tracing
hooks, and that's *purely* to avoid the overhead that the current clang CFI
scheme imposes for modules. For that I'd rather fix the CFI scheme, because
that also interacts poorly with static calls to begin with...

Thanks,
Mark.

Re: [PATCH 5/6] sched/preempt: add PREEMPT_DYNAMIC using static keys

[Peter Zijlstra]

On Mon, Dec 13, 2021 at 11:05:01PM +0100, Frederic Weisbecker wrote:

> Anyone has an opinion on that? Can we do better on the arm64 static call side
> or should we resign ourself to using that static keys direction?

I don't hate this thing.

Re: [PATCH 5/6] sched/preempt: add PREEMPT_DYNAMIC using static keys

[Frederic Weisbecker]

On Tue, Nov 09, 2021 at 05:24:07PM +0000, Mark Rutland wrote:
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index e5359b09de1d..8a94ccfc7dc8 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -93,7 +93,7 @@ struct user;
>  extern int __cond_resched(void);
>  # define might_resched() __cond_resched()
>  
> -#elif defined(CONFIG_PREEMPT_DYNAMIC)
> +#elif defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
>  
>  extern int __cond_resched(void);
>  
> @@ -104,6 +104,11 @@ static __always_inline void might_resched(void)
>  	static_call_mod(might_resched)();
>  }
>  
> +#elif defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)
> +
> +extern int dynamic_might_resched(void);
> +# define might_resched() dynamic_might_resched()
> +
>  #else
>  
>  # define might_resched() do { } while (0)
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 78c351e35fec..7710b6593c72 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -2008,7 +2008,7 @@ static inline int test_tsk_need_resched(struct task_struct *tsk)
>  #if !defined(CONFIG_PREEMPTION) || defined(CONFIG_PREEMPT_DYNAMIC)
>  extern int __cond_resched(void);
>  
> -#ifdef CONFIG_PREEMPT_DYNAMIC
> +#if defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
>  
>  DECLARE_STATIC_CALL(cond_resched, __cond_resched);
>  
> @@ -2017,6 +2017,14 @@ static __always_inline int _cond_resched(void)
>  	return static_call_mod(cond_resched)();
>  }
>  
> +#elif defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)
> +extern int dynamic_cond_resched(void);
> +
> +static __always_inline int _cond_resched(void)
> +{
> +	return dynamic_cond_resched();

So in the end this is creating an indirect call for every preemption entrypoint.
It seems to me that this loses the whole point of using static keys.

Is there something that prevents from using inlines or macros?

Thanks.

Re: [PATCH 5/6] sched/preempt: add PREEMPT_DYNAMIC using static keys

[Mark Rutland]

On Thu, Feb 03, 2022 at 12:21:45AM +0100, Frederic Weisbecker wrote:
> On Tue, Nov 09, 2021 at 05:24:07PM +0000, Mark Rutland wrote:
> > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> > index e5359b09de1d..8a94ccfc7dc8 100644
> > --- a/include/linux/kernel.h
> > +++ b/include/linux/kernel.h
> > @@ -93,7 +93,7 @@ struct user;
> >  extern int __cond_resched(void);
> >  # define might_resched() __cond_resched()
> >  
> > -#elif defined(CONFIG_PREEMPT_DYNAMIC)
> > +#elif defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
> >  
> >  extern int __cond_resched(void);
> >  
> > @@ -104,6 +104,11 @@ static __always_inline void might_resched(void)
> >  	static_call_mod(might_resched)();
> >  }
> >  
> > +#elif defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)
> > +
> > +extern int dynamic_might_resched(void);
> > +# define might_resched() dynamic_might_resched()
> > +
> >  #else
> >  
> >  # define might_resched() do { } while (0)
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index 78c351e35fec..7710b6593c72 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -2008,7 +2008,7 @@ static inline int test_tsk_need_resched(struct task_struct *tsk)
> >  #if !defined(CONFIG_PREEMPTION) || defined(CONFIG_PREEMPT_DYNAMIC)
> >  extern int __cond_resched(void);
> >  
> > -#ifdef CONFIG_PREEMPT_DYNAMIC
> > +#if defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
> >  
> >  DECLARE_STATIC_CALL(cond_resched, __cond_resched);
> >  
> > @@ -2017,6 +2017,14 @@ static __always_inline int _cond_resched(void)
> >  	return static_call_mod(cond_resched)();
> >  }
> >  
> > +#elif defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)
> > +extern int dynamic_cond_resched(void);
> > +
> > +static __always_inline int _cond_resched(void)
> > +{
> > +	return dynamic_cond_resched();
> 
> So in the end this is creating an indirect call for every preemption entrypoint.

Huh? "indirect call" usually means a branch to a function pointer, and I don't
think that's what you mean here. Do you just mean that we add a (direct)
call+return?

This gets inlined, and will be just a direct call to dynamic_cond_resched().
e,g. on arm64 this will be a single instruction:

	bl	dynamic_cond_resched

... and (as the commit message desribes) then the implementation of
dynamic_cond_resched will be the same as the regular __cond_resched *but* the
static key trampoline is inlined at the start, e.g.

| <dynamic_cond_resched>:
|        bti     c
|        b       <dynamic_cond_resched+0x10>
|        mov     w0, #0x0                        // #0
|        ret
|        mrs     x0, sp_el0
|        ldr     x0, [x0, #8]
|        cbnz    x0, <dynamic_cond_resched+0x8>
|        paciasp
|        stp     x29, x30, [sp, #-16]!
|        mov     x29, sp
|        bl      <preempt_schedule_common>
|        mov     w0, #0x1                        // #1
|        ldp     x29, x30, [sp], #16
|        autiasp
|        ret

... compared to the regular form of the function:

| <__cond_resched>:
|        bti     c
|        mrs     x0, sp_el0
|        ldr     x1, [x0, #8]
|        cbz     x1, <__cond_resched+0x18>
|        mov     w0, #0x0                        // #0
|        ret
|        paciasp
|        stp     x29, x30, [sp, #-16]!
|        mov     x29, sp
|        bl      <preempt_schedule_common>
|        mov     w0, #0x1                        // #1
|        ldp     x29, x30, [sp], #16
|        autiasp
|        ret

> It seems to me that this loses the whole point of using static keys.

As above, I don't think that's the case. Relative to static keys using
trampolines (which is all arm64 can implement), the gain is that we inline the
trampoline into the *callee*. That saves on I-cache footprint, the compiler can
generate the early returns more optimally (and compatibly with an CFI scheme we
wish to use), and we don't have to maintain a separate patching mechanism.

If you think that static call trampolines lose the whole point of static keys
then we've lost to begin with, since that's all we can reasonably implement.

> Is there something that prevents from using inlines or macros?

Inlining of *what* ?

Thanks,
Mark.

Re: [PATCH 5/6] sched/preempt: add PREEMPT_DYNAMIC using static keys

[Frederic Weisbecker]

On Thu, Feb 03, 2022 at 09:51:46AM +0000, Mark Rutland wrote:
> On Thu, Feb 03, 2022 at 12:21:45AM +0100, Frederic Weisbecker wrote:
> > > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > > index 78c351e35fec..7710b6593c72 100644
> > > --- a/include/linux/sched.h
> > > +++ b/include/linux/sched.h
> > > @@ -2008,7 +2008,7 @@ static inline int test_tsk_need_resched(struct task_struct *tsk)
> > >  #if !defined(CONFIG_PREEMPTION) || defined(CONFIG_PREEMPT_DYNAMIC)
> > >  extern int __cond_resched(void);
> > >  
> > > -#ifdef CONFIG_PREEMPT_DYNAMIC
> > > +#if defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
> > >  
> > >  DECLARE_STATIC_CALL(cond_resched, __cond_resched);
> > >  
> > > @@ -2017,6 +2017,14 @@ static __always_inline int _cond_resched(void)
> > >  	return static_call_mod(cond_resched)();
> > >  }
> > >  
> > > +#elif defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)
> > > +extern int dynamic_cond_resched(void);
> > > +
> > > +static __always_inline int _cond_resched(void)
> > > +{
> > > +	return dynamic_cond_resched();
> > 
> > So in the end this is creating an indirect call for every preemption entrypoint.
> 
> Huh? "indirect call" usually means a branch to a function pointer, and I don't
> think that's what you mean here. Do you just mean that we add a (direct)
> call+return?

Right, basic terminology and me...

> 
> This gets inlined, and will be just a direct call to dynamic_cond_resched().
> e,g. on arm64 this will be a single instruction:
> 
> 	bl	dynamic_cond_resched
> 
> ... and (as the commit message desribes) then the implementation of
> dynamic_cond_resched will be the same as the regular __cond_resched *but* the
> static key trampoline is inlined at the start, e.g.
> 
> | <dynamic_cond_resched>:
> |        bti     c
> |        b       <dynamic_cond_resched+0x10>
> |        mov     w0, #0x0                        // #0
> |        ret
> |        mrs     x0, sp_el0
> |        ldr     x0, [x0, #8]
> |        cbnz    x0, <dynamic_cond_resched+0x8>
> |        paciasp
> |        stp     x29, x30, [sp, #-16]!
> |        mov     x29, sp
> |        bl      <preempt_schedule_common>
> |        mov     w0, #0x1                        // #1
> |        ldp     x29, x30, [sp], #16
> |        autiasp
> |        ret
> 
> ... compared to the regular form of the function:
> 
> | <__cond_resched>:
> |        bti     c
> |        mrs     x0, sp_el0
> |        ldr     x1, [x0, #8]
> |        cbz     x1, <__cond_resched+0x18>
> |        mov     w0, #0x0                        // #0
> |        ret
> |        paciasp
> |        stp     x29, x30, [sp, #-16]!
> |        mov     x29, sp
> |        bl      <preempt_schedule_common>
> |        mov     w0, #0x1                        // #1
> |        ldp     x29, x30, [sp], #16
> |        autiasp
> |        ret

Who reads changelogs anyway? ;-)

Ok I didn't know about that. Is this a guaranteed behaviour everywhere?
Perhaps put a big fat comment below HAVE_PREEMPT_DYNAMIC_KEY help to tell
about this expectation as I guess it depends on arch/compiler?

Thanks.

Re: [PATCH 5/6] sched/preempt: add PREEMPT_DYNAMIC using static keys

[Mark Rutland]

On Thu, Feb 03, 2022 at 12:34:53PM +0100, Frederic Weisbecker wrote:
> On Thu, Feb 03, 2022 at 09:51:46AM +0000, Mark Rutland wrote:
> > On Thu, Feb 03, 2022 at 12:21:45AM +0100, Frederic Weisbecker wrote:
> > > > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > > > index 78c351e35fec..7710b6593c72 100644
> > > > --- a/include/linux/sched.h
> > > > +++ b/include/linux/sched.h
> > > > @@ -2008,7 +2008,7 @@ static inline int test_tsk_need_resched(struct task_struct *tsk)
> > > >  #if !defined(CONFIG_PREEMPTION) || defined(CONFIG_PREEMPT_DYNAMIC)
> > > >  extern int __cond_resched(void);
> > > >  
> > > > -#ifdef CONFIG_PREEMPT_DYNAMIC
> > > > +#if defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
> > > >  
> > > >  DECLARE_STATIC_CALL(cond_resched, __cond_resched);
> > > >  
> > > > @@ -2017,6 +2017,14 @@ static __always_inline int _cond_resched(void)
> > > >  	return static_call_mod(cond_resched)();
> > > >  }
> > > >  
> > > > +#elif defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)
> > > > +extern int dynamic_cond_resched(void);
> > > > +
> > > > +static __always_inline int _cond_resched(void)
> > > > +{
> > > > +	return dynamic_cond_resched();
> > > 
> > > So in the end this is creating an indirect call for every preemption entrypoint.
> > 
> > Huh? "indirect call" usually means a branch to a function pointer, and I don't
> > think that's what you mean here. Do you just mean that we add a (direct)
> > call+return?
> 
> Right, basic terminology and me...

No problem; just wanted to make sure we were talking about the same thing! :)

> > This gets inlined, and will be just a direct call to dynamic_cond_resched().
> > e,g. on arm64 this will be a single instruction:
> > 
> > 	bl	dynamic_cond_resched
> > 
> > ... and (as the commit message desribes) then the implementation of
> > dynamic_cond_resched will be the same as the regular __cond_resched *but* the
> > static key trampoline is inlined at the start, e.g.
> > 
> > | <dynamic_cond_resched>:
> > |        bti     c
> > |        b       <dynamic_cond_resched+0x10>
> > |        mov     w0, #0x0                        // #0
> > |        ret
> > |        mrs     x0, sp_el0
> > |        ldr     x0, [x0, #8]
> > |        cbnz    x0, <dynamic_cond_resched+0x8>
> > |        paciasp
> > |        stp     x29, x30, [sp, #-16]!
> > |        mov     x29, sp
> > |        bl      <preempt_schedule_common>
> > |        mov     w0, #0x1                        // #1
> > |        ldp     x29, x30, [sp], #16
> > |        autiasp
> > |        ret
> > 
> > ... compared to the regular form of the function:
> > 
> > | <__cond_resched>:
> > |        bti     c
> > |        mrs     x0, sp_el0
> > |        ldr     x1, [x0, #8]
> > |        cbz     x1, <__cond_resched+0x18>
> > |        mov     w0, #0x0                        // #0
> > |        ret
> > |        paciasp
> > |        stp     x29, x30, [sp, #-16]!
> > |        mov     x29, sp
> > |        bl      <preempt_schedule_common>
> > |        mov     w0, #0x1                        // #1
> > |        ldp     x29, x30, [sp], #16
> > |        autiasp
> > |        ret
> 
> Who reads changelogs anyway? ;-)
> 
> Ok I didn't know about that. Is this a guaranteed behaviour everywhere?

For anyone with static keys based on jump labels it should look roughly as
above. The *precise* codegen will depend on a bunch of details, but the whole
point of jump labels and static keys is to permit codegen like this.

> Perhaps put a big fat comment below HAVE_PREEMPT_DYNAMIC_KEY help to tell
> about this expectation as I guess it depends on arch/compiler?

Sure; I'll come up with something for v2.

Thanks,
Mark.

[PATCH 6/6] arm64: support PREEMPT_DYNAMIC

[Mark Rutland]

This patch enables support for PREEMPT_DYNAMIC on arm64, using static
keys to disable the out-of-line forms of the preemption functions. This
side-steps a number of pain points highlighted with static calls, and I
expect the overhead to be similar (and hopefully lower in some cases)
than would be the case for non-inlined static calls, by virtue of
effectively inlining the trampolines into the callees.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Frederic Weisbecker <frederic@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/Kconfig               |  1 +
 arch/arm64/include/asm/preempt.h | 16 ++++++++++++++--
 arch/arm64/kernel/entry-common.c |  9 +++++++++
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index c4207cf9bb17..928b710465ce 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -191,6 +191,7 @@ config ARM64
 	select HAVE_PERF_EVENTS
 	select HAVE_PERF_REGS
 	select HAVE_PERF_USER_STACK_DUMP
+	select HAVE_PREEMPT_DYNAMIC_KEY
 	select HAVE_REGS_AND_STACK_ACCESS_API
 	select HAVE_POSIX_CPU_TIMERS_TASK_WORK
 	select HAVE_FUNCTION_ARG_ACCESS_API
diff --git a/arch/arm64/include/asm/preempt.h b/arch/arm64/include/asm/preempt.h
index e83f0982b99c..73623f719d7e 100644
--- a/arch/arm64/include/asm/preempt.h
+++ b/arch/arm64/include/asm/preempt.h
@@ -80,10 +80,22 @@ static inline bool should_resched(int preempt_offset)
 }
 
 #ifdef CONFIG_PREEMPTION
+#ifdef CONFIG_PREEMPT_DYNAMIC
+
+DECLARE_STATIC_KEY_TRUE(sk_dynamic_irqentry_exit_cond_resched);
+void dynamic_preempt_schedule(void);
+#define __preempt_schedule()		dynamic_preempt_schedule()
+void dynamic_preempt_schedule_notrace(void);
+#define __preempt_schedule_notrace()	dynamic_preempt_schedule_notrace()
+
+#else
+
 void preempt_schedule(void);
-#define __preempt_schedule() preempt_schedule()
+#define __preempt_schedule()		preempt_schedule()
 void preempt_schedule_notrace(void);
-#define __preempt_schedule_notrace() preempt_schedule_notrace()
+#define __preempt_schedule_notrace()	preempt_schedule_notrace()
+
+#endif
 #endif /* CONFIG_PREEMPTION */
 
 #endif /* __ASM_PREEMPT_H */
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index f7408edf8571..f096678dd580 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -220,10 +220,19 @@ static void noinstr arm64_exit_el1_dbg(struct pt_regs *regs)
 		lockdep_hardirqs_on(CALLER_ADDR0);
 }
 
+#ifdef CONFIG_PREEMPT_DYNAMIC
+DEFINE_STATIC_KEY_TRUE(sk_dynamic_irqentry_exit_cond_resched);
+#endif
+
 static void __sched arm64_preempt_schedule_irq(void)
 {
 	lockdep_assert_irqs_disabled();
 
+#ifdef CONFIG_PREEMPT_DYNAMIC
+	if (!static_branch_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
+		return;
+#endif
+
 	/*
 	 * DAIF.DA are cleared at the start of IRQ/FIQ handling, and when GIC
 	 * priority masking is used the GIC irqchip driver will clear DAIF.IF
-- 
2.11.0