On Fri 2025-04-11 09:37:19, Nam Cao wrote: > Each RV monitor has one static buffer to send to the reactors. If multiple > errors are detected simultaneously, the one buffer could be overwritten. > > Instead, leave it to the reactors to handle buffering. > > include/linux/panic.h | 3 +++ > include/linux/printk.h | 5 ++++ > include/linux/rv.h | 9 +++++-- > include/rv/da_monitor.h | 45 +++++++------------------------- > kernel/panic.c | 17 ++++++++---- > kernel/printk/internal.h | 1 - > kernel/trace/rv/reactor_panic.c | 8 ++++-- > kernel/trace/rv/reactor_printk.c | 8 ++++-- > kernel/trace/rv/rv_reactors.c | 2 +- > 9 files changed, 50 insertions(+), 48 deletions(-)
For the changes in the printk and panic code: Reviewed-by: Petr Mladek <[email protected]> # printk, panic I have just briefly looked at the changes in the rv code. I wonder if a __printf(1, 2) declaration might be needed in the printk and panic reactors code, see below. > --- a/include/linux/rv.h > +++ b/include/linux/rv.h > @@ -38,7 +38,7 @@ union rv_task_monitor { > struct rv_reactor { > const char *name; > const char *description; > - void (*react)(char *msg); > + __printf(1, 2) void (*react)(const char *msg, ...); > }; > #endif > > @@ -50,7 +50,7 @@ struct rv_monitor { > void (*disable)(void); > void (*reset)(void); > #ifdef CONFIG_RV_REACTORS > - void (*react)(char *msg); > + __printf(1, 2) void (*react)(const char *msg, ...); > #endif > }; > > --- a/kernel/trace/rv/reactor_printk.c > +++ b/kernel/trace/rv/reactor_printk.c > @@ -12,9 +12,13 @@ > #include <linux/init.h> > #include <linux/rv.h> > > -static void rv_printk_reaction(char *msg) > +static void rv_printk_reaction(const char *msg, ...) I wonder whether "make W=1 kernel/trace/rv/reactor_printk.o" would start complaining about that this function is a candidate for ‘gnu_printf’ format attribute. I am not sure. Maybe it is enough that this function is later assigned to the .react callback in struct rv_reactor. I wanted to tried it myself. But I was not able to compile the code in linux-next. I got something like: ./include/linux/rv.h: In function ‘rv_ltl_valid_state’: ./include/linux/rv.h:55:43: error: ‘struct ltl_monitor’ has no member named ‘states’ 55 | for (int i = 0; i < ARRAY_SIZE(mon->states); ++i) { | ^~ ... I am actually not sure against which tree I should apply this patchset. It did apply on linux-next after skipping the 1st patch. But it does not compile there. And there are more conflicts when I tried to apply it on Linus' master. > { > - printk_deferred(msg); > + va_list args; > + > + va_start(args, msg); > + vprintk_deferred(msg, args); > + va_end(args); > } The __printf statement might be missing also in the other two reactors (panic, nop). Best Regards, Petr
