Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] The test `test_react_callback_invoked` spuriously fails if the global reactor dispatch is disabled via sysfs. - [Medium] The 5ms busy-wait in `mock_react` is mathematically insufficient to guarantee a timer interrupt on systems with HZ < 200, making the intended lockdep coverage flaky. --
commit 2f80d797b5152331b6b88a29924dc870e4e2b6a0 Author: Wen Yang <[email protected]> rv/reactors: add KUnit tests for reactor registration and dispatch This commit adds KUnit tests for the RV reactor register/unregister lifecycle. It includes coverage for duplicate registration, name-length rejection, and verifying that rv_react() correctly dispatches to the callback. > diff --git a/kernel/trace/rv/rv_reactors_kunit.c > b/kernel/trace/rv/rv_reactors_kunit.c > new file mode 100644 > index 0000000000000..2ccbe55ce4087 > --- /dev/null > +++ b/kernel/trace/rv/rv_reactors_kunit.c [ ... ] > +__printf(1, 0) static void mock_react(const char *msg, va_list args) > +{ > + react_call_count++; > + /* Busy-wait so a timer interrupt fires inside rv_react(). */ > + mdelay(5); [Severity: Medium] Will this 5ms busy-wait reliably trigger a timer interrupt on systems where HZ < 200? On systems configured with HZ=100, where ticks occur every 10ms, a 5ms delay will only span a timer tick around half the time. Can this lead to flaky test coverage when trying to exercise the LD_WAIT_SPIN lockdep wait context? > +} [ ... ] > +static void test_react_callback_invoked(struct kunit *test) > +{ > + struct rv_monitor monitor = { > + .name = "kunit_dispatch_monitor", > + .react = mock_react, > + }; > + > + react_call_count = 0; > + rv_react(&monitor, "callback invocation test"); > + KUNIT_EXPECT_EQ(test, react_call_count, 1); [Severity: Medium] Can this assertion spuriously fail if the global reacting_on state is disabled? If the test executes while the global state is disabled, such as via /sys/kernel/tracing/rv/reacting_on, rv_react() short-circuits: kernel/trace/rv/rv_reactors.c:rv_react() { ... if (!rv_reacting_on() || !monitor->react) return; ... } Will this cause the test to fail by not invoking the callback at all? > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
