rv: Do not rely on clean monitor when initialising HA

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 use a separate da_reset_state() skiping timer
cancellation.
New monitors (e.g. new tasks) are always zero-initialised so it is safe
to rely on the monitoring flag for those.

Reported-by: Wen Yang <wen.yang@linux.dev>
Closes: https://lore.kernel.org/lkml/d02c656aada7d071f083460a5c9a454363669b61.1778522945.git.wen.yang@linux.dev
Suggested-by: Nam Cao <namcao@linutronix.de>
Fixes: f5587d1b6ec9 ("rv: Add Hybrid Automata monitor type")
Reviewed-by: Wen Yang <wen.yang@linux.dev>
Reviewed-by: Nam Cao <namcao@linutronix.de>
Link: https://lore.kernel.org/r/20260601153840.124372-7-gmonaco@redhat.com
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>

diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index 60dc39f2620a2d7bce2ba07d80a89e850a7e9f31..ec9bc88bd4c42caca95d08808d49975be24e57e2 100644 (file)
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -76,14 +76,22 @@ static void react(enum states curr_state, enum events event)
                 model_get_state_name(curr_state));
 }
 
+/*
+ * da_monitor_reset_state - reset a monitor and setting it to init state
+ */
+static inline void da_monitor_reset_state(struct da_monitor *da_mon)
+{
+       WRITE_ONCE(da_mon->monitoring, 0);
+       da_mon->curr_state = model_get_initial_state();
+}
+
 /*
  * da_monitor_reset - reset a monitor and setting it to init state
  */
 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();
+       da_monitor_reset_state(da_mon);
 }
 
 /*
@@ -158,12 +166,28 @@ static struct da_monitor *da_get_monitor(void)
        return &DA_MON_NAME;
 }
 
+/*
+ * __da_monitor_reset_all - reset the single monitor
+ */
+static void __da_monitor_reset_all(void (*reset)(struct da_monitor *))
+{
+       reset(da_get_monitor());
+}
+
 /*
  * da_monitor_reset_all - reset the single monitor
  */
 static void da_monitor_reset_all(void)
 {
-       da_monitor_reset(da_get_monitor());
+       __da_monitor_reset_all(da_monitor_reset);
+}
+
+/*
+ * da_monitor_reset_state_all - reset the single monitor
+ */
+static inline void da_monitor_reset_state_all(void)
+{
+       __da_monitor_reset_all(da_monitor_reset_state);
 }
 
 /*
@@ -171,7 +195,7 @@ static void da_monitor_reset_all(void)
  */
 static inline int da_monitor_init(void)
 {
-       da_monitor_reset_all();
+       da_monitor_reset_state_all();
        return 0;
 }
 
@@ -202,25 +226,41 @@ static struct da_monitor *da_get_monitor(void)
 }
 
 /*
- * da_monitor_reset_all - reset all CPUs' monitor
+ * __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 *))
 {
        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);
        }
 }
 
+/*
+ * da_monitor_reset_all - reset all CPUs' monitor
+ */
+static void da_monitor_reset_all(void)
+{
+       __da_monitor_reset_all(da_monitor_reset);
+}
+
+/*
+ * da_monitor_reset_state_all - reset all CPUs' monitor
+ */
+static inline void da_monitor_reset_state_all(void)
+{
+       __da_monitor_reset_all(da_monitor_reset_state);
+}
+
 /*
  * da_monitor_init - initialize all CPUs' monitor
  */
 static inline int da_monitor_init(void)
 {
-       da_monitor_reset_all();
+       da_monitor_reset_state_all();
        return 0;
 }
 
@@ -269,19 +309,29 @@ 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 *))
 {
        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);
 }
 
+static void da_monitor_reset_all(void)
+{
+       __da_monitor_reset_all(da_monitor_reset);
+}
+
+static inline void da_monitor_reset_state_all(void)
+{
+       __da_monitor_reset_all(da_monitor_reset_state);
+}
+
 /*
  * da_monitor_init - initialize the per-task monitor
  *
@@ -298,7 +348,7 @@ static int da_monitor_init(void)
 
        task_mon_slot = slot;
 
-       da_monitor_reset_all();
+       da_monitor_reset_state_all();
        return 0;
 }
 
@@ -490,15 +540,24 @@ 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 *))
 {
        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();
+               reset(&mon_storage->rv.da_mon);
+}
+
+static void da_monitor_reset_all(void)
+{
+       __da_monitor_reset_all(da_monitor_reset);
+}
+
+static inline void da_monitor_reset_state_all(void)
+{
+       __da_monitor_reset_all(da_monitor_reset_state);
 }
 
 static inline int da_monitor_init(void)

diff --git a/include/rv/ha_monitor.h b/include/rv/ha_monitor.h
index d59507e8cb30d86b2477ef52949b2ff42c42b493..bd87055567cc2e615a2144cac8a704541216633f 100644 (file)
--- a/include/rv/ha_monitor.h
+++ b/include/rv/ha_monitor.h
@@ -153,12 +153,12 @@ static inline void ha_monitor_init_env(struct da_monitor *da_mon)
  * Called from a hook in the DA reset functions, it supplies the da_mon
  * corresponding to the current ha_mon.
  * Not all hybrid automata require the timer, still clear it for simplicity.
+ * Monitors that never started have their timer uninitialized, do not stop those.
  */
 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)))
                ha_cancel_timer(ha_mon);
 }