* [PATCH v5 7/9] rv: Replace tss and sncid monitors with more complete sts
[not found] <20250728135022.255578-1-gmonaco@redhat.com>
@ 2025-07-28 13:50 ` Gabriele Monaco
2025-07-28 15:53 ` Nam Cao
2025-07-28 13:50 ` [PATCH v5 8/9] rv: Add nrp and sssw per-task monitors Gabriele Monaco
2025-07-28 13:50 ` [PATCH v5 9/9] rv: Add opid per-cpu monitor Gabriele Monaco
2 siblings, 1 reply; 15+ messages in thread
From: Gabriele Monaco @ 2025-07-28 13:50 UTC (permalink / raw)
To: linux-kernel, Steven Rostedt, Jonathan Corbet, Masami Hiramatsu,
linux-trace-kernel, linux-doc
Cc: Gabriele Monaco, Ingo Molnar, Peter Zijlstra, Nam Cao,
Tomas Glozar, Juri Lelli, Clark Williams, John Kacur
The tss monitor currently guarantees task switches can happen only while
scheduling, whereas the sncid monitor enforces scheduling occurs with
interrupt disabled.
Replace the monitors with a more comprehensive specification which
implies both but also ensures that:
* each scheduler call disable interrupts to switch
* each task switch happens with interrupts disabled
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
Documentation/trace/rv/monitor_sched.rst | 87 +++++-----
kernel/trace/rv/Kconfig | 3 +-
kernel/trace/rv/Makefile | 3 +-
kernel/trace/rv/monitors/sncid/Kconfig | 15 --
kernel/trace/rv/monitors/sncid/sncid.c | 95 -----------
kernel/trace/rv/monitors/sncid/sncid.h | 49 ------
kernel/trace/rv/monitors/sncid/sncid_trace.h | 15 --
kernel/trace/rv/monitors/sts/Kconfig | 19 +++
kernel/trace/rv/monitors/sts/sts.c | 156 ++++++++++++++++++
kernel/trace/rv/monitors/sts/sts.h | 117 +++++++++++++
.../{tss/tss_trace.h => sts/sts_trace.h} | 8 +-
kernel/trace/rv/monitors/tss/Kconfig | 14 --
kernel/trace/rv/monitors/tss/tss.c | 90 ----------
kernel/trace/rv/monitors/tss/tss.h | 47 ------
kernel/trace/rv/rv_trace.h | 3 +-
tools/verification/models/sched/sncid.dot | 18 --
tools/verification/models/sched/sts.dot | 38 +++++
tools/verification/models/sched/tss.dot | 18 --
18 files changed, 385 insertions(+), 410 deletions(-)
delete mode 100644 kernel/trace/rv/monitors/sncid/Kconfig
delete mode 100644 kernel/trace/rv/monitors/sncid/sncid.c
delete mode 100644 kernel/trace/rv/monitors/sncid/sncid.h
delete mode 100644 kernel/trace/rv/monitors/sncid/sncid_trace.h
create mode 100644 kernel/trace/rv/monitors/sts/Kconfig
create mode 100644 kernel/trace/rv/monitors/sts/sts.c
create mode 100644 kernel/trace/rv/monitors/sts/sts.h
rename kernel/trace/rv/monitors/{tss/tss_trace.h => sts/sts_trace.h} (67%)
delete mode 100644 kernel/trace/rv/monitors/tss/Kconfig
delete mode 100644 kernel/trace/rv/monitors/tss/tss.c
delete mode 100644 kernel/trace/rv/monitors/tss/tss.h
delete mode 100644 tools/verification/models/sched/sncid.dot
create mode 100644 tools/verification/models/sched/sts.dot
delete mode 100644 tools/verification/models/sched/tss.dot
diff --git a/Documentation/trace/rv/monitor_sched.rst b/Documentation/trace/rv/monitor_sched.rst
index 24b2c62a3bc2..6c4c00216c07 100644
--- a/Documentation/trace/rv/monitor_sched.rst
+++ b/Documentation/trace/rv/monitor_sched.rst
@@ -40,26 +40,6 @@ defined in by Daniel Bristot in [1].
Currently we included the following:
-Monitor tss
-~~~~~~~~~~~
-
-The task switch while scheduling (tss) monitor ensures a task switch happens
-only in scheduling context, that is inside a call to `__schedule`::
-
- |
- |
- v
- +-----------------+
- | thread | <+
- +-----------------+ |
- | |
- | schedule_entry | schedule_exit
- v |
- sched_switch |
- +--------------- |
- | sched |
- +--------------> -+
-
Monitor sco
~~~~~~~~~~~
@@ -144,26 +124,55 @@ does not enable preemption::
|
scheduling_contex -+
-Monitor sncid
-~~~~~~~~~~~~~
-
-The schedule not called with interrupt disabled (sncid) monitor ensures
-schedule is not called with interrupt disabled::
+Monitor sts
+~~~~~~~~~~~
- |
- |
- v
- schedule_entry +--------------+
- schedule_exit | |
- +----------------- | can_sched |
- | | |
- +----------------> | | <+
- +--------------+ |
- | |
- | irq_disable | irq_enable
- v |
- |
- cant_sched -+
+The schedule implies task switch (sts) monitor ensures a task switch happens
+only in scheduling context and up to once, as well as scheduling occurs with
+interrupts enabled but no task switch can happen before interrupts are
+disabled. When the next task picked for execution is the same as the previously
+running one, no real task switch occurs but interrupts are disabled nonetheless::
+
+ irq_entry |
+ +----+ |
+ v | v
+ +------------+ irq_enable #===================# irq_disable
+ | | ------------> H H irq_entry
+ | cant_sched | <------------ H H irq_enable
+ | | irq_disable H can_sched H --------------+
+ +------------+ H H |
+ H H |
+ +---------------> H H <-------------+
+ | #===================#
+ | |
+ schedule_exit | schedule_entry
+ | v
+ | +-------------------+ irq_enable
+ | | scheduling | <---------------+
+ | +-------------------+ |
+ | | |
+ | | irq_disable +--------+ irq_entry
+ | v | | --------+
+ | +-------------------+ irq_entry | in_irq | |
+ | | | -----------> | | <-------+
+ | | disable_to_switch | +--------+
+ | | | --+
+ | +-------------------+ |
+ | | |
+ | | sched_switch |
+ | v |
+ | +-------------------+ |
+ | | switching | | irq_enable
+ | +-------------------+ |
+ | | |
+ | | irq_enable |
+ | v |
+ | +-------------------+ |
+ +-- | enable_to_exit | <-+
+ +-------------------+
+ ^ | irq_disable
+ | | irq_entry
+ +---------------+ irq_enable
References
----------
diff --git a/kernel/trace/rv/Kconfig b/kernel/trace/rv/Kconfig
index 34164eb4ec91..b688b24081c8 100644
--- a/kernel/trace/rv/Kconfig
+++ b/kernel/trace/rv/Kconfig
@@ -50,12 +50,11 @@ source "kernel/trace/rv/monitors/wip/Kconfig"
source "kernel/trace/rv/monitors/wwnr/Kconfig"
source "kernel/trace/rv/monitors/sched/Kconfig"
-source "kernel/trace/rv/monitors/tss/Kconfig"
source "kernel/trace/rv/monitors/sco/Kconfig"
source "kernel/trace/rv/monitors/snroc/Kconfig"
source "kernel/trace/rv/monitors/scpd/Kconfig"
source "kernel/trace/rv/monitors/snep/Kconfig"
-source "kernel/trace/rv/monitors/sncid/Kconfig"
+source "kernel/trace/rv/monitors/sts/Kconfig"
# Add new sched monitors here
source "kernel/trace/rv/monitors/rtapp/Kconfig"
diff --git a/kernel/trace/rv/Makefile b/kernel/trace/rv/Makefile
index 13ec2944c665..1939d3d7621c 100644
--- a/kernel/trace/rv/Makefile
+++ b/kernel/trace/rv/Makefile
@@ -6,15 +6,14 @@ obj-$(CONFIG_RV) += rv.o
obj-$(CONFIG_RV_MON_WIP) += monitors/wip/wip.o
obj-$(CONFIG_RV_MON_WWNR) += monitors/wwnr/wwnr.o
obj-$(CONFIG_RV_MON_SCHED) += monitors/sched/sched.o
-obj-$(CONFIG_RV_MON_TSS) += monitors/tss/tss.o
obj-$(CONFIG_RV_MON_SCO) += monitors/sco/sco.o
obj-$(CONFIG_RV_MON_SNROC) += monitors/snroc/snroc.o
obj-$(CONFIG_RV_MON_SCPD) += monitors/scpd/scpd.o
obj-$(CONFIG_RV_MON_SNEP) += monitors/snep/snep.o
-obj-$(CONFIG_RV_MON_SNCID) += monitors/sncid/sncid.o
obj-$(CONFIG_RV_MON_RTAPP) += monitors/rtapp/rtapp.o
obj-$(CONFIG_RV_MON_PAGEFAULT) += monitors/pagefault/pagefault.o
obj-$(CONFIG_RV_MON_SLEEP) += monitors/sleep/sleep.o
+obj-$(CONFIG_RV_MON_STS) += monitors/sts/sts.o
# Add new monitors here
obj-$(CONFIG_RV_REACTORS) += rv_reactors.o
obj-$(CONFIG_RV_REACT_PRINTK) += reactor_printk.o
diff --git a/kernel/trace/rv/monitors/sncid/Kconfig b/kernel/trace/rv/monitors/sncid/Kconfig
deleted file mode 100644
index 3a5639feaaaf..000000000000
--- a/kernel/trace/rv/monitors/sncid/Kconfig
+++ /dev/null
@@ -1,15 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only
-#
-config RV_MON_SNCID
- depends on RV
- depends on TRACE_IRQFLAGS
- depends on RV_MON_SCHED
- default y
- select DA_MON_EVENTS_IMPLICIT
- bool "sncid monitor"
- help
- Monitor to ensure schedule is not called with interrupt disabled.
- This monitor is part of the sched monitors collection.
-
- For further information, see:
- Documentation/trace/rv/monitor_sched.rst
diff --git a/kernel/trace/rv/monitors/sncid/sncid.c b/kernel/trace/rv/monitors/sncid/sncid.c
deleted file mode 100644
index c8491f426365..000000000000
--- a/kernel/trace/rv/monitors/sncid/sncid.c
+++ /dev/null
@@ -1,95 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#include <linux/ftrace.h>
-#include <linux/tracepoint.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/rv.h>
-#include <rv/instrumentation.h>
-#include <rv/da_monitor.h>
-
-#define MODULE_NAME "sncid"
-
-#include <trace/events/sched.h>
-#include <trace/events/preemptirq.h>
-#include <rv_trace.h>
-#include <monitors/sched/sched.h>
-
-#include "sncid.h"
-
-static struct rv_monitor rv_sncid;
-DECLARE_DA_MON_PER_CPU(sncid, unsigned char);
-
-static void handle_irq_disable(void *data, unsigned long ip, unsigned long parent_ip)
-{
- da_handle_event_sncid(irq_disable_sncid);
-}
-
-static void handle_irq_enable(void *data, unsigned long ip, unsigned long parent_ip)
-{
- da_handle_start_event_sncid(irq_enable_sncid);
-}
-
-static void handle_schedule_entry(void *data, bool preempt)
-{
- da_handle_start_event_sncid(schedule_entry_sncid);
-}
-
-static void handle_schedule_exit(void *data, bool is_switch)
-{
- da_handle_start_event_sncid(schedule_exit_sncid);
-}
-
-static int enable_sncid(void)
-{
- int retval;
-
- retval = da_monitor_init_sncid();
- if (retval)
- return retval;
-
- rv_attach_trace_probe("sncid", irq_disable, handle_irq_disable);
- rv_attach_trace_probe("sncid", irq_enable, handle_irq_enable);
- rv_attach_trace_probe("sncid", sched_entry_tp, handle_schedule_entry);
- rv_attach_trace_probe("sncid", sched_exit_tp, handle_schedule_exit);
-
- return 0;
-}
-
-static void disable_sncid(void)
-{
- rv_sncid.enabled = 0;
-
- rv_detach_trace_probe("sncid", irq_disable, handle_irq_disable);
- rv_detach_trace_probe("sncid", irq_enable, handle_irq_enable);
- rv_detach_trace_probe("sncid", sched_entry_tp, handle_schedule_entry);
- rv_detach_trace_probe("sncid", sched_exit_tp, handle_schedule_exit);
-
- da_monitor_destroy_sncid();
-}
-
-static struct rv_monitor rv_sncid = {
- .name = "sncid",
- .description = "schedule not called with interrupt disabled.",
- .enable = enable_sncid,
- .disable = disable_sncid,
- .reset = da_monitor_reset_all_sncid,
- .enabled = 0,
-};
-
-static int __init register_sncid(void)
-{
- return rv_register_monitor(&rv_sncid, &rv_sched);
-}
-
-static void __exit unregister_sncid(void)
-{
- rv_unregister_monitor(&rv_sncid);
-}
-
-module_init(register_sncid);
-module_exit(unregister_sncid);
-
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Gabriele Monaco <gmonaco@redhat.com>");
-MODULE_DESCRIPTION("sncid: schedule not called with interrupt disabled.");
diff --git a/kernel/trace/rv/monitors/sncid/sncid.h b/kernel/trace/rv/monitors/sncid/sncid.h
deleted file mode 100644
index 21304725142b..000000000000
--- a/kernel/trace/rv/monitors/sncid/sncid.h
+++ /dev/null
@@ -1,49 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Automatically generated C representation of sncid automaton
- * For further information about this format, see kernel documentation:
- * Documentation/trace/rv/deterministic_automata.rst
- */
-
-enum states_sncid {
- can_sched_sncid = 0,
- cant_sched_sncid,
- state_max_sncid
-};
-
-#define INVALID_STATE state_max_sncid
-
-enum events_sncid {
- irq_disable_sncid = 0,
- irq_enable_sncid,
- schedule_entry_sncid,
- schedule_exit_sncid,
- event_max_sncid
-};
-
-struct automaton_sncid {
- char *state_names[state_max_sncid];
- char *event_names[event_max_sncid];
- unsigned char function[state_max_sncid][event_max_sncid];
- unsigned char initial_state;
- bool final_states[state_max_sncid];
-};
-
-static const struct automaton_sncid automaton_sncid = {
- .state_names = {
- "can_sched",
- "cant_sched"
- },
- .event_names = {
- "irq_disable",
- "irq_enable",
- "schedule_entry",
- "schedule_exit"
- },
- .function = {
- { cant_sched_sncid, INVALID_STATE, can_sched_sncid, can_sched_sncid },
- { INVALID_STATE, can_sched_sncid, INVALID_STATE, INVALID_STATE },
- },
- .initial_state = can_sched_sncid,
- .final_states = { 1, 0 },
-};
diff --git a/kernel/trace/rv/monitors/sncid/sncid_trace.h b/kernel/trace/rv/monitors/sncid/sncid_trace.h
deleted file mode 100644
index 3ce42a57671d..000000000000
--- a/kernel/trace/rv/monitors/sncid/sncid_trace.h
+++ /dev/null
@@ -1,15 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-
-/*
- * Snippet to be included in rv_trace.h
- */
-
-#ifdef CONFIG_RV_MON_SNCID
-DEFINE_EVENT(event_da_monitor, event_sncid,
- TP_PROTO(char *state, char *event, char *next_state, bool final_state),
- TP_ARGS(state, event, next_state, final_state));
-
-DEFINE_EVENT(error_da_monitor, error_sncid,
- TP_PROTO(char *state, char *event),
- TP_ARGS(state, event));
-#endif /* CONFIG_RV_MON_SNCID */
diff --git a/kernel/trace/rv/monitors/sts/Kconfig b/kernel/trace/rv/monitors/sts/Kconfig
new file mode 100644
index 000000000000..7d1ff0f6fc91
--- /dev/null
+++ b/kernel/trace/rv/monitors/sts/Kconfig
@@ -0,0 +1,19 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+config RV_MON_STS
+ depends on RV
+ depends on TRACE_IRQFLAGS
+ depends on RV_MON_SCHED
+ default y
+ select DA_MON_EVENTS_IMPLICIT
+ bool "sts monitor"
+ help
+ Monitor to ensure relationships between scheduler and task switches
+ * the scheduler is called and returns with interrupts disabled
+ * each call to the scheduler has up to one switch
+ * switches only happen inside the scheduler
+ * each call to the scheduler disables interrupts to switch
+ This monitor is part of the sched monitors collection.
+
+ For further information, see:
+ Documentation/trace/rv/monitor_sched.rst
diff --git a/kernel/trace/rv/monitors/sts/sts.c b/kernel/trace/rv/monitors/sts/sts.c
new file mode 100644
index 000000000000..c4a9cd67c1d2
--- /dev/null
+++ b/kernel/trace/rv/monitors/sts/sts.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/ftrace.h>
+#include <linux/tracepoint.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/rv.h>
+#include <rv/instrumentation.h>
+#include <rv/da_monitor.h>
+
+#define MODULE_NAME "sts"
+
+#include <trace/events/sched.h>
+#include <trace/events/irq.h>
+#include <trace/events/preemptirq.h>
+#include <rv_trace.h>
+#include <monitors/sched/sched.h>
+
+#include "sts.h"
+
+static struct rv_monitor rv_sts;
+DECLARE_DA_MON_PER_CPU(sts, unsigned char);
+
+#ifdef CONFIG_X86_LOCAL_APIC
+#include <asm/trace/irq_vectors.h>
+
+static void handle_vector_irq_entry(void *data, int vector)
+{
+ da_handle_event_sts(irq_entry_sts);
+}
+
+static void attach_vector_irq(void)
+{
+ rv_attach_trace_probe("sts", local_timer_entry, handle_vector_irq_entry);
+ if (IS_ENABLED(CONFIG_IRQ_WORK))
+ rv_attach_trace_probe("sts", irq_work_entry, handle_vector_irq_entry);
+ if (IS_ENABLED(CONFIG_SMP)) {
+ rv_attach_trace_probe("sts", reschedule_entry, handle_vector_irq_entry);
+ rv_attach_trace_probe("sts", call_function_entry, handle_vector_irq_entry);
+ rv_attach_trace_probe("sts", call_function_single_entry, handle_vector_irq_entry);
+ }
+}
+
+static void detach_vector_irq(void)
+{
+ rv_detach_trace_probe("sts", local_timer_entry, handle_vector_irq_entry);
+ if (IS_ENABLED(CONFIG_IRQ_WORK))
+ rv_detach_trace_probe("sts", irq_work_entry, handle_vector_irq_entry);
+ if (IS_ENABLED(CONFIG_SMP)) {
+ rv_detach_trace_probe("sts", reschedule_entry, handle_vector_irq_entry);
+ rv_detach_trace_probe("sts", call_function_entry, handle_vector_irq_entry);
+ rv_detach_trace_probe("sts", call_function_single_entry, handle_vector_irq_entry);
+ }
+}
+
+#else
+/* We assume irq_entry tracepoints are sufficient on other architectures */
+static void attach_vector_irq(void) { }
+static void detach_vector_irq(void) { }
+#endif
+
+static void handle_irq_disable(void *data, unsigned long ip, unsigned long parent_ip)
+{
+ da_handle_event_sts(irq_disable_sts);
+}
+
+static void handle_irq_enable(void *data, unsigned long ip, unsigned long parent_ip)
+{
+ da_handle_event_sts(irq_enable_sts);
+}
+
+static void handle_irq_entry(void *data, int irq, struct irqaction *action)
+{
+ da_handle_event_sts(irq_entry_sts);
+}
+
+static void handle_sched_switch(void *data, bool preempt,
+ struct task_struct *prev,
+ struct task_struct *next,
+ unsigned int prev_state)
+{
+ da_handle_event_sts(sched_switch_sts);
+}
+
+static void handle_schedule_entry(void *data, bool preempt)
+{
+ da_handle_event_sts(schedule_entry_sts);
+}
+
+static void handle_schedule_exit(void *data, bool is_switch)
+{
+ da_handle_start_event_sts(schedule_exit_sts);
+}
+
+static int enable_sts(void)
+{
+ int retval;
+
+ retval = da_monitor_init_sts();
+ if (retval)
+ return retval;
+
+ rv_attach_trace_probe("sts", irq_disable, handle_irq_disable);
+ rv_attach_trace_probe("sts", irq_enable, handle_irq_enable);
+ rv_attach_trace_probe("sts", irq_handler_entry, handle_irq_entry);
+ rv_attach_trace_probe("sts", sched_switch, handle_sched_switch);
+ rv_attach_trace_probe("sts", sched_entry_tp, handle_schedule_entry);
+ rv_attach_trace_probe("sts", sched_exit_tp, handle_schedule_exit);
+ attach_vector_irq();
+
+ return 0;
+}
+
+static void disable_sts(void)
+{
+ rv_sts.enabled = 0;
+
+ rv_detach_trace_probe("sts", irq_disable, handle_irq_disable);
+ rv_detach_trace_probe("sts", irq_enable, handle_irq_enable);
+ rv_detach_trace_probe("sts", irq_handler_entry, handle_irq_entry);
+ rv_detach_trace_probe("sts", sched_switch, handle_sched_switch);
+ rv_detach_trace_probe("sts", sched_entry_tp, handle_schedule_entry);
+ rv_detach_trace_probe("sts", sched_exit_tp, handle_schedule_exit);
+ detach_vector_irq();
+
+ da_monitor_destroy_sts();
+}
+
+/*
+ * This is the monitor register section.
+ */
+static struct rv_monitor rv_sts = {
+ .name = "sts",
+ .description = "schedule implies task switch.",
+ .enable = enable_sts,
+ .disable = disable_sts,
+ .reset = da_monitor_reset_all_sts,
+ .enabled = 0,
+};
+
+static int __init register_sts(void)
+{
+ return rv_register_monitor(&rv_sts, &rv_sched);
+}
+
+static void __exit unregister_sts(void)
+{
+ rv_unregister_monitor(&rv_sts);
+}
+
+module_init(register_sts);
+module_exit(unregister_sts);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Gabriele Monaco <gmonaco@redhat.com>");
+MODULE_DESCRIPTION("sts: schedule implies task switch.");
diff --git a/kernel/trace/rv/monitors/sts/sts.h b/kernel/trace/rv/monitors/sts/sts.h
new file mode 100644
index 000000000000..3368b6599a00
--- /dev/null
+++ b/kernel/trace/rv/monitors/sts/sts.h
@@ -0,0 +1,117 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Automatically generated C representation of sts automaton
+ * For further information about this format, see kernel documentation:
+ * Documentation/trace/rv/deterministic_automata.rst
+ */
+
+enum states_sts {
+ can_sched_sts = 0,
+ cant_sched_sts,
+ disable_to_switch_sts,
+ enable_to_exit_sts,
+ in_irq_sts,
+ scheduling_sts,
+ switching_sts,
+ state_max_sts
+};
+
+#define INVALID_STATE state_max_sts
+
+enum events_sts {
+ irq_disable_sts = 0,
+ irq_enable_sts,
+ irq_entry_sts,
+ sched_switch_sts,
+ schedule_entry_sts,
+ schedule_exit_sts,
+ event_max_sts
+};
+
+struct automaton_sts {
+ char *state_names[state_max_sts];
+ char *event_names[event_max_sts];
+ unsigned char function[state_max_sts][event_max_sts];
+ unsigned char initial_state;
+ bool final_states[state_max_sts];
+};
+
+static const struct automaton_sts automaton_sts = {
+ .state_names = {
+ "can_sched",
+ "cant_sched",
+ "disable_to_switch",
+ "enable_to_exit",
+ "in_irq",
+ "scheduling",
+ "switching"
+ },
+ .event_names = {
+ "irq_disable",
+ "irq_enable",
+ "irq_entry",
+ "sched_switch",
+ "schedule_entry",
+ "schedule_exit"
+ },
+ .function = {
+ {
+ cant_sched_sts,
+ INVALID_STATE,
+ INVALID_STATE,
+ INVALID_STATE,
+ scheduling_sts,
+ INVALID_STATE
+ },
+ {
+ INVALID_STATE,
+ can_sched_sts,
+ cant_sched_sts,
+ INVALID_STATE,
+ INVALID_STATE,
+ INVALID_STATE
+ },
+ {
+ INVALID_STATE,
+ enable_to_exit_sts,
+ in_irq_sts,
+ switching_sts,
+ INVALID_STATE,
+ INVALID_STATE
+ },
+ {
+ enable_to_exit_sts,
+ enable_to_exit_sts,
+ enable_to_exit_sts,
+ INVALID_STATE,
+ INVALID_STATE,
+ can_sched_sts
+ },
+ {
+ INVALID_STATE,
+ scheduling_sts,
+ in_irq_sts,
+ INVALID_STATE,
+ INVALID_STATE,
+ INVALID_STATE
+ },
+ {
+ disable_to_switch_sts,
+ INVALID_STATE,
+ INVALID_STATE,
+ INVALID_STATE,
+ INVALID_STATE,
+ INVALID_STATE
+ },
+ {
+ INVALID_STATE,
+ enable_to_exit_sts,
+ INVALID_STATE,
+ INVALID_STATE,
+ INVALID_STATE,
+ INVALID_STATE
+ },
+ },
+ .initial_state = can_sched_sts,
+ .final_states = { 1, 0, 0, 0, 0, 0, 0 },
+};
diff --git a/kernel/trace/rv/monitors/tss/tss_trace.h b/kernel/trace/rv/monitors/sts/sts_trace.h
similarity index 67%
rename from kernel/trace/rv/monitors/tss/tss_trace.h
rename to kernel/trace/rv/monitors/sts/sts_trace.h
index 4619dbb50cc0..d78beb58d5b3 100644
--- a/kernel/trace/rv/monitors/tss/tss_trace.h
+++ b/kernel/trace/rv/monitors/sts/sts_trace.h
@@ -4,12 +4,12 @@
* Snippet to be included in rv_trace.h
*/
-#ifdef CONFIG_RV_MON_TSS
-DEFINE_EVENT(event_da_monitor, event_tss,
+#ifdef CONFIG_RV_MON_STS
+DEFINE_EVENT(event_da_monitor, event_sts,
TP_PROTO(char *state, char *event, char *next_state, bool final_state),
TP_ARGS(state, event, next_state, final_state));
-DEFINE_EVENT(error_da_monitor, error_tss,
+DEFINE_EVENT(error_da_monitor, error_sts,
TP_PROTO(char *state, char *event),
TP_ARGS(state, event));
-#endif /* CONFIG_RV_MON_TSS */
+#endif /* CONFIG_RV_MON_STS */
diff --git a/kernel/trace/rv/monitors/tss/Kconfig b/kernel/trace/rv/monitors/tss/Kconfig
deleted file mode 100644
index 479f86f52e60..000000000000
--- a/kernel/trace/rv/monitors/tss/Kconfig
+++ /dev/null
@@ -1,14 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only
-#
-config RV_MON_TSS
- depends on RV
- depends on RV_MON_SCHED
- default y
- select DA_MON_EVENTS_IMPLICIT
- bool "tss monitor"
- help
- Monitor to ensure sched_switch happens only in scheduling context.
- This monitor is part of the sched monitors collection.
-
- For further information, see:
- Documentation/trace/rv/monitor_sched.rst
diff --git a/kernel/trace/rv/monitors/tss/tss.c b/kernel/trace/rv/monitors/tss/tss.c
deleted file mode 100644
index 95ebd15131f5..000000000000
--- a/kernel/trace/rv/monitors/tss/tss.c
+++ /dev/null
@@ -1,90 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#include <linux/ftrace.h>
-#include <linux/tracepoint.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/rv.h>
-#include <rv/instrumentation.h>
-#include <rv/da_monitor.h>
-
-#define MODULE_NAME "tss"
-
-#include <trace/events/sched.h>
-#include <rv_trace.h>
-#include <monitors/sched/sched.h>
-
-#include "tss.h"
-
-static struct rv_monitor rv_tss;
-DECLARE_DA_MON_PER_CPU(tss, unsigned char);
-
-static void handle_sched_switch(void *data, bool preempt,
- struct task_struct *prev,
- struct task_struct *next,
- unsigned int prev_state)
-{
- da_handle_event_tss(sched_switch_tss);
-}
-
-static void handle_schedule_entry(void *data, bool preempt)
-{
- da_handle_event_tss(schedule_entry_tss);
-}
-
-static void handle_schedule_exit(void *data, bool is_switch)
-{
- da_handle_start_event_tss(schedule_exit_tss);
-}
-
-static int enable_tss(void)
-{
- int retval;
-
- retval = da_monitor_init_tss();
- if (retval)
- return retval;
-
- rv_attach_trace_probe("tss", sched_switch, handle_sched_switch);
- rv_attach_trace_probe("tss", sched_entry_tp, handle_schedule_entry);
- rv_attach_trace_probe("tss", sched_exit_tp, handle_schedule_exit);
-
- return 0;
-}
-
-static void disable_tss(void)
-{
- rv_tss.enabled = 0;
-
- rv_detach_trace_probe("tss", sched_switch, handle_sched_switch);
- rv_detach_trace_probe("tss", sched_entry_tp, handle_schedule_entry);
- rv_detach_trace_probe("tss", sched_exit_tp, handle_schedule_exit);
-
- da_monitor_destroy_tss();
-}
-
-static struct rv_monitor rv_tss = {
- .name = "tss",
- .description = "task switch while scheduling.",
- .enable = enable_tss,
- .disable = disable_tss,
- .reset = da_monitor_reset_all_tss,
- .enabled = 0,
-};
-
-static int __init register_tss(void)
-{
- return rv_register_monitor(&rv_tss, &rv_sched);
-}
-
-static void __exit unregister_tss(void)
-{
- rv_unregister_monitor(&rv_tss);
-}
-
-module_init(register_tss);
-module_exit(unregister_tss);
-
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Gabriele Monaco <gmonaco@redhat.com>");
-MODULE_DESCRIPTION("tss: task switch while scheduling.");
diff --git a/kernel/trace/rv/monitors/tss/tss.h b/kernel/trace/rv/monitors/tss/tss.h
deleted file mode 100644
index f0a36fda1b87..000000000000
--- a/kernel/trace/rv/monitors/tss/tss.h
+++ /dev/null
@@ -1,47 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Automatically generated C representation of tss automaton
- * For further information about this format, see kernel documentation:
- * Documentation/trace/rv/deterministic_automata.rst
- */
-
-enum states_tss {
- thread_tss = 0,
- sched_tss,
- state_max_tss
-};
-
-#define INVALID_STATE state_max_tss
-
-enum events_tss {
- sched_switch_tss = 0,
- schedule_entry_tss,
- schedule_exit_tss,
- event_max_tss
-};
-
-struct automaton_tss {
- char *state_names[state_max_tss];
- char *event_names[event_max_tss];
- unsigned char function[state_max_tss][event_max_tss];
- unsigned char initial_state;
- bool final_states[state_max_tss];
-};
-
-static const struct automaton_tss automaton_tss = {
- .state_names = {
- "thread",
- "sched"
- },
- .event_names = {
- "sched_switch",
- "schedule_entry",
- "schedule_exit"
- },
- .function = {
- { INVALID_STATE, sched_tss, INVALID_STATE },
- { sched_tss, INVALID_STATE, thread_tss },
- },
- .initial_state = thread_tss,
- .final_states = { 1, 0 },
-};
diff --git a/kernel/trace/rv/rv_trace.h b/kernel/trace/rv/rv_trace.h
index 3af46cd185b3..97b2f7e07f27 100644
--- a/kernel/trace/rv/rv_trace.h
+++ b/kernel/trace/rv/rv_trace.h
@@ -58,11 +58,10 @@ DECLARE_EVENT_CLASS(error_da_monitor,
);
#include <monitors/wip/wip_trace.h>
-#include <monitors/tss/tss_trace.h>
#include <monitors/sco/sco_trace.h>
#include <monitors/scpd/scpd_trace.h>
#include <monitors/snep/snep_trace.h>
-#include <monitors/sncid/sncid_trace.h>
+#include <monitors/sts/sts_trace.h>
// Add new monitors based on CONFIG_DA_MON_EVENTS_IMPLICIT here
#endif /* CONFIG_DA_MON_EVENTS_IMPLICIT */
diff --git a/tools/verification/models/sched/sncid.dot b/tools/verification/models/sched/sncid.dot
deleted file mode 100644
index 072851721b50..000000000000
--- a/tools/verification/models/sched/sncid.dot
+++ /dev/null
@@ -1,18 +0,0 @@
-digraph state_automaton {
- center = true;
- size = "7,11";
- {node [shape = plaintext, style=invis, label=""] "__init_can_sched"};
- {node [shape = ellipse] "can_sched"};
- {node [shape = plaintext] "can_sched"};
- {node [shape = plaintext] "cant_sched"};
- "__init_can_sched" -> "can_sched";
- "can_sched" [label = "can_sched", color = green3];
- "can_sched" -> "can_sched" [ label = "schedule_entry\nschedule_exit" ];
- "can_sched" -> "cant_sched" [ label = "irq_disable" ];
- "cant_sched" [label = "cant_sched"];
- "cant_sched" -> "can_sched" [ label = "irq_enable" ];
- { rank = min ;
- "__init_can_sched";
- "can_sched";
- }
-}
diff --git a/tools/verification/models/sched/sts.dot b/tools/verification/models/sched/sts.dot
new file mode 100644
index 000000000000..8f5f38be04d5
--- /dev/null
+++ b/tools/verification/models/sched/sts.dot
@@ -0,0 +1,38 @@
+digraph state_automaton {
+ center = true;
+ size = "7,11";
+ {node [shape = plaintext, style=invis, label=""] "__init_can_sched"};
+ {node [shape = doublecircle] "can_sched"};
+ {node [shape = circle] "can_sched"};
+ {node [shape = circle] "cant_sched"};
+ {node [shape = circle] "disable_to_switch"};
+ {node [shape = circle] "enable_to_exit"};
+ {node [shape = circle] "in_irq"};
+ {node [shape = circle] "scheduling"};
+ {node [shape = circle] "switching"};
+ "__init_can_sched" -> "can_sched";
+ "can_sched" [label = "can_sched", color = green3];
+ "can_sched" -> "cant_sched" [ label = "irq_disable" ];
+ "can_sched" -> "scheduling" [ label = "schedule_entry" ];
+ "cant_sched" [label = "cant_sched"];
+ "cant_sched" -> "can_sched" [ label = "irq_enable" ];
+ "cant_sched" -> "cant_sched" [ label = "irq_entry" ];
+ "disable_to_switch" [label = "disable_to_switch"];
+ "disable_to_switch" -> "enable_to_exit" [ label = "irq_enable" ];
+ "disable_to_switch" -> "in_irq" [ label = "irq_entry" ];
+ "disable_to_switch" -> "switching" [ label = "sched_switch" ];
+ "enable_to_exit" [label = "enable_to_exit"];
+ "enable_to_exit" -> "can_sched" [ label = "schedule_exit" ];
+ "enable_to_exit" -> "enable_to_exit" [ label = "irq_disable\nirq_entry\nirq_enable" ];
+ "in_irq" [label = "in_irq"];
+ "in_irq" -> "in_irq" [ label = "irq_entry" ];
+ "in_irq" -> "scheduling" [ label = "irq_enable" ];
+ "scheduling" [label = "scheduling"];
+ "scheduling" -> "disable_to_switch" [ label = "irq_disable" ];
+ "switching" [label = "switching"];
+ "switching" -> "enable_to_exit" [ label = "irq_enable" ];
+ { rank = min ;
+ "__init_can_sched";
+ "can_sched";
+ }
+}
diff --git a/tools/verification/models/sched/tss.dot b/tools/verification/models/sched/tss.dot
deleted file mode 100644
index 7dfa1d9121bb..000000000000
--- a/tools/verification/models/sched/tss.dot
+++ /dev/null
@@ -1,18 +0,0 @@
-digraph state_automaton {
- center = true;
- size = "7,11";
- {node [shape = plaintext] "sched"};
- {node [shape = plaintext, style=invis, label=""] "__init_thread"};
- {node [shape = ellipse] "thread"};
- {node [shape = plaintext] "thread"};
- "__init_thread" -> "thread";
- "sched" [label = "sched"];
- "sched" -> "sched" [ label = "sched_switch" ];
- "sched" -> "thread" [ label = "schedule_exit" ];
- "thread" [label = "thread", color = green3];
- "thread" -> "sched" [ label = "schedule_entry" ];
- { rank = min ;
- "__init_thread";
- "thread";
- }
-}
--
2.50.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v5 7/9] rv: Replace tss and sncid monitors with more complete sts
2025-07-28 13:50 ` [PATCH v5 7/9] rv: Replace tss and sncid monitors with more complete sts Gabriele Monaco
@ 2025-07-28 15:53 ` Nam Cao
2025-07-29 8:46 ` Gabriele Monaco
2025-07-30 16:28 ` Nam Cao
0 siblings, 2 replies; 15+ messages in thread
From: Nam Cao @ 2025-07-28 15:53 UTC (permalink / raw)
To: Gabriele Monaco
Cc: linux-kernel, Steven Rostedt, Jonathan Corbet, Masami Hiramatsu,
linux-trace-kernel, linux-doc, Ingo Molnar, Peter Zijlstra,
Tomas Glozar, Juri Lelli, Clark Williams, John Kacur
On Mon, Jul 28, 2025 at 03:50:19PM +0200, Gabriele Monaco wrote:
> The tss monitor currently guarantees task switches can happen only while
> scheduling, whereas the sncid monitor enforces scheduling occurs with
> interrupt disabled.
>
> Replace the monitors with a more comprehensive specification which
> implies both but also ensures that:
> * each scheduler call disable interrupts to switch
> * each task switch happens with interrupts disabled
>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
I gave this a try on riscv64 and observed some errors:
[ 620.696055] rv: monitor sts does not allow event sched_switch on state enable_to_exit
[ 621.047705] rv: monitor sts does not allow event sched_switch on state enable_to_exit
[ 642.440209] rv: monitor sts does not allow event sched_switch on state enable_to_exit
I tested with two user programs:
int main() { asm ("unimp"); }
int main() { asm ("ebreak"); }
The two programs are repeatedly executed:
#!/bin/bash
./test1 &
./test2 &
# ... repeat lots of time
Any idea?
Nam
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v5 7/9] rv: Replace tss and sncid monitors with more complete sts
2025-07-28 15:53 ` Nam Cao
@ 2025-07-29 8:46 ` Gabriele Monaco
2025-07-29 9:25 ` Nam Cao
2025-07-30 16:28 ` Nam Cao
1 sibling, 1 reply; 15+ messages in thread
From: Gabriele Monaco @ 2025-07-29 8:46 UTC (permalink / raw)
To: Nam Cao
Cc: linux-kernel, Steven Rostedt, linux-trace-kernel, linux-doc,
Ingo Molnar, Peter Zijlstra, Tomas Glozar, Juri Lelli,
Clark Williams, John Kacur
On Mon, 2025-07-28 at 17:53 +0200, Nam Cao wrote:
> I gave this a try on riscv64 and observed some errors:
>
> [ 620.696055] rv: monitor sts does not allow event sched_switch on
> state enable_to_exit
> [ 621.047705] rv: monitor sts does not allow event sched_switch on
> state enable_to_exit
> [ 642.440209] rv: monitor sts does not allow event sched_switch on
> state enable_to_exit
>
> I tested with two user programs:
>
> int main() { asm ("unimp"); }
> int main() { asm ("ebreak"); }
>
> The two programs are repeatedly executed:
>
> #!/bin/bash
> ./test1 &
> ./test2 &
> # ... repeat lots of time
>
> Any idea?
Mmh I see what you're doing here..
Those instructions are supposed to raise some sort of exception in the
CPU which apparently disables and enables interrupts without raising an
interrupt handler tracepoint (the discriminator for this monitor).
This lets the monitor believe we passed the time a switch is possible
and complain when it actually sees one.
I still couldn't reproduce it on my VM, yet I find the timing a bit
strange: it's alright we handle the illegal instruction like this, but
do we really end up doing that while scheduling although it doesn't
look like an interrupt?!
Could you share a bit more about your riscv setup? It might some
configuration/hardware specific thing.
Thanks for finding this,
Gabriele
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v5 7/9] rv: Replace tss and sncid monitors with more complete sts
2025-07-29 8:46 ` Gabriele Monaco
@ 2025-07-29 9:25 ` Nam Cao
2025-07-29 9:37 ` Nam Cao
0 siblings, 1 reply; 15+ messages in thread
From: Nam Cao @ 2025-07-29 9:25 UTC (permalink / raw)
To: Gabriele Monaco
Cc: linux-kernel, Steven Rostedt, linux-trace-kernel, linux-doc,
Ingo Molnar, Peter Zijlstra, Tomas Glozar, Juri Lelli,
Clark Williams, John Kacur
[-- Attachment #1: Type: text/plain, Size: 1922 bytes --]
On Tue, Jul 29, 2025 at 10:46:51AM +0200, Gabriele Monaco wrote:
> On Mon, 2025-07-28 at 17:53 +0200, Nam Cao wrote:
> > I gave this a try on riscv64 and observed some errors:
> >
> > [ 620.696055] rv: monitor sts does not allow event sched_switch on
> > state enable_to_exit
> > [ 621.047705] rv: monitor sts does not allow event sched_switch on
> > state enable_to_exit
> > [ 642.440209] rv: monitor sts does not allow event sched_switch on
> > state enable_to_exit
> >
> > I tested with two user programs:
> >
> > int main() { asm ("unimp"); }
> > int main() { asm ("ebreak"); }
> >
> > The two programs are repeatedly executed:
> >
> > #!/bin/bash
> > ./test1 &
> > ./test2 &
> > # ... repeat lots of time
> >
> > Any idea?
>
> Mmh I see what you're doing here..
> Those instructions are supposed to raise some sort of exception in the
> CPU which apparently disables and enables interrupts without raising an
> interrupt handler tracepoint (the discriminator for this monitor).
> This lets the monitor believe we passed the time a switch is possible
> and complain when it actually sees one.
>
> I still couldn't reproduce it on my VM, yet I find the timing a bit
> strange: it's alright we handle the illegal instruction like this, but
> do we really end up doing that while scheduling although it doesn't
> look like an interrupt?!
>
> Could you share a bit more about your riscv setup? It might some
> configuration/hardware specific thing.
Kernel:
- base: ftrace/for-next
- config: defconfig + mod2noconfig + PREEMPT_RT + monitors
Hardware:
qemu-system-riscv64 -machine virt \
-kernel ../linux/arch/riscv/boot/Image \
-append "console=ttyS0 root=/dev/vda rw" \
-nographic \
-drive if=virtio,format=raw,file=riscv64.img \
-smp 4 -m 4G
riscv64.img is a Debian trixie image from debootstrap
Test:
echo 0 > /proc/sys/debug/exception-trace
./testall # see attached
[-- Attachment #2: test.sh --]
[-- Type: application/x-sh, Size: 373 bytes --]
[-- Attachment #3: testall --]
[-- Type: text/plain, Size: 480 bytes --]
#!/bin/bash
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
./test.sh &
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v5 7/9] rv: Replace tss and sncid monitors with more complete sts
2025-07-29 9:25 ` Nam Cao
@ 2025-07-29 9:37 ` Nam Cao
2025-07-29 14:06 ` Gabriele Monaco
0 siblings, 1 reply; 15+ messages in thread
From: Nam Cao @ 2025-07-29 9:37 UTC (permalink / raw)
To: Gabriele Monaco
Cc: linux-kernel, Steven Rostedt, linux-trace-kernel, linux-doc,
Ingo Molnar, Peter Zijlstra, Tomas Glozar, Juri Lelli,
Clark Williams, John Kacur
On Tue, Jul 29, 2025 at 11:25:12AM +0200, Nam Cao wrote:
> On Tue, Jul 29, 2025 at 10:46:51AM +0200, Gabriele Monaco wrote:
> > On Mon, 2025-07-28 at 17:53 +0200, Nam Cao wrote:
> > > I gave this a try on riscv64 and observed some errors:
> > >
> > > [ 620.696055] rv: monitor sts does not allow event sched_switch on
> > > state enable_to_exit
> > > [ 621.047705] rv: monitor sts does not allow event sched_switch on
> > > state enable_to_exit
> > > [ 642.440209] rv: monitor sts does not allow event sched_switch on
> > > state enable_to_exit
> > >
> > > I tested with two user programs:
> > >
> > > int main() { asm ("unimp"); }
> > > int main() { asm ("ebreak"); }
> > >
> > > The two programs are repeatedly executed:
> > >
> > > #!/bin/bash
> > > ./test1 &
> > > ./test2 &
> > > # ... repeat lots of time
> > >
> > > Any idea?
> >
> > Mmh I see what you're doing here..
> > Those instructions are supposed to raise some sort of exception in the
> > CPU which apparently disables and enables interrupts without raising an
> > interrupt handler tracepoint (the discriminator for this monitor).
> > This lets the monitor believe we passed the time a switch is possible
> > and complain when it actually sees one.
> >
> > I still couldn't reproduce it on my VM, yet I find the timing a bit
> > strange: it's alright we handle the illegal instruction like this, but
> > do we really end up doing that while scheduling although it doesn't
> > look like an interrupt?!
> >
> > Could you share a bit more about your riscv setup? It might some
> > configuration/hardware specific thing.
>
> Kernel:
> - base: ftrace/for-next
> - config: defconfig + mod2noconfig + PREEMPT_RT + monitors
>
> Hardware:
> qemu-system-riscv64 -machine virt \
> -kernel ../linux/arch/riscv/boot/Image \
> -append "console=ttyS0 root=/dev/vda rw" \
> -nographic \
> -drive if=virtio,format=raw,file=riscv64.img \
> -smp 4 -m 4G
>
> riscv64.img is a Debian trixie image from debootstrap
>
> Test:
> echo 0 > /proc/sys/debug/exception-trace
> ./testall # see attached
I should note that this takes a few tries before something shows up.
Below is the backtrace, in case it helps:
illegal 3246 [000] 1020.132675: rv:error_sts: event sched_switch not expected in the state enable_to_exit
ffffffff8013231c __traceiter_error_sts+0x28 ([kernel.kallsyms])
ffffffff8013231c __traceiter_error_sts+0x28 ([kernel.kallsyms])
ffffffff80138aa4 da_event_sts+0x198 ([kernel.kallsyms])
ffffffff80138cf0 handle_sched_switch+0x46 ([kernel.kallsyms])
ffffffff80aaf222 __schedule+0x4ba ([kernel.kallsyms])
ffffffff80aafb80 preempt_schedule_irq+0x32 ([kernel.kallsyms])
ffffffff80aac714 irqentry_exit+0x76 ([kernel.kallsyms])
ffffffff80aac1dc do_irq+0x38 ([kernel.kallsyms])
ffffffff80ab7da6 __lock_text_end+0x12e ([kernel.kallsyms])
ffffffff80a93e50 mas_find+0x0 ([kernel.kallsyms])
ffffffff8021ea60 vms_clear_ptes+0xe8 ([kernel.kallsyms])
ffffffff8021f81a vms_complete_munmap_vmas+0x58 ([kernel.kallsyms])
ffffffff80220706 do_vmi_align_munmap+0x15c ([kernel.kallsyms])
ffffffff802207d0 do_vmi_munmap+0xa6 ([kernel.kallsyms])
ffffffff80221f3c __vm_munmap+0xa2 ([kernel.kallsyms])
ffffffff8020be7c vm_munmap+0xe ([kernel.kallsyms])
ffffffff802bbdbe elf_load+0x14c ([kernel.kallsyms])
ffffffff802bc1f4 load_elf_binary+0x36e ([kernel.kallsyms])
ffffffff80264426 bprm_execve+0x254 ([kernel.kallsyms])
ffffffff8026570c do_execveat_common.isra.0+0x11e ([kernel.kallsyms])
ffffffff802664de __riscv_sys_execve+0x32 ([kernel.kallsyms])
ffffffff80aabf84 do_trap_ecall_u+0x1bc ([kernel.kallsyms])
ffffffff80ab7dc8 __lock_text_end+0x150 ([kernel.kallsyms])
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v5 7/9] rv: Replace tss and sncid monitors with more complete sts
2025-07-29 9:37 ` Nam Cao
@ 2025-07-29 14:06 ` Gabriele Monaco
2025-07-30 12:52 ` Nam Cao
0 siblings, 1 reply; 15+ messages in thread
From: Gabriele Monaco @ 2025-07-29 14:06 UTC (permalink / raw)
To: Nam Cao
Cc: linux-kernel, Steven Rostedt, linux-trace-kernel, linux-doc,
Ingo Molnar, Peter Zijlstra, Tomas Glozar, Juri Lelli,
Clark Williams, John Kacur
On Tue, 2025-07-29 at 11:37 +0200, Nam Cao wrote:
> On Tue, Jul 29, 2025 at 11:25:12AM +0200, Nam Cao wrote:
> > Kernel:
> > - base: ftrace/for-next
I assume you mean rv/for-next ? The one that includes all changes as of
yesterday.
> > - config: defconfig + mod2noconfig + PREEMPT_RT + monitors
> >
> > Hardware:
> > qemu-system-riscv64 -machine virt \
> > -kernel ../linux/arch/riscv/boot/Image \
> > -append "console=ttyS0 root=/dev/vda rw" \
> > -nographic \
> > -drive if=virtio,format=raw,file=riscv64.img \
> > -smp 4 -m 4G
> >
> > riscv64.img is a Debian trixie image from debootstrap
> >
> > Test:
> > echo 0 > /proc/sys/debug/exception-trace
> > ./testall # see attached
>
> I should note that this takes a few tries before something shows up.
>
Thanks for all the details, but I still can't reproduce nor understand
what can be triggering the issue.
I tried enabling sts and setting panic as the reactor (to avoid missing
it with all the rubbish that gets printed on the dmesg) and run
testall. Still cannot see the error.
What might help would be to see the trace with irq_enable and
irq_disable around the error, something like (not tested):
trace-cmd stream -e irq_enable -e irq_disable -e error_sts -e
irq_handler_entry -- sh testall | grep -B 10 error
The problem here is not when the error occurs, but a couple of events
earlier (where I possibly miss something that looks like an interrupt).
Thanks,
Gabriele
> Below is the backtrace, in case it helps:
>
> illegal 3246 [000] 1020.132675: rv:error_sts: event sched_switch
> not expected in the state enable_to_exit
> ffffffff8013231c __traceiter_error_sts+0x28
> ([kernel.kallsyms])
> ffffffff8013231c __traceiter_error_sts+0x28
> ([kernel.kallsyms])
> ffffffff80138aa4 da_event_sts+0x198 ([kernel.kallsyms])
> ffffffff80138cf0 handle_sched_switch+0x46 ([kernel.kallsyms])
> ffffffff80aaf222 __schedule+0x4ba ([kernel.kallsyms])
> ffffffff80aafb80 preempt_schedule_irq+0x32
> ([kernel.kallsyms])
> ffffffff80aac714 irqentry_exit+0x76 ([kernel.kallsyms])
> ffffffff80aac1dc do_irq+0x38 ([kernel.kallsyms])
> ffffffff80ab7da6 __lock_text_end+0x12e ([kernel.kallsyms])
> ffffffff80a93e50 mas_find+0x0 ([kernel.kallsyms])
> ffffffff8021ea60 vms_clear_ptes+0xe8 ([kernel.kallsyms])
> ffffffff8021f81a vms_complete_munmap_vmas+0x58
> ([kernel.kallsyms])
> ffffffff80220706 do_vmi_align_munmap+0x15c
> ([kernel.kallsyms])
> ffffffff802207d0 do_vmi_munmap+0xa6 ([kernel.kallsyms])
> ffffffff80221f3c __vm_munmap+0xa2 ([kernel.kallsyms])
> ffffffff8020be7c vm_munmap+0xe ([kernel.kallsyms])
> ffffffff802bbdbe elf_load+0x14c ([kernel.kallsyms])
> ffffffff802bc1f4 load_elf_binary+0x36e ([kernel.kallsyms])
> ffffffff80264426 bprm_execve+0x254 ([kernel.kallsyms])
> ffffffff8026570c do_execveat_common.isra.0+0x11e
> ([kernel.kallsyms])
> ffffffff802664de __riscv_sys_execve+0x32 ([kernel.kallsyms])
> ffffffff80aabf84 do_trap_ecall_u+0x1bc ([kernel.kallsyms])
> ffffffff80ab7dc8 __lock_text_end+0x150 ([kernel.kallsyms])
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 7/9] rv: Replace tss and sncid monitors with more complete sts
2025-07-29 14:06 ` Gabriele Monaco
@ 2025-07-30 12:52 ` Nam Cao
2025-07-30 14:16 ` Nam Cao
0 siblings, 1 reply; 15+ messages in thread
From: Nam Cao @ 2025-07-30 12:52 UTC (permalink / raw)
To: Gabriele Monaco
Cc: linux-kernel, Steven Rostedt, linux-trace-kernel, linux-doc,
Ingo Molnar, Peter Zijlstra, Tomas Glozar, Juri Lelli,
Clark Williams, John Kacur
On Tue, Jul 29, 2025 at 04:06:17PM +0200, Gabriele Monaco wrote:
> On Tue, 2025-07-29 at 11:37 +0200, Nam Cao wrote:
> > On Tue, Jul 29, 2025 at 11:25:12AM +0200, Nam Cao wrote:
> > > Kernel:
> > > - base: ftrace/for-next
>
> I assume you mean rv/for-next ? The one that includes all changes as of
> yesterday.
I meant I apply this series on top of ftrace/for-next. But that one is
close enough.
> Thanks for all the details, but I still can't reproduce nor understand
> what can be triggering the issue.
>
> I tried enabling sts and setting panic as the reactor (to avoid missing
> it with all the rubbish that gets printed on the dmesg) and run
> testall. Still cannot see the error.
Welcome to the "but it works on my machine" camp. I was there 2 weeks ago,
it was not fun.
> What might help would be to see the trace with irq_enable and
> irq_disable around the error, something like (not tested):
>
> trace-cmd stream -e irq_enable -e irq_disable -e error_sts -e
> irq_handler_entry -- sh testall | grep -B 10 error
I do not have trace-cmd in the riscv image, but I do have perf. I will give
it a try.
> The problem here is not when the error occurs, but a couple of events
> earlier (where I possibly miss something that looks like an interrupt).
I just accidentally hit this error again, not on riscv but on x86, while
doing something unrelated. Let me figure out a minimal way to reproduce it.
Nam
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 7/9] rv: Replace tss and sncid monitors with more complete sts
2025-07-30 12:52 ` Nam Cao
@ 2025-07-30 14:16 ` Nam Cao
2025-07-30 14:44 ` Nam Cao
0 siblings, 1 reply; 15+ messages in thread
From: Nam Cao @ 2025-07-30 14:16 UTC (permalink / raw)
To: Gabriele Monaco
Cc: linux-kernel, Steven Rostedt, linux-trace-kernel, linux-doc,
Ingo Molnar, Peter Zijlstra, Tomas Glozar, Juri Lelli,
Clark Williams, John Kacur
On Wed, Jul 30, 2025 at 02:52:25PM +0200, Nam Cao wrote:
> I do not have trace-cmd in the riscv image, but I do have perf. I will give
> it a try.
Instead, I replaced with tracepoints with trace_printk() and managed to
captured the log moments before disaster below.
I'm not sure what I'm seeing, just dumping this here, maybe you have an
idea.
Quite interesting that the last "normal" line for cpu1 is:
test-762 [001] dn... 112.407548: da_event_sts: cant_sched x irq_enable -> can_sched (final)
But in the next error line, it mentions the "enable_to_exit". Where did
this state come from??
<...>-1621 [001] d.... 119.919846: da_event_sts: rv: monitor sts does not allow event sched_switch on state enable_to_exit
Something very strange is going on here..
Nam
test-1198 [000] d.... 112.407332: da_event_sts: cant_sched x irq_enable -> can_sched (final)
test-762 [001] d.... 112.407338: da_event_sts: can_sched x irq_disable -> cant_sched
test-762 [001] d.... 112.407347: da_event_sts: cant_sched x irq_enable -> can_sched (final)
test-1198 [000] d.... 112.407351: da_event_sts: can_sched x irq_disable -> cant_sched
test-762 [001] d.... 112.407363: da_event_sts: can_sched x irq_disable -> cant_sched
test-1198 [000] d.h.. 112.407370: da_event_sts: cant_sched x irq_entry -> cant_sched
test-762 [001] d.h.. 112.407383: da_event_sts: cant_sched x irq_entry -> cant_sched
test-637 [002] dn... 112.407413: da_event_sts: cant_sched x irq_enable -> can_sched (final)
illegal-1091 [003] dns.. 112.407418: da_event_sts: in_irq x irq_enable -> scheduling
test-637 [002] .n... 112.407424: da_event_sts: can_sched x schedule_entry -> scheduling
illegal-1091 [003] dns.. 112.407431: da_event_sts: scheduling x irq_disable -> disable_to_switch
test-637 [002] dn... 112.407433: da_event_sts: scheduling x irq_disable -> disable_to_switch
illegal-1091 [003] dns.. 112.407438: da_event_sts: disable_to_switch x irq_enable -> enable_to_exit
illegal-1091 [003] dns.. 112.407448: da_event_sts: enable_to_exit x irq_disable -> enable_to_exit
test-1198 [000] dns.. 112.407459: da_event_sts: cant_sched x irq_enable -> can_sched (final)
illegal-1091 [003] dns.. 112.407460: da_event_sts: enable_to_exit x irq_enable -> enable_to_exit
test-637 [002] d.... 112.407468: da_event_sts: disable_to_switch x sched_switch -> switching
test-1198 [000] dns.. 112.407472: da_event_sts: can_sched x irq_disable -> cant_sched
illegal-1091 [003] dns.. 112.407472: da_event_sts: enable_to_exit x irq_disable -> enable_to_exit
illegal-1091 [003] dn... 112.407481: da_event_sts: enable_to_exit x irq_enable -> enable_to_exit
test-1198 [000] dns.. 112.407483: da_event_sts: cant_sched x irq_enable -> can_sched (final)
test-762 [001] dns.. 112.407488: da_event_sts: cant_sched x irq_enable -> can_sched (final)
illegal-1091 [003] dn... 112.407492: da_event_sts: enable_to_exit x irq_disable -> enable_to_exit
test-1198 [000] dns.. 112.407493: da_event_sts: can_sched x irq_disable -> cant_sched
test-762 [001] dns.. 112.407501: da_event_sts: can_sched x irq_disable -> cant_sched
test-1198 [000] dns.. 112.407501: da_event_sts: cant_sched x irq_enable -> can_sched (final)
test-762 [001] dns.. 112.407508: da_event_sts: cant_sched x irq_enable -> can_sched (final)
test-1198 [000] dns.. 112.407513: da_event_sts: can_sched x irq_disable -> cant_sched
test-649 [002] d.... 112.407517: da_event_sts: switching x irq_enable -> enable_to_exit
test-762 [001] dns.. 112.407518: da_event_sts: can_sched x irq_disable -> cant_sched
test-1198 [000] dns.. 112.407521: da_event_sts: cant_sched x irq_enable -> can_sched (final)
test-762 [001] dns.. 112.407529: da_event_sts: cant_sched x irq_enable -> can_sched (final)
test-649 [002] ..... 112.407530: da_event_sts: enable_to_exit x schedule_exit -> can_sched (final)
test-1198 [000] dns.. 112.407541: da_event_sts: can_sched x irq_disable -> cant_sched
test-762 [001] dns.. 112.407541: da_event_sts: can_sched x irq_disable -> cant_sched
test-1198 [000] dns.. 112.407548: da_event_sts: cant_sched x irq_enable -> can_sched (final)
test-762 [001] dn... 112.407548: da_event_sts: cant_sched x irq_enable -> can_sched (final)
test-1198 [000] dns.. 112.407560: da_event_sts: can_sched x irq_disable -> cant_sched
test-1198 [000] dns.. 112.407567: da_event_sts: cant_sched x irq_enable -> can_sched (final)
test-1198 [000] dns.. 112.407578: da_event_sts: can_sched x irq_disable -> cant_sched
test-1198 [000] dns.. 112.407585: da_event_sts: cant_sched x irq_enable -> can_sched (final)
test-649 [002] d.... 112.407596: da_event_sts: can_sched x irq_disable -> cant_sched
illegal-1091 [003] d.... 112.410126: da_event_sts: rv: monitor sts does not allow event sched_switch on state enable_to_exit
<...>-1621 [001] d.... 119.919846: da_event_sts: rv: monitor sts does not allow event sched_switch on state enable_to_exit
test-642 [001] d.... 121.459650: da_event_sts: rv: monitor sts does not allow event sched_switch on state enable_to_exit
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v5 7/9] rv: Replace tss and sncid monitors with more complete sts
2025-07-30 14:16 ` Nam Cao
@ 2025-07-30 14:44 ` Nam Cao
2025-07-30 15:09 ` Nam Cao
0 siblings, 1 reply; 15+ messages in thread
From: Nam Cao @ 2025-07-30 14:44 UTC (permalink / raw)
To: Gabriele Monaco
Cc: linux-kernel, Steven Rostedt, linux-trace-kernel, linux-doc,
Ingo Molnar, Peter Zijlstra, Tomas Glozar, Juri Lelli,
Clark Williams, John Kacur
On Wed, Jul 30, 2025 at 04:16:46PM +0200, Nam Cao wrote:
> Quite interesting that the last "normal" line for cpu1 is:
>
> test-762 [001] dn... 112.407548: da_event_sts: cant_sched x irq_enable -> can_sched (final)
>
> But in the next error line, it mentions the "enable_to_exit". Where did
> this state come from??
>
> <...>-1621 [001] d.... 119.919846: da_event_sts: rv: monitor sts does not allow event sched_switch on state enable_to_exit
Never mind about this one, only cpu3 is accurate here, because I cut off
the trace_printk() as soon as the first error appears:
diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index 17fa4f6e5ea6..927cf2cda03f 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -18,15 +18,22 @@
#ifdef CONFIG_RV_REACTORS
+static bool nam_stop = true;
+
#define DECLARE_RV_REACTING_HELPERS(name, type) \
static void cond_react_##name(type curr_state, type event) \
{ \
if (!rv_reacting_on() || !rv_##name.react) \
return; \
+ nam_stop = true;\
rv_##name.react("rv: monitor %s does not allow event %s on state %s\n", \
#name, \
model_get_event_name_##name(event), \
model_get_state_name_##name(curr_state)); \
+ trace_printk("rv: monitor %s does not allow event %s on state %s\n", \
+ #name, \
+ model_get_event_name_##name(event), \
+ model_get_state_name_##name(curr_state)); \
}
#else /* CONFIG_RV_REACTOR */
@@ -136,6 +143,14 @@ da_event_##name(struct da_monitor *da_mon, enum events_##name event) \
model_get_event_name_##name(event), \
model_get_state_name_##name(next_state), \
model_is_final_state_##name(next_state)); \
+ if (nam_stop)\
+ return true;\
+ trace_printk("%s x %s -> %s%s\n", \
+ model_get_state_name_##name(curr_state), \
+ model_get_event_name_##name(event), \
+ model_get_state_name_##name(next_state), \
+ model_is_final_state_##name(next_state)? \
+ " (final)" : ""); \
return true; \
} \
} \
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v5 7/9] rv: Replace tss and sncid monitors with more complete sts
2025-07-30 14:44 ` Nam Cao
@ 2025-07-30 15:09 ` Nam Cao
0 siblings, 0 replies; 15+ messages in thread
From: Nam Cao @ 2025-07-30 15:09 UTC (permalink / raw)
To: Gabriele Monaco
Cc: linux-kernel, Steven Rostedt, linux-trace-kernel, linux-doc,
Ingo Molnar, Peter Zijlstra, Tomas Glozar, Juri Lelli,
Clark Williams, John Kacur
Here is a deeper log, CPU3 only:
illegal-1091 [003] d.... 112.407127: da_event_sts: can_sched x irq_disable -> cant_sched
illegal-1091 [003] d.... 112.407163: da_event_sts: cant_sched x irq_enable -> can_sched (final)
illegal-1091 [003] d.... 112.407177: da_event_sts: can_sched x irq_disable -> cant_sched
illegal-1091 [003] d.... 112.407186: da_event_sts: cant_sched x irq_enable -> can_sched (final)
illegal-1091 [003] d.... 112.407196: da_event_sts: can_sched x irq_disable -> cant_sched
illegal-1091 [003] d.... 112.407203: da_event_sts: cant_sched x irq_enable -> can_sched (final)
illegal-1091 [003] d.... 112.407212: da_event_sts: can_sched x irq_disable -> cant_sched
illegal-1091 [003] d.... 112.407218: da_event_sts: cant_sched x irq_enable -> can_sched (final)
illegal-1091 [003] d.... 112.407227: da_event_sts: can_sched x irq_disable -> cant_sched
illegal-1091 [003] d.... 112.407234: da_event_sts: cant_sched x irq_enable -> can_sched (final)
illegal-1091 [003] d.... 112.407254: da_event_sts: can_sched x irq_disable -> cant_sched
illegal-1091 [003] d.... 112.407261: da_event_sts: cant_sched x irq_enable -> can_sched (final)
illegal-1091 [003] ..... 112.407271: da_event_sts: can_sched x schedule_entry -> scheduling
illegal-1091 [003] d.... 112.407312: da_event_sts: scheduling x irq_disable -> disable_to_switch
illegal-1091 [003] d.h.. 112.407332: da_event_sts: disable_to_switch x irq_entry -> in_irq
illegal-1091 [003] dns.. 112.407418: da_event_sts: in_irq x irq_enable -> scheduling
illegal-1091 [003] dns.. 112.407431: da_event_sts: scheduling x irq_disable -> disable_to_switch
illegal-1091 [003] dns.. 112.407438: da_event_sts: disable_to_switch x irq_enable -> enable_to_exit
illegal-1091 [003] dns.. 112.407448: da_event_sts: enable_to_exit x irq_disable -> enable_to_exit
illegal-1091 [003] dns.. 112.407460: da_event_sts: enable_to_exit x irq_enable -> enable_to_exit
illegal-1091 [003] dns.. 112.407472: da_event_sts: enable_to_exit x irq_disable -> enable_to_exit
illegal-1091 [003] dn... 112.407481: da_event_sts: enable_to_exit x irq_enable -> enable_to_exit
illegal-1091 [003] dn... 112.407492: da_event_sts: enable_to_exit x irq_disable -> enable_to_exit
illegal-1091 [003] d.... 112.410126: da_event_sts: rv: monitor sts does not allow event sched_switch on state enable_to_exit
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 7/9] rv: Replace tss and sncid monitors with more complete sts
2025-07-28 15:53 ` Nam Cao
2025-07-29 8:46 ` Gabriele Monaco
@ 2025-07-30 16:28 ` Nam Cao
1 sibling, 0 replies; 15+ messages in thread
From: Nam Cao @ 2025-07-30 16:28 UTC (permalink / raw)
To: Gabriele Monaco
Cc: linux-kernel, Steven Rostedt, Jonathan Corbet, Masami Hiramatsu,
linux-trace-kernel, linux-doc, Ingo Molnar, Peter Zijlstra,
Tomas Glozar, Juri Lelli, Clark Williams, John Kacur
On Mon, Jul 28, 2025 at 05:53:34PM +0200, Nam Cao wrote:
> On Mon, Jul 28, 2025 at 03:50:19PM +0200, Gabriele Monaco wrote:
> > The tss monitor currently guarantees task switches can happen only while
> > scheduling, whereas the sncid monitor enforces scheduling occurs with
> > interrupt disabled.
> >
> > Replace the monitors with a more comprehensive specification which
> > implies both but also ensures that:
> > * each scheduler call disable interrupts to switch
> > * each task switch happens with interrupts disabled
> >
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
>
> I gave this a try on riscv64 and observed some errors:
>
> [ 620.696055] rv: monitor sts does not allow event sched_switch on state enable_to_exit
> [ 621.047705] rv: monitor sts does not allow event sched_switch on state enable_to_exit
> [ 642.440209] rv: monitor sts does not allow event sched_switch on state enable_to_exit
>
> I tested with two user programs:
>
> int main() { asm ("unimp"); }
> int main() { asm ("ebreak"); }
>
> The two programs are repeatedly executed:
>
> #!/bin/bash
> ./test1 &
> ./test2 &
> # ... repeat lots of time
Okay, I think I know why..
It seems the monitor is in scheduling state. Then it sees a pair of
irq_disable and irq_enable, and it mistakenly thinks that this is the
is_switch==false case in __schedule. So it thinks it is at the end of
__schedule(), and does not expect a switch_switch.
However, this is wrong. The irq_disable and irq_enable pair is not from
__schedule(), it is from softirq (see below).
In short, the monitor thinks it is at the end of __schedule(), but actually
it is still at the beginning.
That's just from my limited understanding of the model, so I may be wrong.
What do you think?
Nam
test-256 [002] dns.. 63.070743: da_event_sts: scheduling x irq_disable -> disable_to_switch
test-256 [002] dns.. 63.070748: <stack trace>
=> trace_dump_stack
=> da_event_sts
=> handle_irq_disable
=> trace_hardirqs_off.part.0
=> trace_hardirqs_off
=> note_gp_changes
=> rcu_core
=> rcu_core_si
=> handle_softirqs
=> __irq_exit_rcu
=> irq_exit_rcu
=> handle_riscv_irq
=> call_on_irq_stack
test-256 [002] dns.. 63.070755: da_event_sts: disable_to_switch x irq_enable -> enable_to_exit
test-256 [002] dns.. 63.070760: <stack trace>
=> trace_dump_stack
=> da_event_sts
=> handle_irq_enable
=> trace_hardirqs_on
=> note_gp_changes
=> rcu_core
=> rcu_core_si
=> handle_softirqs
=> __irq_exit_rcu
=> irq_exit_rcu
=> handle_riscv_irq
=> call_on_irq_stack
=> call_on_irq_stack
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v5 8/9] rv: Add nrp and sssw per-task monitors
[not found] <20250728135022.255578-1-gmonaco@redhat.com>
2025-07-28 13:50 ` [PATCH v5 7/9] rv: Replace tss and sncid monitors with more complete sts Gabriele Monaco
@ 2025-07-28 13:50 ` Gabriele Monaco
2025-07-28 15:56 ` Nam Cao
2025-07-28 13:50 ` [PATCH v5 9/9] rv: Add opid per-cpu monitor Gabriele Monaco
2 siblings, 1 reply; 15+ messages in thread
From: Gabriele Monaco @ 2025-07-28 13:50 UTC (permalink / raw)
To: linux-kernel, Steven Rostedt, Jonathan Corbet, Masami Hiramatsu,
linux-trace-kernel, linux-doc
Cc: Gabriele Monaco, Ingo Molnar, Peter Zijlstra, Nam Cao,
Tomas Glozar, Juri Lelli, Clark Williams, John Kacur
Add 2 per-task monitors as part of the sched model:
* nrp: need-resched preempts
Monitor to ensure preemption requires need resched.
* sssw: set state sleep and wakeup
Monitor to ensure sched_set_state to sleepable leads to sleeping and
sleeping tasks require wakeup.
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
Documentation/trace/rv/monitor_sched.rst | 167 +++++++++++++++++++++
kernel/trace/rv/Kconfig | 2 +
kernel/trace/rv/Makefile | 2 +
kernel/trace/rv/monitors/nrp/Kconfig | 16 ++
kernel/trace/rv/monitors/nrp/nrp.c | 138 +++++++++++++++++
kernel/trace/rv/monitors/nrp/nrp.h | 75 +++++++++
kernel/trace/rv/monitors/nrp/nrp_trace.h | 15 ++
kernel/trace/rv/monitors/sched/Kconfig | 1 +
kernel/trace/rv/monitors/sssw/Kconfig | 15 ++
kernel/trace/rv/monitors/sssw/sssw.c | 116 ++++++++++++++
kernel/trace/rv/monitors/sssw/sssw.h | 105 +++++++++++++
kernel/trace/rv/monitors/sssw/sssw_trace.h | 15 ++
kernel/trace/rv/rv_trace.h | 2 +
tools/verification/models/sched/nrp.dot | 29 ++++
tools/verification/models/sched/sssw.dot | 30 ++++
15 files changed, 728 insertions(+)
create mode 100644 kernel/trace/rv/monitors/nrp/Kconfig
create mode 100644 kernel/trace/rv/monitors/nrp/nrp.c
create mode 100644 kernel/trace/rv/monitors/nrp/nrp.h
create mode 100644 kernel/trace/rv/monitors/nrp/nrp_trace.h
create mode 100644 kernel/trace/rv/monitors/sssw/Kconfig
create mode 100644 kernel/trace/rv/monitors/sssw/sssw.c
create mode 100644 kernel/trace/rv/monitors/sssw/sssw.h
create mode 100644 kernel/trace/rv/monitors/sssw/sssw_trace.h
create mode 100644 tools/verification/models/sched/nrp.dot
create mode 100644 tools/verification/models/sched/sssw.dot
diff --git a/Documentation/trace/rv/monitor_sched.rst b/Documentation/trace/rv/monitor_sched.rst
index 6c4c00216c07..11ef963cb578 100644
--- a/Documentation/trace/rv/monitor_sched.rst
+++ b/Documentation/trace/rv/monitor_sched.rst
@@ -174,6 +174,173 @@ running one, no real task switch occurs but interrupts are disabled nonetheless:
| | irq_entry
+---------------+ irq_enable
+Monitor nrp
+-----------
+
+The need resched preempts (nrp) monitor ensures preemption requires
+``need_resched``. Only kernel preemption is considered, since preemption
+while returning to userspace, for this monitor, is indistinguishable from
+``sched_switch_yield`` (described in the sssw monitor).
+A kernel preemption is whenever ``__schedule`` is called with the preemption
+flag set to true (e.g. from preempt_enable or exiting from interrupts). This
+type of preemption occurs after the need for ``rescheduling`` has been set.
+This is not valid for the *lazy* variant of the flag, which causes only
+userspace preemption.
+A ``schedule_entry_preempt`` may involve a task switch or not, in the latter
+case, a task goes through the scheduler from a preemption context but it is
+picked as the next task to run. Since the scheduler runs, this clears the need
+to reschedule. The ``any_thread_running`` state does not imply the monitored
+task is not running as this monitor does not track the outcome of scheduling.
+
+In theory, a preemption can only occur after the ``need_resched`` flag is set. In
+practice, however, it is possible to see a preemption where the flag is not
+set. This can happen in one specific condition::
+
+ need_resched
+ preempt_schedule()
+ preempt_schedule_irq()
+ __schedule()
+ !need_resched
+ __schedule()
+
+In the situation above, standard preemption starts (e.g. from preempt_enable
+when the flag is set), an interrupt occurs before scheduling and, on its exit
+path, it schedules, which clears the ``need_resched`` flag.
+When the preempted task runs again, the standard preemption started earlier
+resumes, although the flag is no longer set. The monitor considers this a
+``nested_preemption``, this allows another preemption without re-setting the
+flag. This condition relaxes the monitor constraints and may catch false
+negatives (i.e. no real ``nested_preemptions``) but makes the monitor more
+robust and able to validate other scenarios.
+For simplicity, the monitor starts in ``preempt_irq``, although no interrupt
+occurred, as the situation above is hard to pinpoint::
+
+ schedule_entry
+ irq_entry #===========================================#
+ +-------------------------- H H
+ | H H
+ +-------------------------> H any_thread_running H
+ H H
+ +-------------------------> H H
+ | #===========================================#
+ | schedule_entry | ^
+ | schedule_entry_preempt | sched_need_resched | schedule_entry
+ | | schedule_entry_preempt
+ | v |
+ | +----------------------+ |
+ | +--- | | |
+ | sched_need_resched | | rescheduling | -+
+ | +--> | |
+ | +----------------------+
+ | | irq_entry
+ | v
+ | +----------------------+
+ | | | ---+
+ | ---> | | | sched_need_resched
+ | | preempt_irq | | irq_entry
+ | | | <--+
+ | | | <--+
+ | +----------------------+ |
+ | | schedule_entry | sched_need_resched
+ | | schedule_entry_preempt |
+ | v |
+ | +-----------------------+ |
+ +-------------------------- | nested_preempt | --+
+ +-----------------------+
+ ^ irq_entry |
+ +-------------------+
+
+Due to how the ``need_resched`` flag on the preemption count works on arm64,
+this monitor is unstable on that architecture, as it often records preemption
+when the flag is not set, even in presence of the workaround above.
+For the time being, the monitor is disabled by default on arm64.
+
+Monitor sssw
+------------
+
+The set state sleep and wakeup (sssw) monitor ensures ``set_state`` to
+sleepable leads to sleeping and sleeping tasks require wakeup. It includes the
+following types of switch:
+
+* ``switch_suspend``:
+ a task puts itself to sleep, this can happen only after explicitly setting
+ the task to ``sleepable``. After a task is suspended, it needs to be woken up
+ (``waking`` state) before being switched in again.
+ Setting the task's state to ``sleepable`` can be reverted before switching if it
+ is woken up or set to ``runnable``.
+* ``switch_blocking``:
+ a special case of a ``switch_suspend`` where the task is waiting on a
+ sleeping RT lock (``PREEMPT_RT`` only), it is common to see wakeup and set
+ state events racing with each other and this leads the model to perceive this
+ type of switch when the task is not set to sleepable. This is a limitation of
+ the model in SMP system and workarounds may slow down the system.
+* ``switch_preempt``:
+ a task switch as a result of kernel preemption (``schedule_entry_preempt`` in
+ the nrp model).
+* ``switch_yield``:
+ a task explicitly calls the scheduler or is preempted while returning to
+ userspace. It can happen after a ``yield`` system call, from the idle task or
+ if the ``need_resched`` flag is set. By definition, a task cannot yield while
+ ``sleepable`` as that would be a suspension. A special case of a yield occurs
+ when a task in ``TASK_INTERRUPTIBLE`` calls the scheduler while a signal is
+ pending. The task doesn't go through the usual blocking/waking and is set
+ back to runnable, the resulting switch (if there) looks like a yield to the
+ ``signal_wakeup`` state and is followed by the signal delivery. From this
+ state, the monitor expects a signal even if it sees a wakeup event, although
+ not necessary, to rule out false negatives.
+
+This monitor doesn't include a running state, ``sleepable`` and ``runnable``
+are only referring to the task's desired state, which could be scheduled out
+(e.g. due to preemption). However, it does include the event
+``sched_switch_in`` to represent when a task is allowed to become running. This
+can be triggered also by preemption, but cannot occur after the task got to
+``sleeping`` before a ``wakeup`` occurs::
+
+ +--------------------------------------------------------------------------+
+ | |
+ | |
+ | switch_suspend | |
+ | switch_blocking | |
+ v v |
+ +----------+ #==========================# set_state_runnable |
+ | | H H wakeup |
+ | | H H switch_in |
+ | | H H switch_yield |
+ | sleeping | H H switch_preempt |
+ | | H H signal_deliver |
+ | | switch_ H H ------+ |
+ | | _blocking H runnable H | |
+ | | <----------- H H <-----+ |
+ +----------+ H H |
+ | wakeup H H |
+ +---------------------> H H |
+ H H |
+ +---------> H H |
+ | #==========================# |
+ | | ^ |
+ | | | set_state_runnable |
+ | | | wakeup |
+ | set_state_sleepable | +------------------------+
+ | v | |
+ | +--------------------------+ set_state_sleepable
+ | | | switch_in
+ | | | switch_preempt
+ signal_deliver | sleepable | signal_deliver
+ | | | ------+
+ | | | |
+ | | | <-----+
+ | +--------------------------+
+ | | ^
+ | switch_yield | set_state_sleepable
+ | v |
+ | +---------------+ |
+ +---------- | signal_wakeup | -+
+ +---------------+
+ ^ | switch_in
+ | | switch_preempt
+ | | switch_yield
+ +-----------+ wakeup
+
References
----------
diff --git a/kernel/trace/rv/Kconfig b/kernel/trace/rv/Kconfig
index b688b24081c8..59d0db898d4a 100644
--- a/kernel/trace/rv/Kconfig
+++ b/kernel/trace/rv/Kconfig
@@ -55,6 +55,8 @@ source "kernel/trace/rv/monitors/snroc/Kconfig"
source "kernel/trace/rv/monitors/scpd/Kconfig"
source "kernel/trace/rv/monitors/snep/Kconfig"
source "kernel/trace/rv/monitors/sts/Kconfig"
+source "kernel/trace/rv/monitors/nrp/Kconfig"
+source "kernel/trace/rv/monitors/sssw/Kconfig"
# Add new sched monitors here
source "kernel/trace/rv/monitors/rtapp/Kconfig"
diff --git a/kernel/trace/rv/Makefile b/kernel/trace/rv/Makefile
index 1939d3d7621c..2afac88539d3 100644
--- a/kernel/trace/rv/Makefile
+++ b/kernel/trace/rv/Makefile
@@ -14,6 +14,8 @@ obj-$(CONFIG_RV_MON_RTAPP) += monitors/rtapp/rtapp.o
obj-$(CONFIG_RV_MON_PAGEFAULT) += monitors/pagefault/pagefault.o
obj-$(CONFIG_RV_MON_SLEEP) += monitors/sleep/sleep.o
obj-$(CONFIG_RV_MON_STS) += monitors/sts/sts.o
+obj-$(CONFIG_RV_MON_NRP) += monitors/nrp/nrp.o
+obj-$(CONFIG_RV_MON_SSSW) += monitors/sssw/sssw.o
# Add new monitors here
obj-$(CONFIG_RV_REACTORS) += rv_reactors.o
obj-$(CONFIG_RV_REACT_PRINTK) += reactor_printk.o
diff --git a/kernel/trace/rv/monitors/nrp/Kconfig b/kernel/trace/rv/monitors/nrp/Kconfig
new file mode 100644
index 000000000000..f5ec08f65535
--- /dev/null
+++ b/kernel/trace/rv/monitors/nrp/Kconfig
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+config RV_MON_NRP
+ depends on RV
+ depends on RV_MON_SCHED
+ default y if !ARM64
+ select DA_MON_EVENTS_ID
+ bool "nrp monitor"
+ help
+ Monitor to ensure preemption requires need resched.
+ This monitor is part of the sched monitors collection.
+
+ This monitor is unstable on arm64, say N unless you are testing it.
+
+ For further information, see:
+ Documentation/trace/rv/monitor_sched.rst
diff --git a/kernel/trace/rv/monitors/nrp/nrp.c b/kernel/trace/rv/monitors/nrp/nrp.c
new file mode 100644
index 000000000000..5a83b7171432
--- /dev/null
+++ b/kernel/trace/rv/monitors/nrp/nrp.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/ftrace.h>
+#include <linux/tracepoint.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/rv.h>
+#include <rv/instrumentation.h>
+#include <rv/da_monitor.h>
+
+#define MODULE_NAME "nrp"
+
+#include <trace/events/irq.h>
+#include <trace/events/sched.h>
+#include <rv_trace.h>
+#include <monitors/sched/sched.h>
+
+#include "nrp.h"
+
+static struct rv_monitor rv_nrp;
+DECLARE_DA_MON_PER_TASK(nrp, unsigned char);
+
+#ifdef CONFIG_X86_LOCAL_APIC
+#include <asm/trace/irq_vectors.h>
+
+static void handle_vector_irq_entry(void *data, int vector)
+{
+ da_handle_event_nrp(current, irq_entry_nrp);
+}
+
+static void attach_vector_irq(void)
+{
+ rv_attach_trace_probe("nrp", local_timer_entry, handle_vector_irq_entry);
+ if (IS_ENABLED(CONFIG_IRQ_WORK))
+ rv_attach_trace_probe("nrp", irq_work_entry, handle_vector_irq_entry);
+ if (IS_ENABLED(CONFIG_SMP)) {
+ rv_attach_trace_probe("nrp", reschedule_entry, handle_vector_irq_entry);
+ rv_attach_trace_probe("nrp", call_function_entry, handle_vector_irq_entry);
+ rv_attach_trace_probe("nrp", call_function_single_entry, handle_vector_irq_entry);
+ }
+}
+
+static void detach_vector_irq(void)
+{
+ rv_detach_trace_probe("nrp", local_timer_entry, handle_vector_irq_entry);
+ if (IS_ENABLED(CONFIG_IRQ_WORK))
+ rv_detach_trace_probe("nrp", irq_work_entry, handle_vector_irq_entry);
+ if (IS_ENABLED(CONFIG_SMP)) {
+ rv_detach_trace_probe("nrp", reschedule_entry, handle_vector_irq_entry);
+ rv_detach_trace_probe("nrp", call_function_entry, handle_vector_irq_entry);
+ rv_detach_trace_probe("nrp", call_function_single_entry, handle_vector_irq_entry);
+ }
+}
+
+#else
+/* We assume irq_entry tracepoints are sufficient on other architectures */
+static void attach_vector_irq(void) { }
+static void detach_vector_irq(void) { }
+#endif
+
+static void handle_irq_entry(void *data, int irq, struct irqaction *action)
+{
+ da_handle_event_nrp(current, irq_entry_nrp);
+}
+
+static void handle_sched_need_resched(void *data, struct task_struct *tsk,
+ int cpu, int tif)
+{
+ /*
+ * Although need_resched leads to both the rescheduling and preempt_irq
+ * states, it is safer to start the monitor always in preempt_irq,
+ * which may not mirror the system state but makes the monitor simpler,
+ */
+ if (tif == TIF_NEED_RESCHED)
+ da_handle_start_event_nrp(tsk, sched_need_resched_nrp);
+}
+
+static void handle_schedule_entry(void *data, bool preempt)
+{
+ if (preempt)
+ da_handle_event_nrp(current, schedule_entry_preempt_nrp);
+ else
+ da_handle_event_nrp(current, schedule_entry_nrp);
+}
+
+static int enable_nrp(void)
+{
+ int retval;
+
+ retval = da_monitor_init_nrp();
+ if (retval)
+ return retval;
+
+ rv_attach_trace_probe("nrp", irq_handler_entry, handle_irq_entry);
+ rv_attach_trace_probe("nrp", sched_set_need_resched_tp, handle_sched_need_resched);
+ rv_attach_trace_probe("nrp", sched_entry_tp, handle_schedule_entry);
+ attach_vector_irq();
+
+ return 0;
+}
+
+static void disable_nrp(void)
+{
+ rv_nrp.enabled = 0;
+
+ rv_detach_trace_probe("nrp", irq_handler_entry, handle_irq_entry);
+ rv_detach_trace_probe("nrp", sched_set_need_resched_tp, handle_sched_need_resched);
+ rv_detach_trace_probe("nrp", sched_entry_tp, handle_schedule_entry);
+ detach_vector_irq();
+
+ da_monitor_destroy_nrp();
+}
+
+static struct rv_monitor rv_nrp = {
+ .name = "nrp",
+ .description = "need resched preempts.",
+ .enable = enable_nrp,
+ .disable = disable_nrp,
+ .reset = da_monitor_reset_all_nrp,
+ .enabled = 0,
+};
+
+static int __init register_nrp(void)
+{
+ return rv_register_monitor(&rv_nrp, &rv_sched);
+}
+
+static void __exit unregister_nrp(void)
+{
+ rv_unregister_monitor(&rv_nrp);
+}
+
+module_init(register_nrp);
+module_exit(unregister_nrp);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Gabriele Monaco <gmonaco@redhat.com>");
+MODULE_DESCRIPTION("nrp: need resched preempts.");
diff --git a/kernel/trace/rv/monitors/nrp/nrp.h b/kernel/trace/rv/monitors/nrp/nrp.h
new file mode 100644
index 000000000000..c9f12207cbf6
--- /dev/null
+++ b/kernel/trace/rv/monitors/nrp/nrp.h
@@ -0,0 +1,75 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Automatically generated C representation of nrp automaton
+ * For further information about this format, see kernel documentation:
+ * Documentation/trace/rv/deterministic_automata.rst
+ */
+
+enum states_nrp {
+ preempt_irq_nrp = 0,
+ any_thread_running_nrp,
+ nested_preempt_nrp,
+ rescheduling_nrp,
+ state_max_nrp
+};
+
+#define INVALID_STATE state_max_nrp
+
+enum events_nrp {
+ irq_entry_nrp = 0,
+ sched_need_resched_nrp,
+ schedule_entry_nrp,
+ schedule_entry_preempt_nrp,
+ event_max_nrp
+};
+
+struct automaton_nrp {
+ char *state_names[state_max_nrp];
+ char *event_names[event_max_nrp];
+ unsigned char function[state_max_nrp][event_max_nrp];
+ unsigned char initial_state;
+ bool final_states[state_max_nrp];
+};
+
+static const struct automaton_nrp automaton_nrp = {
+ .state_names = {
+ "preempt_irq",
+ "any_thread_running",
+ "nested_preempt",
+ "rescheduling"
+ },
+ .event_names = {
+ "irq_entry",
+ "sched_need_resched",
+ "schedule_entry",
+ "schedule_entry_preempt"
+ },
+ .function = {
+ {
+ preempt_irq_nrp,
+ preempt_irq_nrp,
+ nested_preempt_nrp,
+ nested_preempt_nrp
+ },
+ {
+ any_thread_running_nrp,
+ rescheduling_nrp,
+ any_thread_running_nrp,
+ INVALID_STATE
+ },
+ {
+ nested_preempt_nrp,
+ preempt_irq_nrp,
+ any_thread_running_nrp,
+ any_thread_running_nrp
+ },
+ {
+ preempt_irq_nrp,
+ rescheduling_nrp,
+ any_thread_running_nrp,
+ any_thread_running_nrp
+ },
+ },
+ .initial_state = preempt_irq_nrp,
+ .final_states = { 0, 1, 0, 0 },
+};
diff --git a/kernel/trace/rv/monitors/nrp/nrp_trace.h b/kernel/trace/rv/monitors/nrp/nrp_trace.h
new file mode 100644
index 000000000000..2e13497de3b6
--- /dev/null
+++ b/kernel/trace/rv/monitors/nrp/nrp_trace.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Snippet to be included in rv_trace.h
+ */
+
+#ifdef CONFIG_RV_MON_NRP
+DEFINE_EVENT(event_da_monitor_id, event_nrp,
+ TP_PROTO(int id, char *state, char *event, char *next_state, bool final_state),
+ TP_ARGS(id, state, event, next_state, final_state));
+
+DEFINE_EVENT(error_da_monitor_id, error_nrp,
+ TP_PROTO(int id, char *state, char *event),
+ TP_ARGS(id, state, event));
+#endif /* CONFIG_RV_MON_NRP */
diff --git a/kernel/trace/rv/monitors/sched/Kconfig b/kernel/trace/rv/monitors/sched/Kconfig
index ae3eb410abd7..aa16456da864 100644
--- a/kernel/trace/rv/monitors/sched/Kconfig
+++ b/kernel/trace/rv/monitors/sched/Kconfig
@@ -2,6 +2,7 @@
#
config RV_MON_SCHED
depends on RV
+ depends on RV_PER_TASK_MONITORS >= 3
bool "sched monitor"
help
Collection of monitors to check the scheduler behaves according to specifications.
diff --git a/kernel/trace/rv/monitors/sssw/Kconfig b/kernel/trace/rv/monitors/sssw/Kconfig
new file mode 100644
index 000000000000..23b7eeb38bbf
--- /dev/null
+++ b/kernel/trace/rv/monitors/sssw/Kconfig
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+config RV_MON_SSSW
+ depends on RV
+ depends on RV_MON_SCHED
+ default y
+ select DA_MON_EVENTS_ID
+ bool "sssw monitor"
+ help
+ Monitor to ensure sched_set_state to sleepable leads to sleeping and
+ sleeping tasks require wakeup.
+ This monitor is part of the sched monitors collection.
+
+ For further information, see:
+ Documentation/trace/rv/monitor_sched.rst
diff --git a/kernel/trace/rv/monitors/sssw/sssw.c b/kernel/trace/rv/monitors/sssw/sssw.c
new file mode 100644
index 000000000000..84b8d890d9d4
--- /dev/null
+++ b/kernel/trace/rv/monitors/sssw/sssw.c
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/ftrace.h>
+#include <linux/tracepoint.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/rv.h>
+#include <rv/instrumentation.h>
+#include <rv/da_monitor.h>
+
+#define MODULE_NAME "sssw"
+
+#include <trace/events/sched.h>
+#include <trace/events/signal.h>
+#include <rv_trace.h>
+#include <monitors/sched/sched.h>
+
+#include "sssw.h"
+
+static struct rv_monitor rv_sssw;
+DECLARE_DA_MON_PER_TASK(sssw, unsigned char);
+
+static void handle_sched_set_state(void *data, struct task_struct *tsk, int state)
+{
+ if (state == TASK_RUNNING)
+ da_handle_start_event_sssw(tsk, sched_set_state_runnable_sssw);
+ else
+ da_handle_event_sssw(tsk, sched_set_state_sleepable_sssw);
+}
+
+static void handle_sched_switch(void *data, bool preempt,
+ struct task_struct *prev,
+ struct task_struct *next,
+ unsigned int prev_state)
+{
+ if (preempt)
+ da_handle_event_sssw(prev, sched_switch_preempt_sssw);
+ else if (prev_state == TASK_RUNNING)
+ da_handle_event_sssw(prev, sched_switch_yield_sssw);
+ else if (prev_state == TASK_RTLOCK_WAIT)
+ /* special case of sleeping task with racy conditions */
+ da_handle_event_sssw(prev, sched_switch_blocking_sssw);
+ else
+ da_handle_event_sssw(prev, sched_switch_suspend_sssw);
+ da_handle_event_sssw(next, sched_switch_in_sssw);
+}
+
+static void handle_sched_wakeup(void *data, struct task_struct *p)
+{
+ /*
+ * Wakeup can also lead to signal_wakeup although the system is
+ * actually runnable. The monitor can safely start with this event.
+ */
+ da_handle_start_event_sssw(p, sched_wakeup_sssw);
+}
+
+static void handle_signal_deliver(void *data, int sig,
+ struct kernel_siginfo *info,
+ struct k_sigaction *ka)
+{
+ da_handle_event_sssw(current, signal_deliver_sssw);
+}
+
+static int enable_sssw(void)
+{
+ int retval;
+
+ retval = da_monitor_init_sssw();
+ if (retval)
+ return retval;
+
+ rv_attach_trace_probe("sssw", sched_set_state_tp, handle_sched_set_state);
+ rv_attach_trace_probe("sssw", sched_switch, handle_sched_switch);
+ rv_attach_trace_probe("sssw", sched_wakeup, handle_sched_wakeup);
+ rv_attach_trace_probe("sssw", signal_deliver, handle_signal_deliver);
+
+ return 0;
+}
+
+static void disable_sssw(void)
+{
+ rv_sssw.enabled = 0;
+
+ rv_detach_trace_probe("sssw", sched_set_state_tp, handle_sched_set_state);
+ rv_detach_trace_probe("sssw", sched_switch, handle_sched_switch);
+ rv_detach_trace_probe("sssw", sched_wakeup, handle_sched_wakeup);
+ rv_detach_trace_probe("sssw", signal_deliver, handle_signal_deliver);
+
+ da_monitor_destroy_sssw();
+}
+
+static struct rv_monitor rv_sssw = {
+ .name = "sssw",
+ .description = "set state sleep and wakeup.",
+ .enable = enable_sssw,
+ .disable = disable_sssw,
+ .reset = da_monitor_reset_all_sssw,
+ .enabled = 0,
+};
+
+static int __init register_sssw(void)
+{
+ return rv_register_monitor(&rv_sssw, &rv_sched);
+}
+
+static void __exit unregister_sssw(void)
+{
+ rv_unregister_monitor(&rv_sssw);
+}
+
+module_init(register_sssw);
+module_exit(unregister_sssw);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Gabriele Monaco <gmonaco@redhat.com>");
+MODULE_DESCRIPTION("sssw: set state sleep and wakeup.");
diff --git a/kernel/trace/rv/monitors/sssw/sssw.h b/kernel/trace/rv/monitors/sssw/sssw.h
new file mode 100644
index 000000000000..243d54050c94
--- /dev/null
+++ b/kernel/trace/rv/monitors/sssw/sssw.h
@@ -0,0 +1,105 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Automatically generated C representation of sssw automaton
+ * For further information about this format, see kernel documentation:
+ * Documentation/trace/rv/deterministic_automata.rst
+ */
+
+enum states_sssw {
+ runnable_sssw = 0,
+ signal_wakeup_sssw,
+ sleepable_sssw,
+ sleeping_sssw,
+ state_max_sssw
+};
+
+#define INVALID_STATE state_max_sssw
+
+enum events_sssw {
+ sched_set_state_runnable_sssw = 0,
+ sched_set_state_sleepable_sssw,
+ sched_switch_blocking_sssw,
+ sched_switch_in_sssw,
+ sched_switch_preempt_sssw,
+ sched_switch_suspend_sssw,
+ sched_switch_yield_sssw,
+ sched_wakeup_sssw,
+ signal_deliver_sssw,
+ event_max_sssw
+};
+
+struct automaton_sssw {
+ char *state_names[state_max_sssw];
+ char *event_names[event_max_sssw];
+ unsigned char function[state_max_sssw][event_max_sssw];
+ unsigned char initial_state;
+ bool final_states[state_max_sssw];
+};
+
+static const struct automaton_sssw automaton_sssw = {
+ .state_names = {
+ "runnable",
+ "signal_wakeup",
+ "sleepable",
+ "sleeping"
+ },
+ .event_names = {
+ "sched_set_state_runnable",
+ "sched_set_state_sleepable",
+ "sched_switch_blocking",
+ "sched_switch_in",
+ "sched_switch_preempt",
+ "sched_switch_suspend",
+ "sched_switch_yield",
+ "sched_wakeup",
+ "signal_deliver"
+ },
+ .function = {
+ {
+ runnable_sssw,
+ sleepable_sssw,
+ sleeping_sssw,
+ runnable_sssw,
+ runnable_sssw,
+ INVALID_STATE,
+ runnable_sssw,
+ runnable_sssw,
+ runnable_sssw
+ },
+ {
+ INVALID_STATE,
+ sleepable_sssw,
+ INVALID_STATE,
+ signal_wakeup_sssw,
+ signal_wakeup_sssw,
+ INVALID_STATE,
+ signal_wakeup_sssw,
+ signal_wakeup_sssw,
+ runnable_sssw
+ },
+ {
+ runnable_sssw,
+ sleepable_sssw,
+ sleeping_sssw,
+ sleepable_sssw,
+ sleepable_sssw,
+ sleeping_sssw,
+ signal_wakeup_sssw,
+ runnable_sssw,
+ sleepable_sssw
+ },
+ {
+ INVALID_STATE,
+ INVALID_STATE,
+ INVALID_STATE,
+ INVALID_STATE,
+ INVALID_STATE,
+ INVALID_STATE,
+ INVALID_STATE,
+ runnable_sssw,
+ INVALID_STATE
+ },
+ },
+ .initial_state = runnable_sssw,
+ .final_states = { 1, 0, 0, 0 },
+};
diff --git a/kernel/trace/rv/monitors/sssw/sssw_trace.h b/kernel/trace/rv/monitors/sssw/sssw_trace.h
new file mode 100644
index 000000000000..6c03cfc6960b
--- /dev/null
+++ b/kernel/trace/rv/monitors/sssw/sssw_trace.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Snippet to be included in rv_trace.h
+ */
+
+#ifdef CONFIG_RV_MON_SSSW
+DEFINE_EVENT(event_da_monitor_id, event_sssw,
+ TP_PROTO(int id, char *state, char *event, char *next_state, bool final_state),
+ TP_ARGS(id, state, event, next_state, final_state));
+
+DEFINE_EVENT(error_da_monitor_id, error_sssw,
+ TP_PROTO(int id, char *state, char *event),
+ TP_ARGS(id, state, event));
+#endif /* CONFIG_RV_MON_SSSW */
diff --git a/kernel/trace/rv/rv_trace.h b/kernel/trace/rv/rv_trace.h
index 97b2f7e07f27..e4e78d23b7b0 100644
--- a/kernel/trace/rv/rv_trace.h
+++ b/kernel/trace/rv/rv_trace.h
@@ -123,6 +123,8 @@ DECLARE_EVENT_CLASS(error_da_monitor_id,
#include <monitors/wwnr/wwnr_trace.h>
#include <monitors/snroc/snroc_trace.h>
+#include <monitors/nrp/nrp_trace.h>
+#include <monitors/sssw/sssw_trace.h>
// Add new monitors based on CONFIG_DA_MON_EVENTS_ID here
#endif /* CONFIG_DA_MON_EVENTS_ID */
diff --git a/tools/verification/models/sched/nrp.dot b/tools/verification/models/sched/nrp.dot
new file mode 100644
index 000000000000..77bb64669416
--- /dev/null
+++ b/tools/verification/models/sched/nrp.dot
@@ -0,0 +1,29 @@
+digraph state_automaton {
+ center = true;
+ size = "7,11";
+ {node [shape = doublecircle] "any_thread_running"};
+ {node [shape = circle] "any_thread_running"};
+ {node [shape = circle] "nested_preempt"};
+ {node [shape = plaintext, style=invis, label=""] "__init_preempt_irq"};
+ {node [shape = circle] "preempt_irq"};
+ {node [shape = circle] "rescheduling"};
+ "__init_preempt_irq" -> "preempt_irq";
+ "any_thread_running" [label = "any_thread_running", color = green3];
+ "any_thread_running" -> "any_thread_running" [ label = "schedule_entry\nirq_entry" ];
+ "any_thread_running" -> "rescheduling" [ label = "sched_need_resched" ];
+ "nested_preempt" [label = "nested_preempt"];
+ "nested_preempt" -> "any_thread_running" [ label = "schedule_entry_preempt\nschedule_entry" ];
+ "nested_preempt" -> "nested_preempt" [ label = "irq_entry" ];
+ "nested_preempt" -> "preempt_irq" [ label = "sched_need_resched" ];
+ "preempt_irq" [label = "preempt_irq"];
+ "preempt_irq" -> "nested_preempt" [ label = "schedule_entry_preempt\nschedule_entry" ];
+ "preempt_irq" -> "preempt_irq" [ label = "irq_entry\nsched_need_resched" ];
+ "rescheduling" [label = "rescheduling"];
+ "rescheduling" -> "any_thread_running" [ label = "schedule_entry_preempt\nschedule_entry" ];
+ "rescheduling" -> "preempt_irq" [ label = "irq_entry" ];
+ "rescheduling" -> "rescheduling" [ label = "sched_need_resched" ];
+ { rank = min ;
+ "__init_preempt_irq";
+ "preempt_irq";
+ }
+}
diff --git a/tools/verification/models/sched/sssw.dot b/tools/verification/models/sched/sssw.dot
new file mode 100644
index 000000000000..4994c3e876be
--- /dev/null
+++ b/tools/verification/models/sched/sssw.dot
@@ -0,0 +1,30 @@
+digraph state_automaton {
+ center = true;
+ size = "7,11";
+ {node [shape = plaintext, style=invis, label=""] "__init_runnable"};
+ {node [shape = doublecircle] "runnable"};
+ {node [shape = circle] "runnable"};
+ {node [shape = circle] "signal_wakeup"};
+ {node [shape = circle] "sleepable"};
+ {node [shape = circle] "sleeping"};
+ "__init_runnable" -> "runnable";
+ "runnable" [label = "runnable", color = green3];
+ "runnable" -> "runnable" [ label = "sched_set_state_runnable\nsched_wakeup\nsched_switch_in\nsched_switch_yield\nsched_switch_preempt\nsignal_deliver" ];
+ "runnable" -> "sleepable" [ label = "sched_set_state_sleepable" ];
+ "runnable" -> "sleeping" [ label = "sched_switch_blocking" ];
+ "signal_wakeup" [label = "signal_wakeup"];
+ "signal_wakeup" -> "runnable" [ label = "signal_deliver" ];
+ "signal_wakeup" -> "signal_wakeup" [ label = "sched_switch_in\nsched_switch_preempt\nsched_switch_yield\nsched_wakeup" ];
+ "signal_wakeup" -> "sleepable" [ label = "sched_set_state_sleepable" ];
+ "sleepable" [label = "sleepable"];
+ "sleepable" -> "runnable" [ label = "sched_set_state_runnable\nsched_wakeup" ];
+ "sleepable" -> "signal_wakeup" [ label = "sched_switch_yield" ];
+ "sleepable" -> "sleepable" [ label = "sched_set_state_sleepable\nsched_switch_in\nsched_switch_preempt\nsignal_deliver" ];
+ "sleepable" -> "sleeping" [ label = "sched_switch_suspend\nsched_switch_blocking" ];
+ "sleeping" [label = "sleeping"];
+ "sleeping" -> "runnable" [ label = "sched_wakeup" ];
+ { rank = min ;
+ "__init_runnable";
+ "runnable";
+ }
+}
--
2.50.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v5 8/9] rv: Add nrp and sssw per-task monitors
2025-07-28 13:50 ` [PATCH v5 8/9] rv: Add nrp and sssw per-task monitors Gabriele Monaco
@ 2025-07-28 15:56 ` Nam Cao
0 siblings, 0 replies; 15+ messages in thread
From: Nam Cao @ 2025-07-28 15:56 UTC (permalink / raw)
To: Gabriele Monaco
Cc: linux-kernel, Steven Rostedt, Jonathan Corbet, Masami Hiramatsu,
linux-trace-kernel, linux-doc, Ingo Molnar, Peter Zijlstra,
Tomas Glozar, Juri Lelli, Clark Williams, John Kacur
On Mon, Jul 28, 2025 at 03:50:20PM +0200, Gabriele Monaco wrote:
> Add 2 per-task monitors as part of the sched model:
>
> * nrp: need-resched preempts
> Monitor to ensure preemption requires need resched.
> * sssw: set state sleep and wakeup
> Monitor to ensure sched_set_state to sleepable leads to sleeping and
> sleeping tasks require wakeup.
>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
I can't comment on the model, but the monitor implementation looks okay. I
ran the two monitors on x86, powerpc64, riscv64 and didn't see anything
suspicious:
Acked-by: Nam Cao <namcao@linutronix.de>
Tested-by: Nam Cao <namcao@linutronix.de>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v5 9/9] rv: Add opid per-cpu monitor
[not found] <20250728135022.255578-1-gmonaco@redhat.com>
2025-07-28 13:50 ` [PATCH v5 7/9] rv: Replace tss and sncid monitors with more complete sts Gabriele Monaco
2025-07-28 13:50 ` [PATCH v5 8/9] rv: Add nrp and sssw per-task monitors Gabriele Monaco
@ 2025-07-28 13:50 ` Gabriele Monaco
2025-07-28 15:58 ` Nam Cao
2 siblings, 1 reply; 15+ messages in thread
From: Gabriele Monaco @ 2025-07-28 13:50 UTC (permalink / raw)
To: linux-kernel, Steven Rostedt, Jonathan Corbet, Masami Hiramatsu,
linux-trace-kernel, linux-doc
Cc: Gabriele Monaco, Ingo Molnar, Peter Zijlstra, Nam Cao,
Tomas Glozar, Juri Lelli, Clark Williams, John Kacur
Add a per-cpu monitor as part of the sched model:
* opid: operations with preemption and irq disabled
Monitor to ensure wakeup and need_resched occur with irq and
preemption disabled or in irq handlers.
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
Documentation/trace/rv/monitor_sched.rst | 55 +++++++
kernel/trace/rv/Kconfig | 1 +
kernel/trace/rv/Makefile | 1 +
kernel/trace/rv/monitors/opid/Kconfig | 19 +++
kernel/trace/rv/monitors/opid/opid.c | 168 +++++++++++++++++++++
kernel/trace/rv/monitors/opid/opid.h | 104 +++++++++++++
kernel/trace/rv/monitors/opid/opid_trace.h | 15 ++
kernel/trace/rv/rv_trace.h | 1 +
tools/verification/models/sched/opid.dot | 35 +++++
9 files changed, 399 insertions(+)
create mode 100644 kernel/trace/rv/monitors/opid/Kconfig
create mode 100644 kernel/trace/rv/monitors/opid/opid.c
create mode 100644 kernel/trace/rv/monitors/opid/opid.h
create mode 100644 kernel/trace/rv/monitors/opid/opid_trace.h
create mode 100644 tools/verification/models/sched/opid.dot
diff --git a/Documentation/trace/rv/monitor_sched.rst b/Documentation/trace/rv/monitor_sched.rst
index 11ef963cb578..3f8381ad9ec7 100644
--- a/Documentation/trace/rv/monitor_sched.rst
+++ b/Documentation/trace/rv/monitor_sched.rst
@@ -341,6 +341,61 @@ can be triggered also by preemption, but cannot occur after the task got to
| | switch_yield
+-----------+ wakeup
+Monitor opid
+------------
+
+The operations with preemption and irq disabled (opid) monitor ensures
+operations like ``wakeup`` and ``need_resched`` occur with interrupts and
+preemption disabled or during interrupt context, in such case preemption may
+not be disabled explicitly.
+``need_resched`` can be set by some RCU internals functions, in which case it
+doesn't match a task wakeup and might occur with only interrupts disabled::
+
+ | sched_need_resched
+ | sched_waking
+ | irq_entry
+ | +--------------------+
+ v v |
+ +------------------------------------------------------+
+ +----------- | disabled | <+
+ | +------------------------------------------------------+ |
+ | | ^ |
+ | | preempt_disable sched_need_resched |
+ | preempt_enable | +--------------------+ |
+ | v | v | |
+ | +------------------------------------------------------+ |
+ | | irq_disabled | |
+ | +------------------------------------------------------+ |
+ | | | ^ |
+ | irq_entry irq_entry | | |
+ | sched_need_resched v | irq_disable |
+ | sched_waking +--------------+ | | |
+ | +----- | | irq_enable | |
+ | | | in_irq | | | |
+ | +----> | | | | |
+ | +--------------+ | | irq_disable
+ | | | | |
+ | irq_enable | irq_enable | | |
+ | v v | |
+ | #======================================================# |
+ | H enabled H |
+ | #======================================================# |
+ | | ^ ^ preempt_enable | |
+ | preempt_disable preempt_enable +--------------------+ |
+ | v | |
+ | +------------------+ | |
+ +----------> | preempt_disabled | -+ |
+ +------------------+ |
+ | |
+ +-------------------------------------------------------+
+
+This monitor is designed to work on ``PREEMPT_RT`` kernels, the special case of
+events occurring in interrupt context is a shortcut to identify valid scenarios
+where the preemption tracepoints might not be visible, during interrupts
+preemption is always disabled. On non- ``PREEMPT_RT`` kernels, the interrupts
+might invoke a softirq to set ``need_resched`` and wake up a task. This is
+another special case that is currently not supported by the monitor.
+
References
----------
diff --git a/kernel/trace/rv/Kconfig b/kernel/trace/rv/Kconfig
index 59d0db898d4a..5b4be87ba59d 100644
--- a/kernel/trace/rv/Kconfig
+++ b/kernel/trace/rv/Kconfig
@@ -57,6 +57,7 @@ source "kernel/trace/rv/monitors/snep/Kconfig"
source "kernel/trace/rv/monitors/sts/Kconfig"
source "kernel/trace/rv/monitors/nrp/Kconfig"
source "kernel/trace/rv/monitors/sssw/Kconfig"
+source "kernel/trace/rv/monitors/opid/Kconfig"
# Add new sched monitors here
source "kernel/trace/rv/monitors/rtapp/Kconfig"
diff --git a/kernel/trace/rv/Makefile b/kernel/trace/rv/Makefile
index 2afac88539d3..750e4ad6fa0f 100644
--- a/kernel/trace/rv/Makefile
+++ b/kernel/trace/rv/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_RV_MON_SLEEP) += monitors/sleep/sleep.o
obj-$(CONFIG_RV_MON_STS) += monitors/sts/sts.o
obj-$(CONFIG_RV_MON_NRP) += monitors/nrp/nrp.o
obj-$(CONFIG_RV_MON_SSSW) += monitors/sssw/sssw.o
+obj-$(CONFIG_RV_MON_OPID) += monitors/opid/opid.o
# Add new monitors here
obj-$(CONFIG_RV_REACTORS) += rv_reactors.o
obj-$(CONFIG_RV_REACT_PRINTK) += reactor_printk.o
diff --git a/kernel/trace/rv/monitors/opid/Kconfig b/kernel/trace/rv/monitors/opid/Kconfig
new file mode 100644
index 000000000000..561d32da572b
--- /dev/null
+++ b/kernel/trace/rv/monitors/opid/Kconfig
@@ -0,0 +1,19 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+config RV_MON_OPID
+ depends on RV
+ depends on TRACE_IRQFLAGS
+ depends on TRACE_PREEMPT_TOGGLE
+ depends on RV_MON_SCHED
+ default y if PREEMPT_RT
+ select DA_MON_EVENTS_IMPLICIT
+ bool "opid monitor"
+ help
+ Monitor to ensure operations like wakeup and need resched occur with
+ interrupts and preemption disabled or during IRQs, where preemption
+ may not be disabled explicitly.
+
+ This monitor is unstable on !PREEMPT_RT, say N unless you are testing it.
+
+ For further information, see:
+ Documentation/trace/rv/monitor_sched.rst
diff --git a/kernel/trace/rv/monitors/opid/opid.c b/kernel/trace/rv/monitors/opid/opid.c
new file mode 100644
index 000000000000..50d64e7fb8c4
--- /dev/null
+++ b/kernel/trace/rv/monitors/opid/opid.c
@@ -0,0 +1,168 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/ftrace.h>
+#include <linux/tracepoint.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/rv.h>
+#include <rv/instrumentation.h>
+#include <rv/da_monitor.h>
+
+#define MODULE_NAME "opid"
+
+#include <trace/events/sched.h>
+#include <trace/events/irq.h>
+#include <trace/events/preemptirq.h>
+#include <rv_trace.h>
+#include <monitors/sched/sched.h>
+
+#include "opid.h"
+
+static struct rv_monitor rv_opid;
+DECLARE_DA_MON_PER_CPU(opid, unsigned char);
+
+#ifdef CONFIG_X86_LOCAL_APIC
+#include <asm/trace/irq_vectors.h>
+
+static void handle_vector_irq_entry(void *data, int vector)
+{
+ da_handle_event_opid(irq_entry_opid);
+}
+
+static void attach_vector_irq(void)
+{
+ rv_attach_trace_probe("opid", local_timer_entry, handle_vector_irq_entry);
+ if (IS_ENABLED(CONFIG_IRQ_WORK))
+ rv_attach_trace_probe("opid", irq_work_entry, handle_vector_irq_entry);
+ if (IS_ENABLED(CONFIG_SMP)) {
+ rv_attach_trace_probe("opid", reschedule_entry, handle_vector_irq_entry);
+ rv_attach_trace_probe("opid", call_function_entry, handle_vector_irq_entry);
+ rv_attach_trace_probe("opid", call_function_single_entry, handle_vector_irq_entry);
+ }
+}
+
+static void detach_vector_irq(void)
+{
+ rv_detach_trace_probe("opid", local_timer_entry, handle_vector_irq_entry);
+ if (IS_ENABLED(CONFIG_IRQ_WORK))
+ rv_detach_trace_probe("opid", irq_work_entry, handle_vector_irq_entry);
+ if (IS_ENABLED(CONFIG_SMP)) {
+ rv_detach_trace_probe("opid", reschedule_entry, handle_vector_irq_entry);
+ rv_detach_trace_probe("opid", call_function_entry, handle_vector_irq_entry);
+ rv_detach_trace_probe("opid", call_function_single_entry, handle_vector_irq_entry);
+ }
+}
+
+#else
+/* We assume irq_entry tracepoints are sufficient on other architectures */
+static void attach_vector_irq(void) { }
+static void detach_vector_irq(void) { }
+#endif
+
+static void handle_irq_disable(void *data, unsigned long ip, unsigned long parent_ip)
+{
+ da_handle_event_opid(irq_disable_opid);
+}
+
+static void handle_irq_enable(void *data, unsigned long ip, unsigned long parent_ip)
+{
+ da_handle_event_opid(irq_enable_opid);
+}
+
+static void handle_irq_entry(void *data, int irq, struct irqaction *action)
+{
+ da_handle_event_opid(irq_entry_opid);
+}
+
+static void handle_preempt_disable(void *data, unsigned long ip, unsigned long parent_ip)
+{
+ da_handle_event_opid(preempt_disable_opid);
+}
+
+static void handle_preempt_enable(void *data, unsigned long ip, unsigned long parent_ip)
+{
+ da_handle_event_opid(preempt_enable_opid);
+}
+
+static void handle_sched_need_resched(void *data, struct task_struct *tsk, int cpu, int tif)
+{
+ /* The monitor's intitial state is not in_irq */
+ if (this_cpu_read(hardirq_context))
+ da_handle_event_opid(sched_need_resched_opid);
+ else
+ da_handle_start_event_opid(sched_need_resched_opid);
+}
+
+static void handle_sched_waking(void *data, struct task_struct *p)
+{
+ /* The monitor's intitial state is not in_irq */
+ if (this_cpu_read(hardirq_context))
+ da_handle_event_opid(sched_waking_opid);
+ else
+ da_handle_start_event_opid(sched_waking_opid);
+}
+
+static int enable_opid(void)
+{
+ int retval;
+
+ retval = da_monitor_init_opid();
+ if (retval)
+ return retval;
+
+ rv_attach_trace_probe("opid", irq_disable, handle_irq_disable);
+ rv_attach_trace_probe("opid", irq_enable, handle_irq_enable);
+ rv_attach_trace_probe("opid", irq_handler_entry, handle_irq_entry);
+ rv_attach_trace_probe("opid", preempt_disable, handle_preempt_disable);
+ rv_attach_trace_probe("opid", preempt_enable, handle_preempt_enable);
+ rv_attach_trace_probe("opid", sched_set_need_resched_tp, handle_sched_need_resched);
+ rv_attach_trace_probe("opid", sched_waking, handle_sched_waking);
+ attach_vector_irq();
+
+ return 0;
+}
+
+static void disable_opid(void)
+{
+ rv_opid.enabled = 0;
+
+ rv_detach_trace_probe("opid", irq_disable, handle_irq_disable);
+ rv_detach_trace_probe("opid", irq_enable, handle_irq_enable);
+ rv_detach_trace_probe("opid", irq_handler_entry, handle_irq_entry);
+ rv_detach_trace_probe("opid", preempt_disable, handle_preempt_disable);
+ rv_detach_trace_probe("opid", preempt_enable, handle_preempt_enable);
+ rv_detach_trace_probe("opid", sched_set_need_resched_tp, handle_sched_need_resched);
+ rv_detach_trace_probe("opid", sched_waking, handle_sched_waking);
+ detach_vector_irq();
+
+ da_monitor_destroy_opid();
+}
+
+/*
+ * This is the monitor register section.
+ */
+static struct rv_monitor rv_opid = {
+ .name = "opid",
+ .description = "operations with preemption and irq disabled.",
+ .enable = enable_opid,
+ .disable = disable_opid,
+ .reset = da_monitor_reset_all_opid,
+ .enabled = 0,
+};
+
+static int __init register_opid(void)
+{
+ return rv_register_monitor(&rv_opid, &rv_sched);
+}
+
+static void __exit unregister_opid(void)
+{
+ rv_unregister_monitor(&rv_opid);
+}
+
+module_init(register_opid);
+module_exit(unregister_opid);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Gabriele Monaco <gmonaco@redhat.com>");
+MODULE_DESCRIPTION("opid: operations with preemption and irq disabled.");
diff --git a/kernel/trace/rv/monitors/opid/opid.h b/kernel/trace/rv/monitors/opid/opid.h
new file mode 100644
index 000000000000..b4b8c2ff7f64
--- /dev/null
+++ b/kernel/trace/rv/monitors/opid/opid.h
@@ -0,0 +1,104 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Automatically generated C representation of opid automaton
+ * For further information about this format, see kernel documentation:
+ * Documentation/trace/rv/deterministic_automata.rst
+ */
+
+enum states_opid {
+ disabled_opid = 0,
+ enabled_opid,
+ in_irq_opid,
+ irq_disabled_opid,
+ preempt_disabled_opid,
+ state_max_opid
+};
+
+#define INVALID_STATE state_max_opid
+
+enum events_opid {
+ irq_disable_opid = 0,
+ irq_enable_opid,
+ irq_entry_opid,
+ preempt_disable_opid,
+ preempt_enable_opid,
+ sched_need_resched_opid,
+ sched_waking_opid,
+ event_max_opid
+};
+
+struct automaton_opid {
+ char *state_names[state_max_opid];
+ char *event_names[event_max_opid];
+ unsigned char function[state_max_opid][event_max_opid];
+ unsigned char initial_state;
+ bool final_states[state_max_opid];
+};
+
+static const struct automaton_opid automaton_opid = {
+ .state_names = {
+ "disabled",
+ "enabled",
+ "in_irq",
+ "irq_disabled",
+ "preempt_disabled"
+ },
+ .event_names = {
+ "irq_disable",
+ "irq_enable",
+ "irq_entry",
+ "preempt_disable",
+ "preempt_enable",
+ "sched_need_resched",
+ "sched_waking"
+ },
+ .function = {
+ {
+ INVALID_STATE,
+ preempt_disabled_opid,
+ disabled_opid,
+ INVALID_STATE,
+ irq_disabled_opid,
+ disabled_opid,
+ disabled_opid
+ },
+ {
+ irq_disabled_opid,
+ INVALID_STATE,
+ INVALID_STATE,
+ preempt_disabled_opid,
+ enabled_opid,
+ INVALID_STATE,
+ INVALID_STATE
+ },
+ {
+ INVALID_STATE,
+ enabled_opid,
+ in_irq_opid,
+ INVALID_STATE,
+ INVALID_STATE,
+ in_irq_opid,
+ in_irq_opid
+ },
+ {
+ INVALID_STATE,
+ enabled_opid,
+ in_irq_opid,
+ disabled_opid,
+ INVALID_STATE,
+ irq_disabled_opid,
+ INVALID_STATE
+ },
+ {
+ disabled_opid,
+ INVALID_STATE,
+ INVALID_STATE,
+ INVALID_STATE,
+ enabled_opid,
+ INVALID_STATE,
+ INVALID_STATE
+ },
+ },
+ .initial_state = disabled_opid,
+ .final_states = { 0, 1, 0, 0, 0 },
+};
diff --git a/kernel/trace/rv/monitors/opid/opid_trace.h b/kernel/trace/rv/monitors/opid/opid_trace.h
new file mode 100644
index 000000000000..3df6ff955c30
--- /dev/null
+++ b/kernel/trace/rv/monitors/opid/opid_trace.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Snippet to be included in rv_trace.h
+ */
+
+#ifdef CONFIG_RV_MON_OPID
+DEFINE_EVENT(event_da_monitor, event_opid,
+ TP_PROTO(char *state, char *event, char *next_state, bool final_state),
+ TP_ARGS(state, event, next_state, final_state));
+
+DEFINE_EVENT(error_da_monitor, error_opid,
+ TP_PROTO(char *state, char *event),
+ TP_ARGS(state, event));
+#endif /* CONFIG_RV_MON_OPID */
diff --git a/kernel/trace/rv/rv_trace.h b/kernel/trace/rv/rv_trace.h
index e4e78d23b7b0..4a6faddac614 100644
--- a/kernel/trace/rv/rv_trace.h
+++ b/kernel/trace/rv/rv_trace.h
@@ -62,6 +62,7 @@ DECLARE_EVENT_CLASS(error_da_monitor,
#include <monitors/scpd/scpd_trace.h>
#include <monitors/snep/snep_trace.h>
#include <monitors/sts/sts_trace.h>
+#include <monitors/opid/opid_trace.h>
// Add new monitors based on CONFIG_DA_MON_EVENTS_IMPLICIT here
#endif /* CONFIG_DA_MON_EVENTS_IMPLICIT */
diff --git a/tools/verification/models/sched/opid.dot b/tools/verification/models/sched/opid.dot
new file mode 100644
index 000000000000..840052f6952b
--- /dev/null
+++ b/tools/verification/models/sched/opid.dot
@@ -0,0 +1,35 @@
+digraph state_automaton {
+ center = true;
+ size = "7,11";
+ {node [shape = plaintext, style=invis, label=""] "__init_disabled"};
+ {node [shape = circle] "disabled"};
+ {node [shape = doublecircle] "enabled"};
+ {node [shape = circle] "enabled"};
+ {node [shape = circle] "in_irq"};
+ {node [shape = circle] "irq_disabled"};
+ {node [shape = circle] "preempt_disabled"};
+ "__init_disabled" -> "disabled";
+ "disabled" [label = "disabled"];
+ "disabled" -> "disabled" [ label = "sched_need_resched\nsched_waking\nirq_entry" ];
+ "disabled" -> "irq_disabled" [ label = "preempt_enable" ];
+ "disabled" -> "preempt_disabled" [ label = "irq_enable" ];
+ "enabled" [label = "enabled", color = green3];
+ "enabled" -> "enabled" [ label = "preempt_enable" ];
+ "enabled" -> "irq_disabled" [ label = "irq_disable" ];
+ "enabled" -> "preempt_disabled" [ label = "preempt_disable" ];
+ "in_irq" [label = "in_irq"];
+ "in_irq" -> "enabled" [ label = "irq_enable" ];
+ "in_irq" -> "in_irq" [ label = "sched_need_resched\nsched_waking\nirq_entry" ];
+ "irq_disabled" [label = "irq_disabled"];
+ "irq_disabled" -> "disabled" [ label = "preempt_disable" ];
+ "irq_disabled" -> "enabled" [ label = "irq_enable" ];
+ "irq_disabled" -> "in_irq" [ label = "irq_entry" ];
+ "irq_disabled" -> "irq_disabled" [ label = "sched_need_resched" ];
+ "preempt_disabled" [label = "preempt_disabled"];
+ "preempt_disabled" -> "disabled" [ label = "irq_disable" ];
+ "preempt_disabled" -> "enabled" [ label = "preempt_enable" ];
+ { rank = min ;
+ "__init_disabled";
+ "disabled";
+ }
+}
--
2.50.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v5 9/9] rv: Add opid per-cpu monitor
2025-07-28 13:50 ` [PATCH v5 9/9] rv: Add opid per-cpu monitor Gabriele Monaco
@ 2025-07-28 15:58 ` Nam Cao
0 siblings, 0 replies; 15+ messages in thread
From: Nam Cao @ 2025-07-28 15:58 UTC (permalink / raw)
To: Gabriele Monaco
Cc: linux-kernel, Steven Rostedt, Jonathan Corbet, Masami Hiramatsu,
linux-trace-kernel, linux-doc, Ingo Molnar, Peter Zijlstra,
Tomas Glozar, Juri Lelli, Clark Williams, John Kacur
On Mon, Jul 28, 2025 at 03:50:21PM +0200, Gabriele Monaco wrote:
> Add a per-cpu monitor as part of the sched model:
> * opid: operations with preemption and irq disabled
> Monitor to ensure wakeup and need_resched occur with irq and
> preemption disabled or in irq handlers.
>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
The monitor implementation looks fine:
Acked-by: Nam Cao <namcao@linutronix.de>
And on x86 and riscv64:
Tested-by: Nam Cao <namcao@linutronix.de>
^ permalink raw reply [flat|nested] 15+ messages in thread