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

On Wed, 2026-07-08 at 23:38 +0800, [email protected] wrote:
> From: Wen Yang <[email protected]>
> 
> When env_store is U64_MAX (its initial sentinel value),
> ha_invariant_passed_ns() returns 0 immediately without initializing
> env_store to the current clock.  Subsequent calls to
> ha_check_invariant_ns() then find env_store still at U64_MAX, causing
> the elapsed comparison to wrap and always report the invariant as
> satisfied, silently masking any violations.
> 
> Fix by calling ha_reset_clk_ns() to establish the guard on the first
> invocation instead of returning early.  Apply the same fix to
> ha_invariant_passed_jiffy().
> 
> Signed-off-by: Wen Yang <[email protected]>

While this looks alright, I have something else in mind for the env variables
lifecycle. After [1] is merged, I'd like to merge [2] which is basically
inspired on your idea of resetting variables on start, just it's embedded in the
framework (and requires [1] to get rid of the double representation for env
variables).
This is probably going to happen after the next merge window.

I believe you can keep this patch for now and your model wouldn't need change
after adding [2] instead.

Thoughts?

Thanks,
Gabriele

[1] -
https://lore.kernel.org/lkml/c0f600dcbf3d8b487c944406851a39146f4d91fa.1781847583.git.namcao@linutronix.de/
[2] -
https://web.git.kernel.org/pub/scm/linux/kernel/git/gmonaco/linux.git/commit/?h=rv_hybrid_automata_enq_lark&id=d7c5c40c81d9b928aa9f86c4115429bffd223a8c

> ---
>  include/rv/ha_monitor.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/rv/ha_monitor.h b/include/rv/ha_monitor.h
> index 83199f90afe8..dddf5694bcc8 100644
> --- a/include/rv/ha_monitor.h
> +++ b/include/rv/ha_monitor.h
> @@ -375,12 +375,12 @@ static inline bool ha_check_invariant_ns(struct
> ha_monitor *ha_mon,
>  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;
> @@ -414,12 +414,12 @@ static inline bool ha_check_invariant_jiffy(struct
> ha_monitor *ha_mon,
>  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;