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

[Wen Yang <wen.yang@linux.dev>]

On 7/6/26 22:41, Gabriele Monaco wrote:
> 
> 
> 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?
> 

Thanks,
v2 uses mdelay(5) for the busy-wait. The goal is simply to hold the CPU 
inside rv_react()'s lockdep context; so mdelay() is a pure calibrated 
spin with no scheduler interaction, making the test's causal chain 
clearer than sched_clock().

The remaining comments are also very important, and we will address them 
in v2.

--
Best wishes,
Wen