Re: [PATCH 5/5] rv: Add rts monitor
From: Gabriele Monaco <gmonaco@redhat.com>
Date: 2025-08-06 09:03:42
Also in:
lkml
On Wed, 2025-08-06 at 10:46 +0200, Nam Cao wrote:
On Wed, Aug 06, 2025 at 10:15:48AM +0200, Gabriele Monaco wrote:quoted
I didn't make it on time before your V2, I assume you solved already so you might ignore this. You kinda have something like the da_monitor_enabled: the rv_ltl_all_atoms_known I wonder if you could define LTL_RT_TASK_ENQUEUED only when you actually know it (or are reasonably sure based on your internal counter). Or at least not set all atoms until the monitor is fully set up.The rv_ltl_all_atoms_known() thingy is for situation where relevant tracepoints have not been hit yet. This case is slightly different, the tracepoint has been hit. And it is not clear how to implement the "reasonably sure based on your internal counter" part.quoted
Anyway reordering the tracepoints registration is likely necessary whatever you do, but I'm afraid a problem like this can occur pretty often with this type of monitors.What I have in v2 is a workaround only, by reordering the tracepoint registrations. The root problem is not specific to this monitor, but all LTL monitors. My idea for the real fix is the untested patch below. I will send it separately. It is not urgent, so I can wait for your DA macro removal patch to be merged first.
Alright, I get it, let's continue with the workaround for now, I'm going to have a look at your V2. Thanks, Gabriele
quoted hunk ↗ jump to hunk
As I'm sending the patch to you, I realized that the patch effectively nullifies ltl_atoms_init(). So I will need to fix that up.. Nam commit 7fbb9a99f1a95e5149d476fa3d83a60be1a9a579 Author: Nam Cao [off-list ref] Date: Tue Aug 5 22:47:49 2025 +0200 rv: Share the da_monitor_enabled_##name() function with LTL The LTL monitors also need the functionality that da_monitor_enabled_##name() offers. This is useful to prevent the automaton from being executed before the monitor is completely enabled, preventing the situation where the monitors run before all tracepoints are registered. This situation can cause a false positive error, because the monitors do not see some events and do not validate properly. Pull da_monitor_enabled_##name() to be in the common header, and use it for both LTL and DA. Signed-off-by: Nam Cao [off-list ref]diff --git a/include/linux/rv.h b/include/linux/rv.h index 1aa01d98e390..8a885b3665a8 100644 --- a/include/linux/rv.h +++ b/include/linux/rv.h@@ -119,6 +119,14 @@ int rv_register_monitor(struct rv_monitor*monitor, struct rv_monitor *parent); int rv_get_task_monitor_slot(void); void rv_put_task_monitor_slot(int slot); +static inline bool rv_monitor_enabled(struct rv_monitor *monitor) +{ + if (unlikely(!rv_monitoring_on())) + return 0; + + return likely(monitor->enabled); +} + #ifdef CONFIG_RV_REACTORS bool rv_reacting_on(void); int rv_unregister_reactor(struct rv_reactor *reactor);diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h index 17fa4f6e5ea6..92b8a8c0b9b7 100644 --- a/include/rv/da_monitor.h +++ b/include/rv/da_monitor.h@@ -74,29 +74,12 @@ static inline bool da_monitoring_##name(structda_monitor *da_mon) \ return da_mon-quoted
monitoring; \} \ \ - /* \ - * da_monitor_enabled_##name - checks if the monitor is enabled \ - */ \ -static inline bool da_monitor_enabled_##name(void) \ - { \ - /* global switch */ \ - if (unlikely(!rv_monitoring_on())) \ - return 0; \ - \ - /* monitor enabled */ \ - if (unlikely(!rv_##name.enabled)) \ - return 0; \ - \ - return 1; \ - } \ - \ /* \ * da_monitor_handling_event_##name - checks if the monitor is ready to handle events \ */ \ static inline bool da_monitor_handling_event_##name(struct da_monitor *da_mon) \ { \ - \ - if (!da_monitor_enabled_##name()) \ + if (!rv_monitor_enabled(&rv_##name)) \ return 0; \ \ /* monitor is actually monitoring */ \@@ -390,7 +373,7 @@ static inline boolda_handle_start_event_##name(enum events_##name event) \ { \ struct da_monitor *da_mon; \ \ - if (!da_monitor_enabled_##name()) \ + if (!rv_monitor_enabled(&rv_##name)) \ return 0; \ \ da_mon = da_get_monitor_##name(); \@@ -415,7 +398,7 @@ static inline boolda_handle_start_run_event_##name(enum events_##name event) { \ struct da_monitor *da_mon; \ \ - if (!da_monitor_enabled_##name()) \ + if (!rv_monitor_enabled(&rv_##name)) \ return 0; \ \ da_mon = da_get_monitor_##name(); \@@ -475,7 +458,7 @@ da_handle_start_event_##name(struct task_struct*tsk, enum events_##name event) { \ struct da_monitor *da_mon; \ \ - if (!da_monitor_enabled_##name()) \ + if (!rv_monitor_enabled(&rv_##name)) \ return 0; \ \ da_mon = da_get_monitor_##name(tsk); \@@ -501,7 +484,7 @@ da_handle_start_run_event_##name(structtask_struct *tsk, enum events_##name eve { \ struct da_monitor *da_mon; \ \ - if (!da_monitor_enabled_##name()) \ + if (!rv_monitor_enabled(&rv_##name)) \ return 0; \ \ da_mon = da_get_monitor_##name(tsk); \diff --git a/include/rv/ltl_monitor.h b/include/rv/ltl_monitor.h index 29bbf86d1a52..85a3d07a0303 100644 --- a/include/rv/ltl_monitor.h +++ b/include/rv/ltl_monitor.h@@ -16,6 +16,8 @@#error "Please include $(MODEL_NAME).h generated by rvgen" #endif +#define RV_MONITOR_NAME CONCATENATE(rv_, MONITOR_NAME) + #if LTL_MONITOR_TYPE == LTL_TASK_MONITOR #define TARGET_PRINT_FORMAT "%s[%d]"@@ -33,7 +35,6 @@ typedef unsigned int monitor_target;#endif #ifdef CONFIG_RV_REACTORS -#define RV_MONITOR_NAME CONCATENATE(rv_, MONITOR_NAME) static struct rv_monitor RV_MONITOR_NAME; static struct ltl_monitor *ltl_get_monitor(monitor_target target);@@ -156,6 +157,9 @@ static void ltl_attempt_start(monitor_targettarget, struct ltl_monitor *mon) static inline void ltl_atom_set(struct ltl_monitor *mon, enum ltl_atom atom, bool value) { + if (!rv_monitor_enabled(&RV_MONITOR_NAME)) + return; + __clear_bit(atom, mon->unknown_atoms); if (value) __set_bit(atom, mon->atoms);