Re: [PATCH v3 11/17] rv: Prevent unintentional tracepoints during KUnit tests

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

On 6/25/26 20:14, Gabriele Monaco wrote:
> Monitor initialisation also called during KUnit tests may register some
> tracepoints, this can lead to issues since we don't expect real monitor
> events running during KUnit tests.
> 
> Prevent tracepoint registration if an RV KUnit test is running.
> 
> Reviewed-by: Nam Cao <namcao@linutronix.de>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
> ---
>   include/rv/instrumentation.h |  5 +++++
>   include/rv/kunit.h           |  2 ++
>   kernel/trace/rv/rv.c         | 11 +++++++++++
>   3 files changed, 18 insertions(+)
> 
> diff --git a/include/rv/instrumentation.h b/include/rv/instrumentation.h
> index d4e7a02ede..761f8f147d 100644
> --- a/include/rv/instrumentation.h
> +++ b/include/rv/instrumentation.h
> @@ -9,12 +9,15 @@
>    */
>   
>   #include <linux/ftrace.h>
> +#include <rv/kunit.h>
>   
>   /*
>    * rv_attach_trace_probe - check and attach a handler function to a tracepoint
>    */
>   #define rv_attach_trace_probe(monitor, tp, rv_handler)					\
>   	do {										\
> +		if (rv_mon_test_is_running())						\
> +			break;								\
>   		check_trace_callback_type_##tp(rv_handler);				\
>   		WARN_ONCE(register_trace_##tp(rv_handler, NULL),			\
>   				"fail attaching " #monitor " " #tp "handler");		\
> @@ -25,5 +28,7 @@
>    */
>   #define rv_detach_trace_probe(monitor, tp, rv_handler)					\
>   	do {										\
> +		if (rv_mon_test_is_running())						\
> +			break;								\
>   		unregister_trace_##tp(rv_handler, NULL);				\
>   	} while (0)
> diff --git a/include/rv/kunit.h b/include/rv/kunit.h
> index 5e8c604cf7..0c4c4bc933 100644
> --- a/include/rv/kunit.h
> +++ b/include/rv/kunit.h
> @@ -19,6 +19,7 @@
>   
>   int rv_set_testing(struct kunit_suite *suite);
>   void rv_clear_testing(struct kunit_suite *suite);
> +bool rv_mon_test_is_running(void);
>   struct task_struct *rv_get_current(void);
>   
>   struct rv_kunit_ctx {
> @@ -59,6 +60,7 @@ void teardown_test(void *arg);
>   #else /* !CONFIG_RV_MONITORS_KUNIT_TEST */
>   
>   #define rv_get_current() current
> +#define rv_mon_test_is_running() false
>   
>   #endif /* CONFIG_RV_MONITORS_KUNIT_TEST */
>   #endif /* _RV_KUNIT_H */
> diff --git a/kernel/trace/rv/rv.c b/kernel/trace/rv/rv.c
> index 61d4f13960..86ef5dee7f 100644
> --- a/kernel/trace/rv/rv.c
> +++ b/kernel/trace/rv/rv.c
> @@ -864,6 +864,14 @@ int __init rv_init_interface(void)
>   #include <rv/kunit.h>
>   #include <kunit/visibility.h>
>   
> +static bool rv_mon_test_running;
> +
> +bool rv_mon_test_is_running(void)
> +{
> +	return rv_mon_test_running;
> +}
> +EXPORT_SYMBOL_GPL(rv_mon_test_is_running);
> +

The flag rv_mon_test_running is a plain bool with no ordering
guarantees, and the tracepoint handlers that call 
rv_mon_test_is_running() may execute on
arbitrary CPUs. A store to rv_mon_test_running on the test CPU may not
be visible to another CPU's tracepoint handler.
What abbout READ_ONCE/WRITE_ONCE?

--
Best wishes,
Wen




>   /*
>    * rv_set_testing - ensure mutual exclusion between KUnit tests and real monitors
>    *
> @@ -886,6 +894,8 @@ int rv_set_testing(struct kunit_suite *suite)
>   		}
>   	}
>   
> +	rv_mon_test_running = true;
> +
>   	return 0;
>   }
>   EXPORT_SYMBOL_IF_KUNIT(rv_set_testing);
> @@ -895,6 +905,7 @@ EXPORT_SYMBOL_IF_KUNIT(rv_set_testing);
>    */
>   void rv_clear_testing(struct kunit_suite *suite)
>   {
> +	rv_mon_test_running = false;
>   	mutex_unlock(&rv_interface_lock);
>   }
>   EXPORT_SYMBOL_IF_KUNIT(rv_clear_testing);