Re: [PATCH v3] softirq: Preserve interrupt context during IRQ exit

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2026-09-24 06:15:38 [+0200], Karl Mehltretter wrote:
> __irq_exit_rcu() removes HARDIRQ_OFFSET first and then runs
> hrtimer_rearm_deferred(), invoke_softirq() and wake_timersd(). This is
> interrupt exit work with interrupts disabled, but preempt_count already
> describes the interrupted task again. Everything which derives the
> context from preempt_count gets it wrong in that window:

On the return from interrupt path __irq_exit_rcu() removes
HARDIRQ_OFFSET from the preemption counter at the very top of the
function. Everything after that will get the current context reported as
TASK instead of HARDIRQ. Everything in that function, such as
invoke_softirq(), is aware of the situation and does not rely on the
counter. There are some exceptions:

>   - ftrace, perf and the ring buffer record task context and use the
…
> Notes:
>     v3 also adds a pending softirq check after the final
>     preempt_count_sub(). A preempt_enable tracepoint callback can raise a
>     softirq there, after the softirq and timer thread checks. With an RCU
>     reader in that callback I saw 10 NOHZ tick-stop warnings per boot in
>     QEMU without the new check, and none on the base kernel or with the
>     check. The check wakes ksoftirqd for newly pending softirqs. Does this
>     case justify the extra check on every IRQ exit?

No, I don't think so. This is not the normal path. I would rather prefer
of not being able to attach a tracepoint to preempt_count_sub() within
__irq_exit_rcu() than this. Also one can attach a tracepoint to
tick_irq_exit() leaving you with the exact same problem. So I would
rather have __irq_exit_rcu() in a section where tracepoints can not be
attached or have the consequences that someone did that. Or maybe have
tracepoints/BPF taught to not do this here.

We may legitimately leave invoke_softirq() with pending softirqs. So you
probably for that reason mask everything out and have a few other
conditions for that reason. Does not justify it.

…
>     - Rebased on tip/master c81f6d2398d0.

The hash here is not really helpful because tip/master is getting thrown
away. tip/master as of today, better yet the branch that it will be
probably applied to.

> --- a/Documentation/core-api/entry.rst
> +++ b/Documentation/core-api/entry.rst
> @@ -197,8 +197,9 @@ return true, handles NOHZ tick state and interrupt time accounting. This
>  means that up to the point where irq_enter_rcu() is invoked in_hardirq()
>  returns false.
>  
> -irq_exit_rcu() handles interrupt time accounting, undoes the preemption
> -count update and eventually handles soft interrupts and NOHZ tick state.
> +irq_exit_rcu() handles interrupt time accounting, handles soft interrupts if
> +possible, undoes the preemption count update and finally handles the NOHZ tick
> +state.

Changed the ordering to the code flow. Okay.

  irq_exit_rcu() handles interrupt time accounting, eventually handles
  soft interrupts, undoes the preemption count update and handles NOHZ
  tick state.

Or leave it.

>  In theory, the preemption count could be updated in irqentry_enter(). In
>  practice, deferring this update to irq_enter_rcu() allows the preemption-count
> @@ -207,10 +208,13 @@ irqentry_exit(), which are described in the next paragraph. The only downside
>  is that the early entry code up to irq_enter_rcu() must be aware that the
>  preemption count has not yet been updated with the HARDIRQ_OFFSET state.
>  
> -Note that irq_exit_rcu() must remove HARDIRQ_OFFSET from the preemption count
> -before it handles soft interrupts, whose handlers must run in BH context rather
> -than irq-disabled context. In addition, irqentry_exit() might schedule, which
> -also requires that HARDIRQ_OFFSET has been removed from the preemption count.

> +Note that soft interrupt handlers must run in BH context rather than in hard
> +interrupt context. irq_exit_rcu() therefore replaces HARDIRQ_OFFSET with
> +SOFTIRQ_OFFSET in the preemption count while it handles soft interrupts and
> +puts HARDIRQ_OFFSET back afterwards, so that the remaining interrupt exit work
> +is still attributed to the interrupt. HARDIRQ_OFFSET is removed before
> +irq_exit_rcu() returns because irqentry_exit() might schedule, which requires
> +that HARDIRQ_OFFSET has been removed from the preemption count.

Good.

>  Even though interrupt handlers are expected to run with local interrupts
>  disabled, interrupt nesting is common from an entry/exit perspective. For
> diff --git a/kernel/softirq.c b/kernel/softirq.c
> index c3729c5b284b0..efa6707edc0e9 100644
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -350,8 +350,8 @@ static inline void ksoftirqd_run_end(void)

As mentioned, the changes below look good except for the check
in __irq_exit_rcu() which feels a bit much.

Sebastian