lkml.org 
[lkml]   [2022]   [Jul]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH V7 04/16] rv/include: Add deterministic automata monitor definition via C macros
From
On 7/27/22 17:29, Tao Zhou wrote:
>> +/*
>> + * 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); \
>
> } \
>

IOW,

The code is checking twice if the monitor is enabled in these two cases:
- da_handle_start_run_event_##name()
- da_handle_start_event_##name()

Because it is checking in these functions first and then again in __da_handle_event_##name().

The function da_handle_event_##name() is not checking if the monitors are enabled because
__da_handle_event_##name() does it.

By adding the check on da_handle_event_##name(), we can remove it in
__da_handle_event_##name(). Making the check run only once for all cases.

This is an optimization, and it makes sense.

(changed return value to bool)

-- Daniel

\
 
 \ /
  Last update: 2022-07-27 18:07    [W:0.078 / U:0.388 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site