On Mon, Jul 25, 2022 at 10:11:16PM +0200, Daniel Bristot de Oliveira wrote:
> +/*
> + * Event handler for per_task monitors.
> + */
> +#define DECLARE_DA_MON_MODEL_HANDLER_PER_TASK(name, type) \
> + \
> +static inline type da_event_##name(struct da_monitor *da_mon, struct task_struct *tsk, \
Not sure here the type `type` why not the bool. The return value is ture/false.
I checked the caller of this function, the return value is stored on `int`.
> + enum events_##name event) \
> +{ \
> + type curr_state = da_monitor_curr_state_##name(da_mon); \
> + type next_state = model_get_next_state_##name(curr_state, event); \
> + \
> + if (next_state != INVALID_STATE) { \
> + da_monitor_set_state_##name(da_mon, next_state); \
> + \
> + trace_event_##name(tsk->pid, \
> + model_get_state_name_##name(curr_state), \
> + model_get_event_name_##name(event), \
> + model_get_state_name_##name(next_state), \
> + model_is_final_state_##name(next_state)); \
> + \
> + return true; \
> + } \
> + \
> + if (rv_reacting_on_##name()) \
> + cond_react_##name(format_react_msg_##name(curr_state, event)); \
> + \
> + trace_error_##name(tsk->pid, \
> + model_get_state_name_##name(curr_state), \
> + model_get_event_name_##name(event)); \
> + \
> + return false; \
> +}
[snip]
> +/*
> + * Handle event for implicit monitor: da_get_monitor_##name() will figure out
> + * the monitor.
> + */
> +#define DECLARE_DA_MON_MONITOR_HANDLER_IMPLICIT(name, type) \
> + \
> +static inline void __da_handle_event_##name(struct da_monitor *da_mon, \
> + enum events_##name event) \
> +{ \
> + int retval; \
> + \
> + retval = da_monitor_handling_event_##name(da_mon); \
> + if (!retval) \
> + return; \
I checked the callers of __da_handle_event_##name():
da_handle_event_##name() for all cases need the above check.
da_handle_start_event_##name() for all cases may not need this check.
(this function checked the enable first and the da_monitoring later and if
it is not monitoring it will start monitoring and return, the later event
handler will not be called. Otherwise enable is enabled, da_monitoring is
monitoring)
da_handle_start_run_event_##name() for implicit case may not need this check.
(almost the same with the above, the difference is if da-monitor is not
monitoring, it will start monitoring and not return and do the event handler,
here enable is enabled and da_monitoring is monitoring, if I am not wrong)
So after another(v7) looking at this patch, I realized that this check can
be omited in two cases(all three cases). Just in fuction da_handle_event_##name()
we need to do da_monitor_handling_event_##name().
So I'd write like this:
static inline void __da_handle_event_##name(struct da_monitor *da_mon, \
enum events_##name event) \
{ \
int retval; \
\
retval = da_event_##name(da_mon, event); \
if (!retval) \
da_monitor_reset_##name(da_mon); \
} \
static inline void da_handle_event_##name(enum events_##name event) \
{ \
struct da_monitor *da_mon = da_get_monitor_##name(); \
int retval; \
\
retval = da_monitor_handling_event_##name(da_mon); \
if (!retval) \
return; \
\
__da_handle_event_##name(da_mon, event); \
} \
> + \
> + retval = da_event_##name(da_mon, event); \
> + if (!retval) \
> + da_monitor_reset_##name(da_mon); \
> +} \
> + \
> +/* \
> + * da_handle_event_##name - handle an event \
> + */ \
> +static inline void da_handle_event_##name(enum events_##name event) \
> +{ \
> + struct da_monitor *da_mon = da_get_monitor_##name(); \
> + __da_handle_event_##name(da_mon, event); \
> +} \
> + \
> +/* \
> + * da_handle_start_event_##name - start monitoring or handle event \
> + * \
> + * This function is used notify the monitor that the system is returning \
/used/used to/ :-) My wording is not well, sorry for not convenience, Thanks,
> + * to the initial state, so the monitor can start monitoring in the next event. \
> + * Thus: \
> + * \
> + * If the monitor already started, handle the event. \
> + * If the monitor did not start yet, start the monitor but skip the event. \
> + */ \
> +static inline bool da_handle_start_event_##name(enum events_##name event) \
> +{ \
> + struct da_monitor *da_mon; \
> + \
> + if (!da_monitor_enabled_##name()) \
> + return 0; \
> + \
> + da_mon = da_get_monitor_##name(); \
> + \
> + if (unlikely(!da_monitoring_##name(da_mon))) { \
> + da_monitor_start_##name(da_mon); \
> + return 0; \
> + } \
> + \
> + __da_handle_event_##name(da_mon, event); \
> + \
> + return 1; \
> +} \
> + \
> +/* \
> + * da_handle_start_run_event_##name - start monitoring and handle event \
> + * \
> + * This function is used notify the monitor that the system is in the \
> + * initial state, so the monitor can start monitoring and handling event. \
> + */ \
> +static inline bool da_handle_start_run_event_##name(enum events_##name event) \
> +{ \
> + struct da_monitor *da_mon; \
> + \
> + if (!da_monitor_enabled_##name()) \
> + return 0; \
> + \
> + da_mon = da_get_monitor_##name(); \
> + \
> + if (unlikely(!da_monitoring_##name(da_mon))) \
> + da_monitor_start_##name(da_mon); \
> + \
> + __da_handle_event_##name(da_mon, event); \
> + \
> + return 1; \
> +}
> +
> +/*
> + * Handle event for per task.
> + */
> +#define DECLARE_DA_MON_MONITOR_HANDLER_PER_TASK(name, type) \
> + \
> +static inline void \
> +__da_handle_event_##name(struct da_monitor *da_mon, struct task_struct *tsk, \
> + enum events_##name event) \
> +{ \
> + int retval; \
> + \
> + retval = da_monitor_handling_event_##name(da_mon); \
> + if (!retval) \
> + return; \
> + \
> + retval = da_event_##name(da_mon, tsk, event); \
> + if (!retval) \
> + da_monitor_reset_##name(da_mon); \
> +} \
> + \
> +/* \
> + * da_handle_event_##name - handle an event \
> + */ \
> +static inline void \
> +da_handle_event_##name(struct task_struct *tsk, enum events_##name event) \
> +{ \
> + struct da_monitor *da_mon = da_get_monitor_##name(tsk); \
> + __da_handle_event_##name(da_mon, tsk, event); \
> +} \
> + \
> +/* \
> + * da_handle_start_event_##name - start monitoring or handle event \
> + * \
> + * This function is used notify the monitor that the system is returning \
> + * to the initial state, so the monitor can start monitoring in the next event. \
> + * Thus: \
> + * \
> + * If the monitor already started, handle the event. \
> + * If the monitor did not start yet, start the monitor but skip the event. \
> + */ \
> +static inline bool \
> +da_handle_start_event_##name(struct task_struct *tsk, enum events_##name event) \
> +{ \
> + struct da_monitor *da_mon; \
> + \
> + if (!da_monitor_enabled_##name()) \
> + return 0; \
> + \
> + da_mon = da_get_monitor_##name(tsk); \
> + \
> + if (unlikely(!da_monitoring_##name(da_mon))) { \
> + da_monitor_start_##name(da_mon); \
> + return 0; \
> + } \
> + \
> + __da_handle_event_##name(da_mon, tsk, event); \
> + \
> + return 1; \
> +}
