Re: [PATCH v3 05/13] rv: Fix monitor start ordering and memory ordering for monitoring flag

[Gabriele Monaco]

On Mon, 2026-06-01 at 08:55 +0200, Nam Cao wrote:
> > diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> > index a7e103654..60dc39f26 100644
> > --- a/include/rv/da_monitor.h
> > +++ b/include/rv/da_monitor.h
> > @@ -82,7 +82,7 @@ static void react(enum states curr_state, enum
> > events event)
> >  static inline void da_monitor_reset(struct da_monitor *da_mon)
> >  {
> >  	da_monitor_reset_hook(da_mon);
> > -	da_mon->monitoring = 0;
> > +	WRITE_ONCE(da_mon->monitoring, 0);
> >  	da_mon->curr_state = model_get_initial_state();
> >  }
> 
> Looking at this again, do you need to change it to
> 
> static inline void da_monitor_reset(struct da_monitor *da_mon)
> {
> 	WRITE_ONCE(da_mon->monitoring, 0);
>         smp_mb();
> 	da_monitor_reset_hook(da_mon);
> 	da_mon->curr_state = model_get_initial_state();
> }
> 

This order won't work.
Monitor reset relies on monitoring to be true to stop the timer. The
same function is called also on monitor start and there the timer may
not have been initialised yet, we use monitoring to discriminate the
two.

> To prevent another task from seeing monitoring=1 while the timer is
> already cancelled?

I'm not sure what could go wrong in this scenario. Perhaps another
event that could start the monitor would miss the chance because it
doesn't see a reaction occurred (monitor looks like still running)?

Or a concurring event would run on top of a reaction, potentially
moving the state machine further or causing another reaction.

Those aren't really disasters and I think could happen also without
enforcing an order with the timer, nothing checks the timer's status.
Following events don't even need to know whether the timer was armed at
all, in the worst case we may be stopping twice.

Am I missing something here?

Thanks,
Gabriele