Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] softirq: tick_irq_exit() called before trace_hardirq_context_exit() handoff --
commit 2beb0d9dac0a7b76bfe5b73b550fff3be238bbab Author: Karl Mehltretter <[email protected]> tracing/irq: Add generic IRQ context boundary events This commit adds tracepoints at the generic hardirq and serving-softirq context boundaries. It also updates softirq pending snapshots and wakes worker threads when callbacks attached to boundary events raise new work. > diff --git a/kernel/softirq.c b/kernel/softirq.c > index be49e6469f3ad..2e3b6fd3f6d5d 100644 > --- a/kernel/softirq.c > +++ b/kernel/softirq.c [ ... ] > @@ -722,6 +770,7 @@ void irq_enter_rcu(void) > tick_irq_enter(); > > account_hardirq_enter(current); > + trace_hardirq_context_entry(hardirq_count() >> HARDIRQ_SHIFT); > } > > /** > @@ -796,6 +845,36 @@ static inline void __irq_exit_rcu(void) > wake_timersd(); > > tick_irq_exit(); > + if (trace_hardirq_context_exit_enabled()) { > + bool ksoftirqd_wake_requested = false; > + bool timersd_wake_requested = false; > + > + trace_hardirq_context_exit(hardirq_count() >> HARDIRQ_SHIFT); > + > + /* Hand work raised by the final boundary event to its worker. > */ > + if (hardirq_count() == HARDIRQ_OFFSET) { > + if (irq_count() == HARDIRQ_OFFSET && > + unlikely(local_softirq_pending()) && > + should_wake_ksoftirqd()) { > + wakeup_softirqd(); > + ksoftirqd_wake_requested = true; > + } > + > + if (IS_ENABLED(CONFIG_IRQ_FORCED_THREADING) && > + force_irqthreads() && > local_timers_pending_force_th()) { > + wake_timersd(); > + timersd_wake_requested = true; > + } > + > + /* The timersd wakeup can itself raise a softirq. */ > + if (timersd_wake_requested && > + !ksoftirqd_wake_requested && > + irq_count() == HARDIRQ_OFFSET && > + unlikely(local_softirq_pending()) && > + should_wake_ksoftirqd()) > + wakeup_softirqd(); > + } > + } > preempt_count_sub(HARDIRQ_OFFSET); > } [Severity: Medium] Should tick_irq_exit() be called after the trace_hardirq_context_exit() block rather than before it? In irq_enter_rcu(), tick_irq_enter() runs before trace_hardirq_context_entry() so that the nohz clock wakeup and idle cputime accounting state are established before the entry event fires. In __irq_exit_rcu(), however, tick_irq_exit() runs before trace_hardirq_context_exit() and the subsequent wakeup_softirqd() and wake_timersd() calls: kernel/softirq.c:tick_irq_exit() { if ((sched_core_idle_cpu(cpu) && !need_resched()) || tick_nohz_full_cpu(cpu)) { if (hardirq_count() == HARDIRQ_OFFSET) tick_nohz_irq_exit(); } } kernel/time/tick-sched.c:tick_nohz_irq_exit() { struct tick_sched *ts = this_cpu_ptr(&tick_cpu_sched); if (tick_sched_flag_test(ts, TS_FLAG_INIDLE)) { tick_nohz_clock_sleep(ts); ts->idle_entrytime = ktime_get(); if (tick_sched_flag_test(ts, TS_FLAG_STOPPED)) kcpustat_irq_exit(ts->idle_entrytime); } else { tick_nohz_full_update_tick(ts); } } On an idle cpu where need_resched() is false before the tracepoint, tick_irq_exit() calls tick_nohz_irq_exit(), which places sched_clock and kcpustat back into idle sleep via tick_nohz_clock_sleep() and kcpustat_irq_exit(). If the hardirq_context_exit callback then raises work and wakes ksoftirqd or ktimerd, try_to_wake_up() reads sched_clock_cpu() and sets need_resched after the idle clock and cputime state have already gone to sleep. Similarly, on a nohz_full cpu, tick_nohz_irq_exit() calls tick_nohz_full_update_tick() to evaluate and potentially stop the tick before trace_hardirq_context_exit() runs. Any timer, softirq, rcu, or irq_work state raised by the exit callback is missed by that evaluation, or forces an extra self-ipi when waking ksoftirqd after the tick was just stopped. Would moving tick_irq_exit() to just after the trace_hardirq_context_exit_enabled() block (while hardirq_count() is still equal to the outermost offset) avoid this regression and keep entry and exit symmetric? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
