diff mbox series

[v2] riscv: support PREEMPT_DYNAMIC with static keys

Message ID 20230716164925.1858-1-jszhang@kernel.org (mailing list archive)
State Accepted
Commit 15e062726f551e4acbe2739763d6e66562ec0a41
Headers show
Series [v2] riscv: support PREEMPT_DYNAMIC with static keys | expand

Checks

Context Check Description
conchuod/cover_letter success Single patches do not need cover letters
conchuod/tree_selection success Guessed tree name to be for-next at HEAD 471aba2e4760
conchuod/fixes_present success Fixes tag not required for -next series
conchuod/maintainers_pattern success MAINTAINERS pattern errors before the patch: 4 and now 4
conchuod/verify_signedoff success Signed-off-by tag matches author and committer
conchuod/kdoc success Errors and warnings before: 0 this patch: 0
conchuod/build_rv64_clang_allmodconfig success Errors and warnings before: 2775 this patch: 2773
conchuod/module_param success Was 0 now: 0
conchuod/build_rv64_gcc_allmodconfig success Errors and warnings before: 15778 this patch: 15770
conchuod/build_rv32_defconfig success Build OK
conchuod/dtb_warn_rv64 success Errors and warnings before: 3 this patch: 3
conchuod/header_inline success No static functions without inline keyword in header files
conchuod/checkpatch success total: 0 errors, 0 warnings, 0 checks, 29 lines checked
conchuod/build_rv64_nommu_k210_defconfig success Build OK
conchuod/verify_fixes success No Fixes tag
conchuod/build_rv64_nommu_virt_defconfig success Build OK

Commit Message

Jisheng Zhang July 16, 2023, 4:49 p.m. UTC
Currently, each architecture can support PREEMPT_DYNAMIC through
either static calls or static keys. To support PREEMPT_DYNAMIC on
riscv, we face three choices:

1. only add static calls support to riscv
As Mark pointed out in commit 99cf983cc8bc ("sched/preempt: Add
PREEMPT_DYNAMIC using static keys"), static keys "...should have
slightly lower overhead than non-inline static calls, as this
effectively inlines each trampoline into the start of its callee. This
may avoid redundant work, and may integrate better with CFI schemes."
So even we add static calls(without inline static calls) to riscv,
static keys is still a better choice.

2. add static calls and inline static calls to riscv
Per my understanding, inline static calls requires objtool support
which is not easy.

3. use static keys

While riscv doesn't have static calls support, it supports static keys
perfectly. So this patch selects HAVE_PREEMPT_DYNAMIC_KEY to enable
support for PREEMPT_DYNAMIC on riscv, so that the preemption model can
be chosen at boot time. It also patches asm-generic/preempt.h, mainly
to add __preempt_schedule() and __preempt_schedule_notrace() macros
for PREEMPT_DYNAMIC case. Other architectures which use generic
preempt.h can also benefit from this patch by simply selecting
HAVE_PREEMPT_DYNAMIC_KEY to enable PREEMPT_DYNAMIC if they supports
static keys.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
since v1:
 - keep Kconfig entries sorted
 - group asm-generic modifications under CONFIG_PREEMPT_DYNAMIC &&
   CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)

 arch/riscv/Kconfig            |  1 +
 include/asm-generic/preempt.h | 14 +++++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

Comments

Conor Dooley July 24, 2023, 9:59 a.m. UTC | #1
On Mon, Jul 17, 2023 at 12:49:25AM +0800, Jisheng Zhang wrote:
> Currently, each architecture can support PREEMPT_DYNAMIC through
> either static calls or static keys. To support PREEMPT_DYNAMIC on
> riscv, we face three choices:
> 
> 1. only add static calls support to riscv
> As Mark pointed out in commit 99cf983cc8bc ("sched/preempt: Add
> PREEMPT_DYNAMIC using static keys"), static keys "...should have
> slightly lower overhead than non-inline static calls, as this
> effectively inlines each trampoline into the start of its callee. This
> may avoid redundant work, and may integrate better with CFI schemes."
> So even we add static calls(without inline static calls) to riscv,
> static keys is still a better choice.
> 
> 2. add static calls and inline static calls to riscv
> Per my understanding, inline static calls requires objtool support
> which is not easy.
> 
> 3. use static keys
> 
> While riscv doesn't have static calls support, it supports static keys
> perfectly. So this patch selects HAVE_PREEMPT_DYNAMIC_KEY to enable
> support for PREEMPT_DYNAMIC on riscv, so that the preemption model can
> be chosen at boot time. It also patches asm-generic/preempt.h, mainly
> to add __preempt_schedule() and __preempt_schedule_notrace() macros
> for PREEMPT_DYNAMIC case. Other architectures which use generic
> preempt.h can also benefit from this patch by simply selecting
> HAVE_PREEMPT_DYNAMIC_KEY to enable PREEMPT_DYNAMIC if they supports
> static keys.
> 
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
> since v1:
>  - keep Kconfig entries sorted
>  - group asm-generic modifications under CONFIG_PREEMPT_DYNAMIC &&
>    CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)
> 
>  arch/riscv/Kconfig            |  1 +
>  include/asm-generic/preempt.h | 14 +++++++++++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 4c07b9189c86..686df6902947 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -130,6 +130,7 @@ config RISCV
>  	select HAVE_PERF_REGS
>  	select HAVE_PERF_USER_STACK_DUMP
>  	select HAVE_POSIX_CPU_TIMERS_TASK_WORK
> +	select HAVE_PREEMPT_DYNAMIC_KEY if !XIP_KERNEL

Had a go of this, and it seems fine to me, as do the asm-generic bits
seem fine from a single arch perspective.
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.
patchwork-bot+linux-riscv@kernel.org Aug. 30, 2023, 1:20 p.m. UTC | #2
Hello:

This patch was applied to riscv/linux.git (for-next)
by Palmer Dabbelt <palmer@rivosinc.com>:

On Mon, 17 Jul 2023 00:49:25 +0800 you wrote:
> Currently, each architecture can support PREEMPT_DYNAMIC through
> either static calls or static keys. To support PREEMPT_DYNAMIC on
> riscv, we face three choices:
> 
> 1. only add static calls support to riscv
> As Mark pointed out in commit 99cf983cc8bc ("sched/preempt: Add
> PREEMPT_DYNAMIC using static keys"), static keys "...should have
> slightly lower overhead than non-inline static calls, as this
> effectively inlines each trampoline into the start of its callee. This
> may avoid redundant work, and may integrate better with CFI schemes."
> So even we add static calls(without inline static calls) to riscv,
> static keys is still a better choice.
> 
> [...]

Here is the summary with links:
  - [v2] riscv: support PREEMPT_DYNAMIC with static keys
    https://git.kernel.org/riscv/c/15e062726f55

You are awesome, thank you!
diff mbox series

Patch

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 4c07b9189c86..686df6902947 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -130,6 +130,7 @@  config RISCV
 	select HAVE_PERF_REGS
 	select HAVE_PERF_USER_STACK_DUMP
 	select HAVE_POSIX_CPU_TIMERS_TASK_WORK
+	select HAVE_PREEMPT_DYNAMIC_KEY if !XIP_KERNEL
 	select HAVE_REGS_AND_STACK_ACCESS_API
 	select HAVE_RETHOOK if !XIP_KERNEL
 	select HAVE_RSEQ
diff --git a/include/asm-generic/preempt.h b/include/asm-generic/preempt.h
index b4d43a4af5f7..51f8f3881523 100644
--- a/include/asm-generic/preempt.h
+++ b/include/asm-generic/preempt.h
@@ -80,9 +80,21 @@  static __always_inline bool should_resched(int preempt_offset)
 
 #ifdef CONFIG_PREEMPTION
 extern asmlinkage void preempt_schedule(void);
-#define __preempt_schedule() preempt_schedule()
 extern asmlinkage void preempt_schedule_notrace(void);
+
+#if defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)
+
+void dynamic_preempt_schedule(void);
+void dynamic_preempt_schedule_notrace(void);
+#define __preempt_schedule()		dynamic_preempt_schedule()
+#define __preempt_schedule_notrace()	dynamic_preempt_schedule_notrace()
+
+#else /* !CONFIG_PREEMPT_DYNAMIC || !CONFIG_HAVE_PREEMPT_DYNAMIC_KEY*/
+
+#define __preempt_schedule() preempt_schedule()
 #define __preempt_schedule_notrace() preempt_schedule_notrace()
+
+#endif /* CONFIG_PREEMPT_DYNAMIC && CONFIG_HAVE_PREEMPT_DYNAMIC_KEY*/
 #endif /* CONFIG_PREEMPTION */
 
 #endif /* __ASM_PREEMPT_H */