[PATCH v3 00/13] rv: Fixes on Deterministic and Hybrid Automata

[PATCH v3 00/13] rv: Fixes on Deterministic and Hybrid Automata

[Gabriele Monaco]

Fix issues that were reported by bots or visible only after integration:
 * Make sure timers are always terminated and waited for when disabling
   the monitor or when the target terminates
 * Run per-cpu monitors with migration disabled since preemption is now
   enabled from tracepoints
 * Fix a wrong __user specifier in a helper function
 * Other cleanup and concurrency issues

Differences since V2 [1]:
* Applied from reviews changes to commit messages
* Rearranged order to put non-fixes and not-reviewed patches in the end
* Synchronise monitors before resetting them to avoid rearming
* Protect against racing timer callbacks during destruction

Differences since V1 [2]:
* Fix memory consistency with timer callbacks racing with resets
* Add per-obj deallocation hook in rvgen generated code
* Do not rely on clean monitor when initialising HA
* Add tracepoint synchronisation before returning per-task slots
* Fix suffix strip in dot2k
* Generate stub deallocation hooks instead of failing build when per-obj
  miss those

[1] - https://lore.kernel.org/lkml/20260527062313.39908-1-gmonaco@redhat.com
[2] - https://lore.kernel.org/lkml/20260512140250.262190-1-gmonaco@redhat.com

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Nam Cao <namcao@linutronix.de>
Cc: Wen Yang <wen.yang@linux.dev>
Cc: linux-trace-kernel@vger.kernel.org

Gabriele Monaco (12):
  rv: Fix __user specifier usage in extract_params()
  rv: Reset per-task DA monitors before releasing the slot
  rv: Prevent in-flight per-task handlers from using invalid slots
  rv: Ensure all pending probes terminate on per-obj monitor destroy
  rv: Do not rely on clean monitor when initialising HA
  rv: Add automatic cleanup handlers for per-task HA monitors
  rv: Ensure synchronous cleanup for HA monitors
  rv: Prevent task migration while handling per-CPU events
  rv: Use 0 to check preemption enabled in opid
  verification/rvgen: Fix suffix strip in dot2k
  rv: Fix read_lock scope in per-task DA cleanup
  verification/rvgen: Generate cleanup hook for per-obj monitor

Wen Yang (1):
  rv: Fix monitor start ordering and memory ordering for monitoring flag

 include/rv/da_monitor.h                       |  67 ++++++++---
 include/rv/ha_monitor.h                       | 104 +++++++++++++++++-
 include/rv/ltl_monitor.h                      |   1 +
 kernel/trace/rv/monitors/deadline/deadline.h  |   3 +-
 kernel/trace/rv/monitors/nomiss/nomiss.c      |   4 +-
 kernel/trace/rv/monitors/opid/opid.c          |  12 +-
 kernel/trace/rv/monitors/stall/stall.c        |   4 +-
 tools/verification/rvgen/rvgen/dot2k.py       |  19 +++-
 .../rvgen/rvgen/templates/dot2k/main.c        |   4 +-
 9 files changed, 180 insertions(+), 38 deletions(-)


base-commit: 8fde5d1d47f69db6082dfa34500c27f8485389a5
-- 
2.54.0

[PATCH v3 01/13] rv: Fix __user specifier usage in extract_params()

[Gabriele Monaco]

The attributes variables extracted from syscalls in the helper are both
defined with the __user specifier although only the actual pointer to
user data should be marked.

Remove the __user specifier from attr.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202604150820.Ny143u6X-lkp@intel.com
Fixes: b133207deb72 ("rv: Add nomiss deadline monitor")
Reviewed-by: Wen Yang <wen.yang@linux.dev>
Reviewed-by: Nam Cao <namcao@linutronix.de>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 kernel/trace/rv/monitors/deadline/deadline.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/rv/monitors/deadline/deadline.h b/kernel/trace/rv/monitors/deadline/deadline.h
index 0bbfd2543..78fca873d 100644
--- a/kernel/trace/rv/monitors/deadline/deadline.h
+++ b/kernel/trace/rv/monitors/deadline/deadline.h
@@ -95,7 +95,8 @@ static inline u8 get_server_type(struct task_struct *tsk)
 static inline int extract_params(struct pt_regs *regs, long id, pid_t *pid_out)
 {
 	size_t size = offsetofend(struct sched_attr, sched_flags);
-	struct sched_attr __user *uattr, attr;
+	struct sched_attr __user *uattr;
+	struct sched_attr attr;
 	int new_policy = -1, ret;
 	unsigned long args[6];
 
-- 
2.54.0

[PATCH v3 02/13] rv: Reset per-task DA monitors before releasing the slot

[Gabriele Monaco]

Per-task monitors use task_mon_slot to determine which slot in the array
to use for the monitor. During destruction, this slot is returned but
this is done before resetting the monitor. As a result, the monitor's
reset is in fact resetting a slot that is outside of the array
(RV_PER_TASK_MONITOR_INIT).

Release the slot only after the reset to avoid out-of-bound memory
access.

Fixes: f5587d1b6ec93 ("rv: Add Hybrid Automata monitor type")
Cc: stable@vger.kernel.org
Suggested-by: Wen Yang <wen.yang@linux.dev>
Reviewed-by: Wen Yang <wen.yang@linux.dev>
Reviewed-by: Nam Cao <namcao@linutronix.de>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 include/rv/da_monitor.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index 39765ff6f..1459fb3df 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -309,10 +309,11 @@ static inline void da_monitor_destroy(void)
 		WARN_ONCE(1, "Disabling a disabled monitor: " __stringify(MONITOR_NAME));
 		return;
 	}
-	rv_put_task_monitor_slot(task_mon_slot);
-	task_mon_slot = RV_PER_TASK_MONITOR_INIT;
 
 	da_monitor_reset_all();
+
+	rv_put_task_monitor_slot(task_mon_slot);
+	task_mon_slot = RV_PER_TASK_MONITOR_INIT;
 }
 
 #elif RV_MON_TYPE == RV_MON_PER_OBJ
-- 
2.54.0

[PATCH v3 03/13] rv: Prevent in-flight per-task handlers from using invalid slots

[Gabriele Monaco]

Per-task monitors use a slot in the task_struct->rv[] array and store
that locally (e.g. task_mon_slot), this slot is returned during the
destruction process but currently hanlers can be running while that slot
is returning and this race may lead to accessing an invalid slot.

Synchronise with all in-flight tracepoint handlers using
tracepoint_synchronize_unregister() before returning the slot.

Fixes: f5587d1b6ec9 ("rv: Add Hybrid Automata monitor type")
Fixes: a9769a5b9878 ("rv: Add support for LTL monitors")
Suggested-by: Wen Yang <wen.yang@linux.dev>
Reviewed-by: Nam Cao <namcao@linutronix.de>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 include/rv/da_monitor.h  | 4 ++++
 include/rv/ltl_monitor.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index 1459fb3df..cc97cc5df 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -302,6 +302,9 @@ static int da_monitor_init(void)
 
 /*
  * da_monitor_destroy - return the allocated slot
+ *
+ * Wait for all in-flight handlers before returning the slot to avoid
+ * out-of-bound accesses.
  */
 static inline void da_monitor_destroy(void)
 {
@@ -310,6 +313,7 @@ static inline void da_monitor_destroy(void)
 		return;
 	}
 
+	tracepoint_synchronize_unregister();
 	da_monitor_reset_all();
 
 	rv_put_task_monitor_slot(task_mon_slot);
diff --git a/include/rv/ltl_monitor.h b/include/rv/ltl_monitor.h
index eff60cd61..38e792401 100644
--- a/include/rv/ltl_monitor.h
+++ b/include/rv/ltl_monitor.h
@@ -77,6 +77,7 @@ static void ltl_monitor_destroy(void)
 {
 	rv_detach_trace_probe(name, task_newtask, handle_task_newtask);
 
+	tracepoint_synchronize_unregister();
 	rv_put_task_monitor_slot(ltl_monitor_slot);
 	ltl_monitor_slot = RV_PER_TASK_MONITOR_INIT;
 }
-- 
2.54.0

[PATCH v3 04/13] rv: Ensure all pending probes terminate on per-obj monitor destroy

[Gabriele Monaco]

The monitor disable/destroy sequence detaches all probes and resets the
monitor's data, however it doesn't wait for pending probes. This is an
issue with per-object monitors, which free the monitor storage.

Call tracepoint_synchronize_unregister() to make sure to wait for all
pending probes before destroying the monitor storage.

Fixes: 4a24127bd6cb ("rv: Add support for per-object monitors in DA/HA")
Reviewed-by: Wen Yang <wen.yang@linux.dev>
Reviewed-by: Nam Cao <namcao@linutronix.de>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 include/rv/da_monitor.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index cc97cc5df..a7e103654 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -511,9 +511,10 @@ static inline void da_monitor_destroy(void)
 	struct hlist_node *tmp;
 	int bkt;
 
+	tracepoint_synchronize_unregister();
 	/*
-	 * This function is called after all probes are disabled, we need only
-	 * worry about concurrency against old events.
+	 * This function is called after all probes are disabled and no longer
+	 * pending, we can safely assume no concurrent user.
 	 */
 	synchronize_rcu();
 	hash_for_each_safe(da_monitor_ht, bkt, tmp, mon_storage, node) {
-- 
2.54.0

[PATCH v3 05/13] rv: Fix monitor start ordering and memory ordering for monitoring flag

[Gabriele Monaco]

From: Wen Yang <wen.yang@linux.dev>

da_monitor_start() set monitoring=1 before calling da_monitor_init_hook(),
may racing with the sched_switch handler:

  da_monitor_start()               sched_switch handler
  -------------------------        ---------------------------------
  da_mon->monitoring = 1;
                                   if (da_monitoring(da_mon))  /* true  */
                                       ha_start_timer_ns(...);
                                       /* hrtimer->base == NULL, crash */
  da_monitor_init_hook(da_mon);
  /* hrtimer_setup() sets base */

Fix the ordering and pair with release/acquire semantics:

  da_monitor_init_hook(da_mon);
  smp_store_release(&da_mon->monitoring, 1);    /* da_monitor_start()  */
  return smp_load_acquire(&da_mon->monitoring); /* da_monitoring()     */

On ARM64 a plain STR + LDR does not form a release-acquire pair, so
the load can observe monitoring=1 while hrtimer->base is still NULL.
The plain accesses are also data races under KCSAN.

Use WRITE_ONCE for the monitoring=0 store in da_monitor_reset() to
cover the reset path.

Fixes: 792575348ff7 ("rv/include: Add deterministic automata monitor definition via C macros")
Signed-off-by: Wen Yang <wen.yang@linux.dev>
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Reviewed-by: Nam Cao <namcao@linutronix.de>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 include/rv/da_monitor.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index a7e103654..60dc39f26 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -82,7 +82,7 @@ static void react(enum states curr_state, enum events event)
 static inline void da_monitor_reset(struct da_monitor *da_mon)
 {
 	da_monitor_reset_hook(da_mon);
-	da_mon->monitoring = 0;
+	WRITE_ONCE(da_mon->monitoring, 0);
 	da_mon->curr_state = model_get_initial_state();
 }
 
@@ -95,8 +95,9 @@ 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->monitoring = 1;
 	da_monitor_init_hook(da_mon);
+	/* Pairs with smp_load_acquire in da_monitoring(). */
+	smp_store_release(&da_mon->monitoring, 1);
 }
 
 /*
@@ -104,7 +105,8 @@ static inline void da_monitor_start(struct da_monitor *da_mon)
  */
 static inline bool da_monitoring(struct da_monitor *da_mon)
 {
-	return da_mon->monitoring;
+	/* Pairs with smp_store_release in da_monitor_start(). */
+	return smp_load_acquire(&da_mon->monitoring);
 }
 
 /*
-- 
2.54.0

Re: [PATCH v3 05/13] rv: Fix monitor start ordering and memory ordering for monitoring flag

[Nam Cao]

Gabriele Monaco <gmonaco@redhat.com> writes:

> From: Wen Yang <wen.yang@linux.dev>
>
> da_monitor_start() set monitoring=1 before calling da_monitor_init_hook(),
> may racing with the sched_switch handler:
>
>   da_monitor_start()               sched_switch handler
>   -------------------------        ---------------------------------
>   da_mon->monitoring = 1;
>                                    if (da_monitoring(da_mon))  /* true  */
>                                        ha_start_timer_ns(...);
>                                        /* hrtimer->base == NULL, crash */
>   da_monitor_init_hook(da_mon);
>   /* hrtimer_setup() sets base */
>
> Fix the ordering and pair with release/acquire semantics:
>
>   da_monitor_init_hook(da_mon);
>   smp_store_release(&da_mon->monitoring, 1);    /* da_monitor_start()  */
>   return smp_load_acquire(&da_mon->monitoring); /* da_monitoring()     */
>
> On ARM64 a plain STR + LDR does not form a release-acquire pair, so
> the load can observe monitoring=1 while hrtimer->base is still NULL.
> The plain accesses are also data races under KCSAN.
>
> Use WRITE_ONCE for the monitoring=0 store in da_monitor_reset() to
> cover the reset path.
>
> Fixes: 792575348ff7 ("rv/include: Add deterministic automata monitor definition via C macros")
> Signed-off-by: Wen Yang <wen.yang@linux.dev>
> Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
> Reviewed-by: Nam Cao <namcao@linutronix.de>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
> ---
>  include/rv/da_monitor.h | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> index a7e103654..60dc39f26 100644
> --- a/include/rv/da_monitor.h
> +++ b/include/rv/da_monitor.h
> @@ -82,7 +82,7 @@ static void react(enum states curr_state, enum events event)
>  static inline void da_monitor_reset(struct da_monitor *da_mon)
>  {
>  	da_monitor_reset_hook(da_mon);
> -	da_mon->monitoring = 0;
> +	WRITE_ONCE(da_mon->monitoring, 0);
>  	da_mon->curr_state = model_get_initial_state();
>  }

Looking at this again, do you need to change it to

static inline void da_monitor_reset(struct da_monitor *da_mon)
{
	WRITE_ONCE(da_mon->monitoring, 0);
        smp_mb();
	da_monitor_reset_hook(da_mon);
	da_mon->curr_state = model_get_initial_state();
}

To prevent another task from seeing monitoring=1 while the timer is
already cancelled?

Nam

Re: [PATCH v3 05/13] rv: Fix monitor start ordering and memory ordering for monitoring flag

[Gabriele Monaco]

On Mon, 2026-06-01 at 08:55 +0200, Nam Cao wrote:
> > diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> > index a7e103654..60dc39f26 100644
> > --- a/include/rv/da_monitor.h
> > +++ b/include/rv/da_monitor.h
> > @@ -82,7 +82,7 @@ static void react(enum states curr_state, enum
> > events event)
> >  static inline void da_monitor_reset(struct da_monitor *da_mon)
> >  {
> >  	da_monitor_reset_hook(da_mon);
> > -	da_mon->monitoring = 0;
> > +	WRITE_ONCE(da_mon->monitoring, 0);
> >  	da_mon->curr_state = model_get_initial_state();
> >  }
> 
> Looking at this again, do you need to change it to
> 
> static inline void da_monitor_reset(struct da_monitor *da_mon)
> {
> 	WRITE_ONCE(da_mon->monitoring, 0);
>         smp_mb();
> 	da_monitor_reset_hook(da_mon);
> 	da_mon->curr_state = model_get_initial_state();
> }
> 

This order won't work.
Monitor reset relies on monitoring to be true to stop the timer. The
same function is called also on monitor start and there the timer may
not have been initialised yet, we use monitoring to discriminate the
two.

> To prevent another task from seeing monitoring=1 while the timer is
> already cancelled?

I'm not sure what could go wrong in this scenario. Perhaps another
event that could start the monitor would miss the chance because it
doesn't see a reaction occurred (monitor looks like still running)?

Or a concurring event would run on top of a reaction, potentially
moving the state machine further or causing another reaction.

Those aren't really disasters and I think could happen also without
enforcing an order with the timer, nothing checks the timer's status.
Following events don't even need to know whether the timer was armed at
all, in the worst case we may be stopping twice.

Am I missing something here?

Thanks,
Gabriele

Re: [PATCH v3 05/13] rv: Fix monitor start ordering and memory ordering for monitoring flag

[Nam Cao]

Gabriele Monaco <gmonaco@redhat.com> writes:
> This order won't work.
> Monitor reset relies on monitoring to be true to stop the timer. The
> same function is called also on monitor start and there the timer may
> not have been initialised yet, we use monitoring to discriminate the
> two.

Right.

> Am I missing something here?

No, just me being confused.

Nam

[PATCH v3 06/13] rv: Do not rely on clean monitor when initialising HA

[Gabriele Monaco]

Hybrid Automata monitors hook into the DA implementation when doing
da_monitor_reset(). This function is called both on initialisation and
teardown, HA monitors try to cancel a timer only when it's initialised
relying on the da_mon->monitoring flag. This flag could however be
corrupted during initialisation. This happens for instance on per-task
monitors that share the same storage with different type of monitors
like LTL or in case of races during a previous teardown.

Stop relying on the monitoring flag during initialisation, assume that
can have any value, so skip timer cancellation in any case when a local
flag is set. New monitors (e.g. new tasks) are always zero-initialised
so they are safe.

Reported-by: Wen Yang <wen.yang@linux.dev>
Closes: https://lore.kernel.org/lkml/d02c656aada7d071f083460a5c9a454363669b61.1778522945.git.wen.yang@linux.dev
Fixes: f5587d1b6ec9 ("rv: Add Hybrid Automata monitor type")
Reviewed-by: Wen Yang <wen.yang@linux.dev>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 include/rv/ha_monitor.h                       | 31 ++++++++++++++++++-
 kernel/trace/rv/monitors/nomiss/nomiss.c      |  4 +--
 kernel/trace/rv/monitors/opid/opid.c          |  4 +--
 kernel/trace/rv/monitors/stall/stall.c        |  4 +--
 .../rvgen/rvgen/templates/dot2k/main.c        |  4 +--
 5 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/include/rv/ha_monitor.h b/include/rv/ha_monitor.h
index d59507e8c..ad3347af1 100644
--- a/include/rv/ha_monitor.h
+++ b/include/rv/ha_monitor.h
@@ -115,6 +115,35 @@ static enum hrtimer_restart ha_monitor_timer_callback(struct hrtimer *hrtimer);
 #define ha_get_ns() 0
 #endif /* HA_CLK_NS */
 
+static bool ha_mon_initializing;
+
+static int ha_monitor_init(void)
+{
+	int ret;
+
+	ha_mon_initializing = true;
+	ret = da_monitor_init();
+	ha_mon_initializing = false;
+	return ret;
+}
+
+static void ha_monitor_destroy(void)
+{
+	da_monitor_destroy();
+}
+
+/*
+ * ha_monitor_uninitialized - are fields like the timer initialized?
+ *
+ * On a clean monitor, we can assume an active monitor (monitoring) is
+ * initialized, however the monitoring field cannot be trusted during
+ * initialization.
+ */
+static inline bool ha_monitor_uninitialized(struct da_monitor *da_mon)
+{
+	return ha_mon_initializing || !da_monitoring(da_mon);
+}
+
 /* Should be supplied by the monitor */
 static u64 ha_get_env(struct ha_monitor *ha_mon, enum envs env, u64 time_ns);
 static bool ha_verify_constraint(struct ha_monitor *ha_mon,
@@ -159,7 +188,7 @@ static inline void ha_monitor_reset_env(struct da_monitor *da_mon)
 	struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
 
 	/* Initialisation resets the monitor before initialising the timer */
-	if (likely(da_monitoring(da_mon)))
+	if (likely(!ha_monitor_uninitialized(da_mon)))
 		ha_cancel_timer(ha_mon);
 }
 
diff --git a/kernel/trace/rv/monitors/nomiss/nomiss.c b/kernel/trace/rv/monitors/nomiss/nomiss.c
index 31f90f363..8ead8783c 100644
--- a/kernel/trace/rv/monitors/nomiss/nomiss.c
+++ b/kernel/trace/rv/monitors/nomiss/nomiss.c
@@ -227,7 +227,7 @@ static int enable_nomiss(void)
 {
 	int retval;
 
-	retval = da_monitor_init();
+	retval = ha_monitor_init();
 	if (retval)
 		return retval;
 
@@ -263,7 +263,7 @@ static void disable_nomiss(void)
 	rv_detach_trace_probe("nomiss", sched_switch, handle_sched_switch);
 	rv_detach_trace_probe("nomiss", sched_wakeup, handle_sched_wakeup);
 
-	da_monitor_destroy();
+	ha_monitor_destroy();
 }
 
 static struct rv_monitor rv_this = {
diff --git a/kernel/trace/rv/monitors/opid/opid.c b/kernel/trace/rv/monitors/opid/opid.c
index 4594c7c46..2922318c6 100644
--- a/kernel/trace/rv/monitors/opid/opid.c
+++ b/kernel/trace/rv/monitors/opid/opid.c
@@ -73,7 +73,7 @@ static int enable_opid(void)
 {
 	int retval;
 
-	retval = da_monitor_init();
+	retval = ha_monitor_init();
 	if (retval)
 		return retval;
 
@@ -90,7 +90,7 @@ static void disable_opid(void)
 	rv_detach_trace_probe("opid", sched_set_need_resched_tp, handle_sched_need_resched);
 	rv_detach_trace_probe("opid", sched_waking, handle_sched_waking);
 
-	da_monitor_destroy();
+	ha_monitor_destroy();
 }
 
 /*
diff --git a/kernel/trace/rv/monitors/stall/stall.c b/kernel/trace/rv/monitors/stall/stall.c
index 9ccfda6b0..3c38fb1a0 100644
--- a/kernel/trace/rv/monitors/stall/stall.c
+++ b/kernel/trace/rv/monitors/stall/stall.c
@@ -103,7 +103,7 @@ static int enable_stall(void)
 {
 	int retval;
 
-	retval = da_monitor_init();
+	retval = ha_monitor_init();
 	if (retval)
 		return retval;
 
@@ -120,7 +120,7 @@ static void disable_stall(void)
 	rv_detach_trace_probe("stall", sched_switch, handle_sched_switch);
 	rv_detach_trace_probe("stall", sched_wakeup, handle_sched_wakeup);
 
-	da_monitor_destroy();
+	ha_monitor_destroy();
 }
 
 static struct rv_monitor rv_this = {
diff --git a/tools/verification/rvgen/rvgen/templates/dot2k/main.c b/tools/verification/rvgen/rvgen/templates/dot2k/main.c
index bf0999f66..889446760 100644
--- a/tools/verification/rvgen/rvgen/templates/dot2k/main.c
+++ b/tools/verification/rvgen/rvgen/templates/dot2k/main.c
@@ -35,7 +35,7 @@ static int enable_%%MODEL_NAME%%(void)
 {
 	int retval;
 
-	retval = da_monitor_init();
+	retval = %%MONITOR_CLASS%%_monitor_init();
 	if (retval)
 		return retval;
 
@@ -50,7 +50,7 @@ static void disable_%%MODEL_NAME%%(void)
 
 %%TRACEPOINT_DETACH%%
 
-	da_monitor_destroy();
+	%%MONITOR_CLASS%%_monitor_destroy();
 }
 
 /*
-- 
2.54.0

Re: [PATCH v3 06/13] rv: Do not rely on clean monitor when initialising HA

[Nam Cao]

Gabriele Monaco <gmonaco@redhat.com> writes:
> +static bool ha_mon_initializing;

The global variable makes me a bit uncomfortable (a quick google will
tell why this is not the best pattern).

I am sure there are better ways to differentiate when we are
initializing vs destroying. How about the incomplete sketch below? I
doubt it even builds, just give an idea.

Nam

diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index 39765ff6f098..0549d3a35ee0 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -159,9 +159,14 @@ static struct da_monitor *da_get_monitor(void)
 /*
  * da_monitor_reset_all - reset the single monitor
  */
-static void da_monitor_reset_all(void)
+static void da_monitor_reset_all(void (*reset)(struct da_monitor *da_mon))
 {
-	da_monitor_reset(da_get_monitor());
+	fn(da_get_monitor());
+}
+
+static inline void _da_monitor_init(struct da_monitor *da_mon)
+{
+	memset(da_monitor, 0, sizeof(*da_mon));
 }
 
 /*
@@ -169,7 +174,7 @@ static void da_monitor_reset_all(void)
  */
 static inline int da_monitor_init(void)
 {
-	da_monitor_reset_all();
+	da_monitor_reset_all(_da_monitor_init);
 	return 0;
 }
 
@@ -178,7 +183,7 @@ static inline int da_monitor_init(void)
  */
 static inline void da_monitor_destroy(void)
 {
-	da_monitor_reset_all();
+	da_monitor_reset_all(da_monitor_reset);
 }
 
 #elif RV_MON_TYPE == RV_MON_PER_CPU
@@ -202,14 +207,14 @@ static struct da_monitor *da_get_monitor(void)
 /*
  * da_monitor_reset_all - reset all CPUs' monitor
  */
-static void da_monitor_reset_all(void)
+static void da_monitor_reset_all(void (*reset)(struct da_monitor *da_mon))
 {
 	struct da_monitor *da_mon;
 	int cpu;
 
 	for_each_cpu(cpu, cpu_online_mask) {
 		da_mon = per_cpu_ptr(&DA_MON_NAME, cpu);
-		da_monitor_reset(da_mon);
+		reset(da_mon);
 	}
 }
 
@@ -267,16 +272,16 @@ static inline da_id_type da_get_id(struct da_monitor *da_mon)
 	return da_get_target(da_mon)->pid;
 }
 
-static void da_monitor_reset_all(void)
+static void da_monitor_reset_all(void (*reset)(struct da_monitor *da_mon))
 {
 	struct task_struct *g, *p;
 	int cpu;
 
 	read_lock(&tasklist_lock);
 	for_each_process_thread(g, p)
-		da_monitor_reset(da_get_monitor(p));
+		reset(da_get_monitor(p));
 	for_each_present_cpu(cpu)
-		da_monitor_reset(da_get_monitor(idle_task(cpu)));
+		reset(da_get_monitor(idle_task(cpu)));
 	read_unlock(&tasklist_lock);
 }
 
@@ -483,14 +488,14 @@ static inline void da_destroy_storage(da_id_type id)
 	kfree_rcu(mon_storage, rcu);
 }
 
-static void da_monitor_reset_all(void)
+static void da_monitor_reset_all(void (*reset)(struct da_monitor *da_mon))
 {
 	struct da_monitor_storage *mon_storage;
 	int bkt;
 
 	rcu_read_lock();
 	hash_for_each_rcu(da_monitor_ht, bkt, mon_storage, node)
-		da_monitor_reset(&mon_storage->rv.da_mon);
+		reset(&mon_storage->rv.da_mon);
 	rcu_read_unlock();
 }
 

Re: [PATCH v3 06/13] rv: Do not rely on clean monitor when initialising HA

[Nam Cao]

Nam Cao <namcao@linutronix.de> writes:
> Gabriele Monaco <gmonaco@redhat.com> writes:
>> +static bool ha_mon_initializing;
>
> The global variable makes me a bit uncomfortable (a quick google will
> tell why this is not the best pattern).
>
> I am sure there are better ways to differentiate when we are
> initializing vs destroying. How about the incomplete sketch below? I
> doubt it even builds, just give an idea.

Or instead of function pointer, we can also pass a bool flag whether the
timer should be reset.

Re: [PATCH v3 06/13] rv: Do not rely on clean monitor when initialising HA

[Gabriele Monaco]

On Mon, 2026-06-01 at 09:31 +0200, Nam Cao wrote:
> Nam Cao <namcao@linutronix.de> writes:
> > Gabriele Monaco <gmonaco@redhat.com> writes:
> > > +static bool ha_mon_initializing;
> > 
> > The global variable makes me a bit uncomfortable (a quick google
> > will tell why this is not the best pattern).
> > 
> > I am sure there are better ways to differentiate when we are
> > initializing vs destroying. How about the incomplete sketch below?
> > I doubt it even builds, just give an idea.
> 
> Or instead of function pointer, we can also pass a bool flag whether
> the timer should be reset.

Good point, we could probably do a bit better than the current in
separating initialisation and destruction/reaction. I'm going to have a
thought.
We are also protecting against tasks where the monitor never started,
so they never got initialised before destruction. This makes it harder
to distinguish.

One thing to note is that we should probably keep the reset signature
constant as that's a member in struct rv_monitor. But that's probably
not a big issue.

Thanks,
Gabriele

[PATCH v3 07/13] rv: Add automatic cleanup handlers for per-task HA monitors

[Gabriele Monaco]

Hybrid automata monitors may start timers, depending on the model, these
may remain active on an exiting task and cause false positives or even
access freed memory.

Add an enable/disable hook in the HA code, currently only populated by
the per-task handler for registration and deregistration.
This hooks to the sched_process_exit event and ensures the timer is
stopped for every exiting task. The handler is enabled automatically but
may be disabled, for instance if the monitor uses the event for another
purpose (but should still manually ensure timers are stopped).

Fixes: f5587d1b6ec9 ("rv: Add Hybrid Automata monitor type")
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 include/rv/ha_monitor.h | 45 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/include/rv/ha_monitor.h b/include/rv/ha_monitor.h
index ad3347af1..0aeb6bc38 100644
--- a/include/rv/ha_monitor.h
+++ b/include/rv/ha_monitor.h
@@ -28,6 +28,7 @@ static inline void ha_monitor_init_env(struct da_monitor *da_mon);
 static inline void ha_monitor_reset_env(struct da_monitor *da_mon);
 static inline void ha_setup_timer(struct ha_monitor *ha_mon);
 static inline bool ha_cancel_timer(struct ha_monitor *ha_mon);
+static inline void ha_cancel_timer_sync(struct ha_monitor *ha_mon);
 static bool ha_monitor_handle_constraint(struct da_monitor *da_mon,
 					 enum states curr_state,
 					 enum events event,
@@ -37,6 +38,26 @@ static bool ha_monitor_handle_constraint(struct da_monitor *da_mon,
 #define da_monitor_init_hook ha_monitor_init_env
 #define da_monitor_reset_hook ha_monitor_reset_env
 
+#if !defined(HA_SKIP_AUTO_CLEANUP) && RV_MON_TYPE == RV_MON_PER_TASK
+/*
+ * Automatic cleanup handlers for per-task HA monitors, only skip if you know
+ * what you are doing (e.g. you want to implement cleanup manually in another
+ * handler doing more things).
+ */
+static void ha_handle_sched_process_exit(void *data, struct task_struct *p,
+					 bool group_dead);
+
+#define ha_monitor_enable_hook()                                             \
+	rv_attach_trace_probe(__stringify(MONITOR_NAME), sched_process_exit, \
+			      ha_handle_sched_process_exit)
+#define ha_monitor_disable_hook()                                            \
+	rv_detach_trace_probe(__stringify(MONITOR_NAME), sched_process_exit, \
+			      ha_handle_sched_process_exit)
+#else
+#define ha_monitor_enable_hook() ((void)0)
+#define ha_monitor_disable_hook() ((void)0)
+#endif
+
 #include <rv/da_monitor.h>
 #include <linux/seq_buf.h>
 
@@ -123,12 +144,15 @@ static int ha_monitor_init(void)
 
 	ha_mon_initializing = true;
 	ret = da_monitor_init();
+	if (ret == 0)
+		ha_monitor_enable_hook();
 	ha_mon_initializing = false;
 	return ret;
 }
 
 static void ha_monitor_destroy(void)
 {
+	ha_monitor_disable_hook();
 	da_monitor_destroy();
 }
 
@@ -229,6 +253,18 @@ static inline void ha_trace_error_env(struct ha_monitor *ha_mon,
 {
 	CONCATENATE(trace_error_env_, MONITOR_NAME)(id, curr_state, event, env);
 }
+
+#if !defined(HA_SKIP_AUTO_CLEANUP) && RV_MON_TYPE == RV_MON_PER_TASK
+static void ha_handle_sched_process_exit(void *data, struct task_struct *p,
+					 bool group_dead)
+{
+	struct da_monitor *da_mon = da_get_monitor(p);
+
+	if (likely(!ha_monitor_uninitialized(da_mon)))
+		ha_cancel_timer_sync(to_ha_monitor(da_mon));
+}
+#endif
+
 #endif /* RV_MON_TYPE */
 
 /*
@@ -441,6 +477,10 @@ static inline bool ha_cancel_timer(struct ha_monitor *ha_mon)
 {
 	return timer_delete(&ha_mon->timer);
 }
+static inline void ha_cancel_timer_sync(struct ha_monitor *ha_mon)
+{
+	timer_delete_sync(&ha_mon->timer);
+}
 #elif HA_TIMER_TYPE == HA_TIMER_HRTIMER
 /*
  * Helper functions to handle the monitor timer.
@@ -492,6 +532,10 @@ static inline bool ha_cancel_timer(struct ha_monitor *ha_mon)
 {
 	return hrtimer_try_to_cancel(&ha_mon->hrtimer) == 1;
 }
+static inline void ha_cancel_timer_sync(struct ha_monitor *ha_mon)
+{
+	hrtimer_cancel(&ha_mon->hrtimer);
+}
 #else /* HA_TIMER_NONE */
 /*
  * Start function is intentionally not defined, monitors using timers must
@@ -502,6 +546,7 @@ static inline bool ha_cancel_timer(struct ha_monitor *ha_mon)
 {
 	return false;
 }
+static inline void ha_cancel_timer_sync(struct ha_monitor *ha_mon) { }
 #endif
 
 #endif
-- 
2.54.0

Re: [PATCH v3 07/13] rv: Add automatic cleanup handlers for per-task HA monitors

[Nam Cao]

Gabriele Monaco <gmonaco@redhat.com> writes:
> @@ -123,12 +144,15 @@ static int ha_monitor_init(void)
>  
>  	ha_mon_initializing = true;
>  	ret = da_monitor_init();
> +	if (ret == 0)
> +		ha_monitor_enable_hook();
>  	ha_mon_initializing = false;
>  	return ret;
>  }

What if between da_monitor_init() and ha_monitor_enable_hook(), a task
exits while a timer is still active, and then the timer callback is invoked?

Extremely rare, but I think it can be fixed easily by reordering the two functions.

>  static void ha_monitor_destroy(void)
>  {
> +	ha_monitor_disable_hook();
>  	da_monitor_destroy();
>  }

Same here, there is small window between the two function calls.

Nam

Re: [PATCH v3 07/13] rv: Add automatic cleanup handlers for per-task HA monitors

[Gabriele Monaco]

On Mon, 2026-06-01 at 09:39 +0200, Nam Cao wrote:
> Gabriele Monaco <gmonaco@redhat.com> writes:
> > @@ -123,12 +144,15 @@ static int ha_monitor_init(void)
> >  
> >  	ha_mon_initializing = true;
> >  	ret = da_monitor_init();
> > +	if (ret == 0)
> > +		ha_monitor_enable_hook();
> >  	ha_mon_initializing = false;
> >  	return ret;
> >  }
> 
> What if between da_monitor_init() and ha_monitor_enable_hook(), a
> task exits while a timer is still active, and then the timer callback
> is invoked?

We are initialising, timers shouldn't be active, that sits right before
setting up other hooks, and the exit hooks this way is just the first
of them.

By the way, in this case, we likely have a valid reset scenario on an
invalid (uninitialised) timer. This is also what checking the
monitoring flag guards against.
In short, in that handler we really should reset, but need to know
whether we ever initialised in the first place.

> Extremely rare, but I think it can be fixed easily by reordering the
> two functions.
> 
> >  static void ha_monitor_destroy(void)
> >  {
> > +	ha_monitor_disable_hook();
> >  	da_monitor_destroy();
> >  }
> 
> Same here, there is small window between the two function calls.

Likewise here, we removed all hooks, then da_monitor_destroy() is going
to sync with them and clean everything up. Swapping them will expose
more races because we dropped the slot by then.

Am I missing something?

Thanks,
Gabriele

Re: [PATCH v3 07/13] rv: Add automatic cleanup handlers for per-task HA monitors

[Nam Cao]

Gabriele Monaco <gmonaco@redhat.com> writes:
> Hybrid automata monitors may start timers, depending on the model, these
> may remain active on an exiting task and cause false positives or even
> access freed memory.
>
> Add an enable/disable hook in the HA code, currently only populated by
> the per-task handler for registration and deregistration.
> This hooks to the sched_process_exit event and ensures the timer is
> stopped for every exiting task. The handler is enabled automatically but
> may be disabled, for instance if the monitor uses the event for another
> purpose (but should still manually ensure timers are stopped).
>
> Fixes: f5587d1b6ec9 ("rv: Add Hybrid Automata monitor type")
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>

Reviewed-by: Nam Cao <namcao@linutronix.de>

[PATCH v3 08/13] rv: Ensure synchronous cleanup for HA monitors

[Gabriele Monaco]

HA monitors may start timers, all cleanup functions currently stop the
timers asynchronously to avoid sleeping in the wrong context.
Nothing makes sure running callbacks terminate on cleanup.

Run the entire HA timer callback in an RCU read-side critical section,
this way we can simply synchronize_rcu() with any pending timer and are
sure any cleanup using kfree_rcu() runs after callbacks terminated.
Additionally make sure any unlikely callback running late won't run any
code if the monitor is marked as disabled or if destruction started.
Use memory barriers to serialise with racing resets.

Fixes: f5587d1b6ec9 ("rv: Add Hybrid Automata monitor type")
Fixes: 4a24127bd6cb ("rv: Add support for per-object monitors in DA/HA")
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 include/rv/da_monitor.h | 26 +++++++++++++++++++++-----
 include/rv/ha_monitor.h | 28 +++++++++++++++++++++++++---
 2 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index 60dc39f26..03c1150b1 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -57,6 +57,15 @@ static struct rv_monitor rv_this;
 #define da_monitor_reset_hook(da_mon)
 #endif
 
+/*
+ * Hook to allow the implementation of hybrid automata: define it with a
+ * function that waits for the termination of all monitors background
+ * activities (e.g. all timers). This hook can sleep.
+ */
+#ifndef da_monitor_sync_hook
+#define da_monitor_sync_hook()
+#endif
+
 /*
  * Type for the target id, default to int but can be overridden.
  * A long type can work as hash table key (PER_OBJ) but will be downgraded to
@@ -83,7 +92,8 @@ static inline void da_monitor_reset(struct da_monitor *da_mon)
 {
 	da_monitor_reset_hook(da_mon);
 	WRITE_ONCE(da_mon->monitoring, 0);
-	da_mon->curr_state = model_get_initial_state();
+	/* Pair with load in __ha_monitor_timer_callback */
+	smp_store_release(&da_mon->curr_state, model_get_initial_state());
 }
 
 /*
@@ -181,6 +191,7 @@ static inline int da_monitor_init(void)
 static inline void da_monitor_destroy(void)
 {
 	da_monitor_reset_all();
+	da_monitor_sync_hook();
 }
 
 #elif RV_MON_TYPE == RV_MON_PER_CPU
@@ -230,6 +241,7 @@ static inline int da_monitor_init(void)
 static inline void da_monitor_destroy(void)
 {
 	da_monitor_reset_all();
+	da_monitor_sync_hook();
 }
 
 #elif RV_MON_TYPE == RV_MON_PER_TASK
@@ -317,6 +329,7 @@ static inline void da_monitor_destroy(void)
 
 	tracepoint_synchronize_unregister();
 	da_monitor_reset_all();
+	da_monitor_sync_hook();
 
 	rv_put_task_monitor_slot(task_mon_slot);
 	task_mon_slot = RV_PER_TASK_MONITOR_INIT;
@@ -495,10 +508,9 @@ static void da_monitor_reset_all(void)
 	struct da_monitor_storage *mon_storage;
 	int bkt;
 
-	rcu_read_lock();
+	guard(rcu)();
 	hash_for_each_rcu(da_monitor_ht, bkt, mon_storage, node)
 		da_monitor_reset(&mon_storage->rv.da_mon);
-	rcu_read_unlock();
 }
 
 static inline int da_monitor_init(void)
@@ -514,13 +526,17 @@ static inline void da_monitor_destroy(void)
 	int bkt;
 
 	tracepoint_synchronize_unregister();
+	scoped_guard(rcu) {
+		hash_for_each_rcu(da_monitor_ht, bkt, mon_storage, node) {
+			da_monitor_reset_hook(&mon_storage->rv.da_mon);
+		}
+	}
+	da_monitor_sync_hook();
 	/*
 	 * This function is called after all probes are disabled and no longer
 	 * pending, we can safely assume no concurrent user.
 	 */
-	synchronize_rcu();
 	hash_for_each_safe(da_monitor_ht, bkt, tmp, mon_storage, node) {
-		da_monitor_reset_hook(&mon_storage->rv.da_mon);
 		hash_del_rcu(&mon_storage->node);
 		kfree(mon_storage);
 	}
diff --git a/include/rv/ha_monitor.h b/include/rv/ha_monitor.h
index 0aeb6bc38..86bd1cab8 100644
--- a/include/rv/ha_monitor.h
+++ b/include/rv/ha_monitor.h
@@ -37,6 +37,7 @@ static bool ha_monitor_handle_constraint(struct da_monitor *da_mon,
 #define da_monitor_event_hook ha_monitor_handle_constraint
 #define da_monitor_init_hook ha_monitor_init_env
 #define da_monitor_reset_hook ha_monitor_reset_env
+#define da_monitor_sync_hook() synchronize_rcu()
 
 #if !defined(HA_SKIP_AUTO_CLEANUP) && RV_MON_TYPE == RV_MON_PER_TASK
 /*
@@ -137,11 +138,13 @@ static enum hrtimer_restart ha_monitor_timer_callback(struct hrtimer *hrtimer);
 #endif /* HA_CLK_NS */
 
 static bool ha_mon_initializing;
+static bool ha_mon_destroying;
 
 static int ha_monitor_init(void)
 {
 	int ret;
 
+	WRITE_ONCE(ha_mon_destroying, false);
 	ha_mon_initializing = true;
 	ret = da_monitor_init();
 	if (ret == 0)
@@ -152,6 +155,7 @@ static int ha_monitor_init(void)
 
 static void ha_monitor_destroy(void)
 {
+	WRITE_ONCE(ha_mon_destroying, true);
 	ha_monitor_disable_hook();
 	da_monitor_destroy();
 }
@@ -302,12 +306,30 @@ static bool ha_monitor_handle_constraint(struct da_monitor *da_mon,
 	return false;
 }
 
+/*
+ * __ha_monitor_timer_callback - generic callback representation
+ *
+ * This callback runs in an RCU read-side critical section to allow the
+ * destruction sequence to easily synchronize_rcu() with all pending timers
+ * after asynchronously disabling them. The ha_mon_destroying check ensures
+ * any callback entering the RCU section after synchronize_rcu() completes
+ * will see the flag and bail out immediately.
+ */
 static inline void __ha_monitor_timer_callback(struct ha_monitor *ha_mon)
 {
-	enum states curr_state = READ_ONCE(ha_mon->da_mon.curr_state);
 	DECLARE_SEQ_BUF(env_string, ENV_BUFFER_SIZE);
-	u64 time_ns = ha_get_ns();
-
+	enum states curr_state;
+	u64 time_ns;
+
+	guard(rcu)();
+	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);
+	if (unlikely(!da_monitor_handling_event(&ha_mon->da_mon)))
+		return;
+
+	time_ns = ha_get_ns();
 	ha_get_env_string(&env_string, ha_mon, time_ns);
 	ha_react(curr_state, EVENT_NONE, env_string.buffer);
 	ha_trace_error_env(ha_mon, model_get_state_name(curr_state),
-- 
2.54.0

Re: [PATCH v3 08/13] rv: Ensure synchronous cleanup for HA monitors

[Nam Cao]

>  static bool ha_mon_initializing;
> +static bool ha_mon_destroying;
>  
>  static int ha_monitor_init(void)
>  {
>  	int ret;
>  
> +	WRITE_ONCE(ha_mon_destroying, false);
>  	ha_mon_initializing = true;
>  	ret = da_monitor_init();
>  	if (ret == 0)
> @@ -152,6 +155,7 @@ static int ha_monitor_init(void)
>  
>  static void ha_monitor_destroy(void)
>  {
> +	WRITE_ONCE(ha_mon_destroying, true);
>  	ha_monitor_disable_hook();
>  	da_monitor_destroy();
>  }
> @@ -302,12 +306,30 @@ static bool ha_monitor_handle_constraint(struct da_monitor *da_mon,
>  	return false;
>  }
>  
> +/*
> + * __ha_monitor_timer_callback - generic callback representation
> + *
> + * This callback runs in an RCU read-side critical section to allow the
> + * destruction sequence to easily synchronize_rcu() with all pending timers
> + * after asynchronously disabling them. The ha_mon_destroying check ensures
> + * any callback entering the RCU section after synchronize_rcu() completes
> + * will see the flag and bail out immediately.
> + */
>  static inline void __ha_monitor_timer_callback(struct ha_monitor *ha_mon)
>  {
> -	enum states curr_state = READ_ONCE(ha_mon->da_mon.curr_state);
>  	DECLARE_SEQ_BUF(env_string, ENV_BUFFER_SIZE);
> -	u64 time_ns = ha_get_ns();
> -
> +	enum states curr_state;
> +	u64 time_ns;
> +
> +	guard(rcu)();
> +	if (unlikely(READ_ONCE(ha_mon_destroying)))

Instead of this global variable, can we instead use da_mon->monitoring?

> +		return;
> +	/* Ensure consistent curr_state if we race with da_monitor_reset */
> +	curr_state = smp_load_acquire(&ha_mon->da_mon.curr_state);
> +	if (unlikely(!da_monitor_handling_event(&ha_mon->da_mon)))
> +		return;
> +
> +	time_ns = ha_get_ns();
>  	ha_get_env_string(&env_string, ha_mon, time_ns);
>  	ha_react(curr_state, EVENT_NONE, env_string.buffer);
>  	ha_trace_error_env(ha_mon, model_get_state_name(curr_state),
> -- 
> 2.54.0

Re: [PATCH v3 08/13] rv: Ensure synchronous cleanup for HA monitors

[Gabriele Monaco]

On Mon, 2026-06-01 at 09:52 +0200, Nam Cao wrote:
> > +/*
> > + * __ha_monitor_timer_callback - generic callback representation
> > + *
> > + * This callback runs in an RCU read-side critical section to
> > allow the
> > + * destruction sequence to easily synchronize_rcu() with all
> > pending timers
> > + * after asynchronously disabling them. The ha_mon_destroying
> > check ensures
> > + * any callback entering the RCU section after synchronize_rcu()
> > completes
> > + * will see the flag and bail out immediately.
> > + */
> >  static inline void __ha_monitor_timer_callback(struct ha_monitor
> > *ha_mon)
> >  {
> > -	enum states curr_state = READ_ONCE(ha_mon-
> > >da_mon.curr_state);
> >  	DECLARE_SEQ_BUF(env_string, ENV_BUFFER_SIZE);
> > -	u64 time_ns = ha_get_ns();
> > -
> > +	enum states curr_state;
> > +	u64 time_ns;
> > +
> > +	guard(rcu)();
> > +	if (unlikely(READ_ONCE(ha_mon_destroying)))
> 
> Instead of this global variable, can we instead use da_mon-
> >monitoring?
> 

We already do in da_monitor_handling_event(), this is guarding for the
very unlikely scenario of:

1. timer callback firing (just before guard(rcu))
2. sync rcu, return before waiting for that one
3. reset and free memory
4. guard(rcu) now and check the state in da_mon (freed)

I'm not too confident saying this cannot happen under PREEMPT_RT.

I know the easy alternative would be to synchronously stop timers on
destruction, but that gets complicated with per-task monitors, where we
iterate over task grabbing a read lock.

Am I just too paranoid?

Thanks,
Gabriele

> > +		return;
> > +	/* Ensure consistent curr_state if we race with
> > da_monitor_reset */
> > +	curr_state = smp_load_acquire(&ha_mon->da_mon.curr_state);
> > +	if (unlikely(!da_monitor_handling_event(&ha_mon->da_mon)))
> > +		return;
> > +
> > +	time_ns = ha_get_ns();
> >  	ha_get_env_string(&env_string, ha_mon, time_ns);
> >  	ha_react(curr_state, EVENT_NONE, env_string.buffer);
> >  	ha_trace_error_env(ha_mon,
> > model_get_state_name(curr_state),
> > -- 
> > 2.54.0

[PATCH v3 09/13] rv: Prevent task migration while handling per-CPU events

[Gabriele Monaco]

Tracepoint handlers are fully preemptible after a46023d5616 ("tracing:
Guard __DECLARE_TRACE() use of __DO_TRACE_CALL() with SRCU-fast"). When
a per-CPU monitor handles an event, it retrieves the monitor state using
a per-CPU pointer. If the event itself doesn't disable preemption, the
task can migrate to a different CPU and we risk updating the wrong
monitor.

Mitigate this by explicitly disabling task migration before acquiring
the monitor pointer. This cannot guarantee the monitor runs on the
correct CPU but reduces the race condition window and prevents warnings.

Reviewed-by: Wen Yang <wen.yang@linux.dev>
Reviewed-by: Nam Cao <namcao@linutronix.de>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 include/rv/da_monitor.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index 03c1150b1..ae8c35fcb 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -194,6 +194,10 @@ static inline void da_monitor_destroy(void)
 	da_monitor_sync_hook();
 }
 
+#ifndef da_implicit_guard
+#define da_implicit_guard()
+#endif
+
 #elif RV_MON_TYPE == RV_MON_PER_CPU
 /*
  * Functions to define, init and get a per-cpu monitor.
@@ -244,6 +248,10 @@ static inline void da_monitor_destroy(void)
 	da_monitor_sync_hook();
 }
 
+#ifndef da_implicit_guard
+#define da_implicit_guard() guard(migrate)()
+#endif
+
 #elif RV_MON_TYPE == RV_MON_PER_TASK
 /*
  * Functions to define, init and get a per-task monitor.
@@ -700,6 +708,7 @@ static inline bool __da_handle_start_run_event(struct da_monitor *da_mon,
  */
 static inline void da_handle_event(enum events event)
 {
+	da_implicit_guard();
 	__da_handle_event(da_get_monitor(), event, 0);
 }
 
@@ -715,6 +724,7 @@ static inline void da_handle_event(enum events event)
  */
 static inline bool da_handle_start_event(enum events event)
 {
+	da_implicit_guard();
 	return __da_handle_start_event(da_get_monitor(), event, 0);
 }
 
@@ -726,6 +736,7 @@ static inline bool da_handle_start_event(enum events event)
  */
 static inline bool da_handle_start_run_event(enum events event)
 {
+	da_implicit_guard();
 	return __da_handle_start_run_event(da_get_monitor(), event, 0);
 }
 
-- 
2.54.0

[PATCH v3 10/13] rv: Use 0 to check preemption enabled in opid

[Gabriele Monaco]

Tracepoint handlers no longer run with preemption disabled by default
since a46023d5616 ("tracing: Guard __DECLARE_TRACE() use of
__DO_TRACE_CALL() with SRCU-fast"), the opid monitor should now count 1
in the preemption count as preemption disabled.

Change the rule for preempt_off to preempt > 0.

Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 kernel/trace/rv/monitors/opid/opid.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/kernel/trace/rv/monitors/opid/opid.c b/kernel/trace/rv/monitors/opid/opid.c
index 2922318c6..3b6a85e81 100644
--- a/kernel/trace/rv/monitors/opid/opid.c
+++ b/kernel/trace/rv/monitors/opid/opid.c
@@ -22,14 +22,8 @@ static u64 ha_get_env(struct ha_monitor *ha_mon, enum envs_opid env, u64 time_ns
 	if (env == irq_off_opid)
 		return irqs_disabled();
 	else if (env == preempt_off_opid) {
-		/*
-		 * If CONFIG_PREEMPTION is enabled, then the tracepoint itself disables
-		 * preemption (adding one to the preempt_count). Since we are
-		 * interested in the preempt_count at the time the tracepoint was
-		 * hit, we consider 1 as still enabled.
-		 */
 		if (IS_ENABLED(CONFIG_PREEMPTION))
-			return (preempt_count() & PREEMPT_MASK) > 1;
+			return (preempt_count() & PREEMPT_MASK) > 0;
 		return true;
 	}
 	return ENV_INVALID_VALUE;
-- 
2.54.0

Re: [PATCH v3 10/13] rv: Use 0 to check preemption enabled in opid

[Nam Cao]

Gabriele Monaco <gmonaco@redhat.com> writes:
> Tracepoint handlers no longer run with preemption disabled by default
> since a46023d5616 ("tracing: Guard __DECLARE_TRACE() use of
> __DO_TRACE_CALL() with SRCU-fast"), the opid monitor should now count 1
> in the preemption count as preemption disabled.
>
> Change the rule for preempt_off to preempt > 0.
>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>

Reviewed-by: Nam Cao <namcao@linutronix.de>

[PATCH v3 11/13] verification/rvgen: Fix suffix strip in dot2k

[Gabriele Monaco]

__start_to_invariant_check() and __get_constraint_env() parse the
environment variable's name from sources that have it padded with the
monitor name. This is removed using rstrip(), which is not meant to
strip a substring but rather a set of characters.

Use removesuffix() to actually get rid of the trailing _<monitor name>.

Fixes: a82adadb16894 ("verification/rvgen: Add support for Hybrid Automata")
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 tools/verification/rvgen/rvgen/dot2k.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/verification/rvgen/rvgen/dot2k.py b/tools/verification/rvgen/rvgen/dot2k.py
index e6f476b90..110cfd69e 100644
--- a/tools/verification/rvgen/rvgen/dot2k.py
+++ b/tools/verification/rvgen/rvgen/dot2k.py
@@ -215,14 +215,14 @@ class ha2k(dot2k):
     def __get_constraint_env(self, constr: str) -> str:
         """Extract the second argument from an ha_ function"""
         env = constr.split("(")[1].split()[1].rstrip(")").rstrip(",")
-        assert env.rstrip(f"_{self.name}") in self.envs
+        assert env.removesuffix(f"_{self.name}") in self.envs
         return env
 
     def __start_to_invariant_check(self, constr: str) -> str:
         # by default assume the timer has ns expiration
         env = self.__get_constraint_env(constr)
         clock_type = "ns"
-        if self.env_types.get(env.rstrip(f"_{self.name}")) == "j":
+        if self.env_types.get(env.removesuffix(f"_{self.name}")) == "j":
             clock_type = "jiffy"
 
         return f"return ha_check_invariant_{clock_type}(ha_mon, {env}, time_ns)"
-- 
2.54.0

Re: [PATCH v3 11/13] verification/rvgen: Fix suffix strip in dot2k

[Nam Cao]

Gabriele Monaco <gmonaco@redhat.com> writes:

> __start_to_invariant_check() and __get_constraint_env() parse the
> environment variable's name from sources that have it padded with the
> monitor name. This is removed using rstrip(), which is not meant to
> strip a substring but rather a set of characters.
>
> Use removesuffix() to actually get rid of the trailing _<monitor name>.
>
> Fixes: a82adadb16894 ("verification/rvgen: Add support for Hybrid Automata")
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>

Reviewed-by: Nam Cao <namcao@linutronix.de>

[PATCH v3 12/13] rv: Fix read_lock scope in per-task DA cleanup

[Gabriele Monaco]

The da_monitor_reset_all() function for per-task monitors takes
tasklist_lock while iterating over tasks, then keeps it also while
iterating over idle tasks (one per CPU). The latter is not necessary
since the lock needs to guard only for_each_process_thread().

Use a scoped_guard for more compact syntax and adjust the scope only
where the lock is necessary.

Reviewed-by: Wen Yang <wen.yang@linux.dev>
Reviewed-by: Nam Cao <namcao@linutronix.de>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 include/rv/da_monitor.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index ae8c35fcb..f4fc22cac 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -294,12 +294,12 @@ static void da_monitor_reset_all(void)
 	struct task_struct *g, *p;
 	int cpu;
 
-	read_lock(&tasklist_lock);
-	for_each_process_thread(g, p)
-		da_monitor_reset(da_get_monitor(p));
+	scoped_guard(read_lock, &tasklist_lock) {
+		for_each_process_thread(g, p)
+			da_monitor_reset(da_get_monitor(p));
+	}
 	for_each_present_cpu(cpu)
 		da_monitor_reset(da_get_monitor(idle_task(cpu)));
-	read_unlock(&tasklist_lock);
 }
 
 /*
-- 
2.54.0

[PATCH v3 13/13] verification/rvgen: Generate cleanup hook for per-obj monitor

[Gabriele Monaco]

Per-object monitors can allocate memory dynamically and such memory is
required for the lifetime of the object, then it should be freed with
the appropriate call.

Force the generation scripts to add a cleanup function the user will
need to wire to the appropriate event (e.g. sched_process_exit for
tasks). This can be safely removed if the object will never cease to
exist before disabling the monitor (e.g. if following only static
variables).

Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 tools/verification/rvgen/rvgen/dot2k.py | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/tools/verification/rvgen/rvgen/dot2k.py b/tools/verification/rvgen/rvgen/dot2k.py
index 110cfd69e..3060aa4b9 100644
--- a/tools/verification/rvgen/rvgen/dot2k.py
+++ b/tools/verification/rvgen/rvgen/dot2k.py
@@ -17,6 +17,9 @@ from .automata import _EventConstraintKey, _StateConstraintKey, AutomataError
 class dot2k(Monitor, Dot2c):
     template_dir = "dot2k"
 
+    # only needed for the per-obj cleanup hook
+    cleanup_marker = "obj_cleanup"
+
     def __init__(self, file_path, MonitorType, extra_params={}):
         self.monitor_type = MonitorType
         Monitor.__init__(self, extra_params)
@@ -56,18 +59,30 @@ class dot2k(Monitor, Dot2c):
                 buff.append(f"\tda_{handle}({event}{self.enum_suffix});")
             buff.append("}")
             buff.append("")
+        if self.monitor_type == "per_obj":
+            buff.append("/* XXX: obj is being destroyed, remove if not required (e.g. obj is static) */")
+            buff.append(f"static void handle_{self.cleanup_marker}(void *data, /* XXX: fill header */)")
+            buff.append("{")
+            buff.append("\tint id = /* XXX: how do I get the id? */;")
+            buff.append("\tda_destroy_storage(id);")
+            buff.append("}")
+            buff.append("")
         return '\n'.join(buff)
 
     def fill_tracepoint_attach_probe(self) -> str:
         buff = []
         for event in self.events:
             buff.append(f"\trv_attach_trace_probe(\"{self.name}\", /* XXX: tracepoint */, handle_{event});")
+        if self.monitor_type == "per_obj":
+            buff.append(f"\trv_attach_trace_probe(\"{self.name}\", /* XXX: cleanup tracepoint */, handle_{self.cleanup_marker});")
         return '\n'.join(buff)
 
     def fill_tracepoint_detach_helper(self) -> str:
         buff = []
         for event in self.events:
             buff.append(f"\trv_detach_trace_probe(\"{self.name}\", /* XXX: tracepoint */, handle_{event});")
+        if self.monitor_type == "per_obj":
+            buff.append(f"\trv_detach_trace_probe(\"{self.name}\", /* XXX: cleanup tracepoint */, handle_{self.cleanup_marker});")
         return '\n'.join(buff)
 
     def fill_model_h_header(self) -> list[str]:
-- 
2.54.0

Re: [PATCH v3 13/13] verification/rvgen: Generate cleanup hook for per-obj monitor

[Nam Cao]

Gabriele Monaco <gmonaco@redhat.com> writes:

> Per-object monitors can allocate memory dynamically and such memory is
> required for the lifetime of the object, then it should be freed with
> the appropriate call.
>
> Force the generation scripts to add a cleanup function the user will
> need to wire to the appropriate event (e.g. sched_process_exit for
> tasks). This can be safely removed if the object will never cease to
> exist before disabling the monitor (e.g. if following only static
> variables).
>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>

Reviewed-by: Nam Cao <namcao@linutronix.de>

Re: [PATCH v3 00/13] rv: Fixes on Deterministic and Hybrid Automata

[Wen Yang]

Thank you for this series -- it addresses several issues in the RV 
framework. Our pending series [PATCH v3 tlob] also depends on it; we 
would appreciate if it could be picked up for the next merge window.


--
Best wishes,
Wen


On 5/30/26 22:16, Gabriele Monaco wrote:
> Fix issues that were reported by bots or visible only after integration:
>   * Make sure timers are always terminated and waited for when disabling
>     the monitor or when the target terminates
>   * Run per-cpu monitors with migration disabled since preemption is now
>     enabled from tracepoints
>   * Fix a wrong __user specifier in a helper function
>   * Other cleanup and concurrency issues
> 
> Differences since V2 [1]:
> * Applied from reviews changes to commit messages
> * Rearranged order to put non-fixes and not-reviewed patches in the end
> * Synchronise monitors before resetting them to avoid rearming
> * Protect against racing timer callbacks during destruction
> 
> Differences since V1 [2]:
> * Fix memory consistency with timer callbacks racing with resets
> * Add per-obj deallocation hook in rvgen generated code
> * Do not rely on clean monitor when initialising HA
> * Add tracepoint synchronisation before returning per-task slots
> * Fix suffix strip in dot2k
> * Generate stub deallocation hooks instead of failing build when per-obj
>    miss those
> 
> [1] - https://lore.kernel.org/lkml/20260527062313.39908-1-gmonaco@redhat.com
> [2] - https://lore.kernel.org/lkml/20260512140250.262190-1-gmonaco@redhat.com
> 
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Nam Cao <namcao@linutronix.de>
> Cc: Wen Yang <wen.yang@linux.dev>
> Cc: linux-trace-kernel@vger.kernel.org
> 
> Gabriele Monaco (12):
>    rv: Fix __user specifier usage in extract_params()
>    rv: Reset per-task DA monitors before releasing the slot
>    rv: Prevent in-flight per-task handlers from using invalid slots
>    rv: Ensure all pending probes terminate on per-obj monitor destroy
>    rv: Do not rely on clean monitor when initialising HA
>    rv: Add automatic cleanup handlers for per-task HA monitors
>    rv: Ensure synchronous cleanup for HA monitors
>    rv: Prevent task migration while handling per-CPU events
>    rv: Use 0 to check preemption enabled in opid
>    verification/rvgen: Fix suffix strip in dot2k
>    rv: Fix read_lock scope in per-task DA cleanup
>    verification/rvgen: Generate cleanup hook for per-obj monitor
> 
> Wen Yang (1):
>    rv: Fix monitor start ordering and memory ordering for monitoring flag
> 
>   include/rv/da_monitor.h                       |  67 ++++++++---
>   include/rv/ha_monitor.h                       | 104 +++++++++++++++++-
>   include/rv/ltl_monitor.h                      |   1 +
>   kernel/trace/rv/monitors/deadline/deadline.h  |   3 +-
>   kernel/trace/rv/monitors/nomiss/nomiss.c      |   4 +-
>   kernel/trace/rv/monitors/opid/opid.c          |  12 +-
>   kernel/trace/rv/monitors/stall/stall.c        |   4 +-
>   tools/verification/rvgen/rvgen/dot2k.py       |  19 +++-
>   .../rvgen/rvgen/templates/dot2k/main.c        |   4 +-
>   9 files changed, 180 insertions(+), 38 deletions(-)
> 
> 
> base-commit: 8fde5d1d47f69db6082dfa34500c27f8485389a5