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

[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 panic reactor covering:
> - Reactor registration and unregistration lifecycle
> - Panic notifier chain reachability
> 
> The real rv_panic_reaction() calls vpanic(), which is __noreturn and
> halts the system. KUnit cannot test across that boundary. Instead, the
> test drives atomic_notifier_call_chain(&panic_notifier_list, ...) directly
> with a high-priority mock notifier that returns NOTIFY_STOP, verifying
> that the panic notification propagates without triggering real handlers
> (kdump, watchdog, reboot).
> 
> The mock notifier busy-waits to simulate real handler execution time
> (e.g., crash_save_vmcoreinfo, emergency_restart preamble) under the
> panic context constraints.
> 
> Signed-off-by: Wen Yang <wen.yang@linux.dev>
> ---
>  kernel/trace/rv/Kconfig               |  10 +++
>  kernel/trace/rv/Makefile              |   1 +
>  kernel/trace/rv/reactor_panic_kunit.c | 106 ++++++++++++++++++++++++++
>  3 files changed, 117 insertions(+)
>  create mode 100644 kernel/trace/rv/reactor_panic_kunit.c
> 
> diff --git a/kernel/trace/rv/Kconfig b/kernel/trace/rv/Kconfig
> index ff47895c897f..6c6c43c5f86c 100644
> --- a/kernel/trace/rv/Kconfig
> +++ b/kernel/trace/rv/Kconfig
> @@ -121,3 +121,13 @@ config RV_REACT_PANIC
>  	help
>  	  Enables the panic reactor. The panic reactor emits a printk()
>  	  message if an exception is found and panic()s the system.
> +
> +config RV_REACT_PANIC_KUNIT
> +	bool "KUnit tests for reactor_panic" if !KUNIT_ALL_TESTS
> +	depends on RV_REACT_PANIC && KUNIT
> +	default KUNIT_ALL_TESTS
> +	help
> +	  This builds KUnit tests for the panic reactor. These are only
> +	  for development and testing, not for regular kernel use cases.
> +
> +	  If unsure, say N.
> diff --git a/kernel/trace/rv/Makefile b/kernel/trace/rv/Makefile
> index ef0a2dcb927c..2ebfe5e5068c 100644
> --- a/kernel/trace/rv/Makefile
> +++ b/kernel/trace/rv/Makefile
> @@ -25,3 +25,4 @@ 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
> +obj-$(CONFIG_RV_REACT_PANIC_KUNIT) += reactor_panic_kunit.o
> diff --git a/kernel/trace/rv/reactor_panic_kunit.c
> b/kernel/trace/rv/reactor_panic_kunit.c
> new file mode 100644
> index 000000000000..f9a09ae7aaad
> --- /dev/null
> +++ b/kernel/trace/rv/reactor_panic_kunit.c
> @@ -0,0 +1,106 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit tests for reactor_panic
> + *
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/rv.h>
> +#include <linux/panic_notifier.h>
> +#include <linux/notifier.h>
> +#include <linux/limits.h>
> +#include <linux/sched/clock.h>
> +#include <linux/processor.h>
> +
> +/* Simulated execution time for mock panic notifier (nanoseconds). */
> +#define RV_PANIC_NOTIFIER_EXEC_NS	2000000ULL
> +
> +/* Test state */
> +static struct {
> +	bool notifier_called;
> +} panic_test_state;

A struct isn't really needed I guess, but good to have a way to test there was a
call.

> +
> +/*
> + * Mock panic notifier callback.
> + *
> + * Runs at INT_MAX priority and returns NOTIFY_STOP to prevent real panic
> + * handlers (kdump, watchdog) from executing during the test.  Busy-waits
> + * RV_PANIC_NOTIFIER_EXEC_NS to simulate a real handler's execution time.
> + */
> +static int mock_panic_notifier_fn(struct notifier_block *nb,
> +				  unsigned long action, void *data)
> +{
> +	char *msg = data;
> +	u64 start = sched_clock();
> +
> +	panic_test_state.notifier_called = true;
> +	pr_emerg("KUnit: reactor_panic test intercepted panic notifier:
> %s\n",
> +		 msg ? msg : "(no message)");

Since we cannot panic(), do we really need to test yet another mock reactor?
What if you put the notifier_called in the other test and drop this test
altogether?

We can simulate what panic() does right now, but it isn't too representative if
this ever changes.

Also it's good to make this reactor test buildable as a module. Currently some
functions aren't exported, feel free to just EXPORT_SYMBOL_GPL() them. There is
no need not to have modular reactors, just nobody did it before.

Thanks,
Gabriele

> +
> +	while (sched_clock() - start < RV_PANIC_NOTIFIER_EXEC_NS)
> +		cpu_relax();
> +
> +	return NOTIFY_STOP;
> +}
> +
> +static struct notifier_block mock_panic_nb = {
> +	.notifier_call	= mock_panic_notifier_fn,
> +	.priority	= INT_MAX,
> +};
> +
> +static struct rv_reactor mock_panic_reactor = {
> +	.name		= "test_panic",
> +	.description	= "test panic reactor",
> +};
> +
> +static int reactor_panic_kunit_init(struct kunit *test)
> +{
> +	panic_test_state.notifier_called = false;
> +	return 0;
> +}
> +
> +/* Test 1: register and unregister reactor */
> +static void test_panic_register_unregister(struct kunit *test)
> +{
> +	int ret;
> +
> +	ret = rv_register_reactor(&mock_panic_reactor);
> +	KUNIT_EXPECT_EQ(test, ret, 0);
> +	KUNIT_EXPECT_STREQ(test, mock_panic_reactor.name, "test_panic");
> +
> +	rv_unregister_reactor(&mock_panic_reactor);
> +}
> +
> +/*
> + * Test 2: panic notifier chain is reachable.
> + *
> + * vpanic() calls atomic_notifier_call_chain(&panic_notifier_list, ...).
> + * Drive the chain directly to verify panic notifiers receive the
> notification —
> + * the observable side-effect of reactor_panic without halting the system.
> + */
> +static void test_panic_notifier_called(struct kunit *test)
> +{
> +	atomic_notifier_chain_register(&panic_notifier_list, &mock_panic_nb);
> +	atomic_notifier_call_chain(&panic_notifier_list, 0,
> +				   "panic violation message");
> +	atomic_notifier_chain_unregister(&panic_notifier_list,
> &mock_panic_nb);
> +
> +	KUNIT_EXPECT_TRUE(test, panic_test_state.notifier_called);
> +}
> +
> +static struct kunit_case reactor_panic_kunit_cases[] = {
> +	KUNIT_CASE(test_panic_register_unregister),
> +	KUNIT_CASE(test_panic_notifier_called),
> +	{}
> +};
> +
> +static struct kunit_suite reactor_panic_kunit_suite = {
> +	.name		= "rv_reactor_panic",
> +	.init		= reactor_panic_kunit_init,
> +	.test_cases	= reactor_panic_kunit_cases,
> +};
> +
> +kunit_test_suite(reactor_panic_kunit_suite);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("KUnit tests for reactor_panic");