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

[Karl Mehltretter]

On 2026-09-17 17:21:50 [+0200], Sebastian Andrzej Siewior wrote:
> The breakage is limited to KCSAN & friends within the window during
> transition to softirq and out. There is nothing else? Well, the timer
> wake looks wrong in trace, noted.

I know of nothing that is broken today. It is more than instrumentation
though. Code that reads the context from preempt_count sees the
interrupted task in that window. Besides ftrace, KCSAN, KMSAN, KCOV and
the printk caller id I found:

 - can_spin_trylock() and local_trylock() on RT refuse hard interrupt
   context. A trylock on top of a task that is blocked on a lock
   confuses the PI code. In that window they do not refuse. BPF attached
   to sched_waking or sched_wakeup reaches them through kmalloc_nolock().

 - oops_end() and make_task_dead() test in_interrupt(). Today an oops in
   that window is treated like an oops in task context and kills the
   interrupted task. With HARDIRQ_OFFSET set it panics with "Fatal
   exception in interrupt", like an oops in the handler itself.

 - rcu_read_unlock_special() and raise_softirq_irqoff(). See the end of
   this mail.

I'll list these in the changelog. I will also say what the patch does
not cover. tick_irq_exit() and the other deferred rearm sites still run
after HARDIRQ_OFFSET is removed.

> /*
>  * This is only entered on return from interrupt. Preemption disabled
>  * locations remains unchanged, the context (-HARDIRQ +SOFTIRQ) is
>  * updated and lockdep is let known.
>  */

I'll use it, reworded, and add two points. in_hardirq() is sampled
because __do_softirq() is reached through do_softirq_own_stack() on
several architectures and cannot take an argument. The raw operation is
used because the preemption disabled section from irq_enter_rcu()
continues.

> The casts look odd. We need this? It is defined as long, yes, but
> preempt_count accepts an int only so it will throw the upper bits away.

The value is the same. Without the casts gcc warns:

  warning: overflow in conversion from 'long unsigned int' to 'int'
  changes value from '18446744073692774656' to '-16776960' [-Woverflow]

-Woverflow is on by default. I'll swap the operands:

	__preempt_count_sub(HARDIRQ_OFFSET - SOFTIRQ_OFFSET);

and use __preempt_count_add() at the end. The difference is positive
and fits an int. No cast and no warning.

> I think we want to ensure that irq_count() == SOFTIRQ_OFFSET

Will do. This path is !PREEMPT_RT only, so irq_count() is the plain
preempt_count() one.

> that is quite some WARN_ON_ONCE. We would like to see just
> HARDIRQ_OFFSET at the end. Or SOFTIRQ_OFFSET before the end. One should
> be enough or the math is wrong.

softirq_handle_end() will have one. It checks irq_count() ==
HARDIRQ_OFFSET after the addition. In softirq_handle_begin() I will move
lockdep_softirqs_off() before the assertion. Then the lockdep softirq
state is consistent if the assertion fires and printk runs.

> irq_enter_rcu() did preempt_count_add(HARDIRQ_OFFSET), did record
> task_struct::preempt_disable_ip. [...] This looks like an improvement.

Yes. The preemptoff tracer changes too. It reports the hard interrupt
and the softirq processing after it as one section. I'll add both to
the changelog.

> Why is this preempt_count() instead irq_count. Why is there
> IRQ_EXIT_TIMERS? It is almost as the first check except now we would
> like to ignore the additional softirq_count().

Yes, that is the intent. irq_count() would skip the wakeup when the
interrupt hit a BH disabled or softirq serving section. The old test
did not skip it, and nothing else handles pending_timer_softirq.

I'll drop the macro and the raw preempt_count() and use

	!in_nmi() && hardirq_count() == HARDIRQ_OFFSET

This is the old test, evaluated before HARDIRQ_OFFSET is removed. It
reads like the first check without softirq_count(). I'll add a comment
that says why softirq_count() is left out.

I also want to change the order in your code. With HARDIRQ_OFFSET set,
raise_softirq_irqoff() does not wake ksoftirqd. rcu_read_unlock_special()
raises RCU_SOFTIRQ instead of setting NEED_RESCHED. Both assume that
interrupt exit handles pending softirqs. In v2 that is not true for a
softirq raised inside wake_timersd(), because the wakeup comes after the
pending check. The timer thread would handle it, because run_ktimerd()
handles all vectors. I do not want to rely on that, but wake the timer
thread first:

	if (IS_ENABLED(CONFIG_IRQ_FORCED_THREADING) && force_irqthreads() &&
	    local_timers_pending_force_th() &&
	    !in_nmi() && hardirq_count() == HARDIRQ_OFFSET)
		wake_timersd();

	if (irq_count() == HARDIRQ_OFFSET && local_softirq_pending()) {
		hrtimer_rearm_deferred();
		invoke_softirq();
	}

	preempt_count_sub(HARDIRQ_OFFSET);
	tick_irq_exit();

Today the wakeup already runs before the rearm when no softirq is
pending. Does the old order have a reason that I do not see? Then I
keep it and document that the timer thread handles such a softirq.

Karl