ARM64 requires an additional check whether to reschedule on return
from interrupt. So add arch_irqentry_exit_need_resched() as the default
NOP implementation and hook it up into the need_resched() condition in
raw_irqentry_exit_cond_resched(). This allows ARM64 to implement
the architecture specific version for switching over to
the generic entry code.
To align the structure of the code with irqentry_exit_cond_resched()
from the generic entry code, hoist the need_irq_preemption()
and IS_ENABLED() check earlier. And different preemption check functions
are defined based on whether dynamic preemption is enabled.
Suggested-by: Mark Rutland <mark.rutland@arm.com>
Suggested-by: Kevin Brodsky <kevin.brodsky@arm.com>
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
arch/arm64/include/asm/preempt.h | 4 ++++
arch/arm64/kernel/entry-common.c | 35 ++++++++++++++++++--------------
kernel/entry/common.c | 16 ++++++++++++++-
3 files changed, 39 insertions(+), 16 deletions(-)
diff --git a/arch/arm64/include/asm/preempt.h b/arch/arm64/include/asm/preempt.h
index 0159b625cc7f..0f0ba250efe8 100644
--- a/arch/arm64/include/asm/preempt.h
+++ b/arch/arm64/include/asm/preempt.h
@@ -85,6 +85,7 @@ static inline bool should_resched(int preempt_offset)
void preempt_schedule(void);
void preempt_schedule_notrace(void);
+void raw_irqentry_exit_cond_resched(void);
#ifdef CONFIG_PREEMPT_DYNAMIC
DECLARE_STATIC_KEY_TRUE(sk_dynamic_irqentry_exit_cond_resched);
@@ -92,11 +93,14 @@ 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()
+void dynamic_irqentry_exit_cond_resched(void);
+#define irqentry_exit_cond_resched() dynamic_irqentry_exit_cond_resched()
#else /* CONFIG_PREEMPT_DYNAMIC */
#define __preempt_schedule() preempt_schedule()
#define __preempt_schedule_notrace() preempt_schedule_notrace()
+#define irqentry_exit_cond_resched() raw_irqentry_exit_cond_resched()
#endif /* CONFIG_PREEMPT_DYNAMIC */
#endif /* CONFIG_PREEMPTION */
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 7c2299c1ba79..4f92664fd46c 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -285,19 +285,8 @@ 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);
-#define need_irq_preemption() \
- (static_branch_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
-#else
-#define need_irq_preemption() (IS_ENABLED(CONFIG_PREEMPTION))
-#endif
-
static inline bool arm64_preempt_schedule_irq(void)
{
- if (!need_irq_preemption())
- return false;
-
/*
* 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
@@ -672,6 +661,24 @@ static __always_inline void __el1_pnmi(struct pt_regs *regs,
arm64_exit_nmi(regs, state);
}
+void raw_irqentry_exit_cond_resched(void)
+{
+ if (!preempt_count()) {
+ if (need_resched() && arm64_preempt_schedule_irq())
+ preempt_schedule_irq();
+ }
+}
+
+#ifdef CONFIG_PREEMPT_DYNAMIC
+DEFINE_STATIC_KEY_TRUE(sk_dynamic_irqentry_exit_cond_resched);
+void dynamic_irqentry_exit_cond_resched(void)
+{
+ if (!static_branch_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
+ return;
+ raw_irqentry_exit_cond_resched();
+}
+#endif
+
static __always_inline void __el1_irq(struct pt_regs *regs,
void (*handler)(struct pt_regs *))
{
@@ -681,10 +688,8 @@ static __always_inline void __el1_irq(struct pt_regs *regs,
do_interrupt_handler(regs, handler);
irq_exit_rcu();
- if (!preempt_count() && need_resched()) {
- if (arm64_preempt_schedule_irq())
- preempt_schedule_irq();
- }
+ if (IS_ENABLED(CONFIG_PREEMPTION))
+ irqentry_exit_cond_resched();
exit_to_kernel_mode(regs, state);
}
diff --git a/kernel/entry/common.c b/kernel/entry/common.c
index b82032777310..4aa9656fa1b4 100644
--- a/kernel/entry/common.c
+++ b/kernel/entry/common.c
@@ -142,6 +142,20 @@ noinstr irqentry_state_t irqentry_enter(struct pt_regs *regs)
return ret;
}
+/**
+ * arch_irqentry_exit_need_resched - Architecture specific need resched function
+ *
+ * Invoked from raw_irqentry_exit_cond_resched() to check if need resched.
+ * Defaults return true.
+ *
+ * The main purpose is to permit arch to skip preempt a task from an IRQ.
+ */
+static inline bool arch_irqentry_exit_need_resched(void);
+
+#ifndef arch_irqentry_exit_need_resched
+static inline bool arch_irqentry_exit_need_resched(void) { return true; }
+#endif
+
void raw_irqentry_exit_cond_resched(void)
{
if (!preempt_count()) {
@@ -149,7 +163,7 @@ void raw_irqentry_exit_cond_resched(void)
rcu_irq_exit_check_preempt();
if (IS_ENABLED(CONFIG_DEBUG_ENTRY))
WARN_ON_ONCE(!on_thread_stack());
- if (need_resched())
+ if (need_resched() && arch_irqentry_exit_need_resched())
preempt_schedule_irq();
}
}
--
2.34.1On Tue, Jul 29, 2025 at 09:54:54AM +0800, Jinjie Ruan wrote:
> ARM64 requires an additional check whether to reschedule on return
> from interrupt. So add arch_irqentry_exit_need_resched() as the default
> NOP implementation and hook it up into the need_resched() condition in
> raw_irqentry_exit_cond_resched(). This allows ARM64 to implement
> the architecture specific version for switching over to
> the generic entry code.
>
> To align the structure of the code with irqentry_exit_cond_resched()
> from the generic entry code, hoist the need_irq_preemption()
> and IS_ENABLED() check earlier. And different preemption check functions
> are defined based on whether dynamic preemption is enabled.
>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Suggested-by: Kevin Brodsky <kevin.brodsky@arm.com>
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
> arch/arm64/include/asm/preempt.h | 4 ++++
> arch/arm64/kernel/entry-common.c | 35 ++++++++++++++++++--------------
> kernel/entry/common.c | 16 ++++++++++++++-
> 3 files changed, 39 insertions(+), 16 deletions(-)
Can you please split the change to kernel/entry/common.c into a separate
patch? That doesn't depend on the arm64-specific changes, and it'll make
it easier to handle any conflcits when merging this.
Mark.
>
> diff --git a/arch/arm64/include/asm/preempt.h b/arch/arm64/include/asm/preempt.h
> index 0159b625cc7f..0f0ba250efe8 100644
> --- a/arch/arm64/include/asm/preempt.h
> +++ b/arch/arm64/include/asm/preempt.h
> @@ -85,6 +85,7 @@ static inline bool should_resched(int preempt_offset)
> void preempt_schedule(void);
> void preempt_schedule_notrace(void);
>
> +void raw_irqentry_exit_cond_resched(void);
> #ifdef CONFIG_PREEMPT_DYNAMIC
>
> DECLARE_STATIC_KEY_TRUE(sk_dynamic_irqentry_exit_cond_resched);
> @@ -92,11 +93,14 @@ 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()
> +void dynamic_irqentry_exit_cond_resched(void);
> +#define irqentry_exit_cond_resched() dynamic_irqentry_exit_cond_resched()
>
> #else /* CONFIG_PREEMPT_DYNAMIC */
>
> #define __preempt_schedule() preempt_schedule()
> #define __preempt_schedule_notrace() preempt_schedule_notrace()
> +#define irqentry_exit_cond_resched() raw_irqentry_exit_cond_resched()
>
> #endif /* CONFIG_PREEMPT_DYNAMIC */
> #endif /* CONFIG_PREEMPTION */
> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
> index 7c2299c1ba79..4f92664fd46c 100644
> --- a/arch/arm64/kernel/entry-common.c
> +++ b/arch/arm64/kernel/entry-common.c
> @@ -285,19 +285,8 @@ 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);
> -#define need_irq_preemption() \
> - (static_branch_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
> -#else
> -#define need_irq_preemption() (IS_ENABLED(CONFIG_PREEMPTION))
> -#endif
> -
> static inline bool arm64_preempt_schedule_irq(void)
> {
> - if (!need_irq_preemption())
> - return false;
> -
> /*
> * 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
> @@ -672,6 +661,24 @@ static __always_inline void __el1_pnmi(struct pt_regs *regs,
> arm64_exit_nmi(regs, state);
> }
>
> +void raw_irqentry_exit_cond_resched(void)
> +{
> + if (!preempt_count()) {
> + if (need_resched() && arm64_preempt_schedule_irq())
> + preempt_schedule_irq();
> + }
> +}
> +
> +#ifdef CONFIG_PREEMPT_DYNAMIC
> +DEFINE_STATIC_KEY_TRUE(sk_dynamic_irqentry_exit_cond_resched);
> +void dynamic_irqentry_exit_cond_resched(void)
> +{
> + if (!static_branch_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
> + return;
> + raw_irqentry_exit_cond_resched();
> +}
> +#endif
> +
> static __always_inline void __el1_irq(struct pt_regs *regs,
> void (*handler)(struct pt_regs *))
> {
> @@ -681,10 +688,8 @@ static __always_inline void __el1_irq(struct pt_regs *regs,
> do_interrupt_handler(regs, handler);
> irq_exit_rcu();
>
> - if (!preempt_count() && need_resched()) {
> - if (arm64_preempt_schedule_irq())
> - preempt_schedule_irq();
> - }
> + if (IS_ENABLED(CONFIG_PREEMPTION))
> + irqentry_exit_cond_resched();
>
> exit_to_kernel_mode(regs, state);
> }
> diff --git a/kernel/entry/common.c b/kernel/entry/common.c
> index b82032777310..4aa9656fa1b4 100644
> --- a/kernel/entry/common.c
> +++ b/kernel/entry/common.c
> @@ -142,6 +142,20 @@ noinstr irqentry_state_t irqentry_enter(struct pt_regs *regs)
> return ret;
> }
>
> +/**
> + * arch_irqentry_exit_need_resched - Architecture specific need resched function
> + *
> + * Invoked from raw_irqentry_exit_cond_resched() to check if need resched.
> + * Defaults return true.
> + *
> + * The main purpose is to permit arch to skip preempt a task from an IRQ.
> + */
> +static inline bool arch_irqentry_exit_need_resched(void);
> +
> +#ifndef arch_irqentry_exit_need_resched
> +static inline bool arch_irqentry_exit_need_resched(void) { return true; }
> +#endif
> +
> void raw_irqentry_exit_cond_resched(void)
> {
> if (!preempt_count()) {
> @@ -149,7 +163,7 @@ void raw_irqentry_exit_cond_resched(void)
> rcu_irq_exit_check_preempt();
> if (IS_ENABLED(CONFIG_DEBUG_ENTRY))
> WARN_ON_ONCE(!on_thread_stack());
> - if (need_resched())
> + if (need_resched() && arch_irqentry_exit_need_resched())
> preempt_schedule_irq();
> }
> }
> --
> 2.34.1
>
On 2025/8/12 19:13, Mark Rutland wrote:
> On Tue, Jul 29, 2025 at 09:54:54AM +0800, Jinjie Ruan wrote:
>> ARM64 requires an additional check whether to reschedule on return
>> from interrupt. So add arch_irqentry_exit_need_resched() as the default
>> NOP implementation and hook it up into the need_resched() condition in
>> raw_irqentry_exit_cond_resched(). This allows ARM64 to implement
>> the architecture specific version for switching over to
>> the generic entry code.
>>
>> To align the structure of the code with irqentry_exit_cond_resched()
>> from the generic entry code, hoist the need_irq_preemption()
>> and IS_ENABLED() check earlier. And different preemption check functions
>> are defined based on whether dynamic preemption is enabled.
>>
>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>> Suggested-by: Kevin Brodsky <kevin.brodsky@arm.com>
>> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>> arch/arm64/include/asm/preempt.h | 4 ++++
>> arch/arm64/kernel/entry-common.c | 35 ++++++++++++++++++--------------
>> kernel/entry/common.c | 16 ++++++++++++++-
>> 3 files changed, 39 insertions(+), 16 deletions(-)
>
> Can you please split the change to kernel/entry/common.c into a separate
> patch? That doesn't depend on the arm64-specific changes, and it'll make
> it easier to handle any conflcits when merging this.
Sure, I'll split the change into separate patch.
>
> Mark.
>
>>
>> diff --git a/arch/arm64/include/asm/preempt.h b/arch/arm64/include/asm/preempt.h
>> index 0159b625cc7f..0f0ba250efe8 100644
>> --- a/arch/arm64/include/asm/preempt.h
>> +++ b/arch/arm64/include/asm/preempt.h
>> @@ -85,6 +85,7 @@ static inline bool should_resched(int preempt_offset)
>> void preempt_schedule(void);
>> void preempt_schedule_notrace(void);
>>
>> +void raw_irqentry_exit_cond_resched(void);
>> #ifdef CONFIG_PREEMPT_DYNAMIC
>>
>> DECLARE_STATIC_KEY_TRUE(sk_dynamic_irqentry_exit_cond_resched);
>> @@ -92,11 +93,14 @@ 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()
>> +void dynamic_irqentry_exit_cond_resched(void);
>> +#define irqentry_exit_cond_resched() dynamic_irqentry_exit_cond_resched()
>>
>> #else /* CONFIG_PREEMPT_DYNAMIC */
>>
>> #define __preempt_schedule() preempt_schedule()
>> #define __preempt_schedule_notrace() preempt_schedule_notrace()
>> +#define irqentry_exit_cond_resched() raw_irqentry_exit_cond_resched()
>>
>> #endif /* CONFIG_PREEMPT_DYNAMIC */
>> #endif /* CONFIG_PREEMPTION */
>> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
>> index 7c2299c1ba79..4f92664fd46c 100644
>> --- a/arch/arm64/kernel/entry-common.c
>> +++ b/arch/arm64/kernel/entry-common.c
>> @@ -285,19 +285,8 @@ 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);
>> -#define need_irq_preemption() \
>> - (static_branch_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
>> -#else
>> -#define need_irq_preemption() (IS_ENABLED(CONFIG_PREEMPTION))
>> -#endif
>> -
>> static inline bool arm64_preempt_schedule_irq(void)
>> {
>> - if (!need_irq_preemption())
>> - return false;
>> -
>> /*
>> * 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
>> @@ -672,6 +661,24 @@ static __always_inline void __el1_pnmi(struct pt_regs *regs,
>> arm64_exit_nmi(regs, state);
>> }
>>
>> +void raw_irqentry_exit_cond_resched(void)
>> +{
>> + if (!preempt_count()) {
>> + if (need_resched() && arm64_preempt_schedule_irq())
>> + preempt_schedule_irq();
>> + }
>> +}
>> +
>> +#ifdef CONFIG_PREEMPT_DYNAMIC
>> +DEFINE_STATIC_KEY_TRUE(sk_dynamic_irqentry_exit_cond_resched);
>> +void dynamic_irqentry_exit_cond_resched(void)
>> +{
>> + if (!static_branch_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
>> + return;
>> + raw_irqentry_exit_cond_resched();
>> +}
>> +#endif
>> +
>> static __always_inline void __el1_irq(struct pt_regs *regs,
>> void (*handler)(struct pt_regs *))
>> {
>> @@ -681,10 +688,8 @@ static __always_inline void __el1_irq(struct pt_regs *regs,
>> do_interrupt_handler(regs, handler);
>> irq_exit_rcu();
>>
>> - if (!preempt_count() && need_resched()) {
>> - if (arm64_preempt_schedule_irq())
>> - preempt_schedule_irq();
>> - }
>> + if (IS_ENABLED(CONFIG_PREEMPTION))
>> + irqentry_exit_cond_resched();
>>
>> exit_to_kernel_mode(regs, state);
>> }
>> diff --git a/kernel/entry/common.c b/kernel/entry/common.c
>> index b82032777310..4aa9656fa1b4 100644
>> --- a/kernel/entry/common.c
>> +++ b/kernel/entry/common.c
>> @@ -142,6 +142,20 @@ noinstr irqentry_state_t irqentry_enter(struct pt_regs *regs)
>> return ret;
>> }
>>
>> +/**
>> + * arch_irqentry_exit_need_resched - Architecture specific need resched function
>> + *
>> + * Invoked from raw_irqentry_exit_cond_resched() to check if need resched.
>> + * Defaults return true.
>> + *
>> + * The main purpose is to permit arch to skip preempt a task from an IRQ.
>> + */
>> +static inline bool arch_irqentry_exit_need_resched(void);
>> +
>> +#ifndef arch_irqentry_exit_need_resched
>> +static inline bool arch_irqentry_exit_need_resched(void) { return true; }
>> +#endif
>> +
>> void raw_irqentry_exit_cond_resched(void)
>> {
>> if (!preempt_count()) {
>> @@ -149,7 +163,7 @@ void raw_irqentry_exit_cond_resched(void)
>> rcu_irq_exit_check_preempt();
>> if (IS_ENABLED(CONFIG_DEBUG_ENTRY))
>> WARN_ON_ONCE(!on_thread_stack());
>> - if (need_resched())
>> + if (need_resched() && arch_irqentry_exit_need_resched())
>> preempt_schedule_irq();
>> }
>> }
>> --
>> 2.34.1
>>
>
>
On 29/07/2025 02:54, Jinjie Ruan wrote:
> ARM64 requires an additional check whether to reschedule on return
> from interrupt. So add arch_irqentry_exit_need_resched() as the default
> NOP implementation and hook it up into the need_resched() condition in
> raw_irqentry_exit_cond_resched(). This allows ARM64 to implement
> the architecture specific version for switching over to
> the generic entry code.
>
> To align the structure of the code with irqentry_exit_cond_resched()
> from the generic entry code, hoist the need_irq_preemption()
> and IS_ENABLED() check earlier. And different preemption check functions
> are defined based on whether dynamic preemption is enabled.
>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Suggested-by: Kevin Brodsky <kevin.brodsky@arm.com>
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
Unrelated to the other thread : I noticed that compiling this patch
with `allnoconfig` would fail :
- `raw_irqentry_exit_cond_resched` has no previous prototype,
as it is defined within `#ifdef CONFIG_PREEMPTION`
- `irqentry_exit_cond_resched()` is not declared, as it is also within
`#ifdef CONFIG_PREEMPTION`
The patch below fixes the issue, but introduces merge conflicts in
patches 6 and 7, plus the `#ifdef` needs to be moved accordingly
in patch 6 and the empty "without preemption" `irq_exit_cond_resched()`
needs to be removed in patch 7.
I hope this can be useful,
Ada
---
diff --git a/arch/arm64/include/asm/preempt.h
b/arch/arm64/include/asm/preempt.h
index 0f0ba250efe8..d9aba8b1e466 100644
--- a/arch/arm64/include/asm/preempt.h
+++ b/arch/arm64/include/asm/preempt.h
@@ -103,6 +103,8 @@ void dynamic_irqentry_exit_cond_resched(void);
#define irqentry_exit_cond_resched() raw_irqentry_exit_cond_resched()
#endif /* CONFIG_PREEMPT_DYNAMIC */
+#else /* CONFIG_PREEMPTION */
+#define irqentry_exit_cond_resched() {}
#endif /* CONFIG_PREEMPTION */
#endif /* __ASM_PREEMPT_H */
diff --git a/arch/arm64/kernel/entry-common.c
b/arch/arm64/kernel/entry-common.c
index 4f92664fd46c..abd7a315145e 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -661,6 +661,7 @@ static __always_inline void __el1_pnmi(struct
pt_regs *regs,
arm64_exit_nmi(regs, state);
}
+#ifdef CONFIG_PREEMPTION
void raw_irqentry_exit_cond_resched(void)
{
if (!preempt_count()) {
@@ -668,6 +669,7 @@ void raw_irqentry_exit_cond_resched(void)
preempt_schedule_irq();
}
}
+#endif
#ifdef CONFIG_PREEMPT_DYNAMIC
DEFINE_STATIC_KEY_TRUE(sk_dynamic_irqentry_exit_cond_resched);
On 2025/8/12 0:02, Ada Couprie Diaz wrote:
> On 29/07/2025 02:54, Jinjie Ruan wrote:
>
>> ARM64 requires an additional check whether to reschedule on return
>> from interrupt. So add arch_irqentry_exit_need_resched() as the default
>> NOP implementation and hook it up into the need_resched() condition in
>> raw_irqentry_exit_cond_resched(). This allows ARM64 to implement
>> the architecture specific version for switching over to
>> the generic entry code.
>>
>> To align the structure of the code with irqentry_exit_cond_resched()
>> from the generic entry code, hoist the need_irq_preemption()
>> and IS_ENABLED() check earlier. And different preemption check functions
>> are defined based on whether dynamic preemption is enabled.
>>
>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>> Suggested-by: Kevin Brodsky <kevin.brodsky@arm.com>
>> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
> Unrelated to the other thread : I noticed that compiling this patch
> with `allnoconfig` would fail :
> - `raw_irqentry_exit_cond_resched` has no previous prototype,
> as it is defined within `#ifdef CONFIG_PREEMPTION`
> - `irqentry_exit_cond_resched()` is not declared, as it is also within
> `#ifdef CONFIG_PREEMPTION`
You are right, thank you! I'll fix it.
>
> The patch below fixes the issue, but introduces merge conflicts in
> patches 6 and 7, plus the `#ifdef` needs to be moved accordingly
> in patch 6 and the empty "without preemption" `irq_exit_cond_resched()`
> needs to be removed in patch 7.
>
> I hope this can be useful,
> Ada
>
> ---
> diff --git a/arch/arm64/include/asm/preempt.h
> b/arch/arm64/include/asm/preempt.h
> index 0f0ba250efe8..d9aba8b1e466 100644
> --- a/arch/arm64/include/asm/preempt.h
> +++ b/arch/arm64/include/asm/preempt.h
> @@ -103,6 +103,8 @@ void dynamic_irqentry_exit_cond_resched(void);
> #define irqentry_exit_cond_resched() raw_irqentry_exit_cond_resched()
>
> #endif /* CONFIG_PREEMPT_DYNAMIC */
> +#else /* CONFIG_PREEMPTION */
> +#define irqentry_exit_cond_resched() {}
> #endif /* CONFIG_PREEMPTION */
>
> #endif /* __ASM_PREEMPT_H */
> diff --git a/arch/arm64/kernel/entry-common.c
> b/arch/arm64/kernel/entry-common.c
> index 4f92664fd46c..abd7a315145e 100644
> --- a/arch/arm64/kernel/entry-common.c
> +++ b/arch/arm64/kernel/entry-common.c
> @@ -661,6 +661,7 @@ static __always_inline void __el1_pnmi(struct
> pt_regs *regs,
> arm64_exit_nmi(regs, state);
> }
>
> +#ifdef CONFIG_PREEMPTION
> void raw_irqentry_exit_cond_resched(void)
> {
> if (!preempt_count()) {
> @@ -668,6 +669,7 @@ void raw_irqentry_exit_cond_resched(void)
> preempt_schedule_irq();
> }
> }
> +#endif
>
> #ifdef CONFIG_PREEMPT_DYNAMIC
> DEFINE_STATIC_KEY_TRUE(sk_dynamic_irqentry_exit_cond_resched);
>
>
Hi Jinjie,
On 29/07/2025 02:54, Jinjie Ruan wrote:
> ARM64 requires an additional check whether to reschedule on return
> from interrupt. So add arch_irqentry_exit_need_resched() as the default
> NOP implementation and hook it up into the need_resched() condition in
> raw_irqentry_exit_cond_resched(). This allows ARM64 to implement
> the architecture specific version for switching over to
> the generic entry code.
I was a bit confused by this, as I didn't see the link with the generic
entry
given you implement `raw_irqentry_exit_cond_resched()` in arch/arm64
as well in this patch : I expected the arm64 implementation to be added.
I share more thoughts below.
What do you think about something along those lines ?
Compared to the generic entry code, arm64 does additional checks
when deciding to reschedule on return from an interrupt.
Introduce arch_irqentry_exit_need_resched() in the need_resched() condition
of the generic raw_irqentry_exit_cond_resched(), with a NOP default.
This will allow arm64 to implement its architecture specific checks when
switching over to the generic entry code.
> [...]
> diff --git a/kernel/entry/common.c b/kernel/entry/common.c
> index b82032777310..4aa9656fa1b4 100644
> --- a/kernel/entry/common.c
> +++ b/kernel/entry/common.c
> @@ -142,6 +142,20 @@ noinstr irqentry_state_t irqentry_enter(struct pt_regs *regs)
> return ret;
> }
>
> +/**
> + * arch_irqentry_exit_need_resched - Architecture specific need resched function
> + *
> + * Invoked from raw_irqentry_exit_cond_resched() to check if need resched.
Very nit : "to check if resched is needed." ?
> + * Defaults return true.
> + *
> + * The main purpose is to permit arch to skip preempt a task from an IRQ.
If feel that "to avoid preemption of a task" instead of "to skip preempt
a task"
would make this much clearer, what do you think ?
> + */
> +static inline bool arch_irqentry_exit_need_resched(void);
> +
> +#ifndef arch_irqentry_exit_need_resched
> +static inline bool arch_irqentry_exit_need_resched(void) { return true; }
> +#endif
> +
I've had some trouble reviewing this patch : on the one hand because
I didn't notice `arch_irqentry_exit_need_resched()` was added in
the common entry code, which is on me !
On the other hand, I felt that the patch itself was a bit disconnected :
we add `arch_irqentry_exit_need_resched()` in the common entry code,
with a default NOP, but in the same function we add to arm64,
while mentioning that this is for arm64's additional checks,
which we only implement in patch 7.
Would it make sense to move the `arch_irqentry_exit_need_resched()`
part of the patch to patch 7, so that the introduction and
arch-specific implementation appear together ?
To me it seems easier to wrap my head around, as it would look like
"Move arm64 to generic entry, but it does additional checks : add a new
arch-specific function controlling re-scheduling, defaulting to true,
and implement it for arm64". I feel it could help making patch 7's
commit message clearer as well.
From what I gathered on the archive `arch_irqentry_exit_need_resched()`
being added here was suggested previously, so others might not have the
same opinion.
Maybe improving the commit message and comment for this would be enough
as well, as per my suggestions above.
Otherwise the changes make sense and I don't see any functional issues !
Thanks,
Ada
On 2025/8/5 23:06, Ada Couprie Diaz wrote:
> Hi Jinjie,
>
> On 29/07/2025 02:54, Jinjie Ruan wrote:
>> ARM64 requires an additional check whether to reschedule on return
>> from interrupt. So add arch_irqentry_exit_need_resched() as the default
>> NOP implementation and hook it up into the need_resched() condition in
>> raw_irqentry_exit_cond_resched(). This allows ARM64 to implement
>> the architecture specific version for switching over to
>> the generic entry code.
> I was a bit confused by this, as I didn't see the link with the generic
> entry
> given you implement `raw_irqentry_exit_cond_resched()` in arch/arm64
> as well in this patch : I expected the arm64 implementation to be added.
> I share more thoughts below.
>
> What do you think about something along those lines ?
>
> Compared to the generic entry code, arm64 does additional checks
> when deciding to reschedule on return from an interrupt.
> Introduce arch_irqentry_exit_need_resched() in the need_resched()
> condition
> of the generic raw_irqentry_exit_cond_resched(), with a NOP default.
> This will allow arm64 to implement its architecture specific checks
> when
> switching over to the generic entry code.
This revision makes it easier for people to understand.
>
>> [...]
>> diff --git a/kernel/entry/common.c b/kernel/entry/common.c
>> index b82032777310..4aa9656fa1b4 100644
>> --- a/kernel/entry/common.c
>> +++ b/kernel/entry/common.c
>> @@ -142,6 +142,20 @@ noinstr irqentry_state_t irqentry_enter(struct
>> pt_regs *regs)
>> return ret;
>> }
>> +/**
>> + * arch_irqentry_exit_need_resched - Architecture specific need
>> resched function
>> + *
>> + * Invoked from raw_irqentry_exit_cond_resched() to check if need
>> resched.
> Very nit : "to check if resched is needed." ?
This is good.
>> + * Defaults return true.
>> + *
>> + * The main purpose is to permit arch to skip preempt a task from an
>> IRQ.
> If feel that "to avoid preemption of a task" instead of "to skip preempt
> a task"
> would make this much clearer, what do you think ?
Yes, this is more clearer.
>> + */
>> +static inline bool arch_irqentry_exit_need_resched(void);
>> +
>> +#ifndef arch_irqentry_exit_need_resched
>> +static inline bool arch_irqentry_exit_need_resched(void) { return
>> true; }
>> +#endif
>> +
>
> I've had some trouble reviewing this patch : on the one hand because
> I didn't notice `arch_irqentry_exit_need_resched()` was added in
> the common entry code, which is on me !
> On the other hand, I felt that the patch itself was a bit disconnected :
> we add `arch_irqentry_exit_need_resched()` in the common entry code,
> with a default NOP, but in the same function we add to arm64,
> while mentioning that this is for arm64's additional checks,
> which we only implement in patch 7.
>
> Would it make sense to move the `arch_irqentry_exit_need_resched()`
> part of the patch to patch 7, so that the introduction and
> arch-specific implementation appear together ?
> To me it seems easier to wrap my head around, as it would look like
> "Move arm64 to generic entry, but it does additional checks : add a new
> arch-specific function controlling re-scheduling, defaulting to true,
> and implement it for arm64". I feel it could help making patch 7's
> commit message clearer as well.
>
> From what I gathered on the archive `arch_irqentry_exit_need_resched()`
> being added here was suggested previously, so others might not have the
> same opinion.
>
> Maybe improving the commit message and comment for this would be enough
> as well, as per my suggestions above.
>
>
> Otherwise the changes make sense and I don't see any functional issues !
>
> Thanks,
> Ada
>
>
On 2025/8/5 23:06, Ada Couprie Diaz wrote:
> Hi Jinjie,
>
> On 29/07/2025 02:54, Jinjie Ruan wrote:
>> ARM64 requires an additional check whether to reschedule on return
>> from interrupt. So add arch_irqentry_exit_need_resched() as the default
>> NOP implementation and hook it up into the need_resched() condition in
>> raw_irqentry_exit_cond_resched(). This allows ARM64 to implement
>> the architecture specific version for switching over to
>> the generic entry code.
> I was a bit confused by this, as I didn't see the link with the generic
> entry
> given you implement `raw_irqentry_exit_cond_resched()` in arch/arm64
> as well in this patch : I expected the arm64 implementation to be added.
> I share more thoughts below.
>
> What do you think about something along those lines ?
>
> Compared to the generic entry code, arm64 does additional checks
> when deciding to reschedule on return from an interrupt.
> Introduce arch_irqentry_exit_need_resched() in the need_resched()
> condition
> of the generic raw_irqentry_exit_cond_resched(), with a NOP default.
> This will allow arm64 to implement its architecture specific checks
> when
> switching over to the generic entry code.
>
>> [...]
>> diff --git a/kernel/entry/common.c b/kernel/entry/common.c
>> index b82032777310..4aa9656fa1b4 100644
>> --- a/kernel/entry/common.c
>> +++ b/kernel/entry/common.c
>> @@ -142,6 +142,20 @@ noinstr irqentry_state_t irqentry_enter(struct
>> pt_regs *regs)
>> return ret;
>> }
>> +/**
>> + * arch_irqentry_exit_need_resched - Architecture specific need
>> resched function
>> + *
>> + * Invoked from raw_irqentry_exit_cond_resched() to check if need
>> resched.
> Very nit : "to check if resched is needed." ?
>> + * Defaults return true.
>> + *
>> + * The main purpose is to permit arch to skip preempt a task from an
>> IRQ.
> If feel that "to avoid preemption of a task" instead of "to skip preempt
> a task"
> would make this much clearer, what do you think ?
>> + */
>> +static inline bool arch_irqentry_exit_need_resched(void);
>> +
>> +#ifndef arch_irqentry_exit_need_resched
>> +static inline bool arch_irqentry_exit_need_resched(void) { return
>> true; }
>> +#endif
>> +
>
> I've had some trouble reviewing this patch : on the one hand because
> I didn't notice `arch_irqentry_exit_need_resched()` was added in
> the common entry code, which is on me !
> On the other hand, I felt that the patch itself was a bit disconnected :
> we add `arch_irqentry_exit_need_resched()` in the common entry code,
> with a default NOP, but in the same function we add to arm64,
> while mentioning that this is for arm64's additional checks,
> which we only implement in patch 7.
Yes, it does.
>
> Would it make sense to move the `arch_irqentry_exit_need_resched()`
> part of the patch to patch 7, so that the introduction and
> arch-specific implementation appear together ?
> To me it seems easier to wrap my head around, as it would look like
> "Move arm64 to generic entry, but it does additional checks : add a new
> arch-specific function controlling re-scheduling, defaulting to true,
> and implement it for arm64". I feel it could help making patch 7's
> commit message clearer as well.
>
> From what I gathered on the archive `arch_irqentry_exit_need_resched()`
> being added here was suggested previously, so others might not have the
> same opinion.
Yes, introduce `arch_irqentry_exit_need_resched()` here may help
understand the patch's refactoring purpose.
>
> Maybe improving the commit message and comment for this would be enough
> as well, as per my suggestions above.
Thank you! I'll improve the commit message and comment.
>
>
> Otherwise the changes make sense and I don't see any functional issues !
>
> Thanks,
> Ada
>
>
On 06/08/2025 07:39, Jinjie Ruan wrote: > On 2025/8/5 23:06, Ada Couprie Diaz wrote: >> Hi Jinjie, >> >> On 29/07/2025 02:54, Jinjie Ruan wrote: >>> ARM64 requires an additional check whether to reschedule on return >>> from interrupt. So add arch_irqentry_exit_need_resched() as the default >>> NOP implementation and hook it up into the need_resched() condition in >>> raw_irqentry_exit_cond_resched(). This allows ARM64 to implement >>> the architecture specific version for switching over to >>> the generic entry code. >>> [...] >> I've had some trouble reviewing this patch : on the one hand because >> I didn't notice `arch_irqentry_exit_need_resched()` was added in >> the common entry code, which is on me ! >> On the other hand, I felt that the patch itself was a bit disconnected : >> we add `arch_irqentry_exit_need_resched()` in the common entry code, >> with a default NOP, but in the same function we add to arm64, >> while mentioning that this is for arm64's additional checks, >> which we only implement in patch 7. > Yes, it does. > >> Would it make sense to move the `arch_irqentry_exit_need_resched()` >> part of the patch to patch 7, so that the introduction and >> arch-specific implementation appear together ? >> To me it seems easier to wrap my head around, as it would look like >> "Move arm64 to generic entry, but it does additional checks : add a new >> arch-specific function controlling re-scheduling, defaulting to true, >> and implement it for arm64". I feel it could help making patch 7's >> commit message clearer as well. >> >> From what I gathered on the archive `arch_irqentry_exit_need_resched()` >> being added here was suggested previously, so others might not have the >> same opinion. > Yes, introduce `arch_irqentry_exit_need_resched()` here may help > understand the patch's refactoring purpose. I can see that as well. I shared my opinion in case it could be useful, but as I mentioned in my reply to the cover : it's not a big issue and I'm happy for `arch_irqentry_exit_need_resched()` to be implemented here if that makes more sense ! >> Maybe improving the commit message and comment for this would be enough >> as well, as per my suggestions above. > Thank you! I'll improve the commit message and comment. > My pleasure ! Ada
© 2016 - 2025 Red Hat, Inc.