On Fri, 2025-10-17 at 15:05 +0200, Nam Cao wrote:

Ok, now things start to make sense. Thanks for the explanation.

At least to me, using the same variable to store different time values
is a bit confusing.

Is it possible that we always store the timestamp of the last clock reset?

The invariant bound value (N) is fixed for each state, so we can have
the bound value in ha_verify_invariants() instead. For example, the
Python script can generate something like

static inline bool ha_verify_invariants(struct ha_monitor *ha_mon,
                                       enum states curr_state, enum events
event,
                                       enum states next_state, u64 time_ns)
{
       if (curr_state == enqueued_stall)
               return ha_check_invariant_jiffy(ha_mon, threshold_jiffies,
time_ns);
       return true;
}

Is that possible?

Alright, that /should/ be possible, provided the value used to set invariants is
constant or at least doesn't change until we leave the state.
This seems a fair assumption to make but doesn't stand for the throttle monitor,
in that case I read the remaining runtime from the dl entity, that one is
updated frequently, for instance when a task is throttled, it's negative, but
this doesn't mean the invariant should expect time to be negative.

Runtime is consumed only when a task is running, so here I use an invariant set
up on the /remaining/ runtime when reaching the running state, that's why also
switch_in resets the clock (runtime is not replenished, but the runtime_left
value doesn't need to be subtracted anything).

An alternative would be to have some sort of pause/resume operations on clocks,
and a task would just pause the clock when preempted, but those operations are
not backed up by theory and wouldn't really simplify the implementation (use 2
variables per clock or a single one and some hack to mark it as paused).

Again, there may be better ways, but I found this one the "simplest".

Does it makes sense or am I just crystallising to this implementation?

Thanks,
Gabriele

quoted

Kinda, it would solve the problem for this specific subtraction, but racing
handlers could still lead to problems although the subtraction is "correct".

Since this is the only time the env storage needs to be an atomic_t and it's
fairly rare (only complicated models require calling this function at all,
others are happy with READ_ONCE/WRITE_ONCE) I didn't want to change the
storage
implementation for some perceived safety.

I wrote that comment exactly to motivate why we aren't using atomic_t, but I
should probably reword that. Does this make sense to you?

I think if we always store the timestamp since last reset, we can get
rid of this function. Let's see how that discussion go..

quoted