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 <[email protected]> Closes: https://lore.kernel.org/lkml/d02c656aada7d071f083460a5c9a454363669b61.1778522945.git.wen.y...@linux.dev Suggested-by: Nam Cao <[email protected]> Fixes: f5587d1b6ec9 ("rv: Add Hybrid Automata monitor type") Reviewed-by: Wen Yang <[email protected]> Signed-off-by: Gabriele Monaco <[email protected]> --- include/rv/da_monitor.h | 91 +++++++++++++++++++++++++++++++++-------- include/rv/ha_monitor.h | 2 +- 2 files changed, 76 insertions(+), 17 deletions(-) diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h index 60dc39f26..ec9bc88bd 100644 --- 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 d59507e8c..bd8705556 100644 --- 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); } -- 2.54.0
