On Fri, 2026-09-11 at 14:34 +0200, Tobias Schaffner wrote:
> With the core facility in place, hook it into the DA/HA layer so that any
> per-CPU automaton monitor also reports how long it lingers in each state.
> 
> Pack the automaton state and its entry timestamp into one word so the
> transition cmpxchg updates both atomically. This prevents nested events
> from attributing dwell time to the wrong edge and requires a native
> 64-bit cmpxchg.
> 
> Account a transition as soon as its state change is committed. An HA
> constraint that rejects the transition can therefore reset the monitor
> without leaving an accounting window for nested events.
> 
> Add a selftest that enables a per-CPU monitor, checks the stats file
> appears and is populated under load, and skips cleanly otherwise.
> 
> Signed-off-by: Tobias Schaffner <[email protected]>
> ---

Sorry for taking that long, I could just have a quick look at your patches, the
approach looks fine but I'm going to need more time to check them carefully,
likely after the LPC conference.

I'm not too fond of how you split into patches. What's the reason for splitting
1/3 and 2/3 ? It seems they both add the new functionality, one step at a time.

...
> +/* Edge statistics pack the state and its entry timestamp into one word. */
> +#ifdef CONFIG_RV_EDGE_STAT
> +#define da_state_t   unsigned long
> +#else
> +#define da_state_t   unsigned int
> +#endif

Also try to define things in order, da_state_t is used in 2/3 but not defined
until 3/3. This doesn't break builds (the header is unused until 3/3) but is
just harder to review.

I find it more natural to split it into 1 add new functionality, 2 wire it, 3
test it. What do you think?

Thanks,
Gabriele


Reply via email to