Re: [PATCH v3 4/9] rv/ha: fix ha_invariant_passed_ns silent bypass of invariant check

[Gabriele Monaco <gmonaco@redhat.com>]

On Mon, 2026-06-08 at 00:13 +0800, wen.yang@linux.dev wrote:
> From: Wen Yang <wen.yang@linux.dev>
> 
> The function is documented as "prepare the invariant and return the
> time
> since reset", but on the first call (env_store == U64_MAX) it exits
> early without calling ha_set_invariant_ns():
> 
>   if (ha_monitor_env_invalid(ha_mon, env))  /* env_store == U64_MAX
> */
>       return 0;   /* ha_set_invariant_ns skipped, env_store stays
> U64_MAX */
>   ...
>   ha_set_invariant_ns(ha_mon, env, expire - passed, time_ns);
> 
> This leaves env_store == U64_MAX, so ha_check_invariant_ns() always
> passes on the first activation regardless of elapsed time:
> 
>   return READ_ONCE(ha_mon->env_store[env]) >= time_ns;  /* U64_MAX >=
> any */
> 
> Fix: establish the guard before converting to the invariant:
> 
>   if (ha_monitor_env_invalid(ha_mon, env))
>       ha_reset_clk_ns(ha_mon, env, time_ns); /* guard: env_store =
> time_ns */
>   passed = ha_get_env(ha_mon, env, time_ns);
>   ha_set_invariant_ns(ha_mon, env, expire - passed, time_ns);
>                                  /* invariant: env_store = time_ns +
> expire */
> 
> Apply the same fix to ha_invariant_passed_jiffy().
> 
> Signed-off-by: Wen Yang <wen.yang@linux.dev>

This is neat and probably works just fine in your case. Anyway I'm
planning on a more seamless solution not involving invalid states at
all. Initialisation would include a reset, which should work better on
monitors doing da_handle_start(), so not running the first event.
That's, by the way, pretty much what you wanted by doing reset() on the
(virtual) init transition.

We first needs Nam's simplification though [1]. We can synchronise
during this development cycle, you probably won't really need to bother
for now.

Thanks,
Gabriele

[1] -
https://lore.kernel.org/lkml/08188c28f274da63a3f8549add3086a92aef45e5.1780908661.git.namcao@linutronix.de

> ---
>  include/rv/ha_monitor.h | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/include/rv/ha_monitor.h b/include/rv/ha_monitor.h
> index 28d3c74cabfc..e5860900a337 100644
> --- a/include/rv/ha_monitor.h
> +++ b/include/rv/ha_monitor.h
> @@ -365,16 +365,22 @@ static inline bool ha_check_invariant_ns(struct
> ha_monitor *ha_mon,
>  }
>  /*
>   * ha_invariant_passed_ns - prepare the invariant and return the
> time since reset
> + *
> + * If the env has not been initialised yet (first entry into a state
> with an
> + * invariant), anchor the guard clock at the current time so that
> the full
> + * budget is available from this point.  This preserves the
> documented
> + * guard→invariant ordering: ha_set_invariant_ns() is always
> preceded by a
> + * valid guard representation in env_store.
>   */
>  static inline u64 ha_invariant_passed_ns(struct ha_monitor *ha_mon,
> enum envs env,
>  				   u64 expire, u64 time_ns)
>  {
> -	u64 passed = 0;
> +	u64 passed;
>  
>  	if (env < 0 || env >= ENV_MAX_STORED)
>  		return 0;
>  	if (ha_monitor_env_invalid(ha_mon, env))
> -		return 0;
> +		ha_reset_clk_ns(ha_mon, env, time_ns);
>  	passed = ha_get_env(ha_mon, env, time_ns);
>  	ha_set_invariant_ns(ha_mon, env, expire - passed, time_ns);
>  	return passed;
> @@ -404,16 +410,19 @@ static inline bool
> ha_check_invariant_jiffy(struct ha_monitor *ha_mon,
>  }
>  /*
>   * ha_invariant_passed_jiffy - prepare the invariant and return the
> time since reset
> + *
> + * Same first-use semantics as ha_invariant_passed_ns(): anchor the
> guard clock
> + * now if the env has not been initialised.
>   */
>  static inline u64 ha_invariant_passed_jiffy(struct ha_monitor
> *ha_mon, enum envs env,
>  				      u64 expire, u64 time_ns)
>  {
> -	u64 passed = 0;
> +	u64 passed;
>  
>  	if (env < 0 || env >= ENV_MAX_STORED)
>  		return 0;
>  	if (ha_monitor_env_invalid(ha_mon, env))
> -		return 0;
> +		ha_reset_clk_jiffy(ha_mon, env);
>  	passed = ha_get_env(ha_mon, env, time_ns);
>  	ha_set_invariant_jiffy(ha_mon, env, expire - passed);
>  	return passed;