Re: [PATCH v3 08/13] rv: Ensure synchronous cleanup for HA monitors

[Nam Cao <namcao@linutronix.de>]

>  static bool ha_mon_initializing;
> +static bool ha_mon_destroying;
>  
>  static int ha_monitor_init(void)
>  {
>  	int ret;
>  
> +	WRITE_ONCE(ha_mon_destroying, false);
>  	ha_mon_initializing = true;
>  	ret = da_monitor_init();
>  	if (ret == 0)
> @@ -152,6 +155,7 @@ static int ha_monitor_init(void)
>  
>  static void ha_monitor_destroy(void)
>  {
> +	WRITE_ONCE(ha_mon_destroying, true);
>  	ha_monitor_disable_hook();
>  	da_monitor_destroy();
>  }
> @@ -302,12 +306,30 @@ static bool ha_monitor_handle_constraint(struct da_monitor *da_mon,
>  	return false;
>  }
>  
> +/*
> + * __ha_monitor_timer_callback - generic callback representation
> + *
> + * This callback runs in an RCU read-side critical section to allow the
> + * destruction sequence to easily synchronize_rcu() with all pending timers
> + * after asynchronously disabling them. The ha_mon_destroying check ensures
> + * any callback entering the RCU section after synchronize_rcu() completes
> + * will see the flag and bail out immediately.
> + */
>  static inline void __ha_monitor_timer_callback(struct ha_monitor *ha_mon)
>  {
> -	enum states curr_state = READ_ONCE(ha_mon->da_mon.curr_state);
>  	DECLARE_SEQ_BUF(env_string, ENV_BUFFER_SIZE);
> -	u64 time_ns = ha_get_ns();
> -
> +	enum states curr_state;
> +	u64 time_ns;
> +
> +	guard(rcu)();
> +	if (unlikely(READ_ONCE(ha_mon_destroying)))

Instead of this global variable, can we instead use da_mon->monitoring?

> +		return;
> +	/* Ensure consistent curr_state if we race with da_monitor_reset */
> +	curr_state = smp_load_acquire(&ha_mon->da_mon.curr_state);
> +	if (unlikely(!da_monitor_handling_event(&ha_mon->da_mon)))
> +		return;
> +
> +	time_ns = ha_get_ns();
>  	ha_get_env_string(&env_string, ha_mon, time_ns);
>  	ha_react(curr_state, EVENT_NONE, env_string.buffer);
>  	ha_trace_error_env(ha_mon, model_get_state_name(curr_state),
> -- 
> 2.54.0