Re: [PATCH v5 13/17] rv: Add KUnit mock for current
Wen Yang <[email protected]> Sun, 2 Aug 2026 12:55:52 +0800
| Newsgroups | org.kernel.vger.linux-trace-kernel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 7/30/26 13:13, Gabriele Monaco wrote: > > > Il 29 luglio 2026 18:17:31 UTC, Wen Yang <[email protected]> ha scritto: > >>> +#define rv_get_current() (unlikely(kunit_get_current_test()) ? rv_get_mock_current() : current) > > ... > >>> +/* >>> + * rv_get_mock_current() is called only if we are running from a KUnit test. >>> + * This can occur from a legitimate RV test or any unrelated test running when >>> + * a real RV monitor is active and triggering events. >>> + * We assume the former case is the only one where mock_current is not NULL and >>> + * can occur only sequentially (KUnit doesn't run tests in parallel). >>> + * We cannot rely on the test's context because there is no way to safely >>> + * understand from which test we are running and KUnit utilities require >>> + * locking, which is unsafe from NMI or scheduling context. >>> + * Note that it is not possible for a real RV monitor to run when the RV KUnit >>> + * tests are running (see rv_set_testing()). >>> + */ >>> +static struct task_struct *mock_current; >>> + >>> +void rv_mock_current(struct task_struct *tsk) >>> +{ >>> + mock_current = tsk; >>> +} >>> +EXPORT_SYMBOL_IF_KUNIT(rv_mock_current); >>> + >>> +struct task_struct *rv_get_mock_current(void) >>> +{ >>> + return mock_current ?: current; >>> +} >>> +EXPORT_SYMBOL_GPL(rv_get_mock_current); >>> #endif >> >> rv_mock_current() uses EXPORT_SYMBOL_IF_KUNIT, but rv_get_mock_current() uses EXPORT_SYMBOL_GPL. Both are defined inside the same CONFIG_RV_MONITORS_KUNIT_TEST block, so rv_get_mock_current should use EXPORT_SYMBOL_IF_KUNIT as well, otherwise it leaks a test-only symbol into production builds. >> >> With that fixed: >> Reviewed-by: Wen Yang <[email protected]> > > Thanks for the review. > This was intentional however: rv_get_current() can be called by any monitor, those don't have to be KUnit. > Since rv_get_current() is a macro also calling rv_get_mock_current() we need to be able to link that too. > > The idea is that a "real" (non-kunit) monitor handler could be run when interrupting a KUnit test (not an RV one, we make sure of that). In that case we do call rv_get_mock_current() and return current after the function call. > > rv_mock_current() CANNOT be called outside of the RV KUnit test cases, it uses a global variable (for problems I tried to explain in the comment), so should be exported only to KUnit and called directly from the test case. > > Does it make sense to you? > Thanks for the explanation. That does make sense. One small thought though -- the kernel already provides KUNIT_STATIC_STUB_REDIRECT as the idiomatic way to handle this kind of pattern. It allows a production function to be transparently redirected to a stub during KUnit tests, with zero overhead when no test is running (since it uses the same static_branch_unlikely(&kunit_running) path as kunit_get_current_test()). This might be a cleaner fit here, since it avoids exporting rv_mock_current() beyond KUnit and keeps the redirection logic internal to the test harness. Just wanted to throw that out there -- what do you think? diff --git a/kernel/trace/rv/rv_monitors_test.c b/kernel/trace/rv/rv_monitors_test.c index 8026176eee90..f4e3e2e4a6d4 100644 --- a/kernel/trace/rv/rv_monitors_test.c +++ b/kernel/trace/rv/rv_monitors_test.c ... +struct task_struct *rv_current(void) +{ + KUNIT_STATIC_STUB_REDIRECT(rv_current); + return current; +} +EXPORT_SYMBOL_GPL(rv_current); ... diff --git a/kernel/trace/rv/rv_monitors_test.c b/kernel/trace/rv/rv_monitors_test.c index 8026176eee90..f4e3e2e4a6d4 100644 --- a/kernel/trace/rv/rv_monitors_test.c +++ b/kernel/trace/rv/rv_monitors_test.c ... +static struct task_struct *mock_current_task; + +static struct task_struct *rv_mock_current_fn(void) +{ + return mock_current_task ?: current; +} + +void rv_mock_current(struct kunit *test, struct task_struct *tsk) +{ + mock_current_task = tsk; + if (tsk) + kunit_activate_static_stub(test, rv_current, rv_mock_current_fn); + else + kunit_deactivate_static_stub(test, rv_current); +} +EXPORT_SYMBOL_IF_KUNIT(rv_mock_current); diff --git a/kernel/trace/rv/monitors/pagefault/pagefault_kunit.c b/kernel/trace/rv/monitors/pagefault/pagefault_kunit.c index unchanged..unchanged 100644 --- a/kernel/trace/rv/monitors/pagefault/pagefault_kunit.c +++ b/kernel/trace/rv/monitors/pagefault/pagefault_kunit.c @@ -nn,7 +nn,7 @@ static void rv_test_pagefault(struct kunit *test) rv_pagefault_ops.handle_task_newtask(NULL, target, 0); - rv_mock_current(target); + rv_mock_current(test, target); -- Best wishes, Wen