The only thing the reactors can do with the passed in varargs is to
convert it into a va_list. Do that in a central helper instead.
It simplifies the reactors, removes some hairy macro-generated code
and introduces a convenient hook point to modify reactor behavior.
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
include/linux/rv.h | 11 +++++++++--
include/rv/da_monitor.h | 35 ++++++++++-------------------------
include/rv/ltl_monitor.h | 18 +++++-------------
kernel/trace/rv/reactor_panic.c | 6 +-----
kernel/trace/rv/reactor_printk.c | 6 +-----
kernel/trace/rv/rv_reactors.c | 16 +++++++++++++++-
6 files changed, 41 insertions(+), 51 deletions(-)
diff --git a/include/linux/rv.h b/include/linux/rv.h
index 9520aab34bcbe36c730bc6ab20bed71c8eee52bf..b567b0191e67f7dfab74e2aad6de3ed63d94058d 100644
--- a/include/linux/rv.h
+++ b/include/linux/rv.h
@@ -88,7 +88,7 @@ union rv_task_monitor {
struct rv_reactor {
const char *name;
const char *description;
- __printf(1, 2) void (*react)(const char *msg, ...);
+ __printf(1, 0) void (*react)(const char *msg, va_list args);
struct list_head list;
};
#endif
@@ -102,7 +102,7 @@ struct rv_monitor {
void (*reset)(void);
#ifdef CONFIG_RV_REACTORS
struct rv_reactor *reactor;
- __printf(1, 2) void (*react)(const char *msg, ...);
+ __printf(1, 0) void (*react)(const char *msg, va_list args);
#endif
struct list_head list;
struct rv_monitor *parent;
@@ -119,11 +119,18 @@ void rv_put_task_monitor_slot(int slot);
bool rv_reacting_on(void);
int rv_unregister_reactor(struct rv_reactor *reactor);
int rv_register_reactor(struct rv_reactor *reactor);
+__printf(2, 3)
+void rv_react(struct rv_monitor *monitor, const char *msg, ...);
#else
static inline bool rv_reacting_on(void)
{
return false;
}
+
+__printf(2, 3)
+static inline void rv_react(struct rv_monitor *monitor, const char *msg, ...)
+{
+}
#endif /* CONFIG_RV_REACTORS */
#endif /* CONFIG_RV */
diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index 17fa4f6e5ea63fad29fd4349c54720944a179e38..0cef64366538c90dab8f76fbf5d2aaacdd61e2e7 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -16,34 +16,19 @@
#include <linux/bug.h>
#include <linux/sched.h>
-#ifdef CONFIG_RV_REACTORS
-
-#define DECLARE_RV_REACTING_HELPERS(name, type) \
-static void cond_react_##name(type curr_state, type event) \
-{ \
- if (!rv_reacting_on() || !rv_##name.react) \
- return; \
- rv_##name.react("rv: monitor %s does not allow event %s on state %s\n", \
- #name, \
- model_get_event_name_##name(event), \
- model_get_state_name_##name(curr_state)); \
-}
-
-#else /* CONFIG_RV_REACTOR */
-
-#define DECLARE_RV_REACTING_HELPERS(name, type) \
-static void cond_react_##name(type curr_state, type event) \
-{ \
- return; \
-}
-#endif
-
/*
* Generic helpers for all types of deterministic automata monitors.
*/
#define DECLARE_DA_MON_GENERIC_HELPERS(name, type) \
\
-DECLARE_RV_REACTING_HELPERS(name, type) \
+static void react_##name(type curr_state, type event) \
+{ \
+ rv_react(&rv_##name, \
+ "rv: monitor %s does not allow event %s on state %s\n", \
+ #name, \
+ model_get_event_name_##name(event), \
+ model_get_state_name_##name(curr_state)); \
+} \
\
/* \
* da_monitor_reset_##name - reset a monitor and setting it to init state \
@@ -126,7 +111,7 @@ da_event_##name(struct da_monitor *da_mon, enum events_##name event) \
for (int i = 0; i < MAX_DA_RETRY_RACING_EVENTS; i++) { \
next_state = model_get_next_state_##name(curr_state, event); \
if (next_state == INVALID_STATE) { \
- cond_react_##name(curr_state, event); \
+ react_##name(curr_state, event); \
trace_error_##name(model_get_state_name_##name(curr_state), \
model_get_event_name_##name(event)); \
return false; \
@@ -165,7 +150,7 @@ static inline bool da_event_##name(struct da_monitor *da_mon, struct task_struct
for (int i = 0; i < MAX_DA_RETRY_RACING_EVENTS; i++) { \
next_state = model_get_next_state_##name(curr_state, event); \
if (next_state == INVALID_STATE) { \
- cond_react_##name(curr_state, event); \
+ react_##name(curr_state, event); \
trace_error_##name(tsk->pid, \
model_get_state_name_##name(curr_state), \
model_get_event_name_##name(event)); \
diff --git a/include/rv/ltl_monitor.h b/include/rv/ltl_monitor.h
index 5368cf5fd623e74a5739d2e0b3fc2c27c4bad597..00c42b36f961a00ee473aa58f14b015308523eb0 100644
--- a/include/rv/ltl_monitor.h
+++ b/include/rv/ltl_monitor.h
@@ -16,21 +16,12 @@
#error "Please include $(MODEL_NAME).h generated by rvgen"
#endif
-#ifdef CONFIG_RV_REACTORS
#define RV_MONITOR_NAME CONCATENATE(rv_, MONITOR_NAME)
-static struct rv_monitor RV_MONITOR_NAME;
-static void rv_cond_react(struct task_struct *task)
-{
- if (!rv_reacting_on() || !RV_MONITOR_NAME.react)
- return;
- RV_MONITOR_NAME.react("rv: "__stringify(MONITOR_NAME)": %s[%d]: violation detected\n",
- task->comm, task->pid);
-}
+#ifdef CONFIG_RV_REACTORS
+static struct rv_monitor RV_MONITOR_NAME;
#else
-static void rv_cond_react(struct task_struct *task)
-{
-}
+extern struct rv_monitor RV_MONITOR_NAME;
#endif
static int ltl_monitor_slot = RV_PER_TASK_MONITOR_INIT;
@@ -98,7 +89,8 @@ static void ltl_monitor_destroy(void)
static void ltl_illegal_state(struct task_struct *task, struct ltl_monitor *mon)
{
CONCATENATE(trace_error_, MONITOR_NAME)(task);
- rv_cond_react(task);
+ rv_react(&RV_MONITOR_NAME, "rv: "__stringify(MONITOR_NAME)": %s[%d]: violation detected\n",
+ task->comm, task->pid);
}
static void ltl_attempt_start(struct task_struct *task, struct ltl_monitor *mon)
diff --git a/kernel/trace/rv/reactor_panic.c b/kernel/trace/rv/reactor_panic.c
index 74c6bcc2c7494408770881dda2b0de885c5eb88c..76537b8a4343cbd0d715f60db3cc8868e71c1c0b 100644
--- a/kernel/trace/rv/reactor_panic.c
+++ b/kernel/trace/rv/reactor_panic.c
@@ -13,13 +13,9 @@
#include <linux/init.h>
#include <linux/rv.h>
-__printf(1, 2) static void rv_panic_reaction(const char *msg, ...)
+__printf(1, 0) static void rv_panic_reaction(const char *msg, va_list args)
{
- va_list args;
-
- va_start(args, msg);
vpanic(msg, args);
- va_end(args);
}
static struct rv_reactor rv_panic = {
diff --git a/kernel/trace/rv/reactor_printk.c b/kernel/trace/rv/reactor_printk.c
index 2dae2916c05fd17397195e37d9b42d24cea24b9c..48c934e315b31c14d3a5b4fa3ec334ef71f9e390 100644
--- a/kernel/trace/rv/reactor_printk.c
+++ b/kernel/trace/rv/reactor_printk.c
@@ -12,13 +12,9 @@
#include <linux/init.h>
#include <linux/rv.h>
-__printf(1, 2) static void rv_printk_reaction(const char *msg, ...)
+__printf(1, 0) static void rv_printk_reaction(const char *msg, va_list args)
{
- va_list args;
-
- va_start(args, msg);
vprintk_deferred(msg, args);
- va_end(args);
}
static struct rv_reactor rv_printk = {
diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
index d32859fec238371b5721e08cf6558f0805565f7b..cb1a5968055abb22439a066b4e25dad98f078389 100644
--- a/kernel/trace/rv/rv_reactors.c
+++ b/kernel/trace/rv/rv_reactors.c
@@ -438,7 +438,7 @@ int reactor_populate_monitor(struct rv_monitor *mon)
/*
* Nop reactor register
*/
-__printf(1, 2) static void rv_nop_reaction(const char *msg, ...)
+__printf(1, 0) static void rv_nop_reaction(const char *msg, va_list args)
{
}
@@ -477,3 +477,17 @@ int init_rv_reactors(struct dentry *root_dir)
out_err:
return -ENOMEM;
}
+
+void rv_react(struct rv_monitor *monitor, const char *msg, ...)
+{
+ va_list args;
+
+ if (!rv_reacting_on() || !monitor->react)
+ return;
+
+ va_start(args, msg);
+
+ monitor->react(msg, args);
+
+ va_end(args);
+}
--
2.51.0
On Tue, 2025-10-14 at 07:51 +0200, Thomas Weißschuh wrote:
> The only thing the reactors can do with the passed in varargs is to
> convert it into a va_list. Do that in a central helper instead.
> It simplifies the reactors, removes some hairy macro-generated code
> and introduces a convenient hook point to modify reactor behavior.
>
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
> --- a/include/rv/da_monitor.h
> +++ b/include/rv/da_monitor.h
> @@ -16,34 +16,19 @@
> #include <linux/bug.h>
> #include <linux/sched.h>
>
> -#ifdef CONFIG_RV_REACTORS
> -
> -#define DECLARE_RV_REACTING_HELPERS(name,
> type) \
> -static void cond_react_##name(type curr_state, type
> event) \
> -
> { \
> - if (!rv_reacting_on() ||
> !rv_##name.react) \
> -
> return; \
> - rv_##name.react("rv: monitor %s does not allow event %s on state
> %s\n", \
> -
> #name, \
> -
> model_get_event_name_##name(event), \
> -
> model_get_state_name_##name(curr_state)); \
> -}
> -
The overall change looks good to me, just keep in mind there is an ongoing
refactor [1] for the da_monitor header to get rid of those macros, basically
making it more similar to the current ltl.
Depending on which gets merged first, you may need some (trivial) adaptation of
this.
Going to test it out more carefully in the next days.
Thanks,
Gabriele
[1] - https://lore.kernel.org/lkml/20250919140954.104920-1-gmonaco@redhat.com
> -#else /* CONFIG_RV_REACTOR */
> -
> -#define DECLARE_RV_REACTING_HELPERS(name,
> type) \
> -static void cond_react_##name(type curr_state, type
> event) \
> -
> { \
> -
> return; \
> -}
> -#endif
> -
> /*
> * Generic helpers for all types of deterministic automata monitors.
> */
> #define DECLARE_DA_MON_GENERIC_HELPERS(name,
> type) \
>
> \
> -DECLARE_RV_REACTING_HELPERS(name,
> type) \
> +static void react_##name(type curr_state, type
> event) \
> +{
> \
> + rv_react(&rv_##name,
> \
> + "rv: monitor %s does not allow event %s on state
> %s\n", \
> +
> #name, \
> +
> model_get_event_name_##name(event), \
> +
> model_get_state_name_##name(curr_state)); \
> +}
> \
>
> \
> /*
> \
> * da_monitor_reset_##name - reset a monitor and setting it to init
> state \
> @@ -126,7 +111,7 @@ da_event_##name(struct da_monitor *da_mon, enum
> events_##name event) \
> for (int i = 0; i < MAX_DA_RETRY_RACING_EVENTS; i++)
> { \
> next_state = model_get_next_state_##name(curr_state,
> event); \
> if (next_state == INVALID_STATE)
> { \
> - cond_react_##name(curr_state,
> event); \
> + react_##name(curr_state,
> event); \
> trace_error_##name(model_get_state_name_##name(curr_s
> tate), \
>
> model_get_event_name_##name(event)); \
> return
> false; \
> @@ -165,7 +150,7 @@ static inline bool da_event_##name(struct da_monitor
> *da_mon, struct task_struct
> for (int i = 0; i < MAX_DA_RETRY_RACING_EVENTS; i++)
> { \
> next_state = model_get_next_state_##name(curr_state,
> event); \
> if (next_state == INVALID_STATE)
> { \
> - cond_react_##name(curr_state,
> event); \
> + react_##name(curr_state,
> event); \
> trace_error_##name(tsk-
> >pid, \
>
> model_get_state_name_##name(curr_state), \
>
> model_get_event_name_##name(event)); \
> diff --git a/include/rv/ltl_monitor.h b/include/rv/ltl_monitor.h
> index
> 5368cf5fd623e74a5739d2e0b3fc2c27c4bad597..00c42b36f961a00ee473aa58f14b01530852
> 3eb0 100644
> --- a/include/rv/ltl_monitor.h
> +++ b/include/rv/ltl_monitor.h
> @@ -16,21 +16,12 @@
> #error "Please include $(MODEL_NAME).h generated by rvgen"
> #endif
>
> -#ifdef CONFIG_RV_REACTORS
> #define RV_MONITOR_NAME CONCATENATE(rv_, MONITOR_NAME)
> -static struct rv_monitor RV_MONITOR_NAME;
>
> -static void rv_cond_react(struct task_struct *task)
> -{
> - if (!rv_reacting_on() || !RV_MONITOR_NAME.react)
> - return;
> - RV_MONITOR_NAME.react("rv: "__stringify(MONITOR_NAME)": %s[%d]:
> violation detected\n",
> - task->comm, task->pid);
> -}
> +#ifdef CONFIG_RV_REACTORS
> +static struct rv_monitor RV_MONITOR_NAME;
> #else
> -static void rv_cond_react(struct task_struct *task)
> -{
> -}
> +extern struct rv_monitor RV_MONITOR_NAME;
> #endif
>
> static int ltl_monitor_slot = RV_PER_TASK_MONITOR_INIT;
> @@ -98,7 +89,8 @@ static void ltl_monitor_destroy(void)
> static void ltl_illegal_state(struct task_struct *task, struct ltl_monitor
> *mon)
> {
> CONCATENATE(trace_error_, MONITOR_NAME)(task);
> - rv_cond_react(task);
> + rv_react(&RV_MONITOR_NAME, "rv: "__stringify(MONITOR_NAME)": %s[%d]:
> violation detected\n",
> + task->comm, task->pid);
> }
>
> static void ltl_attempt_start(struct task_struct *task, struct ltl_monitor
> *mon)
> diff --git a/kernel/trace/rv/reactor_panic.c b/kernel/trace/rv/reactor_panic.c
> index
> 74c6bcc2c7494408770881dda2b0de885c5eb88c..76537b8a4343cbd0d715f60db3cc8868e71c
> 1c0b 100644
> --- a/kernel/trace/rv/reactor_panic.c
> +++ b/kernel/trace/rv/reactor_panic.c
> @@ -13,13 +13,9 @@
> #include <linux/init.h>
> #include <linux/rv.h>
>
> -__printf(1, 2) static void rv_panic_reaction(const char *msg, ...)
> +__printf(1, 0) static void rv_panic_reaction(const char *msg, va_list args)
> {
> - va_list args;
> -
> - va_start(args, msg);
> vpanic(msg, args);
> - va_end(args);
> }
>
> static struct rv_reactor rv_panic = {
> diff --git a/kernel/trace/rv/reactor_printk.c
> b/kernel/trace/rv/reactor_printk.c
> index
> 2dae2916c05fd17397195e37d9b42d24cea24b9c..48c934e315b31c14d3a5b4fa3ec334ef71f9
> e390 100644
> --- a/kernel/trace/rv/reactor_printk.c
> +++ b/kernel/trace/rv/reactor_printk.c
> @@ -12,13 +12,9 @@
> #include <linux/init.h>
> #include <linux/rv.h>
>
> -__printf(1, 2) static void rv_printk_reaction(const char *msg, ...)
> +__printf(1, 0) static void rv_printk_reaction(const char *msg, va_list args)
> {
> - va_list args;
> -
> - va_start(args, msg);
> vprintk_deferred(msg, args);
> - va_end(args);
> }
>
> static struct rv_reactor rv_printk = {
> diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
> index
> d32859fec238371b5721e08cf6558f0805565f7b..cb1a5968055abb22439a066b4e25dad98f07
> 8389 100644
> --- a/kernel/trace/rv/rv_reactors.c
> +++ b/kernel/trace/rv/rv_reactors.c
> @@ -438,7 +438,7 @@ int reactor_populate_monitor(struct rv_monitor *mon)
> /*
> * Nop reactor register
> */
> -__printf(1, 2) static void rv_nop_reaction(const char *msg, ...)
> +__printf(1, 0) static void rv_nop_reaction(const char *msg, va_list args)
> {
> }
>
> @@ -477,3 +477,17 @@ int init_rv_reactors(struct dentry *root_dir)
> out_err:
> return -ENOMEM;
> }
> +
> +void rv_react(struct rv_monitor *monitor, const char *msg, ...)
> +{
> + va_list args;
> +
> + if (!rv_reacting_on() || !monitor->react)
> + return;
> +
> + va_start(args, msg);
> +
> + monitor->react(msg, args);
> +
> + va_end(args);
> +}
© 2016 - 2025 Red Hat, Inc.