Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] rv: kunit: rq->nr_pinned leak on kunit_skip() under guard(migrate)() --
commit 3183a8d5a8d274f51929b98832f2ad4013dd57a7 Author: Karl Mehltretter <[email protected]> rv: Add KUnit coverage for IRQ context transitions This commit adds KUnit test cases for the irqstate runtime verification monitor covering generated state tables, invalid transitions, enable-time synchronization, nested hardirqs, shared IRQ actions, PREEMPT_RT serving intervals, and live tracepoint wiring. > diff --git a/kernel/trace/rv/monitors/irqstate/irqstate_kunit.c > b/kernel/trace/rv/monitors/irqstate/irqstate_kunit.c > new file mode 100644 > index 0000000000000..c5b8834fea3d9 > --- /dev/null > +++ b/kernel/trace/rv/monitors/irqstate/irqstate_kunit.c > @@ -0,0 +1,1384 @@ [ ... ] > +/* Exercise non-RT inline softirq processing from a real hardirq exit. */ > +static void rv_test_irqstate_irq_exit_inline(struct kunit *test) > +{ [ ... ] > + KUNIT_ASSERT_FALSE(test, in_interrupt()); > + KUNIT_ASSERT_FALSE(test, irqs_disabled()); > + KUNIT_ASSERT_EQ(test, softirq_count(), 0U); > + guard(migrate)(); > + inline_ctx->owner = current; > + inline_ctx->owner_cpu = raw_smp_processor_id(); > + inline_ctx->softirqd = this_cpu_ksoftirqd(); > + KUNIT_ASSERT_NOT_NULL(test, inline_ctx->softirqd); > + if (local_softirq_pending() || > + !wait_task_inactive(inline_ctx->softirqd, TASK_INTERRUPTIBLE)) { > + kunit_skip(test, "Requires an idle local ksoftirqd"); > + return; > + } > + saved_count = preempt_count(); > + saved_softirq = softirq_count(); > + local_irq_save(flags); > + if (local_softirq_pending() || task_is_running(inline_ctx->softirqd)) { > + local_irq_restore(flags); > + kunit_skip(test, "Local ksoftirqd stopped being idle"); > + return; > + } > + WRITE_ONCE(inline_ctx->armed, true); > + WRITE_ONCE(inline_ctx->raise_on_exit, true); > + irq_enter_rcu(); > + tasklet_schedule(&inline_ctx->tasklet); > + irq_exit_rcu(); > + ran_inline = inline_ctx->tasklet_runs != 0; > + inline_ctx->exit_tasklet_pending = > + !!(local_softirq_pending() & BIT(TASKLET_SOFTIRQ)); > + /* > + * A zero return means the exit handoff already made ksoftirqd runnable. > + * A one return also makes the failure path safe to drain. > + */ > + inline_ctx->softirqd_already_runnable = > + !wake_up_process(inline_ctx->softirqd); > + WRITE_ONCE(inline_ctx->armed, false); > + local_irq_restore(flags); > + > + /* Drain a tasklet diverted to ksoftirqd before inspecting or skipping. > */ > + kunit_release_action(test, irqstate_inline_bh_kill_all, inline_ctx); > + if (!ran_inline) { > + kunit_skip(test, "IRQ exit diverted pending softirq to > ksoftirqd"); > + return; > + } [Severity: High] Does calling kunit_skip() or KUNIT_ASSERT_*() while guard(migrate)() is active in kernel/trace/rv/monitors/irqstate/irqstate_kunit.c leak the runqueue nr_pinned counter? In rv_test_irqstate_irq_exit_inline(), guard(migrate)() invokes migrate_disable() -> __migrate_disable() in include/linux/sched.h: guard(preempt)(); this_rq_pinned()++; p->migration_disabled = 1; Inside that scope, rv_test_irqstate_irq_exit_inline() calls kunit_skip() when local_softirq_pending() is non-zero, when ksoftirqd is not idle, or when !ran_inline (for example, when forced IRQ threading diverts the tasklet to ksoftirqd). However, kunit_skip() in include/kunit/test.h is: #define kunit_skip(test, fmt, ...) \ do { \ kunit_mark_skipped((test), fmt, ##__VA_ARGS__); \ kunit_try_catch_throw(&((test)->try_catch)); \ } while (0) Both kunit_skip() and failed KUNIT_ASSERT_*() macros call kunit_try_catch_throw(), which terminates the KUnit runner kthread directly via kthread_exit(0) -> do_exit(0) without returning to the caller or unwinding the C stack. Because scope-based cleanup destructors do not run when kthread_exit(0) exits the thread, migrate_enable() -> __migrate_enable() never executes to decrement this_rq_pinned() (rq->nr_pinned). Wouldn't that leave rq_has_pinned_tasks(rq) permanently true on that CPU and cause subsequent CPU hotplug offline operations to hang in balance_hotplug_wait()? The same issue also applies if any KUNIT_ASSERT_*() fails while guard(migrate)() is held in rv_test_irqstate(), rv_test_irqstate_shared_actions(), rv_test_irqstate_sync(), rv_test_irqstate_irq_exit_inline(), rv_test_irqstate_preempt(), rv_test_irqstate_rt_prepare_irq(), and rv_test_irqstate_rt_prepare_switch(). Could migration disabling be managed with a kunit_add_action_or_reset() cleanup action, or released before calling kunit_skip() and KUNIT_ASSERT_*() macros? [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
