* [PATCH v6 07/16] rv: Convert the opid monitor to a hybrid automaton
From: Gabriele Monaco @ 2026-02-25 9:51 UTC (permalink / raw)
To: linux-kernel, Steven Rostedt, Nam Cao, Juri Lelli,
Gabriele Monaco, Jonathan Corbet, Masami Hiramatsu,
linux-trace-kernel, linux-doc
Cc: Tomas Glozar, Clark Williams, John Kacur
In-Reply-To: <20260225095122.80683-1-gmonaco@redhat.com>
The opid monitor validates that wakeup and need_resched events only
occur with interrupts and preemption disabled by following the
preemptirq tracepoints.
As reported in [1], those tracepoints might be inaccurate in some
situations (e.g. NMIs).
Since the monitor doesn't validate other ordering properties, remove the
dependency on preemptirq tracepoints and convert the monitor to a hybrid
automaton to validate the constraint during event handling.
This makes the monitor more robust by also removing the workaround for
interrupts missing the preemption tracepoints, which was working on
PREEMPT_RT only and allows the monitor to be built on kernels without
the preemptirqs tracepoints.
[1] - https://lore.kernel.org/lkml/20250625120823.60600-1-gmonaco@redhat.com
Reviewed-by: Nam Cao <namcao@linutronix.de>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
Notes:
V4:
Use da_handle_start_run_event not to lose the first event
Documentation/trace/rv/monitor_sched.rst | 62 +++---------
kernel/trace/rv/monitors/opid/Kconfig | 11 +-
kernel/trace/rv/monitors/opid/opid.c | 111 +++++++--------------
kernel/trace/rv/monitors/opid/opid.h | 86 ++++------------
kernel/trace/rv/monitors/opid/opid_trace.h | 4 +
kernel/trace/rv/rv_trace.h | 2 +-
tools/verification/models/sched/opid.dot | 36 ++-----
7 files changed, 82 insertions(+), 230 deletions(-)
diff --git a/Documentation/trace/rv/monitor_sched.rst b/Documentation/trace/rv/monitor_sched.rst
index 3f8381ad9ec7..0b96d6e147c6 100644
--- a/Documentation/trace/rv/monitor_sched.rst
+++ b/Documentation/trace/rv/monitor_sched.rst
@@ -346,55 +346,21 @@ 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.
+preemption disabled.
``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.
+doesn't match a task wakeup and might occur with only interrupts disabled.
+The interrupt and preemption status are validated by the hybrid automaton
+constraints when processing the events::
+
+ |
+ |
+ v
+ #=========# sched_need_resched;irq_off == 1
+ H H sched_waking;irq_off == 1 && preempt_off == 1
+ H any H ------------------------------------------------+
+ H H |
+ H H <-----------------------------------------------+
+ #=========#
References
----------
diff --git a/kernel/trace/rv/monitors/opid/Kconfig b/kernel/trace/rv/monitors/opid/Kconfig
index 561d32da572b..6d02e239b684 100644
--- a/kernel/trace/rv/monitors/opid/Kconfig
+++ b/kernel/trace/rv/monitors/opid/Kconfig
@@ -2,18 +2,13 @@
#
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
+ default y
+ select HA_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.
+ interrupts and preemption disabled.
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
index 25a40e90fa40..4594c7c46601 100644
--- a/kernel/trace/rv/monitors/opid/opid.c
+++ b/kernel/trace/rv/monitors/opid/opid.c
@@ -10,94 +10,63 @@
#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>
#define RV_MON_TYPE RV_MON_PER_CPU
#include "opid.h"
-#include <rv/da_monitor.h>
+#include <rv/ha_monitor.h>
-#ifdef CONFIG_X86_LOCAL_APIC
-#include <asm/trace/irq_vectors.h>
-
-static void handle_vector_irq_entry(void *data, int vector)
+static u64 ha_get_env(struct ha_monitor *ha_mon, enum envs_opid env, u64 time_ns)
{
- da_handle_event(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);
+ if (env == irq_off_opid)
+ return irqs_disabled();
+ else if (env == preempt_off_opid) {
+ /*
+ * If CONFIG_PREEMPTION is enabled, then the tracepoint itself disables
+ * preemption (adding one to the preempt_count). Since we are
+ * interested in the preempt_count at the time the tracepoint was
+ * hit, we consider 1 as still enabled.
+ */
+ if (IS_ENABLED(CONFIG_PREEMPTION))
+ return (preempt_count() & PREEMPT_MASK) > 1;
+ return true;
}
+ return ENV_INVALID_VALUE;
}
-static void detach_vector_irq(void)
+static inline bool ha_verify_guards(struct ha_monitor *ha_mon,
+ enum states curr_state, enum events event,
+ enum states next_state, u64 time_ns)
{
- 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);
- }
+ bool res = true;
+
+ if (curr_state == any_opid && event == sched_need_resched_opid)
+ res = ha_get_env(ha_mon, irq_off_opid, time_ns) == 1ull;
+ else if (curr_state == any_opid && event == sched_waking_opid)
+ res = ha_get_env(ha_mon, irq_off_opid, time_ns) == 1ull &&
+ ha_get_env(ha_mon, preempt_off_opid, time_ns) == 1ull;
+ return res;
}
-#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)
+static bool ha_verify_constraint(struct ha_monitor *ha_mon,
+ enum states curr_state, enum events event,
+ enum states next_state, u64 time_ns)
{
- da_handle_event(irq_disable_opid);
-}
+ if (!ha_verify_guards(ha_mon, curr_state, event, next_state, time_ns))
+ return false;
-static void handle_irq_enable(void *data, unsigned long ip, unsigned long parent_ip)
-{
- da_handle_event(irq_enable_opid);
-}
-
-static void handle_irq_entry(void *data, int irq, struct irqaction *action)
-{
- da_handle_event(irq_entry_opid);
-}
-
-static void handle_preempt_disable(void *data, unsigned long ip, unsigned long parent_ip)
-{
- da_handle_event(preempt_disable_opid);
-}
-
-static void handle_preempt_enable(void *data, unsigned long ip, unsigned long parent_ip)
-{
- da_handle_event(preempt_enable_opid);
+ return true;
}
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(sched_need_resched_opid);
- else
- da_handle_start_event(sched_need_resched_opid);
+ da_handle_start_run_event(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(sched_waking_opid);
- else
- da_handle_start_event(sched_waking_opid);
+ da_handle_start_run_event(sched_waking_opid);
}
static int enable_opid(void)
@@ -108,14 +77,8 @@ static int enable_opid(void)
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;
}
@@ -124,14 +87,8 @@ static void disable_opid(void)
{
rv_this.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();
}
diff --git a/kernel/trace/rv/monitors/opid/opid.h b/kernel/trace/rv/monitors/opid/opid.h
index 092992514970..fb0aa4c28aa6 100644
--- a/kernel/trace/rv/monitors/opid/opid.h
+++ b/kernel/trace/rv/monitors/opid/opid.h
@@ -8,30 +8,31 @@
#define MONITOR_NAME opid
enum states_opid {
- disabled_opid,
- enabled_opid,
- in_irq_opid,
- irq_disabled_opid,
- preempt_disabled_opid,
+ any_opid,
state_max_opid,
};
#define INVALID_STATE state_max_opid
enum events_opid {
- irq_disable_opid,
- irq_enable_opid,
- irq_entry_opid,
- preempt_disable_opid,
- preempt_enable_opid,
sched_need_resched_opid,
sched_waking_opid,
event_max_opid,
};
+enum envs_opid {
+ irq_off_opid,
+ preempt_off_opid,
+ env_max_opid,
+ env_max_stored_opid = irq_off_opid,
+};
+
+_Static_assert(env_max_stored_opid <= MAX_HA_ENV_LEN, "Not enough slots");
+
struct automaton_opid {
char *state_names[state_max_opid];
char *event_names[event_max_opid];
+ char *env_names[env_max_opid];
unsigned char function[state_max_opid][event_max_opid];
unsigned char initial_state;
bool final_states[state_max_opid];
@@ -39,68 +40,19 @@ struct automaton_opid {
static const struct automaton_opid automaton_opid = {
.state_names = {
- "disabled",
- "enabled",
- "in_irq",
- "irq_disabled",
- "preempt_disabled",
+ "any",
},
.event_names = {
- "irq_disable",
- "irq_enable",
- "irq_entry",
- "preempt_disable",
- "preempt_enable",
"sched_need_resched",
"sched_waking",
},
+ .env_names = {
+ "irq_off",
+ "preempt_off",
+ },
.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,
- },
+ { any_opid, any_opid },
},
- .initial_state = disabled_opid,
- .final_states = { 0, 1, 0, 0, 0 },
+ .initial_state = any_opid,
+ .final_states = { 1 },
};
diff --git a/kernel/trace/rv/monitors/opid/opid_trace.h b/kernel/trace/rv/monitors/opid/opid_trace.h
index 3df6ff955c30..b04005b64208 100644
--- a/kernel/trace/rv/monitors/opid/opid_trace.h
+++ b/kernel/trace/rv/monitors/opid/opid_trace.h
@@ -12,4 +12,8 @@ DEFINE_EVENT(event_da_monitor, event_opid,
DEFINE_EVENT(error_da_monitor, error_opid,
TP_PROTO(char *state, char *event),
TP_ARGS(state, event));
+
+DEFINE_EVENT(error_env_da_monitor, error_env_opid,
+ TP_PROTO(char *state, char *event, char *env),
+ TP_ARGS(state, event, env));
#endif /* CONFIG_RV_MON_OPID */
diff --git a/kernel/trace/rv/rv_trace.h b/kernel/trace/rv/rv_trace.h
index 1661f8fe4a88..9e8072d863a2 100644
--- a/kernel/trace/rv/rv_trace.h
+++ b/kernel/trace/rv/rv_trace.h
@@ -62,7 +62,6 @@ 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
#ifdef CONFIG_HA_MON_EVENTS_IMPLICIT
@@ -91,6 +90,7 @@ DECLARE_EVENT_CLASS(error_env_da_monitor,
__get_str(env))
);
+#include <monitors/opid/opid_trace.h>
// Add new monitors based on CONFIG_HA_MON_EVENTS_IMPLICIT here
#endif
diff --git a/tools/verification/models/sched/opid.dot b/tools/verification/models/sched/opid.dot
index 840052f6952b..511051fce430 100644
--- a/tools/verification/models/sched/opid.dot
+++ b/tools/verification/models/sched/opid.dot
@@ -1,35 +1,13 @@
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" ];
+ {node [shape = plaintext, style=invis, label=""] "__init_any"};
+ {node [shape = doublecircle] "any"};
+ "__init_any" -> "any";
+ "any" [label = "any", color = green3];
+ "any" -> "any" [ label = "sched_need_resched;irq_off == 1\nsched_waking;irq_off == 1 && preempt_off == 1" ];
{ rank = min ;
- "__init_disabled";
- "disabled";
+ "__init_any";
+ "any";
}
}
--
2.53.0
^ permalink raw reply related
* [PATCH v6 06/16] rv: Add sample hybrid monitors stall
From: Gabriele Monaco @ 2026-02-25 9:51 UTC (permalink / raw)
To: linux-kernel, Steven Rostedt, Nam Cao, Juri Lelli,
Jonathan Corbet, Gabriele Monaco, Masami Hiramatsu, linux-doc,
linux-trace-kernel
Cc: Tomas Glozar, Clark Williams, John Kacur
In-Reply-To: <20260225095122.80683-1-gmonaco@redhat.com>
Add a sample monitor to showcase hybrid/timed automata.
The stall monitor identifies tasks stalled for longer than a threshold
and reacts when that happens.
Reviewed-by: Nam Cao <namcao@linutronix.de>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
Notes:
V3:
* Extend stall model to handle preemption
Documentation/tools/rv/index.rst | 1 +
Documentation/tools/rv/rv-mon-stall.rst | 44 ++++++
Documentation/trace/rv/index.rst | 1 +
Documentation/trace/rv/monitor_stall.rst | 43 ++++++
kernel/trace/rv/Kconfig | 1 +
kernel/trace/rv/Makefile | 1 +
kernel/trace/rv/monitors/stall/Kconfig | 13 ++
kernel/trace/rv/monitors/stall/stall.c | 150 +++++++++++++++++++
kernel/trace/rv/monitors/stall/stall.h | 81 ++++++++++
kernel/trace/rv/monitors/stall/stall_trace.h | 19 +++
kernel/trace/rv/rv_trace.h | 1 +
tools/verification/models/stall.dot | 22 +++
12 files changed, 377 insertions(+)
create mode 100644 Documentation/tools/rv/rv-mon-stall.rst
create mode 100644 Documentation/trace/rv/monitor_stall.rst
create mode 100644 kernel/trace/rv/monitors/stall/Kconfig
create mode 100644 kernel/trace/rv/monitors/stall/stall.c
create mode 100644 kernel/trace/rv/monitors/stall/stall.h
create mode 100644 kernel/trace/rv/monitors/stall/stall_trace.h
create mode 100644 tools/verification/models/stall.dot
diff --git a/Documentation/tools/rv/index.rst b/Documentation/tools/rv/index.rst
index fd42b0017d07..2aaa01c9fe48 100644
--- a/Documentation/tools/rv/index.rst
+++ b/Documentation/tools/rv/index.rst
@@ -16,3 +16,4 @@ Runtime verification (rv) tool
rv-mon-wip
rv-mon-wwnr
rv-mon-sched
+ rv-mon-stall
diff --git a/Documentation/tools/rv/rv-mon-stall.rst b/Documentation/tools/rv/rv-mon-stall.rst
new file mode 100644
index 000000000000..c79d7c2e4dd4
--- /dev/null
+++ b/Documentation/tools/rv/rv-mon-stall.rst
@@ -0,0 +1,44 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============
+rv-mon-stall
+============
+--------------------
+Stalled task monitor
+--------------------
+
+:Manual section: 1
+
+SYNOPSIS
+========
+
+**rv mon stall** [*OPTIONS*]
+
+DESCRIPTION
+===========
+
+The stalled task (**stall**) monitor is a sample per-task timed monitor that
+checks if tasks are scheduled within a defined threshold after they are ready.
+
+See kernel documentation for further information about this monitor:
+<https://docs.kernel.org/trace/rv/monitor_stall.html>
+
+OPTIONS
+=======
+
+.. include:: common_ikm.rst
+
+SEE ALSO
+========
+
+**rv**\(1), **rv-mon**\(1)
+
+Linux kernel *RV* documentation:
+<https://www.kernel.org/doc/html/latest/trace/rv/index.html>
+
+AUTHOR
+======
+
+Written by Gabriele Monaco <gmonaco@redhat.com>
+
+.. include:: common_appendix.rst
diff --git a/Documentation/trace/rv/index.rst b/Documentation/trace/rv/index.rst
index ad298784bda2..bf9962f49959 100644
--- a/Documentation/trace/rv/index.rst
+++ b/Documentation/trace/rv/index.rst
@@ -16,3 +16,4 @@ Runtime Verification
monitor_wwnr.rst
monitor_sched.rst
monitor_rtapp.rst
+ monitor_stall.rst
diff --git a/Documentation/trace/rv/monitor_stall.rst b/Documentation/trace/rv/monitor_stall.rst
new file mode 100644
index 000000000000..d29e820b2433
--- /dev/null
+++ b/Documentation/trace/rv/monitor_stall.rst
@@ -0,0 +1,43 @@
+Monitor stall
+=============
+
+- Name: stall - stalled task monitor
+- Type: per-task hybrid automaton
+- Author: Gabriele Monaco <gmonaco@redhat.com>
+
+Description
+-----------
+
+The stalled task (stall) monitor is a sample per-task timed monitor that checks
+if tasks are scheduled within a defined threshold after they are ready::
+
+ |
+ |
+ v
+ #==========================#
+ +-----------------> H dequeued H
+ | #==========================#
+ | |
+ sched_switch_wait | sched_wakeup;reset(clk)
+ | v
+ | +--------------------------+ <+
+ | | enqueued | | sched_wakeup
+ | | clk < threshold_jiffies | -+
+ | +--------------------------+
+ | | ^
+ | sched_switch_in sched_switch_preempt;reset(clk)
+ | v |
+ | +--------------------------+
+ +------------------ | running |
+ +--------------------------+
+ ^ sched_switch_in |
+ | sched_wakeup |
+ +----------------------+
+
+The threshold can be configured as a parameter by either booting with the
+``stall.threshold_jiffies=<new value>`` argument or writing a new value to
+``/sys/module/stall/parameters/threshold_jiffies``.
+
+Specification
+-------------
+Graphviz Dot file in tools/verification/models/stall.dot
diff --git a/kernel/trace/rv/Kconfig b/kernel/trace/rv/Kconfig
index 4ad392dfc57f..720fbe4935f8 100644
--- a/kernel/trace/rv/Kconfig
+++ b/kernel/trace/rv/Kconfig
@@ -78,6 +78,7 @@ source "kernel/trace/rv/monitors/pagefault/Kconfig"
source "kernel/trace/rv/monitors/sleep/Kconfig"
# Add new rtapp monitors here
+source "kernel/trace/rv/monitors/stall/Kconfig"
# Add new monitors here
config RV_REACTORS
diff --git a/kernel/trace/rv/Makefile b/kernel/trace/rv/Makefile
index 750e4ad6fa0f..51c95e2d2da6 100644
--- a/kernel/trace/rv/Makefile
+++ b/kernel/trace/rv/Makefile
@@ -17,6 +17,7 @@ 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
+obj-$(CONFIG_RV_MON_STALL) += monitors/stall/stall.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/stall/Kconfig b/kernel/trace/rv/monitors/stall/Kconfig
new file mode 100644
index 000000000000..6f846b642544
--- /dev/null
+++ b/kernel/trace/rv/monitors/stall/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+config RV_MON_STALL
+ depends on RV
+ select HA_MON_EVENTS_ID
+ bool "stall monitor"
+ help
+ Enable the stall sample monitor that illustrates the usage of hybrid
+ automata monitors. It can be used to identify tasks stalled for
+ longer than a threshold.
+
+ For further information, see:
+ Documentation/trace/rv/monitor_stall.rst
diff --git a/kernel/trace/rv/monitors/stall/stall.c b/kernel/trace/rv/monitors/stall/stall.c
new file mode 100644
index 000000000000..9ccfda6b0e73
--- /dev/null
+++ b/kernel/trace/rv/monitors/stall/stall.c
@@ -0,0 +1,150 @@
+// 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>
+
+#define MODULE_NAME "stall"
+
+#include <trace/events/sched.h>
+#include <rv_trace.h>
+
+#define RV_MON_TYPE RV_MON_PER_TASK
+#define HA_TIMER_TYPE HA_TIMER_WHEEL
+#include "stall.h"
+#include <rv/ha_monitor.h>
+
+static u64 threshold_jiffies = 1000;
+module_param(threshold_jiffies, ullong, 0644);
+
+static u64 ha_get_env(struct ha_monitor *ha_mon, enum envs_stall env, u64 time_ns)
+{
+ if (env == clk_stall)
+ return ha_get_clk_jiffy(ha_mon, env);
+ return ENV_INVALID_VALUE;
+}
+
+static void ha_reset_env(struct ha_monitor *ha_mon, enum envs_stall env, u64 time_ns)
+{
+ if (env == clk_stall)
+ ha_reset_clk_jiffy(ha_mon, env);
+}
+
+static inline bool ha_verify_invariants(struct ha_monitor *ha_mon,
+ enum states curr_state, enum events event,
+ enum states next_state, u64 time_ns)
+{
+ if (curr_state == enqueued_stall)
+ return ha_check_invariant_jiffy(ha_mon, clk_stall, time_ns);
+ return true;
+}
+
+static inline bool ha_verify_guards(struct ha_monitor *ha_mon,
+ enum states curr_state, enum events event,
+ enum states next_state, u64 time_ns)
+{
+ bool res = true;
+
+ if (curr_state == dequeued_stall && event == sched_wakeup_stall)
+ ha_reset_env(ha_mon, clk_stall, time_ns);
+ else if (curr_state == running_stall && event == sched_switch_preempt_stall)
+ ha_reset_env(ha_mon, clk_stall, time_ns);
+ return res;
+}
+
+static inline void ha_setup_invariants(struct ha_monitor *ha_mon,
+ enum states curr_state, enum events event,
+ enum states next_state, u64 time_ns)
+{
+ if (next_state == curr_state)
+ return;
+ if (next_state == enqueued_stall)
+ ha_start_timer_jiffy(ha_mon, clk_stall, threshold_jiffies, time_ns);
+ else if (curr_state == enqueued_stall)
+ ha_cancel_timer(ha_mon);
+}
+
+static bool ha_verify_constraint(struct ha_monitor *ha_mon,
+ enum states curr_state, enum events event,
+ enum states next_state, u64 time_ns)
+{
+ if (!ha_verify_invariants(ha_mon, curr_state, event, next_state, time_ns))
+ return false;
+
+ if (!ha_verify_guards(ha_mon, curr_state, event, next_state, time_ns))
+ return false;
+
+ ha_setup_invariants(ha_mon, curr_state, event, next_state, time_ns);
+
+ return true;
+}
+
+static void handle_sched_switch(void *data, bool preempt,
+ struct task_struct *prev,
+ struct task_struct *next,
+ unsigned int prev_state)
+{
+ if (!preempt && prev_state != TASK_RUNNING)
+ da_handle_start_event(prev, sched_switch_wait_stall);
+ else
+ da_handle_event(prev, sched_switch_preempt_stall);
+ da_handle_event(next, sched_switch_in_stall);
+}
+
+static void handle_sched_wakeup(void *data, struct task_struct *p)
+{
+ da_handle_event(p, sched_wakeup_stall);
+}
+
+static int enable_stall(void)
+{
+ int retval;
+
+ retval = da_monitor_init();
+ if (retval)
+ return retval;
+
+ rv_attach_trace_probe("stall", sched_switch, handle_sched_switch);
+ rv_attach_trace_probe("stall", sched_wakeup, handle_sched_wakeup);
+
+ return 0;
+}
+
+static void disable_stall(void)
+{
+ rv_this.enabled = 0;
+
+ rv_detach_trace_probe("stall", sched_switch, handle_sched_switch);
+ rv_detach_trace_probe("stall", sched_wakeup, handle_sched_wakeup);
+
+ da_monitor_destroy();
+}
+
+static struct rv_monitor rv_this = {
+ .name = "stall",
+ .description = "identify tasks stalled for longer than a threshold.",
+ .enable = enable_stall,
+ .disable = disable_stall,
+ .reset = da_monitor_reset_all,
+ .enabled = 0,
+};
+
+static int __init register_stall(void)
+{
+ return rv_register_monitor(&rv_this, NULL);
+}
+
+static void __exit unregister_stall(void)
+{
+ rv_unregister_monitor(&rv_this);
+}
+
+module_init(register_stall);
+module_exit(unregister_stall);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Gabriele Monaco <gmonaco@redhat.com>");
+MODULE_DESCRIPTION("stall: identify tasks stalled for longer than a threshold.");
diff --git a/kernel/trace/rv/monitors/stall/stall.h b/kernel/trace/rv/monitors/stall/stall.h
new file mode 100644
index 000000000000..638520cb1082
--- /dev/null
+++ b/kernel/trace/rv/monitors/stall/stall.h
@@ -0,0 +1,81 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Automatically generated C representation of stall automaton
+ * For further information about this format, see kernel documentation:
+ * Documentation/trace/rv/deterministic_automata.rst
+ */
+
+#define MONITOR_NAME stall
+
+enum states_stall {
+ dequeued_stall,
+ enqueued_stall,
+ running_stall,
+ state_max_stall,
+};
+
+#define INVALID_STATE state_max_stall
+
+enum events_stall {
+ sched_switch_in_stall,
+ sched_switch_preempt_stall,
+ sched_switch_wait_stall,
+ sched_wakeup_stall,
+ event_max_stall,
+};
+
+enum envs_stall {
+ clk_stall,
+ env_max_stall,
+ env_max_stored_stall = env_max_stall,
+};
+
+_Static_assert(env_max_stored_stall <= MAX_HA_ENV_LEN, "Not enough slots");
+
+struct automaton_stall {
+ char *state_names[state_max_stall];
+ char *event_names[event_max_stall];
+ char *env_names[env_max_stall];
+ unsigned char function[state_max_stall][event_max_stall];
+ unsigned char initial_state;
+ bool final_states[state_max_stall];
+};
+
+static const struct automaton_stall automaton_stall = {
+ .state_names = {
+ "dequeued",
+ "enqueued",
+ "running",
+ },
+ .event_names = {
+ "sched_switch_in",
+ "sched_switch_preempt",
+ "sched_switch_wait",
+ "sched_wakeup",
+ },
+ .env_names = {
+ "clk",
+ },
+ .function = {
+ {
+ INVALID_STATE,
+ INVALID_STATE,
+ INVALID_STATE,
+ enqueued_stall,
+ },
+ {
+ running_stall,
+ INVALID_STATE,
+ INVALID_STATE,
+ enqueued_stall,
+ },
+ {
+ running_stall,
+ enqueued_stall,
+ dequeued_stall,
+ running_stall,
+ },
+ },
+ .initial_state = dequeued_stall,
+ .final_states = { 1, 0, 0 },
+};
diff --git a/kernel/trace/rv/monitors/stall/stall_trace.h b/kernel/trace/rv/monitors/stall/stall_trace.h
new file mode 100644
index 000000000000..6a7cc1b1d040
--- /dev/null
+++ b/kernel/trace/rv/monitors/stall/stall_trace.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Snippet to be included in rv_trace.h
+ */
+
+#ifdef CONFIG_RV_MON_STALL
+DEFINE_EVENT(event_da_monitor_id, event_stall,
+ 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_stall,
+ TP_PROTO(int id, char *state, char *event),
+ TP_ARGS(id, state, event));
+
+DEFINE_EVENT(error_env_da_monitor_id, error_env_stall,
+ TP_PROTO(int id, char *state, char *event, char *env),
+ TP_ARGS(id, state, event, env));
+#endif /* CONFIG_RV_MON_STALL */
diff --git a/kernel/trace/rv/rv_trace.h b/kernel/trace/rv/rv_trace.h
index 7c598967bc0e..1661f8fe4a88 100644
--- a/kernel/trace/rv/rv_trace.h
+++ b/kernel/trace/rv/rv_trace.h
@@ -187,6 +187,7 @@ DECLARE_EVENT_CLASS(error_env_da_monitor_id,
__get_str(env))
);
+#include <monitors/stall/stall_trace.h>
// Add new monitors based on CONFIG_HA_MON_EVENTS_ID here
#endif
diff --git a/tools/verification/models/stall.dot b/tools/verification/models/stall.dot
new file mode 100644
index 000000000000..50077d1dff74
--- /dev/null
+++ b/tools/verification/models/stall.dot
@@ -0,0 +1,22 @@
+digraph state_automaton {
+ center = true;
+ size = "7,11";
+ {node [shape = circle] "enqueued"};
+ {node [shape = plaintext, style=invis, label=""] "__init_dequeued"};
+ {node [shape = doublecircle] "dequeued"};
+ {node [shape = circle] "running"};
+ "__init_dequeued" -> "dequeued";
+ "enqueued" [label = "enqueued\nclk < threshold_jiffies"];
+ "running" [label = "running"];
+ "dequeued" [label = "dequeued", color = green3];
+ "running" -> "running" [ label = "sched_switch_in\nsched_wakeup" ];
+ "enqueued" -> "enqueued" [ label = "sched_wakeup" ];
+ "enqueued" -> "running" [ label = "sched_switch_in" ];
+ "running" -> "dequeued" [ label = "sched_switch_wait" ];
+ "dequeued" -> "enqueued" [ label = "sched_wakeup;reset(clk)" ];
+ "running" -> "enqueued" [ label = "sched_switch_preempt;reset(clk)" ];
+ { rank = min ;
+ "__init_dequeued";
+ "dequeued";
+ }
+}
--
2.53.0
^ permalink raw reply related
* [PATCH v6 05/16] Documentation/rv: Add documentation about hybrid automata
From: Gabriele Monaco @ 2026-02-25 9:51 UTC (permalink / raw)
To: linux-kernel, Steven Rostedt, Nam Cao, Juri Lelli,
Gabriele Monaco, Jonathan Corbet, linux-trace-kernel, linux-doc
Cc: Tomas Glozar, Clark Williams, John Kacur
In-Reply-To: <20260225095122.80683-1-gmonaco@redhat.com>
Describe theory and implementation of hybrid automata in the dedicated
page hybrid_automata.rst
Include a section on how to integrate a hybrid automaton in
monitor_synthesis.rst
Also remove a hanging $ in deterministic_automata.rst
Reviewed-by: Nam Cao <namcao@linutronix.de>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
Notes:
V3:
* Improve documentation clarity after review
.../trace/rv/deterministic_automata.rst | 2 +-
Documentation/trace/rv/hybrid_automata.rst | 341 ++++++++++++++++++
Documentation/trace/rv/index.rst | 1 +
Documentation/trace/rv/monitor_synthesis.rst | 117 +++++-
4 files changed, 458 insertions(+), 3 deletions(-)
create mode 100644 Documentation/trace/rv/hybrid_automata.rst
diff --git a/Documentation/trace/rv/deterministic_automata.rst b/Documentation/trace/rv/deterministic_automata.rst
index d0638f95a455..7a1c2b20ec72 100644
--- a/Documentation/trace/rv/deterministic_automata.rst
+++ b/Documentation/trace/rv/deterministic_automata.rst
@@ -11,7 +11,7 @@ where:
- *E* is the finite set of events;
- x\ :subscript:`0` is the initial state;
- X\ :subscript:`m` (subset of *X*) is the set of marked (or final) states.
-- *f* : *X* x *E* -> *X* $ is the transition function. It defines the state
+- *f* : *X* x *E* -> *X* is the transition function. It defines the state
transition in the occurrence of an event from *E* in the state *X*. In the
special case of deterministic automata, the occurrence of the event in *E*
in a state in *X* has a deterministic next state from *X*.
diff --git a/Documentation/trace/rv/hybrid_automata.rst b/Documentation/trace/rv/hybrid_automata.rst
new file mode 100644
index 000000000000..39c037a71b89
--- /dev/null
+++ b/Documentation/trace/rv/hybrid_automata.rst
@@ -0,0 +1,341 @@
+Hybrid Automata
+===============
+
+Hybrid automata are an extension of deterministic automata, there are several
+definitions of hybrid automata in the literature. The adaptation implemented
+here is formally denoted by G and defined as a 7-tuple:
+
+ *G* = { *X*, *E*, *V*, *f*, x\ :subscript:`0`, X\ :subscript:`m`, *i* }
+
+- *X* is the set of states;
+- *E* is the finite set of events;
+- *V* is the finite set of environment variables;
+- x\ :subscript:`0` is the initial state;
+- X\ :subscript:`m` (subset of *X*) is the set of marked (or final) states.
+- *f* : *X* x *E* x *C(V)* -> *X* is the transition function.
+ It defines the state transition in the occurrence of an event from *E* in the
+ state *X*. Unlike deterministic automata, the transition function also
+ includes guards from the set of all possible constraints (defined as *C(V)*).
+ Guards can be true or false with the valuation of *V* when the event occurs,
+ and the transition is possible only when constraints are true. Similarly to
+ deterministic automata, the occurrence of the event in *E* in a state in *X*
+ has a deterministic next state from *X*, if the guard is true.
+- *i* : *X* -> *C'(V)* is the invariant assignment function, this is a
+ constraint assigned to each state in *X*, every state in *X* must be left
+ before the invariant turns to false. We can omit the representation of
+ invariants whose value is true regardless of the valuation of *V*.
+
+The set of all possible constraints *C(V)* is defined according to the
+following grammar:
+
+ g = v < c | v > c | v <= c | v >= c | v == c | v != c | g && g | true
+
+With v a variable in *V* and c a numerical value.
+
+We define the special case of hybrid automata whose variables grow with uniform
+rates as timed automata. In this case, the variables are called clocks.
+As the name implies, timed automata can be used to describe real time.
+Additionally, clocks support another type of guard which always evaluates to true:
+
+ reset(v)
+
+The reset constraint is used to set the value of a clock to 0.
+
+The set of invariant constraints *C'(V)* is a subset of *C(V)* including only
+constraint of the form:
+
+ g = v < c | true
+
+This simplifies the implementation as a clock expiration is a necessary and
+sufficient condition for the violation of invariants while still allowing more
+complex constraints to be specified as guards.
+
+It is important to note that any hybrid automaton is a valid deterministic
+automaton with additional guards and invariants. Those can only further
+constrain what transitions are valid but it is not possible to define
+transition functions starting from the same state in *X* and the same event in
+*E* but ending up in different states in *X* based on the valuation of *V*.
+
+Examples
+--------
+
+Wip as hybrid automaton
+~~~~~~~~~~~~~~~~~~~~~~~
+
+The 'wip' (wakeup in preemptive) example introduced as a deterministic automaton
+can also be described as:
+
+- *X* = { ``any_thread_running`` }
+- *E* = { ``sched_waking`` }
+- *V* = { ``preemptive`` }
+- x\ :subscript:`0` = ``any_thread_running``
+- X\ :subscript:`m` = {``any_thread_running``}
+- *f* =
+ - *f*\ (``any_thread_running``, ``sched_waking``, ``preemptive==0``) = ``any_thread_running``
+- *i* =
+ - *i*\ (``any_thread_running``) = ``true``
+
+Which can be represented graphically as::
+
+ |
+ |
+ v
+ #====================# sched_waking;preemptive==0
+ H H ------------------------------+
+ H any_thread_running H |
+ H H <-----------------------------+
+ #====================#
+
+In this example, by using the preemptive state of the system as an environment
+variable, we can assert this constraint on ``sched_waking`` without requiring
+preemption events (as we would in a deterministic automaton), which can be
+useful in case those events are not available or not reliable on the system.
+
+Since all the invariants in *i* are true, we can omit them from the representation.
+
+Stall model with guards (iteration 1)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+As a sample timed automaton we can define 'stall' as:
+
+- *X* = { ``dequeued``, ``enqueued``, ``running``}
+- *E* = { ``enqueue``, ``dequeue``, ``switch_in``}
+- *V* = { ``clk`` }
+- x\ :subscript:`0` = ``dequeue``
+- X\ :subscript:`m` = {``dequeue``}
+- *f* =
+ - *f*\ (``enqueued``, ``switch_in``, ``clk < threshold``) = ``running``
+ - *f*\ (``running``, ``dequeue``) = ``dequeued``
+ - *f*\ (``dequeued``, ``enqueue``, ``reset(clk)``) = ``enqueued``
+- *i* = *omitted as all true*
+
+Graphically represented as::
+
+ |
+ |
+ v
+ #============================#
+ H dequeued H <+
+ #============================# |
+ | |
+ | enqueue; reset(clk) |
+ v |
+ +----------------------------+ |
+ | enqueued | | dequeue
+ +----------------------------+ |
+ | |
+ | switch_in; clk < threshold |
+ v |
+ +----------------------------+ |
+ | running | -+
+ +----------------------------+
+
+This model imposes that the time between when a task is enqueued (it becomes
+runnable) and when the task gets to run must be lower than a certain threshold.
+A failure in this model means that the task is starving.
+One problem in using guards on the edges in this case is that the model will
+not report a failure until the ``switch_in`` event occurs. This means that,
+according to the model, it is valid for the task never to run.
+
+Stall model with invariants (iteration 2)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The first iteration isn't exactly what was intended, we can change the model as:
+
+- *X* = { ``dequeued``, ``enqueued``, ``running``}
+- *E* = { ``enqueue``, ``dequeue``, ``switch_in``}
+- *V* = { ``clk`` }
+- x\ :subscript:`0` = ``dequeue``
+- X\ :subscript:`m` = {``dequeue``}
+- *f* =
+ - *f*\ (``enqueued``, ``switch_in``) = ``running``
+ - *f*\ (``running``, ``dequeue``) = ``dequeued``
+ - *f*\ (``dequeued``, ``enqueue``, ``reset(clk)``) = ``enqueued``
+- *i* =
+ - *i*\ (``enqueued``) = ``clk < threshold``
+
+Graphically::
+
+ |
+ |
+ v
+ #=========================#
+ H dequeued H <+
+ #=========================# |
+ | |
+ | enqueue; reset(clk) |
+ v |
+ +-------------------------+ |
+ | enqueued | |
+ | clk < threshold | | dequeue
+ +-------------------------+ |
+ | |
+ | switch_in |
+ v |
+ +-------------------------+ |
+ | running | -+
+ +-------------------------+
+
+In this case, we moved the guard as an invariant to the ``enqueued`` state,
+this means we not only forbid the occurrence of ``switch_in`` when ``clk`` is
+past the threshold but also mark as invalid in case we are *still* in
+``enqueued`` after the threshold. This model is effectively in an invalid state
+as soon as a task is starving, rather than when the starving task finally runs.
+
+Hybrid Automaton in C
+---------------------
+
+The definition of hybrid automata in C is heavily based on the deterministic
+automata one. Specifically, we add the set of environment variables and the
+constraints (both guards on transitions and invariants on states) as follows.
+This is a combination of both iterations of the stall example::
+
+ /* enum representation of X (set of states) to be used as index */
+ enum states {
+ dequeued,
+ enqueued,
+ running,
+ state_max,
+ };
+
+ #define INVALID_STATE state_max
+
+ /* enum representation of E (set of events) to be used as index */
+ enum events {
+ dequeue,
+ enqueue,
+ switch_in,
+ event_max,
+ };
+
+ /* enum representation of V (set of environment variables) to be used as index */
+ enum envs {
+ clk,
+ env_max,
+ env_max_stored = env_max,
+ };
+
+ struct automaton {
+ char *state_names[state_max]; // X: the set of states
+ char *event_names[event_max]; // E: the finite set of events
+ char *env_names[env_max]; // V: the finite set of env vars
+ unsigned char function[state_max][event_max]; // f: transition function
+ unsigned char initial_state; // x_0: the initial state
+ bool final_states[state_max]; // X_m: the set of marked states
+ };
+
+ struct automaton aut = {
+ .state_names = {
+ "dequeued",
+ "enqueued",
+ "running",
+ },
+ .event_names = {
+ "dequeue",
+ "enqueue",
+ "switch_in",
+ },
+ .env_names = {
+ "clk",
+ },
+ .function = {
+ { INVALID_STATE, enqueued, INVALID_STATE },
+ { INVALID_STATE, INVALID_STATE, running },
+ { dequeued, INVALID_STATE, INVALID_STATE },
+ },
+ .initial_state = dequeued,
+ .final_states = { 1, 0, 0 },
+ };
+
+ static bool verify_constraint(enum states curr_state, enum events event,
+ enum states next_state)
+ {
+ bool res = true;
+
+ /* Validate guards as part of f */
+ if (curr_state == enqueued && event == sched_switch_in)
+ res = get_env(clk) < threshold;
+ else if (curr_state == dequeued && event == sched_wakeup)
+ reset_env(clk);
+
+ /* Validate invariants in i */
+ if (next_state == curr_state || !res)
+ return res;
+ if (next_state == enqueued)
+ ha_start_timer_jiffy(ha_mon, clk, threshold_jiffies);
+ else if (curr_state == enqueued)
+ res = !ha_cancel_timer(ha_mon);
+ return res;
+ }
+
+The function ``verify_constraint``, here reported as simplified, checks guards,
+performs resets and starts timers to validate invariants according to
+specification.
+Due to the complex nature of environment variables, the user needs to provide
+functions to get and reset environment variables that are not common clocks
+(e.g. clocks with ns or jiffy granularity).
+Since invariants are only defined as clock expirations (e.g. *clk <
+threshold*), reaching the expiration of a timer armed when entering the state
+is in fact a failure in the model and triggers a reaction. Leaving the state
+stops the timer.
+
+It is important to note that timers implemented with hrtimers introduce
+overhead, if the monitor has several instances (e.g. all tasks) this can become
+an issue. The impact can be decreased using the timer wheel (``HA_TIMER_TYPE``
+set to ``HA_TIMER_WHEEL``), this lowers the responsiveness of the timer without
+damaging the accuracy of the model, since the invariant condition is checked
+before disabling the timer in case the callback is late.
+Alternatively, if the monitor is guaranteed to *eventually* leave the state and
+the incurred delay to wait for the next event is acceptable, guards can be used
+in place of invariants, as seen in the stall example.
+
+Graphviz .dot format
+--------------------
+
+Also the Graphviz representation of hybrid automata is an extension of the
+deterministic automata one. Specifically, guards can be provided in the event
+name separated by ``;``::
+
+ "state_start" -> "state_dest" [ label = "sched_waking;preemptible==0;reset(clk)" ];
+
+Invariant can be specified in the state label (not the node name!) separated by ``\n``::
+
+ "enqueued" [label = "enqueued\nclk < threshold_jiffies"];
+
+Constraints can be specified as valid C comparisons and allow spaces, the first
+element of the comparison must be the clock while the second is a numerical or
+parametrised value. Guards allow comparisons to be combined with boolean
+operations (``&&`` and ``||``), resets must be separated from other constraints.
+
+This is the full example of the last version of the 'stall' model in DOT::
+
+ digraph state_automaton {
+ {node [shape = circle] "enqueued"};
+ {node [shape = plaintext, style=invis, label=""] "__init_dequeued"};
+ {node [shape = doublecircle] "dequeued"};
+ {node [shape = circle] "running"};
+ "__init_dequeued" -> "dequeued";
+ "enqueued" [label = "enqueued\nclk < threshold_jiffies"];
+ "running" [label = "running"];
+ "dequeued" [label = "dequeued"];
+ "enqueued" -> "running" [ label = "switch_in" ];
+ "running" -> "dequeued" [ label = "dequeue" ];
+ "dequeued" -> "enqueued" [ label = "enqueue;reset(clk)" ];
+ { rank = min ;
+ "__init_dequeued";
+ "dequeued";
+ }
+ }
+
+References
+----------
+
+One book covering model checking and timed automata is::
+
+ Christel Baier and Joost-Pieter Katoen: Principles of Model Checking,
+ The MIT Press, 2008.
+
+Hybrid automata are described in detail in::
+
+ Thomas Henzinger: The theory of hybrid automata,
+ Proceedings 11th Annual IEEE Symposium on Logic in Computer Science, 1996.
diff --git a/Documentation/trace/rv/index.rst b/Documentation/trace/rv/index.rst
index a2812ac5cfeb..ad298784bda2 100644
--- a/Documentation/trace/rv/index.rst
+++ b/Documentation/trace/rv/index.rst
@@ -9,6 +9,7 @@ Runtime Verification
runtime-verification.rst
deterministic_automata.rst
linear_temporal_logic.rst
+ hybrid_automata.rst
monitor_synthesis.rst
da_monitor_instrumentation.rst
monitor_wip.rst
diff --git a/Documentation/trace/rv/monitor_synthesis.rst b/Documentation/trace/rv/monitor_synthesis.rst
index cc5f97977a29..2c1b5a0ae154 100644
--- a/Documentation/trace/rv/monitor_synthesis.rst
+++ b/Documentation/trace/rv/monitor_synthesis.rst
@@ -18,8 +18,8 @@ functions that glue the monitor to the system reference model, and the
trace output as a reaction to event parsing and exceptions, as depicted
below::
- Linux +----- RV Monitor ----------------------------------+ Formal
- Realm | | Realm
+ Linux +---- RV Monitor ----------------------------------+ Formal
+ Realm | | Realm
+-------------------+ +----------------+ +-----------------+
| Linux kernel | | Monitor | | Reference |
| Tracing | -> | Instance(s) | <- | Model |
@@ -45,6 +45,7 @@ creating monitors. The header files are:
* rv/da_monitor.h for deterministic automaton monitor.
* rv/ltl_monitor.h for linear temporal logic monitor.
+ * rv/ha_monitor.h for hybrid automaton monitor.
rvgen
-----
@@ -252,6 +253,118 @@ the task, the monitor may need some time to start validating tasks which have
been running before the monitor is enabled. Therefore, it is recommended to
start the tasks of interest after enabling the monitor.
+rv/ha_monitor.h
++++++++++++++++
+
+The implementation of hybrid automaton monitors derives directly from the
+deterministic automaton one. Despite using a different header
+(``ha_monitor.h``) the functions to handle events are the same (e.g.
+``da_handle_event``).
+
+Additionally, the `rvgen` tool populates skeletons for the
+``ha_verify_constraint``, ``ha_get_env`` and ``ha_reset_env`` based on the
+monitor specification in the monitor source file.
+
+``ha_verify_constraint`` is typically ready as it is generated by `rvgen`:
+
+* standard constraints on edges are turned into the form::
+
+ res = ha_get_env(ha_mon, ENV) < VALUE;
+
+* reset constraints are turned into the form::
+
+ ha_reset_env(ha_mon, ENV);
+
+* constraints on the state are implemented using timers
+
+ - armed before entering the state
+
+ - cancelled while entering any other state
+
+ - untouched if the state does not change as a result of the event
+
+ - checked if the timer expired but the callback did not run
+
+ - available implementation are `HA_TIMER_HRTIMER` and `HA_TIMER_WHEEL`
+
+ - hrtimers are more precise but may have higher overhead
+
+ - select by defining `HA_TIMER_TYPE` before including the header::
+
+ #define HA_TIMER_TYPE HA_TIMER_HRTIMER
+
+Constraint values can be specified in different forms:
+
+* literal value (with optional unit). E.g.::
+
+ preemptive == 0
+ clk < 100ns
+ threshold <= 10j
+
+* constant value (uppercase string). E.g.::
+
+ clk < MAX_NS
+
+* parameter (lowercase string). E.g.::
+
+ clk <= threshold_jiffies
+
+* macro (uppercase string with parentheses). E.g.::
+
+ clk < MAX_NS()
+
+* function (lowercase string with parentheses). E.g.::
+
+ clk <= threshold_jiffies()
+
+In all cases, `rvgen` will try to understand the type of the environment
+variable from the name or unit. For instance, constants or parameters
+terminating with ``_NS`` or ``_jiffies`` are intended as clocks with ns and jiffy
+granularity, respectively. Literals with measure unit `j` are jiffies and if a
+time unit is specified (`ns` to `s`), `rvgen` will convert the value to `ns`.
+
+Constants need to be defined by the user (but unlike the name, they don't
+necessarily need to be defined as constants). Parameters get converted to
+module parameters and the user needs to provide a default value.
+Also function and macros are defined by the user, by default they get as an
+argument the ``ha_monitor``, a common usage would be to get the required value
+from the target, e.g. the task in per-task monitors, using the helper
+``ha_get_target(ha_mon)``.
+
+If `rvgen` determines that the variable is a clock, it provides the getter and
+resetter based on the unit. Otherwise, the user needs to provide an appropriate
+definition.
+Typically non-clock environment variables are not reset. In such case only the
+getter skeleton will be present in the file generated by `rvgen`.
+For instance, the getter for preemptive can be filled as::
+
+ static u64 ha_get_env(struct ha_monitor *ha_mon, enum envs env)
+ {
+ if (env == preemptible)
+ return preempt_count() == 0;
+ return ENV_INVALID_VALUE;
+ }
+
+The function is supplied the ``ha_mon`` parameter in case some storage is
+required (as it is for clocks), but environment variables without reset do not
+require a storage and can ignore that argument.
+The number of environment variables requiring a storage is limited by
+``MAX_HA_ENV_LEN``, however such limitation doesn't stand for other variables.
+
+Finally, constraints on states are only valid for clocks and only if the
+constraint is of the form `clk < N`. This is because such constraints are
+implemented with the expiration of a timer.
+Typically the clock variables are reset just before arming the timer, but this
+doesn't have to be the case and the available functions take care of it.
+It is a responsibility of per-task monitors to make sure no timer is left
+running when the task exits.
+
+By default the generator implements timers with hrtimers (setting
+``HA_TIMER_TYPE`` to ``HA_TIMER_HRTIMER``), this gives better responsiveness
+but higher overhead. The timer wheel (``HA_TIMER_WHEEL``) is a good alternative
+for monitors with several instances (e.g. per-task) that achieves lower
+overhead with increased latency, yet without compromising precision.
+
Final remarks
-------------
--
2.53.0
^ permalink raw reply related
* [PATCH v6 04/16] verification/rvgen: Add support for Hybrid Automata
From: Gabriele Monaco @ 2026-02-25 9:51 UTC (permalink / raw)
To: linux-kernel, Steven Rostedt, Nam Cao, Juri Lelli,
Gabriele Monaco, linux-trace-kernel
Cc: Tomas Glozar, Clark Williams, John Kacur
In-Reply-To: <20260225095122.80683-1-gmonaco@redhat.com>
Add the possibility to parse dot files as hybrid automata and generate
the necessary code from rvgen.
Hybrid automata are very similar to deterministic ones and most
functionality is shared, the dot files include also constraints together
with event names (separated by ;) and state names (separated by \n).
The tool can now generate the appropriate code to validate constraints
at runtime according to the dot specification.
Reviewed-by: Nam Cao <namcao@linutronix.de>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
Notes:
V6:
* Use f-strings in newly added code and cleanup
* Sort constraints for predictable generated code
V4:
* Sort self_loop_reset_events to avoid unpredictable order
tools/verification/rvgen/__main__.py | 8 +-
tools/verification/rvgen/rvgen/automata.py | 146 +++++-
tools/verification/rvgen/rvgen/dot2c.py | 47 ++
tools/verification/rvgen/rvgen/dot2k.py | 474 +++++++++++++++++-
tools/verification/rvgen/rvgen/generator.py | 2 +
.../rvgen/rvgen/templates/dot2k/main.c | 2 +-
.../rvgen/templates/dot2k/trace_hybrid.h | 16 +
7 files changed, 679 insertions(+), 16 deletions(-)
create mode 100644 tools/verification/rvgen/rvgen/templates/dot2k/trace_hybrid.h
diff --git a/tools/verification/rvgen/__main__.py b/tools/verification/rvgen/__main__.py
index fa6fc1f4de2f..b8e07e463293 100644
--- a/tools/verification/rvgen/__main__.py
+++ b/tools/verification/rvgen/__main__.py
@@ -9,7 +9,7 @@
# Documentation/trace/rv/da_monitor_synthesis.rst
if __name__ == '__main__':
- from rvgen.dot2k import dot2k
+ from rvgen.dot2k import da2k, ha2k
from rvgen.generator import Monitor
from rvgen.container import Container
from rvgen.ltl2k import ltl2k
@@ -29,7 +29,7 @@ if __name__ == '__main__':
monitor_parser.add_argument("-p", "--parent", dest="parent",
required=False, help="Create a monitor nested to parent")
monitor_parser.add_argument('-c', "--class", dest="monitor_class",
- help="Monitor class, either \"da\" or \"ltl\"")
+ help="Monitor class, either \"da\", \"ha\" or \"ltl\"")
monitor_parser.add_argument('-s', "--spec", dest="spec", help="Monitor specification file")
monitor_parser.add_argument('-t', "--monitor_type", dest="monitor_type",
help=f"Available options: {', '.join(Monitor.monitor_types.keys())}")
@@ -43,7 +43,9 @@ if __name__ == '__main__':
if params.subcmd == "monitor":
print("Opening and parsing the specification file %s" % params.spec)
if params.monitor_class == "da":
- monitor = dot2k(params.spec, params.monitor_type, vars(params))
+ monitor = da2k(params.spec, params.monitor_type, vars(params))
+ elif params.monitor_class == "ha":
+ monitor = ha2k(params.spec, params.monitor_type, vars(params))
elif params.monitor_class == "ltl":
monitor = ltl2k(params.spec, params.monitor_type, vars(params))
else:
diff --git a/tools/verification/rvgen/rvgen/automata.py b/tools/verification/rvgen/rvgen/automata.py
index 34a2e2a6b217..5c1c5597d839 100644
--- a/tools/verification/rvgen/rvgen/automata.py
+++ b/tools/verification/rvgen/rvgen/automata.py
@@ -9,24 +9,64 @@
# Documentation/trace/rv/deterministic_automata.rst
import ntpath
+import re
+from typing import Iterator
+
+class _ConstraintKey:
+ """Base class for constraint keys."""
+
+class _StateConstraintKey(_ConstraintKey, int):
+ """Key for a state constraint. Under the hood just state_id."""
+ def __new__(cls, state_id: int):
+ return super().__new__(cls, state_id)
+
+class _EventConstraintKey(_ConstraintKey, tuple):
+ """Key for an event constraint. Under the hood just tuple(state_id,event_id)."""
+ def __new__(cls, state_id: int, event_id: int):
+ return super().__new__(cls, (state_id, event_id))
class Automata:
"""Automata class: Reads a dot file and part it as an automata.
+ It supports both deterministic and hybrid automata.
+
Attributes:
dot_file: A dot file with an state_automaton definition.
"""
invalid_state_str = "INVALID_STATE"
+ # val can be numerical, uppercase (constant or macro), lowercase (parameter or function)
+ # only numerical values should have units
+ constraint_rule = re.compile(r"""
+ ^
+ (?P<env>[a-zA-Z_][a-zA-Z0-9_]+) # C-like identifier for the env var
+ (?P<op>[!<=>]{1,2}) # operator
+ (?P<val>
+ [0-9]+ | # numerical value
+ [A-Z_]+\(\) | # macro
+ [A-Z_]+ | # constant
+ [a-z_]+\(\) | # function
+ [a-z_]+ # parameter
+ )
+ (?P<unit>[a-z]{1,2})? # optional unit for numerical values
+ """, re.VERBOSE)
+ constraint_reset = re.compile(r"^reset\((?P<env>[a-zA-Z_][a-zA-Z0-9_]+)\)")
def __init__(self, file_path, model_name=None):
self.__dot_path = file_path
self.name = model_name or self.__get_model_name()
self.__dot_lines = self.__open_dot()
self.states, self.initial_state, self.final_states = self.__get_state_variables()
- self.events = self.__get_event_variables()
- self.function = self.__create_matrix()
+ self.env_types = {}
+ self.env_stored = set()
+ self.constraint_vars = set()
+ self.self_loop_reset_events = set()
+ self.events, self.envs = self.__get_event_variables()
+ self.function, self.constraints = self.__create_matrix()
self.events_start, self.events_start_run = self.__store_init_events()
+ self.env_stored = sorted(self.env_stored)
+ self.constraint_vars = sorted(self.constraint_vars)
+ self.self_loop_reset_events = sorted(self.self_loop_reset_events)
def __get_model_name(self) -> str:
basename = ntpath.basename(self.__dot_path)
@@ -116,30 +156,93 @@ class Automata:
return states, initial_state, final_states
- def __get_event_variables(self) -> list[str]:
+ def __get_event_variables(self) -> tuple[list[str], list[str]]:
# here we are at the begin of transitions, take a note, we will return later.
cursor = self.__get_cursor_begin_events()
events = []
+ envs = []
while self.__dot_lines[cursor].lstrip()[0] == '"':
# transitions have the format:
# "all_fired" -> "both_fired" [ label = "disable_irq" ];
# ------------ event is here ------------^^^^^
if self.__dot_lines[cursor].split()[1] == "->":
line = self.__dot_lines[cursor].split()
- event = "".join(line[line.index("label")+2:-1]).replace('"', '')
+ event = "".join(line[line.index("label") + 2:-1]).replace('"', '')
# when a transition has more than one lables, they are like this
# "local_irq_enable\nhw_local_irq_enable_n"
# so split them.
for i in event.split("\\n"):
- events.append(i)
+ # if the event contains a constraint (hybrid automata),
+ # it will be separated by a ";":
+ # "sched_switch;x<1000;reset(x)"
+ ev, *constr = i.split(";")
+ if constr:
+ if len(constr) > 2:
+ raise ValueError("Only 1 constraint and 1 reset are supported")
+ envs += self.__extract_env_var(constr)
+ events.append(ev)
+ else:
+ # state labels have the format:
+ # "enable_fired" [label = "enable_fired\ncondition"];
+ # ----- label is here -----^^^^^
+ # label and node name must be the same, condition is optional
+ state = self.__dot_lines[cursor].split("label")[1].split('"')[1]
+ _, *constr = state.split("\\n")
+ if constr:
+ if len(constr) > 1:
+ raise ValueError("Only 1 constraint is supported in the state")
+ envs += self.__extract_env_var([constr[0].replace(" ", "")])
cursor += 1
- return sorted(set(events))
-
- def __create_matrix(self) -> list[list[str]]:
+ return sorted(set(events)), sorted(set(envs))
+
+ def _split_constraint_expr(self, constr: list[str]) -> Iterator[tuple[str,
+ str | None]]:
+ """
+ Get a list of strings of the type constr1 && constr2 and returns a list of
+ constraints and separators: [[constr1,"&&"],[constr2,None]]
+ """
+ exprs = []
+ seps = []
+ for c in constr:
+ while "&&" in c or "||" in c:
+ a = c.find("&&")
+ o = c.find("||")
+ pos = a if o < 0 or 0 < a < o else o
+ exprs.append(c[:pos].replace(" ", ""))
+ seps.append(c[pos:pos + 2].replace(" ", ""))
+ c = c[pos + 2:].replace(" ", "")
+ exprs.append(c)
+ seps.append(None)
+ return zip(exprs, seps)
+
+ def __extract_env_var(self, constraint: list[str]) -> list[str]:
+ env = []
+ for c, _ in self._split_constraint_expr(constraint):
+ rule = self.constraint_rule.search(c)
+ reset = self.constraint_reset.search(c)
+ if rule:
+ env.append(rule["env"])
+ if rule.groupdict().get("unit"):
+ self.env_types[rule["env"]] = rule["unit"]
+ if rule["val"][0].isalpha():
+ self.constraint_vars.add(rule["val"])
+ # try to infer unit from constants or parameters
+ val_for_unit = rule["val"].lower().replace("()", "")
+ if val_for_unit.endswith("_ns"):
+ self.env_types[rule["env"]] = "ns"
+ if val_for_unit.endswith("_jiffies"):
+ self.env_types[rule["env"]] = "j"
+ if reset:
+ env.append(reset["env"])
+ # environment variables that are reset need a storage
+ self.env_stored.add(reset["env"])
+ return env
+
+ def __create_matrix(self) -> tuple[list[list[str]], dict[_ConstraintKey, list[str]]]:
# transform the array into a dictionary
events = self.events
states = self.states
@@ -157,6 +260,7 @@ class Automata:
# declare the matrix....
matrix = [[ self.invalid_state_str for x in range(nr_event)] for y in range(nr_state)]
+ constraints: dict[_ConstraintKey, list[str]] = {}
# and we are back! Let's fill the matrix
cursor = self.__get_cursor_begin_events()
@@ -166,12 +270,24 @@ class Automata:
line = self.__dot_lines[cursor].split()
origin_state = line[0].replace('"','').replace(',','_')
dest_state = line[2].replace('"','').replace(',','_')
- possible_events = "".join(line[line.index("label")+2:-1]).replace('"', '')
+ possible_events = "".join(line[line.index("label") + 2:-1]).replace('"', '')
for event in possible_events.split("\\n"):
+ event, *constr = event.split(";")
+ if constr:
+ key = _EventConstraintKey(states_dict[origin_state], events_dict[event])
+ constraints[key] = constr
+ # those events reset also on self loops
+ if origin_state == dest_state and "reset" in "".join(constr):
+ self.self_loop_reset_events.add(event)
matrix[states_dict[origin_state]][events_dict[event]] = dest_state
+ else:
+ state = self.__dot_lines[cursor].split("label")[1].split('"')[1]
+ state, *constr = state.replace(" ", "").split("\\n")
+ if constr:
+ constraints[_StateConstraintKey(states_dict[state])] = constr
cursor += 1
- return matrix
+ return matrix, constraints
def __store_init_events(self) -> tuple[list[bool], list[bool]]:
events_start = [False] * len(self.events)
@@ -203,3 +319,13 @@ class Automata:
if any(self.events_start):
return False
return self.events_start_run[self.events.index(event)]
+
+ def is_hybrid_automata(self) -> bool:
+ return bool(self.envs)
+
+ def is_event_constraint(self, key: _ConstraintKey) -> bool:
+ """
+ Given the key in self.constraints return true if it is an event
+ constraint, false if it is a state constraint
+ """
+ return isinstance(key, _EventConstraintKey)
diff --git a/tools/verification/rvgen/rvgen/dot2c.py b/tools/verification/rvgen/rvgen/dot2c.py
index 06a26bf15a7e..f779d9528af3 100644
--- a/tools/verification/rvgen/rvgen/dot2c.py
+++ b/tools/verification/rvgen/rvgen/dot2c.py
@@ -19,6 +19,7 @@ class Dot2c(Automata):
enum_suffix = ""
enum_states_def = "states"
enum_events_def = "events"
+ enum_envs_def = "envs"
struct_automaton_def = "automaton"
var_automaton_def = "aut"
@@ -61,6 +62,37 @@ class Dot2c(Automata):
return buff
+ def __get_non_stored_envs(self) -> list[str]:
+ return [e for e in self.envs if e not in self.env_stored]
+
+ def __get_enum_envs_content(self) -> list[str]:
+ buff = []
+ # We first place env variables that have a u64 storage.
+ # Those are limited by MAX_HA_ENV_LEN, other variables
+ # are read only and don't require a storage.
+ unstored = self.__get_non_stored_envs()
+ for env in list(self.env_stored) + unstored:
+ buff.append(f"\t{env}{self.enum_suffix},")
+
+ buff.append(f"\tenv_max{self.enum_suffix},")
+ max_stored = unstored[0] if len(unstored) else "env_max"
+ buff.append(f"\tenv_max_stored{self.enum_suffix} = {max_stored}{self.enum_suffix},")
+
+ return buff
+
+ def format_envs_enum(self) -> list[str]:
+ buff = []
+ if self.is_hybrid_automata():
+ buff.append(f"enum {self.enum_envs_def} {{")
+ buff += self.__get_enum_envs_content()
+ buff.append("};\n")
+ buff.append(f"_Static_assert(env_max_stored{self.enum_suffix} <= MAX_HA_ENV_LEN,"
+ ' "Not enough slots");')
+ if {"ns", "us", "ms", "s"}.intersection(self.env_types.values()):
+ buff.append("#define HA_CLK_NS")
+ buff.append("")
+ return buff
+
def get_minimun_type(self) -> str:
min_type = "unsigned char"
@@ -81,6 +113,8 @@ class Dot2c(Automata):
buff.append("struct %s {" % self.struct_automaton_def)
buff.append("\tchar *state_names[state_max%s];" % (self.enum_suffix))
buff.append("\tchar *event_names[event_max%s];" % (self.enum_suffix))
+ if self.is_hybrid_automata():
+ buff.append(f"\tchar *env_names[env_max{self.enum_suffix}];")
buff.append("\t%s function[state_max%s][event_max%s];" % (min_type, self.enum_suffix, self.enum_suffix))
buff.append("\t%s initial_state;" % min_type)
buff.append("\tbool final_states[state_max%s];" % (self.enum_suffix))
@@ -113,6 +147,17 @@ class Dot2c(Automata):
return buff
+ def format_aut_init_envs_string(self) -> list[str]:
+ buff = []
+ if self.is_hybrid_automata():
+ buff.append("\t.env_names = {")
+ # maintain consistent order with the enum
+ ordered_envs = list(self.env_stored) + self.__get_non_stored_envs()
+ buff.append(self.__get_string_vector_per_line_content(ordered_envs))
+ buff.append("\t},")
+
+ return buff
+
def __get_max_strlen_of_states(self) -> int:
max_state_name = max(self.states, key = len).__len__()
return max(max_state_name, self.invalid_state_str.__len__())
@@ -205,10 +250,12 @@ class Dot2c(Automata):
buff += self.format_states_enum()
buff += self.format_invalid_state()
buff += self.format_events_enum()
+ buff += self.format_envs_enum()
buff += self.format_automaton_definition()
buff += self.format_aut_init_header()
buff += self.format_aut_init_states_string()
buff += self.format_aut_init_events_string()
+ buff += self.format_aut_init_envs_string()
buff += self.format_aut_init_function()
buff += self.format_aut_init_initial_state()
buff += self.format_aut_init_final_states()
diff --git a/tools/verification/rvgen/rvgen/dot2k.py b/tools/verification/rvgen/rvgen/dot2k.py
index 6128fe238430..3cdc8cfb6be5 100644
--- a/tools/verification/rvgen/rvgen/dot2k.py
+++ b/tools/verification/rvgen/rvgen/dot2k.py
@@ -8,8 +8,10 @@
# For further information, see:
# Documentation/trace/rv/da_monitor_synthesis.rst
+from collections import deque
from .dot2c import Dot2c
from .generator import Monitor
+from .automata import _EventConstraintKey, _StateConstraintKey
class dot2k(Monitor, Dot2c):
@@ -20,12 +22,16 @@ class dot2k(Monitor, Dot2c):
Monitor.__init__(self, extra_params)
Dot2c.__init__(self, file_path, extra_params.get("model_name"))
self.enum_suffix = "_%s" % self.name
+ self.monitor_class = extra_params["monitor_class"]
def fill_monitor_type(self) -> str:
- return self.monitor_type.upper()
+ buff = [ self.monitor_type.upper() ]
+ buff += self._fill_timer_type()
+ return "\n".join(buff)
def fill_tracepoint_handlers_skel(self) -> str:
buff = []
+ buff += self._fill_hybrid_definitions()
for event in self.events:
buff.append("static void handle_%s(void *data, /* XXX: fill header */)" % event)
buff.append("{")
@@ -77,6 +83,7 @@ class dot2k(Monitor, Dot2c):
#
self.enum_states_def = "states_%s" % self.name
self.enum_events_def = "events_%s" % self.name
+ self.enum_envs_def = f"envs_{self.name}"
self.struct_automaton_def = "automaton_%s" % self.name
self.var_automaton_def = "automaton_%s" % self.name
@@ -107,8 +114,14 @@ class dot2k(Monitor, Dot2c):
("char *", "state"),
("char *", "event"),
]
+ tp_args_error_env = tp_args_error + [("char *", "env")]
+ tp_args_dict = {
+ "event": tp_args_event,
+ "error": tp_args_error,
+ "error_env": tp_args_error_env
+ }
tp_args_id = ("int ", "id")
- tp_args = tp_args_event if tp_type == "event" else tp_args_error
+ tp_args = tp_args_dict[tp_type]
if self.monitor_type == "per_task":
tp_args.insert(0, tp_args_id)
tp_proto_c = ", ".join([a+b for a,b in tp_args])
@@ -117,6 +130,14 @@ class dot2k(Monitor, Dot2c):
buff.append(" TP_ARGS(%s)" % tp_args_c)
return '\n'.join(buff)
+ def _fill_hybrid_definitions(self) -> list:
+ """Stub, not valid for deterministic automata"""
+ return []
+
+ def _fill_timer_type(self) -> list:
+ """Stub, not valid for deterministic automata"""
+ return []
+
def fill_main_c(self) -> str:
main_c = super().fill_main_c()
@@ -127,5 +148,454 @@ class dot2k(Monitor, Dot2c):
main_c = main_c.replace("%%MIN_TYPE%%", min_type)
main_c = main_c.replace("%%NR_EVENTS%%", str(nr_events))
main_c = main_c.replace("%%MONITOR_TYPE%%", monitor_type)
+ main_c = main_c.replace("%%MONITOR_CLASS%%", self.monitor_class)
return main_c
+
+class da2k(dot2k):
+ """Deterministic automata only"""
+ def __init__(self, *args, **kwargs):
+ super().__init__(*args, **kwargs)
+ if self.is_hybrid_automata():
+ raise ValueError("Detected hybrid automata, use the 'ha' class")
+
+class ha2k(dot2k):
+ """Hybrid automata only"""
+ def __init__(self, *args, **kwargs):
+ super().__init__(*args, **kwargs)
+ if not self.is_hybrid_automata():
+ raise ValueError("Detected deterministic automata, use the 'da' class")
+ self.trace_h = self._read_template_file("trace_hybrid.h")
+ self.__parse_constraints()
+
+ def fill_monitor_class_type(self) -> str:
+ if self.monitor_type == "per_task":
+ return "HA_MON_EVENTS_ID"
+ return "HA_MON_EVENTS_IMPLICIT"
+
+ def fill_monitor_class(self) -> str:
+ """
+ Used for tracepoint classes, since they are shared we keep da
+ instead of ha (also for the ha specific tracepoints).
+ The tracepoint class is not visible to the tools.
+ """
+ return super().fill_monitor_class()
+
+ def __adjust_value(self, value: str | int, unit: str | None) -> str:
+ """Adjust the value in ns"""
+ try:
+ value = int(value)
+ except ValueError:
+ # it's a constant, a parameter or a function
+ if value.endswith("()"):
+ return value.replace("()", "(ha_mon)")
+ return value
+ match unit:
+ case "us":
+ value *= 10**3
+ case "ms":
+ value *= 10**6
+ case "s":
+ value *= 10**9
+ return str(value) + "ull"
+
+ def __parse_single_constraint(self, rule: dict, value: str) -> str:
+ return f"ha_get_env(ha_mon, {rule["env"]}{self.enum_suffix}, time_ns) {rule["op"]} {value}"
+
+ def __get_constraint_env(self, constr: str) -> str:
+ """Extract the second argument from an ha_ function"""
+ env = constr.split("(")[1].split()[1].rstrip(")").rstrip(",")
+ assert env.rstrip(f"_{self.name}") in self.envs
+ return env
+
+ def __start_to_invariant_check(self, constr: str) -> str:
+ # by default assume the timer has ns expiration
+ env = self.__get_constraint_env(constr)
+ clock_type = "ns"
+ if self.env_types.get(env.rstrip(f"_{self.name}")) == "j":
+ clock_type = "jiffy"
+
+ return f"return ha_check_invariant_{clock_type}(ha_mon, {env}, time_ns)"
+
+ def __start_to_conv(self, constr: str) -> str:
+ """
+ Undo the storage conversion done by ha_start_timer_
+ """
+ return "ha_inv_to_guard" + constr[constr.find("("):]
+
+ def __parse_timer_constraint(self, rule: dict, value: str) -> str:
+ # by default assume the timer has ns expiration
+ clock_type = "ns"
+ if self.env_types.get(rule["env"]) == "j":
+ clock_type = "jiffy"
+
+ return (f"ha_start_timer_{clock_type}(ha_mon, {rule["env"]}{self.enum_suffix},"
+ f" {value}, time_ns)")
+
+ def __format_guard_rules(self, rules: list[str]) -> list[str]:
+ """
+ Merge guard constraints as a single C return statement.
+ If the rules include a stored env, also check its validity.
+ Break lines in a best effort way that tries to keep readability.
+ """
+ if not rules:
+ return []
+
+ invalid_checks = [f"ha_monitor_env_invalid(ha_mon, {env}{self.enum_suffix}) ||"
+ for env in self.env_stored if any(env in rule for rule in rules)]
+ if invalid_checks and len(rules) > 1:
+ rules[0] = "(" + rules[0]
+ rules[-1] = rules[-1] + ")"
+ rules = invalid_checks + rules
+
+ separator = "\n\t\t " if sum(len(r) for r in rules) > 80 else " "
+ return ["res = " + separator.join(rules)]
+
+ def __validate_constraint(self, key: tuple[int, int] | int, constr: str,
+ rule, reset) -> None:
+ # event constrains are tuples and allow both rules and reset
+ # state constraints are only used for expirations (e.g. clk<N)
+ if self.is_event_constraint(key):
+ if not rule and not reset:
+ raise ValueError("Unrecognised event constraint "
+ f"({self.states[key[0]]}/{self.events[key[1]]}: {constr})")
+ if rule and (rule["env"] in self.env_types and
+ rule["env"] not in self.env_stored):
+ raise ValueError("Clocks in hybrid automata always require a storage"
+ f" ({rule["env"]})")
+ else:
+ if not rule:
+ raise ValueError("Unrecognised state constraint "
+ f"({self.states[key]}: {constr})")
+ if rule["env"] not in self.env_stored:
+ raise ValueError("State constraints always require a storage "
+ f"({rule["env"]})")
+ if rule["op"] not in ["<", "<="]:
+ raise ValueError("State constraints must be clock expirations like"
+ f" clk<N ({rule.string})")
+
+ def __parse_constraints(self) -> None:
+ self.guards: dict[_EventConstraintKey, str] = {}
+ self.invariants: dict[_StateConstraintKey, str] = {}
+ for key, constraint in self.constraints.items():
+ rules = []
+ resets = []
+ for c, sep in self._split_constraint_expr(constraint):
+ rule = self.constraint_rule.search(c)
+ reset = self.constraint_reset.search(c)
+ self.__validate_constraint(key, c, rule, reset)
+ if rule:
+ value = rule["val"]
+ value_len = len(rule["val"])
+ unit = None
+ if rule.groupdict().get("unit"):
+ value_len += len(rule["unit"])
+ unit = rule["unit"]
+ c = c[:-(value_len)]
+ value = self.__adjust_value(value, unit)
+ if self.is_event_constraint(key):
+ c = self.__parse_single_constraint(rule, value)
+ if sep:
+ c += f" {sep}"
+ else:
+ c = self.__parse_timer_constraint(rule, value)
+ rules.append(c)
+ if reset:
+ c = f"ha_reset_env(ha_mon, {reset["env"]}{self.enum_suffix}, time_ns)"
+ resets.append(c)
+ if self.is_event_constraint(key):
+ res = self.__format_guard_rules(rules) + resets
+ self.guards[key] = ";".join(res)
+ else:
+ self.invariants[key] = rules[0]
+
+ def __fill_verify_invariants_func(self) -> list[str]:
+ buff = []
+ if not self.invariants:
+ return []
+
+ buff.append(
+f"""static inline bool ha_verify_invariants(struct ha_monitor *ha_mon,
+\t\t\t\t\tenum {self.enum_states_def} curr_state, enum {self.enum_events_def} event,
+\t\t\t\t\tenum {self.enum_states_def} next_state, u64 time_ns)
+{{""")
+
+ _else = ""
+ for state, constr in sorted(self.invariants.items()):
+ check_str = self.__start_to_invariant_check(constr)
+ buff.append(f"\t{_else}if (curr_state == {self.states[state]}{self.enum_suffix})")
+ buff.append(f"\t\t{check_str};")
+ _else = "else "
+
+ buff.append("\treturn true;\n}\n")
+ return buff
+
+ def __fill_convert_inv_guard_func(self) -> list[str]:
+ buff = []
+ if not self.invariants:
+ return []
+
+ conflict_guards, conflict_invs = self.__find_inv_conflicts()
+ if not conflict_guards and not conflict_invs:
+ return []
+
+ buff.append(
+f"""static inline void ha_convert_inv_guard(struct ha_monitor *ha_mon,
+\t\t\t\t\tenum {self.enum_states_def} curr_state, enum {self.enum_events_def} event,
+\t\t\t\t\tenum {self.enum_states_def} next_state, u64 time_ns)
+{{""")
+ buff.append("\tif (curr_state == next_state)\n\t\treturn;")
+
+ _else = ""
+ for state, constr in sorted(self.invariants.items()):
+ # a state with invariant can reach us without reset
+ # multiple conflicts must have the same invariant, otherwise we cannot
+ # know how to reset the value
+ conf_i = [start for start, end in conflict_invs if end == state]
+ # we can reach a guard without reset
+ conf_g = [e for s, e in conflict_guards if s == state]
+ if not conf_i and not conf_g:
+ continue
+ buff.append(f"\t{_else}if (curr_state == {self.states[state]}{self.enum_suffix})")
+
+ buff.append(f"\t\t{self.__start_to_conv(constr)};")
+ _else = "else "
+
+ buff.append("}\n")
+ return buff
+
+ def __fill_verify_guards_func(self) -> list[str]:
+ buff = []
+ if not self.guards:
+ return []
+
+ buff.append(
+f"""static inline bool ha_verify_guards(struct ha_monitor *ha_mon,
+\t\t\t\t enum {self.enum_states_def} curr_state, enum {self.enum_events_def} event,
+\t\t\t\t enum {self.enum_states_def} next_state, u64 time_ns)
+{{
+\tbool res = true;
+""")
+
+ _else = ""
+ for edge, constr in sorted(self.guards.items()):
+ buff.append(f"\t{_else}if (curr_state == "
+ f"{self.states[edge[0]]}{self.enum_suffix} && "
+ f"event == {self.events[edge[1]]}{self.enum_suffix})")
+ if constr.count(";") > 0:
+ buff[-1] += " {"
+ buff += [f"\t\t{c};" for c in constr.split(";")]
+ if constr.count(";") > 0:
+ _else = "} else "
+ else:
+ _else = "else "
+ if _else[0] == "}":
+ buff.append("\t}")
+ buff.append("\treturn res;\n}\n")
+ return buff
+
+ def __find_inv_conflicts(self) -> tuple[set[tuple[int, _EventConstraintKey]],
+ set[tuple[int, _StateConstraintKey]]]:
+ """
+ Run a breadth first search from all states with an invariant.
+ Find any conflicting constraints reachable from there, this can be
+ another state with an invariant or an edge with a non-reset guard.
+ Stop when we find a reset.
+
+ Return the set of conflicting guards and invariants as tuples of
+ conflicting state and constraint key.
+ """
+ conflict_guards: set[tuple[int, _EventConstraintKey]] = set()
+ conflict_invs: set[tuple[int, _StateConstraintKey]] = set()
+ for start_idx in self.invariants:
+ queue = deque([(start_idx, 0)]) # (state_idx, distance)
+ env = self.__get_constraint_env(self.invariants[start_idx])
+
+ while queue:
+ curr_idx, distance = queue.popleft()
+
+ # Check state condition
+ if curr_idx != start_idx and curr_idx in self.invariants:
+ conflict_invs.add((start_idx, _StateConstraintKey(curr_idx)))
+ continue
+
+ # Check if we should stop
+ if distance > len(self.states):
+ break
+ if curr_idx != start_idx and distance > 1:
+ continue
+
+ for event_idx, next_state_name in enumerate(self.function[curr_idx]):
+ if next_state_name == self.invalid_state_str:
+ continue
+ curr_guard = self.guards.get((curr_idx, event_idx), "")
+ if "reset" in curr_guard and env in curr_guard:
+ continue
+
+ if env in curr_guard:
+ conflict_guards.add((start_idx,
+ _EventConstraintKey(curr_idx, event_idx)))
+ continue
+
+ next_idx = self.states.index(next_state_name)
+ queue.append((next_idx, distance + 1))
+
+ return conflict_guards, conflict_invs
+
+ def __fill_setup_invariants_func(self) -> list[str]:
+ buff = []
+ if not self.invariants:
+ return []
+
+ buff.append(
+f"""static inline void ha_setup_invariants(struct ha_monitor *ha_mon,
+\t\t\t\t enum {self.enum_states_def} curr_state, enum {self.enum_events_def} event,
+\t\t\t\t enum {self.enum_states_def} next_state, u64 time_ns)
+{{""")
+
+ conditions = ["next_state == curr_state"]
+ conditions += [f"event != {e}{self.enum_suffix}"
+ for e in self.self_loop_reset_events]
+ condition_str = " && ".join(conditions)
+ buff.append(f"\tif ({condition_str})\n\t\treturn;")
+
+ _else = ""
+ for state, constr in sorted(self.invariants.items()):
+ buff.append(f"\t{_else}if (next_state == {self.states[state]}{self.enum_suffix})")
+ buff.append(f"\t\t{constr};")
+ _else = "else "
+
+ for state in self.invariants:
+ buff.append(f"\telse if (curr_state == {self.states[state]}{self.enum_suffix})")
+ buff.append("\t\tha_cancel_timer(ha_mon);")
+
+ buff.append("}\n")
+ return buff
+
+ def __fill_constr_func(self) -> list[str]:
+ buff = []
+ if not self.constraints:
+ return []
+
+ buff.append(
+"""/*
+ * These functions are used to validate state transitions.
+ *
+ * They are generated by parsing the model, there is usually no need to change them.
+ * If the monitor requires a timer, there are functions responsible to arm it when
+ * the next state has a constraint, cancel it in any other case and to check
+ * that it didn't expire before the callback run. Transitions to the same state
+ * without a reset never affect timers.
+ * Due to the different representations between invariants and guards, there is
+ * a function to convert it in case invariants or guards are reachable from
+ * another invariant without reset. Those are not present if not required in
+ * the model. This is all automatic but is worth checking because it may show
+ * errors in the model (e.g. missing resets).
+ */""")
+
+ buff += self.__fill_verify_invariants_func()
+ inv_conflicts = self.__fill_convert_inv_guard_func()
+ buff += inv_conflicts
+ buff += self.__fill_verify_guards_func()
+ buff += self.__fill_setup_invariants_func()
+
+ buff.append(
+f"""static bool ha_verify_constraint(struct ha_monitor *ha_mon,
+\t\t\t\t enum {self.enum_states_def} curr_state, enum {self.enum_events_def} event,
+\t\t\t\t enum {self.enum_states_def} next_state, u64 time_ns)
+{{""")
+
+ if self.invariants:
+ buff.append("\tif (!ha_verify_invariants(ha_mon, curr_state, "
+ "event, next_state, time_ns))\n\t\treturn false;\n")
+ if inv_conflicts:
+ buff.append("\tha_convert_inv_guard(ha_mon, curr_state, event, "
+ "next_state, time_ns);\n")
+
+ if self.guards:
+ buff.append("\tif (!ha_verify_guards(ha_mon, curr_state, event, "
+ "next_state, time_ns))\n\t\treturn false;\n")
+
+ if self.invariants:
+ buff.append("\tha_setup_invariants(ha_mon, curr_state, event, next_state, time_ns);\n")
+
+ buff.append("\treturn true;\n}\n")
+ return buff
+
+ def __fill_env_getter(self, env: str) -> str:
+ if env in self.env_types:
+ match self.env_types[env]:
+ case "ns" | "us" | "ms" | "s":
+ return "ha_get_clk_ns(ha_mon, env, time_ns);"
+ case "j":
+ return "ha_get_clk_jiffy(ha_mon, env);"
+ return f"/* XXX: how do I read {env}? */"
+
+ def __fill_env_resetter(self, env: str) -> str:
+ if env in self.env_types:
+ match self.env_types[env]:
+ case "ns" | "us" | "ms" | "s":
+ return "ha_reset_clk_ns(ha_mon, env, time_ns);"
+ case "j":
+ return "ha_reset_clk_jiffy(ha_mon, env);"
+ return f"/* XXX: how do I reset {env}? */"
+
+ def __fill_hybrid_get_reset_functions(self) -> list[str]:
+ buff = []
+ if self.is_hybrid_automata():
+ for var in self.constraint_vars:
+ if var.endswith("()"):
+ func_name = var.replace("()", "")
+ if func_name.isupper():
+ buff.append(f"#define {func_name}(ha_mon) "
+ f"/* XXX: what is {func_name}(ha_mon)? */\n")
+ else:
+ buff.append(f"static inline u64 {func_name}(struct ha_monitor *ha_mon)\n{{")
+ buff.append(f"\treturn /* XXX: what is {func_name}(ha_mon)? */;")
+ buff.append("}\n")
+ elif var.isupper():
+ buff.append(f"#define {var} /* XXX: what is {var}? */\n")
+ else:
+ buff.append(f"static u64 {var} = /* XXX: default value */;")
+ buff.append(f"module_param({var}, ullong, 0644);\n")
+ buff.append("""/*
+ * These functions define how to read and reset the environment variable.
+ *
+ * Common environment variables like ns-based and jiffy-based clocks have
+ * pre-define getters and resetters you can use. The parser can infer the type
+ * of the environment variable if you supply a measure unit in the constraint.
+ * If you define your own functions, make sure to add appropriate memory
+ * barriers if required.
+ * Some environment variables don't require a storage as they read a system
+ * state (e.g. preemption count). Those variables are never reset, so we don't
+ * define a reset function on monitors only relying on this type of variables.
+ */""")
+ buff.append("static u64 ha_get_env(struct ha_monitor *ha_mon, "
+ f"enum envs{self.enum_suffix} env, u64 time_ns)\n{{")
+ _else = ""
+ for env in self.envs:
+ buff.append(f"\t{_else}if (env == {env}{self.enum_suffix})")
+ buff.append(f"\t\treturn {self.__fill_env_getter(env)}")
+ _else = "else "
+ buff.append("\treturn ENV_INVALID_VALUE;\n}\n")
+ if len(self.env_stored):
+ buff.append("static void ha_reset_env(struct ha_monitor *ha_mon, "
+ f"enum envs{self.enum_suffix} env, u64 time_ns)\n{{")
+ _else = ""
+ for env in self.env_stored:
+ buff.append(f"\t{_else}if (env == {env}{self.enum_suffix})")
+ buff.append(f"\t\t{self.__fill_env_resetter(env)}")
+ _else = "else "
+ buff.append("}\n")
+ return buff
+
+ def _fill_hybrid_definitions(self) -> list[str]:
+ return self.__fill_hybrid_get_reset_functions() + self.__fill_constr_func()
+
+ def _fill_timer_type(self) -> list:
+ if self.invariants:
+ return [
+ "/* XXX: If the monitor has several instances, consider HA_TIMER_WHEEL */",
+ "#define HA_TIMER_TYPE HA_TIMER_HRTIMER"
+ ]
+ return []
diff --git a/tools/verification/rvgen/rvgen/generator.py b/tools/verification/rvgen/rvgen/generator.py
index 3441385c1177..b80af3fd6701 100644
--- a/tools/verification/rvgen/rvgen/generator.py
+++ b/tools/verification/rvgen/rvgen/generator.py
@@ -255,12 +255,14 @@ class Monitor(RVGenerator):
monitor_class_type = self.fill_monitor_class_type()
tracepoint_args_skel_event = self.fill_tracepoint_args_skel("event")
tracepoint_args_skel_error = self.fill_tracepoint_args_skel("error")
+ tracepoint_args_skel_error_env = self.fill_tracepoint_args_skel("error_env")
trace_h = trace_h.replace("%%MODEL_NAME%%", self.name)
trace_h = trace_h.replace("%%MODEL_NAME_UP%%", self.name.upper())
trace_h = trace_h.replace("%%MONITOR_CLASS%%", monitor_class)
trace_h = trace_h.replace("%%MONITOR_CLASS_TYPE%%", monitor_class_type)
trace_h = trace_h.replace("%%TRACEPOINT_ARGS_SKEL_EVENT%%", tracepoint_args_skel_event)
trace_h = trace_h.replace("%%TRACEPOINT_ARGS_SKEL_ERROR%%", tracepoint_args_skel_error)
+ trace_h = trace_h.replace("%%TRACEPOINT_ARGS_SKEL_ERROR_ENV%%", tracepoint_args_skel_error_env)
return trace_h
def print_files(self):
diff --git a/tools/verification/rvgen/rvgen/templates/dot2k/main.c b/tools/verification/rvgen/rvgen/templates/dot2k/main.c
index a14e4f0883db..bf0999f6657a 100644
--- a/tools/verification/rvgen/rvgen/templates/dot2k/main.c
+++ b/tools/verification/rvgen/rvgen/templates/dot2k/main.c
@@ -21,7 +21,7 @@
*/
#define RV_MON_TYPE RV_MON_%%MONITOR_TYPE%%
#include "%%MODEL_NAME%%.h"
-#include <rv/da_monitor.h>
+#include <rv/%%MONITOR_CLASS%%_monitor.h>
/*
* This is the instrumentation part of the monitor.
diff --git a/tools/verification/rvgen/rvgen/templates/dot2k/trace_hybrid.h b/tools/verification/rvgen/rvgen/templates/dot2k/trace_hybrid.h
new file mode 100644
index 000000000000..c8290e9ba2f4
--- /dev/null
+++ b/tools/verification/rvgen/rvgen/templates/dot2k/trace_hybrid.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Snippet to be included in rv_trace.h
+ */
+
+#ifdef CONFIG_RV_MON_%%MODEL_NAME_UP%%
+DEFINE_EVENT(event_%%MONITOR_CLASS%%, event_%%MODEL_NAME%%,
+%%TRACEPOINT_ARGS_SKEL_EVENT%%);
+
+DEFINE_EVENT(error_%%MONITOR_CLASS%%, error_%%MODEL_NAME%%,
+%%TRACEPOINT_ARGS_SKEL_ERROR%%);
+
+DEFINE_EVENT(error_env_%%MONITOR_CLASS%%, error_env_%%MODEL_NAME%%,
+%%TRACEPOINT_ARGS_SKEL_ERROR_ENV%%);
+#endif /* CONFIG_RV_MON_%%MODEL_NAME_UP%% */
--
2.53.0
^ permalink raw reply related
* [PATCH v6 03/16] verification/rvgen: Allow spaces in and events strings
From: Gabriele Monaco @ 2026-02-25 9:51 UTC (permalink / raw)
To: linux-kernel, Steven Rostedt, Nam Cao, Juri Lelli,
Gabriele Monaco, linux-trace-kernel
Cc: Tomas Glozar, Clark Williams, John Kacur
In-Reply-To: <20260225095122.80683-1-gmonaco@redhat.com>
Currently the automata parser assumes event strings don't have any
space, this stands true for event names, but can be a wrong assumption
if we want to store other information in the event strings (e.g.
constraints for hybrid automata).
Adapt the parser logic to allow spaces in the event strings.
Reviewed-by: Nam Cao <namcao@linutronix.de>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
Notes:
V6:
* Use f-strings in newly added code and cleanup
tools/verification/rvgen/rvgen/automata.py | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/tools/verification/rvgen/rvgen/automata.py b/tools/verification/rvgen/rvgen/automata.py
index 3f06aef8d4fd..34a2e2a6b217 100644
--- a/tools/verification/rvgen/rvgen/automata.py
+++ b/tools/verification/rvgen/rvgen/automata.py
@@ -127,14 +127,13 @@ class Automata:
# ------------ event is here ------------^^^^^
if self.__dot_lines[cursor].split()[1] == "->":
line = self.__dot_lines[cursor].split()
- event = line[-2].replace('"','')
+ event = "".join(line[line.index("label")+2:-1]).replace('"', '')
# when a transition has more than one lables, they are like this
# "local_irq_enable\nhw_local_irq_enable_n"
# so split them.
- event = event.replace("\\n", " ")
- for i in event.split():
+ for i in event.split("\\n"):
events.append(i)
cursor += 1
@@ -167,8 +166,8 @@ class Automata:
line = self.__dot_lines[cursor].split()
origin_state = line[0].replace('"','').replace(',','_')
dest_state = line[2].replace('"','').replace(',','_')
- possible_events = line[-2].replace('"','').replace("\\n", " ")
- for event in possible_events.split():
+ possible_events = "".join(line[line.index("label")+2:-1]).replace('"', '')
+ for event in possible_events.split("\\n"):
matrix[states_dict[origin_state]][events_dict[event]] = dest_state
cursor += 1
--
2.53.0
^ permalink raw reply related
* [PATCH v6 02/16] rv: Add Hybrid Automata monitor type
From: Gabriele Monaco @ 2026-02-25 9:51 UTC (permalink / raw)
To: linux-kernel, Steven Rostedt, Nam Cao, Juri Lelli,
Gabriele Monaco, Masami Hiramatsu, linux-trace-kernel
Cc: Tomas Glozar, Clark Williams, John Kacur
In-Reply-To: <20260225095122.80683-1-gmonaco@redhat.com>
Deterministic automata define which events are allowed in every state,
but cannot define more sophisticated constraint taking into account the
system's environment (e.g. time or other states not producing events).
Add the Hybrid Automata monitor type as an extension of Deterministic
automata where each state transition is validating a constraint on a
finite number of environment variables.
Hybrid automata can be used to implement timed automata, where the
environment variables are clocks.
Also implement the necessary functionality to handle clock constraints
(ns or jiffy granularity) on state and events.
Reviewed-by: Nam Cao <namcao@linutronix.de>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
Notes:
V5:
* Remove useless variable reset when resetting Hybrid Automata
V4:
* Improve ns to jiffy rounding in HA timers
V3:
* Improve functions naming for HA helpers
include/linux/rv.h | 38 +++
include/rv/da_monitor.h | 73 +++++-
include/rv/ha_monitor.h | 475 +++++++++++++++++++++++++++++++++++++
kernel/trace/rv/Kconfig | 13 +
kernel/trace/rv/rv_trace.h | 63 +++++
5 files changed, 658 insertions(+), 4 deletions(-)
create mode 100644 include/rv/ha_monitor.h
diff --git a/include/linux/rv.h b/include/linux/rv.h
index 58774eb3aecf..0aef9e3c785c 100644
--- a/include/linux/rv.h
+++ b/include/linux/rv.h
@@ -81,11 +81,49 @@ struct ltl_monitor {};
#endif /* CONFIG_RV_LTL_MONITOR */
+#ifdef CONFIG_RV_HA_MONITOR
+/*
+ * In the future, hybrid automata may rely on multiple
+ * environment variables, e.g. different clocks started at
+ * different times or running at different speed.
+ * For now we support only 1 variable.
+ */
+#define MAX_HA_ENV_LEN 1
+
+/*
+ * Monitors can pick the preferred timer implementation:
+ * No timer: if monitors don't have state invariants.
+ * Timer wheel: lightweight invariants check but far less precise.
+ * Hrtimer: accurate invariants check with higher overhead.
+ */
+#define HA_TIMER_NONE 0
+#define HA_TIMER_WHEEL 1
+#define HA_TIMER_HRTIMER 2
+
+/*
+ * Hybrid automaton per-object variables.
+ */
+struct ha_monitor {
+ struct da_monitor da_mon;
+ u64 env_store[MAX_HA_ENV_LEN];
+ union {
+ struct hrtimer hrtimer;
+ struct timer_list timer;
+ };
+};
+
+#else
+
+struct ha_monitor { };
+
+#endif /* CONFIG_RV_HA_MONITOR */
+
#define RV_PER_TASK_MONITOR_INIT (CONFIG_RV_PER_TASK_MONITORS)
union rv_task_monitor {
struct da_monitor da_mon;
struct ltl_monitor ltl_mon;
+ struct ha_monitor ha_mon;
};
#ifdef CONFIG_RV_REACTORS
diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index 89a0b81d4b3e..ab5fe0896a46 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -3,9 +3,9 @@
* Copyright (C) 2019-2022 Red Hat, Inc. Daniel Bristot de Oliveira <bristot@kernel.org>
*
* Deterministic automata (DA) monitor functions, to be used together
- * with automata models in C generated by the dot2k tool.
+ * with automata models in C generated by the rvgen tool.
*
- * The dot2k tool is available at tools/verification/dot2k/
+ * The rvgen tool is available at tools/verification/rvgen/
*
* For further information, see:
* Documentation/trace/rv/monitor_synthesis.rst
@@ -28,6 +28,33 @@
static struct rv_monitor rv_this;
+/*
+ * Hook to allow the implementation of hybrid automata: define it with a
+ * function that takes curr_state, event and next_state and returns true if the
+ * environment constraints (e.g. timing) are satisfied, false otherwise.
+ */
+#ifndef da_monitor_event_hook
+#define da_monitor_event_hook(...) true
+#endif
+
+/*
+ * Hook to allow the implementation of hybrid automata: define it with a
+ * function that takes the da_monitor and performs further initialisation
+ * (e.g. reset set up timers).
+ */
+#ifndef da_monitor_init_hook
+#define da_monitor_init_hook(da_mon)
+#endif
+
+/*
+ * Hook to allow the implementation of hybrid automata: define it with a
+ * function that takes the da_monitor and performs further reset (e.g. reset
+ * all clocks).
+ */
+#ifndef da_monitor_reset_hook
+#define da_monitor_reset_hook(da_mon)
+#endif
+
/*
* Type for the target id, default to int but can be overridden.
*/
@@ -49,6 +76,7 @@ static void react(enum states curr_state, enum events event)
*/
static inline void da_monitor_reset(struct da_monitor *da_mon)
{
+ da_monitor_reset_hook(da_mon);
da_mon->monitoring = 0;
da_mon->curr_state = model_get_initial_state();
}
@@ -63,6 +91,7 @@ static inline void da_monitor_start(struct da_monitor *da_mon)
{
da_mon->curr_state = model_get_initial_state();
da_mon->monitoring = 1;
+ da_monitor_init_hook(da_mon);
}
/*
@@ -142,7 +171,10 @@ static inline int da_monitor_init(void)
/*
* da_monitor_destroy - destroy the monitor
*/
-static inline void da_monitor_destroy(void) { }
+static inline void da_monitor_destroy(void)
+{
+ da_monitor_reset_all();
+}
#elif RV_MON_TYPE == RV_MON_PER_CPU
/*
@@ -188,7 +220,10 @@ static inline int da_monitor_init(void)
/*
* da_monitor_destroy - destroy the monitor
*/
-static inline void da_monitor_destroy(void) { }
+static inline void da_monitor_destroy(void)
+{
+ da_monitor_reset_all();
+}
#elif RV_MON_TYPE == RV_MON_PER_TASK
/*
@@ -209,6 +244,24 @@ static inline struct da_monitor *da_get_monitor(struct task_struct *tsk)
return &tsk->rv[task_mon_slot].da_mon;
}
+/*
+ * da_get_task - return the task associated to the monitor
+ */
+static inline struct task_struct *da_get_task(struct da_monitor *da_mon)
+{
+ return container_of(da_mon, struct task_struct, rv[task_mon_slot].da_mon);
+}
+
+/*
+ * da_get_id - return the id associated to the monitor
+ *
+ * For per-task monitors, the id is the task's PID.
+ */
+static inline da_id_type da_get_id(struct da_monitor *da_mon)
+{
+ return da_get_task(da_mon)->pid;
+}
+
static void da_monitor_reset_all(void)
{
struct task_struct *g, *p;
@@ -253,6 +306,8 @@ static inline void da_monitor_destroy(void)
}
rv_put_task_monitor_slot(task_mon_slot);
task_mon_slot = RV_PER_TASK_MONITOR_INIT;
+
+ da_monitor_reset_all();
}
#endif /* RV_MON_TYPE */
@@ -279,6 +334,14 @@ static inline void da_trace_error(struct da_monitor *da_mon,
CONCATENATE(trace_error_, MONITOR_NAME)(curr_state, event);
}
+/*
+ * da_get_id - unused for implicit monitors
+ */
+static inline da_id_type da_get_id(struct da_monitor *da_mon)
+{
+ return 0;
+}
+
#elif RV_MON_TYPE == RV_MON_PER_TASK
/*
* Trace events for per_task monitors, report the PID of the task.
@@ -323,6 +386,8 @@ static inline bool da_event(struct da_monitor *da_mon, enum events event, da_id_
return false;
}
if (likely(try_cmpxchg(&da_mon->curr_state, &curr_state, next_state))) {
+ if (!da_monitor_event_hook(da_mon, curr_state, event, next_state, id))
+ return false;
da_trace_event(da_mon, model_get_state_name(curr_state),
model_get_event_name(event),
model_get_state_name(next_state),
diff --git a/include/rv/ha_monitor.h b/include/rv/ha_monitor.h
new file mode 100644
index 000000000000..b6cf3b2ba989
--- /dev/null
+++ b/include/rv/ha_monitor.h
@@ -0,0 +1,475 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2025-2028 Red Hat, Inc. Gabriele Monaco <gmonaco@redhat.com>
+ *
+ * Hybrid automata (HA) monitor functions, to be used together
+ * with automata models in C generated by the rvgen tool.
+ *
+ * This type of monitors extends the Deterministic automata (DA) class by
+ * adding a set of environment variables (e.g. clocks) that can be used to
+ * constraint the valid transitions.
+ *
+ * The rvgen tool is available at tools/verification/rvgen/
+ *
+ * For further information, see:
+ * Documentation/trace/rv/monitor_synthesis.rst
+ */
+
+#ifndef _RV_HA_MONITOR_H
+#define _RV_HA_MONITOR_H
+
+#include <rv/automata.h>
+
+#ifndef da_id_type
+#define da_id_type int
+#endif
+
+static inline void ha_monitor_init_env(struct da_monitor *da_mon);
+static inline void ha_monitor_reset_env(struct da_monitor *da_mon);
+static inline void ha_setup_timer(struct ha_monitor *ha_mon);
+static inline bool ha_cancel_timer(struct ha_monitor *ha_mon);
+static bool ha_monitor_handle_constraint(struct da_monitor *da_mon,
+ enum states curr_state,
+ enum events event,
+ enum states next_state,
+ da_id_type id);
+#define da_monitor_event_hook ha_monitor_handle_constraint
+#define da_monitor_init_hook ha_monitor_init_env
+#define da_monitor_reset_hook ha_monitor_reset_env
+
+#include <rv/da_monitor.h>
+#include <linux/seq_buf.h>
+
+/* This simplifies things since da_mon and ha_mon coexist in the same union */
+_Static_assert(offsetof(struct ha_monitor, da_mon) == 0,
+ "da_mon must be the first element in an ha_mon!");
+#define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
+
+#define ENV_MAX CONCATENATE(env_max_, MONITOR_NAME)
+#define ENV_MAX_STORED CONCATENATE(env_max_stored_, MONITOR_NAME)
+#define envs CONCATENATE(envs_, MONITOR_NAME)
+
+/* Environment storage before being reset */
+#define ENV_INVALID_VALUE U64_MAX
+/* Error with no event occurs only on timeouts */
+#define EVENT_NONE EVENT_MAX
+#define EVENT_NONE_LBL "none"
+#define ENV_BUFFER_SIZE 64
+
+#ifdef CONFIG_RV_REACTORS
+
+/*
+ * ha_react - trigger the reaction after a failed environment constraint
+ *
+ * The transition from curr_state with event is otherwise valid, but the
+ * environment constraint is false. This function can be called also with no
+ * event from a timer (state constraints only).
+ */
+static void ha_react(enum states curr_state, enum events event, char *env)
+{
+ rv_react(&rv_this,
+ "rv: monitor %s does not allow event %s on state %s with env %s\n",
+ __stringify(MONITOR_NAME),
+ event == EVENT_NONE ? EVENT_NONE_LBL : model_get_event_name(event),
+ model_get_state_name(curr_state), env);
+}
+
+#else /* CONFIG_RV_REACTOR */
+
+static void ha_react(enum states curr_state, enum events event, char *env) { }
+#endif
+
+/*
+ * model_get_state_name - return the (string) name of the given state
+ */
+static char *model_get_env_name(enum envs env)
+{
+ if ((env < 0) || (env >= ENV_MAX))
+ return "INVALID";
+
+ return RV_AUTOMATON_NAME.env_names[env];
+}
+
+/*
+ * Monitors requiring a timer implementation need to request it explicitly.
+ */
+#ifndef HA_TIMER_TYPE
+#define HA_TIMER_TYPE HA_TIMER_NONE
+#endif
+
+#if HA_TIMER_TYPE == HA_TIMER_WHEEL
+static void ha_monitor_timer_callback(struct timer_list *timer);
+#elif HA_TIMER_TYPE == HA_TIMER_HRTIMER
+static enum hrtimer_restart ha_monitor_timer_callback(struct hrtimer *hrtimer);
+#endif
+
+/*
+ * ktime_get_ns is expensive, since we usually don't require precise accounting
+ * of changes within the same event, cache the current time at the beginning of
+ * the constraint handler and use the cache for subsequent calls.
+ * Monitors without ns clocks automatically skip this.
+ */
+#ifdef HA_CLK_NS
+#define ha_get_ns() ktime_get_ns()
+#else
+#define ha_get_ns() 0
+#endif /* HA_CLK_NS */
+
+/* Should be supplied by the monitor */
+static u64 ha_get_env(struct ha_monitor *ha_mon, enum envs env, u64 time_ns);
+static bool ha_verify_constraint(struct ha_monitor *ha_mon,
+ enum states curr_state,
+ enum events event,
+ enum states next_state,
+ u64 time_ns);
+
+/*
+ * ha_monitor_reset_all_stored - reset all environment variables in the monitor
+ */
+static inline void ha_monitor_reset_all_stored(struct ha_monitor *ha_mon)
+{
+ for (int i = 0; i < ENV_MAX_STORED; i++)
+ WRITE_ONCE(ha_mon->env_store[i], ENV_INVALID_VALUE);
+}
+
+/*
+ * ha_monitor_init_env - setup timer and reset all environment
+ *
+ * Called from a hook in the DA start functions, it supplies the da_mon
+ * corresponding to the current ha_mon.
+ * Not all hybrid automata require the timer, still set it for simplicity.
+ */
+static inline void ha_monitor_init_env(struct da_monitor *da_mon)
+{
+ struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
+
+ ha_monitor_reset_all_stored(ha_mon);
+ ha_setup_timer(ha_mon);
+}
+
+/*
+ * ha_monitor_reset_env - stop timer and reset all environment
+ *
+ * Called from a hook in the DA reset functions, it supplies the da_mon
+ * corresponding to the current ha_mon.
+ * Not all hybrid automata require the timer, still clear it for simplicity.
+ */
+static inline void ha_monitor_reset_env(struct da_monitor *da_mon)
+{
+ struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
+
+ /* Initialisation resets the monitor before initialising the timer */
+ if (likely(da_monitoring(da_mon)))
+ ha_cancel_timer(ha_mon);
+}
+
+/*
+ * ha_monitor_env_invalid - return true if env has not been initialised
+ */
+static inline bool ha_monitor_env_invalid(struct ha_monitor *ha_mon, enum envs env)
+{
+ return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
+}
+
+static inline void ha_get_env_string(struct seq_buf *s,
+ struct ha_monitor *ha_mon, u64 time_ns)
+{
+ const char *format_str = "%s=%llu";
+
+ for (int i = 0; i < ENV_MAX; i++) {
+ seq_buf_printf(s, format_str, model_get_env_name(i),
+ ha_get_env(ha_mon, i, time_ns));
+ format_str = ",%s=%llu";
+ }
+}
+
+#if RV_MON_TYPE == RV_MON_GLOBAL || RV_MON_TYPE == RV_MON_PER_CPU
+static inline void ha_trace_error_env(struct ha_monitor *ha_mon,
+ char *curr_state, char *event, char *env,
+ da_id_type id)
+{
+ CONCATENATE(trace_error_env_, MONITOR_NAME)(curr_state, event, env);
+}
+#elif RV_MON_TYPE == RV_MON_PER_TASK
+static inline void ha_trace_error_env(struct ha_monitor *ha_mon,
+ char *curr_state, char *event, char *env,
+ da_id_type id)
+{
+ CONCATENATE(trace_error_env_, MONITOR_NAME)(id, curr_state, event, env);
+}
+#endif /* RV_MON_TYPE */
+
+/*
+ * ha_get_monitor - return the current monitor
+ */
+#define ha_get_monitor(...) to_ha_monitor(da_get_monitor(__VA_ARGS__))
+
+/*
+ * ha_monitor_handle_constraint - handle the constraint on the current transition
+ *
+ * If the monitor implementation defines a constraint in the transition from
+ * curr_state to event, react and trace appropriately as well as return false.
+ * This function is called from the hook in the DA event handle function and
+ * triggers a failure in the monitor.
+ */
+static bool ha_monitor_handle_constraint(struct da_monitor *da_mon,
+ enum states curr_state,
+ enum events event,
+ enum states next_state,
+ da_id_type id)
+{
+ struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
+ u64 time_ns = ha_get_ns();
+ DECLARE_SEQ_BUF(env_string, ENV_BUFFER_SIZE);
+
+ if (ha_verify_constraint(ha_mon, curr_state, event, next_state, time_ns))
+ return true;
+
+ ha_get_env_string(&env_string, ha_mon, time_ns);
+ ha_react(curr_state, event, env_string.buffer);
+ ha_trace_error_env(ha_mon,
+ model_get_state_name(curr_state),
+ model_get_event_name(event),
+ env_string.buffer, id);
+ return false;
+}
+
+static inline void __ha_monitor_timer_callback(struct ha_monitor *ha_mon)
+{
+ enum states curr_state = READ_ONCE(ha_mon->da_mon.curr_state);
+ DECLARE_SEQ_BUF(env_string, ENV_BUFFER_SIZE);
+ u64 time_ns = ha_get_ns();
+
+ ha_get_env_string(&env_string, ha_mon, time_ns);
+ ha_react(curr_state, EVENT_NONE, env_string.buffer);
+ ha_trace_error_env(ha_mon, model_get_state_name(curr_state),
+ EVENT_NONE_LBL, env_string.buffer,
+ da_get_id(&ha_mon->da_mon));
+
+ da_monitor_reset(&ha_mon->da_mon);
+}
+
+/*
+ * The clock variables have 2 different representations in the env_store:
+ * - The guard representation is the timestamp of the last reset
+ * - The invariant representation is the timestamp when the invariant expires
+ * As the representations are incompatible, care must be taken when switching
+ * between them: the invariant representation can only be used when starting a
+ * timer when the previous representation was guard (e.g. no other invariant
+ * started since the last reset operation).
+ * Likewise, switching from invariant to guard representation without a reset
+ * can be done only by subtracting the exact value used to start the invariant.
+ *
+ * Reading the environment variable (ha_get_clk) also reflects this difference
+ * any reads in states that have an invariant return the (possibly negative)
+ * time since expiration, other reads return the time since last reset.
+ */
+
+/*
+ * Helper functions for env variables describing clocks with ns granularity
+ */
+static inline u64 ha_get_clk_ns(struct ha_monitor *ha_mon, enum envs env, u64 time_ns)
+{
+ return time_ns - READ_ONCE(ha_mon->env_store[env]);
+}
+static inline void ha_reset_clk_ns(struct ha_monitor *ha_mon, enum envs env, u64 time_ns)
+{
+ WRITE_ONCE(ha_mon->env_store[env], time_ns);
+}
+static inline void ha_set_invariant_ns(struct ha_monitor *ha_mon, enum envs env,
+ u64 value, u64 time_ns)
+{
+ WRITE_ONCE(ha_mon->env_store[env], time_ns + value);
+}
+static inline bool ha_check_invariant_ns(struct ha_monitor *ha_mon,
+ enum envs env, u64 time_ns)
+{
+ return READ_ONCE(ha_mon->env_store[env]) >= time_ns;
+}
+/*
+ * ha_invariant_passed_ns - prepare the invariant and return the time since reset
+ */
+static inline u64 ha_invariant_passed_ns(struct ha_monitor *ha_mon, enum envs env,
+ u64 expire, u64 time_ns)
+{
+ u64 passed = 0;
+
+ if (env < 0 || env >= ENV_MAX_STORED)
+ return 0;
+ if (ha_monitor_env_invalid(ha_mon, env))
+ return 0;
+ passed = ha_get_env(ha_mon, env, time_ns);
+ ha_set_invariant_ns(ha_mon, env, expire - passed, time_ns);
+ return passed;
+}
+
+/*
+ * Helper functions for env variables describing clocks with jiffy granularity
+ */
+static inline u64 ha_get_clk_jiffy(struct ha_monitor *ha_mon, enum envs env)
+{
+ return get_jiffies_64() - READ_ONCE(ha_mon->env_store[env]);
+}
+static inline void ha_reset_clk_jiffy(struct ha_monitor *ha_mon, enum envs env)
+{
+ WRITE_ONCE(ha_mon->env_store[env], get_jiffies_64());
+}
+static inline void ha_set_invariant_jiffy(struct ha_monitor *ha_mon,
+ enum envs env, u64 value)
+{
+ WRITE_ONCE(ha_mon->env_store[env], get_jiffies_64() + value);
+}
+static inline bool ha_check_invariant_jiffy(struct ha_monitor *ha_mon,
+ enum envs env, u64 time_ns)
+{
+ return time_after64(READ_ONCE(ha_mon->env_store[env]), get_jiffies_64());
+
+}
+/*
+ * ha_invariant_passed_jiffy - prepare the invariant and return the time since reset
+ */
+static inline u64 ha_invariant_passed_jiffy(struct ha_monitor *ha_mon, enum envs env,
+ u64 expire, u64 time_ns)
+{
+ u64 passed = 0;
+
+ if (env < 0 || env >= ENV_MAX_STORED)
+ return 0;
+ if (ha_monitor_env_invalid(ha_mon, env))
+ return 0;
+ passed = ha_get_env(ha_mon, env, time_ns);
+ ha_set_invariant_jiffy(ha_mon, env, expire - passed);
+ return passed;
+}
+
+/*
+ * Retrieve the last reset time (guard representation) from the invariant
+ * representation (expiration).
+ * It the caller's responsibility to make sure the storage was actually in the
+ * invariant representation (e.g. the current state has an invariant).
+ * The provided value must be the same used when starting the invariant.
+ *
+ * This function's access to the storage is NOT atomic, due to the rarity when
+ * this is used. If a monitor allows writes concurrent to this, likely
+ * other things are broken and need rethinking the model or additional locking.
+ */
+static inline void ha_inv_to_guard(struct ha_monitor *ha_mon, enum envs env,
+ u64 value, u64 time_ns)
+{
+ WRITE_ONCE(ha_mon->env_store[env], READ_ONCE(ha_mon->env_store[env]) - value);
+}
+
+#if HA_TIMER_TYPE == HA_TIMER_WHEEL
+/*
+ * Helper functions to handle the monitor timer.
+ * Not all monitors require a timer, in such case the timer will be set up but
+ * never armed.
+ * Timers start since the last reset of the supplied env or from now if env is
+ * not an environment variable. If env was not initialised no timer starts.
+ * Timers can expire on any CPU unless the monitor is per-cpu,
+ * where we assume every event occurs on the local CPU.
+ */
+static void ha_monitor_timer_callback(struct timer_list *timer)
+{
+ struct ha_monitor *ha_mon = container_of(timer, struct ha_monitor, timer);
+
+ __ha_monitor_timer_callback(ha_mon);
+}
+static inline void ha_setup_timer(struct ha_monitor *ha_mon)
+{
+ int mode = 0;
+
+ if (RV_MON_TYPE == RV_MON_PER_CPU)
+ mode |= TIMER_PINNED;
+ timer_setup(&ha_mon->timer, ha_monitor_timer_callback, mode);
+}
+static inline void ha_start_timer_jiffy(struct ha_monitor *ha_mon, enum envs env,
+ u64 expire, u64 time_ns)
+{
+ u64 passed = ha_invariant_passed_jiffy(ha_mon, env, expire, time_ns);
+
+ mod_timer(&ha_mon->timer, get_jiffies_64() + expire - passed);
+}
+static inline void ha_start_timer_ns(struct ha_monitor *ha_mon, enum envs env,
+ u64 expire, u64 time_ns)
+{
+ u64 passed = ha_invariant_passed_ns(ha_mon, env, expire, time_ns);
+
+ ha_start_timer_jiffy(ha_mon, ENV_MAX_STORED,
+ nsecs_to_jiffies(expire - passed + TICK_NSEC - 1), time_ns);
+}
+/*
+ * ha_cancel_timer - Cancel the timer
+ *
+ * Returns:
+ * * 1 when the timer was active
+ * * 0 when the timer was not active or running a callback
+ */
+static inline bool ha_cancel_timer(struct ha_monitor *ha_mon)
+{
+ return timer_delete(&ha_mon->timer);
+}
+#elif HA_TIMER_TYPE == HA_TIMER_HRTIMER
+/*
+ * Helper functions to handle the monitor timer.
+ * Not all monitors require a timer, in such case the timer will be set up but
+ * never armed.
+ * Timers start since the last reset of the supplied env or from now if env is
+ * not an environment variable. If env was not initialised no timer starts.
+ * Timers can expire on any CPU unless the monitor is per-cpu,
+ * where we assume every event occurs on the local CPU.
+ */
+static enum hrtimer_restart ha_monitor_timer_callback(struct hrtimer *hrtimer)
+{
+ struct ha_monitor *ha_mon = container_of(hrtimer, struct ha_monitor, hrtimer);
+
+ __ha_monitor_timer_callback(ha_mon);
+ return HRTIMER_NORESTART;
+}
+static inline void ha_setup_timer(struct ha_monitor *ha_mon)
+{
+ hrtimer_setup(&ha_mon->hrtimer, ha_monitor_timer_callback,
+ CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
+}
+static inline void ha_start_timer_ns(struct ha_monitor *ha_mon, enum envs env,
+ u64 expire, u64 time_ns)
+{
+ int mode = HRTIMER_MODE_REL_HARD;
+ u64 passed = ha_invariant_passed_ns(ha_mon, env, expire, time_ns);
+
+ if (RV_MON_TYPE == RV_MON_PER_CPU)
+ mode |= HRTIMER_MODE_PINNED;
+ hrtimer_start(&ha_mon->hrtimer, ns_to_ktime(expire - passed), mode);
+}
+static inline void ha_start_timer_jiffy(struct ha_monitor *ha_mon, enum envs env,
+ u64 expire, u64 time_ns)
+{
+ u64 passed = ha_invariant_passed_jiffy(ha_mon, env, expire, time_ns);
+
+ ha_start_timer_ns(ha_mon, ENV_MAX_STORED,
+ jiffies_to_nsecs(expire - passed), time_ns);
+}
+/*
+ * ha_cancel_timer - Cancel the timer
+ *
+ * Returns:
+ * * 1 when the timer was active
+ * * 0 when the timer was not active or running a callback
+ */
+static inline bool ha_cancel_timer(struct ha_monitor *ha_mon)
+{
+ return hrtimer_try_to_cancel(&ha_mon->hrtimer) == 1;
+}
+#else /* HA_TIMER_NONE */
+/*
+ * Start function is intentionally not defined, monitors using timers must
+ * set HA_TIMER_TYPE to either HA_TIMER_WHEEL or HA_TIMER_HRTIMER.
+ */
+static inline void ha_setup_timer(struct ha_monitor *ha_mon) { }
+static inline bool ha_cancel_timer(struct ha_monitor *ha_mon)
+{
+ return false;
+}
+#endif
+
+#endif
diff --git a/kernel/trace/rv/Kconfig b/kernel/trace/rv/Kconfig
index 5b4be87ba59d..4ad392dfc57f 100644
--- a/kernel/trace/rv/Kconfig
+++ b/kernel/trace/rv/Kconfig
@@ -23,6 +23,19 @@ config LTL_MON_EVENTS_ID
config RV_LTL_MONITOR
bool
+config RV_HA_MONITOR
+ bool
+
+config HA_MON_EVENTS_IMPLICIT
+ select DA_MON_EVENTS_IMPLICIT
+ select RV_HA_MONITOR
+ bool
+
+config HA_MON_EVENTS_ID
+ select DA_MON_EVENTS_ID
+ select RV_HA_MONITOR
+ bool
+
menuconfig RV
bool "Runtime Verification"
select TRACING
diff --git a/kernel/trace/rv/rv_trace.h b/kernel/trace/rv/rv_trace.h
index 4a6faddac614..7c598967bc0e 100644
--- a/kernel/trace/rv/rv_trace.h
+++ b/kernel/trace/rv/rv_trace.h
@@ -65,6 +65,36 @@ DECLARE_EVENT_CLASS(error_da_monitor,
#include <monitors/opid/opid_trace.h>
// Add new monitors based on CONFIG_DA_MON_EVENTS_IMPLICIT here
+#ifdef CONFIG_HA_MON_EVENTS_IMPLICIT
+/* For simplicity this class is marked as DA although relevant only for HA */
+DECLARE_EVENT_CLASS(error_env_da_monitor,
+
+ TP_PROTO(char *state, char *event, char *env),
+
+ TP_ARGS(state, event, env),
+
+ TP_STRUCT__entry(
+ __string( state, state )
+ __string( event, event )
+ __string( env, env )
+ ),
+
+ TP_fast_assign(
+ __assign_str(state);
+ __assign_str(event);
+ __assign_str(env);
+ ),
+
+ TP_printk("event %s not expected in the state %s with env %s",
+ __get_str(event),
+ __get_str(state),
+ __get_str(env))
+);
+
+// Add new monitors based on CONFIG_HA_MON_EVENTS_IMPLICIT here
+
+#endif
+
#endif /* CONFIG_DA_MON_EVENTS_IMPLICIT */
#ifdef CONFIG_DA_MON_EVENTS_ID
@@ -128,6 +158,39 @@ DECLARE_EVENT_CLASS(error_da_monitor_id,
#include <monitors/sssw/sssw_trace.h>
// Add new monitors based on CONFIG_DA_MON_EVENTS_ID here
+#ifdef CONFIG_HA_MON_EVENTS_ID
+/* For simplicity this class is marked as DA although relevant only for HA */
+DECLARE_EVENT_CLASS(error_env_da_monitor_id,
+
+ TP_PROTO(int id, char *state, char *event, char *env),
+
+ TP_ARGS(id, state, event, env),
+
+ TP_STRUCT__entry(
+ __field( int, id )
+ __string( state, state )
+ __string( event, event )
+ __string( env, env )
+ ),
+
+ TP_fast_assign(
+ __assign_str(state);
+ __assign_str(event);
+ __assign_str(env);
+ __entry->id = id;
+ ),
+
+ TP_printk("%d: event %s not expected in the state %s with env %s",
+ __entry->id,
+ __get_str(event),
+ __get_str(state),
+ __get_str(env))
+);
+
+// Add new monitors based on CONFIG_HA_MON_EVENTS_ID here
+
+#endif
+
#endif /* CONFIG_DA_MON_EVENTS_ID */
#ifdef CONFIG_LTL_MON_EVENTS_ID
DECLARE_EVENT_CLASS(event_ltl_monitor_id,
--
2.53.0
^ permalink raw reply related
* [PATCH v6 01/16] rv: Unify DA event handling functions across monitor types
From: Gabriele Monaco @ 2026-02-25 9:51 UTC (permalink / raw)
To: linux-kernel, Steven Rostedt, Nam Cao, Juri Lelli,
Gabriele Monaco, linux-trace-kernel
Cc: Tomas Glozar, Clark Williams, John Kacur
In-Reply-To: <20260225095122.80683-1-gmonaco@redhat.com>
The DA event handling functions are mostly duplicated because the
per-task monitors need to propagate the task struct while others do not.
Unify the functions, handle the difference by always passing an
identifier which is the task's pid for per-task monitors but is ignored
for the other types. Only keep the actual tracepoint calling separated.
Reviewed-by: Nam Cao <namcao@linutronix.de>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
Notes:
V3:
* Rearrange start/handle helpers to share more code
include/rv/da_monitor.h | 303 +++++++++++++++++-----------------------
1 file changed, 132 insertions(+), 171 deletions(-)
diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index 7511f5464c48..89a0b81d4b3e 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -28,6 +28,13 @@
static struct rv_monitor rv_this;
+/*
+ * Type for the target id, default to int but can be overridden.
+ */
+#ifndef da_id_type
+#define da_id_type int
+#endif
+
static void react(enum states curr_state, enum events event)
{
rv_react(&rv_this,
@@ -97,90 +104,6 @@ static inline bool da_monitor_handling_event(struct da_monitor *da_mon)
return 1;
}
-#if RV_MON_TYPE == RV_MON_GLOBAL || RV_MON_TYPE == RV_MON_PER_CPU
-/*
- * Event handler for implicit monitors. Implicit monitor is the one which the
- * handler does not need to specify which da_monitor to manipulate. Examples
- * of implicit monitor are the per_cpu or the global ones.
- *
- * Retry in case there is a race between getting and setting the next state,
- * warn and reset the monitor if it runs out of retries. The monitor should be
- * able to handle various orders.
- */
-
-static inline bool da_event(struct da_monitor *da_mon, enum events event)
-{
- enum states curr_state, next_state;
-
- curr_state = READ_ONCE(da_mon->curr_state);
- for (int i = 0; i < MAX_DA_RETRY_RACING_EVENTS; i++) {
- next_state = model_get_next_state(curr_state, event);
- if (next_state == INVALID_STATE) {
- react(curr_state, event);
- CONCATENATE(trace_error_, MONITOR_NAME)(
- model_get_state_name(curr_state),
- model_get_event_name(event));
- return false;
- }
- if (likely(try_cmpxchg(&da_mon->curr_state, &curr_state, next_state))) {
- CONCATENATE(trace_event_, MONITOR_NAME)(
- model_get_state_name(curr_state),
- model_get_event_name(event),
- model_get_state_name(next_state),
- model_is_final_state(next_state));
- return true;
- }
- }
-
- trace_rv_retries_error(__stringify(MONITOR_NAME), model_get_event_name(event));
- pr_warn("rv: " __stringify(MAX_DA_RETRY_RACING_EVENTS)
- " retries reached for event %s, resetting monitor %s",
- model_get_event_name(event), __stringify(MONITOR_NAME));
- return false;
-}
-
-#elif RV_MON_TYPE == RV_MON_PER_TASK
-/*
- * Event handler for per_task monitors.
- *
- * Retry in case there is a race between getting and setting the next state,
- * warn and reset the monitor if it runs out of retries. The monitor should be
- * able to handle various orders.
- */
-
-static inline bool da_event(struct da_monitor *da_mon, struct task_struct *tsk,
- enum events event)
-{
- enum states curr_state, next_state;
-
- curr_state = READ_ONCE(da_mon->curr_state);
- for (int i = 0; i < MAX_DA_RETRY_RACING_EVENTS; i++) {
- next_state = model_get_next_state(curr_state, event);
- if (next_state == INVALID_STATE) {
- react(curr_state, event);
- CONCATENATE(trace_error_, MONITOR_NAME)(tsk->pid,
- model_get_state_name(curr_state),
- model_get_event_name(event));
- return false;
- }
- if (likely(try_cmpxchg(&da_mon->curr_state, &curr_state, next_state))) {
- CONCATENATE(trace_event_, MONITOR_NAME)(tsk->pid,
- model_get_state_name(curr_state),
- model_get_event_name(event),
- model_get_state_name(next_state),
- model_is_final_state(next_state));
- return true;
- }
- }
-
- trace_rv_retries_error(__stringify(MONITOR_NAME), model_get_event_name(event));
- pr_warn("rv: " __stringify(MAX_DA_RETRY_RACING_EVENTS)
- " retries reached for event %s, resetting monitor %s",
- model_get_event_name(event), __stringify(MONITOR_NAME));
- return false;
-}
-#endif /* RV_MON_TYPE */
-
#if RV_MON_TYPE == RV_MON_GLOBAL
/*
* Functions to define, init and get a global monitor.
@@ -335,115 +258,179 @@ static inline void da_monitor_destroy(void)
#if RV_MON_TYPE == RV_MON_GLOBAL || RV_MON_TYPE == RV_MON_PER_CPU
/*
- * Handle event for implicit monitor: da_get_monitor() will figure out
- * the monitor.
+ * Trace events for implicit monitors. Implicit monitor is the one which the
+ * handler does not need to specify which da_monitor to manipulate. Examples
+ * of implicit monitor are the per_cpu or the global ones.
*/
-static inline void __da_handle_event(struct da_monitor *da_mon,
- enum events event)
+static inline void da_trace_event(struct da_monitor *da_mon,
+ char *curr_state, char *event,
+ char *next_state, bool is_final,
+ da_id_type id)
{
- bool retval;
+ CONCATENATE(trace_event_, MONITOR_NAME)(curr_state, event, next_state,
+ is_final);
+}
- retval = da_event(da_mon, event);
- if (!retval)
- da_monitor_reset(da_mon);
+static inline void da_trace_error(struct da_monitor *da_mon,
+ char *curr_state, char *event,
+ da_id_type id)
+{
+ CONCATENATE(trace_error_, MONITOR_NAME)(curr_state, event);
}
+#elif RV_MON_TYPE == RV_MON_PER_TASK
/*
- * da_handle_event - handle an event
+ * Trace events for per_task monitors, report the PID of the task.
*/
-static inline void da_handle_event(enum events event)
-{
- struct da_monitor *da_mon = da_get_monitor();
- bool retval;
- retval = da_monitor_handling_event(da_mon);
- if (!retval)
- return;
+static inline void da_trace_event(struct da_monitor *da_mon,
+ char *curr_state, char *event,
+ char *next_state, bool is_final,
+ da_id_type id)
+{
+ CONCATENATE(trace_event_, MONITOR_NAME)(id, curr_state, event,
+ next_state, is_final);
+}
- __da_handle_event(da_mon, event);
+static inline void da_trace_error(struct da_monitor *da_mon,
+ char *curr_state, char *event,
+ da_id_type id)
+{
+ CONCATENATE(trace_error_, MONITOR_NAME)(id, curr_state, event);
}
+#endif /* RV_MON_TYPE */
/*
- * da_handle_start_event - start monitoring or handle event
- *
- * This function is used to notify the monitor that the system is returning
- * to the initial state, so the monitor can start monitoring in the next event.
- * Thus:
+ * da_event - handle an event for the da_mon
*
- * If the monitor already started, handle the event.
- * If the monitor did not start yet, start the monitor but skip the event.
+ * This function is valid for both implicit and id monitors.
+ * Retry in case there is a race between getting and setting the next state,
+ * warn and reset the monitor if it runs out of retries. The monitor should be
+ * able to handle various orders.
*/
-static inline bool da_handle_start_event(enum events event)
+static inline bool da_event(struct da_monitor *da_mon, enum events event, da_id_type id)
{
- struct da_monitor *da_mon;
+ enum states curr_state, next_state;
- if (!da_monitor_enabled())
- return 0;
+ curr_state = READ_ONCE(da_mon->curr_state);
+ for (int i = 0; i < MAX_DA_RETRY_RACING_EVENTS; i++) {
+ next_state = model_get_next_state(curr_state, event);
+ if (next_state == INVALID_STATE) {
+ react(curr_state, event);
+ da_trace_error(da_mon, model_get_state_name(curr_state),
+ model_get_event_name(event), id);
+ return false;
+ }
+ if (likely(try_cmpxchg(&da_mon->curr_state, &curr_state, next_state))) {
+ da_trace_event(da_mon, model_get_state_name(curr_state),
+ model_get_event_name(event),
+ model_get_state_name(next_state),
+ model_is_final_state(next_state), id);
+ return true;
+ }
+ }
+
+ trace_rv_retries_error(__stringify(MONITOR_NAME), model_get_event_name(event));
+ pr_warn("rv: " __stringify(MAX_DA_RETRY_RACING_EVENTS)
+ " retries reached for event %s, resetting monitor %s",
+ model_get_event_name(event), __stringify(MONITOR_NAME));
+ return false;
+}
- da_mon = da_get_monitor();
+static inline void __da_handle_event_common(struct da_monitor *da_mon,
+ enum events event, da_id_type id)
+{
+ if (!da_event(da_mon, event, id))
+ da_monitor_reset(da_mon);
+}
+static inline void __da_handle_event(struct da_monitor *da_mon,
+ enum events event, da_id_type id)
+{
+ if (da_monitor_handling_event(da_mon))
+ __da_handle_event_common(da_mon, event, id);
+}
+
+static inline bool __da_handle_start_event(struct da_monitor *da_mon,
+ enum events event, da_id_type id)
+{
+ if (!da_monitor_enabled())
+ return 0;
if (unlikely(!da_monitoring(da_mon))) {
da_monitor_start(da_mon);
return 0;
}
- __da_handle_event(da_mon, event);
+ __da_handle_event_common(da_mon, event, id);
return 1;
}
-/*
- * da_handle_start_run_event - start monitoring and handle event
- *
- * This function is used to notify the monitor that the system is in the
- * initial state, so the monitor can start monitoring and handling event.
- */
-static inline bool da_handle_start_run_event(enum events event)
+static inline bool __da_handle_start_run_event(struct da_monitor *da_mon,
+ enum events event, da_id_type id)
{
- struct da_monitor *da_mon;
-
if (!da_monitor_enabled())
return 0;
-
- da_mon = da_get_monitor();
-
if (unlikely(!da_monitoring(da_mon)))
da_monitor_start(da_mon);
- __da_handle_event(da_mon, event);
+ __da_handle_event_common(da_mon, event, id);
return 1;
}
-#elif RV_MON_TYPE == RV_MON_PER_TASK
+#if RV_MON_TYPE == RV_MON_GLOBAL || RV_MON_TYPE == RV_MON_PER_CPU
/*
- * Handle event for per task.
+ * Handle event for implicit monitor: da_get_monitor() will figure out
+ * the monitor.
*/
-static inline void __da_handle_event(struct da_monitor *da_mon,
- struct task_struct *tsk, enum events event)
+/*
+ * da_handle_event - handle an event
+ */
+static inline void da_handle_event(enum events event)
{
- bool retval;
+ __da_handle_event(da_get_monitor(), event, 0);
+}
- retval = da_event(da_mon, tsk, event);
- if (!retval)
- da_monitor_reset(da_mon);
+/*
+ * da_handle_start_event - start monitoring or handle event
+ *
+ * This function is used to notify the monitor that the system is returning
+ * to the initial state, so the monitor can start monitoring in the next event.
+ * Thus:
+ *
+ * If the monitor already started, handle the event.
+ * If the monitor did not start yet, start the monitor but skip the event.
+ */
+static inline bool da_handle_start_event(enum events event)
+{
+ return __da_handle_start_event(da_get_monitor(), event, 0);
}
/*
- * da_handle_event - handle an event
+ * da_handle_start_run_event - start monitoring and handle event
+ *
+ * This function is used to notify the monitor that the system is in the
+ * initial state, so the monitor can start monitoring and handling event.
*/
-static inline void da_handle_event(struct task_struct *tsk, enum events event)
+static inline bool da_handle_start_run_event(enum events event)
{
- struct da_monitor *da_mon = da_get_monitor(tsk);
- bool retval;
+ return __da_handle_start_run_event(da_get_monitor(), event, 0);
+}
- retval = da_monitor_handling_event(da_mon);
- if (!retval)
- return;
+#elif RV_MON_TYPE == RV_MON_PER_TASK
+/*
+ * Handle event for per task.
+ */
- __da_handle_event(da_mon, tsk, event);
+/*
+ * da_handle_event - handle an event
+ */
+static inline void da_handle_event(struct task_struct *tsk, enum events event)
+{
+ __da_handle_event(da_get_monitor(tsk), event, tsk->pid);
}
/*
@@ -459,21 +446,7 @@ static inline void da_handle_event(struct task_struct *tsk, enum events event)
static inline bool da_handle_start_event(struct task_struct *tsk,
enum events event)
{
- struct da_monitor *da_mon;
-
- if (!da_monitor_enabled())
- return 0;
-
- da_mon = da_get_monitor(tsk);
-
- if (unlikely(!da_monitoring(da_mon))) {
- da_monitor_start(da_mon);
- return 0;
- }
-
- __da_handle_event(da_mon, tsk, event);
-
- return 1;
+ return __da_handle_start_event(da_get_monitor(tsk), event, tsk->pid);
}
/*
@@ -485,19 +458,7 @@ static inline bool da_handle_start_event(struct task_struct *tsk,
static inline bool da_handle_start_run_event(struct task_struct *tsk,
enum events event)
{
- struct da_monitor *da_mon;
-
- if (!da_monitor_enabled())
- return 0;
-
- da_mon = da_get_monitor(tsk);
-
- if (unlikely(!da_monitoring(da_mon)))
- da_monitor_start(da_mon);
-
- __da_handle_event(da_mon, tsk, event);
-
- return 1;
+ return __da_handle_start_run_event(da_get_monitor(tsk), event, tsk->pid);
}
#endif /* RV_MON_TYPE */
--
2.53.0
^ permalink raw reply related
* [PATCH v6 00/16] rv: Add Hybrid Automata monitor type, per-object and deadline monitors
From: Gabriele Monaco @ 2026-02-25 9:51 UTC (permalink / raw)
To: linux-kernel, Steven Rostedt, Nam Cao, Juri Lelli
Cc: Gabriele Monaco, Tomas Glozar, Clark Williams, John Kacur,
linux-trace-kernel
This series contains several related changes, the main areas are:
* hybrid automata
Hybrid automata are an extension of deterministic automata where each
state transition is validating a constraint on a finite number of
environment variables.
Hybrid automata can be used to implement timed automata, where the
environment variables are clocks.
* per-object monitors
Define the generic per-object monitor allow RV monitors on any kind
of object where the user can specify how to get an id (e.g. pid for
tasks) and the data type for the monitor_target (e.g. struct
task_struct * for tasks).
The monitor storage (e.g. the rv monitor, pointer to the target, etc.)
is stored in a hash table indexed by id.
* deadline monitors collection
Add the throttle and nomiss monitors to validate timing aspects of
the deadline scheduler, as they work for tasks and servers, their
inclusion requires also per-object monitors (for dl entities).
Also add the boost and laxity monitors specific to servers.
The entire series can also be found on:
git.kernel.org/pub/scm/linux/kernel/git/gmonaco/linux.git rv_hybrid_automata
Changes since V5 [1]:
* Export task_is_scx_enabled() for monitors code to determine if tasks are ext
* tracepoints:
- Add dl_se type to differentiate between fair and ext servers
- Add event to track dl_update_curr not firing other events
* monitors:
- Add support for ext server
- Remove mostly unused dl_server_start from nomiss
- Allow to skip runtime constraint on throttle monitor
- Consider also BATCH and IDLE as fair policies
- Handle events only for supported deadline entities (future proof)
- Add missing transition running -> zero_laxity_wait
- Handle dl_update event in laxity to cover missing update without
enqueue nor replenish when multiple server types are active
- Cleanup and sort constraints as generated
* rvgen:
- Use f-strings in newly added code and cleanup
- Sort constraints for predictable generated code
Changes since V4 [2]:
* Do not fire enqueue tracepoint for delayed enqueues
* Do not use boosted dl_se in monitors
* Add enqueue/dequeue validation on snroc model
* Do not export pi_of to deadline.h
* Remove useless variable reset when resetting Hybrid Automata
Changes since V3 [3]:
* Improve ns to jiffy rounding in HA timers
* Use da_handle_start_run_event not to lose the first event in opid
* Sort self_loop_reset_events in rvgen to avoid unpredictable order
* Add enqueue/dequeue tracepoints (Nam Cao)
* Rename handle_syscall as it collides with some UM function
* Simplify idle handling on nomiss and throttle from sleeping
* Improve switch_out for servers in throttle
* Rely on enqueue/dequeue tracepoints instead of syscalls
* Improve timing conditions in laxity and handle resume action
* Remove fragile Stopping state from boost
[1] - https://lore.kernel.org/lkml/20260122155500.362683-1-gmonaco@redhat.com
[2] - https://lore.kernel.org/lkml/20260116123911.130300-1-gmonaco@redhat.com
[3] - https://lore.kernel.org/lkml/20251205131621.135513-1-gmonaco@redhat.com
To: Steven Rostedt <rostedt@goodmis.org>
To: Nam Cao <namcao@linutronix.de>
To: Juri Lelli <jlelli@redhat.com>
Cc: Tomas Glozar <tglozar@redhat.com>
Cc: Clark Williams <williams@redhat.com>
Cc: John Kacur <jkacur@redhat.com>
Cc: linux-trace-kernel@vger.kernel.org
Gabriele Monaco (15):
rv: Unify DA event handling functions across monitor types
rv: Add Hybrid Automata monitor type
verification/rvgen: Allow spaces in and events strings
verification/rvgen: Add support for Hybrid Automata
Documentation/rv: Add documentation about hybrid automata
rv: Add sample hybrid monitors stall
rv: Convert the opid monitor to a hybrid automaton
rv: Add enqueue/dequeue to snroc monitor
rv: Add support for per-object monitors in DA/HA
verification/rvgen: Add support for per-obj monitors
sched: Add deadline tracepoints
sched/deadline: Move some utility functions to deadline.h
sched_ext: Export task_is_scx_enabled() for verification
rv: Add deadline monitors
rv: Add dl_server specific monitors
Nam Cao (1):
sched: Add task enqueue/dequeue trace points
Documentation/tools/rv/index.rst | 1 +
Documentation/tools/rv/rv-mon-stall.rst | 44 ++
.../trace/rv/deterministic_automata.rst | 2 +-
Documentation/trace/rv/hybrid_automata.rst | 341 ++++++++++
Documentation/trace/rv/index.rst | 3 +
Documentation/trace/rv/monitor_deadline.rst | 278 ++++++++
Documentation/trace/rv/monitor_sched.rst | 101 ++-
Documentation/trace/rv/monitor_stall.rst | 43 ++
Documentation/trace/rv/monitor_synthesis.rst | 117 +++-
include/linux/rv.h | 39 ++
include/linux/sched/deadline.h | 29 +
include/linux/sched/ext.h | 2 +
include/rv/da_monitor.h | 644 +++++++++++++-----
include/rv/ha_monitor.h | 478 +++++++++++++
include/trace/events/sched.h | 34 +
kernel/sched/core.c | 16 +-
kernel/sched/deadline.c | 53 +-
kernel/sched/ext.c | 8 +
kernel/sched/sched.h | 2 +
kernel/trace/rv/Kconfig | 21 +
kernel/trace/rv/Makefile | 6 +
kernel/trace/rv/monitors/boost/Kconfig | 15 +
kernel/trace/rv/monitors/boost/boost.c | 258 +++++++
kernel/trace/rv/monitors/boost/boost.h | 146 ++++
kernel/trace/rv/monitors/boost/boost_trace.h | 19 +
kernel/trace/rv/monitors/deadline/Kconfig | 10 +
kernel/trace/rv/monitors/deadline/deadline.c | 35 +
kernel/trace/rv/monitors/deadline/deadline.h | 206 ++++++
kernel/trace/rv/monitors/laxity/Kconfig | 14 +
kernel/trace/rv/monitors/laxity/laxity.c | 279 ++++++++
kernel/trace/rv/monitors/laxity/laxity.h | 140 ++++
.../trace/rv/monitors/laxity/laxity_trace.h | 19 +
kernel/trace/rv/monitors/nomiss/Kconfig | 15 +
kernel/trace/rv/monitors/nomiss/nomiss.c | 287 ++++++++
kernel/trace/rv/monitors/nomiss/nomiss.h | 123 ++++
.../trace/rv/monitors/nomiss/nomiss_trace.h | 19 +
kernel/trace/rv/monitors/opid/Kconfig | 11 +-
kernel/trace/rv/monitors/opid/opid.c | 111 +--
kernel/trace/rv/monitors/opid/opid.h | 86 +--
kernel/trace/rv/monitors/opid/opid_trace.h | 4 +
kernel/trace/rv/monitors/snroc/snroc.c | 18 +-
kernel/trace/rv/monitors/snroc/snroc.h | 46 +-
kernel/trace/rv/monitors/stall/Kconfig | 13 +
kernel/trace/rv/monitors/stall/stall.c | 150 ++++
kernel/trace/rv/monitors/stall/stall.h | 81 +++
kernel/trace/rv/monitors/stall/stall_trace.h | 19 +
kernel/trace/rv/monitors/throttle/Kconfig | 15 +
kernel/trace/rv/monitors/throttle/throttle.c | 279 ++++++++
kernel/trace/rv/monitors/throttle/throttle.h | 116 ++++
.../rv/monitors/throttle/throttle_trace.h | 19 +
kernel/trace/rv/rv_trace.h | 70 +-
tools/verification/models/deadline/boost.dot | 48 ++
tools/verification/models/deadline/laxity.dot | 37 +
tools/verification/models/deadline/nomiss.dot | 41 ++
.../verification/models/deadline/throttle.dot | 44 ++
tools/verification/models/sched/opid.dot | 36 +-
tools/verification/models/sched/snroc.dot | 30 +-
tools/verification/models/stall.dot | 22 +
tools/verification/rvgen/__main__.py | 8 +-
tools/verification/rvgen/rvgen/automata.py | 151 +++-
tools/verification/rvgen/rvgen/dot2c.py | 47 ++
tools/verification/rvgen/rvgen/dot2k.py | 489 ++++++++++++-
tools/verification/rvgen/rvgen/generator.py | 4 +-
.../rvgen/rvgen/templates/dot2k/main.c | 2 +-
.../rvgen/templates/dot2k/trace_hybrid.h | 16 +
65 files changed, 5379 insertions(+), 481 deletions(-)
create mode 100644 Documentation/tools/rv/rv-mon-stall.rst
create mode 100644 Documentation/trace/rv/hybrid_automata.rst
create mode 100644 Documentation/trace/rv/monitor_deadline.rst
create mode 100644 Documentation/trace/rv/monitor_stall.rst
create mode 100644 include/rv/ha_monitor.h
create mode 100644 kernel/trace/rv/monitors/boost/Kconfig
create mode 100644 kernel/trace/rv/monitors/boost/boost.c
create mode 100644 kernel/trace/rv/monitors/boost/boost.h
create mode 100644 kernel/trace/rv/monitors/boost/boost_trace.h
create mode 100644 kernel/trace/rv/monitors/deadline/Kconfig
create mode 100644 kernel/trace/rv/monitors/deadline/deadline.c
create mode 100644 kernel/trace/rv/monitors/deadline/deadline.h
create mode 100644 kernel/trace/rv/monitors/laxity/Kconfig
create mode 100644 kernel/trace/rv/monitors/laxity/laxity.c
create mode 100644 kernel/trace/rv/monitors/laxity/laxity.h
create mode 100644 kernel/trace/rv/monitors/laxity/laxity_trace.h
create mode 100644 kernel/trace/rv/monitors/nomiss/Kconfig
create mode 100644 kernel/trace/rv/monitors/nomiss/nomiss.c
create mode 100644 kernel/trace/rv/monitors/nomiss/nomiss.h
create mode 100644 kernel/trace/rv/monitors/nomiss/nomiss_trace.h
create mode 100644 kernel/trace/rv/monitors/stall/Kconfig
create mode 100644 kernel/trace/rv/monitors/stall/stall.c
create mode 100644 kernel/trace/rv/monitors/stall/stall.h
create mode 100644 kernel/trace/rv/monitors/stall/stall_trace.h
create mode 100644 kernel/trace/rv/monitors/throttle/Kconfig
create mode 100644 kernel/trace/rv/monitors/throttle/throttle.c
create mode 100644 kernel/trace/rv/monitors/throttle/throttle.h
create mode 100644 kernel/trace/rv/monitors/throttle/throttle_trace.h
create mode 100644 tools/verification/models/deadline/boost.dot
create mode 100644 tools/verification/models/deadline/laxity.dot
create mode 100644 tools/verification/models/deadline/nomiss.dot
create mode 100644 tools/verification/models/deadline/throttle.dot
create mode 100644 tools/verification/models/stall.dot
create mode 100644 tools/verification/rvgen/rvgen/templates/dot2k/trace_hybrid.h
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
--
2.53.0
^ permalink raw reply
* [PATCH] kernel/trace/ftrace: introduce ftrace module notifier
From: chensong_2000 @ 2026-02-25 5:46 UTC (permalink / raw)
To: mcgrof, petr.pavlu, da.gomez, samitolvanen, atomlin, rostedt,
mhiramat, mark.rutland, mathieu.desnoyers
Cc: linux-modules, linux-kernel, linux-trace-kernel, Song Chen
From: Song Chen <chensong_2000@189.cn>
Like kprobe, fprobe and btf, this patch attempts to introduce
a notifier_block for ftrace to decouple its initialization from
load_module.
Below is the table of ftrace fucntions calls in different
module state:
MODULE_STATE_UNFORMED ftrace_module_init
MODULE_STATE_COMING ftrace_module_enable
MODULE_STATE_LIVE ftrace_free_mem
MODULE_STATE_GOING ftrace_release_mod
Unlike others, ftrace module notifier must take care of state
MODULE_STATE_UNFORMED to ensure calling ftrace_module_init
before complete_formation which changes module's text property.
That pretty much remains same logic with its original design,
the only thing that changes is blocking_notifier_call_chain
(MODULE_STATE_GOING) has to be moved from coming_cleanup to
ddebug_cleanup in function load_module to ensure
ftrace_release_mod is invoked in case complete_formation fails.
Signed-off-by: Song Chen <chensong_2000@189.cn>
---
kernel/module/main.c | 14 ++++----------
kernel/trace/ftrace.c | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 710ee30b3bea..5dc0a980e9bd 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -45,7 +45,6 @@
#include <linux/license.h>
#include <asm/sections.h>
#include <linux/tracepoint.h>
-#include <linux/ftrace.h>
#include <linux/livepatch.h>
#include <linux/async.h>
#include <linux/percpu.h>
@@ -836,7 +835,6 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
blocking_notifier_call_chain(&module_notify_list,
MODULE_STATE_GOING, mod);
klp_module_going(mod);
- ftrace_release_mod(mod);
async_synchronize_full();
@@ -3067,8 +3065,6 @@ static noinline int do_init_module(struct module *mod)
if (!mod->async_probe_requested)
async_synchronize_full();
- ftrace_free_mem(mod, mod->mem[MOD_INIT_TEXT].base,
- mod->mem[MOD_INIT_TEXT].base + mod->mem[MOD_INIT_TEXT].size);
mutex_lock(&module_mutex);
/* Drop initial reference. */
module_put(mod);
@@ -3131,7 +3127,6 @@ static noinline int do_init_module(struct module *mod)
blocking_notifier_call_chain(&module_notify_list,
MODULE_STATE_GOING, mod);
klp_module_going(mod);
- ftrace_release_mod(mod);
free_module(mod);
wake_up_all(&module_wq);
@@ -3278,7 +3273,6 @@ static int prepare_coming_module(struct module *mod)
{
int err;
- ftrace_module_enable(mod);
err = klp_module_coming(mod);
if (err)
return err;
@@ -3461,7 +3455,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
init_build_id(mod, info);
/* Ftrace init must be called in the MODULE_STATE_UNFORMED state */
- ftrace_module_init(mod);
+ blocking_notifier_call_chain(&module_notify_list,
+ MODULE_STATE_UNFORMED, mod);
/* Finally it's fully formed, ready to start executing. */
err = complete_formation(mod, info);
@@ -3513,8 +3508,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
coming_cleanup:
mod->state = MODULE_STATE_GOING;
destroy_params(mod->kp, mod->num_kp);
- blocking_notifier_call_chain(&module_notify_list,
- MODULE_STATE_GOING, mod);
klp_module_going(mod);
bug_cleanup:
mod->state = MODULE_STATE_GOING;
@@ -3524,7 +3517,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
mutex_unlock(&module_mutex);
ddebug_cleanup:
- ftrace_release_mod(mod);
+ blocking_notifier_call_chain(&module_notify_list,
+ MODULE_STATE_GOING, mod);
synchronize_rcu();
kfree(mod->args);
free_arch_cleanup:
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 3ec2033c0774..47c74d4a2425 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5223,6 +5223,43 @@ static int __init ftrace_mod_cmd_init(void)
}
core_initcall(ftrace_mod_cmd_init);
+static int ftrace_module_callback(struct notifier_block *nb, unsigned long op,
+ void *module)
+{
+ struct module *mod = module;
+
+ switch (op) {
+ case MODULE_STATE_UNFORMED:
+ ftrace_module_init(mod);
+ break;
+ case MODULE_STATE_COMING:
+ ftrace_module_enable(mod);
+ break;
+ case MODULE_STATE_LIVE:
+ ftrace_free_mem(mod, mod->mem[MOD_INIT_TEXT].base,
+ mod->mem[MOD_INIT_TEXT].base + mod->mem[MOD_INIT_TEXT].size);
+ break;
+ case MODULE_STATE_GOING:
+ ftrace_release_mod(mod);
+ break;
+ default:
+ break;
+ }
+
+ return notifier_from_errno(0);
+}
+
+static struct notifier_block ftrace_module_nb = {
+ .notifier_call = ftrace_module_callback,
+ .priority = 0
+};
+
+static int __init ftrace_register_module_notifier(void)
+{
+ return register_module_notifier(&ftrace_module_nb);
+}
+core_initcall(ftrace_register_module_notifier);
+
static void function_trace_probe_call(unsigned long ip, unsigned long parent_ip,
struct ftrace_ops *op, struct ftrace_regs *fregs)
{
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v4 3/3] PCI: dw-rockchip: Add pcie_ltssm_state_transition trace support
From: Shawn Lin @ 2026-02-25 1:25 UTC (permalink / raw)
To: Steven Rostedt
Cc: shawn.lin, Manivannan Sadhasivam, Bjorn Helgaas, linux-rockchip,
linux-pci, linux-trace-kernel, linux-doc, Masami Hiramatsu
In-Reply-To: <20260224091601.48a7b3c0@fedora>
Hi Steven,
在 2026/02/24 星期二 22:16, Steven Rostedt 写道:
> On Tue, 24 Feb 2026 09:11:15 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
>>> +#ifdef CONFIG_TRACING
>>> +static void rockchip_pcie_ltssm_trace_work(struct work_struct *work)
>>> +{
>>> + struct rockchip_pcie *rockchip = container_of(work,
>>> + struct rockchip_pcie,
>>> + trace_work.work);
>>> + struct dw_pcie *pci = &rockchip->pci;
>>> + enum dw_pcie_ltssm state;
>>> + u32 i, l1ss, prev_val = DW_PCIE_LTSSM_UNKNOWN, rate, val;
>>> +
>>> + if (!pci_ltssm_tp_enabled())
>>> + goto skip_trace;
>>
>> You can use:
>>
>> if (!trace_pcie_ltssm_state_transition_enabled())
>> goto skip_trace;
>>
>> The above is a static branch. That means when tracing is disabled, it is
>> basically:
>>
>> goto skip_trace;
>>
>> and when tracing is enabled it is a nop.
I must admit I borrow it from arch/powerpc/include/asm/trace.h and
include/trace/events/i2c.h for reference, where the reg and unreg
just increase and decrease the ref count to indicate if the tp
should be continued. Sure, the static branch could be used instead,
even without reg and unreg implementation.
>>
..
>>> +}
>>> +
>
> Hmm, so basically you only want to call the work when tracing is
> enabled? That's what I was thinking should be enabled by the reg and
> unreg functions. That is, the reg should enabled the delayed work, and
> the unreg should disable it from being called.
>
> This looks like it calls the work regardless of if tracing is enabled
> or not. Why waste the cycles when tracing is disabled?
I looked into implementing reg and unreg callbacks to directly schedule
and cancel the delayed work. The challenge is that this tracepoint
belongs to the shared PCI subsystem trace hierarchy, while the polling
work itself is per-controller. I haven't found a clean way to register
per-driver callbacks in this common context.
Creating a separate Rockchip-specific tracepoint via
tracepoint_probe_register() would detach it from the standard PCIe trace
event hierarchy, which seems undesirable.
As a practical middle ground, I implement reg and unreg to maintain a
user count like this v4. All drivers using this tracepoint would then
rely on the count to gate their work execution, making the delayed work
essentially a no-op when tracing is disabled.
Alternatively, we could simply revert to the V3 approach and rely
entirely on the trace_pcie_ltssm_state_transition_enabled() static
branch check, which would remove the need for reg and unreg altogether.
If you have better suggestions or can point me to a preferred pattern
for this, I'd appreciate your advice.
>
> -- Steve
>
^ permalink raw reply
* Re: [PATCH bpf-next 02/17] bpf: Use mutex lock pool for bpf trampolines
From: Alexei Starovoitov @ 2026-02-24 17:13 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
linux-trace-kernel, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, Menglong Dong, Steven Rostedt
In-Reply-To: <aZ2ZQnBHPz3UO1wr@krava>
On Tue, Feb 24, 2026 at 4:27 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> nope, the rfc had workaround for lockdep ;-)
>
> +#ifdef CONFIG_LOCKDEP
> + mutex_init_with_key(&tr->mutex, &__lockdep_no_track__);
> +#else
The only user of lockdep_set_notrack_class() was removed from the tree.
We shouldn't introduce new special cases.
> but I overlooked lockdep config for this version
>
> > MAX_LOCK_DEPTH is indeed 48.
> >
> > See fs/configfs/inode.c and default_group_class.
> > It does:
> > lockdep_set_class(&inode->i_rwsem,
> > &default_group_class[depth - 1]);
> >
> > the idea here is that the number of lockdep keys doesn't have
> > to be equal to the actual number of mutexes.
>
> I see, thanks for the pointer
>
> >
> > I guess we can keep a total of 32 mutexes to avoid making it too fancy.
> > Please add a comment explaining 32 and why it needs lockdep_key.
>
> ok
>
> >
> > I thought declaring all mutexes as static will avoid the need for the key,
> > but DEFINE_MUTEX doesn't support an array.
> > So since we need a loop anyway to init mutex and the key,
> > let's keep kmalloc_array() above. Which is now renamed to kmalloc_objs()
> > after 7.0-rc1.
>
> I don't mind either way, meanwhile I used this version:
>
> static struct {
> struct mutex mutex;
> struct lock_class_key key;
> } trampoline_locks[TRAMPOLINE_LOCKS_TABLE_SIZE];
>
> for (i = 0; i < TRAMPOLINE_LOCKS_TABLE_SIZE; i++)
> __mutex_init(&trampoline_locks[i].mutex, "trampoline_lock", &trampoline_locks[i].key);
works for me.
^ permalink raw reply
* Re: [LSF/MM/BPF TOPIC][RFC PATCH v4 00/27] Private Memory Nodes (w/ Compressed RAM)
From: Gregory Price @ 2026-02-24 16:54 UTC (permalink / raw)
To: Alistair Popple
Cc: lsf-pc, linux-kernel, linux-cxl, cgroups, linux-mm,
linux-trace-kernel, damon, kernel-team, gregkh, rafael, dakr,
dave, jonathan.cameron, dave.jiang, alison.schofield,
vishal.l.verma, ira.weiny, dan.j.williams, longman, akpm, david,
lorenzo.stoakes, Liam.Howlett, vbabka, rppt, surenb, mhocko,
osalvador, ziy, matthew.brost, joshua.hahnjy, rakie.kim,
byungchul, ying.huang, axelrasmussen, yuanchu, weixugc,
yury.norov, linux, mhiramat, mathieu.desnoyers, tj, hannes,
mkoutny, jackmanb, sj, baolin.wang, npache, ryan.roberts,
dev.jain, baohua, lance.yang, muchun.song, xu.xin16,
chengming.zhou, jannh, linmiaohe, nao.horiguchi, pfalcato,
rientjes, shakeel.butt, riel, harry.yoo, cl, roman.gushchin,
chrisl, kasong, shikemeng, nphamcs, bhe, zhengqi.arch,
terry.bowman
In-Reply-To: <aZ3BEn_73Rk8Fn7L@gourry-fedora-PF4VCD3F>
On Tue, Feb 24, 2026 at 10:17:38AM -0500, Gregory Price wrote:
> - Changing validations on node states
> - mempolicy checks N_MEMORY membership, so you have to hack
> N_MEMORY onto ZONE_DEVICE
> (or teach it about a new node state... N_MEMORY_PRIVATE)
>
This gave me something to chew on
I think this can be done without introducing N_MEMORY_PRIVATE and just
checking: NODE_DATA(target_nid)->private
meaning these nodes can just be N_MEMORY with the same isolations.
I'll look at this a bit more.
~Gregory
^ permalink raw reply
* Re: [PATCH 3/4] mm: convert compaction to zone lock wrappers
From: Dmitry Ilvokhin @ 2026-02-24 15:50 UTC (permalink / raw)
To: Cheatham, Benjamin
Cc: linux-kernel, linux-mm, linux-trace-kernel, linux-cxl,
kernel-team, Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Brendan Jackman,
Johannes Weiner, Zi Yan, Oscar Salvador, Qi Zheng, Shakeel Butt,
Axel Rasmussen, Yuanchu Xie, Wei Xu
In-Reply-To: <74fc1f28-b77e-4b9c-b208-51babae9d18e@amd.com>
On Fri, Feb 20, 2026 at 01:10:05PM -0600, Cheatham, Benjamin wrote:
> On 2/11/2026 9:22 AM, Dmitry Ilvokhin wrote:
> > Compaction uses compact_lock_irqsave(), which currently operates
> > on a raw spinlock_t pointer so that it can be used for both
> > zone->lock and lru_lock. Since zone lock operations are now wrapped,
> > compact_lock_irqsave() can no longer operate directly on a spinlock_t
> > when the lock belongs to a zone.
> >
> > Introduce struct compact_lock to abstract the underlying lock type. The
> > structure carries a lock type enum and a union holding either a zone
> > pointer or a raw spinlock_t pointer, and dispatches to the appropriate
> > lock/unlock helper.
> >
> > No functional change intended.
> >
> > Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
> > ---
> > mm/compaction.c | 108 +++++++++++++++++++++++++++++++++++++++---------
> > 1 file changed, 89 insertions(+), 19 deletions(-)
> >
> > diff --git a/mm/compaction.c b/mm/compaction.c
> > index 1e8f8eca318c..1b000d2b95b2 100644
> > --- a/mm/compaction.c
> > +++ b/mm/compaction.c
> > @@ -24,6 +24,7 @@
> > #include <linux/page_owner.h>
> > #include <linux/psi.h>
> > #include <linux/cpuset.h>
> > +#include <linux/zone_lock.h>
> > #include "internal.h"
> >
> > #ifdef CONFIG_COMPACTION
> > @@ -493,6 +494,65 @@ static bool test_and_set_skip(struct compact_control *cc, struct page *page)
> > }
> > #endif /* CONFIG_COMPACTION */
> >
> > +enum compact_lock_type {
> > + COMPACT_LOCK_ZONE,
> > + COMPACT_LOCK_RAW_SPINLOCK,
> > +};
> > +
> > +struct compact_lock {
> > + enum compact_lock_type type;
> > + union {
> > + struct zone *zone;
> > + spinlock_t *lock; /* Reference to lru lock */
> > + };
> > +};
> > +
> > +static bool compact_do_zone_trylock_irqsave(struct zone *zone,
> > + unsigned long *flags)
> > +{
> > + return zone_trylock_irqsave(zone, *flags);
> > +}
> > +
> > +static bool compact_do_raw_trylock_irqsave(spinlock_t *lock,
> > + unsigned long *flags)
> > +{
> > + return spin_trylock_irqsave(lock, *flags);
> > +}
> > +
> > +static bool compact_do_trylock_irqsave(struct compact_lock lock,
> > + unsigned long *flags)
> > +{
> > + if (lock.type == COMPACT_LOCK_ZONE)
> > + return compact_do_zone_trylock_irqsave(lock.zone, flags);
> > +
> > + return compact_do_raw_trylock_irqsave(lock.lock, flags);
> > +}
>
> Nit: You could remove the helpers above and just do the calls directly in this function, though
> it would remove the parity with the compact helpers. compact_do_lock_irqsave() helpers can stay
> since they have the __acquires() annotations.
Yes, I agree, there is no much value in this wrappers, will remove them,
thanks!
> > +
> > +static void compact_do_zone_lock_irqsave(struct zone *zone,
> > + unsigned long *flags)
> > +__acquires(zone->lock)
> > +{
> > + zone_lock_irqsave(zone, *flags);
> > +}
> > +
> > +static void compact_do_raw_lock_irqsave(spinlock_t *lock,
> > + unsigned long *flags)
> > +__acquires(lock)
> > +{
> > + spin_lock_irqsave(lock, *flags);
> > +}
> > +
> > +static void compact_do_lock_irqsave(struct compact_lock lock,
> > + unsigned long *flags)
> > +{
> > + if (lock.type == COMPACT_LOCK_ZONE) {
> > + compact_do_zone_lock_irqsave(lock.zone, flags);
> > + return;
> > + }
> > +
> > + return compact_do_raw_lock_irqsave(lock.lock, flags);
>
> You don't need the return statement here (and you shouldn't be returning a value at all).
Yes, agree, will fix in v2.
>
> It may be cleaner to just do an if-else statement here instead.
>
> > +}
> > +
> > /*
> > * Compaction requires the taking of some coarse locks that are potentially
> > * very heavily contended. For async compaction, trylock and record if the
> > @@ -502,19 +562,19 @@ static bool test_and_set_skip(struct compact_control *cc, struct page *page)
> > *
> > * Always returns true which makes it easier to track lock state in callers.
> > */
> > -static bool compact_lock_irqsave(spinlock_t *lock, unsigned long *flags,
> > - struct compact_control *cc)
> > - __acquires(lock)
> > +static bool compact_lock_irqsave(struct compact_lock lock,
> > + unsigned long *flags,
> > + struct compact_control *cc)
> > {
> > /* Track if the lock is contended in async mode */
> > if (cc->mode == MIGRATE_ASYNC && !cc->contended) {
> > - if (spin_trylock_irqsave(lock, *flags))
> > + if (compact_do_trylock_irqsave(lock, flags))
> > return true;
> >
> > cc->contended = true;
> > }
> >
> > - spin_lock_irqsave(lock, *flags);
> > + compact_do_lock_irqsave(lock, flags);
> > return true;
> > }
> >
> > @@ -530,11 +590,13 @@ static bool compact_lock_irqsave(spinlock_t *lock, unsigned long *flags,
> > * Returns true if compaction should abort due to fatal signal pending.
> > * Returns false when compaction can continue.
> > */
> > -static bool compact_unlock_should_abort(spinlock_t *lock,
> > - unsigned long flags, bool *locked, struct compact_control *cc)
> > +static bool compact_unlock_should_abort(struct zone *zone,
> > + unsigned long flags,
> > + bool *locked,
> > + struct compact_control *cc)
> > {
> > if (*locked) {
> > - spin_unlock_irqrestore(lock, flags);
> > + zone_unlock_irqrestore(zone, flags);
>
> I would move this (and other wrapper changes below that don't use compact_*) to the last patch. I understand you
> didn't change it due to location but I would argue it isn't really relevant to what's being added in this patch
> and fits better in the last.
Thanks for the suggestion. Totally makes sense to me, will do in v2 as well.
>
> > *locked = false;
> > }
> >
> > @@ -582,9 +644,8 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,
> > * contention, to give chance to IRQs. Abort if fatal signal
> > * pending.
> > */
> > - if (!(blockpfn % COMPACT_CLUSTER_MAX)
> > - && compact_unlock_should_abort(&cc->zone->lock, flags,
> > - &locked, cc))
> > + if (!(blockpfn % COMPACT_CLUSTER_MAX) &&
> > + compact_unlock_should_abort(cc->zone, flags, &locked, cc))
> > break;
> >
> > nr_scanned++;
> > @@ -613,8 +674,12 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,
> >
> > /* If we already hold the lock, we can skip some rechecking. */
> > if (!locked) {
> > - locked = compact_lock_irqsave(&cc->zone->lock,
> > - &flags, cc);
> > + struct compact_lock zol = {
> > + .type = COMPACT_LOCK_ZONE,
> > + .zone = cc->zone,
> > + };
> > +
> > + locked = compact_lock_irqsave(zol, &flags, cc);
> >
> > /* Recheck this is a buddy page under lock */
> > if (!PageBuddy(page))
> > @@ -649,7 +714,7 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,
> > }
> >
> > if (locked)
> > - spin_unlock_irqrestore(&cc->zone->lock, flags);
> > + zone_unlock_irqrestore(cc->zone, flags);
> >
> > /*
> > * Be careful to not go outside of the pageblock.
> > @@ -1157,10 +1222,15 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
> >
> > /* If we already hold the lock, we can skip some rechecking */
> > if (lruvec != locked) {
> > + struct compact_lock zol = {
> > + .type = COMPACT_LOCK_RAW_SPINLOCK,
> > + .lock = &lruvec->lru_lock,
> > + };
> > +
> > if (locked)
> > unlock_page_lruvec_irqrestore(locked, flags);
> >
> > - compact_lock_irqsave(&lruvec->lru_lock, &flags, cc);
> > + compact_lock_irqsave(zol, &flags, cc);
> > locked = lruvec;
> >
> > lruvec_memcg_debug(lruvec, folio);
> > @@ -1555,7 +1625,7 @@ static void fast_isolate_freepages(struct compact_control *cc)
> > if (!area->nr_free)
> > continue;
> >
> > - spin_lock_irqsave(&cc->zone->lock, flags);
> > + zone_lock_irqsave(cc->zone, flags);
> > freelist = &area->free_list[MIGRATE_MOVABLE];
> > list_for_each_entry_reverse(freepage, freelist, buddy_list) {
> > unsigned long pfn;
> > @@ -1614,7 +1684,7 @@ static void fast_isolate_freepages(struct compact_control *cc)
> > }
> > }
> >
> > - spin_unlock_irqrestore(&cc->zone->lock, flags);
> > + zone_unlock_irqrestore(cc->zone, flags);
> >
> > /* Skip fast search if enough freepages isolated */
> > if (cc->nr_freepages >= cc->nr_migratepages)
> > @@ -1988,7 +2058,7 @@ static unsigned long fast_find_migrateblock(struct compact_control *cc)
> > if (!area->nr_free)
> > continue;
> >
> > - spin_lock_irqsave(&cc->zone->lock, flags);
> > + zone_lock_irqsave(cc->zone, flags);
> > freelist = &area->free_list[MIGRATE_MOVABLE];
> > list_for_each_entry(freepage, freelist, buddy_list) {
> > unsigned long free_pfn;
> > @@ -2021,7 +2091,7 @@ static unsigned long fast_find_migrateblock(struct compact_control *cc)
> > break;
> > }
> > }
> > - spin_unlock_irqrestore(&cc->zone->lock, flags);
> > + zone_unlock_irqrestore(cc->zone, flags);
> > }
> >
> > cc->total_migrate_scanned += nr_scanned;
>
^ permalink raw reply
* Re: [PATCH v4 1/3] PCI: trace: Add PCI controller LTSSM transition tracepoint
From: Ilpo Järvinen @ 2026-02-24 15:46 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Shawn Lin, Bjorn Helgaas, linux-rockchip, linux-pci,
linux-trace-kernel, linux-doc, Steven Rostedt, Masami Hiramatsu
In-Reply-To: <2sgfbgsgrbztqadhzz6wf6b7j2lmyzmhwugr3tycsjeaik5xdz@ncehsafpi2y5>
[-- Attachment #1: Type: text/plain, Size: 5896 bytes --]
On Tue, 24 Feb 2026, Manivannan Sadhasivam wrote:
> On Tue, Feb 24, 2026 at 05:22:35PM +0200, Ilpo Järvinen wrote:
> > On Thu, 22 Jan 2026, Shawn Lin wrote:
> >
> > > Some platforms may provide LTSSM trace functionality, recording historical
> > > LTSSM state transition information. This is very useful for debugging, such
> > > as when certain devices cannot be recognized or link broken during test.
> > > Implement the pci controller tracepoint for recording LTSSM and rate.
> > >
> > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> > > ---
> > >
> > > Changes in v4:
> > > - use TRACE_EVENT_FN to notify when to start and stop the tracepoint,
> > > and export pci_ltssm_tp_enabled() for host drivers to use
> > >
> > > Changes in v3:
> > > - add TRACE_DEFINE_ENUM for all enums(Steven Rostedt)
> > >
> > > Changes in v2: None
> > >
> > > drivers/pci/trace.c | 20 ++++++++++++
> > > include/linux/pci.h | 4 +++
> > > include/trace/events/pci_controller.h | 57 +++++++++++++++++++++++++++++++++++
> > > 3 files changed, 81 insertions(+)
> > > create mode 100644 include/trace/events/pci_controller.h
> > >
> > > diff --git a/drivers/pci/trace.c b/drivers/pci/trace.c
> > > index cf11abc..d351a51 100644
> > > --- a/drivers/pci/trace.c
> > > +++ b/drivers/pci/trace.c
> > > @@ -9,3 +9,23 @@
> > >
> > > #define CREATE_TRACE_POINTS
> > > #include <trace/events/pci.h>
> > > +#include <trace/events/pci_controller.h>
> > > +
> > > +static atomic_t pcie_ltssm_tp_enabled = ATOMIC_INIT(0);
> > > +
> > > +bool pci_ltssm_tp_enabled(void)
> > > +{
> > > + return atomic_read(&pcie_ltssm_tp_enabled) > 0;
> > > +}
> > > +EXPORT_SYMBOL(pci_ltssm_tp_enabled);
> > > +
> > > +int pci_ltssm_tp_reg(void)
> > > +{
> > > + atomic_inc(&pcie_ltssm_tp_enabled);
> > > + return 0;
> > > +}
> > > +
> > > +void pci_ltssm_tp_unreg(void)
> > > +{
> > > + atomic_dec(&pcie_ltssm_tp_enabled);
> > > +}
> > > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > > index e7cb527..ac25a3e 100644
> > > --- a/include/linux/pci.h
> > > +++ b/include/linux/pci.h
> > > @@ -2770,6 +2770,10 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
> > > }
> > > #endif
> > >
> > > +#ifdef CONFIG_TRACING
> > > +bool pci_ltssm_tp_enabled(void);
> > > +#endif
> > > +
> > > void pci_add_dma_alias(struct pci_dev *dev, u8 devfn_from, unsigned nr_devfns);
> > > bool pci_devs_are_dma_aliases(struct pci_dev *dev1, struct pci_dev *dev2);
> > > int pci_for_each_dma_alias(struct pci_dev *pdev,
> > > diff --git a/include/trace/events/pci_controller.h b/include/trace/events/pci_controller.h
> > > new file mode 100644
> > > index 0000000..db4a960
> > > --- /dev/null
> > > +++ b/include/trace/events/pci_controller.h
> > > @@ -0,0 +1,57 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +#undef TRACE_SYSTEM
> > > +#define TRACE_SYSTEM pci_controller
> >
> > I find putting this into "pci_controller" little odd as LTSSM is related
> > to PCIe links/ports. To me looks something that belongs to the existing
> > include/trace/events/pci.h.
>
> I suggested 'pci_controller.h' since these tracepoints are only going to be used
> by the controller drivers. Putting it under 'pci.h' will imply that these can be
> used by the client drivers also.
PCIe r7 spec has Flit Performance Measurement Extended Capability that
seems to have something for LTSSM tracking and those seem more generic
than just for controllers (I've not spent much time on trying to fully
understand those capabilities, just recalled seeing them earlier).
--
i.
> > > +#if !defined(_TRACE_HW_EVENT_PCI_CONTROLLER_H) || defined(TRACE_HEADER_MULTI_READ)
> > > +#define _TRACE_HW_EVENT_PCI_CONTROLLER_H
> > > +
> > > +#include <uapi/linux/pci_regs.h>
> > > +#include <linux/tracepoint.h>
> > > +
> > > +TRACE_DEFINE_ENUM(PCIE_SPEED_2_5GT);
> > > +TRACE_DEFINE_ENUM(PCIE_SPEED_5_0GT);
> > > +TRACE_DEFINE_ENUM(PCIE_SPEED_8_0GT);
> > > +TRACE_DEFINE_ENUM(PCIE_SPEED_16_0GT);
> > > +TRACE_DEFINE_ENUM(PCIE_SPEED_32_0GT);
> > > +TRACE_DEFINE_ENUM(PCIE_SPEED_64_0GT);
> > > +TRACE_DEFINE_ENUM(PCI_SPEED_UNKNOWN);
> > > +
> > > +extern int pci_ltssm_tp_reg(void);
> > > +extern void pci_ltssm_tp_unreg(void);
> > > +
> > > +TRACE_EVENT_FN(pcie_ltssm_state_transition,
> > > + TP_PROTO(const char *dev_name, const char *state, u32 rate),
> > > + TP_ARGS(dev_name, state, rate),
> > > +
> > > + TP_STRUCT__entry(
> > > + __string(dev_name, dev_name)
> > > + __string(state, state)
> > > + __field(u32, rate)
> > > + ),
> > > +
> > > + TP_fast_assign(
> > > + __assign_str(dev_name);
> > > + __assign_str(state);
> > > + __entry->rate = rate;
> > > + ),
> > > +
> > > + TP_printk("dev: %s state: %s rate: %s",
> > > + __get_str(dev_name), __get_str(state),
> > > + __print_symbolic(__entry->rate,
> > > + { PCIE_SPEED_2_5GT, "2.5 GT/s" },
> > > + { PCIE_SPEED_5_0GT, "5.0 GT/s" },
> > > + { PCIE_SPEED_8_0GT, "8.0 GT/s" },
> > > + { PCIE_SPEED_16_0GT, "16.0 GT/s" },
> > > + { PCIE_SPEED_32_0GT, "32.0 GT/s" },
> > > + { PCIE_SPEED_64_0GT, "64.0 GT/s" },
> > > + { PCI_SPEED_UNKNOWN, "Unknown" }
> >
> > Why are these done inline instead of using EM/EMe()? Or simply with
> > pci_speed_string()?
> >
> >
> > Unrelated to this, sadly I failed to notice Shuai's version of
> > pcie_link_event() did not translate link speeds (my own version used
> > pci_speed_string()).
> >
> > > + )
> > > + ),
> > > +
> > > + pci_ltssm_tp_reg, pci_ltssm_tp_unreg
> > > +);
> > > +
> > > +#endif /* _TRACE_HW_EVENT_PCI_CONTROLLER_H */
> > > +
> > > +/* This part must be outside protection */
> > > +#include <trace/define_trace.h>
> > >
> >
> > --
> > i.
> >
>
>
--
i.
^ permalink raw reply
* Re: [PATCH v4 1/3] PCI: trace: Add PCI controller LTSSM transition tracepoint
From: Manivannan Sadhasivam @ 2026-02-24 15:35 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Shawn Lin, Bjorn Helgaas, linux-rockchip, linux-pci,
linux-trace-kernel, linux-doc, Steven Rostedt, Masami Hiramatsu
In-Reply-To: <7d195f03-978a-2d0d-acdf-e583e68377f2@linux.intel.com>
On Tue, Feb 24, 2026 at 05:22:35PM +0200, Ilpo Järvinen wrote:
> On Thu, 22 Jan 2026, Shawn Lin wrote:
>
> > Some platforms may provide LTSSM trace functionality, recording historical
> > LTSSM state transition information. This is very useful for debugging, such
> > as when certain devices cannot be recognized or link broken during test.
> > Implement the pci controller tracepoint for recording LTSSM and rate.
> >
> > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> > ---
> >
> > Changes in v4:
> > - use TRACE_EVENT_FN to notify when to start and stop the tracepoint,
> > and export pci_ltssm_tp_enabled() for host drivers to use
> >
> > Changes in v3:
> > - add TRACE_DEFINE_ENUM for all enums(Steven Rostedt)
> >
> > Changes in v2: None
> >
> > drivers/pci/trace.c | 20 ++++++++++++
> > include/linux/pci.h | 4 +++
> > include/trace/events/pci_controller.h | 57 +++++++++++++++++++++++++++++++++++
> > 3 files changed, 81 insertions(+)
> > create mode 100644 include/trace/events/pci_controller.h
> >
> > diff --git a/drivers/pci/trace.c b/drivers/pci/trace.c
> > index cf11abc..d351a51 100644
> > --- a/drivers/pci/trace.c
> > +++ b/drivers/pci/trace.c
> > @@ -9,3 +9,23 @@
> >
> > #define CREATE_TRACE_POINTS
> > #include <trace/events/pci.h>
> > +#include <trace/events/pci_controller.h>
> > +
> > +static atomic_t pcie_ltssm_tp_enabled = ATOMIC_INIT(0);
> > +
> > +bool pci_ltssm_tp_enabled(void)
> > +{
> > + return atomic_read(&pcie_ltssm_tp_enabled) > 0;
> > +}
> > +EXPORT_SYMBOL(pci_ltssm_tp_enabled);
> > +
> > +int pci_ltssm_tp_reg(void)
> > +{
> > + atomic_inc(&pcie_ltssm_tp_enabled);
> > + return 0;
> > +}
> > +
> > +void pci_ltssm_tp_unreg(void)
> > +{
> > + atomic_dec(&pcie_ltssm_tp_enabled);
> > +}
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index e7cb527..ac25a3e 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -2770,6 +2770,10 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
> > }
> > #endif
> >
> > +#ifdef CONFIG_TRACING
> > +bool pci_ltssm_tp_enabled(void);
> > +#endif
> > +
> > void pci_add_dma_alias(struct pci_dev *dev, u8 devfn_from, unsigned nr_devfns);
> > bool pci_devs_are_dma_aliases(struct pci_dev *dev1, struct pci_dev *dev2);
> > int pci_for_each_dma_alias(struct pci_dev *pdev,
> > diff --git a/include/trace/events/pci_controller.h b/include/trace/events/pci_controller.h
> > new file mode 100644
> > index 0000000..db4a960
> > --- /dev/null
> > +++ b/include/trace/events/pci_controller.h
> > @@ -0,0 +1,57 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#undef TRACE_SYSTEM
> > +#define TRACE_SYSTEM pci_controller
>
> I find putting this into "pci_controller" little odd as LTSSM is related
> to PCIe links/ports. To me looks something that belongs to the existing
> include/trace/events/pci.h.
>
I suggested 'pci_controller.h' since these tracepoints are only going to be used
by the controller drivers. Putting it under 'pci.h' will imply that these can be
used by the client drivers also.
- Mani
> > +#if !defined(_TRACE_HW_EVENT_PCI_CONTROLLER_H) || defined(TRACE_HEADER_MULTI_READ)
> > +#define _TRACE_HW_EVENT_PCI_CONTROLLER_H
> > +
> > +#include <uapi/linux/pci_regs.h>
> > +#include <linux/tracepoint.h>
> > +
> > +TRACE_DEFINE_ENUM(PCIE_SPEED_2_5GT);
> > +TRACE_DEFINE_ENUM(PCIE_SPEED_5_0GT);
> > +TRACE_DEFINE_ENUM(PCIE_SPEED_8_0GT);
> > +TRACE_DEFINE_ENUM(PCIE_SPEED_16_0GT);
> > +TRACE_DEFINE_ENUM(PCIE_SPEED_32_0GT);
> > +TRACE_DEFINE_ENUM(PCIE_SPEED_64_0GT);
> > +TRACE_DEFINE_ENUM(PCI_SPEED_UNKNOWN);
> > +
> > +extern int pci_ltssm_tp_reg(void);
> > +extern void pci_ltssm_tp_unreg(void);
> > +
> > +TRACE_EVENT_FN(pcie_ltssm_state_transition,
> > + TP_PROTO(const char *dev_name, const char *state, u32 rate),
> > + TP_ARGS(dev_name, state, rate),
> > +
> > + TP_STRUCT__entry(
> > + __string(dev_name, dev_name)
> > + __string(state, state)
> > + __field(u32, rate)
> > + ),
> > +
> > + TP_fast_assign(
> > + __assign_str(dev_name);
> > + __assign_str(state);
> > + __entry->rate = rate;
> > + ),
> > +
> > + TP_printk("dev: %s state: %s rate: %s",
> > + __get_str(dev_name), __get_str(state),
> > + __print_symbolic(__entry->rate,
> > + { PCIE_SPEED_2_5GT, "2.5 GT/s" },
> > + { PCIE_SPEED_5_0GT, "5.0 GT/s" },
> > + { PCIE_SPEED_8_0GT, "8.0 GT/s" },
> > + { PCIE_SPEED_16_0GT, "16.0 GT/s" },
> > + { PCIE_SPEED_32_0GT, "32.0 GT/s" },
> > + { PCIE_SPEED_64_0GT, "64.0 GT/s" },
> > + { PCI_SPEED_UNKNOWN, "Unknown" }
>
> Why are these done inline instead of using EM/EMe()? Or simply with
> pci_speed_string()?
>
>
> Unrelated to this, sadly I failed to notice Shuai's version of
> pcie_link_event() did not translate link speeds (my own version used
> pci_speed_string()).
>
> > + )
> > + ),
> > +
> > + pci_ltssm_tp_reg, pci_ltssm_tp_unreg
> > +);
> > +
> > +#endif /* _TRACE_HW_EVENT_PCI_CONTROLLER_H */
> > +
> > +/* This part must be outside protection */
> > +#include <trace/define_trace.h>
> >
>
> --
> i.
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH 1/4] mm: introduce zone lock wrappers
From: Shakeel Butt @ 2026-02-24 15:31 UTC (permalink / raw)
To: Dmitry Ilvokhin
Cc: Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Brendan Jackman,
Johannes Weiner, Zi Yan, Oscar Salvador, Qi Zheng, Axel Rasmussen,
Yuanchu Xie, Wei Xu, linux-kernel, linux-mm, linux-trace-kernel,
linux-cxl, kernel-team
In-Reply-To: <3826dd6dc55a9c5721ec3de85f019764a6cf3222.1770821420.git.d@ilvokhin.com>
On Wed, Feb 11, 2026 at 03:22:13PM +0000, Dmitry Ilvokhin wrote:
> Add thin wrappers around zone lock acquire/release operations. This
> prepares the code for future tracepoint instrumentation without
> modifying individual call sites.
>
> Centralizing zone lock operations behind wrappers allows future
> instrumentation or debugging hooks to be added without touching
> all users.
>
> No functional change intended. The wrappers are introduced in
> preparation for subsequent patches and are not yet used.
>
> Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
^ permalink raw reply
* Re: [PATCH 1/4] mm: introduce zone lock wrappers
From: Shakeel Butt @ 2026-02-24 15:30 UTC (permalink / raw)
To: Dmitry Ilvokhin
Cc: Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Brendan Jackman,
Johannes Weiner, Zi Yan, Oscar Salvador, Qi Zheng, Axel Rasmussen,
Yuanchu Xie, Wei Xu, linux-kernel, linux-mm, linux-trace-kernel,
linux-cxl, kernel-team
In-Reply-To: <aZ3BLKzhIIZvkbwL@shell.ilvokhin.com>
On Tue, Feb 24, 2026 at 03:18:04PM +0000, Dmitry Ilvokhin wrote:
> On Mon, Feb 23, 2026 at 02:36:01PM -0800, Shakeel Butt wrote:
> > On Wed, Feb 11, 2026 at 03:22:13PM +0000, Dmitry Ilvokhin wrote:
> > > Add thin wrappers around zone lock acquire/release operations. This
> > > prepares the code for future tracepoint instrumentation without
> > > modifying individual call sites.
> > >
> > > Centralizing zone lock operations behind wrappers allows future
> > > instrumentation or debugging hooks to be added without touching
> > > all users.
> > >
> > > No functional change intended. The wrappers are introduced in
> > > preparation for subsequent patches and are not yet used.
> > >
> > > Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
> > > ---
> > > MAINTAINERS | 1 +
> > > include/linux/zone_lock.h | 38 ++++++++++++++++++++++++++++++++++++++
> > > 2 files changed, 39 insertions(+)
> > > create mode 100644 include/linux/zone_lock.h
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index b4088f7290be..680c9ae02d7e 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -16498,6 +16498,7 @@ F: include/linux/pgtable.h
> > > F: include/linux/ptdump.h
> > > F: include/linux/vmpressure.h
> > > F: include/linux/vmstat.h
> > > +F: include/linux/zone_lock.h
> > > F: kernel/fork.c
> > > F: mm/Kconfig
> > > F: mm/debug.c
> > > diff --git a/include/linux/zone_lock.h b/include/linux/zone_lock.h
> > > new file mode 100644
> > > index 000000000000..c531e26280e6
> > > --- /dev/null
> > > +++ b/include/linux/zone_lock.h
> > > @@ -0,0 +1,38 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +#ifndef _LINUX_ZONE_LOCK_H
> > > +#define _LINUX_ZONE_LOCK_H
> > > +
> > > +#include <linux/mmzone.h>
> > > +#include <linux/spinlock.h>
> > > +
> > > +static inline void zone_lock_init(struct zone *zone)
> > > +{
> > > + spin_lock_init(&zone->lock);
> > > +}
> > > +
> > > +#define zone_lock_irqsave(zone, flags) \
> > > +do { \
> > > + spin_lock_irqsave(&(zone)->lock, flags); \
> > > +} while (0)
> > > +
> > > +#define zone_trylock_irqsave(zone, flags) \
> > > +({ \
> > > + spin_trylock_irqsave(&(zone)->lock, flags); \
> > > +})
> >
> > Any reason you used macros for above two and inlined functions for remaining?
> >
>
> The reason for using macros in those two cases is that they need to
> modify the flags variable passed by the caller, just like
> spin_lock_irqsave() and spin_trylock_irqsave() do. I followed the same
> convention here.
>
> If we used normal inline functions instead, we would need to pass a
> pointer to flags, which would change the call sites and diverge from the
> existing *_irqsave() locking pattern.
>
> There is also a difference between zone_lock_irqsave() and
> zone_trylock_irqsave() implementations: the former is implemented as a
> do { } while (0) macro since it does not return a value, while the
> latter uses a GCC extension in order to return the trylock result. This
> matches spin_lock_* convention as well.
>
Cool, thanks for the explanation.
^ permalink raw reply
* Re: [PATCH v4 1/3] PCI: trace: Add PCI controller LTSSM transition tracepoint
From: Ilpo Järvinen @ 2026-02-24 15:22 UTC (permalink / raw)
To: Shawn Lin
Cc: Manivannan Sadhasivam, Bjorn Helgaas, linux-rockchip, linux-pci,
linux-trace-kernel, linux-doc, Steven Rostedt, Masami Hiramatsu
In-Reply-To: <1769047340-113287-2-git-send-email-shawn.lin@rock-chips.com>
On Thu, 22 Jan 2026, Shawn Lin wrote:
> Some platforms may provide LTSSM trace functionality, recording historical
> LTSSM state transition information. This is very useful for debugging, such
> as when certain devices cannot be recognized or link broken during test.
> Implement the pci controller tracepoint for recording LTSSM and rate.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
>
> Changes in v4:
> - use TRACE_EVENT_FN to notify when to start and stop the tracepoint,
> and export pci_ltssm_tp_enabled() for host drivers to use
>
> Changes in v3:
> - add TRACE_DEFINE_ENUM for all enums(Steven Rostedt)
>
> Changes in v2: None
>
> drivers/pci/trace.c | 20 ++++++++++++
> include/linux/pci.h | 4 +++
> include/trace/events/pci_controller.h | 57 +++++++++++++++++++++++++++++++++++
> 3 files changed, 81 insertions(+)
> create mode 100644 include/trace/events/pci_controller.h
>
> diff --git a/drivers/pci/trace.c b/drivers/pci/trace.c
> index cf11abc..d351a51 100644
> --- a/drivers/pci/trace.c
> +++ b/drivers/pci/trace.c
> @@ -9,3 +9,23 @@
>
> #define CREATE_TRACE_POINTS
> #include <trace/events/pci.h>
> +#include <trace/events/pci_controller.h>
> +
> +static atomic_t pcie_ltssm_tp_enabled = ATOMIC_INIT(0);
> +
> +bool pci_ltssm_tp_enabled(void)
> +{
> + return atomic_read(&pcie_ltssm_tp_enabled) > 0;
> +}
> +EXPORT_SYMBOL(pci_ltssm_tp_enabled);
> +
> +int pci_ltssm_tp_reg(void)
> +{
> + atomic_inc(&pcie_ltssm_tp_enabled);
> + return 0;
> +}
> +
> +void pci_ltssm_tp_unreg(void)
> +{
> + atomic_dec(&pcie_ltssm_tp_enabled);
> +}
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index e7cb527..ac25a3e 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -2770,6 +2770,10 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
> }
> #endif
>
> +#ifdef CONFIG_TRACING
> +bool pci_ltssm_tp_enabled(void);
> +#endif
> +
> void pci_add_dma_alias(struct pci_dev *dev, u8 devfn_from, unsigned nr_devfns);
> bool pci_devs_are_dma_aliases(struct pci_dev *dev1, struct pci_dev *dev2);
> int pci_for_each_dma_alias(struct pci_dev *pdev,
> diff --git a/include/trace/events/pci_controller.h b/include/trace/events/pci_controller.h
> new file mode 100644
> index 0000000..db4a960
> --- /dev/null
> +++ b/include/trace/events/pci_controller.h
> @@ -0,0 +1,57 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM pci_controller
I find putting this into "pci_controller" little odd as LTSSM is related
to PCIe links/ports. To me looks something that belongs to the existing
include/trace/events/pci.h.
> +#if !defined(_TRACE_HW_EVENT_PCI_CONTROLLER_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_HW_EVENT_PCI_CONTROLLER_H
> +
> +#include <uapi/linux/pci_regs.h>
> +#include <linux/tracepoint.h>
> +
> +TRACE_DEFINE_ENUM(PCIE_SPEED_2_5GT);
> +TRACE_DEFINE_ENUM(PCIE_SPEED_5_0GT);
> +TRACE_DEFINE_ENUM(PCIE_SPEED_8_0GT);
> +TRACE_DEFINE_ENUM(PCIE_SPEED_16_0GT);
> +TRACE_DEFINE_ENUM(PCIE_SPEED_32_0GT);
> +TRACE_DEFINE_ENUM(PCIE_SPEED_64_0GT);
> +TRACE_DEFINE_ENUM(PCI_SPEED_UNKNOWN);
> +
> +extern int pci_ltssm_tp_reg(void);
> +extern void pci_ltssm_tp_unreg(void);
> +
> +TRACE_EVENT_FN(pcie_ltssm_state_transition,
> + TP_PROTO(const char *dev_name, const char *state, u32 rate),
> + TP_ARGS(dev_name, state, rate),
> +
> + TP_STRUCT__entry(
> + __string(dev_name, dev_name)
> + __string(state, state)
> + __field(u32, rate)
> + ),
> +
> + TP_fast_assign(
> + __assign_str(dev_name);
> + __assign_str(state);
> + __entry->rate = rate;
> + ),
> +
> + TP_printk("dev: %s state: %s rate: %s",
> + __get_str(dev_name), __get_str(state),
> + __print_symbolic(__entry->rate,
> + { PCIE_SPEED_2_5GT, "2.5 GT/s" },
> + { PCIE_SPEED_5_0GT, "5.0 GT/s" },
> + { PCIE_SPEED_8_0GT, "8.0 GT/s" },
> + { PCIE_SPEED_16_0GT, "16.0 GT/s" },
> + { PCIE_SPEED_32_0GT, "32.0 GT/s" },
> + { PCIE_SPEED_64_0GT, "64.0 GT/s" },
> + { PCI_SPEED_UNKNOWN, "Unknown" }
Why are these done inline instead of using EM/EMe()? Or simply with
pci_speed_string()?
Unrelated to this, sadly I failed to notice Shuai's version of
pcie_link_event() did not translate link speeds (my own version used
pci_speed_string()).
> + )
> + ),
> +
> + pci_ltssm_tp_reg, pci_ltssm_tp_unreg
> +);
> +
> +#endif /* _TRACE_HW_EVENT_PCI_CONTROLLER_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
>
--
i.
^ permalink raw reply
* Re: [PATCH 1/4] mm: introduce zone lock wrappers
From: Dmitry Ilvokhin @ 2026-02-24 15:18 UTC (permalink / raw)
To: Shakeel Butt
Cc: Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Brendan Jackman,
Johannes Weiner, Zi Yan, Oscar Salvador, Qi Zheng, Axel Rasmussen,
Yuanchu Xie, Wei Xu, linux-kernel, linux-mm, linux-trace-kernel,
linux-cxl, kernel-team
In-Reply-To: <aZzWNM06gNYKDoQW@linux.dev>
On Mon, Feb 23, 2026 at 02:36:01PM -0800, Shakeel Butt wrote:
> On Wed, Feb 11, 2026 at 03:22:13PM +0000, Dmitry Ilvokhin wrote:
> > Add thin wrappers around zone lock acquire/release operations. This
> > prepares the code for future tracepoint instrumentation without
> > modifying individual call sites.
> >
> > Centralizing zone lock operations behind wrappers allows future
> > instrumentation or debugging hooks to be added without touching
> > all users.
> >
> > No functional change intended. The wrappers are introduced in
> > preparation for subsequent patches and are not yet used.
> >
> > Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
> > ---
> > MAINTAINERS | 1 +
> > include/linux/zone_lock.h | 38 ++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 39 insertions(+)
> > create mode 100644 include/linux/zone_lock.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index b4088f7290be..680c9ae02d7e 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -16498,6 +16498,7 @@ F: include/linux/pgtable.h
> > F: include/linux/ptdump.h
> > F: include/linux/vmpressure.h
> > F: include/linux/vmstat.h
> > +F: include/linux/zone_lock.h
> > F: kernel/fork.c
> > F: mm/Kconfig
> > F: mm/debug.c
> > diff --git a/include/linux/zone_lock.h b/include/linux/zone_lock.h
> > new file mode 100644
> > index 000000000000..c531e26280e6
> > --- /dev/null
> > +++ b/include/linux/zone_lock.h
> > @@ -0,0 +1,38 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _LINUX_ZONE_LOCK_H
> > +#define _LINUX_ZONE_LOCK_H
> > +
> > +#include <linux/mmzone.h>
> > +#include <linux/spinlock.h>
> > +
> > +static inline void zone_lock_init(struct zone *zone)
> > +{
> > + spin_lock_init(&zone->lock);
> > +}
> > +
> > +#define zone_lock_irqsave(zone, flags) \
> > +do { \
> > + spin_lock_irqsave(&(zone)->lock, flags); \
> > +} while (0)
> > +
> > +#define zone_trylock_irqsave(zone, flags) \
> > +({ \
> > + spin_trylock_irqsave(&(zone)->lock, flags); \
> > +})
>
> Any reason you used macros for above two and inlined functions for remaining?
>
The reason for using macros in those two cases is that they need to
modify the flags variable passed by the caller, just like
spin_lock_irqsave() and spin_trylock_irqsave() do. I followed the same
convention here.
If we used normal inline functions instead, we would need to pass a
pointer to flags, which would change the call sites and diverge from the
existing *_irqsave() locking pattern.
There is also a difference between zone_lock_irqsave() and
zone_trylock_irqsave() implementations: the former is implemented as a
do { } while (0) macro since it does not return a value, while the
latter uses a GCC extension in order to return the trylock result. This
matches spin_lock_* convention as well.
> > +
> > +static inline void zone_unlock_irqrestore(struct zone *zone, unsigned long flags)
> > +{
> > + spin_unlock_irqrestore(&zone->lock, flags);
> > +}
> > +
> > +static inline void zone_lock_irq(struct zone *zone)
> > +{
> > + spin_lock_irq(&zone->lock);
> > +}
> > +
> > +static inline void zone_unlock_irq(struct zone *zone)
> > +{
> > + spin_unlock_irq(&zone->lock);
> > +}
> > +
> > +#endif /* _LINUX_ZONE_LOCK_H */
> > --
> > 2.47.3
> >
^ permalink raw reply
* Re: [LSF/MM/BPF TOPIC][RFC PATCH v4 00/27] Private Memory Nodes (w/ Compressed RAM)
From: Gregory Price @ 2026-02-24 15:17 UTC (permalink / raw)
To: Alistair Popple
Cc: lsf-pc, linux-kernel, linux-cxl, cgroups, linux-mm,
linux-trace-kernel, damon, kernel-team, gregkh, rafael, dakr,
dave, jonathan.cameron, dave.jiang, alison.schofield,
vishal.l.verma, ira.weiny, dan.j.williams, longman, akpm, david,
lorenzo.stoakes, Liam.Howlett, vbabka, rppt, surenb, mhocko,
osalvador, ziy, matthew.brost, joshua.hahnjy, rakie.kim,
byungchul, ying.huang, axelrasmussen, yuanchu, weixugc,
yury.norov, linux, mhiramat, mathieu.desnoyers, tj, hannes,
mkoutny, jackmanb, sj, baolin.wang, npache, ryan.roberts,
dev.jain, baohua, lance.yang, muchun.song, xu.xin16,
chengming.zhou, jannh, linmiaohe, nao.horiguchi, pfalcato,
rientjes, shakeel.butt, riel, harry.yoo, cl, roman.gushchin,
chrisl, kasong, shikemeng, nphamcs, bhe, zhengqi.arch,
terry.bowman
In-Reply-To: <fzy6f6dpv3oq3ksr2mkst7pz3daeb3buhuvdvcw4633pcl7h6u@mxjgiwpg5acv>
On Tue, Feb 24, 2026 at 05:19:11PM +1100, Alistair Popple wrote:
> On 2026-02-22 at 19:48 +1100, Gregory Price <gourry@gourry.net> wrote...
>
> Based on our discussion at LPC I believe one of the primary motivators here was
> to re-use the existing mm buddy allocator rather than writing your own. I remain
> to be convinced that alone is justification enough for doing all this - DRM for
> example already has quite a nice standalone buddy allocator (drm_buddy.c) that
> could presumably be used, or adapted for use, by any device driver.
>
> The interesting part of this series (which I have skimmed but not read in
> detail) is how device memory gets exposed to userspace - this is something that
> existing ZONE_DEVICE implementations don't address, instead leaving it up to
> drivers and associated userspace stacks to deal with allocation, migration, etc.
>
I agree that buddy-access alone is insufficient justification, it
started off that way - but if you want mempolicy/NUMA UAPI access,
it turns into "Re-use all of MM" - and that means using the buddy.
I also expected ZONE_DEVICE vs NODE_DATA to be the primary discussion,
I raise replacing it as a thought experiment, but not the proposal.
The idea that drm/ is going to switch to private nodes is outside the
realm of reality, but part of that is because of years of infrastructure
built on the assumption that re-using mm/ is infeasible.
But, lets talk about DEVICE_COHERENT
---
DEVICE_COHERENT is the odd-man out among ZONE_DEVICE modes. The others
use softleaf entries and don't allow direct mappings.
(DEVICE_PRIVATE sort of does if you squint, but you can also view that
a bit like PROT_NONE or read-only controls to force migrations).
If you take DEVICE_COHERENT and:
- Move pgmap out of the struct page (page_ext, NODE_DATA, etc) to free
the LRU list_head
- Put pages in the buddy (free lists, watermarks, managed_pages) or add
pgmap->device_alloc() at every allocation callsite / buddy hook
- Add LRU support (aging, reclaim, compaction)
- Add isolated gating (new GFP flag and adjusted zonelist filtering)
- Add new dev_pagemap_ops callbacks for the various mm/ features
- Audit evey folio_is_zone_device() to distinguish zone device modes
... you've built N_MEMORY_PRIVATE inside ZONE_DEVICE. Except now
page_zone(page) returns ZONE_DEVICE - so you inherit the wrong
defaults at every existing ZONE_DEVICE check.
Skip-sites become things to opt-out of instead of opting into.
You just end up with
if (folio_is_zone_device(folio))
if (folio_is_my_special_zone_device())
else ....
and this just generalizes to
if (folio_is_private_managed(folio))
folio_managed_my_hooked_operation()
So you get the same code, but have added more complexity to ZONE_DEVICE.
I don't think that's needed if we just recognize ZONE is the wrong
abstraction to be operating on.
Honestly, even ZONE_MOVABLE becomes pointless with N_MEMORY_PRIVATE
if you disallow longterm pinning - because the managing service handles
allocations (it has to inject GFP_PRIVATE to get access) or selectively
enables the mm/ services it knows are safe (mempolicy).
Even if you allow longterm pinning, if your service controls what does
the pinning it can still be reclaimable - just manually (killing
processes) instead of letting hotplug do it via migration.
If your service only allocates movable pages - your ZONE_NORMAL is
effectively ZONE_MOVABLE.
In some cases we use ZONE_MOVABLE to prevent the kernel from allocating
memory onto devices (like CXL). This means struct page is forced to
take up DRAM or use memmap_on_memory - meaning you lose high-value
capacity or sacrifice contiguity (less huge page support).
This entire problem can evaporate if you can just use ZONE_NORMAL.
There are a lot of benefits to just re-using the buddy like this.
Zones are the wrong abstraction and cause more problems.
> > free_folio - mirrors ZONE_DEVICE's
> > folio_split - mirrors ZONE_DEVICE's
> > migrate_to - ... same as ZONE_DEVICE
> > handle_fault - mirrors the ZONE_DEVICE ...
> > memory_failure - parallels memory_failure_dev_pagemap(),
>
> One does not have to squint too hard to see that the above is not so different
> from what ZONE_DEVICE provides today via dev_pagemap_ops(). So I think I think
> it would be worth outlining why the existing ZONE_DEVICE mechanism can't be
> extended to provide these kind of services.
>
> This seems to add a bunch of code just to use NODE_DATA instead of page->pgmap,
> without really explaining why just extending dev_pagemap_ops wouldn't work. The
> obvious reason is that if you want to support things like reclaim, compaction,
> etc. these pages need to be on the LRU, which is a little bit hard when that
> field is also used by the pgmap pointer for ZONE_DEVICE pages.
>
You don't have to squint because it was deliberate :]
The callback similarity is the feature - they're the same logical
operations. The difference is the direction of the defaults.
Extending ZONE_DEVICE into these areas requires the same set of hooks,
plus distinguishing "old ZONE_DEVICE" from "new ZONE_DEVICE".
Where there are new injection sites, it's because ZONE_DEVICE opts
out of ever touching that code in some other silently implied way.
For example, reclaim/compaction doesn't run because ZONE_DEVICE doesn't
add to managed_pages (among other reasons).
You'd have to go figure out how to hack those things into ZONE_DEVICE
*and then* opt every *other* ZONE_DEVICE mode *back out*.
So you still end up with something like this anyway:
static inline bool folio_managed_handle_fault(struct folio *folio,
struct vm_fault *vmf,
enum pgtable_level level,
vm_fault_t *ret)
{
/* Zone device pages use swap entries; handled in do_swap_page */
if (folio_is_zone_device(folio))
return false;
if (folio_is_private_node(folio))
...
return false;
}
> example page_ext could be used. Or I hear struct page may go away in place of
> folios any day now, so maybe that gives us space for both :-)
>
If NUMA is the interface we want, then NODE_DATA is the right direction
regardless of struct page's future or what zone it lives in.
There's no reason to keep per-page pgmap w/ device-to-node mappings.
You can have one driver manage multiple devices with the same numa node
if it uses the same owner context (PFN already differentiates devices).
The existing code allows for this.
> The above also looks pretty similar to the existing ZONE_DEVICE methods for
> doing this which is another reason to argue for just building up the feature set
> of the existing boondoggle rather than adding another thingymebob.
>
> It seems the key thing we are looking for is:
>
> 1) A userspace API to allocate/manage device memory (ie. move_pages(), mbind(),
> etc.)
>
> 2) Allowing reclaim/LRU list processing of device memory.
>
> From my perspective both of these are interesting and I look forward to the
> discussion (hopefully I can make it to LSFMM). Mostly I'm interested in the
> implementation as this does on the surface seem to sprinkle around and duplicate
> a lot of hooks similar to what ZONE_DEVICE already provides.
>
On (1): ZONE_DEVICE NUMA UAPI is harder than it looks from the surface
Much of the kernel mm/ infrastructure is written on top of the buddy and
expects N_MEMORY to be the sole arbiter of "Where to Acquire Pages".
Mempolicy depends on:
- Buddy support or a new alloc hook around the buddy
- Migration support (mbind() after allocation migrates)
- Migration also deeply assumes buddy and LRU support
- Changing validations on node states
- mempolicy checks N_MEMORY membership, so you have to hack
N_MEMORY onto ZONE_DEVICE
(or teach it about a new node state... N_MEMORY_PRIVATE)
Getting mempolicy to work with N_MEMORY_PRIVATE amounts to adding 2
lines of code in vma_alloc_folio_noprof:
struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order,
struct vm_area_struct *vma,
unsigned long addr)
{
if (pol->flags & MPOL_F_PRIVATE)
gfp |= __GFP_PRIVATE;
folio = folio_alloc_mpol_noprof(gfp, order, pol, ilx, numa_node_id());
/* Woo! I faulted a DEVICE PAGE! */
}
But this requires the pages to be managed by the buddy.
The rest of the mempolicy support is around keeping sane nodemasks when
things like cpuset.mems rebinds occur and validating you don't end up
with private nodes that don't support mempolicy in your nodemask.
You have to do all of this anyway, but with the added bonus of fighting
with the overloaded nature of ZONE_DEVICE at every step.
==========
On (2): Assume you solve LRU.
Zone Device has no free lists, managed_pages, or watermarks.
kswapd can't run, compaction has no targets, vmscan's pressure model
doesn't function. These all come for free when the pages are
buddy-managed on a real zone. Why re-invent the wheel?
==========
So you really have two options here:
a) Put pages in the buddy, or
b) Add pgmap->device_alloc() callbacks at every allocation site that
could target a node:
- vma_alloc_folio
- alloc_migration_target
- alloc_demote_folio
- alloc_pages_node
- alloc_contig_pages
- list goes on
Or more likely - hooking get_page_from_freelist. Which at that
point... just use the buddy? You're already deep in the hot path.
>
> For basic allocation I agree this is the case. But there's no reason some device
> allocator library couldn't be written. Or in fact as pointed out above reuse the
> already existing one in drm_buddy.c. So would be interested to hear arguments
> for why allocation has to be done by the mm allocator and/or why an allocation
> library wouldn't work here given DRM already has them.
>
Using the buddy underpins the rest of mm/ services we want to re-use.
That's basically it. Otherwise you have to inject hooks into every
surface that touches the buddy...
... or in the buddy (get_page_from_freelist), at which point why not
just use the buddy?
~Gregory
^ permalink raw reply
* Re: [PATCH v4 3/3] PCI: dw-rockchip: Add pcie_ltssm_state_transition trace support
From: Steven Rostedt @ 2026-02-24 14:16 UTC (permalink / raw)
To: Shawn Lin
Cc: Manivannan Sadhasivam, Bjorn Helgaas, linux-rockchip, linux-pci,
linux-trace-kernel, linux-doc, Masami Hiramatsu
In-Reply-To: <20260224091115.6e32c38e@fedora>
On Tue, 24 Feb 2026 09:11:15 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> > +#ifdef CONFIG_TRACING
> > +static void rockchip_pcie_ltssm_trace_work(struct work_struct *work)
> > +{
> > + struct rockchip_pcie *rockchip = container_of(work,
> > + struct rockchip_pcie,
> > + trace_work.work);
> > + struct dw_pcie *pci = &rockchip->pci;
> > + enum dw_pcie_ltssm state;
> > + u32 i, l1ss, prev_val = DW_PCIE_LTSSM_UNKNOWN, rate, val;
> > +
> > + if (!pci_ltssm_tp_enabled())
> > + goto skip_trace;
>
> You can use:
>
> if (!trace_pcie_ltssm_state_transition_enabled())
> goto skip_trace;
>
> The above is a static branch. That means when tracing is disabled, it is
> basically:
>
> goto skip_trace;
>
> and when tracing is enabled it is a nop.
>
> -- Steve
>
>
> > +
> > + for (i = 0; i < PCIE_DBG_LTSSM_HISTORY_CNT; i++) {
> > + val = rockchip_pcie_readl_apb(rockchip,
> > + PCIE_CLIENT_DBG_FIFO_STATUS);
> > + rate = FIELD_GET(PCIE_DBG_FIFO_RATE_MASK, val);
> > + l1ss = FIELD_GET(PCIE_DBG_FIFO_L1SUB_MASK, val);
> > + val = FIELD_GET(PCIE_LTSSM_STATUS_MASK, val);
> > +
> > + /*
> > + * Hardware Mechanism: The ring FIFO employs two tracking
> > + * counters:
> > + * - 'last-read-point': maintains the user's last read position
> > + * - 'last-valid-point': tracks the HW's last state update
> > + *
> > + * Software Handling: When two consecutive LTSSM states are
> > + * identical, it indicates invalid subsequent data in the FIFO.
> > + * In this case, we skip the remaining entries. The dual counter
> > + * design ensures that on the next state transition, reading can
> > + * resume from the last user position.
> > + */
> > + if ((i > 0 && val == prev_val) || val > DW_PCIE_LTSSM_RCVRY_EQ3)
> > + break;
> > +
> > + state = prev_val = val;
> > + if (val == DW_PCIE_LTSSM_L1_IDLE) {
> > + if (l1ss == 2)
> > + state = DW_PCIE_LTSSM_L1_2;
> > + else if (l1ss == 1)
> > + state = DW_PCIE_LTSSM_L1_1;
> > + }
> > +
> > + trace_pcie_ltssm_state_transition(dev_name(pci->dev),
> > + dw_pcie_ltssm_status_string(state),
> > + ((rate + 1) > pci->max_link_speed) ?
> > + PCI_SPEED_UNKNOWN : PCIE_SPEED_2_5GT + rate);
> > + }
> > +
> > +skip_trace:
> > + schedule_delayed_work(&rockchip->trace_work, msecs_to_jiffies(5000));
> > +}
> > +
Hmm, so basically you only want to call the work when tracing is
enabled? That's what I was thinking should be enabled by the reg and
unreg functions. That is, the reg should enabled the delayed work, and
the unreg should disable it from being called.
This looks like it calls the work regardless of if tracing is enabled
or not. Why waste the cycles when tracing is disabled?
-- Steve
^ permalink raw reply
* Re: [PATCH v4 3/3] PCI: dw-rockchip: Add pcie_ltssm_state_transition trace support
From: Steven Rostedt @ 2026-02-24 14:11 UTC (permalink / raw)
To: Shawn Lin
Cc: Manivannan Sadhasivam, Bjorn Helgaas, linux-rockchip, linux-pci,
linux-trace-kernel, linux-doc, Masami Hiramatsu
In-Reply-To: <1769047340-113287-4-git-send-email-shawn.lin@rock-chips.com>
On Thu, 22 Jan 2026 10:02:20 +0800
Shawn Lin <shawn.lin@rock-chips.com> wrote:
>
> +#ifdef CONFIG_TRACING
> +static void rockchip_pcie_ltssm_trace_work(struct work_struct *work)
> +{
> + struct rockchip_pcie *rockchip = container_of(work,
> + struct rockchip_pcie,
> + trace_work.work);
> + struct dw_pcie *pci = &rockchip->pci;
> + enum dw_pcie_ltssm state;
> + u32 i, l1ss, prev_val = DW_PCIE_LTSSM_UNKNOWN, rate, val;
> +
> + if (!pci_ltssm_tp_enabled())
> + goto skip_trace;
You can use:
if (!trace_pcie_ltssm_state_transition_enabled())
goto skip_trace;
The above is a static branch. That means when tracing is disabled, it is
basically:
goto skip_trace;
and when tracing is enabled it is a nop.
-- Steve
> +
> + for (i = 0; i < PCIE_DBG_LTSSM_HISTORY_CNT; i++) {
> + val = rockchip_pcie_readl_apb(rockchip,
> + PCIE_CLIENT_DBG_FIFO_STATUS);
> + rate = FIELD_GET(PCIE_DBG_FIFO_RATE_MASK, val);
> + l1ss = FIELD_GET(PCIE_DBG_FIFO_L1SUB_MASK, val);
> + val = FIELD_GET(PCIE_LTSSM_STATUS_MASK, val);
> +
> + /*
> + * Hardware Mechanism: The ring FIFO employs two tracking
> + * counters:
> + * - 'last-read-point': maintains the user's last read position
> + * - 'last-valid-point': tracks the HW's last state update
> + *
> + * Software Handling: When two consecutive LTSSM states are
> + * identical, it indicates invalid subsequent data in the FIFO.
> + * In this case, we skip the remaining entries. The dual counter
> + * design ensures that on the next state transition, reading can
> + * resume from the last user position.
> + */
> + if ((i > 0 && val == prev_val) || val > DW_PCIE_LTSSM_RCVRY_EQ3)
> + break;
> +
> + state = prev_val = val;
> + if (val == DW_PCIE_LTSSM_L1_IDLE) {
> + if (l1ss == 2)
> + state = DW_PCIE_LTSSM_L1_2;
> + else if (l1ss == 1)
> + state = DW_PCIE_LTSSM_L1_1;
> + }
> +
> + trace_pcie_ltssm_state_transition(dev_name(pci->dev),
> + dw_pcie_ltssm_status_string(state),
> + ((rate + 1) > pci->max_link_speed) ?
> + PCI_SPEED_UNKNOWN : PCIE_SPEED_2_5GT + rate);
> + }
> +
> +skip_trace:
> + schedule_delayed_work(&rockchip->trace_work, msecs_to_jiffies(5000));
> +}
> +
> +static void rockchip_pcie_ltssm_trace(struct rockchip_pcie *rockchip,
> + bool enable)
> +{
> + if (enable) {
> + rockchip_pcie_writel_apb(rockchip,
> + PCIE_CLIENT_DBG_TRANSITION_DATA,
> + PCIE_CLIENT_DBG_FIFO_PTN_HIT_D0);
> + rockchip_pcie_writel_apb(rockchip,
> + PCIE_CLIENT_DBG_TRANSITION_DATA,
> + PCIE_CLIENT_DBG_FIFO_PTN_HIT_D1);
> + rockchip_pcie_writel_apb(rockchip,
> + PCIE_CLIENT_DBG_TRANSITION_DATA,
> + PCIE_CLIENT_DBG_FIFO_TRN_HIT_D0);
> + rockchip_pcie_writel_apb(rockchip,
> + PCIE_CLIENT_DBG_TRANSITION_DATA,
> + PCIE_CLIENT_DBG_FIFO_TRN_HIT_D1);
> + rockchip_pcie_writel_apb(rockchip,
> + PCIE_CLIENT_DBG_EN,
> + PCIE_CLIENT_DBG_FIFO_MODE_CON);
> +
> + INIT_DELAYED_WORK(&rockchip->trace_work,
> + rockchip_pcie_ltssm_trace_work);
> + schedule_delayed_work(&rockchip->trace_work, 0);
> + } else {
> + rockchip_pcie_writel_apb(rockchip,
> + PCIE_CLIENT_DBG_DIS,
> + PCIE_CLIENT_DBG_FIFO_MODE_CON);
> + cancel_delayed_work_sync(&rockchip->trace_work);
> + }
> +}
> +#else
> +static void rockchip_pcie_ltssm_trace(struct rockchip_pcie *rockchip,
> + bool enable)
> +{
> +}
> +#endif
> +
> static void rockchip_pcie_enable_ltssm(struct rockchip_pcie *rockchip)
> {
> rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_ENABLE_LTSSM,
> @@ -291,6 +398,9 @@ static int rockchip_pcie_start_link(struct dw_pcie *pci)
> * 100us as we don't know how long should the device need to reset.
> */
> msleep(PCIE_T_PVPERL_MS);
> +
> + rockchip_pcie_ltssm_trace(rockchip, true);
> +
> gpiod_set_value_cansleep(rockchip->rst_gpio, 1);
>
> return 0;
> @@ -301,6 +411,7 @@ static void rockchip_pcie_stop_link(struct dw_pcie *pci)
> struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
>
> rockchip_pcie_disable_ltssm(rockchip);
> + rockchip_pcie_ltssm_trace(rockchip, false);
> }
>
> static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
^ permalink raw reply
* Re: [PATCH v4 1/3] PCI: trace: Add PCI controller LTSSM transition tracepoint
From: Steven Rostedt @ 2026-02-24 14:08 UTC (permalink / raw)
To: Shawn Lin
Cc: Manivannan Sadhasivam, Bjorn Helgaas, linux-rockchip, linux-pci,
linux-trace-kernel, linux-doc, Masami Hiramatsu
In-Reply-To: <1769047340-113287-2-git-send-email-shawn.lin@rock-chips.com>
On Thu, 22 Jan 2026 10:02:18 +0800
Shawn Lin <shawn.lin@rock-chips.com> wrote:
> +bool pci_ltssm_tp_enabled(void)
> +{
> + return atomic_read(&pcie_ltssm_tp_enabled) > 0;
> +}
> +EXPORT_SYMBOL(pci_ltssm_tp_enabled);
> +
> +int pci_ltssm_tp_reg(void)
> +{
> + atomic_inc(&pcie_ltssm_tp_enabled);
> + return 0;
> +}
> +
> +void pci_ltssm_tp_unreg(void)
> +{
> + atomic_dec(&pcie_ltssm_tp_enabled);
> +}
This seems totally unnecessary. Why the atomic operations? Why not just
use:
if (trace_pcie_ltssm_state_transition_enabled()) ...
?
-- Steve
^ permalink raw reply
* Re: [PATCH v4 0/3] PCI Controller event and LTSSM tracepoint support
From: Steven Rostedt @ 2026-02-24 14:06 UTC (permalink / raw)
To: Shawn Lin
Cc: Bjorn Helgaas, linux-rockchip, linux-pci, linux-trace-kernel,
linux-doc, Masami Hiramatsu, Manivannan Sadhasivam
In-Reply-To: <5549b86e-36f7-e5b2-e16c-f70e328ca6c2@rock-chips.com>
On Tue, 24 Feb 2026 16:49:47 +0800
Shawn Lin <shawn.lin@rock-chips.com> wrote:
> I'd appreciate it if you could share any concerns you might have about
> v4. :)
Sure. I'll reply to the patches.
-- Steve
^ permalink raw reply
* Re: [RFC PATCH bpf-next 3/3] selftests/bpf: add tests for kprobe.session optimization
From: Jiri Olsa @ 2026-02-24 13:12 UTC (permalink / raw)
To: Andrey Grodzovsky
Cc: bpf, ast, daniel, andrii, rostedt, linux-trace-kernel,
linux-open-source
In-Reply-To: <20260223215113.924599-4-andrey.grodzovsky@crowdstrike.com>
On Mon, Feb 23, 2026 at 04:51:13PM -0500, Andrey Grodzovsky wrote:
> Add two new subtests to kprobe_multi_test to validate the
> kprobe.session exact function name optimization:
SNIP
> +static void test_session_errors(void)
> +{
> + struct kprobe_multi_session_errors *skel = NULL;
> + struct bpf_link *link_wildcard = NULL;
> + struct bpf_link *link_exact = NULL;
> + int err_wildcard, err_exact;
> +
> + skel = kprobe_multi_session_errors__open_and_load();
> + if (!ASSERT_OK_PTR(skel, "kprobe_multi_session_errors__open_and_load"))
> + return;
> +
> + /*
> + * Test error code consistency: both wildcard (slow path) and exact name
> + * (fast path) should return the same error code (ENOENT) for non-existent
> + * functions. This protects against future kernel changes that might alter
> + * error return values.
> + */
> +
> + /* Try to attach with non-existent wildcard pattern (slow path) */
> + link_wildcard = bpf_program__attach(skel->progs.test_nonexistent_wildcard);
> + err_wildcard = -errno;
> + ASSERT_ERR_PTR(link_wildcard, "attach_nonexistent_wildcard");
> + ASSERT_EQ(err_wildcard, -ENOENT, "wildcard_error_enoent");
> +
> + /* Try to attach with non-existent exact name (fast path) */
> + link_exact = bpf_program__attach(skel->progs.test_nonexistent_exact);
> + err_exact = -errno;
> + ASSERT_ERR_PTR(link_exact, "attach_nonexistent_exact");
> + ASSERT_EQ(err_exact, -ENOENT, "exact_error_enoent");
> +
> + /*
> + * Verify both paths return identical error codes - this is critical for
> + * API consistency and prevents user code from breaking when switching
> + * between wildcard patterns and exact function names.
> + */
> + ASSERT_EQ(err_wildcard, err_exact, "error_consistency");
> +
> + kprobe_multi_session_errors__destroy(skel);
there's already subtest for attach failures (test_attach_api_fails),
so maybe let's put this over there?
SNIP
> diff --git a/tools/testing/selftests/bpf/progs/kprobe_multi_session_syms.c b/tools/testing/selftests/bpf/progs/kprobe_multi_session_syms.c
> new file mode 100644
> index 000000000000..6a4bd57af1fc
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/kprobe_multi_session_syms.c
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Test kprobe.session with exact function names to verify syms[] optimization */
> +#include <vmlinux.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +#include <stdbool.h>
> +
> +char _license[] SEC("license") = "GPL";
> +
> +int pid = 0;
> +
> +/* Results for each function: incremented on entry and return */
> +__u64 test1_count = 0;
> +
> +/* Track entry vs return */
> +bool test1_return = false;
> +
> +/*
> + * No tests in here, just to trigger 'bpf_fentry_test*'
> + * through tracing test_run
> + */
> +SEC("fentry/bpf_modify_return_test")
> +int BPF_PROG(trigger)
> +{
> + return 0;
> +}
> +
> +/*
> + * Test 1: Exact function name (no wildcards) - uses fast syms[] path
> + * This should attach via opts.syms array, bypassing kallsyms parsing
> + */
> +SEC("kprobe.session/bpf_fentry_test1")
> +int test_kprobe_syms_1(struct pt_regs *ctx)
perhaps we could execute this as part of test_session_skel_api test?
seems like we could put this directly to progs/kprobe_multi_session.c and
call session_check(ctx) and change test_results validation accordingly
thanks,
jirka
> +{
> + if (bpf_get_current_pid_tgid() >> 32 != pid)
> + return 0;
> +
> + test1_count++;
> +
> + /* Check if this is return probe */
> + if (bpf_session_is_return(ctx))
> + test1_return = true;
> +
> + return 0; /* Always execute return probe */
> +}
> --
> 2.34.1
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox