Re: [PATCH 2/3] rv/reactors: add KUnit tests for reactor_printk

[Gabriele Monaco <gmonaco@redhat.com>]

On Tue, 2026-06-16 at 00:44 +0800, wen.yang@linux.dev wrote:
> From: Wen Yang <wen.yang@linux.dev>
> 
> Add KUnit tests for the printk reactor covering:
> - Reactor registration and unregistration lifecycle
> - React callback invocation via rv_react()
> - Double registration rejection
> - Multiple register/unregister cycles
> 
> The mock callback calls vprintk_deferred() — the same path as the real
> reactor — then busy-waits to simulate I/O back-pressure, exercising the
> LD_WAIT_FREE constraint of rv_react() under load.
> 
> Signed-off-by: Wen Yang <wen.yang@linux.dev>
> ---
>  kernel/trace/rv/Kconfig                |  10 ++
>  kernel/trace/rv/Makefile               |   1 +
>  kernel/trace/rv/reactor_printk_kunit.c | 123 +++++++++++++++++++++++++
>  3 files changed, 134 insertions(+)
>  create mode 100644 kernel/trace/rv/reactor_printk_kunit.c
> 
> diff --git a/kernel/trace/rv/Kconfig b/kernel/trace/rv/Kconfig
> index 3884b14df375..ff47895c897f 100644
> --- a/kernel/trace/rv/Kconfig
> +++ b/kernel/trace/rv/Kconfig
> @@ -104,6 +104,16 @@ config RV_REACT_PRINTK
>  	  Enables the printk reactor. The printk reactor emits a printk()
>  	  message if an exception is found.
>  
> +config RV_REACT_PRINTK_KUNIT
> +	bool "KUnit tests for reactor_printk" if !KUNIT_ALL_TESTS
> +	depends on RV_REACT_PRINTK && KUNIT
> +	default KUNIT_ALL_TESTS
> +	help
> +	  This builds KUnit tests for the printk reactor. These are only
> +	  for development and testing, not for regular kernel use cases.
> +
> +	  If unsure, say N.
> +
>  config RV_REACT_PANIC
>  	bool "Panic reactor"
>  	depends on RV_REACTORS
> diff --git a/kernel/trace/rv/Makefile b/kernel/trace/rv/Makefile
> index 94498da35b37..ef0a2dcb927c 100644
> --- a/kernel/trace/rv/Makefile
> +++ b/kernel/trace/rv/Makefile
> @@ -23,4 +23,5 @@ obj-$(CONFIG_RV_MON_NOMISS) += monitors/nomiss/nomiss.o
>  # Add new monitors here
>  obj-$(CONFIG_RV_REACTORS) += rv_reactors.o
>  obj-$(CONFIG_RV_REACT_PRINTK) += reactor_printk.o
> +obj-$(CONFIG_RV_REACT_PRINTK_KUNIT) += reactor_printk_kunit.o
>  obj-$(CONFIG_RV_REACT_PANIC) += reactor_panic.o
> diff --git a/kernel/trace/rv/reactor_printk_kunit.c
> b/kernel/trace/rv/reactor_printk_kunit.c
> new file mode 100644
> index 000000000000..933aa5602226
> --- /dev/null
> +++ b/kernel/trace/rv/reactor_printk_kunit.c
> @@ -0,0 +1,123 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit tests for reactor_printk
> + *
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/rv.h>
> +#include <linux/printk.h>
> +#include <linux/sched/clock.h>
> +#include <linux/processor.h>
> +
> +/*
> + * Simulated execution time for mock_printk_react (sched_clock units,
> + * nanoseconds).  Models the time a real printk reactor callback may consume
> + * under I/O pressure, exercising the LD_WAIT_FREE constraint of rv_react().
> + */
> +#define MOCK_REACT_DURATION_NS	5000000ULL
> +
> +/*
> + * Mock react callback mirroring rv_printk_reaction().
> + *
> + * Calls vprintk_deferred() — the same path as the real reactor — then holds
> + * the CPU for MOCK_REACT_DURATION_NS via a sched_clock() timed busy-loop,
> + * simulating a callback that is slow due to I/O back-pressure.
> + * sched_clock() is notrace and lock-free; no sleep or lock acquisition is
> + * performed, satisfying the LD_WAIT_FREE constraint of rv_react().
> + */
> +__printf(1, 0) static void mock_printk_react(const char *msg, va_list args)
> +{
> +	u64 start = sched_clock();
> +

I'm fine testing something that looks like the printk reactor rather than the
real thing, but here sched_clock() is playing with preemption and could trigger
one, this isn't accurate.

I'm not sure all implementations are free from this problem, but why don't you
just use mdelay() here?

> +	vprintk_deferred(msg, args);
> +
> +	while (sched_clock() - start < MOCK_REACT_DURATION_NS)
> +		cpu_relax();
> +}
> +
> +static struct rv_reactor mock_printk_reactor = {
> +	.name		= "test_printk",
> +	.description	= "test printk reactor",
> +	.react		= mock_printk_react,
> +};
> +
> +/* Test 1: register and unregister reactor */
> +static void test_printk_register_unregister(struct kunit *test)
> +{
> +	int ret;
> +
> +	ret = rv_register_reactor(&mock_printk_reactor);
> +	KUNIT_EXPECT_EQ(test, ret, 0);

Yeah checking the return value is the maximum we can do without exporting
internal static functions. I'd say we don't need to be too pedantic on that.

> +	KUNIT_EXPECT_STREQ(test, mock_printk_reactor.name, "test_printk");
> +
> +	rv_unregister_reactor(&mock_printk_reactor);
> +}
> +
> +/* Test 2: react callback is invoked via rv_react() */
> +static void test_printk_react_called(struct kunit *test)
> +{
> +	struct rv_reactor reactor = {
> +		.name	= "printk_cb_test",
> +		.react	= mock_printk_react,
> +	};
> +	struct rv_monitor monitor = {
> +		.name	= "test_monitor",
> +		.reactor = &reactor,
> +		.react	= mock_printk_react,
> +	};
> +
> +	rv_react(&monitor, "printk violation message");
> +}
> +
> +/* Test 3: double registration should fail */
> +static void test_printk_double_register(struct kunit *test)
> +{
> +	int ret;
> +
> +	ret = rv_register_reactor(&mock_printk_reactor);
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +
> +	ret = rv_register_reactor(&mock_printk_reactor);
> +	KUNIT_EXPECT_NE(test, ret, 0);
> +
> +	rv_unregister_reactor(&mock_printk_reactor);
> +}
> +
> +/* Test 4: register/unregister cycle */
> +static void test_printk_register_cycle(struct kunit *test)
> +{
> +	int ret, i;
> +
> +	for (i = 0; i < 5; i++) {
> +		ret = rv_register_reactor(&mock_printk_reactor);
> +		KUNIT_EXPECT_EQ(test, ret, 0);
> +
> +		rv_unregister_reactor(&mock_printk_reactor);
> +	}
> +}
> +
> +/* Test 5: react callback is not NULL (printk reactors must provide react) */
> +static void test_printk_react_not_null(struct kunit *test)
> +{
> +	KUNIT_EXPECT_NOT_NULL(test, mock_printk_reactor.react);

As others pointed out, those assertions on a variable you control aren't needed.

Thanks,
Gabriele

> +}
> +
> +static struct kunit_case reactor_printk_kunit_cases[] = {
> +	KUNIT_CASE(test_printk_register_unregister),
> +	KUNIT_CASE(test_printk_react_called),
> +	KUNIT_CASE(test_printk_double_register),
> +	KUNIT_CASE(test_printk_register_cycle),
> +	KUNIT_CASE(test_printk_react_not_null),
> +	{}
> +};
> +
> +static struct kunit_suite reactor_printk_kunit_suite = {
> +	.name		= "rv_reactor_printk",
> +	.test_cases	= reactor_printk_kunit_cases,
> +};
> +
> +kunit_test_suite(reactor_printk_kunit_suite);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("KUnit tests for reactor_printk");