Re: [PATCH v3 4/9] rv/ha: fix ha_invariant_passed_ns silent bypass of invariant check
From: Gabriele Monaco <gmonaco@redhat.com>
Date: 2026-06-15 10:12:31
Also in:
lkml
On Mon, 2026-06-08 at 00:13 +0800, wen.yang@linux.dev wrote:
From: Wen Yang <redacted> 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 <redacted>
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 (local)
quoted hunk ↗ jump to hunk
--- 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(structha_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 boolha_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;