[PATCH v3 0/4] rv: per-edge dwell-time statistics for per-cpu monitors

[PATCH v3 0/4] rv: per-edge dwell-time statistics for per-cpu monitors

[Tobias Schaffner]

Hi Steven, Gabriele,

I am using the RV system for a downstream project called EVL (aka Xenomai4).
It is a small co-kernel system, small enough that I can model the whole
co-kernel with RV instead of just a fragment of it.

The patches in this series add a per-monitor "stats" file that exposes how
often an edge has been visited and the dwell time in the prior state for
per-cpu monitors.

This allows me to not only check if the model is correct but also to see if
all paths are exercised by my stress tooling and to decompose the latency of
the co-kernel's wakeup path.

I think that some of the upstream monitors like e.g. the sts monitor could also
profit from this.

As an example, here is the sts monitor on a StarFive VisionFive2 running an rt
kernel (isolcpus=2-3), after enabling it and generating some scheduler load.
Only the edges the automaton actually takes are non-zero:

  # cat monitors/sched/sts/stats
  # cpu edge label count max_ns sum_ns
  0 0 can_sched:irq_disable 367732 3998500 14773091250
  0 4 can_sched:schedule_entry 40282 398750 45510750
  0 7 cant_sched:irq_enable 367732 32500 532791750
  0 8 cant_sched:irq_entry 26188 6250 38357250
  0 13 disable_to_switch:irq_enable 4933 12500 12275000
  0 14 disable_to_switch:irq_entry 31 2250 35250
  0 15 disable_to_switch:sched_switch 35373 26750 111615750
  0 18 enable_to_exit:irq_disable 373 3250 649750
  0 19 enable_to_exit:irq_enable 373 20000 1676250
  0 20 enable_to_exit:irq_entry 309 4750 420000
  0 23 enable_to_exit:schedule_exit 40298 3000 37335000
  0 25 in_irq:irq_enable 31 12250 140750
  0 30 scheduling:irq_disable 40337 18000 45449500
  0 37 switching:irq_enable 35373 9250 72471000
  [ cpus 1-2 omitted ]
  3 0 can_sched:irq_disable 567 3983750 300860250
  3 4 can_sched:schedule_entry 150 1500 171000
  3 7 cant_sched:irq_enable 567 21500 1777500
  3 8 cant_sched:irq_entry 76 6500 164750
  3 15 disable_to_switch:sched_switch 150 4250 413750
  3 23 enable_to_exit:schedule_exit 150 1500 147750
  3 30 scheduling:irq_disable 150 1500 166000
  3 37 switching:irq_enable 150 3750 277000

The sts model splits the schedule->switch path into separate states, so the
per-edge dwell decomposes the scheduler's interrupts-off window: on cpu0 the
prep phase (disable_to_switch, irqs off until the switch) tops out at 26.8us
and the switch itself (switching:irq_enable) at 9.3us, over 35k switches. The
counts also line up with the model, e.g. every irq_disable in can_sched has a
matching irq_enable in cant_sched (367732 == 367732). The isolated cpus 2-3
take almost no switches (150 on cpu3, versus 35k on cpu0) and their worst case
is tighter still (~4us), so the same file also makes the effect of cpu
isolation visible per cpu.

The changes are gated by CONFIG_RV_EDGE_STAT and dormant until a monitor is
enabled, so existing setups are unaffected. When enabled, each accepted
transition adds one local_clock() and a few lock-free local64_t updates. With
the config off there is no code on the hot path at all.

I focused on per-cpu monitors as a first step. Per-task and per-object
monitors would have to aggregate entities that share a cpu, which is harder
to get right, so I left them out for now.

What is your opinion on this? Do you think this is worth getting upstreamed?

Thanks for taking a look,
Tobias

---

Changes since v2 (addressing Garbrieles suggestions):
 - Resplit the series: one patch adds the facility, one wires it into the
   DA/HA layer and one adds the selftest. da_state_t is now defined before
   its first use.
 - I used rv_create_file() / rv_remove() with RV_MODE_READ and included
   "rv.h" rather than <linux/tracefs.h> as you suggested. That makes rv.h
   reachable from a header every monitor pulls in, so patch 1 guards it
   against double inclusion first.
 - Replaced the pack/unpack macros with FIELD_GET() and FIELD_PREP() as
   suggested.
 - Dropped struct rv_this_edges in favour of a plain per-cpu array of
   struct rv_edge_stat, used through per_cpu_ptr() and sizeof().
 - Added DA_EDGE(), DA_EDGE_STATE() and DA_EDGE_EVENT() instead of open
   coding the state * EVENT_MAX + event arithmetic.
 - New "valid" column, from model_get_next_state() != INVALID_STATE, so a
   non-zero count on an edge the model does not define is visible instead
   of having to be inferred from the label. I also added this to the test.
 - unlikely() on the first-sample check, and dropped the (unsigned int)
   casts from both da_state_of() variants.

Changes since v1:
 - Packed the state and its entry timestamp into one word, so one cmpxchg
   commits both and a nested IRQ can no longer charge a dwell to the
   wrong edge. Hope this approach is fine.
 - Moved the header to include/rv/edge_stat.h (Gabriele).
 - Implemented it all in the da_monitor layer so that rv.c is untouched
   and the file is created from da_monitor_init() (Gabriele). I did not
   create a matrix living in struct da_monitor though. I think that this
   would need a fixed upper bound as rv.h does not know how many states
   and events the monitor will have. But this would lead to a large array
   in any monitor including per-task monitors.
 - Used local64_try_cmpxchg(), bound the loop at
   MAX_DA_RETRY_RACING_EVENTS and warn now if it is hit (Gabriele).
 - Static per-cpu matrix instead of alloc_percpu() (sashiko).
 - depend on 64BIT, so GENERIC_ATOMIC64 is unreachable and the 32-bit
   NMI deadlock cannot happen at all (sashiko).
 - Reset and report for_each_possible_cpu(), and reset before the
   tracepoints are registered (sashiko).
 - stats file is 0440 now, and a failed create is propagated.
 - Columns are now count sum_ns max_ns.

Tobias Schaffner (4):
  rv: guard rv.h against double inclusion
  rv: add per-edge dwell-time statistics
  rv: collect per-edge dwell time for per-cpu DA/HA monitors
  selftests/verification: test per-edge dwell-time statistics

 .../trace/rv/runtime-verification.rst         |  28 +++
 include/linux/rv.h                            |   9 +-
 include/rv/da_monitor.h                       |  16 +-
 include/rv/edge_stat.h                        | 211 ++++++++++++++++++
 include/rv/ha_monitor.h                       |   2 +-
 kernel/trace/rv/Kconfig                       |  12 +
 kernel/trace/rv/rv.h                          |   5 +
 tools/testing/selftests/verification/config   |   3 +
 .../verification/test.d/rv_edge_stats.tc      |  37 +++
 9 files changed, 318 insertions(+), 5 deletions(-)
 create mode 100644 include/rv/edge_stat.h
 create mode 100644 tools/testing/selftests/verification/test.d/rv_edge_stats.tc

-- 
2.53.0

[PATCH v3 1/4] rv: guard rv.h against double inclusion

[Tobias Schaffner]

rv.h has no include guard. Only three C files include it directly and
none of them twice, but anything that includes it from a header hits
this.

Add the guard, so rv.h can be included from a header as well.

Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
---
 kernel/trace/rv/rv.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/trace/rv/rv.h b/kernel/trace/rv/rv.h
index 2c0f51ff9d5c..62779e6b74bd 100644
--- a/kernel/trace/rv/rv.h
+++ b/kernel/trace/rv/rv.h
@@ -1,4 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _KERNEL_TRACE_RV_H
+#define _KERNEL_TRACE_RV_H
+
 #include <linux/mutex.h>
 
 struct rv_interface {
@@ -45,3 +48,5 @@ static inline int init_rv_reactors(struct dentry *root_dir)
 	return 0;
 }
 #endif
+
+#endif /* _KERNEL_TRACE_RV_H */
-- 
2.53.0

[PATCH v3 2/4] rv: add per-edge dwell-time statistics

[Tobias Schaffner]

Add CONFIG_RV_EDGE_STAT, an optional facility that records how long a
monitor's automaton dwells in a state before leaving it and accounts that
dwell to the edge it leaves by.

Keep the state and its entry timestamp in one word so a transition commits
both with a single cmpxchg and nested transitions cannot charge a dwell to
the wrong edge. This requires a native 64-bit cmpxchg.

The counters are per-CPU and lock-free, so a monitor's hot path can update
them without disabling interrupts and without perturbing the latency being
measured. Only the owning CPU writes them, so a reader snapshots them with
local64_read() with no IPI and no locking on the accounting path.

Storage is a static per-CPU array of STATE_MAX * EVENT_MAX edges per
monitor, exposed through a per-monitor "stats" tracefs file. Report whether
the model defines each edge, so a count on an edge that should never be
taken is visible. Nothing uses the facility yet.

Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
---
 include/linux/rv.h      |   7 ++
 include/rv/edge_stat.h  | 211 ++++++++++++++++++++++++++++++++++++++++
 kernel/trace/rv/Kconfig |  12 +++
 3 files changed, 230 insertions(+)
 create mode 100644 include/rv/edge_stat.h

diff --git a/include/linux/rv.h b/include/linux/rv.h
index 541ba404926a..1dc7c45f476c 100644
--- a/include/linux/rv.h
+++ b/include/linux/rv.h
@@ -21,6 +21,13 @@
 #include <linux/list.h>
 #include <linux/types.h>
 
+/* 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
+
 /*
  * Deterministic automaton per-object variables.
  */
diff --git a/include/rv/edge_stat.h b/include/rv/edge_stat.h
new file mode 100644
index 000000000000..8b99e8520e1e
--- /dev/null
+++ b/include/rv/edge_stat.h
@@ -0,0 +1,211 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Per-edge dwell-time statistics for RV monitors.
+ *
+ * Copyright (C) 2026 Siemens AG
+ * Author: Tobias Schaffner <tobias.schaffner@siemens.com>
+ */
+#ifndef _RV_EDGE_STAT_H
+#define _RV_EDGE_STAT_H
+
+#include <linux/rv.h>
+
+#ifdef CONFIG_RV_EDGE_STAT
+
+#include <linux/args.h>
+#include <linux/atomic.h>
+#include <linux/bitfield.h>
+#include <linux/bug.h>
+#include <linux/compiler.h>
+#include <linux/percpu.h>
+#include <linux/sched/clock.h>
+#include <linux/seq_file.h>
+#include <linux/types.h>
+#include <asm/local64.h>
+#include <rv/automata.h>
+#include "rv.h"
+
+/*
+ * Per-CPU counters kept in local64_t so accounting is safe against interrupt
+ * and NMI nesting on the owning CPU without disabling interrupts. Only the
+ * owning CPU writes.
+ */
+struct rv_edge_stat {
+	local64_t	count;
+	local64_t	sum_ns;
+	local64_t	max_ns;
+};
+
+/*
+ * State and entry timestamp share one word so a transition commits both with
+ * one cmpxchg. Comparing the timestamp also detects nested transitions that
+ * return to the same state.
+ */
+#define RV_STATE_BITS		8
+#define RV_STATE_MASK		GENMASK(RV_STATE_BITS - 1, 0)
+#define RV_TS_MASK		GENMASK(BITS_PER_LONG - 1, RV_STATE_BITS)
+
+#define da_state_of(w)		FIELD_GET(RV_STATE_MASK, w)
+#define da_ts_of(w)		FIELD_GET(RV_TS_MASK, w)
+#define da_state_pack(s, ts)	(FIELD_PREP(RV_TS_MASK, ts) | \
+				 FIELD_PREP(RV_STATE_MASK, s))
+
+static __always_inline
+void rv_edge_stat_account(struct rv_edge_stat *s, u64 dwell_ns)
+{
+	s64 max;
+	int i;
+
+	local64_inc(&s->count);
+	local64_add(dwell_ns, &s->sum_ns);
+
+	/* Keep the largest dwell; bound retries if a nested update races us. */
+	max = local64_read(&s->max_ns);
+	for (i = 0; dwell_ns > (u64)max; i++) {
+		if (i == MAX_DA_RETRY_RACING_EVENTS) {
+			WARN_ONCE(1, "rv: edge-stat max update exceeded %d retries\n",
+				  MAX_DA_RETRY_RACING_EVENTS);
+			break;
+		}
+		if (local64_try_cmpxchg(&s->max_ns, &max, dwell_ns))
+			break;
+	}
+}
+
+static_assert(STATE_MAX <= (1U << RV_STATE_BITS),
+	      "automaton has more states than the packed state word can hold");
+
+#if RV_MON_TYPE == RV_MON_PER_CPU
+
+/*
+ * One edge per (state, event) pair. Static storage avoids allocation lifetime
+ * races. Only the owning CPU writes, so readers can snapshot counters with
+ * local64_read().
+ */
+#define RV_THIS_NR_EDGES	(STATE_MAX * EVENT_MAX)
+
+#define DA_EDGE(state, event)	((state) * EVENT_MAX + (event))
+#define DA_EDGE_STATE(edge)	((edge) / EVENT_MAX)
+#define DA_EDGE_EVENT(edge)	((edge) % EVENT_MAX)
+
+#define DA_MON_EDGES		CONCATENATE(da_mon_edges_, MONITOR_NAME)
+
+static DEFINE_PER_CPU(struct rv_edge_stat[RV_THIS_NR_EDGES], DA_MON_EDGES);
+static struct dentry *rv_this_stats_file;
+
+/* Called before tracepoints are registered, so memset cannot race an update. */
+static void rv_edge_stats_reset(void)
+{
+	int cpu;
+
+	for_each_possible_cpu(cpu)
+		memset(per_cpu_ptr(DA_MON_EDGES, cpu), 0, sizeof(DA_MON_EDGES));
+}
+
+static int rv_edge_stats_show(struct seq_file *seq, void *v)
+{
+	unsigned int e;
+	int cpu;
+
+	seq_puts(seq, "# cpu edge label valid count sum_ns max_ns\n");
+	/* Include offline CPUs and keep output stable across CPU hotplug. */
+	for_each_possible_cpu(cpu) {
+		struct rv_edge_stat *edges = per_cpu_ptr(DA_MON_EDGES, cpu);
+
+		for (e = 0; e < RV_THIS_NR_EDGES; e++) {
+			enum states state = DA_EDGE_STATE(e);
+			enum events event = DA_EDGE_EVENT(e);
+
+			/* Edges the model does not define must stay at zero. */
+			seq_printf(seq, "%d %u %s:%s %d %llu %llu %llu\n",
+				   cpu, e,
+				   model_get_state_name(state),
+				   model_get_event_name(event),
+				   model_get_next_state(state, event) != INVALID_STATE,
+				   (u64)local64_read(&edges[e].count),
+				   (u64)local64_read(&edges[e].sum_ns),
+				   (u64)local64_read(&edges[e].max_ns));
+		}
+	}
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(rv_edge_stats);
+
+static int rv_edge_stats_create(struct dentry *parent)
+{
+	rv_edge_stats_reset();
+	rv_this_stats_file = rv_create_file("stats", RV_MODE_READ, parent,
+					    NULL, &rv_edge_stats_fops);
+	return rv_this_stats_file ? 0 : -ENOMEM;
+}
+
+static void rv_edge_stats_remove(void)
+{
+	rv_remove(rv_this_stats_file);
+	rv_this_stats_file = NULL;
+}
+
+/* A zero timestamp marks resets, which can run on a different CPU. */
+#define da_state_entered(s)	da_state_pack((s), local_clock())
+
+static __always_inline void
+rv_edge_account(da_state_t old, da_state_t new, enum states curr, enum events ev)
+{
+	struct rv_edge_stat *edges;
+	u64 prev = da_ts_of(old);
+
+	if (unlikely(!prev))
+		return;
+
+	edges = this_cpu_ptr(DA_MON_EDGES);
+	rv_edge_stat_account(&edges[DA_EDGE(curr, ev)],
+			     (da_ts_of(new) - prev) & FIELD_MAX(RV_TS_MASK));
+}
+
+#else /* per-cpu accounting off for this monitor type */
+
+#define da_state_entered(s)	((da_state_t)(s))
+
+static __always_inline void
+rv_edge_account(da_state_t old, da_state_t new, enum states curr, enum events ev) { }
+
+#endif /* RV_MON_TYPE == RV_MON_PER_CPU */
+
+static __always_inline bool
+da_state_try_commit(da_state_t *word, da_state_t *old, enum states next,
+		    enum events event)
+{
+	da_state_t prev = *old;
+	da_state_t new = da_state_entered(next);
+
+	if (!try_cmpxchg(word, old, new))
+		return false;
+
+	rv_edge_account(prev, new, da_state_of(prev), event);
+	return true;
+}
+
+#else /* !CONFIG_RV_EDGE_STAT */
+
+/*
+ * Feature off: only the hooks the DA/HA layer calls are provided, as trivial
+ * pass-throughs. da_state_t is a plain state word with no packed timestamp.
+ */
+#define da_state_of(w)		(w)
+#define da_state_entered(s)	(s)
+
+static __always_inline bool
+da_state_try_commit(da_state_t *word, da_state_t *old, enum states next,
+		    enum events event)
+{
+	return try_cmpxchg(word, old, next);
+}
+
+#if RV_MON_TYPE == RV_MON_PER_CPU
+static inline int rv_edge_stats_create(struct dentry *parent) { return 0; }
+static inline void rv_edge_stats_remove(void) { }
+#endif
+
+#endif /* CONFIG_RV_EDGE_STAT */
+
+#endif /* _RV_EDGE_STAT_H */
diff --git a/kernel/trace/rv/Kconfig b/kernel/trace/rv/Kconfig
index efa930f94ea4..54f7c50a74a8 100644
--- a/kernel/trace/rv/Kconfig
+++ b/kernel/trace/rv/Kconfig
@@ -59,6 +59,18 @@ config RV_PER_TASK_MONITORS
 	  This option configures the maximum number of per-task RV monitors that can run
 	  simultaneously.
 
+config RV_EDGE_STAT
+	bool "Per-edge dwell-time statistics"
+	depends on RV && 64BIT && DA_MON_EVENTS_IMPLICIT
+	help
+	  Record per-edge dwell-time statistics for per-cpu deterministic and
+	  hybrid automaton monitors and expose them through a per-monitor
+	  "stats" tracefs file. This times each monitored automaton transition
+	  with local_clock(), so leave it off if you do not need the
+	  statistics.
+
+	  If unsure, say N.
+
 source "kernel/trace/rv/monitors/wip/Kconfig"
 source "kernel/trace/rv/monitors/wwnr/Kconfig"
 
-- 
2.53.0

[PATCH v3 3/4] rv: collect per-edge dwell time for per-cpu DA/HA monitors

[Tobias Schaffner]

Hook the edge-stat facility into the DA/HA layer, so that any per-cpu
automaton monitor also reports how long it lingers in each state.

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.

Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
---
 .../trace/rv/runtime-verification.rst         | 28 +++++++++++++++++++
 include/linux/rv.h                            |  2 +-
 include/rv/da_monitor.h                       | 16 +++++++++--
 include/rv/ha_monitor.h                       |  2 +-
 4 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/Documentation/trace/rv/runtime-verification.rst b/Documentation/trace/rv/runtime-verification.rst
index c700dde9259c..012bb037c323 100644
--- a/Documentation/trace/rv/runtime-verification.rst
+++ b/Documentation/trace/rv/runtime-verification.rst
@@ -229,3 +229,31 @@ For example::
    nop
    [panic]
    printk
+
+**monitors/MONITOR/stats**
+
+Present only when the kernel is built with CONFIG_RV_EDGE_STAT=y and *MONITOR*
+is a per-cpu deterministic or hybrid automaton monitor. The file exists while
+the monitor is enabled and reports how long the automaton dwells in each state
+before leaving it, timed with local_clock() and accounted per outgoing edge and
+per CPU.
+
+- The first line is a header naming the columns.
+- Each following line describes one edge on one CPU::
+
+   cpu edge label valid count sum_ns max_ns
+
+  *label* is "state:event", *valid* is 1 if the model defines the edge and 0
+  otherwise, *count* is the number of times the transition was committed, and
+  *sum_ns* and *max_ns* are the total and worst dwell in nanoseconds. An
+  invalid edge always reports a zero count.
+
+The counters are reset each time the monitor is enabled.
+
+For example::
+
+   # cat monitors/wip/stats
+   # cpu edge label valid count sum_ns max_ns
+   0 0 preemptive:preempt_disable 1 4210 95501200 183200
+   0 1 preemptive:preempt_enable 0 0 0 0
+   0 4 non_preemptive:preempt_enable 1 4208 3812900 42600
diff --git a/include/linux/rv.h b/include/linux/rv.h
index 1dc7c45f476c..6c5c2fa7dd68 100644
--- a/include/linux/rv.h
+++ b/include/linux/rv.h
@@ -33,7 +33,7 @@
  */
 struct da_monitor {
 	bool		monitoring;
-	unsigned int	curr_state;
+	da_state_t	curr_state;
 };
 
 #ifdef CONFIG_RV_LTL_MONITOR
diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index 6fc5ef8142ac..c8f7497800d0 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -17,6 +17,7 @@
 #include <rv/automata.h>
 #include <linux/rv.h>
 #include <rv/kunit.h>
+#include <rv/edge_stat.h>
 #include <linux/stringify.h>
 #include <linux/bug.h>
 #include <linux/sched.h>
@@ -113,7 +114,7 @@ static inline void da_monitor_reset(struct da_monitor *da_mon)
  */
 static inline void da_monitor_start(struct da_monitor *da_mon)
 {
-	da_mon->curr_state = model_get_initial_state();
+	da_mon->curr_state = da_state_entered(model_get_initial_state());
 	da_monitor_init_hook(da_mon);
 	/* Pairs with smp_load_acquire in da_monitoring(). */
 	smp_store_release(&da_mon->monitoring, 1);
@@ -276,6 +277,11 @@ static inline void da_monitor_reset_state_all(void)
  */
 static inline int da_monitor_init(void)
 {
+	int retval = rv_edge_stats_create(rv_this.root_d);
+
+	if (retval)
+		return retval;
+
 	da_monitor_reset_state_all();
 	return 0;
 }
@@ -287,6 +293,7 @@ static inline void da_monitor_destroy(void)
 {
 	da_monitor_reset_all();
 	da_monitor_sync_hook();
+	rv_edge_stats_remove();
 }
 
 #ifndef da_implicit_guard
@@ -689,9 +696,11 @@ static inline void da_trace_error(struct da_monitor *da_mon,
 static inline bool da_event(struct da_monitor *da_mon, enum events event, da_id_type id)
 {
 	enum states curr_state, next_state;
+	da_state_t old;
 
-	curr_state = READ_ONCE(da_mon->curr_state);
+	old = READ_ONCE(da_mon->curr_state);
 	for (int i = 0; i < MAX_DA_RETRY_RACING_EVENTS; i++) {
+		curr_state = da_state_of(old);
 		next_state = model_get_next_state(curr_state, event);
 		if (next_state == INVALID_STATE) {
 			react(curr_state, event);
@@ -699,7 +708,8 @@ static inline bool da_event(struct da_monitor *da_mon, enum events event, da_id_
 				       model_get_event_name(event), id);
 			return false;
 		}
-		if (likely(try_cmpxchg(&da_mon->curr_state, &curr_state, next_state))) {
+		if (likely(da_state_try_commit(&da_mon->curr_state, &old,
+					       next_state, event))) {
 			if (!da_monitor_event_hook(da_mon, curr_state, event, next_state, id))
 				return false;
 			da_trace_event(da_mon, model_get_state_name(curr_state),
diff --git a/include/rv/ha_monitor.h b/include/rv/ha_monitor.h
index 6e1c7fe5449a..78d1bb80aefd 100644
--- a/include/rv/ha_monitor.h
+++ b/include/rv/ha_monitor.h
@@ -312,7 +312,7 @@ static inline void __ha_monitor_timer_callback(struct ha_monitor *ha_mon)
 	if (unlikely(READ_ONCE(ha_mon_destroying)))
 		return;
 	/* Ensure consistent curr_state if we race with da_monitor_reset */
-	curr_state = smp_load_acquire(&ha_mon->da_mon.curr_state);
+	curr_state = da_state_of(smp_load_acquire(&ha_mon->da_mon.curr_state));
 	if (unlikely(!da_monitor_handling_event(&ha_mon->da_mon)))
 		return;
 
-- 
2.53.0

Re: [PATCH v3 3/4] rv: collect per-edge dwell time for per-cpu DA/HA monitors

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] rv: bogus initial dwell time in __da_handle_start_run_event()
--

commit 296c211b8b28be395fee98f1ad59896cd58a9336
Author: Tobias Schaffner <tobias.schaffner@siemens.com>

rv: collect per-edge dwell time for per-cpu DA/HA monitors

This commit hooks the edge-stat facility into the DA and HA monitor layers
to record per-edge state dwell times for per-CPU automaton monitors and
documents the resulting stats file in runtime-verification.rst.

> diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> index 6fc5ef8142aca..c8f7497800d07 100644
> --- a/include/rv/da_monitor.h
> +++ b/include/rv/da_monitor.h

[ ... ]

> @@ -113,7 +114,7 @@ static inline void da_monitor_reset(struct da_monitor *da_mon)
>   */
>  static inline void da_monitor_start(struct da_monitor *da_mon)
>  {
> -	da_mon->curr_state = model_get_initial_state();
> +	da_mon->curr_state = da_state_entered(model_get_initial_state());
>  	da_monitor_init_hook(da_mon);
>  	/* Pairs with smp_load_acquire in da_monitoring(). */
>  	smp_store_release(&da_mon->monitoring, 1);

[ ... ]

> @@ -689,9 +696,11 @@ static inline void da_trace_error(struct da_monitor *da_mon,
>  static inline bool da_event(struct da_monitor *da_mon, enum events event, da_id_type id)
>  {
>  	enum states curr_state, next_state;
> +	da_state_t old;
>  
> -	curr_state = READ_ONCE(da_mon->curr_state);
> +	old = READ_ONCE(da_mon->curr_state);
>  	for (int i = 0; i < MAX_DA_RETRY_RACING_EVENTS; i++) {
> +		curr_state = da_state_of(old);
>  		next_state = model_get_next_state(curr_state, event);
>  		if (next_state == INVALID_STATE) {
>  			react(curr_state, event);
> @@ -699,7 +708,8 @@ static inline bool da_event(struct da_monitor *da_mon, enum events event, da_id_
>  				       model_get_event_name(event), id);
>  			return false;
>  		}
> -		if (likely(da_state_try_commit(&da_mon->curr_state, &old,
> -					       next_state, event))) {
> +		if (likely(da_state_try_commit(&da_mon->curr_state, &old,
> +					       next_state, event))) {
>  			if (!da_monitor_event_hook(da_mon, curr_state, event, next_state, id))
>  				return false;
>  			da_trace_event(da_mon, model_get_state_name(curr_state),

[Severity: Medium]
Does stamping da_mon->curr_state with da_state_entered() in
da_monitor_start() record a bogus dwell time when starting a monitor via
__da_handle_start_run_event()?

Unlike __da_handle_start_event(), which calls da_monitor_start() and
returns immediately when entering the initial state,
__da_handle_start_run_event() is used when the system is already in the
initial state for an unknown duration and an outgoing transition occurs:

include/rv/da_monitor.h:__da_handle_start_run_event() {
	if (!da_monitor_enabled())
		return 0;
	if (unlikely(!da_monitoring(da_mon)))
		da_monitor_start(da_mon);

	__da_handle_event_common(da_mon, event, id);

	return 1;
}

When !da_monitoring(da_mon) is true (such as in the opid monitor on the
first handle_sched_need_resched() or handle_sched_waking() event after
enabling or resetting), da_monitor_start() stamps curr_state with a
non-zero local_clock() timestamp via da_state_entered().

Immediately afterwards, __da_handle_event_common() calls da_event(), where
da_state_try_commit() takes a second local_clock() timestamp and calls
rv_edge_account(). Because the timestamp in old is non-zero, won't
rv_edge_account() record the few nanoseconds spent between
da_monitor_start() and da_state_try_commit() as the dwell time for the
initial state's outgoing edge?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261020095512.863754-1-tobias.schaffner@siemens.com?part=3

[PATCH v3 4/4] selftests/verification: test per-edge dwell-time statistics

[Tobias Schaffner]

Enable a per-cpu monitor, check that the stats file appears and is
populated under load, and skip cleanly when CONFIG_RV_EDGE_STAT is off.

Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
---
 tools/testing/selftests/verification/config   |  3 ++
 .../verification/test.d/rv_edge_stats.tc      | 37 +++++++++++++++++++
 2 files changed, 40 insertions(+)
 create mode 100644 tools/testing/selftests/verification/test.d/rv_edge_stats.tc

diff --git a/tools/testing/selftests/verification/config b/tools/testing/selftests/verification/config
index 43072c1c38f4..caf3a96ac8b4 100644
--- a/tools/testing/selftests/verification/config
+++ b/tools/testing/selftests/verification/config
@@ -1 +1,4 @@
 CONFIG_RV=y
+CONFIG_PREEMPT_TRACER=y
+CONFIG_RV_MON_WIP=y
+CONFIG_RV_EDGE_STAT=y
diff --git a/tools/testing/selftests/verification/test.d/rv_edge_stats.tc b/tools/testing/selftests/verification/test.d/rv_edge_stats.tc
new file mode 100644
index 000000000000..c03ed6682b80
--- /dev/null
+++ b/tools/testing/selftests/verification/test.d/rv_edge_stats.tc
@@ -0,0 +1,37 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# description: Test per-edge dwell-time statistics (stats)
+
+MON=wip
+
+check_requires "$MON:monitor"
+
+# The "stats" file is created on enable; if it is still missing the kernel was
+# built without CONFIG_RV_EDGE_STAT, so skip.
+echo 1 > "monitors/$MON/enable"
+if [ ! -e "monitors/$MON/stats" ]; then
+	echo 0 > "monitors/$MON/enable"
+	echo "CONFIG_RV_EDGE_STAT is not enabled."
+	exit_unsupported
+fi
+
+# The first line is the column header; the body has one line per (cpu, edge).
+head -n1 "monitors/$MON/stats" | grep -q "^# cpu edge label valid count sum_ns max_ns"
+[ "$(grep -cvE '^#' "monitors/$MON/stats")" -gt 0 ]
+
+# Drive some scheduler activity so the automaton records transitions.
+for _ in 1 2 3 4 5 6 7 8 9 10; do
+	(true) &
+	wait
+done
+
+# At least one valid edge must now show a non-zero count. Do not require a
+# non-zero dwell: a coarse local_clock() may legitimately return the same
+# value twice.
+grep -vE '^#' "monitors/$MON/stats" | \
+	awk '$4 == 1 && $5 > 0 && $6 >= $7 { hit = 1 } END { exit !hit }'
+
+# An edge the model does not define must never be taken.
+grep -vE '^#' "monitors/$MON/stats" | awk '$4 == 0 && $5 > 0 { exit 1 }'
+
+echo 0 > "monitors/$MON/enable"
-- 
2.53.0