Re: [PATCH v3 07/13] rv: Add automatic cleanup handlers for per-task HA monitors

[Gabriele Monaco <gmonaco@redhat.com>]

On Mon, 2026-06-01 at 09:39 +0200, Nam Cao wrote:
> Gabriele Monaco <gmonaco@redhat.com> writes:
> > @@ -123,12 +144,15 @@ static int ha_monitor_init(void)
> >  
> >  	ha_mon_initializing = true;
> >  	ret = da_monitor_init();
> > +	if (ret == 0)
> > +		ha_monitor_enable_hook();
> >  	ha_mon_initializing = false;
> >  	return ret;
> >  }
> 
> What if between da_monitor_init() and ha_monitor_enable_hook(), a
> task exits while a timer is still active, and then the timer callback
> is invoked?

We are initialising, timers shouldn't be active, that sits right before
setting up other hooks, and the exit hooks this way is just the first
of them.

By the way, in this case, we likely have a valid reset scenario on an
invalid (uninitialised) timer. This is also what checking the
monitoring flag guards against.
In short, in that handler we really should reset, but need to know
whether we ever initialised in the first place.

> Extremely rare, but I think it can be fixed easily by reordering the
> two functions.
> 
> >  static void ha_monitor_destroy(void)
> >  {
> > +	ha_monitor_disable_hook();
> >  	da_monitor_destroy();
> >  }
> 
> Same here, there is small window between the two function calls.

Likewise here, we removed all hooks, then da_monitor_destroy() is going
to sync with them and clean everything up. Swapping them will expose
more races because we dropped the slot by then.

Am I missing something?

Thanks,
Gabriele