Re: [PATCH 04/10] entry: Split kernel mode logic from irqentry_{enter,exit}()

Jinjie Ruan
2026-04-08T01:32:42Z

On 2026/4/7 21:16, Mark Rutland wrote:
> The generic irqentry code has entry/exit functions specifically for
> exceptions taken from user mode, but doesn't have entry/exit functions
> specifically for exceptions taken from kernel mode.
>
> It would be helpful to have separate entry/exit functions specifically
> for exceptions taken from kernel mode. This would make the structure of
> the entry code more consistent, and would make it easier for
> architectures to manage logic specific to exceptions taken from kernel
> mode.
>
> Move the logic specific to kernel mode out of irqentry_enter() and
> irqentry_exit() into new irqentry_enter_from_kernel_mode() and
> irqentry_exit_to_kernel_mode() functions. These are marked
> __always_inline and placed in irq-entry-common.h, as with
> irqentry_enter_from_user_mode() and irqentry_exit_to_user_mode(), so
> that they can be inlined into architecture-specific wrappers. The
> existing out-of-line irqentry_enter() and irqentry_exit() functions
> retained as callers of the new functions.
>
> The lockdep assertion from irqentry_exit() is moved into
> irqentry_exit_to_user_mode() and irqentry_exit_to_kernel_mode(). This
> was previously missing from irqentry_exit_to_user_mode() when called
> directly, and any new lockdep assertion failure relating from this
> change is a latent bug.
>
> Aside from the lockdep change noted above, there should be no functional
> change as a result of this patch.

Reviewed-by: Jinjie Ruan <ruanjinjie@huawei.com>

>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Jinjie Ruan <ruanjinjie@huawei.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@kernel.org>
> Cc: Vladimir Murzin <vladimir.murzin@arm.com>
> Cc: Will Deacon <will@kernel.org>
> ---
> include/linux/irq-entry-common.h | 103 +++++++++++++++++++++++++++++++
> kernel/entry/common.c | 103 +++----------------------------
> 2 files changed, 111 insertions(+), 95 deletions(-)
>
> Thomas/Peter/Andy, as mentioned on IRC, I haven't created kerneldoc
> comments for these new functions because the existing comments don't
> seem all that consistent (e.g. for user mode vs kernel mode), and I
> suspect we want to rewrite them all in one go for wider consistency.
>
> I'm happy to respin this, or to follow-up with that as per your
> preference.
>
> Mark.
>
> diff --git a/include/linux/irq-entry-common.h b/include/linux/irq-entry-common.h
> index d1e8591a59195..2206150e526d8 100644
> --- a/include/linux/irq-entry-common.h
> +++ b/include/linux/irq-entry-common.h
> @@ -304,6 +304,8 @@ static __always_inline void irqentry_enter_from_user_mode(struct pt_regs *regs)
> */
> static __always_inline void irqentry_exit_to_user_mode(struct pt_regs *regs)
> {
> + lockdep_assert_irqs_disabled();
> +
> instrumentation_begin();
> irqentry_exit_to_user_mode_prepare(regs);
> instrumentation_end();
> @@ -356,6 +358,107 @@ void dynamic_irqentry_exit_cond_resched(void);
> #define irqentry_exit_cond_resched() raw_irqentry_exit_cond_resched()
> #endif /* CONFIG_PREEMPT_DYNAMIC */
>
> +static __always_inline irqentry_state_t irqentry_enter_from_kernel_mode(struct pt_regs *regs)
> +{
> + irqentry_state_t ret = {
> + .exit_rcu = false,
> + };
> +
> + /*
> + * If this entry hit the idle task invoke ct_irq_enter() whether
> + * RCU is watching or not.
> + *
> + * Interrupts can nest when the first interrupt invokes softirq
> + * processing on return which enables interrupts.
> + *
> + * Scheduler ticks in the idle task can mark quiescent state and
> + * terminate a grace period, if and only if the timer interrupt is
> + * not nested into another interrupt.
> + *
> + * Checking for rcu_is_watching() here would prevent the nesting
> + * interrupt to invoke ct_irq_enter(). If that nested interrupt is
> + * the tick then rcu_flavor_sched_clock_irq() would wrongfully
> + * assume that it is the first interrupt and eventually claim
> + * quiescent state and end grace periods prematurely.
> + *
> + * Unconditionally invoke ct_irq_enter() so RCU state stays
> + * consistent.
> + *
> + * TINY_RCU does not support EQS, so let the compiler eliminate
> + * this part when enabled.
> + */
> + if (!IS_ENABLED(CONFIG_TINY_RCU) &&
> + (is_idle_task(current) || arch_in_rcu_eqs())) {
> + /*
> + * If RCU is not watching then the same careful
> + * sequence vs. lockdep and tracing is required
> + * as in irqentry_enter_from_user_mode().
> + */
> + lockdep_hardirqs_off(CALLER_ADDR0);
> + ct_irq_enter();
> + instrumentation_begin();
> + kmsan_unpoison_entry_regs(regs);
> + trace_hardirqs_off_finish();
> + instrumentation_end();
> +
> + ret.exit_rcu = true;
> + return ret;
> + }
> +
> + /*
> + * If RCU is watching then RCU only wants to check whether it needs
> + * to restart the tick in NOHZ mode. rcu_irq_enter_check_tick()
> + * already contains a warning when RCU is not watching, so no point
> + * in having another one here.
> + */
> + lockdep_hardirqs_off(CALLER_ADDR0);
> + instrumentation_begin();
> + kmsan_unpoison_entry_regs(regs);
> + rcu_irq_enter_check_tick();
> + trace_hardirqs_off_finish();
> + instrumentation_end();
> +
> + return ret;
> +}
> +
> +static __always_inline void irqentry_exit_to_kernel_mode(struct pt_regs *regs, irqentry_state_t state)
> +{
> + lockdep_assert_irqs_disabled();
> +
> + if (!regs_irqs_disabled(regs)) {
> + /*
> + * If RCU was not watching on entry this needs to be done
> + * carefully and needs the same ordering of lockdep/tracing
> + * and RCU as the return to user mode path.
> + */
> + if (state.exit_rcu) {
> + instrumentation_begin();
> + /* Tell the tracer that IRET will enable interrupts */
> + trace_hardirqs_on_prepare();
> + lockdep_hardirqs_on_prepare();
> + instrumentation_end();
> + ct_irq_exit();
> + lockdep_hardirqs_on(CALLER_ADDR0);
> + return;
> + }
> +
> + instrumentation_begin();
> + if (IS_ENABLED(CONFIG_PREEMPTION))
> + irqentry_exit_cond_resched();
> +
> + /* Covers both tracing and lockdep */
> + trace_hardirqs_on();
> + instrumentation_end();
> + } else {
> + /*
> + * IRQ flags state is correct already. Just tell RCU if it
> + * was not watching on entry.
> + */
> + if (state.exit_rcu)
> + ct_irq_exit();
> + }
> +}
> +
> /**
> * irqentry_enter - Handle state tracking on ordinary interrupt entries
> * @regs: Pointer to pt_regs of interrupted context
> diff --git a/kernel/entry/common.c b/kernel/entry/common.c
> index b5e05d87ba391..1034be02eae84 100644
> --- a/kernel/entry/common.c
> +++ b/kernel/entry/common.c
> @@ -105,70 +105,16 @@ __always_inline unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
>
> noinstr irqentry_state_t irqentry_enter(struct pt_regs *regs)
> {
> - irqentry_state_t ret = {
> - .exit_rcu = false,
> - };
> -
> if (user_mode(regs)) {
> - irqentry_enter_from_user_mode(regs);
> - return ret;
> - }
> + irqentry_state_t ret = {
> + .exit_rcu = false,
> + };
>
> - /*
> - * If this entry hit the idle task invoke ct_irq_enter() whether
> - * RCU is watching or not.
> - *
> - * Interrupts can nest when the first interrupt invokes softirq
> - * processing on return which enables interrupts.
> - *
> - * Scheduler ticks in the idle task can mark quiescent state and
> - * terminate a grace period, if and only if the timer interrupt is
> - * not nested into another interrupt.
> - *
> - * Checking for rcu_is_watching() here would prevent the nesting
> - * interrupt to invoke ct_irq_enter(). If that nested interrupt is
> - * the tick then rcu_flavor_sched_clock_irq() would wrongfully
> - * assume that it is the first interrupt and eventually claim
> - * quiescent state and end grace periods prematurely.
> - *
> - * Unconditionally invoke ct_irq_enter() so RCU state stays
> - * consistent.
> - *
> - * TINY_RCU does not support EQS, so let the compiler eliminate
> - * this part when enabled.
> - */
> - if (!IS_ENABLED(CONFIG_TINY_RCU) &&
> - (is_idle_task(current) || arch_in_rcu_eqs())) {
> - /*
> - * If RCU is not watching then the same careful
> - * sequence vs. lockdep and tracing is required
> - * as in irqentry_enter_from_user_mode().
> - */
> - lockdep_hardirqs_off(CALLER_ADDR0);
> - ct_irq_enter();
> - instrumentation_begin();
> - kmsan_unpoison_entry_regs(regs);
> - trace_hardirqs_off_finish();
> - instrumentation_end();
> -
> - ret.exit_rcu = true;
> + irqentry_enter_from_user_mode(regs);
> return ret;
> }
>
> - /*
> - * If RCU is watching then RCU only wants to check whether it needs
> - * to restart the tick in NOHZ mode. rcu_irq_enter_check_tick()
> - * already contains a warning when RCU is not watching, so no point
> - * in having another one here.
> - */
> - lockdep_hardirqs_off(CALLER_ADDR0);
> - instrumentation_begin();
> - kmsan_unpoison_entry_regs(regs);
> - rcu_irq_enter_check_tick();
> - trace_hardirqs_off_finish();
> - instrumentation_end();
> -
> - return ret;
> + return irqentry_enter_from_kernel_mode(regs);
> }
>
> /**
> @@ -212,43 +158,10 @@ void dynamic_irqentry_exit_cond_resched(void)
>
> noinstr void irqentry_exit(struct pt_regs *regs, irqentry_state_t state)
> {
> - lockdep_assert_irqs_disabled();
> -
> - /* Check whether this returns to user mode */
> - if (user_mode(regs)) {
> + if (user_mode(regs))
> irqentry_exit_to_user_mode(regs);
> - } else if (!regs_irqs_disabled(regs)) {
> - /*
> - * If RCU was not watching on entry this needs to be done
> - * carefully and needs the same ordering of lockdep/tracing
> - * and RCU as the return to user mode path.
> - */
> - if (state.exit_rcu) {
> - instrumentation_begin();
> - /* Tell the tracer that IRET will enable interrupts */
> - trace_hardirqs_on_prepare();
> - lockdep_hardirqs_on_prepare();
> - instrumentation_end();
> - ct_irq_exit();
> - lockdep_hardirqs_on(CALLER_ADDR0);
> - return;
> - }
> -
> - instrumentation_begin();
> - if (IS_ENABLED(CONFIG_PREEMPTION))
> - irqentry_exit_cond_resched();
> -
> - /* Covers both tracing and lockdep */
> - trace_hardirqs_on();
> - instrumentation_end();
> - } else {
> - /*
> - * IRQ flags state is correct already. Just tell RCU if it
> - * was not watching on entry.
> - */
> - if (state.exit_rcu)
> - ct_irq_exit();
> - }
> + else
> + irqentry_exit_to_kernel_mode(regs, state);
> }
>
> irqentry_state_t noinstr irqentry_nmi_enter(struct pt_regs *regs)