Re: [PATCH v7 02/14] rv: Fix ha_monitor timer and invariant underflow

[Gabriele Monaco <gmonaco@redhat.com>]

On Mon, 2026-10-12 at 03:39 +0800, wen.yang@linux.dev wrote:
> From: Wen Yang <wen.yang@linux.dev>
> 
> ha_start_timer_ns() and ha_start_timer_jiffy() compute the remaining
> budget as expire - passed.  Once the elapsed time passed exceeds the
> budget expire (e.g. a state transition after the invariant should have
> fired, before the timer callback ran), the unsigned subtraction
> underflows to a huge value and the timer is scheduled far into the
> future instead of expiring immediately.
> 
> ha_check_invariant_ns() computes store >= time_ns - expire_ns, which
> underflows when the system uptime is still below expire_ns (a monitor
> armed early in boot with a large budget), reporting a false violation.
> 
> Clamp the timer remainder to 0 (fire immediately) when passed >= expire,
> and replace the invariant check as store + expire_ns >= time_ns so
> neither side can underflow.
> 
> Fixes: f5587d1b6ec9 ("rv: Add Hybrid Automata monitor type")
> Fixes: ab2900ae252b ("rv: Simplify hybrid automata monitors's clock
> variables")
> Signed-off-by: Wen Yang <wen.yang@linux.dev>
> ---

Looks good, thanks

Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>

>  include/rv/ha_monitor.h | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/include/rv/ha_monitor.h b/include/rv/ha_monitor.h
> index 93ac99ff7f1d..499ce60779f9 100644
> --- a/include/rv/ha_monitor.h
> +++ b/include/rv/ha_monitor.h
> @@ -351,7 +351,7 @@ static inline void ha_reset_clk_ns(struct ha_monitor
> *ha_mon, enum envs env, u64
>  static inline bool ha_check_invariant_ns(struct ha_monitor *ha_mon, enum envs
> env,
>  					 u64 time_ns, u64 expire_ns)
>  {
> -	return READ_ONCE(ha_mon->env_store[env]) >= time_ns - expire_ns;
> +	return READ_ONCE(ha_mon->env_store[env]) + expire_ns >= time_ns;
>  }
>  /*
>   * ha_invariant_passed_ns - prepare the invariant and return the time since
> reset
> @@ -410,6 +410,11 @@ static inline void ha_inv_to_guard(struct ha_monitor
> *ha_mon, enum envs env,
>  	WRITE_ONCE(ha_mon->env_store[env], READ_ONCE(ha_mon->env_store[env])
> - value);
>  }
>  
> +static inline u64 ha_time_left(u64 passed, u64 expire)
> +{
> +	return passed < expire ? expire - passed : 0;
> +}
> +
>  #if HA_TIMER_TYPE == HA_TIMER_WHEEL
>  /*
>   * Helper functions to handle the monitor timer.
> @@ -439,15 +444,17 @@ static inline void ha_start_timer_jiffy(struct
> ha_monitor *ha_mon, enum envs env
>  {
>  	u64 passed = ha_invariant_passed_jiffy(ha_mon, env, time_ns);
>  
> -	mod_timer(&ha_mon->timer, get_jiffies_64() + expire - passed);
> +	mod_timer(&ha_mon->timer,
> +		  get_jiffies_64() + ha_time_left(passed, expire));
>  }
>  static inline void ha_start_timer_ns(struct ha_monitor *ha_mon, enum envs
> env,
>  				     u64 expire, u64 time_ns)
>  {
>  	u64 passed = ha_invariant_passed_ns(ha_mon, env, time_ns);
> +	u64 delay = ha_time_left(passed, expire);
>  
>  	ha_start_timer_jiffy(ha_mon, ENV_MAX_STORED,
> -			     nsecs_to_jiffies(expire - passed + TICK_NSEC -
> 1), time_ns);
> +			     nsecs_to_jiffies(delay + TICK_NSEC - 1),
> time_ns);
>  }
>  /*
>   * ha_cancel_timer - Cancel the timer
> @@ -494,7 +501,8 @@ static inline void ha_start_timer_ns(struct ha_monitor
> *ha_mon, enum envs env,
>  
>  	if (RV_MON_TYPE == RV_MON_PER_CPU)
>  		mode |= HRTIMER_MODE_PINNED;
> -	hrtimer_start(&ha_mon->hrtimer, ns_to_ktime(expire - passed), mode);
> +	hrtimer_start(&ha_mon->hrtimer,
> +		      ns_to_ktime(ha_time_left(passed, expire)), mode);
>  }
>  static inline void ha_start_timer_jiffy(struct ha_monitor *ha_mon, enum envs
> env,
>  					u64 expire, u64 time_ns)
> @@ -502,7 +510,8 @@ static inline void ha_start_timer_jiffy(struct ha_monitor
> *ha_mon, enum envs env
>  	u64 passed = ha_invariant_passed_jiffy(ha_mon, env, time_ns);
>  
>  	ha_start_timer_ns(ha_mon, ENV_MAX_STORED,
> -			  jiffies_to_nsecs(expire - passed), time_ns);
> +			  jiffies_to_nsecs(ha_time_left(passed, expire)),
> +			  time_ns);
>  }
>  /*
>   * ha_cancel_timer - Cancel the timer