On Mon, 2026-06-08 at 00:13 +0800, [email protected] wrote: > From: Wen Yang <[email protected]> > > 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 <[email protected]>
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.nam...@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;
