On Thu, 2025-03-13 at 09:16 +0100, Nam Cao wrote:
> Hi Gabriele,
>
> On Wed, Mar 12, 2025 at 08:58:56AM +0100, Gabriele Monaco wrote:
> > On Tue, 2025-03-11 at 18:05 +0100, Nam Cao wrote:
> > > diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> > > index 510c88bfabd4..c55d45544a16 100644
> > > --- a/include/rv/da_monitor.h
> > > +++ b/include/rv/da_monitor.h
> > > @@ -16,58 +16,11 @@
> > > #include <linux/bug.h>
> > > #include <linux/sched.h>
> > >
> > > -#ifdef CONFIG_RV_REACTORS
> > > -
> > > -#define DECLARE_RV_REACTING_HELPERS(name,
> > > type) \
> > > -static char
> > > REACT_MSG_##name[1024];
> > > \
> > > -
> > >
> > > \
> > > -static inline char *format_react_msg_##name(type curr_state,
> > > type
> > > event) \
> > > -
> > > {
> > > \
> > > - snprintf(REACT_MSG_##name,
> > > 1024, \
> > > - "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));
> > > \
> > > - return
> > > REACT_MSG_##name;
> > > \
> > > -
> > > }
> > > \
> > > -
> > >
> > > \
> > > -static void cond_react_##name(char
> > > *msg) \
> > > -
> > > {
> > > \
> > > - if
> > > (rv_##name.react)
> > > \
> > > -
> > > rv_##name.react(msg);
> > > \
> > > -
> > > }
> > > \
> > > -
> > >
> > > \
> > > -static bool
> > > rv_reacting_on_##name(void)
> > > \
> > > -
> > > {
> > > \
> > > - return
> > > rv_reacting_on();
> > > \
> > > -}
> > > -
> > > -#else /* CONFIG_RV_REACTOR */
> > > -
> > > -#define DECLARE_RV_REACTING_HELPERS(name,
> > > type) \
> > > -static inline char *format_react_msg_##name(type curr_state,
> > > type
> > > event) \
> > > -
> > > {
> > > \
> > > - return
> > > NULL;
> > > \
> > > -
> > > }
> > > \
> > > -
> > >
> > > \
> > > -static void cond_react_##name(char
> > > *msg) \
> > > -
> > > {
> > > \
> > > -
> > > return;
> > > \
> > > -
> > > }
> > > \
> > > -
> > >
> > > \
> > > -static bool
> > > rv_reacting_on_##name(void)
> > > \
> > > -
> > > {
> > > \
> > > - return
> > > 0;
> > > \
> > > -}
> > > -#endif
> > > -
> >
> > I don't think you need to remove those helper functions, why not
> > just
> > having format_react_msg_ prepare the arguments for react?
>
> I'm not sure what you mean. Making format_react_msg_* a macro that is
> preprocessed into arguments? Then make cond_react_*() a variadic
> function,
> so that we can "pass" format_react_msg_* to it?
>
> Going that way would also need a vreact() variant, as cond_react_*()
> cannot
> pass on the variadic arguments to react().
>
> Instead, is it cleaner to do the below?
Hi Nam,
you're right, I got a bit confused, all I meant was to find a way not
to repeat the arguments for implicit and per-task monitors.
What you propose seems perfect to me.
Also for the sake of simplifying things, a bit like you started, we
could have the reacting_on() check inside cond_react and drop the per-
monitor function. I believe the initial idea was to have a reacting_on
toggle for each monitor but since it isn't the case, we don't really
need it.
Thanks,
Gabriele
>
> diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> index 510c88bfabd4..e185ebf894a4 100644
> --- a/include/rv/da_monitor.h
> +++ b/include/rv/da_monitor.h
> @@ -19,22 +19,14 @@
> #ifdef CONFIG_RV_REACTORS
>
> #define DECLARE_RV_REACTING_HELPERS(name,
> type) \
> -static char
> REACT_MSG_##name[1024];
> \
> -
>
> \
> -static inline char *format_react_msg_##name(type curr_state, type
> event) \
> -
> {
> \
> - snprintf(REACT_MSG_##name,
> 1024, \
> - "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));
> \
> - return
> REACT_MSG_##name;
> \
> -
> }
> \
> -
>
> \
> -static void cond_react_##name(char
> *msg) \
> +static void cond_react_##name(type curr_state, type
> event) \
> {
> \
> - if
> (rv_##name.react)
> \
> -
> rv_##name.react(msg);
> \
> + if
> (!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));
> \
> }
> \
>
> \
> static bool
> rv_reacting_on_##name(void)
> \
> @@ -45,12 +37,7 @@ static bool
> rv_reacting_on_##name(void)
> \
> #else /* CONFIG_RV_REACTOR */
>
> #define DECLARE_RV_REACTING_HELPERS(name,
> type) \
> -static inline char *format_react_msg_##name(type curr_state, type
> event) \
> -
> {
> \
> - return
> NULL;
> \
> -
> }
> \
> -
>
> \
> -static void cond_react_##name(char
> *msg) \
> +static void cond_react_##name(type curr_state, type
> event) \
> {
> \
> return;
> \
> }
> \
> @@ -171,7 +158,7 @@ da_event_##name(struct da_monitor *da_mon, enum
> events_##name event) \
> }
> \
>
> \
> if
> (rv_reacting_on_##name())
> \
> -
> cond_react_##name(format_react_msg_##name(curr_state, event));
> \
> + cond_react_##name(curr_state,
> event); \
>
> \
> trace_error_##name(model_get_state_name_##name(curr_state),
> \
>
> model_get_event_name_##name(event)); \
> @@ -203,7 +190,7 @@ static inline bool da_event_##name(struct
> da_monitor *da_mon, struct task_struct
> }
> \
>
> \
> if
> (rv_reacting_on_##name())
> \
> -
> cond_react_##name(format_react_msg_##name(curr_state, event));
> \
> + cond_react_##name(curr_state,
> event); \
>
> \
> trace_error_##name(tsk-
> >pid, \
>
> model_get_state_name_##name(curr_state), \
>
> Best regards,
> Nam
>
- [PATCH 05/10] x86/tracing: Remove redundant trace_pagefa... Nam Cao
- [PATCH 10/10] rv: raise the number of per-task monitor t... Nam Cao
- Re: [PATCH 10/10] rv: raise the number of per-task ... Gabriele Monaco
- [PATCH 09/10] rv: Add rtapp_pagefault monitor Nam Cao
- [PATCH 04/10] rv: Add rtapp_block monitor Nam Cao
- Re: [PATCH 04/10] rv: Add rtapp_block monitor Gabriele Monaco
- Re: [PATCH 04/10] rv: Add rtapp_block monitor kernel test robot
- [PATCH 02/10] rv: Let the reactors take care of buffers Nam Cao
- Re: [PATCH 02/10] rv: Let the reactors take care of... Gabriele Monaco
- Re: [PATCH 02/10] rv: Let the reactors take car... Nam Cao
- Re: [PATCH 02/10] rv: Let the reactors take... Gabriele Monaco
- Re: [PATCH 02/10] rv: Let the reactors take care of... kernel test robot
- Re: [PATCH 02/10] rv: Let the reactors take care of... kernel test robot
