Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] rv: leaked rq->nr_pinned on assertion abort in rv_test_irqexit_live() --
commit 69d8a9063a6f40de41bbbc2fc85512c36f8a0acb Author: Karl Mehltretter <[email protected]> rv: Add KUnit coverage for IRQ-exit scheduling This commit adds KUnit test cases for the irqexit runtime verification monitor, covering its state transitions, synchronization, and live invocations of preempt_schedule_irq() including task switching and CPU migration. > diff --git a/kernel/trace/rv/monitors/irqexit/irqexit_kunit.c > b/kernel/trace/rv/monitors/irqexit/irqexit_kunit.c > new file mode 100644 > index 0000000000000..56a9442e2f704 > --- /dev/null > +++ b/kernel/trace/rv/monitors/irqexit/irqexit_kunit.c [ ... ] > +static void rv_test_irqexit_live(struct kunit *test) > +{ > + struct rv_monitor *mon = rv_irqexit_ops.mon.rv_this; > + struct rv_kunit_ctx *ctx = test->priv; > + struct irqexit_live_ctx *live = (void *)test->param_value; > + struct task_struct *worker; > + unsigned int entries, exits, switches_out, switches_in; > + unsigned int first_entries, first_switches; > + unsigned long flags; > + bool worker_done; > + > + KUNIT_ASSERT_FALSE(test, mon->enabled); > + live->owner = current; > + get_task_struct(live->owner); > + init_completion(&live->worker_done); > + > + active_ctx = ctx; > + mon->react = rv_kunit_mock_react; > + rv_mock_current(NULL); > + KUNIT_ASSERT_EQ(test, 0, > + kunit_add_action_or_reset(test, irqexit_live_cleanup, > live)); > + /* The worker must run on this CPU inside the explicit scheduling loop. > */ > + guard(migrate)(); > + worker = kthread_create(irqexit_live_worker, live, "rv-irqexit-test"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, worker); > + live->worker = worker; > + kthread_bind(worker, smp_processor_id()); > + sched_set_fifo(worker); > + KUNIT_ASSERT_EQ(test, 0, > + register_trace_event_irqexit(irqexit_observe_event, > live)); > + live->observer_attached = true; > + KUNIT_ASSERT_EQ(test, irqexit_observe_boundaries(live), 0); > + > + mon->enabled = 1; > + > + KUNIT_ASSERT_EQ(test, preempt_count(), 0); [Severity: Medium] Does this leak the per-CPU rq->nr_pinned counter if any of the KUNIT_ASSERT_* checks inside the guard(migrate)() scope fail? In kernel/trace/rv/monitors/irqexit/irqexit_kunit.c, rv_test_irqexit_live() enters guard(migrate)(), which calls migrate_disable() -> __migrate_disable() in include/linux/sched.h: this_rq_pinned()++; p->migration_disabled = 1; If a subsequent assertion fails (such as KUNIT_ASSERT_NOT_ERR_OR_NULL() after kthread_create(), or KUNIT_ASSERT_EQ() after register_trace_event_irqexit(), irqexit_observe_boundaries(), or preempt_count()), KUnit aborts the test case via __kunit_abort() -> kunit_try_catch_throw() in lib/kunit/try-catch.c: void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch) { try_catch->try_result = -EFAULT; kthread_exit(0); } Because kthread_exit(0) terminates the KUnit runner kthread directly via do_exit(0) without unwinding the C stack, the scope cleanup destructor for guard(migrate)() (__migrate_enable()) never runs. Neither kthread_exit() nor do_exit() decrements this_rq_pinned(), leaving rq->nr_pinned permanently elevated on that CPU and causing any later CPU hotplug offline operation to hang in balance_hotplug_wait() waiting for !rq_has_pinned_tasks(rq). Could migration disabling be narrowed with scoped_guard(migrate) or explicit migrate_disable() / migrate_enable() around the non-asserting section, or cleaned up via a KUnit action when current->migration_disabled is set? [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
