Add task latency over budget RV monitor
Subsystems (4)
v7 cover Add task latency over budget RV monitor
From: Wen Yang <[email protected]>
This series introduces tlob (task latency over budget), a per-task hybrid automaton RV monitor that measures elapsed wall-clock time across a user-delimited code section and emits an error when the elapsed time exceeds a configurable budget.
The monitor tracks four states (running, waiting, sleeping, stopped) driven by sched_switch and sched_wakeup tracepoints. A single clock invariant, clk_elapsed < BUDGET_NS(), is enforced by a per-task hrtimer (HRTIMER_MODE_REL_HARD). On expiry error_env_tlob is emitted; which phase consumed the budget (running, waiting or sleeping) is recovered from event_tlob in user space, see monitor_tlob.rst.
Tasks are registered for monitoring by writing to the tracefs monitor file:
echo "p:myregion /path/to/binary:OFFSET_START OFFSET_STOP threshold=NS" \
>> /sys/kernel/tracing/rv/monitors/tlob/monitor
Two uprobes delimit the measured section without modifying the target binary. Multiple uprobe pairs can be active simultaneously. A task may repeat the START/STOP sequence any number of times: the entry parks in the automaton's "stopped" state and later restarts in place, reusing the same pool slot (see Patch 11).
The series opens with five patches that fix the framework on their own and apply to the base independently of tlob; the rest add the infrastructure tlob needs and then tlob itself.
Patches 1 and 2 fix the HA environment lifecycle and must stay in this order. Patch 1 stops storing the ENV_INVALID_VALUE sentinel: monitors now reset their stored variables to a real clock anchor on start. Patch 2 then clamps the timer remainder and rewrites the invariant check as store + expire_ns >= time_ns so neither side underflows. That rewrite overflows when the stored value is still the sentinel, so it has to follow Patch 1: the reverse order leaves a window in which a monitor whose clock was never anchored reports spurious violations. Patch 3 adds KUnit coverage for both paths.
Patch 4 is the framework fix tlob depends on. tlob arms its hrtimer from sched_switch and destroys storage from the uprobe and exit paths, so a reader that resolved the storage before hash_del_rcu() can re-arm past the teardown's cancel and leave the timer queued on a pool slot that is freed a grace period later:
CPU2 (sched event for T) CPU1 (echo -:NAME >> monitor)
ha_setup_invariants() reads OPEN
hrtimer_start(): waits on xchg(&rec->state, TLOB_RETIRED)
hrtimer_cpu_base->lock cancel (takes the same lock first)
hash_del_rcu(), call_rcu()
enqueues the hrtimer
GP ends, slot returned to the pool
Both sides contend for the same hrtimer_cpu_base->lock, so a normal lock handoff is enough. The state recheck in ha_setup_invariants() narrows the window but cannot close it: the reader resolved the storage before the unlink, so the first point past it is the RCU callback. Patch 5 adds KUnit coverage for the new free hook.
Series structure
================
Fixes to the framework, independent of tlob:
Patch 1: rv: Force environment reset action on HA monitor start
Cherry-picked from Gabriele's patch. Instead of starting with the ENV_INVALID_VALUE sentinel and waiting for the first explicit reset, ha_monitor_reset_all_stored() now calls the monitor-supplied ha_reset_env(), which sets a fresh clock anchor for ns-granularity clocks. Monitors with no stored variable define HA_NO_RESET, emitted automatically by rvgen. This removes the invalid-env special case from constraint checks and lets tlob re-anchor its clock on restart. Carried unchanged from Gabriele's reply to v6; it is not upstream yet. If it lands through his tree first, it will be dropped from this series.
Patch 2: rv: Fix ha_monitor timer and invariant underflow
Clamp the HA timer remainder to 0 (fire immediately) when the budget already elapsed, and rewrite the invariant check so neither side can underflow. Without it a late state transition arms the timer far into the future and the monitor stops enforcing the budget.
Patch 3: rv: Add a KUnit test for the HA timer underflow
Covers both paths: the invariant holds with the uptime below the budget, and a timer armed after the budget elapsed expires immediately.
Patch 4: rv: Disarm HA timers on the storage free path
Add da_monitor_free_hook(), called right before a per-object storage is freed, and have HA cancel its timer there; kfree_rcu() becomes call_rcu() so the RCU free path has a callback to hook.
Patch 5: rv: Add a KUnit test for the storage free path
KUnit coverage for the Patch 4 free hook.
New infrastructure:
Patch 6: rv: Make da_monitor_reset_hook and EVENT_NONE_LBL overridable
#ifndef guards for da_monitor_reset_hook and EVENT_NONE_LBL.
Patch 7: rv: Introduce DA_MON_ALLOCATION_STRATEGY
Compile-time allocation strategy selector for per-object DA storage:
DA_ALLOC_AUTO (default kmalloc), DA_ALLOC_POOL (pre-allocated mempool), DA_ALLOC_MANUAL (caller-managed). DA_MON_POOL_SIZE alone selects pool mode.
Patch 8: rv: Add per-object teardown hook da_extra_cleanup
Split out of the tlob patch per Gabriele's request, so the framework change can be reviewed on its own.
Patch 9: rv: Add generic uprobe infrastructure for RV monitors
Thin wrapper over uprobe_consumer for path resolution, registration, and safe synchronous teardown. struct rv_uprobe embeds uprobe_consumer directly and holds an inode reference for the probed binary for its lifetime. Also introduces config RV_UPROBE and the Makefile rule to keep git bisect cleanly passing across the series.
tlob:
Patch 10: rv: Add tlob model DOT file
Graphviz DOT specification of the tlob hybrid automaton.
Patch 11: rv: Add tlob hybrid automaton monitor
Main tlob implementation.
Patch 12: rv: Add KUnit tests for the tlob monitor
Parser tests reached through a const rv_tlob_kunit_ops struct of function pointers, covering valid lines, malformed inputs, and out-of-range thresholds.
Patch 13: selftests/verification: Add tlob selftests
Eleven ftrace-style selftests covering binding, violation, multi-binding, multi-task sharing of one binding, overlap registration rejection, unbind/disable on live tasks, start/stop restart, and the per-state breakdown derived from event_tlob.
Patch 14: selftests/ftrace: Walk up parent dirs to find test.d/functions
The ftracetest walk-up change needed to run the verification tests from the verification subdirectory.
Changes since v6
================
The series is now grouped: the framework fixes come first, then the new infrastructure, then tlob.
New in this revision:
- Patches 1 and 2: the HA environment lifecycle and timer fixes, in that order (see above). Patch 3 adds KUnit coverage for both.
- Patch 4 (was 1): the HA timer teardown fix, found while auditing tlob's own teardown. The gap is in the framework; a monitor cannot close it. Patch 5 adds KUnit coverage, and Patch 8 splits the per-object teardown hook out of the tlob patch.
- Dropped detail_env_tlob and the per-state accounting. Which phase consumed the budget is now derived from event_tlob in user space; monitor_tlob.rst documents the recipe. This removes the per-record lock and the accumulator from the hot path, and tlob no longer deviates from "run a model and spot violations" (Gabriele, v2).
- The model gives stopped a self-loop for every scheduler event, and the sched handlers call da_handle_event() unconditionally. The source-side gate that skipped events while stopped is gone; the logic lives in the model (Gabriele, v6).
- tlob_sym.c (a 235-line ELF parser) is replaced by a sym_offset() shell helper built on readelf (Gabriele, v3).
- tlob_preempt_work, identical to tlob_busy_work, is dropped and the busy-wait body is shared (Gabriele, v3).
- The KUnit and DOT commit messages were rewritten as requested
(Gabriele, v6).
- Dropped three patches that no longer stand on their own: "rv: Fix ha_invariant_passed_ns silent bypass of invariant check" (Patch 1 makes it unnecessary), "rv: Check the decoded fields in the tlob parser tests" and "selftests/verification: Fix up the tlob selftests" (folded into the patches they fix, so each patch is complete on its own).
Also folded into the patches they belong to, from earlier review rounds:
the DA_MON_POOL_SIZE dual-macro simplification and DA_ALLOC_MANUAL (Gabriele), the invariant-check rewrite (sashiko-bot), and the tlob teardown/error-path fixes: security_locked_down(LOCKDOWN_TRACEFS) in tlob_monitor_open(), the single tlob_record_state word with TLOB_RETIRED as the only licence to free, tlob_add_uprobe()'s rollback running the full nosync/quiesce/sync/reap sequence, and tlob_remove_all_uprobes() holding tlob_uprobe_mutex across the sync and the reap.
Testing
=======
Environment: x86_64, CONFIG_PREEMPT_RT=y, kernel built with virtme-ng (vng --build), selftests built on the host and run inside the VM via ftracetest --rv ../verification (the verificationtest-ktap entry point, with logs redirected to /tmp):
- KUnit: rv_mon suite (CONFIG_RV_MONITORS_KUNIT_TEST), rv_test_tlob case covering the uprobe-line parser
- 11 ftrace-style selftests: uprobe_bind, uprobe_violation, uprobe_no_event, uprobe_multi, uprobe_multi_instance, uprobe_detail_{running,sleeping,waiting}, uprobe_restart, uprobe_overlap_register, uprobe_restart_violation
Both suites pass. The pre-existing rv_* tests in the same directory and the rest of the rv_mon KUnit suite (nomiss, sco, sssw, sts, opid, pagefault, sleep) keep passing as well.
v6: https://lore.kernel.org/all/[email protected]/
v4: https://lore.kernel.org/all/[email protected]/
v3: https://lore.kernel.org/all/[email protected]/
v2: https://lore.kernel.org/all/[email protected]/
v1: https://lore.kernel.org/all/[email protected]/
Gabriele Monaco (1):
rv: Force environment reset action on HA monitor start
Wen Yang (13):
rv: Fix ha_monitor timer and invariant underflow
rv: Add a KUnit test for the HA timer underflow
rv: Disarm HA timers on the storage free path
rv: Add a KUnit test for the storage free path
rv: Make da_monitor_reset_hook and EVENT_NONE_LBL overridable
rv: Introduce DA_MON_ALLOCATION_STRATEGY
rv: Add per-object teardown hook da_extra_cleanup
rv: Add generic uprobe infrastructure for RV monitors
rv: Add tlob model DOT file
rv: Add tlob hybrid automaton monitor
rv: Add KUnit tests for the tlob monitor selftests/verification: Add tlob selftests selftests/ftrace: Walk up parent dirs to find test.d/functions
Documentation/trace/rv/index.rst | 1 + Documentation/trace/rv/monitor_tlob.rst | 154 +++ include/rv/da_monitor.h | 154 ++- include/rv/ha_monitor.h | 61 +- include/rv/rv_uprobe.h | 35 + kernel/trace/rv/Kconfig | 5 + kernel/trace/rv/Makefile | 2 + kernel/trace/rv/monitors/nomiss/nomiss.c | 2 +- kernel/trace/rv/monitors/opid/opid.h | 1 + kernel/trace/rv/monitors/tlob/Kconfig | 12 + kernel/trace/rv/monitors/tlob/tlob.c | 1049 +++++++++++++++++ kernel/trace/rv/monitors/tlob/tlob.h | 142 +++ kernel/trace/rv/monitors/tlob/tlob_kunit.c | 151 +++ kernel/trace/rv/monitors/tlob/tlob_kunit.h | 18 + kernel/trace/rv/monitors/tlob/tlob_trace.h | 20 + kernel/trace/rv/rv_monitors_test.c | 162 +++ kernel/trace/rv/rv_trace.h | 1 + kernel/trace/rv/rv_uprobe.c | 56 + tools/testing/selftests/ftrace/ftracetest | 19 +- .../testing/selftests/verification/.gitignore | 1 + tools/testing/selftests/verification/Makefile | 4 + tools/testing/selftests/verification/config | 3 + .../selftests/verification/test.d/functions | 9 + .../test.d/tlob/run_tlob_tests.sh | 32 + .../verification/test.d/tlob/uprobe_bind.tc | 47 + .../test.d/tlob/uprobe_breakdown.tc | 136 +++ .../test.d/tlob/uprobe_disable_live.tc | 79 ++ .../verification/test.d/tlob/uprobe_multi.tc | 58 + .../test.d/tlob/uprobe_multi_instance.tc | 65 + .../test.d/tlob/uprobe_no_event.tc | 17 + .../test.d/tlob/uprobe_overlap_register.tc | 66 ++ .../test.d/tlob/uprobe_restart.tc | 66 ++ .../test.d/tlob/uprobe_restart_violation.tc | 48 + .../test.d/tlob/uprobe_unbind_live.tc | 65 + .../test.d/tlob/uprobe_violation.tc | 53 + .../selftests/verification/tlob_target.c | 127 ++ tools/verification/models/tlob.dot | 27 + tools/verification/rvgen/rvgen/dot2c.py | 2 + 38 files changed, 2930 insertions(+), 20 deletions(-) create mode 100644 Documentation/trace/rv/monitor_tlob.rst create mode 100644 include/rv/rv_uprobe.h create mode 100644 kernel/trace/rv/monitors/tlob/Kconfig create mode 100644 kernel/trace/rv/monitors/tlob/tlob.c create mode 100644 kernel/trace/rv/monitors/tlob/tlob.h create mode 100644 kernel/trace/rv/monitors/tlob/tlob_kunit.c create mode 100644 kernel/trace/rv/monitors/tlob/tlob_kunit.h create mode 100644 kernel/trace/rv/monitors/tlob/tlob_trace.h create mode 100644 kernel/trace/rv/rv_uprobe.c create mode 100755 tools/testing/selftests/verification/test.d/tlob/run_tlob_tests.sh create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_bind.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_breakdown.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_disable_live.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_multi.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_multi_instance.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_no_event.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_overlap_register.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_restart.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_restart_violation.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_unbind_live.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_violation.tc create mode 100644 tools/testing/selftests/verification/tlob_target.c create mode 100644 tools/verification/models/tlob.dot
Signature · 1 line
2.25.1
PATCH v7 1/14 Force environment reset action on HA monitor start
From: Gabriele Monaco <[email protected]>
Currently the monitor variables with a storage (e.g. clocks) are initialised as invalid, then the first explicit reset() sets them as valid for constraint. This only adds extra complexity to handle the invalid case in constraints check.
Apply a reset() by default when starting monitors instead of resetting to an invalid state. If monitors have no stored variable (hence no reset function) add a macro HA_NO_RESET to stub it, this is all transparently handled by rvgen.
The reset() action on ns-granularity clocks needs the (possibly cached) current time, read it directly there. This might cause a double call to ktime_get_ns() within the same da_handle_start_run_event() but will occur only the first time and is harmless.
--- include/rv/ha_monitor.h | 8 +++++++- kernel/trace/rv/monitors/opid/opid.h | 1 + tools/verification/rvgen/rvgen/dot2c.py | 2 ++ 3 files changed, 10 insertions(+), 1 deletion(-)
include/rv/ha_monitor.h +7 -1
@@ -159,19 +159,25 @@ static void ha_monitor_destroy(void)/* Should be supplied by the monitor */ static u64 ha_get_env (struct ha_monitor *ha_mon,enum envs env,u64 time_ns); +static void ha_reset_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); +#ifdef HA_NO_RESET +static void ha_reset_env (struct ha_monitor *ha_mon,enum envs env,u64 time_ns) { } +#endif /* * 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) { +u64 time_ns =ha_get_ns (); +for (int i =0 ; i <ENV_MAX_STORED ; i++) -WRITE_ONCE (ha_mon->env_store[i],ENV_INVALID_VALUE ); +ha_reset_env (ha_mon, i, time_ns); }/*
kernel/trace/rv/monitors/opid/opid.h +1 -0
@@ -28,6 +28,7 @@ enum envs_opid { };_Static_assert (env_max_stored_opid <=MAX_HA_ENV_LEN ,"Not enough slots" ); +#define HA_NO_RESET struct automaton_opid {char *state_names[state_max_opid];
tools/verification/rvgen/rvgen/dot2c.py +2 -0
@@ -90,6 +90,8 @@ class Dot2c(Automata): ' "Not enough slots");') if {"ns", "us", "ms", "s"}.intersection(self.env_types.values()): buff.append("#define HA_CLK_NS") + if len(self.env_stored) == 0: + buff.append("#define HA_NO_RESET") buff.append("") return buff
Signature · 1 line
2.25.1
PATCH v7 2/14 Fix ha_monitor timer and invariant underflow
In reply to [email protected]From: Wen Yang <[email protected]>
ha_start_timer_ns() and ha_start_timer_jiffy() compute the remaining budget as expire - passed. Once the elapsed time passed exceeds the budget expire (e.g. a state transition after the invariant should have fired, before the timer callback ran), the unsigned subtraction underflows to a huge value and the timer is scheduled far into the future instead of expiring immediately.
ha_check_invariant_ns() computes store >= time_ns - expire_ns, which underflows when the system uptime is still below expire_ns (a monitor armed early in boot with a large budget), reporting a false violation.
Clamp the timer remainder to 0 (fire immediately) when passed >= expire, and replace the invariant check as store + expire_ns >= time_ns so neither side can underflow.
--- include/rv/ha_monitor.h | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
include/rv/ha_monitor.h +14 -5
@@ -351,7 +351,7 @@ static inline void ha_reset_clk_ns(struct ha_monitor *ha_mon, enum envs env, u64static inline bool ha_check_invariant_ns (struct ha_monitor *ha_mon,enum envs env,u64 time_ns,u64 expire_ns) { -return READ_ONCE (ha_mon->env_store[env]) >= time_ns - expire_ns; +return READ_ONCE (ha_mon->env_store[env]) + expire_ns >= time_ns; }/* * ha_invariant_passed_ns - prepare the invariant and return the time since reset
@@ -410,6 +410,11 @@ static inline void ha_inv_to_guard(struct ha_monitor *ha_mon, enum envs env,WRITE_ONCE (ha_mon->env_store[env],READ_ONCE (ha_mon->env_store[env]) - value); } +static inline u64 ha_time_left (u64 passed,u64 expire) +{ +return passed < expire ? expire - passed :0 ; +} +#if HA_TIMER_TYPE ==HA_TIMER_WHEEL /* * Helper functions to handle the monitor timer.
@@ -439,15 +444,17 @@ static inline void ha_start_timer_jiffy(struct ha_monitor *ha_mon, enum envs env {u64 passed =ha_invariant_passed_jiffy (ha_mon, env, time_ns); -mod_timer (&ha_mon->timer,get_jiffies_64 () + expire - passed); +mod_timer (&ha_mon->timer, +get_jiffies_64 () +ha_time_left (passed, expire)); }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, time_ns); +u64 delay =ha_time_left (passed, expire);ha_start_timer_jiffy (ha_mon,ENV_MAX_STORED , -nsecs_to_jiffies (expire - passed +TICK_NSEC -1 ), time_ns); +nsecs_to_jiffies (delay +TICK_NSEC -1 ), time_ns); }/* * ha_cancel_timer - Cancel the timer
@@ -494,7 +501,8 @@ static inline void ha_start_timer_ns(struct ha_monitor *ha_mon, enum envs env,if (RV_MON_TYPE ==RV_MON_PER_CPU ) mode |=HRTIMER_MODE_PINNED ; -hrtimer_start (&ha_mon->hrtimer,ns_to_ktime (expire - passed), mode); +hrtimer_start (&ha_mon->hrtimer, +ns_to_ktime (ha_time_left (passed, expire)), mode); }static inline void ha_start_timer_jiffy (struct ha_monitor *ha_mon,enum envs env,u64 expire,u64 time_ns)
@@ -502,7 +510,8 @@ static inline void ha_start_timer_jiffy(struct ha_monitor *ha_mon, enum envs envu64 passed =ha_invariant_passed_jiffy (ha_mon, env, time_ns);ha_start_timer_ns (ha_mon,ENV_MAX_STORED , -jiffies_to_nsecs (expire - passed), time_ns); +jiffies_to_nsecs (ha_time_left (passed, expire)), + time_ns); }/* * ha_cancel_timer - Cancel the timer
Signature · 1 line
2.25.1
PATCH v7 3/14 Add a KUnit test for the HA timer underflow
In reply to [email protected]From: Wen Yang <[email protected]>
Add rvtest, a minimal per-object HA monitor, and a test for both underflows fixed by the previous patch: an invariant checked while the uptime is below the budget holds, and a timer armed after the budget elapsed expires now.
--- kernel/trace/rv/rv_monitors_test.c | 122 +++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+)
kernel/trace/rv/rv_monitors_test.c +122 -0
@@ -13,6 +13,87 @@#include <linux/rv.h> #include "rv.h" +#include <rv_trace.h> + +/* rvtest: a minimal per-object HA monitor driven directly by the tests. */ +#define MONITOR_NAME rvtest +#define RV_MON_TYPE RV_MON_PER_OBJ +#define HA_TIMER_TYPE HA_TIMER_HRTIMER +typedef void *monitor_target ; + +/* The framework calls these for every monitor; this one reports nothing. */ +#define trace_event_rvtest (...) do { } while (0) +#define trace_error_rvtest (...) do { } while (0) +#define trace_error_env_rvtest (...) do { } while (0) + +enum states_rvtest { + idle_rvtest, + armed_rvtest, + state_max_rvtest, +}; + +#define INVALID_STATE state_max_rvtest + +enum events_rvtest { + start_rvtest, + event_max_rvtest, +}; + +enum envs_rvtest { + clk_rvtest, + env_max_rvtest, + env_max_stored_rvtest = env_max_rvtest, +}; + +#define HA_CLK_NS + +struct automaton_rvtest { +char *state_names[state_max_rvtest]; +char *event_names[event_max_rvtest]; +char *env_names[env_max_rvtest]; +unsigned char function[state_max_rvtest][event_max_rvtest]; +unsigned char initial_state; +bool final_states[state_max_rvtest]; +}; + +static const struct automaton_rvtest automaton_rvtest = { + .state_names = {"idle" ,"armed" }, + .event_names = {"start" }, + .env_names = {"clk" }, + .function = { + { armed_rvtest }, + {INVALID_STATE }, + }, + .initial_state = idle_rvtest, + .final_states = {1 ,0 }, +}; + +#include <rv/ha_monitor.h> + +#define RVTEST_BUDGET_NS (300ULL * NSEC_PER_MSEC) + +static u64 ha_get_env (struct ha_monitor *ha_mon,enum envs_rvtest env, +u64 time_ns) +{ +return ha_get_clk_ns (ha_mon, env, time_ns); +} + +static void ha_reset_env (struct ha_monitor *ha_mon,enum envs_rvtest env, +u64 time_ns) +{ +ha_reset_clk_ns (ha_mon, env, time_ns); +} + +/* The tests arm the timer themselves. */ +static bool ha_verify_constraint (struct ha_monitor *ha_mon, +enum states curr_state,enum events event, +enum states next_state,u64 time_ns) +{ +return true; +} + +static const struct rv_kunit_mon rvtest_mon =RV_MON_OPS_INIT (); +/* * An easy way to pass the context is to use kunit_get_current_test()->priv, * but this doesn't always work (e.g. a reactor running from another context
@@ -146,6 +227,46 @@ static void rv_test_dummy(struct kunit *test) active_ctx =NULL ; } +/* +* rv_test_rvtest_underflow - the HA clock math must not underflow as u64 +* +* An invariant checked while the uptime is below the budget holds, and a +* timer armed after the budget elapsed expires now. +*/ +static void rv_test_rvtest_underflow (struct kunit *test) +{ +struct ha_monitor *ha_mon; +struct da_monitor *da_mon; +const int id =4242 ; + +prepare_test (test, &rvtest_mon); +da_handle_start_run_event (id,NULL , start_rvtest); + +scoped_guard (rcu) + da_mon =da_get_monitor (id,NULL ); +KUNIT_ASSERT_NOT_NULL (test, da_mon); + ha_mon =to_ha_monitor (da_mon); + +/* Uptime below the budget: time_ns - expire_ns would underflow. */ +ha_reset_clk_ns (ha_mon, clk_rvtest,0 ); +KUNIT_EXPECT_TRUE (test,ha_check_invariant_ns (ha_mon, clk_rvtest,0 , +RVTEST_BUDGET_NS )); + +/* +* Budget already elapsed: expire - passed would underflow and queue +* the timer at KTIME_SEC_MAX. +*/ +da_monitor_reset (&ha_mon->da_mon); +ha_reset_clk_ns (ha_mon, clk_rvtest,0 ); +ha_start_timer_ns (ha_mon, clk_rvtest,RVTEST_BUDGET_NS , +RVTEST_BUDGET_NS +NSEC_PER_SEC ); +KUNIT_EXPECT_TRUE (test, +ktime_before (hrtimer_get_expires (&ha_mon->hrtimer), +ktime_get () +NSEC_PER_SEC )); + +ha_cancel_timer_sync (ha_mon); +} +#include "monitors/sco/sco_kunit.c" #include "monitors/sssw/sssw_kunit.c" #include "monitors/sts/sts_kunit.c"
@@ -163,6 +284,7 @@ static struct kunit_case rv_mon_test_cases[] = {KUNIT_CASE (rv_test_nomiss),KUNIT_CASE (rv_test_pagefault),KUNIT_CASE (rv_test_sleep), + KUNIT_CASE(rv_test_rvtest_underflow), {} };
Signature · 1 line
2.25.1
PATCH v7 4/14 Disarm HA timers on the storage free path
In reply to [email protected]From: Wen Yang <[email protected]>
da_destroy_storage() cancels the HA timer through da_monitor_reset_hook() and only then unlinks the storage with hash_del_rcu(). A handler on another CPU that looked the storage up before the unlink can still call ha_start_timer_*() after the cancel. The timer then stays queued on a storage that kfree_rcu() frees a grace period later, and fires on freed memory, reporting a violation for an object that no longer exists.
A monitor cannot close this from its own handlers: any cancel it issues precedes the unlink. The first point no reader can race with is the RCU callback that frees the storage.
Add da_monitor_free_hook(), called right before a per-object storage is freed, from the RCU callback and from da_monitor_destroy(). HA monitors implement it with ha_cancel_timer_sync(). Replace kfree_rcu() with call_rcu() so there is a callback to hook.
A storage whose monitor never started has a zeroed timer, and hrtimer_cancel() dereferences timer->base. nomiss pre-creates one such storage per possible CPU. Skip them with the new ha_timer_ready().
--- include/rv/da_monitor.h | 23 ++++++++++++++++++++++- include/rv/ha_monitor.h | 26 ++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-)
include/rv/da_monitor.h +22 -1
@@ -67,6 +67,17 @@ static struct rv_monitor rv_this;#define da_monitor_sync_hook ()#endif +/* +* Called on the last reference to a storage, right before it is freed. +* Monitors embedding a timer disarm it here: an earlier point races with a +* reader that resolved the storage before hash_del_rcu(), and +* tracepoint_synchronize_unregister() only covers tracepoint probes. +* Default: no-op. +*/ +#ifndef da_monitor_free_hook +#define da_monitor_free_hook (da_mon) +#endif +/* * Type for the target id, default to int but can be overridden. * A long type can work as hash table key (PER_OBJ) but will be downgraded to
@@ -543,6 +554,15 @@ static inline monitor_target da_get_target_by_id(da_id_type id)return mon_storage->target; } +static void da_kfree_cb (struct rcu_head *head) +{ +struct da_monitor_storage *ms = + container_of(head, struct da_monitor_storage, rcu); + +da_monitor_free_hook (&ms->rv.da_mon); +kfree (ms); +} +/* * da_destroy_storage - destroy the per-object storage *
@@ -564,7 +584,7 @@ static inline void da_destroy_storage(da_id_type id)return ;da_monitor_reset_hook (&mon_storage->rv.da_mon);hash_del_rcu (&mon_storage->node); -kfree_rcu (mon_storage, rcu); +call_rcu (&mon_storage->rcu, da_kfree_cb); }static void __da_monitor_reset_all (void (*reset)(struct da_monitor *))
@@ -607,6 +627,7 @@ static inline void da_monitor_destroy(void)* pending, we can safely assume no concurrent user. */ hash_for_each_safe (da_monitor_ht, bkt, tmp, mon_storage, node) { +da_monitor_free_hook (&mon_storage->rv.da_mon);hash_del_rcu (&mon_storage->node);kfree (mon_storage); }
include/rv/ha_monitor.h +26 -0
@@ -29,6 +29,8 @@ 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 inline void ha_cancel_timer_sync (struct ha_monitor *ha_mon); +static inline bool ha_timer_ready (struct ha_monitor *ha_mon); +static inline void ha_monitor_free_env (struct da_monitor *da_mon); static bool ha_monitor_handle_constraint(struct da_monitor *da_mon, enum states curr_state, enum events event,
@@ -38,6 +40,7 @@ static bool ha_monitor_handle_constraint(struct da_monitor *da_mon,#define da_monitor_init_hook ha_monitor_init_env#define da_monitor_reset_hook ha_monitor_reset_env#define da_monitor_sync_hook () synchronize_rcu() +#define da_monitor_free_hook ha_monitor_free_env#if !defined(HA_SKIP_AUTO_CLEANUP ) &&RV_MON_TYPE ==RV_MON_PER_TASK /*
@@ -467,8 +470,15 @@ static inline bool ha_cancel_timer(struct ha_monitor *ha_mon) {return timer_delete (&ha_mon->timer); } +/* timer_setup() sets it; the cancel paths here never clear it. */ +static inline bool ha_timer_ready (struct ha_monitor *ha_mon) +{ +return READ_ONCE (ha_mon->timer.function); +}static inline void ha_cancel_timer_sync (struct ha_monitor *ha_mon) { +if (!ha_timer_ready (ha_mon)) +return ;timer_delete_sync (&ha_mon->timer); }#elif HA_TIMER_TYPE ==HA_TIMER_HRTIMER
@@ -524,8 +534,18 @@ static inline bool ha_cancel_timer(struct ha_monitor *ha_mon) {return hrtimer_try_to_cancel (&ha_mon->hrtimer) ==1 ; } +/* +* hrtimer_cancel() reads timer->base before anything else; a zeroed timer +* has it NULL and would fault. +*/ +static inline bool ha_timer_ready (struct ha_monitor *ha_mon) +{ +return READ_ONCE (ha_mon->hrtimer.base); +}static inline void ha_cancel_timer_sync (struct ha_monitor *ha_mon) { +if (!ha_timer_ready (ha_mon)) +return ;hrtimer_cancel (&ha_mon->hrtimer); }#else /* HA_TIMER_NONE */
@@ -538,9 +558,15 @@ static inline bool ha_cancel_timer(struct ha_monitor *ha_mon) {return false; } +static inline bool ha_timer_ready (struct ha_monitor *ha_mon) {return false; }static inline void ha_cancel_timer_sync (struct ha_monitor *ha_mon) { }#endif +static inline void ha_monitor_free_env (struct da_monitor *da_mon) +{ +ha_cancel_timer_sync (to_ha_monitor (da_mon)); +} +#if IS_ENABLED (CONFIG_RV_MONITORS_KUNIT_TEST )#ifdef RV_MON_OPS_INIT #undef RV_MON_OPS_INIT
Signature · 1 line
2.25.1
PATCH v7 5/14 Add a KUnit test for the storage free path
In reply to [email protected]From: Wen Yang <[email protected]>
The test resolves a storage, desstroys it with da_destroy_storage(), and then arms the HA timer through the stale pointer. The free hook must disarm that timer too, so it never fires and no reaction is reported.
--- kernel/trace/rv/rv_monitors_test.c | 38 ++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
kernel/trace/rv/rv_monitors_test.c +38 -0
@@ -267,6 +267,43 @@ static void rv_test_rvtest_underflow(struct kunit *test)ha_cancel_timer_sync (ha_mon); } +/* +* rv_test_rvtest_arm_after_destroy - a storage outlived by its own timer +* +* da_destroy_storage() cancels the timer, then unlinks: +* +* da_monitor_reset_hook() cancels +* hash_del_rcu() +* call_rcu() +* +* A reader that resolved the storage before the unlink can arm past that +* cancel. Handlers resolve and use under one guard(rcu)(), so the two steps +* are issued separately here. +*/ +static void rv_test_rvtest_arm_after_destroy (struct kunit *test) +{ +struct rv_kunit_ctx *ctx = test->priv; +const int id =4242 ; +struct da_monitor *stale; + +prepare_test (test, &rvtest_mon); + +da_handle_start_run_event (id,NULL , start_rvtest); + +scoped_guard (rcu)/* resolve before the unlink */ + stale =da_get_monitor (id,NULL ); +KUNIT_ASSERT_NOT_NULL (test, stale); + +da_destroy_storage (id);/* cancel, unlink, call_rcu */ +ha_start_timer_ns (to_ha_monitor (stale), clk_rvtest, +RVTEST_BUDGET_NS ,ha_get_ns ()); + +rcu_barrier (); + +msleep (600 ); +RV_KUNIT_EXPECT_NO_REACTION (test, ctx); +} +#include "monitors/sco/sco_kunit.c" #include "monitors/sssw/sssw_kunit.c" #include "monitors/sts/sts_kunit.c"
@@ -285,6 +322,7 @@ static struct kunit_case rv_mon_test_cases[] = {KUNIT_CASE (rv_test_pagefault),KUNIT_CASE (rv_test_sleep),KUNIT_CASE (rv_test_rvtest_underflow), + KUNIT_CASE(rv_test_rvtest_arm_after_destroy), {} };
Signature · 1 line
2.25.1
PATCH v7 6/14 Make da_monitor_reset_hook and EVENT_NONE_LBL overridable
In reply to [email protected]From: Wen Yang <[email protected]>
Wrap both definitions with #ifndef guards so HA-based monitors can substitute their own implementations before including this header. tlob uses this to define a reset hook that cancels per-task hrtimers on monitor teardown.
Overrides must still call ha_monitor_reset_env() or cancel outstanding timers to avoid timer UAF.
No behaviour change for monitors that do not override either macro.
--- include/rv/ha_monitor.h | 8 ++++++++ 1 file changed, 8 insertions(+)
include/rv/ha_monitor.h +8 -0
@@ -38,8 +38,14 @@ static bool ha_monitor_handle_constraint(struct da_monitor *da_mon, da_id_type id);#define da_monitor_event_hook ha_monitor_handle_constraint#define da_monitor_init_hook ha_monitor_init_env + +/* Overrides must still call ha_monitor_reset_env() or cancel the timer. */ +#ifndef da_monitor_reset_hook#define da_monitor_reset_hook ha_monitor_reset_env +#endif +#ifndef da_monitor_sync_hook#define da_monitor_sync_hook () synchronize_rcu() +#endif #define da_monitor_free_hook ha_monitor_free_env#if !defined(HA_SKIP_AUTO_CLEANUP ) &&RV_MON_TYPE ==RV_MON_PER_TASK
@@ -78,7 +84,9 @@ _Static_assert(offsetof(struct ha_monitor, da_mon) == 0,#define ENV_INVALID_VALUE U64_MAX/* Error with no event occurs only on timeouts */ #define EVENT_NONE EVENT_MAX +#ifndef EVENT_NONE_LBL #define EVENT_NONE_LBL "none" +#endif #define ENV_BUFFER_SIZE 64#ifdef CONFIG_RV_REACTORS
Signature · 1 line
2.25.1
PATCH v7 7/14 Introduce DA_MON_ALLOCATION_STRATEGY
In reply to [email protected]From: Wen Yang <[email protected]>
Per-object DA storage allocation is currently limited to kmalloc on demand. Add a compile-time selector so monitors can choose among three strategies:
DA_ALLOC_AUTO (default) - kmalloc per object on the monitor path
DA_ALLOC_POOL - pre-allocated fixed-size mempool;
selected by defining DA_MON_POOL_SIZE
DA_ALLOC_MANUAL - caller pre-inserts storage; framework
only links the target field
nomiss switches from the implicit DA_SKIP_AUTO_ALLOC override to an explicit DA_MON_ALLOCATION_STRATEGY DA_ALLOC_MANUAL; the behaviours are equivalent (pre-allocated storage, no allocation on the sched_switch start path).
--- include/rv/da_monitor.h | 121 +++++++++++++++++++++-- kernel/trace/rv/monitors/nomiss/nomiss.c | 2 +- 2 files changed, 112 insertions(+), 11 deletions(-)
include/rv/da_monitor.h +111 -10
@@ -14,7 +14,45 @@#ifndef _RV_DA_MONITOR_H#define _RV_DA_MONITOR_H +/* +* Allocation strategy for RV_MON_PER_OBJ monitors, selected before including +* this header. Defining DA_MON_POOL_SIZE N implies DA_ALLOC_POOL. +* +* DA_ALLOC_AUTO - kmalloc on demand; unbounded (default). +* DA_ALLOC_POOL - DA_MON_POOL_SIZE pre-allocated slots. The start handler +* must run in task context; other handlers only call +* da_handle_event() and never allocate. +* DA_ALLOC_MANUAL - caller pre-inserts storage; framework links the target. +*/ +#define DA_ALLOC_AUTO 0 +#define DA_ALLOC_POOL 1 +#define DA_ALLOC_MANUAL 2 + +#ifdef DA_MON_POOL_SIZE +#if defined(DA_MON_ALLOCATION_STRATEGY ) && \ +DA_MON_ALLOCATION_STRATEGY !=DA_ALLOC_POOL +#error "DA_MON_POOL_SIZE implies DA_ALLOC_POOL" +#endif +#if DA_MON_POOL_SIZE ==0 +#error "DA_MON_POOL_SIZE must be non-zero" +#endif +#define DA_MON_ALLOCATION_STRATEGY DA_ALLOC_POOL +#endif /* DA_MON_POOL_SIZE */ + +#ifndef DA_MON_ALLOCATION_STRATEGY +#define DA_MON_ALLOCATION_STRATEGY DA_ALLOC_AUTO +#endif /* DA_MON_ALLOCATION_STRATEGY */ + +/* Zero default keeps pool-mode conditionals compile-time constant. */ +#ifndef DA_MON_POOL_SIZE +#if DA_MON_ALLOCATION_STRATEGY ==DA_ALLOC_POOL +#error "DA_ALLOC_POOL requires DA_MON_POOL_SIZE to be defined and non-zero" +#endif +#define DA_MON_POOL_SIZE 0 +#endif /* DA_MON_POOL_SIZE */ +#include <rv/automata.h> +#include <linux/mempool.h> #include <linux/rv.h> #include <rv/kunit.h> #include <linux/stringify.h>
@@ -563,6 +601,46 @@ static void da_kfree_cb(struct rcu_head *head)kfree (ms); } +/* +* Zero-initialised for non-pool monitors, whose pool paths compile away. +*/ +static mempool_t da_monitor_pool; + +static void da_pool_return_cb (struct rcu_head *head) +{ +struct da_monitor_storage *ms = + container_of(head, struct da_monitor_storage, rcu); + +da_monitor_free_hook (&ms->rv.da_mon); +mempool_free (ms, &da_monitor_pool); +} + +/* +* da_create_pool_storage - pop a free pool slot and insert it into the hash. +* Returns NULL if the pool is exhausted. Caller holds an RCU read-side CS +* and the monitor's serialisation lock. +*/ +static inline struct da_monitor * +da_create_pool_storage (da_id_type id,monitor_target target, +struct da_monitor *da_mon) +{ +struct da_monitor_storage *mon_storage; + +if (da_mon) +return da_mon; + + mon_storage =mempool_alloc_preallocated (&da_monitor_pool); +if (!mon_storage) +return NULL ; +memset (mon_storage,0 ,sizeof (*mon_storage)); + + mon_storage->id = id; + mon_storage->target = target; + +hash_add_rcu (da_monitor_ht, &mon_storage->node, id); +return &mon_storage->rv.da_mon; +} +/* * da_destroy_storage - destroy the per-object storage *
@@ -584,7 +662,10 @@ static inline void da_destroy_storage(da_id_type id)return ;da_monitor_reset_hook (&mon_storage->rv.da_mon);hash_del_rcu (&mon_storage->node); -call_rcu (&mon_storage->rcu, da_kfree_cb); +if (DA_MON_ALLOCATION_STRATEGY ==DA_ALLOC_POOL ) +call_rcu (&mon_storage->rcu, da_pool_return_cb); +else +call_rcu (&mon_storage->rcu, da_kfree_cb); }static void __da_monitor_reset_all (void (*reset)(struct da_monitor *))
@@ -610,6 +691,13 @@ static inline void da_monitor_reset_state_all(void)static inline int da_monitor_init (void ) {hash_init (da_monitor_ht); +if (DA_MON_ALLOCATION_STRATEGY ==DA_ALLOC_POOL ) { +size_t storage_size =sizeof (struct da_monitor_storage ); + +return mempool_init_kmalloc_pool (&da_monitor_pool, +DA_MON_POOL_SIZE , + storage_size); + }return 0 ; }
@@ -629,20 +717,33 @@ static inline void da_monitor_destroy(void)hash_for_each_safe (da_monitor_ht, bkt, tmp, mon_storage, node) {da_monitor_free_hook (&mon_storage->rv.da_mon);hash_del_rcu (&mon_storage->node); -kfree (mon_storage); +if (DA_MON_ALLOCATION_STRATEGY ==DA_ALLOC_POOL ) +mempool_free (mon_storage, &da_monitor_pool); +else +kfree (mon_storage); + } + +if (DA_MON_ALLOCATION_STRATEGY ==DA_ALLOC_POOL ) { +rcu_barrier (); +mempool_exit (&da_monitor_pool); } }/* -* Allow the per-object monitors to run allocation manually, necessary if the -* start condition is in a context problematic for allocation (e.g. scheduling). -* In such case, if the storage was pre-allocated without a target, set it now. +* da_prepare_storage - allocate or link per-object monitor storage. +* Only the first start allocates; later starts reuse the existing da_mon. +* Must run in task context (see the allocation strategies above). */ -#ifdef DA_SKIP_AUTO_ALLOC -#define da_prepare_storage da_fill_empty_storage -#else -#define da_prepare_storage da_create_storage -#endif /* DA_SKIP_AUTO_ALLOC */ +static inline struct da_monitor * +da_prepare_storage (da_id_type id,monitor_target target, +struct da_monitor *da_mon) +{ +if (DA_MON_ALLOCATION_STRATEGY ==DA_ALLOC_POOL ) +return da_create_pool_storage (id, target, da_mon); +if (DA_MON_ALLOCATION_STRATEGY ==DA_ALLOC_MANUAL ) +return da_fill_empty_storage (id, target, da_mon); +return da_create_storage (id, target, da_mon); +}#endif /* RV_MON_TYPE */
kernel/trace/rv/monitors/nomiss/nomiss.c +1 -1
@@ -18,7 +18,7 @@#define RV_MON_TYPE RV_MON_PER_OBJ#define HA_TIMER_TYPE HA_TIMER_WHEEL/* The start condition is on sched_switch, it's dangerous to allocate there */ -#define DA_SKIP_AUTO_ALLOC +#define DA_MON_ALLOCATION_STRATEGY DA_ALLOC_MANUALtypedef struct sched_dl_entity *monitor_target ;#include "nomiss.h" #include <rv/ha_monitor.h>
Signature · 1 line
2.25.1
PATCH v7 8/14 Add per-object teardown hook da_extra_cleanup
In reply to [email protected]From: Wen Yang <[email protected]>
da_monitor_destroy() frees each per-object storage, but a monitor whose target is allocated separately has no point to free it: the target is still reachable from the storage until hash_del_rcu().
Add an optional da_extra_cleanup() hook, called for each storage after da_monitor_free_hook() has disarmed its timer and before the storage is unlinked. It defaults to a no-op. tlob, added later in this series, uses it to retire and free its per-window record.
--- include/rv/da_monitor.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)
include/rv/da_monitor.h +12 -0
@@ -116,6 +116,16 @@ static struct rv_monitor rv_this;#define da_monitor_free_hook (da_mon)#endif +/* +* Per-object teardown hook, called for each entry after +* da_monitor_reset_all() + da_monitor_sync_hook() and before hash_del_rcu(). +* Runs after da_monitor_free_hook(), so it may free the storage's target. +* Default: no-op. +*/ +#ifndef da_extra_cleanup +#define da_extra_cleanup (da_mon) +#endif +/* * Type for the target id, default to int but can be overridden. * A long type can work as hash table key (PER_OBJ) but will be downgraded to
@@ -715,7 +725,9 @@ static inline void da_monitor_destroy(void)* pending, we can safely assume no concurrent user. */ hash_for_each_safe (da_monitor_ht, bkt, tmp, mon_storage, node) { +/* Before da_extra_cleanup(): it frees what the timer reads. */ da_monitor_free_hook (&mon_storage->rv.da_mon); +da_extra_cleanup (&mon_storage->rv.da_mon);hash_del_rcu (&mon_storage->node);if (DA_MON_ALLOCATION_STRATEGY ==DA_ALLOC_POOL )mempool_free (mon_storage, &da_monitor_pool);
Signature · 1 line
2.25.1
PATCH v7 9/14 Add generic uprobe infrastructure for RV monitors
In reply to [email protected]From: Wen Yang <[email protected]>
Monitors that instrument user-space function boundaries need to register uprobes and deregister them safely. Provide a thin wrapper so monitors share a single implementation of this boilerplate.
struct rv_uprobe embeds struct uprobe_consumer directly, avoiding a separate heap allocation per probe. The struct holds an inode reference (acquired via igrab()) for the probed binary for the full uprobe lifetime, as uprobe_register() does not take an inode reference. On unregister, the inode reference is released with iput() only after rv_uprobe_unregister_sync().
rv_uprobe_unregister_sync() calls uprobe_unregister_sync(), draining all in-flight handlers before the containing struct may be freed.
The API provides register, nosync unregister, a global handler barrier (rv_uprobe_unregister_sync), and an active-state predicate.
--- include/rv/rv_uprobe.h | 35 +++++++++++++++++++++++ kernel/trace/rv/Kconfig | 4 +++ kernel/trace/rv/Makefile | 1 + kernel/trace/rv/rv_uprobe.c | 56 +++++++++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+) create mode 100644 include/rv/rv_uprobe.h create mode 100644 kernel/trace/rv/rv_uprobe.c
include/rv/rv_uprobe.h new file +35 -0
@@ -0,0 +1,35 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright (C) 2026 Wen Yang <[email protected]> */ +/* +* Generic uprobe infrastructure for RV monitors. +*/ + +#ifndef _RV_UPROBE_H +#define _RV_UPROBE_H + +#include <linux/types.h> +#include <linux/uprobes.h> + +struct inode ; +struct pt_regs ; + +/* +* Free the containing struct only after rv_uprobe_unregister_nosync() + +* rv_uprobe_unregister_sync() + iput(p->inode). +* +* The inode reference does not pin the dentry, so callers that re-resolve the +* probed path must hold a struct path: 9p cache=none remints an inode per +* lookup. +*/ +struct rv_uprobe { +struct uprobe_consumer uc; +struct uprobe *uprobe; +struct inode *inode; +}; + +bool rv_uprobe_is_registered (const struct rv_uprobe *p); +int rv_uprobe_register (struct inode *inode,loff_t offset,struct rv_uprobe *p); +void rv_uprobe_unregister_nosync (struct rv_uprobe *p); +void rv_uprobe_unregister_sync (void ); + +#endif /* _RV_UPROBE_H */
kernel/trace/rv/Kconfig +4 -0
@@ -86,6 +86,10 @@ source "kernel/trace/rv/monitors/nomiss/Kconfig" # Add new monitors here +config RV_UPROBE + bool + depends on RV && UPROBES + config RV_REACTORS bool "Runtime verification reactors" default y
kernel/trace/rv/Makefile +1 -0
@@ -22,6 +22,7 @@ obj-$(CONFIG_RV_MON_DEADLINE) += monitors/deadline/deadline.o obj-$(CONFIG_RV_MON_NOMISS) += monitors/nomiss/nomiss.o obj-$(CONFIG_RV_MON_WAKEUP) += monitors/wakeup/wakeup.o # Add new monitors here +obj-$(CONFIG_RV_UPROBE) += rv_uprobe.o obj-$(CONFIG_RV_REACTORS) += rv_reactors.o obj-$(CONFIG_RV_REACT_PRINTK) += reactor_printk.o obj-$(CONFIG_RV_REACT_PANIC) += reactor_panic.o
kernel/trace/rv/rv_uprobe.c new file +56 -0
56 lines not on this page · raw message
Signature · 1 line
2.25.1
PATCH v7 10/14 Add tlob model DOT file
In reply to [email protected]From: Wen Yang <[email protected]>
Add the Graphviz DOT specification of the tlob hybrid automaton to tools/verification/models/.
The model has four states (stopped, running, waiting, sleeping) and six events (start, stop, sleep, preempt, wakeup, switch_in). A single clock invariant, clk_elapsed < BUDGET_NS(), holds in running, waiting and sleeping; stopped has no invariant, hence no timer.
wakeup is a self-loop on running and waiting: a task that is already queued can still be the target of a traced wakeup, either by waking itself or through ttwu_runnable() while preempted, and neither case ends the budget window. stopped takes every scheduler event as a self-loop, so a task keeps going through the scheduler between windows.
--- tools/verification/models/tlob.dot | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 tools/verification/models/tlob.dot
tools/verification/models/tlob.dot new file +27 -0
@@ -0,0 +1,27 @@ +digraph state_automaton { + center = true; + size = "7,11"; + {node [shape = plaintext, style=invis, label=""] "__init_stopped"}; + {node [shape = plaintext] "running"}; + {node [shape = plaintext] "waiting"}; + {node [shape = plaintext] "sleeping"}; + {node [shape = ellipse] "stopped"}; + "__init_stopped" -> "stopped"; + "running" [label = "running\nclk_elapsed <= BUDGET_NS()"]; + "waiting" [label = "waiting\nclk_elapsed <= BUDGET_NS()"]; + "sleeping" [label = "sleeping\nclk_elapsed <= BUDGET_NS()"]; + "stopped" [label = "stopped", color = green3]; + "running" -> "sleeping" [ label = "sleep" ]; + "running" -> "waiting" [ label = "preempt" ]; + "running" -> "running" [ label = "wakeup" ]; + "waiting" -> "running" [ label = "switch_in" ]; + "waiting" -> "waiting" [ label = "wakeup" ]; + "sleeping" -> "waiting" [ label = "wakeup" ]; + "running" -> "stopped" [ label = "stop" ]; + "stopped" -> "running" [ label = "start;reset(clk_elapsed)" ]; + "stopped" -> "stopped" [ label = "preempt\nsleep\nswitch_in\nwakeup" ]; + { rank = min ; + "__init_stopped"; + "stopped"; + } +}
Signature · 1 line
2.25.1
PATCH v7 11/14 Add tlob hybrid automaton monitor
In reply to [email protected]From: Wen Yang <[email protected]>
tlob (task latency over budget) is a per-task hybrid automaton RV monitor that tracks wall-clock time across a user-delimited code section and emits an error when the elapsed time exceeds a configurable budget.
The automaton has four states (running, waiting, sleeping, stopped), driven by the sched_switch and sched_wakeup tracepoints plus a user-visible start/stop pair. stop only fires from running and parks the window in stopped, where a later start restarts it in place. A wakeup is a self-loop on running and waiting, since an already queued task can still be woken without leaving its budget window; the elapsed time is always accounted to the state the monitor is leaving. The clk_elapsed < BUDGET_NS() invariant is enforced by a per-task HRTIMER_MODE_REL_HARD timer; on expiry the monitor records a per-state breakdown (running_ns, waiting_ns, sleeping_ns) before emitting error_env_tlob.
--- Documentation/trace/rv/index.rst | 1 + Documentation/trace/rv/monitor_tlob.rst | 154 +++ kernel/trace/rv/Kconfig | 1 + kernel/trace/rv/Makefile | 1 + kernel/trace/rv/monitors/tlob/Kconfig | 12 + kernel/trace/rv/monitors/tlob/tlob.c | 1038 ++++++++++++++++++++ kernel/trace/rv/monitors/tlob/tlob.h | 142 +++ kernel/trace/rv/monitors/tlob/tlob_trace.h | 20 + kernel/trace/rv/rv_trace.h | 1 + 9 files changed, 1370 insertions(+) create mode 100644 Documentation/trace/rv/monitor_tlob.rst create mode 100644 kernel/trace/rv/monitors/tlob/Kconfig create mode 100644 kernel/trace/rv/monitors/tlob/tlob.c create mode 100644 kernel/trace/rv/monitors/tlob/tlob.h create mode 100644 kernel/trace/rv/monitors/tlob/tlob_trace.h
Documentation/trace/rv/index.rst +1 -0
@@ -16,5 +16,6 @@ Runtime Verification monitor_wwnr.rst monitor_sched.rst monitor_rtapp.rst + monitor_tlob.rst monitor_stall.rst monitor_deadline.rst
Documentation/trace/rv/monitor_tlob.rst new file +154 -0
154 lines not on this page · raw message
kernel/trace/rv/Kconfig +1 -0
@@ -84,6 +84,7 @@ source "kernel/trace/rv/monitors/deadline/Kconfig" source "kernel/trace/rv/monitors/nomiss/Kconfig" # Add new deadline monitors here +source "kernel/trace/rv/monitors/tlob/Kconfig" # Add new monitors here config RV_UPROBE
kernel/trace/rv/Makefile +1 -0
@@ -21,6 +21,7 @@ obj-$(CONFIG_RV_MON_STALL) += monitors/stall/stall.o obj-$(CONFIG_RV_MON_DEADLINE) += monitors/deadline/deadline.o obj-$(CONFIG_RV_MON_NOMISS) += monitors/nomiss/nomiss.o obj-$(CONFIG_RV_MON_WAKEUP) += monitors/wakeup/wakeup.o +obj-$(CONFIG_RV_MON_TLOB) += monitors/tlob/tlob.o # Add new monitors here obj-$(CONFIG_RV_UPROBE) += rv_uprobe.o obj-$(CONFIG_RV_REACTORS) += rv_reactors.o
kernel/trace/rv/monitors/tlob/Kconfig new file +12 -0
@@ -0,0 +1,12 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +config RV_MON_TLOB + bool "tlob monitor" + depends on RV && UPROBES && HIGH_RES_TIMERS + select HA_MON_EVENTS_ID + select RV_UPROBE + help + Enable the tlob (task latency over budget) hybrid-automaton RV + monitor. tlob tracks per-task elapsed time across a + user-delimited code section and emits a violation when the + elapsed time exceeds a configurable per-invocation budget.
kernel/trace/rv/monitors/tlob/tlob.c new file +1038 -0
1,038 lines not on this page · raw message
kernel/trace/rv/monitors/tlob/tlob.h new file +142 -0
142 lines not on this page · raw message
kernel/trace/rv/monitors/tlob/tlob_trace.h new file +20 -0
@@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +/* +* Snippet to be included in rv_trace.h +*/ + +#ifdef CONFIG_RV_MON_TLOB +DEFINE_EVENT (event_da_monitor_id, event_tlob, + 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_tlob, + TP_PROTO(int id, char *state, char *event), +TP_ARGS (id, state, event)); + +DEFINE_EVENT (error_env_da_monitor_id, error_env_tlob, + TP_PROTO(int id, char *state, char *event, char *env), +TP_ARGS (id, state, event, env)); +#endif /* CONFIG_RV_MON_TLOB */
kernel/trace/rv/rv_trace.h +1 -0
@@ -189,6 +189,7 @@ DECLARE_EVENT_CLASS(error_env_da_monitor_id,#include <monitors/stall/stall_trace.h> #include <monitors/nomiss/nomiss_trace.h> +#include <monitors/tlob/tlob_trace.h> // Add new monitors based on CONFIG_HA_MON_EVENTS_ID here #endif
Signature · 1 line
2.25.1
PATCH v7 12/14 Add KUnit tests for the tlob monitor
In reply to [email protected]From: Wen Yang <[email protected]>
Add a test case to the shared rv_monitors_test.c suite covering the uprobe-line parser in the tlob monitor.
Cases cover valid inputs, malformed paths and offsets (including negative values), out-of-range thresholds, and valid and invalid remove lines.
--- kernel/trace/rv/monitors/tlob/tlob.c | 11 ++ kernel/trace/rv/monitors/tlob/tlob_kunit.c | 151 +++++++++++++++++++++ kernel/trace/rv/monitors/tlob/tlob_kunit.h | 18 +++ kernel/trace/rv/rv_monitors_test.c | 2 + 4 files changed, 182 insertions(+) create mode 100644 kernel/trace/rv/monitors/tlob/tlob_kunit.c create mode 100644 kernel/trace/rv/monitors/tlob/tlob_kunit.h
kernel/trace/rv/monitors/tlob/tlob.c +11 -0
@@ -863,6 +863,17 @@ static int tlob_parse_remove_line(char *buf, char **name_out)return 0 ; } +#if IS_ENABLED (CONFIG_RV_MONITORS_KUNIT_TEST ) +#include <kunit/visibility.h> +#include "tlob_kunit.h" + +const struct rv_tlob_kunit_ops rv_tlob_kunit_ops = { + .parse_uprobe_line = tlob_parse_uprobe_line, + .parse_remove_line = tlob_parse_remove_line, +}; +EXPORT_SYMBOL_IF_KUNIT (rv_tlob_kunit_ops); +#endif +static int tlob_create_or_delete_uprobe (char *buf) {loff_t offset_start, offset_stop;
kernel/trace/rv/monitors/tlob/tlob_kunit.c new file +151 -0
151 lines not on this page · raw message
kernel/trace/rv/monitors/tlob/tlob_kunit.h new file +18 -0
@@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef __TLOB_KUNIT_H +#define __TLOB_KUNIT_H + +#if IS_ENABLED (CONFIG_RV_MONITORS_KUNIT_TEST ) + +#include <linux/types.h> + +extern const struct rv_tlob_kunit_ops { +int (*parse_uprobe_line)(char *buf,char **name_out,char **path_out, +loff_t *start_out,loff_t *stop_out, +u64 *thr_out); +int (*parse_remove_line)(char *buf,char **name_out); +} rv_tlob_kunit_ops; + +#endif + +#endif /* __TLOB_KUNIT_H */
kernel/trace/rv/rv_monitors_test.c +2 -0
@@ -309,6 +309,7 @@ static void rv_test_rvtest_arm_after_destroy(struct kunit *test)#include "monitors/sts/sts_kunit.c" #include "monitors/opid/opid_kunit.c" #include "monitors/nomiss/nomiss_kunit.c" +#include "monitors/tlob/tlob_kunit.c" #include "monitors/pagefault/pagefault_kunit.c" #include "monitors/sleep/sleep_kunit.c"
@@ -323,6 +324,7 @@ static struct kunit_case rv_mon_test_cases[] = {KUNIT_CASE (rv_test_sleep),KUNIT_CASE (rv_test_rvtest_underflow),KUNIT_CASE (rv_test_rvtest_arm_after_destroy), + KUNIT_CASE(rv_test_tlob), {} };
Signature · 1 line
2.25.1
PATCH v7 13/14 selftests/verification: Add tlob selftests
In reply to [email protected]From: Wen Yang <[email protected]>
Add ten ftrace-style test scripts for the tlob RV monitor under tools/testing/selftests/verification/test.d/tlob/. The tests cover uprobe binding management, budget violation detection, multi-binding, multi-task sharing of one binding, overlap registration rejection, unbind and disable on live tasks, and start/stop restart.
Helper binarie tlob_target is built via TEST_GEN_FILES, following the kselftests convention. run_tlob_tests.sh builds the helpers and delegates argument handling to ftracetest.
--- .../testing/selftests/verification/.gitignore | 1 + tools/testing/selftests/verification/Makefile | 4 + tools/testing/selftests/verification/config | 3 + .../selftests/verification/test.d/functions | 9 ++ .../test.d/tlob/run_tlob_tests.sh | 32 +++++ .../verification/test.d/tlob/uprobe_bind.tc | 47 ++++++ .../test.d/tlob/uprobe_breakdown.tc | 136 ++++++++++++++++++ .../test.d/tlob/uprobe_disable_live.tc | 79 ++++++++++ .../verification/test.d/tlob/uprobe_multi.tc | 58 ++++++++ .../test.d/tlob/uprobe_multi_instance.tc | 65 +++++++++ .../test.d/tlob/uprobe_no_event.tc | 17 +++ .../test.d/tlob/uprobe_overlap_register.tc | 66 +++++++++ .../test.d/tlob/uprobe_restart.tc | 66 +++++++++ .../test.d/tlob/uprobe_restart_violation.tc | 48 +++++++ .../test.d/tlob/uprobe_unbind_live.tc | 65 +++++++++ .../test.d/tlob/uprobe_violation.tc | 53 +++++++ .../selftests/verification/tlob_target.c | 127 ++++++++++++++++ 17 files changed, 876 insertions(+) create mode 100755 tools/testing/selftests/verification/test.d/tlob/run_tlob_tests.sh create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_bind.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_breakdown.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_disable_live.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_multi.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_multi_instance.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_no_event.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_overlap_register.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_restart.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_restart_violation.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_unbind_live.tc create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_violation.tc create mode 100644 tools/testing/selftests/verification/tlob_target.c
tools/testing/selftests/verification/.gitignore +1 -0
@@ -1,2 +1,3 @@ # SPDX-License-Identifier: GPL-2.0-only logs +tlob_target
tools/testing/selftests/verification/Makefile +4 -0
@@ -5,4 +5,8 @@ TEST_PROGS := verificationtest-ktap TEST_FILES := test.d settings EXTRA_CLEAN := $(OUTPUT)/logs/* +TEST_GEN_FILES := tlob_target + include ../lib.mk + +export VERIFICATIONTEST_BINDIR := $(OUTPUT)
tools/testing/selftests/verification/config +3 -0
@@ -1 +1,4 @@ CONFIG_RV=y +CONFIG_RV_MON_TLOB=y +CONFIG_UPROBES=y +CONFIG_HIGH_RES_TIMERS=y
tools/testing/selftests/verification/test.d/functions +9 -0
@@ -25,6 +25,15 @@ check_requires() { # Check required files, monitors and reactors done } +sym_offset() { # <binary> <symbol>: print the ELF file offset of <symbol> + readelf -W -S -s "$1" | awk -v symbol="$2" ' + { gsub(/\[ /, "[") } + $1 ~ /^\[[0-9]+\]$/ { sections[$1]="0x"$4; offsets[$1]="0x"$5 } + $1 ~ /^[0-9]+:$/ && $NF == symbol { addr="0x"$2; sec="["$7"]" } + END { printf "printf \"0x%%x\\n\" $((%s - %s + %s))\n", addr, sections[sec], offsets[sec] } + ' | sh +} + initialize_system() { # Reset RV to initial-state echo > enabled_monitors for m in monitors/*; do
tools/testing/selftests/verification/test.d/tlob/run_tlob_tests.sh new file +32 -0
@@ -0,0 +1,32 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# +# Standalone runner for tlob selftests + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +BINDIR="$(cd "$SCRIPT_DIR/../.." && pwd)" +FTRACETEST="$(cd "$SCRIPT_DIR/../../../ftrace" && pwd)/ftracetest" + +# The helper is built on the host by the selftests Makefile +# (TEST_GEN_FILES in $BINDIR/Makefile); the VM (virtme-ng) mounts the +# source read-only and may lack a compiler, so this runner never builds. +# Build it on the host first: +# make -C "$BINDIR" tlob_target +if [ ! -x "$BINDIR/tlob_target" ]; then + echo "ERROR: tlob_target not built in $BINDIR" >&2 + echo " build it on the host: make -C $BINDIR tlob_target" >&2 + exit 1 +fi + +# Flag (not fail) the helper older than its source: git checkouts can bump +# source mtimes, so a hard check would misfire; a host rebuild picks up +# the new code. +if [ -f "$BINDIR/tlob_target.c" ] && + [ "$BINDIR/tlob_target" -ot "$BINDIR/tlob_target.c" ]; then + echo "WARNING: $BINDIR/tlob_target is older than $BINDIR/tlob_target.c;" >&2 + echo " rebuild on the host to test the current source" >&2 +fi + +export VERIFICATIONTEST_BINDIR="$BINDIR" + +exec "$FTRACETEST" -K --rv "$SCRIPT_DIR" "$@"
tools/testing/selftests/verification/test.d/tlob/uprobe_bind.tc new file +47 -0
47 lines not on this page · raw message
tools/testing/selftests/verification/test.d/tlob/uprobe_breakdown.tc new file +136 -0
136 lines not on this page · raw message
tools/testing/selftests/verification/test.d/tlob/uprobe_disable_live.tc new file +79 -0
79 lines not on this page · raw message
tools/testing/selftests/verification/test.d/tlob/uprobe_multi.tc new file +58 -0
58 lines not on this page · raw message
tools/testing/selftests/verification/test.d/tlob/uprobe_multi_instance.tc new file +65 -0
65 lines not on this page · raw message
tools/testing/selftests/verification/test.d/tlob/uprobe_no_event.tc new file +17 -0
@@ -0,0 +1,17 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0-or-later +# description: Test tlob monitor no spurious events without active uprobe binding +# requires: tlob:monitor + +echo 1 > ../events/rv/error_env_tlob/enable +echo 1 > ../tracing_on +echo 1 > monitors/tlob/enable +echo > ../trace + +sleep 0.5 + +! grep -q "error_env_tlob" ../trace || false + +echo 0 > monitors/tlob/enable +echo 0 > ../events/rv/error_env_tlob/enable +echo > ../trace
tools/testing/selftests/verification/test.d/tlob/uprobe_overlap_register.tc new file +66 -0
66 lines not on this page · raw message
tools/testing/selftests/verification/test.d/tlob/uprobe_restart.tc new file +66 -0
66 lines not on this page · raw message
tools/testing/selftests/verification/test.d/tlob/uprobe_restart_violation.tc new file +48 -0
48 lines not on this page · raw message
tools/testing/selftests/verification/test.d/tlob/uprobe_unbind_live.tc new file +65 -0
65 lines not on this page · raw message
tools/testing/selftests/verification/test.d/tlob/uprobe_violation.tc new file +53 -0
53 lines not on this page · raw message
tools/testing/selftests/verification/tlob_target.c new file +127 -0
127 lines not on this page · raw message
Signature · 1 line
2.25.1
PATCH v7 14/14 selftests/ftrace: Walk up parent dirs to find test.d/functions
In reply to [email protected]From: Wen Yang <[email protected]>
When a subdirectory is passed (e.g. an RV monitor's test directory), ftracetest fell back to its own functions file, losing the RV-specific check_requires handling for ':monitor' and ':reactor' requirements.
Walk up from OPT_TEST_DIR until a directory containing test.d/functions is found, stopping after five levels so a stray argument cannot reach an unrelated functions file further up the tree. When no such directory is found the previous default is kept.
Note that TOP_DIR also feeds LOG_TOP_DIR, so logs now follow the directory the tests were found in: an RV run writes to verification/logs rather than ftrace/logs.
--- tools/testing/selftests/ftrace/ftracetest | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
tools/testing/selftests/ftrace/ftracetest +16 -3
@@ -159,9 +159,22 @@ parse_opts() { # opts if [ -n "$OPT_TEST_CASES" ]; then TEST_CASES=$OPT_TEST_CASES fi - if [ -n "$OPT_TEST_DIR" -a -f "$OPT_TEST_DIR"/test.d/functions ]; then - TOP_DIR=$OPT_TEST_DIR - TEST_DIR=$TOP_DIR/test.d + if [ -n "$OPT_TEST_DIR" ]; then + # A subdirectory of test.d can be given; walk up to the directory + # holding test.d/functions. The bound keeps a stray directory from + # reaching an unrelated functions file further up the tree. + local dir=$OPT_TEST_DIR + local depth=0 + while [ "$depth" -lt 5 ]; do + if [ -f "$dir/test.d/functions" ]; then + TOP_DIR=$dir + TEST_DIR=$TOP_DIR/test.d + break + fi + case "$dir" in /|//|.) break;; esac + dir=$(dirname "$dir") + depth=$((depth + 1)) + done fi }
Signature · 1 line
2.25.1
[email protected]
AI reviewRe: [PATCH v7 09/14] rv: Add generic uprobe infrastructure for RV monitors
In reply to [email protected]Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] uprobes: missing vfsmount reference leads to VFS BUG on unmount - [Medium] uprobes: double…
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] uprobes: missing vfsmount reference leads to VFS BUG on unmount
- [Medium] uprobes: double registration leaks inode reference and corrupts consumer list
- [Low] uprobes: missing <linux/export.h> inclusion
--
commit 27eb3c739b9acf445ce512a1c9be0a7dca8bace0
Author: Wen Yang <[email protected]>
rv: Add generic uprobe infrastructure for RV monitors
Monitors that instrument user-space function boundaries need to register uprobes and deregister them safely. Provide a thin wrapper so monitors share a single implementation of this boilerplate.
Quoted 5 lines
include/rv/rv_uprobe.h new file +0
-0
[ ... ]
include/rv/rv_uprobe.h line 28
Quoted 13 lines
include/rv/rv_uprobe.h new file +13
-0+/* +* Free the containing struct only after rv_uprobe_unregister_nosync() + +* rv_uprobe_unregister_sync() + iput(p->inode). +* +* The inode reference does not pin the dentry, so callers that re-resolve the +* probed path must hold a struct path: 9p cache=none remints an inode per +* lookup. +*/ +struct rv_uprobe { +struct uprobe_consumer uc; +struct uprobe *uprobe; +struct inode *inode; +};
[Severity: High]
Does this struct need to hold a reference to the vfsmount as well?
The core uprobes API documents that the caller of uprobe_register() is required to keep the inode and the containing mount referenced.
If the infrastructure only pins the inode via igrab(), and the caller does not independently hold a mount reference, user-space could unmount the filesystem while the monitor is active.
During unmount, the elevated inode i_count from igrab prevents eviction, which causes the VFS superblock shutdown sequence in generic_shutdown_super() to trigger a bug for busy inodes after unmount.
[ ... ]
Quoted 5 lines
kernel/trace/rv/rv_uprobe.c new file +0
-0
[ ... ]
kernel/trace/rv/rv_uprobe.c line 17
Quoted 9 lines
kernel/trace/rv/rv_uprobe.c new file +9
-0+/* +* uprobe_register() takes no inode reference of its own, so grab one here and +* drop it only after rv_uprobe_unregister_sync(). +*/ +int rv_uprobe_register (struct inode *inode,loff_t offset,struct rv_uprobe *p) +{ +int ret; + + p->inode =igrab (inode);
[Severity: Medium]
What happens if rv_uprobe_register() is called on an already registered struct rv_uprobe?
Since p->inode is unconditionally overwritten with igrab(inode), the previous inode reference seems to be leaked without calling iput().
Additionally, passing an actively linked uprobe_consumer to uprobe_register() again could corrupt the uprobe subsystem's internal consumer lists.
Would it be safer to check if p->inode is already set before proceeding?
Quoted 5 lines
kernel/trace/rv/rv_uprobe.c new file +5
-0+if (!p->inode) +return -ESTALE ; + + p->uprobe =uprobe_register (inode, offset,0 , &p->uc); +if (IS_ERR (p->uprobe)) {
kernel/trace/rv/rv_uprobe.c line 32
Quoted 10 lines
kernel/trace/rv/rv_uprobe.c new file +10
-0+ ret =PTR_ERR (p->uprobe); + p->uprobe =NULL ; +iput (p->inode); + p->inode =NULL ; +return ret; + } + +return 0 ; +} +EXPORT_SYMBOL_GPL (rv_uprobe_register);
[Severity: Low]
Could this EXPORT_SYMBOL_GPL usage cause build failures on some architectures?
The source file uses EXPORT_SYMBOL_GPL but does not explicitly include <linux/export.h>. Relying on implicit transitive inclusion of this header can lead to build breakages on alternative configurations.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9
[email protected]
AI reviewRe: [PATCH v7 05/14] rv: Add a KUnit test for the storage free path
In reply to [email protected]Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] rv: Use-After-Free in rv_test_rvtest_arm_after_destroy due to dropped RCU lock --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] rv: Use-After-Free in rv_test_rvtest_arm_after_destroy due to dropped RCU lock --
commit 62c45f491a4a036b9788ee3cd149fc4c81f32003
Author: Wen Yang <[email protected]>
rv: Add a KUnit test for the storage free path
The test resolves a storage, desstroys it with da_destroy_storage(), and then arms the HA timer through the stale pointer. The free hook must disarm that timer too, so it never fires and no reaction is reported.
Quoted 20 lines
kernel/trace/rv/rv_monitors_test.c +12
-0@@ -146,6 +227,83 @@ static void rv_test_dummy(struct kunit *test) active_ctx =NULL ; } +/* +* rv_test_rvtest_underflow - the HA clock math must not underflow as u64 +* +* An invariant checked while the uptime is below the budget holds, and a +* timer armed after the budget elapsed expires now. +*/ +static void rv_test_rvtest_underflow (struct kunit *test) +{ +struct ha_monitor *ha_mon; +struct da_monitor *da_mon; +const int id =4242 ; +
kernel/trace/rv/rv_monitors_test.c static void rv_test_dummy(struct kunit *test) · line 251
Quoted 10 lines
kernel/trace/rv/rv_monitors_test.c +10
-0+prepare_test (test, &rvtest_mon); +da_handle_start_run_event (id,NULL , start_rvtest); + +scoped_guard (rcu) + da_mon =da_get_monitor (id,NULL ); +KUNIT_ASSERT_NOT_NULL (test, da_mon); + ha_mon =to_ha_monitor (da_mon); + +/* Uptime below the budget: time_ns - expire_ns would underflow. */ +ha_reset_clk_ns (ha_mon, clk_rvtest,0 );
[Severity: Medium]
Are we dereferencing an RCU-protected pointer outside of its RCU read-side critical section here?
The scoped_guard(rcu) is used without braces, meaning the RCU read-side critical section ends immediately after da_get_monitor(). The da_mon variable is then used by to_ha_monitor() and passed to ha_reset_clk_ns().
Even if there is no concurrent deletion in this specific test environment, isn't this an RCU read-side violation that dereferences an RCU-protected pointer after the lock is released?
Quoted 38 lines
kernel/trace/rv/rv_monitors_test.c +38
-0+KUNIT_EXPECT_TRUE (test,ha_check_invariant_ns (ha_mon, clk_rvtest,0 , +RVTEST_BUDGET_NS )); + +/* +* Budget already elapsed: expire - passed would underflow and queue +* the timer at KTIME_SEC_MAX. +*/ +da_monitor_reset (&ha_mon->da_mon); +ha_reset_clk_ns (ha_mon, clk_rvtest,0 ); +ha_start_timer_ns (ha_mon, clk_rvtest,RVTEST_BUDGET_NS , +RVTEST_BUDGET_NS +NSEC_PER_SEC ); +KUNIT_EXPECT_TRUE (test, +ktime_before (hrtimer_get_expires (&ha_mon->hrtimer), +ktime_get () +NSEC_PER_SEC )); + +ha_cancel_timer_sync (ha_mon); +} + +/* +* rv_test_rvtest_arm_after_destroy - a storage outlived by its own timer +* +* da_destroy_storage() cancels the timer, then unlinks: +* +* da_monitor_reset_hook() cancels +* hash_del_rcu() +* call_rcu() +* +* A reader that resolved the storage before the unlink can arm past that +* cancel. Handlers resolve and use under one guard(rcu)(), so the two steps +* are issued separately here. +*/ +static void rv_test_rvtest_arm_after_destroy (struct kunit *test) +{ +struct rv_kunit_ctx *ctx = test->priv; +const int id =4242 ; +struct da_monitor *stale; + +prepare_test (test, &rvtest_mon);
kernel/trace/rv/rv_monitors_test.c static void rv_test_rvtest_underflow(struct kunit *test) · line 299
Quoted 10 lines
kernel/trace/rv/rv_monitors_test.c +10
-0+ +da_handle_start_run_event (id,NULL , start_rvtest); + +scoped_guard (rcu)/* resolve before the unlink */ + stale =da_get_monitor (id,NULL ); +KUNIT_ASSERT_NOT_NULL (test, stale); + +da_destroy_storage (id);/* cancel, unlink, call_rcu */ +ha_start_timer_ns (to_ha_monitor (stale), clk_rvtest, +RVTEST_BUDGET_NS ,ha_get_ns ());
[Severity: Medium]
Does this sequence result in an actual use-after-free rather than just simulating one?
Similar to the above, stale is resolved within a scoped_guard(rcu) that ends immediately. da_destroy_storage() then unlinks the object and defers its destruction via call_rcu().
Since the test thread no longer holds the RCU read lock, it can be preempted, allowing the RCU grace period to complete and free the stale object. When the test thread resumes, doesn't calling ha_start_timer_ns() write to the freed memory?
To safely simulate the race mentioned in the comment, shouldn't the test hold the RCU lock continuously across both the lookup and the timer arming steps?
Quoted 6 lines
kernel/trace/rv/rv_monitors_test.c +6
-0+ +rcu_barrier (); + +msleep (600 ); +RV_KUNIT_EXPECT_NO_REACTION (test, ctx); +}
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
[email protected]
AI reviewRe: [PATCH v7 13/14] selftests/verification: Add tlob selftests
In reply to [email protected]Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] selftests: commit message incorrectly claims run_tlob_tests.sh builds helpers --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] selftests: commit message incorrectly claims run_tlob_tests.sh builds helpers --
commit efda93f229ffc44f9ac300b085e473092fbfd5fd
Author: Wen Yang <[email protected]>
selftests/verification: Add tlob selftests
This commit adds ten ftrace-style test scripts for the tlob RV monitor, along with a target helper binary. It also introduces a runner script to handle ftracetest delegation.
Quoted 3 lines
Helper binarie
tlob_targetis built viaTEST_GEN_FILES, following the kselftests convention.run_tlob_tests.shbuilds the helpers and delegates argument handling to ftracetest.
[Severity: Low]
This isn't a bug in the code, but there is a typo (binarie), and the commit message states that run_tlob_tests.sh builds the helpers. Is this description accurate?
Looking at the added script, it explicitly refuses to build the helper and errors out if the binary is missing:
Quoted 5 lines
tools/testing/selftests/verification/test.d/tlob/run_tlob_tests.sh new file +0
-0
[ ... ]
tools/testing/selftests/verification/test.d/tlob/run_tlob_tests.sh line 19
Quoted 10 lines
tools/testing/selftests/verification/test.d/tlob/run_tlob_tests.sh new file +10
-0+# The helper is built on the host by the selftests Makefile +# (TEST_GEN_FILES in $BINDIR/Makefile); the VM (virtme-ng) mounts the +# source read-only and may lack a compiler, so this runner never builds. +# Build it on the host first: +# make -C "$BINDIR" tlob_target +if [ ! -x "$BINDIR/tlob_target" ]; then + echo "ERROR: tlob_target not built in $BINDIR" >&2 + echo " build it on the host: make -C $BINDIR tlob_target" >&2 + exit 1 +fi
Could the commit message be updated to clarify that the script expects the helper to be pre-built on the host rather than building it automatically?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13
[email protected]
AI reviewRe: [PATCH v7 04/14] rv: Disarm HA timers on the storage free path
In reply to [email protected]Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] rcu: missing rcu_barrier() on module unload causes kernel panic - [Low] timers: teardown path uses…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] rcu: missing rcu_barrier() on module unload causes kernel panic
- [Low] timers: teardown path uses timer_delete_sync() instead of timer_shutdown_sync() --
commit 659afa957e903e72b692e0af300f112338f0d03d
Author: Wen Yang <[email protected]>
rv: Disarm HA timers on the storage free path
This patch addresses a use-after-free issue where an HA timer can be queued on a storage object that is subsequently freed by kfree_rcu(). It adds a da_monitor_free_hook() callback just before the storage is freed to securely cancel the timer. To support this hook, kfree_rcu() is replaced with call_rcu() and a dedicated callback function.
Quoted 4 lines
include/rv/da_monitor.h +0
-0
[ ... ]
Quoted 26 lines
include/rv/da_monitor.h +10
-1@@ -543,6 +554,15 @@ static inline monitor_target da_get_target_by_id(da_id_type id)return mon_storage->target; } +static void da_kfree_cb (struct rcu_head *head) +{ +struct da_monitor_storage *ms = + container_of(head, struct da_monitor_storage, rcu); + +da_monitor_free_hook (&ms->rv.da_mon); +kfree (ms); +} +/* * da_destroy_storage - destroy the per-object storage * @@ -564,7 +584,7 @@ static inline void da_destroy_storage(da_id_type id)return ;da_monitor_reset_hook (&mon_storage->rv.da_mon);hash_del_rcu (&mon_storage->node); -kfree_rcu (mon_storage, rcu); +call_rcu (&mon_storage->rcu, da_kfree_cb); }static void __da_monitor_reset_all (void (*reset)(struct da_monitor *))
include/rv/da_monitor.h static inline void da_monitor_destroy(void) · line 633
Quoted 7 lines
include/rv/da_monitor.h +1
-0@@ -607,6 +627,7 @@ static inline void da_monitor_destroy(void)* pending, we can safely assume no concurrent user. */ hash_for_each_safe (da_monitor_ht, bkt, tmp, mon_storage, node) { +da_monitor_free_hook (&mon_storage->rv.da_mon);hash_del_rcu (&mon_storage->node);kfree (mon_storage); }
[Severity: High]
Can this code cause a kernel panic on module unload if a dynamically monitored object was destroyed right before?
Since da_kfree_cb() is defined as a static function in da_monitor.h, it will be compiled directly into the loadable monitor module's text segment. If da_destroy_storage() calls call_rcu() just before the module is unloaded, the RCU core could attempt to execute da_kfree_cb() after the module's text has been unmapped, causing an instruction page fault.
Should da_monitor_destroy() call rcu_barrier() to wait for these callbacks before the module unloads?
I noticed follow-up patches in the series add rcu_barrier() for the pool allocation strategy, but the default auto allocation strategy seems to be left vulnerable.
Quoted 4 lines
include/rv/ha_monitor.h +0
-0
[ ... ]
Quoted 4 lines
include/rv/ha_monitor.h +0
-0@@ -467,8 +470,15 @@ static inline bool ha_cancel_timer(struct ha_monitor *ha_mon) {return timer_delete (&ha_mon->timer); }
include/rv/ha_monitor.h static inline bool ha_cancel_timer(struct ha_monitor *ha_mon) · line 482
Quoted 10 lines
include/rv/ha_monitor.h +7
-0+/* timer_setup() sets it; the cancel paths here never clear it. */ +static inline bool ha_timer_ready (struct ha_monitor *ha_mon) +{ +return READ_ONCE (ha_mon->timer.function); +}static inline void ha_cancel_timer_sync (struct ha_monitor *ha_mon) { +if (!ha_timer_ready (ha_mon)) +return ;timer_delete_sync (&ha_mon->timer);
[Severity: Low]
Since this is the teardown path prior to freeing the storage object, should this be timer_shutdown_sync() instead?
The kernel timer subsystem guidelines mandate the use of timer_shutdown_sync() for teardown paths because it permanently prevents the timer from being re-armed. While concurrent RCU readers cannot reach the object at this point, using timer_delete_sync() for teardown is a documented anti-pattern.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
[email protected]
AI reviewRe: [PATCH v7 03/14] rv: Add a KUnit test for the HA timer underflow
In reply to [email protected]Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] rcu: scoped_guard(rcu) drops lock prematurely, leaking RCU-protected pointer --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] rcu: scoped_guard(rcu) drops lock prematurely, leaking RCU-protected pointer --
commit 578060191fd58b6c308a14708ec520459063ef75
Author: Wen Yang <[email protected]>
rv: Add a KUnit test for the HA timer underflow
This commit adds a minimal per-object HA monitor called rvtest. It also introduces a test to verify that the HA clock math does not underflow when checking invariants and starting timers.
Quoted 4 lines
kernel/trace/rv/rv_monitors_test.c +0
-0
[ ... ]
Quoted 6 lines
kernel/trace/rv/rv_monitors_test.c +6
-0+static void rv_test_rvtest_underflow (struct kunit *test) +{ +struct ha_monitor *ha_mon; +struct da_monitor *da_mon; +const int id =4242 ; +
kernel/trace/rv/rv_monitors_test.c static void rv_test_dummy(struct kunit *test) · line 251
Quoted 10 lines
kernel/trace/rv/rv_monitors_test.c +10
-0+prepare_test (test, &rvtest_mon); +da_handle_start_run_event (id,NULL , start_rvtest); + +scoped_guard (rcu) + da_mon =da_get_monitor (id,NULL ); +KUNIT_ASSERT_NOT_NULL (test, da_mon); + ha_mon =to_ha_monitor (da_mon); + +/* Uptime below the budget: time_ns - expire_ns would underflow. */ +ha_reset_clk_ns (ha_mon, clk_rvtest,0 );
[Severity: Medium]
Does this scoped_guard drop the lock prematurely?
Because there are no curly braces around the scoped_guard, the RCU read-side critical section only covers the single assignment to da_mon. Immediately after da_get_monitor() returns, the guard scope ends and the lock is released.
However, da_mon and its container ha_mon are dynamically allocated RCU-protected objects. They are extensively used outside the critical section in the subsequent test steps.
For example, ha_reset_clk_ns(), ha_start_timer_ns(), and ha_cancel_timer_sync() all dereference this pointer after the RCU read lock has been dropped. Could this violate RCU lifetime rules and create a use-after-free risk?
[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
[email protected]
AI reviewRe: [PATCH v7 06/14] rv: Make da_monitor_reset_hook and EVENT_NONE_LBL overridable
In reply to [email protected]Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] rv: undocumented API change making da_monitor_sync_hook overridable --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] rv: undocumented API change making da_monitor_sync_hook overridable --
commit 4c23c506287e928a0c126474d4bfba46c2216f6b
Author: Wen Yang <[email protected]>
rv: Make da_monitor_reset_hook and EVENT_NONE_LBL overridable
This commit adds #ifndef guards around da_monitor_reset_hook and EVENT_NONE_LBL in ha_monitor.h. This allows HA-based monitors to override these definitions with their own implementations before including the header, such as cancelling per-task hrtimers on teardown.
Quoted 6 lines
include/rv/ha_monitor.h +0
-0@@ -38,8 +38,14 @@ static bool ha_monitor_handle_constraint(struct da_monitor *da_mon, da_id_type id);
include/rv/ha_monitor.h static bool ha_monitor_handle_constraint(struct da_monitor *da_mon, · line 48
Quoted 10 lines
include/rv/ha_monitor.h +6
-0#define da_monitor_event_hook ha_monitor_handle_constraint#define da_monitor_init_hook ha_monitor_init_env + +/* Overrides must still call ha_monitor_reset_env() or cancel the timer. */ +#ifndef da_monitor_reset_hook#define da_monitor_reset_hook ha_monitor_reset_env +#endif +#ifndef da_monitor_sync_hook#define da_monitor_sync_hook () synchronize_rcu() +#endif
[Severity: Medium]
The commit message explicitly claims to "Make da_monitor_reset_hook and EVENT_NONE_LBL overridable" and mentions wrapping "both definitions".
However, this change also wraps a third macro, da_monitor_sync_hook, making it overridable as well.
Was this third API change intentional? If so, could the commit message be updated to mention it?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
[email protected]
AI reviewRe: [PATCH v7 08/14] rv: Add per-object teardown hook da_extra_cleanup
In reply to [email protected]Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] rv: missing rcu_barrier() for non-pool allocations causes UAF on module unload --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rv: missing rcu_barrier() for non-pool allocations causes UAF on module unload --
commit f8013d6f70621f02511ed8a820d487338e72eed7
Author: Wen Yang <[email protected]>
rv: Add per-object teardown hook da_extra_cleanup
da_monitor_destroy() frees each per-object storage, but a monitor whose target is allocated separately has no point to free it: the target is still reachable from the storage until hash_del_rcu(). Add an optional da_extra_cleanup() hook, called for each storage after da_monitor_free_hook() has disarmed its timer and before the storage is unlinked. It defaults to a no-op. tlob, added later in this series, uses it to retire and free its per-window record.
Quoted 4 lines
include/rv/da_monitor.h +0
-0
[ ... ]
include/rv/da_monitor.h static inline void da_monitor_destroy(void) · line 733
Quoted 10 lines
include/rv/da_monitor.h +2
-0@@ -715,7 +725,9 @@ static inline void da_monitor_destroy(void)* pending, we can safely assume no concurrent user. */ hash_for_each_safe (da_monitor_ht, bkt, tmp, mon_storage, node) { +/* Before da_extra_cleanup(): it frees what the timer reads. */ da_monitor_free_hook (&mon_storage->rv.da_mon); +da_extra_cleanup (&mon_storage->rv.da_mon);hash_del_rcu (&mon_storage->node);if (DA_MON_ALLOCATION_STRATEGY ==DA_ALLOC_POOL )mempool_free (mon_storage, &da_monitor_pool);
[Severity: High]
Does this missing rcu_barrier() for non-pool allocations cause a use-after-free on module unload?
Further down in da_monitor_destroy(), rcu_barrier() is only called for DA_ALLOC_POOL allocations:
include/rv/da_monitor.h:da_monitor_destroy() {
...
if (DA_MON_ALLOCATION_STRATEGY == DA_ALLOC_POOL) {
rcu_barrier();
mempool_exit(&da_monitor_pool);
}
}When a monitor using DA_ALLOC_AUTO or DA_ALLOC_MANUAL is active and an event calls da_destroy_storage(), it queues the da_kfree_cb() callback:
include/rv/da_monitor.h:da_destroy_storage() {
...
if (DA_MON_ALLOCATION_STRATEGY == DA_ALLOC_POOL)
call_rcu(&mon_storage->rcu, da_pool_return_cb);
else
call_rcu(&mon_storage->rcu, da_kfree_cb);
}Because da_kfree_cb() is a static function in this header, its code resides in the loadable module of the specific monitor.
If the user unloads the monitor module immediately after a monitored object is destroyed, da_monitor_destroy() is called during the module exit path. Since there is no rcu_barrier() for non-pool allocations, the module's text segment is unmapped while da_kfree_cb() is still pending in the RCU queues.
This leads to a kernel panic when the RCU core attempts to execute the unmapped callback.
Should the rcu_barrier() be called unconditionally so that all pending RCU callbacks finish executing before the module exits and its memory is unmapped?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8
[email protected]
AI reviewRe: [PATCH v7 07/14] rv: Introduce DA_MON_ALLOCATION_STRATEGY
In reply to [email protected]Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] mm: spinlock_t acquired in tracepoint via DA_ALLOC_POOL - [High] rcu: unprotected hash_add_rcu()…
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] mm: spinlock_t acquired in tracepoint via DA_ALLOC_POOL
- [High] rcu: unprotected hash_add_rcu() corrupts da_monitor_ht
- [High] rv: execution after free on module unload due to missing rcu_barrier()
- [Low] kmemleak: asymmetric kfree() of kmalloc_nolock() storage
--
commit 0a1871af9a55378524a2c76be086f8e3f24c7568
Author: Wen Yang <[email protected]>
rv: Introduce DA_MON_ALLOCATION_STRATEGY
Per-object DA storage allocation is currently limited to kmalloc on demand. Add a compile-time selector so monitors can choose among three strategies:
DA_ALLOC_AUTO (default) - kmalloc per object on the monitor path
DA_ALLOC_POOL - pre-allocated fixed-size mempool;
selected by defining DA_MON_POOL_SIZE
DA_ALLOC_MANUAL - caller pre-inserts storage; framework
only links the target field
nomiss switches from the implicit DA_SKIP_AUTO_ALLOC override to an explicit DA_MON_ALLOCATION_STRATEGY DA_ALLOC_MANUAL; the behaviours are equivalent (pre-allocated storage, no allocation on the sched_switch start path).
Quoted 4 lines
include/rv/da_monitor.h +0
-0
[ ... ]
Quoted 1 line
include/rv/da_monitor.h +0
-0@@ -563,6 +601,46 @@ static void da_kfree_cb(struct rcu_head *head)
[ ... ]
Quoted 7 lines
include/rv/da_monitor.h +7
-0+/* +* da_create_pool_storage - pop a free pool slot and insert it into the hash. +* Returns NULL if the pool is exhausted. Caller holds an RCU read-side CS +* and the monitor's serialisation lock. +*/ +static inline struct da_monitor * +da_create_pool_storage(da_id_type id, monitor_target target,
include/rv/da_monitor.h static void da_kfree_cb(struct rcu_head *head) · line 634
Quoted 10 lines
include/rv/da_monitor.h +10
-0+ struct da_monitor *da_mon) +{ +struct da_monitor_storage *mon_storage; + +if (da_mon) +return da_mon; + + mon_storage =mempool_alloc_preallocated (&da_monitor_pool); +if (!mon_storage) +return NULL ;
[Severity: High]
Does this code acquire a spinlock_t inside an atomic context?
Tracepoint handlers, such as sched_switch handled by nomiss, execute in an atomic context with preemption disabled.
The mempool_alloc_preallocated() function internally acquires the pool's spinlock_t lock.
On preempt-rt kernels, spinlock_t maps to a sleeping rt_mutex. Acquiring it inside an atomic, preemption-disabled context violates the nesting hierarchy, resulting in "Invalid wait context" lockdep splats and a sleep-in-atomic panic.
include/rv/da_monitor.h static void da_kfree_cb(struct rcu_head *head) · line 642
Quoted 8 lines
include/rv/da_monitor.h +8
-0+memset (mon_storage,0 ,sizeof (*mon_storage)); + + mon_storage->id = id; + mon_storage->target = target; + +hash_add_rcu (da_monitor_ht, &mon_storage->node, id); +return &mon_storage->rv.da_mon; +}
[Severity: High]
Will concurrent modifications corrupt the global da_monitor_ht?
Tracepoint handlers execute concurrently across multiple CPUs. When starting a new monitor for an object, the caller only holds an RCU read lock, providing no mutual exclusion against concurrent writers on other CPUs.
Because hash_add_rcu() uses hlist_add_head_rcu() which modifies the global bucket's head pointer, simultaneous unprotected writes from different CPUs tracking different targets could corrupt the bucket's linked list structure and cause traversal hangs or lost objects.
[ ... ]
include/rv/da_monitor.h static inline void da_destroy_storage(da_id_type id) · line 669
Quoted 10 lines
include/rv/da_monitor.h +4
-1@@ -584,7 +662,10 @@ static inline void da_destroy_storage(da_id_type id)return ;da_monitor_reset_hook (&mon_storage->rv.da_mon);hash_del_rcu (&mon_storage->node); -call_rcu (&mon_storage->rcu, da_kfree_cb); +if (DA_MON_ALLOCATION_STRATEGY ==DA_ALLOC_POOL ) +call_rcu (&mon_storage->rcu, da_pool_return_cb); +else +call_rcu (&mon_storage->rcu, da_kfree_cb); }
[Severity: Low]
Are there kmemleak warnings triggered by this asymmetric free?
Monitor storage for DA_ALLOC_AUTO and DA_ALLOC_MANUAL is allocated using kmalloc_nolock() inside da_create_empty_storage().
This path queues da_kfree_cb() which frees the memory using the standard kfree() API.
Allocations made with kmalloc_nolock() must be paired precisely with kfree_nolock(). Mixing them bypasses the kmemleak tracking hooks unevenly, which can cause "Trying to color unknown object" warnings or false memory leak reports.
[ ... ]
Quoted 6 lines
include/rv/da_monitor.h +1
-1@@ -629,20 +717,33 @@ static inline void da_monitor_destroy(void)hash_for_each_safe (da_monitor_ht, bkt, tmp, mon_storage, node) {da_monitor_free_hook (&mon_storage->rv.da_mon);hash_del_rcu (&mon_storage->node); -kfree (mon_storage); +if (DA_MON_ALLOCATION_STRATEGY ==DA_ALLOC_POOL )
include/rv/da_monitor.h static inline void da_monitor_destroy(void) · line 730
Quoted 10 lines
include/rv/da_monitor.h +8
-0+mempool_free (mon_storage, &da_monitor_pool); + else +kfree (mon_storage ); + } + +if (DA_MON_ALLOCATION_STRATEGY ==DA_ALLOC_POOL ) { +rcu_barrier (); +mempool_exit (&da_monitor_pool); } }
[Severity: High]
Is it safe to skip rcu_barrier() for the other allocation strategies?
For DA_ALLOC_AUTO and DA_ALLOC_MANUAL, call_rcu() is used to queue da_kfree_cb. Because da_kfree_cb is declared as static inside include/rv/da_monitor.h, its text is compiled directly into the dynamically loaded RV monitor module.
If da_destroy_storage() is called shortly before the module is unloaded, the callbacks will be queued. If da_monitor_destroy() doesn't call rcu_barrier() to wait for pending callbacks, the module will be unloaded and the RCU softirq will jump to an unmapped callback address, causing a kernel panic.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
[email protected]
AI reviewRe: [PATCH v7 11/14] rv: Add tlob hybrid automaton monitor
In reply to [email protected]Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [Critical] rv: TOCTOU race in ha_setup_invariants allows hrtimer Use-After-Free - [High] rv: deadlock in…
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] rv: TOCTOU race in ha_setup_invariants allows hrtimer Use-After-Free
- [High] rv: deadlock in tlob_binding_lock between process context and RCU softirq
- [High] rv: tlob: list corruption due to lockless da_destroy_storage() calls
- [Medium] rv: race between tlob_stop_task and timer callback causes INVALID_STATE error --
commit 90ea80dda3503cc5ac796d8401bafbbd97a373b9
Author: Wen Yang <[email protected]>
rv: Add tlob hybrid automaton monitor
tlob (task latency over budget) is a per-task hybrid automaton RV monitor that tracks wall-clock time across a user-delimited code section and emits an error when the elapsed time exceeds a configurable budget.
Quoted 3 lines
kernel/trace/rv/monitors/tlob/tlob.c new file +0
-0
[ ... ]
Quoted 18 lines
kernel/trace/rv/monitors/tlob/tlob.c new file +18
-0+static inline void tlob_detach_from_binding (struct tlob_record *rec) +{ +guard (spinlock)(&tlob_binding_lock); + +if (!rec->binding) +return ; + rec->binding =NULL ; +list_del_init (&rec->started_node); +} + +/* +* Per-record teardown during monitor disable. TLOB_RETIRED claims it, so an +* exit or unbind racing disable frees the record once. +*/ +static inline void tlob_extra_cleanup (struct da_monitor *da_mon) +{ +struct ha_monitor *ha_mon =to_ha_monitor (da_mon); +struct tlob_record *rec =ha_get_target (ha_mon);
kernel/trace/rv/monitors/tlob/tlob.c line 286
Quoted 10 lines
kernel/trace/rv/monitors/tlob/tlob.c new file +10
-0+ +if (!rec) +return ; + +if (atomic_xchg (&rec->state,TLOB_RETIRED ) ==TLOB_RETIRED ) +return ; + +tlob_detach_from_binding (rec); +call_rcu (&rec->rcu, tlob_rec_return_cb); +}
[Severity: High]
Does this lock acquisition risk a deadlock between process context and RCU softirq?
The tlob_binding_lock is acquired in process context functions like tlob_start_task() using scoped_guard(spinlock, &tlob_binding_lock), which does not disable softirqs.
During monitor teardown, the RV framework's da_destroy_storage() frees the monitor record via call_rcu(). The RCU callback executes in RCU_SOFTIRQ context and invokes tlob_extra_cleanup(), which calls tlob_detach_from_binding() and acquires the same lock.
If a process holding the lock is interrupted by the RCU softirq on the same CPU, the softirq will deadlock attempting to acquire it. Should tlob_binding_lock be acquired using spin_lock_bh() in all process-context paths?
[ ... ]
Quoted 21 lines
kernel/trace/rv/monitors/tlob/tlob.c new file +21
-0+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) +{ +struct tlob_record *rec =ha_get_target (ha_mon); + +if (next_state == stopped_tlob) +return ; + +if (next_state >= state_max_tlob) { +ha_cancel_timer (ha_mon); +return ; + } + +if (event != start_tlob) { +/* A sched event racing the window end must not re-arm. */ +if (atomic_read_acquire (&rec->state) !=TLOB_OPEN ) +return ; + ha_start_timer_ns(ha_mon, clk_elapsed_tlob,
kernel/trace/rv/monitors/tlob/tlob.c line 182
Quoted 10 lines
kernel/trace/rv/monitors/tlob/tlob.c new file +10
-0+ BUDGET_NS(ha_mon), time_ns); +return ; + } + +ha_reset_clk_ns (ha_mon, clk_elapsed_tlob, time_ns); +ha_start_timer_ns (ha_mon, clk_elapsed_tlob,BUDGET_NS (ha_mon), + time_ns); +/* Reopen only a closed record; a teardown claim keeps it closed. */ +atomic_cmpxchg_release (&rec->state,TLOB_CLOSED ,TLOB_OPEN ); +}
[Severity: Critical]
Can this sequence allow a use-after-free of the hrtimer?
There appears to be a race between the state check and arming the timer. A teardown path can concurrently claim the record by setting its state to TLOB_RETIRED, cancel the timer, and queue the record for freeing via call_rcu().
If the teardown occurs between ha_setup_invariants() checking the state and calling ha_start_timer_ns(), the timer will be freshly re-armed on a record that has already been queued for destruction.
Even in the start_tlob path, ha_start_timer_ns() is called unconditionally before atomic_cmpxchg_release(). If the cmpxchg fails because of a concurrent teardown, the armed timer is never rolled back.
Once the RCU grace period elapses, the record is freed. The still-armed timer remains on the hrtimer rbtree, leading to a use-after-free when it expires.
[ ... ]
Quoted 20 lines
kernel/trace/rv/monitors/tlob/tlob.c new file +20
-0+static void tlob_stop_task (struct task_struct *task, +struct tlob_uprobe_binding *binding) +{ +struct ha_monitor *ha_mon; +struct tlob_record *rec; + +scoped_guard (rcu) { + ha_mon =ha_get_monitor (task->pid,NULL ); +if (!ha_mon) +return ; + + rec =ha_get_target (ha_mon); +if (WARN_ON_ONCE (!rec)) +return ; + +/* Only the opener may end it; NULL (task exit) skips. */ +if (binding &&READ_ONCE (rec->binding) != binding) +return ; + +/* Claim the window end; the timer may have taken it first. */
kernel/trace/rv/monitors/tlob/tlob.c line 454
Quoted 10 lines
kernel/trace/rv/monitors/tlob/tlob.c new file +10
-0+if (atomic_cmpxchg_release (&rec->state,TLOB_OPEN , +TLOB_CLOSED ) !=TLOB_OPEN ) +return ; + +ha_cancel_timer_sync (ha_mon); + } + +/* The DA returns to stopped; the record stays closed. */ +da_handle_event (task->pid,NULL , stop_tlob); +}
[Severity: Medium]
Will this result in an invalid state error if tlob_stop_task() races with the timer callback?
tlob_stop_task() claims the window closure and then calls ha_cancel_timer_sync(). If the hrtimer just expired and its callback is running concurrently on another CPU, ha_cancel_timer_sync() spin-waits for it to complete.
The generic timer callback will unconditionally emit a budget violation error and reset the automaton to stopped_tlob. After ha_cancel_timer_sync() returns, tlob_stop_task() injects the stop_tlob event.
Since the automaton defines stop_tlob from the stopped_tlob state as invalid, this causes the framework to incorrectly flag an invalid transition despite tlob_stop_task() legitimately ending the window.
[ ... ]
Quoted 7 lines
kernel/trace/rv/monitors/tlob/tlob.c new file +7
-0+static void tlob_destroy_claimed (struct tlob_record *rec) +{ +pid_t id = rec->id; + +tlob_detach_from_binding (rec); + +scoped_guard (rcu) {
kernel/trace/rv/monitors/tlob/tlob.c line 476
Quoted 10 lines
kernel/trace/rv/monitors/tlob/tlob.c new file +10
-0+struct ha_monitor *ha_mon =ha_get_monitor (id,NULL ); + +if (!WARN_ON_ONCE (!ha_mon)) +da_monitor_reset (&ha_mon->da_mon); + } +da_destroy_storage (id); + +/* Readers may still hold rec; the pool slot returns after a GP. */ +call_rcu (&rec->rcu, tlob_rec_return_cb); +}
[Severity: High]
Does this code corrupt the hash table list pointers?
da_monitor_ht is a global hash table without internal locking. The function da_destroy_storage() uses hash_del_rcu(), which modifies hlist pointers and requires the caller to hold a writer lock to serialize against concurrent writers.
While tlob_start_task() acquires tlob_binding_lock to serialize additions, tlob_destroy_claimed() and the error path of tlob_start_task() call da_destroy_storage() without holding tlob_binding_lock or any other bucket-level lock.
Concurrent task exits or unbind operations that map to the same hash bucket could corrupt the hash table's linked list.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11
kernel test robot
build botRe: [PATCH v7 03/14] rv: Add a KUnit test for the HA timer underflow
In reply to [email protected]kernel test robot noticed the following build errors:
Hi,
kernel test robot noticed the following build errors:
[auto build test ERROR on trace/for-next]
[also build test ERROR on shuah-kselftest/next shuah-kselftest/fixes linus/master v7.3-rc6 next-20261009] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/wen-yang-linux-dev/rv-Force-environment-reset-action-on-HA-monitor-start/20261012-033907 base: https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace for-next patch link: https://lore.kernel.org/r/cdaf33af866392428f1942cef4e442b1d106989c.1791745088.git.wen.yang%40linux.dev patch subject: [PATCH v7 03/14] rv: Add a KUnit test for the HA timer underflow config: sh-randconfig-1001-20261012 (https://download.01.org/0day-ci/archive/20261012/[email protected]/config) compiler: sh4-linux-gcc (GCC) 16.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261012/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All error/warnings (new ones prefixed by >>):
In file included from include/uapi/linux/posix_types.h:5,
from include/uapi/linux/types.h:14,
from include/linux/types.h:5,
from include/linux/kasan-checks.h:5,
from include/asm-generic/rwonce.h:26,
from ./arch/sh/include/generated/asm/rwonce.h:1,
from include/linux/compiler.h:369,
from include/linux/err.h:5,
from include/kunit/assert.h:12,
from include/kunit/test.h:12,
from include/rv/kunit.h:16,
from kernel/trace/rv/rv_monitors_test.c:10:
>> include/linux/stddef.h:16:33: error: 'struct ha_monitor' has no member named 'da_mon'
16 | #define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)
| ^~~~~~~~~~~~~~~~~~
include/rv/ha_monitor.h:66:16: note: in expansion of macro 'offsetof'
66 | _Static_assert(offsetof(struct ha_monitor, da_mon) == 0,
| ^~~~~~~~
>> include/linux/stddef.h:16:33: error: expression in static assertion is not an integer
include/rv/ha_monitor.h:66:16: note: in expansion of macro 'offsetof'
66 | _Static_assert(offsetof(struct ha_monitor, da_mon) == 0,
| ^~~~~~~~
>> kernel/trace/rv/rv_monitors_test.c:21:23: warning: 'HA_TIMER_HRTIMER' is not defined, evaluates to '0' [-Wundef]
21 | #define HA_TIMER_TYPE HA_TIMER_HRTIMER
| ^~~~~~~~~~~~~~~~
include/rv/ha_monitor.h:122:5: note: in expansion of macro 'HA_TIMER_TYPE'
122 | #if HA_TIMER_TYPE == HA_TIMER_WHEEL
| ^~~~~~~~~~~~~
In file included from kernel/trace/rv/rv_monitors_test.c:71:
>> include/rv/ha_monitor.h:122:22: warning: 'HA_TIMER_WHEEL' is not defined, evaluates to '0' [-Wundef]
122 | #if HA_TIMER_TYPE == HA_TIMER_WHEEL
| ^~~~~~~~~~~~~~
In file included from include/linux/init.h:5,
from include/linux/printk.h:6,
from include/kunit/assert.h:13:
include/rv/ha_monitor.h: In function 'ha_monitor_init_env':
>> include/linux/container_of.h:8:47: error: 'struct ha_monitor' has no member named 'da_mon'
8 | #define typeof_member(T, m) typeof(((T*)0)->m)
| ^~
include/linux/build_bug.h:80:56: note: in definition of macro '__static_assert'
80 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~
include/linux/container_of.h:20:43: note: in expansion of macro 'typeof_member'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~~~
include/rv/ha_monitor.h:68:27: note: in expansion of macro 'container_of'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ^~~~~~~~~~~~
include/rv/ha_monitor.h:192:37: note: in expansion of macro 'to_ha_monitor'
192 | struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~
include/linux/compiler_types.h:610:27: error: expression in static assertion is not an integer
610 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:80:56: note: in definition of macro '__static_assert'
80 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~
include/rv/ha_monitor.h:68:27: note: in expansion of macro 'container_of'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ^~~~~~~~~~~~
include/rv/ha_monitor.h:192:37: note: in expansion of macro 'to_ha_monitor'
192 | struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~
>> include/linux/stddef.h:16:33: error: 'struct ha_monitor' has no member named 'da_mon'
16 | #define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)
| ^~~~~~~~~~~~~~~~~~
include/linux/container_of.h:23:34: note: in expansion of macro 'offsetof'
23 | (type *)((void *)(ptr) - offsetof(type, member)); })
| ^~~~~~~~
include/rv/ha_monitor.h:68:27: note: in expansion of macro 'container_of'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ^~~~~~~~~~~~
include/rv/ha_monitor.h:192:37: note: in expansion of macro 'to_ha_monitor'
192 | struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~
include/rv/ha_monitor.h: In function 'ha_monitor_reset_env':
>> include/linux/container_of.h:8:47: error: 'struct ha_monitor' has no member named 'da_mon'
8 | #define typeof_member(T, m) typeof(((T*)0)->m)
| ^~
include/linux/build_bug.h:80:56: note: in definition of macro '__static_assert'
80 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~
include/linux/container_of.h:20:43: note: in expansion of macro 'typeof_member'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~~~
include/rv/ha_monitor.h:68:27: note: in expansion of macro 'container_of'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ^~~~~~~~~~~~
include/rv/ha_monitor.h:208:37: note: in expansion of macro 'to_ha_monitor'
208 | struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~
include/linux/compiler_types.h:610:27: error: expression in static assertion is not an integer
610 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:80:56: note: in definition of macro '__static_assert'
80 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~
include/rv/ha_monitor.h:68:27: note: in expansion of macro 'container_of'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ^~~~~~~~~~~~
include/rv/ha_monitor.h:208:37: note: in expansion of macro 'to_ha_monitor'
208 | struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~
>> include/linux/stddef.h:16:33: error: 'struct ha_monitor' has no member named 'da_mon'
16 | #define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)
| ^~~~~~~~~~~~~~~~~~
include/linux/container_of.h:23:34: note: in expansion of macro 'offsetof'
23 | (type *)((void *)(ptr) - offsetof(type, member)); })
| ^~~~~~~~
include/rv/ha_monitor.h:68:27: note: in expansion of macro 'container_of'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ^~~~~~~~~~~~
include/rv/ha_monitor.h:208:37: note: in expansion of macro 'to_ha_monitor'
208 | struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~
In file included from <command-line>:
include/rv/ha_monitor.h: In function 'ha_monitor_env_invalid':
>> include/rv/ha_monitor.h:219:32: error: 'struct ha_monitor' has no member named 'env_store'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ^~
include/linux/compiler_types.h:682:23: note: in definition of macro '__compiletime_assert'
682 | if (!(condition)) \
| ^~~~~~~~~
include/linux/compiler_types.h:702:9: note: in expansion of macro '_compiletime_assert'
702 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:9: note: in expansion of macro 'compiletime_assert'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:28: note: in expansion of macro '__native_word'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~
include/asm-generic/rwonce.h:49:9: note: in expansion of macro 'compiletime_assert_rwonce_type'
49 | compiletime_assert_rwonce_type(x); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/rv/ha_monitor.h:219:16: note: in expansion of macro 'READ_ONCE'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ^~~~~~~~~
>> include/rv/ha_monitor.h:219:32: error: 'struct ha_monitor' has no member named 'env_store'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ^~
include/linux/compiler_types.h:682:23: note: in definition of macro '__compiletime_assert'
682 | if (!(condition)) \
| ^~~~~~~~~
include/linux/compiler_types.h:702:9: note: in expansion of macro '_compiletime_assert'
702 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:9: note: in expansion of macro 'compiletime_assert'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:28: note: in expansion of macro '__native_word'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~
include/asm-generic/rwonce.h:49:9: note: in expansion of macro 'compiletime_assert_rwonce_type'
49 | compiletime_assert_rwonce_type(x); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/rv/ha_monitor.h:219:16: note: in expansion of macro 'READ_ONCE'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ^~~~~~~~~
>> include/rv/ha_monitor.h:219:32: error: 'struct ha_monitor' has no member named 'env_store'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ^~
include/linux/compiler_types.h:682:23: note: in definition of macro '__compiletime_assert'
682 | if (!(condition)) \
| ^~~~~~~~~
include/linux/compiler_types.h:702:9: note: in expansion of macro '_compiletime_assert'
702 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:9: note: in expansion of macro 'compiletime_assert'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:28: note: in expansion of macro '__native_word'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~
include/asm-generic/rwonce.h:49:9: note: in expansion of macro 'compiletime_assert_rwonce_type'
49 | compiletime_assert_rwonce_type(x); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/rv/ha_monitor.h:219:16: note: in expansion of macro 'READ_ONCE'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ^~~~~~~~~
>> include/rv/ha_monitor.h:219:32: error: 'struct ha_monitor' has no member named 'env_store'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ^~
include/linux/compiler_types.h:682:23: note: in definition of macro '__compiletime_assert'
682 | if (!(condition)) \
| ^~~~~~~~~
include/linux/compiler_types.h:702:9: note: in expansion of macro '_compiletime_assert'
702 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:9: note: in expansion of macro 'compiletime_assert'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:28: note: in expansion of macro '__native_word'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~
include/asm-generic/rwonce.h:49:9: note: in expansion of macro 'compiletime_assert_rwonce_type'
49 | compiletime_assert_rwonce_type(x); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/rv/ha_monitor.h:219:16: note: in expansion of macro 'READ_ONCE'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ^~~~~~~~~
>> include/rv/ha_monitor.h:219:32: error: 'struct ha_monitor' has no member named 'env_store'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ^~
include/linux/compiler_types.h:682:23: note: in definition of macro '__compiletime_assert'
682 | if (!(condition)) \
| ^~~~~~~~~
include/linux/compiler_types.h:702:9: note: in expansion of macro '_compiletime_assert'
702 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:9: note: in expansion of macro 'compiletime_assert'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:49:9: note: in expansion of macro 'compiletime_assert_rwonce_type'
49 | compiletime_assert_rwonce_type(x); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/rv/ha_monitor.h:219:16: note: in expansion of macro 'READ_ONCE'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ^~~~~~~~~
>> include/rv/ha_monitor.h:219:32: error: 'struct ha_monitor' has no member named 'env_store'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ^~
include/linux/compiler_types.h:635:53: note: in definition of macro '__unqual_scalar_typeof'
635 | #define __unqual_scalar_typeof(x) __typeof_unqual__(x)
| ^
include/asm-generic/rwonce.h:50:9: note: in expansion of macro '__READ_ONCE'
50 | __READ_ONCE(x); \
| ^~~~~~~~~~~
include/rv/ha_monitor.h:219:16: note: in expansion of macro 'READ_ONCE'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ^~~~~~~~~
>> include/rv/ha_monitor.h:219:32: error: 'struct ha_monitor' has no member named 'env_store'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ^~
include/asm-generic/rwonce.h:44:73: note: in definition of macro '__READ_ONCE'
44 | #define __READ_ONCE(x) (*(const volatile __unqual_scalar_typeof(x) *)&(x))
| ^
include/rv/ha_monitor.h:219:16: note: in expansion of macro 'READ_ONCE'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ^~~~~~~~~
>> include/rv/ha_monitor.h:217:80: warning: parameter 'env' set but not used [-Wunused-but-set-parameter=]
217 | static inline bool ha_monitor_env_invalid(struct ha_monitor *ha_mon, enum envs env)
| ~~~~~~~~~~^~~
include/rv/ha_monitor.h: In function 'ha_monitor_handle_constraint':
>> include/linux/container_of.h:8:47: error: 'struct ha_monitor' has no member named 'da_mon'
8 | #define typeof_member(T, m) typeof(((T*)0)->m)
| ^~
include/linux/build_bug.h:80:56: note: in definition of macro '__static_assert'
80 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~
include/linux/container_of.h:20:43: note: in expansion of macro 'typeof_member'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~~~
include/rv/ha_monitor.h:68:27: note: in expansion of macro 'container_of'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ^~~~~~~~~~~~
include/rv/ha_monitor.h:286:37: note: in expansion of macro 'to_ha_monitor'
286 | struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~
include/linux/compiler_types.h:610:27: error: expression in static assertion is not an integer
610 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:80:56: note: in definition of macro '__static_assert'
80 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~
include/rv/ha_monitor.h:68:27: note: in expansion of macro 'container_of'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ^~~~~~~~~~~~
include/rv/ha_monitor.h:286:37: note: in expansion of macro 'to_ha_monitor'
286 | struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~
>> include/linux/stddef.h:16:33: error: 'struct ha_monitor' has no member named 'da_mon'
16 | #define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)
| ^~~~~~~~~~~~~~~~~~
include/linux/container_of.h:23:34: note: in expansion of macro 'offsetof'
23 | (type *)((void *)(ptr) - offsetof(type, member)); })
| ^~~~~~~~
include/rv/ha_monitor.h:68:27: note: in expansion of macro 'container_of'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ^~~~~~~~~~~~
include/rv/ha_monitor.h:286:37: note: in expansion of macro 'to_ha_monitor'
286 | struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~
include/rv/ha_monitor.h: In function '__ha_monitor_timer_callback':
>> include/rv/ha_monitor.h:321:46: error: 'struct ha_monitor' has no member named 'da_mon'
321 | curr_state = smp_load_acquire(&ha_mon->da_mon.curr_state);
| ^~
include/linux/compiler_types.h:635:53: note: in definition of macro '__unqual_scalar_typeof'
635 | #define __unqual_scalar_typeof(x) __typeof_unqual__(x)
| ^
include/rv/ha_monitor.h:321:22: note: in expansion of macro 'smp_load_acquire'
321 | curr_state = smp_load_acquire(&ha_mon->da_mon.curr_state);
| ^~~~~~~~~~~~~~~~
>> include/rv/ha_monitor.h:321:46: error: 'struct ha_monitor' has no member named 'da_mon'
321 | curr_state = smp_load_acquire(&ha_mon->da_mon.curr_state);
| ^~
include/linux/compiler_types.h:682:23: note: in definition of macro '__compiletime_assert'
682 | if (!(condition)) \
| ^~~~~~~~~
include/linux/compiler_types.h:702:9: note: in expansion of macro '_compiletime_assert'
702 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:9: note: in expansion of macro 'compiletime_assert'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:28: note: in expansion of macro '__native_word'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~
include/asm-generic/rwonce.h:49:9: note: in expansion of macro 'compiletime_assert_rwonce_type'
49 | compiletime_assert_rwonce_type(x); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/asm-generic/barrier.h:204:44: note: in expansion of macro 'READ_ONCE'
204 | __unqual_scalar_typeof(*p) ___p1 = READ_ONCE(*p); \
| ^~~~~~~~~
include/rv/ha_monitor.h:321:22: note: in expansion of macro 'smp_load_acquire'
321 | curr_state = smp_load_acquire(&ha_mon->da_mon.curr_state);
| ^~~~~~~~~~~~~~~~
include/rv/ha_monitor.h:321:46: error: 'struct ha_monitor' has no member named 'da_mon'
321 | curr_state = smp_load_acquire(&ha_mon->da_mon.curr_state);
| ^~
include/linux/compiler_types.h:682:23: note: in definition of macro '__compiletime_assert'
682 | if (!(condition)) \
| ^~~~~~~~~
include/linux/compiler_types.h:702:9: note: in expansion of macro '_compiletime_assert'
702 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:9: note: in expansion of macro 'compiletime_assert'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:28: note: in expansion of macro '__native_word'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~
include/asm-generic/rwonce.h:49:9: note: in expansion of macro 'compiletime_assert_rwonce_type'
49 | compiletime_assert_rwonce_type(x); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/asm-generic/barrier.h:204:44: note: in expansion of macro 'READ_ONCE'
204 | __unqual_scalar_typeof(*p) ___p1 = READ_ONCE(*p); \
| ^~~~~~~~~
include/rv/ha_monitor.h:321:22: note: in expansion of macro 'smp_load_acquire'
321 | curr_state = smp_load_acquire(&ha_mon->da_mon.curr_state);
| ^~~~~~~~~~~~~~~~
include/rv/ha_monitor.h:321:46: error: 'struct ha_monitor' has no member named 'da_mon'
321 | curr_state = smp_load_acquire(&ha_mon->da_mon.curr_state);
| ^~
include/linux/compiler_types.h:682:23: note: in definition of macro '__compiletime_assert'
682 | if (!(condition)) \
| ^~~~~~~~~
include/linux/compiler_types.h:702:9: note: in expansion of macro '_compiletime_assert'
702 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:9: note: in expansion of macro 'compiletime_assert'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:28: note: in expansion of macro '__native_word'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~
include/asm-generic/rwonce.h:49:9: note: in expansion of macro 'compiletime_assert_rwonce_type'
49 | compiletime_assert_rwonce_type(x); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/asm-generic/barrier.h:204:44: note: in expansion of macro 'READ_ONCE'
204 | __unqual_scalar_typeof(*p) ___p1 = READ_ONCE(*p); \
| ^~~~~~~~~
include/rv/ha_monitor.h:321:22: note: in expansion of macro 'smp_load_acquire'
321 | curr_state = smp_load_acquire(&ha_mon->da_mon.curr_state);
| ^~~~~~~~~~~~~~~~
include/rv/ha_monitor.h:321:46: error: 'struct ha_monitor' has no member named 'da_mon'
321 | curr_state = smp_load_acquire(&ha_mon->da_mon.curr_state);
| ^~
include/linux/compiler_types.h:682:23: note: in definition of macro '__compiletime_assert'
682 | if (!(condition)) \
| ^~~~~~~~~
include/linux/compiler_types.h:702:9: note: in expansion of macro '_compiletime_assert'
702 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:9: note: in expansion of macro 'compiletime_assert'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:28: note: in expansion of macro '__native_word'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~
include/asm-generic/rwonce.h:49:9: note: in expansion of macro 'compiletime_assert_rwonce_type'
49 | compiletime_assert_rwonce_type(x); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/asm-generic/barrier.h:204:44: note: in expansion of macro 'READ_ONCE'
204 | __unqual_scalar_typeof(*p) ___p1 = READ_ONCE(*p); \
| ^~~~~~~~~
include/rv/ha_monitor.h:321:22: note: in expansion of macro 'smp_load_acquire'
321 | curr_state = smp_load_acquire(&ha_mon->da_mon.curr_state);
| ^~~~~~~~~~~~~~~~
include/rv/ha_monitor.h:321:46: error: 'struct ha_monitor' has no member named 'da_mon'
321 | curr_state = smp_load_acquire(&ha_mon->da_mon.curr_state);
| ^~
include/linux/compiler_types.h:682:23: note: in definition of macro '__compiletime_assert'
682 | if (!(condition)) \vim +16 include/linux/stddef.h
6e2182874324727 Richard Knutsson 2006-09-30 14
^1da177e4c3f415 Linus Torvalds 2005-04-16 15 #undef offsetof14e83077d55ff4b Rasmus Villemoes 2022-03-23 @16 #define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)3876488444e7123 Denys Vlasenko 2015-03-09 17
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
kernel test robot
build botRe: [PATCH v7 03/14] rv: Add a KUnit test for the HA timer underflow
In reply to [email protected]kernel test robot noticed the following build errors:
Hi,
kernel test robot noticed the following build errors:
[auto build test ERROR on trace/for-next]
[also build test ERROR on shuah-kselftest/next shuah-kselftest/fixes linus/master v7.3-rc6 next-20261009] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/wen-yang-linux-dev/rv-Force-environment-reset-action-on-HA-monitor-start/20261012-033907 base: https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace for-next patch link: https://lore.kernel.org/r/cdaf33af866392428f1942cef4e442b1d106989c.1791745088.git.wen.yang%40linux.dev patch subject: [PATCH v7 03/14] rv: Add a KUnit test for the HA timer underflow config: x86_64-randconfig-1005-20261012 (https://download.01.org/0day-ci/archive/20261012/[email protected]/config) compiler: clang version 22.1.8 (https://github.com/llvm/llvm-project ca7933e47d3a3451d81e72ac174dcb5aa28b59d1) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261012/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All error/warnings (new ones prefixed by >>):
In file included from kernel/trace/rv/rv_monitors_test.c:71:
>> include/rv/ha_monitor.h:66:16: error: no member named 'da_mon' in 'ha_monitor'
66 | _Static_assert(offsetof(struct ha_monitor, da_mon) == 0,
| ^ ~~~~~~
include/linux/stddef.h:16:32: note: expanded from macro 'offsetof'
16 | #define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)
| ^ ~~~~~~
In file included from kernel/trace/rv/rv_monitors_test.c:71:
>> include/rv/ha_monitor.h:122:5: warning: 'HA_TIMER_HRTIMER' is not defined, evaluates to 0 [-Wundef]
122 | #if HA_TIMER_TYPE == HA_TIMER_WHEEL
| ^
kernel/trace/rv/rv_monitors_test.c:21:23: note: expanded from macro 'HA_TIMER_TYPE'
21 | #define HA_TIMER_TYPE HA_TIMER_HRTIMER
| ^
In file included from kernel/trace/rv/rv_monitors_test.c:71:
include/rv/ha_monitor.h:122:22: warning: 'HA_TIMER_WHEEL' is not defined, evaluates to 0 [-Wundef]
122 | #if HA_TIMER_TYPE == HA_TIMER_WHEEL
| ^
>> include/rv/ha_monitor.h:192:30: error: no member named 'da_mon' in 'struct ha_monitor'
192 | struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~~~~~~~~~
include/rv/ha_monitor.h:68:63: note: expanded from macro 'to_ha_monitor'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
include/linux/container_of.h:20:56: note: expanded from macro 'container_of'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
21 | __same_type(*(ptr), void), \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
22 | "pointer type mismatch in container_of()"); \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/container_of.h:8:45: note: expanded from macro 'typeof_member'
8 | #define typeof_member(T, m) typeof(((T*)0)->m)
| ^
include/linux/compiler_types.h:610:74: note: expanded from macro '__same_type'
610 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
| ^
include/linux/build_bug.h:79:50: note: expanded from macro 'static_assert'
79 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
| ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:80:56: note: expanded from macro '__static_assert'
80 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
In file included from kernel/trace/rv/rv_monitors_test.c:71:
include/rv/ha_monitor.h:192:30: error: no member named 'da_mon' in 'ha_monitor'
192 | struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~~~~~~~~~
include/rv/ha_monitor.h:68:27: note: expanded from macro 'to_ha_monitor'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ^ ~~~~~~
include/linux/container_of.h:23:27: note: expanded from macro 'container_of'
23 | (type *)((void *)(ptr) - offsetof(type, member)); })
| ^ ~~~~~~
include/linux/stddef.h:16:32: note: expanded from macro 'offsetof'
16 | #define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)
| ^ ~~~~~~
In file included from kernel/trace/rv/rv_monitors_test.c:71:
>> include/rv/ha_monitor.h:192:21: error: initializing 'struct ha_monitor *' with an expression of incompatible type 'void'
192 | struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
| ^ ~~~~~~~~~~~~~~~~~~~~~
include/rv/ha_monitor.h:208:30: error: no member named 'da_mon' in 'struct ha_monitor'
208 | struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~~~~~~~~~
include/rv/ha_monitor.h:68:63: note: expanded from macro 'to_ha_monitor'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
include/linux/container_of.h:20:56: note: expanded from macro 'container_of'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
21 | __same_type(*(ptr), void), \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
22 | "pointer type mismatch in container_of()"); \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/container_of.h:8:45: note: expanded from macro 'typeof_member'
8 | #define typeof_member(T, m) typeof(((T*)0)->m)
| ^
include/linux/compiler_types.h:610:74: note: expanded from macro '__same_type'
610 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
| ^
include/linux/build_bug.h:79:50: note: expanded from macro 'static_assert'
79 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
| ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:80:56: note: expanded from macro '__static_assert'
80 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
In file included from kernel/trace/rv/rv_monitors_test.c:71:
include/rv/ha_monitor.h:208:30: error: no member named 'da_mon' in 'ha_monitor'
208 | struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~~~~~~~~~
include/rv/ha_monitor.h:68:27: note: expanded from macro 'to_ha_monitor'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ^ ~~~~~~
include/linux/container_of.h:23:27: note: expanded from macro 'container_of'
23 | (type *)((void *)(ptr) - offsetof(type, member)); })
| ^ ~~~~~~
include/linux/stddef.h:16:32: note: expanded from macro 'offsetof'
16 | #define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)
| ^ ~~~~~~
In file included from kernel/trace/rv/rv_monitors_test.c:71:
include/rv/ha_monitor.h:208:21: error: initializing 'struct ha_monitor *' with an expression of incompatible type 'void'
208 | struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
| ^ ~~~~~~~~~~~~~~~~~~~~~
>> include/rv/ha_monitor.h:219:27: error: no member named 'env_store' in 'struct ha_monitor'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ~~~~~~ ^
>> include/rv/ha_monitor.h:219:27: error: no member named 'env_store' in 'struct ha_monitor'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ~~~~~~ ^
>> include/rv/ha_monitor.h:219:27: error: no member named 'env_store' in 'struct ha_monitor'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ~~~~~~ ^
>> include/rv/ha_monitor.h:219:27: error: no member named 'env_store' in 'struct ha_monitor'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ~~~~~~ ^
>> include/rv/ha_monitor.h:219:27: error: no member named 'env_store' in 'struct ha_monitor'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ~~~~~~ ^
>> include/rv/ha_monitor.h:219:27: error: no member named 'env_store' in 'struct ha_monitor'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ~~~~~~ ^
>> include/rv/ha_monitor.h:219:27: error: no member named 'env_store' in 'struct ha_monitor'
219 | return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;
| ~~~~~~ ^
include/rv/ha_monitor.h:286:30: error: no member named 'da_mon' in 'struct ha_monitor'
286 | struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~~~~~~~~~
include/rv/ha_monitor.h:68:63: note: expanded from macro 'to_ha_monitor'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
include/linux/container_of.h:20:56: note: expanded from macro 'container_of'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
21 | __same_type(*(ptr), void), \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
22 | "pointer type mismatch in container_of()"); \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/container_of.h:8:45: note: expanded from macro 'typeof_member'
8 | #define typeof_member(T, m) typeof(((T*)0)->m)
| ^
include/linux/compiler_types.h:610:74: note: expanded from macro '__same_type'
610 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
| ^
include/linux/build_bug.h:79:50: note: expanded from macro 'static_assert'
79 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
| ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:80:56: note: expanded from macro '__static_assert'
80 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
In file included from kernel/trace/rv/rv_monitors_test.c:71:
include/rv/ha_monitor.h:286:30: error: no member named 'da_mon' in 'ha_monitor'
286 | struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~~~~~~~~~
include/rv/ha_monitor.h:68:27: note: expanded from macro 'to_ha_monitor'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ^ ~~~~~~
include/linux/container_of.h:23:27: note: expanded from macro 'container_of'
23 | (type *)((void *)(ptr) - offsetof(type, member)); })
| ^ ~~~~~~
include/linux/stddef.h:16:32: note: expanded from macro 'offsetof'
16 | #define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)
| ^ ~~~~~~
In file included from kernel/trace/rv/rv_monitors_test.c:71:
include/rv/ha_monitor.h:286:21: error: initializing 'struct ha_monitor *' with an expression of incompatible type 'void'
286 | struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
| ^ ~~~~~~~~~~~~~~~~~~~~~ include/rv/ha_monitor.h:321:41: error: no member named 'da_mon' in 'struct ha_monitor'
321 | curr_state = smp_load_acquire(&ha_mon->da_mon.curr_state);
| ~~~~~~ ^
include/rv/ha_monitor.h:321:41: error: no member named 'da_mon' in 'struct ha_monitor'
321 | curr_state = smp_load_acquire(&ha_mon->da_mon.curr_state);
| ~~~~~~ ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
2 warnings and 20 errors generated.
vim +66 include/rv/ha_monitor.h
f5587d1b6ec938 Gabriele Monaco 2026-03-30 64
f5587d1b6ec938 Gabriele Monaco 2026-03-30 65 /* This simplifies things since da_mon and ha_mon coexist in the same union */
f5587d1b6ec938 Gabriele Monaco 2026-03-30 @66 _Static_assert(offsetof(struct ha_monitor, da_mon) == 0,
f5587d1b6ec938 Gabriele Monaco 2026-03-30 67 "da_mon must be the first element in an ha_mon!");
f5587d1b6ec938 Gabriele Monaco 2026-03-30 68 #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
f5587d1b6ec938 Gabriele Monaco 2026-03-30 69
f5587d1b6ec938 Gabriele Monaco 2026-03-30 70 #define ENV_MAX CONCATENATE(env_max_, MONITOR_NAME)
f5587d1b6ec938 Gabriele Monaco 2026-03-30 71 #define ENV_MAX_STORED CONCATENATE(env_max_stored_, MONITOR_NAME)
f5587d1b6ec938 Gabriele Monaco 2026-03-30 72 #define envs CONCATENATE(envs_, MONITOR_NAME)
f5587d1b6ec938 Gabriele Monaco 2026-03-30 73
f5587d1b6ec938 Gabriele Monaco 2026-03-30 74 /* Environment storage before being reset */
f5587d1b6ec938 Gabriele Monaco 2026-03-30 75 #define ENV_INVALID_VALUE U64_MAX
f5587d1b6ec938 Gabriele Monaco 2026-03-30 76 /* Error with no event occurs only on timeouts */
f5587d1b6ec938 Gabriele Monaco 2026-03-30 77 #define EVENT_NONE EVENT_MAX
f5587d1b6ec938 Gabriele Monaco 2026-03-30 78 #define EVENT_NONE_LBL "none"
f5587d1b6ec938 Gabriele Monaco 2026-03-30 79 #define ENV_BUFFER_SIZE 64
f5587d1b6ec938 Gabriele Monaco 2026-03-30 80
f5587d1b6ec938 Gabriele Monaco 2026-03-30 81 #ifdef CONFIG_RV_REACTORS
f5587d1b6ec938 Gabriele Monaco 2026-03-30 82
f5587d1b6ec938 Gabriele Monaco 2026-03-30 83 /*
f5587d1b6ec938 Gabriele Monaco 2026-03-30 84 * ha_react - trigger the reaction after a failed environment constraint
f5587d1b6ec938 Gabriele Monaco 2026-03-30 85 *
f5587d1b6ec938 Gabriele Monaco 2026-03-30 86 * The transition from curr_state with event is otherwise valid, but the
f5587d1b6ec938 Gabriele Monaco 2026-03-30 87 * environment constraint is false. This function can be called also with no
f5587d1b6ec938 Gabriele Monaco 2026-03-30 88 * event from a timer (state constraints only).
f5587d1b6ec938 Gabriele Monaco 2026-03-30 89 */
f5587d1b6ec938 Gabriele Monaco 2026-03-30 90 static void ha_react(enum states curr_state, enum events event, char *env)
f5587d1b6ec938 Gabriele Monaco 2026-03-30 91 {
f5587d1b6ec938 Gabriele Monaco 2026-03-30 92 rv_react(&rv_this,
f5587d1b6ec938 Gabriele Monaco 2026-03-30 93 "rv: monitor %s does not allow event %s on state %s with env %s\n",
f5587d1b6ec938 Gabriele Monaco 2026-03-30 94 __stringify(MONITOR_NAME),
f5587d1b6ec938 Gabriele Monaco 2026-03-30 95 event == EVENT_NONE ? EVENT_NONE_LBL : model_get_event_name(event),
f5587d1b6ec938 Gabriele Monaco 2026-03-30 96 model_get_state_name(curr_state), env);
f5587d1b6ec938 Gabriele Monaco 2026-03-30 97 }
f5587d1b6ec938 Gabriele Monaco 2026-03-30 98
f5587d1b6ec938 Gabriele Monaco 2026-03-30 99 #else /* CONFIG_RV_REACTOR */
f5587d1b6ec938 Gabriele Monaco 2026-03-30 100
f5587d1b6ec938 Gabriele Monaco 2026-03-30 101 static void ha_react(enum states curr_state, enum events event, char *env) { }
f5587d1b6ec938 Gabriele Monaco 2026-03-30 102 #endif
f5587d1b6ec938 Gabriele Monaco 2026-03-30 103
f5587d1b6ec938 Gabriele Monaco 2026-03-30 104 /*
f5587d1b6ec938 Gabriele Monaco 2026-03-30 105 * model_get_state_name - return the (string) name of the given state
f5587d1b6ec938 Gabriele Monaco 2026-03-30 106 */
f5587d1b6ec938 Gabriele Monaco 2026-03-30 107 static char *model_get_env_name(enum envs env)
f5587d1b6ec938 Gabriele Monaco 2026-03-30 108 {
f5587d1b6ec938 Gabriele Monaco 2026-03-30 109 if ((env < 0) || (env >= ENV_MAX))
f5587d1b6ec938 Gabriele Monaco 2026-03-30 110 return "INVALID";
f5587d1b6ec938 Gabriele Monaco 2026-03-30 111
f5587d1b6ec938 Gabriele Monaco 2026-03-30 112 return RV_AUTOMATON_NAME.env_names[env];
f5587d1b6ec938 Gabriele Monaco 2026-03-30 113 }
f5587d1b6ec938 Gabriele Monaco 2026-03-30 114
f5587d1b6ec938 Gabriele Monaco 2026-03-30 115 /*
f5587d1b6ec938 Gabriele Monaco 2026-03-30 116 * Monitors requiring a timer implementation need to request it explicitly.
f5587d1b6ec938 Gabriele Monaco 2026-03-30 117 */
f5587d1b6ec938 Gabriele Monaco 2026-03-30 118 #ifndef HA_TIMER_TYPE
f5587d1b6ec938 Gabriele Monaco 2026-03-30 119 #define HA_TIMER_TYPE HA_TIMER_NONE
f5587d1b6ec938 Gabriele Monaco 2026-03-30 120 #endif
f5587d1b6ec938 Gabriele Monaco 2026-03-30 121
f5587d1b6ec938 Gabriele Monaco 2026-03-30 @122 #if HA_TIMER_TYPE == HA_TIMER_WHEEL
f5587d1b6ec938 Gabriele Monaco 2026-03-30 123 static void ha_monitor_timer_callback(struct timer_list *timer);
f5587d1b6ec938 Gabriele Monaco 2026-03-30 124 #elif HA_TIMER_TYPE == HA_TIMER_HRTIMER
f5587d1b6ec938 Gabriele Monaco 2026-03-30 125 static enum hrtimer_restart ha_monitor_timer_callback(struct hrtimer *hrtimer);
f5587d1b6ec938 Gabriele Monaco 2026-03-30 126 #endif
f5587d1b6ec938 Gabriele Monaco 2026-03-30 127
f5587d1b6ec938 Gabriele Monaco 2026-03-30 128 /*
f5587d1b6ec938 Gabriele Monaco 2026-03-30 129 * ktime_get_ns is expensive, since we usually don't require precise accounting
f5587d1b6ec938 Gabriele Monaco 2026-03-30 130 * of changes within the same event, cache the current time at the beginning of
f5587d1b6ec938 Gabriele Monaco 2026-03-30 131 * the constraint handler and use the cache for subsequent calls.
f5587d1b6ec938 Gabriele Monaco 2026-03-30 132 * Monitors without ns clocks automatically skip this.
f5587d1b6ec938 Gabriele Monaco 2026-03-30 133 */
f5587d1b6ec938 Gabriele Monaco 2026-03-30 134 #ifdef HA_CLK_NS
f5587d1b6ec938 Gabriele Monaco 2026-03-30 135 #define ha_get_ns() ktime_get_ns()
f5587d1b6ec938 Gabriele Monaco 2026-03-30 136 #else
f5587d1b6ec938 Gabriele Monaco 2026-03-30 137 #define ha_get_ns() 0
f5587d1b6ec938 Gabriele Monaco 2026-03-30 138 #endif /* HA_CLK_NS */
f5587d1b6ec938 Gabriele Monaco 2026-03-30 139
74e17bd6fc8ed7 Gabriele Monaco 2026-06-01 140 static bool ha_mon_destroying;
74e17bd6fc8ed7 Gabriele Monaco 2026-06-01 141
700782ec8f6589 Gabriele Monaco 2026-06-01 142 static int ha_monitor_init(void)
700782ec8f6589 Gabriele Monaco 2026-06-01 143 {
700782ec8f6589 Gabriele Monaco 2026-06-01 144 int ret;
700782ec8f6589 Gabriele Monaco 2026-06-01 145
74e17bd6fc8ed7 Gabriele Monaco 2026-06-01 146 WRITE_ONCE(ha_mon_destroying, false);
700782ec8f6589 Gabriele Monaco 2026-06-01 147 ret = da_monitor_init();
700782ec8f6589 Gabriele Monaco 2026-06-01 148 if (ret == 0)
700782ec8f6589 Gabriele Monaco 2026-06-01 149 ha_monitor_enable_hook();
700782ec8f6589 Gabriele Monaco 2026-06-01 150 return ret;
700782ec8f6589 Gabriele Monaco 2026-06-01 151 }
700782ec8f6589 Gabriele Monaco 2026-06-01 152
700782ec8f6589 Gabriele Monaco 2026-06-01 153 static void ha_monitor_destroy(void)
700782ec8f6589 Gabriele Monaco 2026-06-01 154 {
74e17bd6fc8ed7 Gabriele Monaco 2026-06-01 155 WRITE_ONCE(ha_mon_destroying, true);
700782ec8f6589 Gabriele Monaco 2026-06-01 156 ha_monitor_disable_hook();
700782ec8f6589 Gabriele Monaco 2026-06-01 157 da_monitor_destroy();
700782ec8f6589 Gabriele Monaco 2026-06-01 158 }
700782ec8f6589 Gabriele Monaco 2026-06-01 159
f5587d1b6ec938 Gabriele Monaco 2026-03-30 160 /* Should be supplied by the monitor */
f5587d1b6ec938 Gabriele Monaco 2026-03-30 161 static u64 ha_get_env(struct ha_monitor *ha_mon, enum envs env, u64 time_ns);
a8db17cc2158f8 Gabriele Monaco 2026-10-12 162 static void ha_reset_env(struct ha_monitor *ha_mon, enum envs env, u64 time_ns);
f5587d1b6ec938 Gabriele Monaco 2026-03-30 163 static bool ha_verify_constraint(struct ha_monitor *ha_mon,
f5587d1b6ec938 Gabriele Monaco 2026-03-30 164 enum states curr_state,
f5587d1b6ec938 Gabriele Monaco 2026-03-30 165 enum events event,
f5587d1b6ec938 Gabriele Monaco 2026-03-30 166 enum states next_state,
f5587d1b6ec938 Gabriele Monaco 2026-03-30 167 u64 time_ns);
a8db17cc2158f8 Gabriele Monaco 2026-10-12 168 #ifdef HA_NO_RESET
a8db17cc2158f8 Gabriele Monaco 2026-10-12 169 static void ha_reset_env(struct ha_monitor *ha_mon, enum envs env, u64 time_ns) { }
a8db17cc2158f8 Gabriele Monaco 2026-10-12 170 #endif
f5587d1b6ec938 Gabriele Monaco 2026-03-30 171
f5587d1b6ec938 Gabriele Monaco 2026-03-30 172 /*
f5587d1b6ec938 Gabriele Monaco 2026-03-30 173 * ha_monitor_reset_all_stored - reset all environment variables in the monitor
f5587d1b6ec938 Gabriele Monaco 2026-03-30 174 */
f5587d1b6ec938 Gabriele Monaco 2026-03-30 175 static inline void ha_monitor_reset_all_stored(struct ha_monitor *ha_mon)
f5587d1b6ec938 Gabriele Monaco 2026-03-30 176 {
a8db17cc2158f8 Gabriele Monaco 2026-10-12 177 u64 time_ns = ha_get_ns();
a8db17cc2158f8 Gabriele Monaco 2026-10-12 178
f5587d1b6ec938 Gabriele Monaco 2026-03-30 179 for (int i = 0; i < ENV_MAX_STORED; i++)
a8db17cc2158f8 Gabriele Monaco 2026-10-12 180 ha_reset_env(ha_mon, i, time_ns);
f5587d1b6ec938 Gabriele Monaco 2026-03-30 181 }
f5587d1b6ec938 Gabriele Monaco 2026-03-30 182
f5587d1b6ec938 Gabriele Monaco 2026-03-30 183 /*
f5587d1b6ec938 Gabriele Monaco 2026-03-30 184 * ha_monitor_init_env - setup timer and reset all environment
f5587d1b6ec938 Gabriele Monaco 2026-03-30 185 *
f5587d1b6ec938 Gabriele Monaco 2026-03-30 186 * Called from a hook in the DA start functions, it supplies the da_mon
f5587d1b6ec938 Gabriele Monaco 2026-03-30 187 * corresponding to the current ha_mon.
f5587d1b6ec938 Gabriele Monaco 2026-03-30 188 * Not all hybrid automata require the timer, still set it for simplicity.
f5587d1b6ec938 Gabriele Monaco 2026-03-30 189 */
f5587d1b6ec938 Gabriele Monaco 2026-03-30 190 static inline void ha_monitor_init_env(struct da_monitor *da_mon)
f5587d1b6ec938 Gabriele Monaco 2026-03-30 191 {
f5587d1b6ec938 Gabriele Monaco 2026-03-30 @192 struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
f5587d1b6ec938 Gabriele Monaco 2026-03-30 193
f5587d1b6ec938 Gabriele Monaco 2026-03-30 194 ha_monitor_reset_all_stored(ha_mon);
f5587d1b6ec938 Gabriele Monaco 2026-03-30 195 ha_setup_timer(ha_mon);
f5587d1b6ec938 Gabriele Monaco 2026-03-30 196 }
f5587d1b6ec938 Gabriele Monaco 2026-03-30 197
f5587d1b6ec938 Gabriele Monaco 2026-03-30 198 /*
f5587d1b6ec938 Gabriele Monaco 2026-03-30 199 * ha_monitor_reset_env - stop timer and reset all environment
f5587d1b6ec938 Gabriele Monaco 2026-03-30 200 *
f5587d1b6ec938 Gabriele Monaco 2026-03-30 201 * Called from a hook in the DA reset functions, it supplies the da_mon
f5587d1b6ec938 Gabriele Monaco 2026-03-30 202 * corresponding to the current ha_mon.
f5587d1b6ec938 Gabriele Monaco 2026-03-30 203 * Not all hybrid automata require the timer, still clear it for simplicity.
713c9e1ea76238 Gabriele Monaco 2026-06-01 204 * Monitors that never started have their timer uninitialized, do not stop those.
f5587d1b6ec938 Gabriele Monaco 2026-03-30 205 */
f5587d1b6ec938 Gabriele Monaco 2026-03-30 206 static inline void ha_monitor_reset_env(struct da_monitor *da_mon)
f5587d1b6ec938 Gabriele Monaco 2026-03-30 207 {
f5587d1b6ec938 Gabriele Monaco 2026-03-30 208 struct ha_monitor *ha_mon = to_ha_monitor(da_mon);
f5587d1b6ec938 Gabriele Monaco 2026-03-30 209
f5587d1b6ec938 Gabriele Monaco 2026-03-30 210 if (likely(da_monitoring(da_mon)))
f5587d1b6ec938 Gabriele Monaco 2026-03-30 211 ha_cancel_timer(ha_mon);
f5587d1b6ec938 Gabriele Monaco 2026-03-30 212 }
f5587d1b6ec938 Gabriele Monaco 2026-03-30 213
f5587d1b6ec938 Gabriele Monaco 2026-03-30 214 /*
f5587d1b6ec938 Gabriele Monaco 2026-03-30 215 * ha_monitor_env_invalid - return true if env has not been initialised
f5587d1b6ec938 Gabriele Monaco 2026-03-30 216 */
f5587d1b6ec938 Gabriele Monaco 2026-03-30 217 static inline bool ha_monitor_env_invalid(struct ha_monitor *ha_mon, enum envs env)
f5587d1b6ec938 Gabriele Monaco 2026-03-30 218 {f5587d1b6ec938 Gabriele Monaco 2026-03-30 @219 return READ_ONCE(ha_mon->env_store[env]) == ENV_INVALID_VALUE;f5587d1b6ec938 Gabriele Monaco 2026-03-30 220 }f5587d1b6ec938 Gabriele Monaco 2026-03-30 221
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
kernel test robot
build botRe: [PATCH v7 03/14] rv: Add a KUnit test for the HA timer underflow
In reply to [email protected]kernel test robot noticed the following build warnings:
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on trace/for-next]
[also build test WARNING on shuah-kselftest/next shuah-kselftest/fixes linus/master v7.3-rc6 next-20261009] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/wen-yang-linux-dev/rv-Force-environment-reset-action-on-HA-monitor-start/20261012-033907 base: https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace for-next patch link: https://lore.kernel.org/r/cdaf33af866392428f1942cef4e442b1d106989c.1791745088.git.wen.yang%40linux.dev patch subject: [PATCH v7 03/14] rv: Add a KUnit test for the HA timer underflow config: x86_64-rhel-9.4-kunit (https://download.01.org/0day-ci/archive/20261012/[email protected]/config) compiler: gcc-14 (Debian 14.2.0-19) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261012/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
| ^~~~~~~~
include/rv/ha_monitor.h:430:37: note: in expansion of macro 'container_of'
430 | struct ha_monitor *ha_mon = container_of(timer, struct ha_monitor, timer);
| ^~~~~~~~~~~~
include/rv/ha_monitor.h: In function 'ha_setup_timer':
include/rv/ha_monitor.h:440:28: error: 'struct ha_monitor' has no member named 'timer'
440 | timer_setup(&ha_mon->timer, ha_monitor_timer_callback, mode);
| ^~
include/linux/timer.h:105:25: note: in definition of macro '__timer_init'
105 | timer_init_key((_timer), (_fn), (_flags), NULL, NULL)
| ^~~~~~
include/rv/ha_monitor.h:440:9: note: in expansion of macro 'timer_setup'
440 | timer_setup(&ha_mon->timer, ha_monitor_timer_callback, mode);
| ^~~~~~~~~~~
include/rv/ha_monitor.h: In function 'ha_start_timer_jiffy':
include/rv/ha_monitor.h:447:26: error: 'struct ha_monitor' has no member named 'timer'
447 | mod_timer(&ha_mon->timer,
| ^~
include/rv/ha_monitor.h: In function 'ha_cancel_timer':
include/rv/ha_monitor.h:468:36: error: 'struct ha_monitor' has no member named 'timer'
468 | return timer_delete(&ha_mon->timer);
| ^~
include/rv/ha_monitor.h: In function 'ha_cancel_timer_sync':
include/rv/ha_monitor.h:472:34: error: 'struct ha_monitor' has no member named 'timer'
472 | timer_delete_sync(&ha_mon->timer);
| ^~
kernel/trace/rv/rv_monitors_test.c: In function 'rv_test_rvtest_underflow':
include/linux/container_of.h:8:47: error: 'struct ha_monitor' has no member named 'da_mon'
8 | #define typeof_member(T, m) typeof(((T*)0)->m)
| ^~
include/linux/build_bug.h:80:56: note: in definition of macro '__static_assert'
80 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~
include/linux/container_of.h:20:43: note: in expansion of macro 'typeof_member'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~~~
include/rv/ha_monitor.h:68:27: note: in expansion of macro 'container_of'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ^~~~~~~~~~~~
kernel/trace/rv/rv_monitors_test.c:248:18: note: in expansion of macro 'to_ha_monitor'
248 | ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~
include/linux/compiler_types.h:610:27: error: expression in static assertion is not an integer
610 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:80:56: note: in definition of macro '__static_assert'
80 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~
include/rv/ha_monitor.h:68:27: note: in expansion of macro 'container_of'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ^~~~~~~~~~~~
kernel/trace/rv/rv_monitors_test.c:248:18: note: in expansion of macro 'to_ha_monitor'
248 | ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~
include/linux/stddef.h:16:33: error: 'struct ha_monitor' has no member named 'da_mon'
16 | #define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)
| ^~~~~~~~~~~~~~~~~~
include/linux/container_of.h:23:34: note: in expansion of macro 'offsetof'
23 | (type *)((void *)(ptr) - offsetof(type, member)); })
| ^~~~~~~~
include/rv/ha_monitor.h:68:27: note: in expansion of macro 'container_of'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ^~~~~~~~~~~~
kernel/trace/rv/rv_monitors_test.c:248:18: note: in expansion of macro 'to_ha_monitor'
248 | ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~
kernel/trace/rv/rv_monitors_test.c:259:33: error: 'struct ha_monitor' has no member named 'da_mon'
259 | da_monitor_reset(&ha_mon->da_mon);
| ^~
kernel/trace/rv/rv_monitors_test.c:264:67: error: 'struct ha_monitor' has no member named 'hrtimer'
264 | ktime_before(hrtimer_get_expires(&ha_mon->hrtimer),
| ^~
include/linux/compiler.h:76:45: note: in definition of macro 'likely'
76 | # define likely(x) __builtin_expect(!!(x), 1)
| ^
include/kunit/test.h:802:9: note: in expansion of macro 'KUNIT_UNARY_ASSERTION'
802 | KUNIT_UNARY_ASSERTION(test, \
| ^~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:998:9: note: in expansion of macro 'KUNIT_TRUE_MSG_ASSERTION'
998 | KUNIT_TRUE_MSG_ASSERTION(test, \
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:995:9: note: in expansion of macro 'KUNIT_EXPECT_TRUE_MSG'
995 | KUNIT_EXPECT_TRUE_MSG(test, condition, NULL)
| ^~~~~~~~~~~~~~~~~~~~~
kernel/trace/rv/rv_monitors_test.c:263:9: note: in expansion of macro 'KUNIT_EXPECT_TRUE'
263 | KUNIT_EXPECT_TRUE(test,
| ^~~~~~~~~~~~~~~~~
include/rv/ha_monitor.h: In function 'ha_cancel_timer':
>> include/rv/ha_monitor.h:469:1: warning: control reaches end of non-void function [-Wreturn-type]
469 | }
| ^
include/rv/ha_monitor.h: In function 'ha_get_clk_ns':
include/rv/ha_monitor.h:346:1: warning: control reaches end of non-void function [-Wreturn-type]
346 | }
| ^
include/rv/ha_monitor.h: In function 'ha_check_invariant_ns':
include/rv/ha_monitor.h:355:1: warning: control reaches end of non-void function [-Wreturn-type]
355 | }
| ^
include/rv/ha_monitor.h: In function 'ha_monitor_env_invalid':
include/rv/ha_monitor.h:220:1: warning: control reaches end of non-void function [-Wreturn-type]
220 | }
| ^
--
| ^~~~~~~~
include/rv/ha_monitor.h:430:37: note: in expansion of macro 'container_of'
430 | struct ha_monitor *ha_mon = container_of(timer, struct ha_monitor, timer);
| ^~~~~~~~~~~~
include/rv/ha_monitor.h: In function 'ha_setup_timer':
include/rv/ha_monitor.h:440:28: error: 'struct ha_monitor' has no member named 'timer'
440 | timer_setup(&ha_mon->timer, ha_monitor_timer_callback, mode);
| ^~
include/linux/timer.h:105:25: note: in definition of macro '__timer_init'
105 | timer_init_key((_timer), (_fn), (_flags), NULL, NULL)
| ^~~~~~
include/rv/ha_monitor.h:440:9: note: in expansion of macro 'timer_setup'
440 | timer_setup(&ha_mon->timer, ha_monitor_timer_callback, mode);
| ^~~~~~~~~~~
include/rv/ha_monitor.h: In function 'ha_start_timer_jiffy':
include/rv/ha_monitor.h:447:26: error: 'struct ha_monitor' has no member named 'timer'
447 | mod_timer(&ha_mon->timer,
| ^~
include/rv/ha_monitor.h: In function 'ha_cancel_timer':
include/rv/ha_monitor.h:468:36: error: 'struct ha_monitor' has no member named 'timer'
468 | return timer_delete(&ha_mon->timer);
| ^~
include/rv/ha_monitor.h: In function 'ha_cancel_timer_sync':
include/rv/ha_monitor.h:472:34: error: 'struct ha_monitor' has no member named 'timer'
472 | timer_delete_sync(&ha_mon->timer);
| ^~
rv_monitors_test.c: In function 'rv_test_rvtest_underflow':
include/linux/container_of.h:8:47: error: 'struct ha_monitor' has no member named 'da_mon'
8 | #define typeof_member(T, m) typeof(((T*)0)->m)
| ^~
include/linux/build_bug.h:80:56: note: in definition of macro '__static_assert'
80 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~
include/linux/container_of.h:20:43: note: in expansion of macro 'typeof_member'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~~~
include/rv/ha_monitor.h:68:27: note: in expansion of macro 'container_of'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ^~~~~~~~~~~~
rv_monitors_test.c:248:18: note: in expansion of macro 'to_ha_monitor'
248 | ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~
include/linux/compiler_types.h:610:27: error: expression in static assertion is not an integer
610 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:80:56: note: in definition of macro '__static_assert'
80 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
20 | static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
| ^~~~~~~~~~~
include/rv/ha_monitor.h:68:27: note: in expansion of macro 'container_of'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ^~~~~~~~~~~~
rv_monitors_test.c:248:18: note: in expansion of macro 'to_ha_monitor'
248 | ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~
include/linux/stddef.h:16:33: error: 'struct ha_monitor' has no member named 'da_mon'
16 | #define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)
| ^~~~~~~~~~~~~~~~~~
include/linux/container_of.h:23:34: note: in expansion of macro 'offsetof'
23 | (type *)((void *)(ptr) - offsetof(type, member)); })
| ^~~~~~~~
include/rv/ha_monitor.h:68:27: note: in expansion of macro 'container_of'
68 | #define to_ha_monitor(da) container_of(da, struct ha_monitor, da_mon)
| ^~~~~~~~~~~~
rv_monitors_test.c:248:18: note: in expansion of macro 'to_ha_monitor'
248 | ha_mon = to_ha_monitor(da_mon);
| ^~~~~~~~~~~~~
rv_monitors_test.c:259:33: error: 'struct ha_monitor' has no member named 'da_mon'
259 | da_monitor_reset(&ha_mon->da_mon);
| ^~
rv_monitors_test.c:264:67: error: 'struct ha_monitor' has no member named 'hrtimer'
264 | ktime_before(hrtimer_get_expires(&ha_mon->hrtimer),
| ^~
include/linux/compiler.h:76:45: note: in definition of macro 'likely'
76 | # define likely(x) __builtin_expect(!!(x), 1)
| ^
include/kunit/test.h:802:9: note: in expansion of macro 'KUNIT_UNARY_ASSERTION'
802 | KUNIT_UNARY_ASSERTION(test, \
| ^~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:998:9: note: in expansion of macro 'KUNIT_TRUE_MSG_ASSERTION'
998 | KUNIT_TRUE_MSG_ASSERTION(test, \
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:995:9: note: in expansion of macro 'KUNIT_EXPECT_TRUE_MSG'
995 | KUNIT_EXPECT_TRUE_MSG(test, condition, NULL)
| ^~~~~~~~~~~~~~~~~~~~~
rv_monitors_test.c:263:9: note: in expansion of macro 'KUNIT_EXPECT_TRUE'
263 | KUNIT_EXPECT_TRUE(test,
| ^~~~~~~~~~~~~~~~~
include/rv/ha_monitor.h: In function 'ha_cancel_timer':
>> include/rv/ha_monitor.h:469:1: warning: control reaches end of non-void function [-Wreturn-type]
469 | }
| ^
include/rv/ha_monitor.h: In function 'ha_get_clk_ns':
include/rv/ha_monitor.h:346:1: warning: control reaches end of non-void function [-Wreturn-type]
346 | }
| ^
include/rv/ha_monitor.h: In function 'ha_check_invariant_ns':
include/rv/ha_monitor.h:355:1: warning: control reaches end of non-void function [-Wreturn-type]
355 | }
| ^
include/rv/ha_monitor.h: In function 'ha_monitor_env_invalid':
include/rv/ha_monitor.h:220:1: warning: control reaches end of non-void function [-Wreturn-type]
220 | }
| ^vim +469 include/rv/ha_monitor.h
d68b31ce9bd4dda Wen Yang 2026-10-12 417
f5587d1b6ec938a Gabriele Monaco 2026-03-30 418 #if HA_TIMER_TYPE == HA_TIMER_WHEEL
f5587d1b6ec938a Gabriele Monaco 2026-03-30 419 /*
f5587d1b6ec938a Gabriele Monaco 2026-03-30 420 * Helper functions to handle the monitor timer.
f5587d1b6ec938a Gabriele Monaco 2026-03-30 421 * Not all monitors require a timer, in such case the timer will be set up but
f5587d1b6ec938a Gabriele Monaco 2026-03-30 422 * never armed.
f5587d1b6ec938a Gabriele Monaco 2026-03-30 423 * Timers start since the last reset of the supplied env or from now if env is
f5587d1b6ec938a Gabriele Monaco 2026-03-30 424 * not an environment variable. If env was not initialised no timer starts.
f5587d1b6ec938a Gabriele Monaco 2026-03-30 425 * Timers can expire on any CPU unless the monitor is per-cpu,
f5587d1b6ec938a Gabriele Monaco 2026-03-30 426 * where we assume every event occurs on the local CPU.
f5587d1b6ec938a Gabriele Monaco 2026-03-30 427 */
f5587d1b6ec938a Gabriele Monaco 2026-03-30 428 static void ha_monitor_timer_callback(struct timer_list *timer)
f5587d1b6ec938a Gabriele Monaco 2026-03-30 429 {
f5587d1b6ec938a Gabriele Monaco 2026-03-30 430 struct ha_monitor *ha_mon = container_of(timer, struct ha_monitor, timer);
f5587d1b6ec938a Gabriele Monaco 2026-03-30 431
f5587d1b6ec938a Gabriele Monaco 2026-03-30 432 __ha_monitor_timer_callback(ha_mon);
f5587d1b6ec938a Gabriele Monaco 2026-03-30 433 }
f5587d1b6ec938a Gabriele Monaco 2026-03-30 434 static inline void ha_setup_timer(struct ha_monitor *ha_mon)
f5587d1b6ec938a Gabriele Monaco 2026-03-30 435 {
f5587d1b6ec938a Gabriele Monaco 2026-03-30 436 int mode = 0;
f5587d1b6ec938a Gabriele Monaco 2026-03-30 437
f5587d1b6ec938a Gabriele Monaco 2026-03-30 438 if (RV_MON_TYPE == RV_MON_PER_CPU)
f5587d1b6ec938a Gabriele Monaco 2026-03-30 439 mode |= TIMER_PINNED;
f5587d1b6ec938a Gabriele Monaco 2026-03-30 440 timer_setup(&ha_mon->timer, ha_monitor_timer_callback, mode);
f5587d1b6ec938a Gabriele Monaco 2026-03-30 441 }
f5587d1b6ec938a Gabriele Monaco 2026-03-30 442 static inline void ha_start_timer_jiffy(struct ha_monitor *ha_mon, enum envs env,
f5587d1b6ec938a Gabriele Monaco 2026-03-30 443 u64 expire, u64 time_ns)
f5587d1b6ec938a Gabriele Monaco 2026-03-30 444 {
ab2900ae252b2a3 Nam Cao 2026-06-19 445 u64 passed = ha_invariant_passed_jiffy(ha_mon, env, time_ns);
f5587d1b6ec938a Gabriele Monaco 2026-03-30 446
d68b31ce9bd4dda Wen Yang 2026-10-12 447 mod_timer(&ha_mon->timer,
d68b31ce9bd4dda Wen Yang 2026-10-12 448 get_jiffies_64() + ha_time_left(passed, expire));
f5587d1b6ec938a Gabriele Monaco 2026-03-30 449 }
f5587d1b6ec938a Gabriele Monaco 2026-03-30 450 static inline void ha_start_timer_ns(struct ha_monitor *ha_mon, enum envs env,
f5587d1b6ec938a Gabriele Monaco 2026-03-30 451 u64 expire, u64 time_ns)
f5587d1b6ec938a Gabriele Monaco 2026-03-30 452 {
ab2900ae252b2a3 Nam Cao 2026-06-19 453 u64 passed = ha_invariant_passed_ns(ha_mon, env, time_ns);
d68b31ce9bd4dda Wen Yang 2026-10-12 454 u64 delay = ha_time_left(passed, expire);
f5587d1b6ec938a Gabriele Monaco 2026-03-30 455
f5587d1b6ec938a Gabriele Monaco 2026-03-30 456 ha_start_timer_jiffy(ha_mon, ENV_MAX_STORED,
d68b31ce9bd4dda Wen Yang 2026-10-12 457 nsecs_to_jiffies(delay + TICK_NSEC - 1), time_ns);
f5587d1b6ec938a Gabriele Monaco 2026-03-30 458 }
f5587d1b6ec938a Gabriele Monaco 2026-03-30 459 /*
f5587d1b6ec938a Gabriele Monaco 2026-03-30 460 * ha_cancel_timer - Cancel the timer
f5587d1b6ec938a Gabriele Monaco 2026-03-30 461 *
f5587d1b6ec938a Gabriele Monaco 2026-03-30 462 * Returns:
f5587d1b6ec938a Gabriele Monaco 2026-03-30 463 * * 1 when the timer was active
f5587d1b6ec938a Gabriele Monaco 2026-03-30 464 * * 0 when the timer was not active or running a callback
f5587d1b6ec938a Gabriele Monaco 2026-03-30 465 */
f5587d1b6ec938a Gabriele Monaco 2026-03-30 466 static inline bool ha_cancel_timer(struct ha_monitor *ha_mon)
f5587d1b6ec938a Gabriele Monaco 2026-03-30 467 {
f5587d1b6ec938a Gabriele Monaco 2026-03-30 468 return timer_delete(&ha_mon->timer);
f5587d1b6ec938a Gabriele Monaco 2026-03-30 @469 }
700782ec8f6589c Gabriele Monaco 2026-06-01 470 static inline void ha_cancel_timer_sync(struct ha_monitor *ha_mon)
700782ec8f6589c Gabriele Monaco 2026-06-01 471 {
700782ec8f6589c Gabriele Monaco 2026-06-01 472 timer_delete_sync(&ha_mon->timer);
700782ec8f6589c Gabriele Monaco 2026-06-01 473 }
f5587d1b6ec938a Gabriele Monaco 2026-03-30 474 #elif HA_TIMER_TYPE == HA_TIMER_HRTIMER
f5587d1b6ec938a Gabriele Monaco 2026-03-30 475 /*
f5587d1b6ec938a Gabriele Monaco 2026-03-30 476 * Helper functions to handle the monitor timer.
f5587d1b6ec938a Gabriele Monaco 2026-03-30 477 * Not all monitors require a timer, in such case the timer will be set up but
f5587d1b6ec938a Gabriele Monaco 2026-03-30 478 * never armed.
f5587d1b6ec938a Gabriele Monaco 2026-03-30 479 * Timers start since the last reset of the supplied env or from now if env is
f5587d1b6ec938a Gabriele Monaco 2026-03-30 480 * not an environment variable. If env was not initialised no timer starts.
f5587d1b6ec938a Gabriele Monaco 2026-03-30 481 * Timers can expire on any CPU unless the monitor is per-cpu,
f5587d1b6ec938a Gabriele Monaco 2026-03-30 482 * where we assume every event occurs on the local CPU.
f5587d1b6ec938a Gabriele Monaco 2026-03-30 483 */
f5587d1b6ec938a Gabriele Monaco 2026-03-30 484 static enum hrtimer_restart ha_monitor_timer_callback(struct hrtimer *hrtimer)
f5587d1b6ec938a Gabriele Monaco 2026-03-30 485 {
f5587d1b6ec938a Gabriele Monaco 2026-03-30 486 struct ha_monitor *ha_mon = container_of(hrtimer, struct ha_monitor, hrtimer);
f5587d1b6ec938a Gabriele Monaco 2026-03-30 487
f5587d1b6ec938a Gabriele Monaco 2026-03-30 488 __ha_monitor_timer_callback(ha_mon);
f5587d1b6ec938a Gabriele Monaco 2026-03-30 489 return HRTIMER_NORESTART;
f5587d1b6ec938a Gabriele Monaco 2026-03-30 490 }
f5587d1b6ec938a Gabriele Monaco 2026-03-30 491 static inline void ha_setup_timer(struct ha_monitor *ha_mon)
f5587d1b6ec938a Gabriele Monaco 2026-03-30 492 {
f5587d1b6ec938a Gabriele Monaco 2026-03-30 493 hrtimer_setup(&ha_mon->hrtimer, ha_monitor_timer_callback,
f5587d1b6ec938a Gabriele Monaco 2026-03-30 494 CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
f5587d1b6ec938a Gabriele Monaco 2026-03-30 495 }
f5587d1b6ec938a Gabriele Monaco 2026-03-30 496 static inline void ha_start_timer_ns(struct ha_monitor *ha_mon, enum envs env,
f5587d1b6ec938a Gabriele Monaco 2026-03-30 497 u64 expire, u64 time_ns)
f5587d1b6ec938a Gabriele Monaco 2026-03-30 498 {
f5587d1b6ec938a Gabriele Monaco 2026-03-30 499 int mode = HRTIMER_MODE_REL_HARD;
ab2900ae252b2a3 Nam Cao 2026-06-19 500 u64 passed = ha_invariant_passed_ns(ha_mon, env, time_ns);
f5587d1b6ec938a Gabriele Monaco 2026-03-30 501
f5587d1b6ec938a Gabriele Monaco 2026-03-30 502 if (RV_MON_TYPE == RV_MON_PER_CPU)
f5587d1b6ec938a Gabriele Monaco 2026-03-30 503 mode |= HRTIMER_MODE_PINNED;
d68b31ce9bd4dda Wen Yang 2026-10-12 504 hrtimer_start(&ha_mon->hrtimer,
d68b31ce9bd4dda Wen Yang 2026-10-12 505 ns_to_ktime(ha_time_left(passed, expire)), mode);
f5587d1b6ec938a Gabriele Monaco 2026-03-30 506 }
f5587d1b6ec938a Gabriele Monaco 2026-03-30 507 static inline void ha_start_timer_jiffy(struct ha_monitor *ha_mon, enum envs env,
f5587d1b6ec938a Gabriele Monaco 2026-03-30 508 u64 expire, u64 time_ns)
f5587d1b6ec938a Gabriele Monaco 2026-03-30 509 {
ab2900ae252b2a3 Nam Cao 2026-06-19 510 u64 passed = ha_invariant_passed_jiffy(ha_mon, env, time_ns);
f5587d1b6ec938a Gabriele Monaco 2026-03-30 511
f5587d1b6ec938a Gabriele Monaco 2026-03-30 512 ha_start_timer_ns(ha_mon, ENV_MAX_STORED,
d68b31ce9bd4dda Wen Yang 2026-10-12 513 jiffies_to_nsecs(ha_time_left(passed, expire)),
d68b31ce9bd4dda Wen Yang 2026-10-12 514 time_ns);
f5587d1b6ec938a Gabriele Monaco 2026-03-30 515 }
f5587d1b6ec938a Gabriele Monaco 2026-03-30 516 /*
f5587d1b6ec938a Gabriele Monaco 2026-03-30 517 * ha_cancel_timer - Cancel the timer
f5587d1b6ec938a Gabriele Monaco 2026-03-30 518 *
f5587d1b6ec938a Gabriele Monaco 2026-03-30 519 * Returns:
f5587d1b6ec938a Gabriele Monaco 2026-03-30 520 * * 1 when the timer was active
f5587d1b6ec938a Gabriele Monaco 2026-03-30 521 * * 0 when the timer was not active or running a callback
f5587d1b6ec938a Gabriele Monaco 2026-03-30 522 */
f5587d1b6ec938a Gabriele Monaco 2026-03-30 523 static inline bool ha_cancel_timer(struct ha_monitor *ha_mon)
f5587d1b6ec938a Gabriele Monaco 2026-03-30 524 {
f5587d1b6ec938a Gabriele Monaco 2026-03-30 525 return hrtimer_try_to_cancel(&ha_mon->hrtimer) == 1;
f5587d1b6ec938a Gabriele Monaco 2026-03-30 526 }
700782ec8f6589c Gabriele Monaco 2026-06-01 527 static inline void ha_cancel_timer_sync(struct ha_monitor *ha_mon)
700782ec8f6589c Gabriele Monaco 2026-06-01 528 {
700782ec8f6589c Gabriele Monaco 2026-06-01 529 hrtimer_cancel(&ha_mon->hrtimer);
700782ec8f6589c Gabriele Monaco 2026-06-01 530 }
f5587d1b6ec938a Gabriele Monaco 2026-03-30 531 #else /* HA_TIMER_NONE */
f5587d1b6ec938a Gabriele Monaco 2026-03-30 532 /*
f5587d1b6ec938a Gabriele Monaco 2026-03-30 533 * Start function is intentionally not defined, monitors using timers must
f5587d1b6ec938a Gabriele Monaco 2026-03-30 534 * set HA_TIMER_TYPE to either HA_TIMER_WHEEL or HA_TIMER_HRTIMER.
f5587d1b6ec938a Gabriele Monaco 2026-03-30 535 */
f5587d1b6ec938a Gabriele Monaco 2026-03-30 536 static inline void ha_setup_timer(struct ha_monitor *ha_mon) { }
f5587d1b6ec938a Gabriele Monaco 2026-03-30 537 static inline bool ha_cancel_timer(struct ha_monitor *ha_mon)
f5587d1b6ec938a Gabriele Monaco 2026-03-30 538 {
f5587d1b6ec938a Gabriele Monaco 2026-03-30 539 return false;
f5587d1b6ec938a Gabriele Monaco 2026-03-30 540 }
700782ec8f6589c Gabriele Monaco 2026-06-01 541 static inline void ha_cancel_timer_sync(struct ha_monitor *ha_mon) { }
f5587d1b6ec938a Gabriele Monaco 2026-03-30 542 #endif
f5587d1b6ec938a Gabriele Monaco 2026-03-30 543 --
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Gabriele Monaco
maintainerQuoted 6 lines · On Mon, 2026-10-12 at 03:39 +0800, [email protected] wrote:
From: Wen Yang <[email protected]>
This series introduces tlob (task latency over budget), a per-task hybrid automaton RV monitor that measures elapsed wall-clock time across a user-delimited code section and emits an error when the elapsed time exceeds a configurable budget.
Hi Wen,
there's been a conversation at the LPC conference about completely migrating RV to BPF.
While we aren't sure of its feasibility yet, and it will take time, it's something to keep in mind for new contributions.
First of all, would dropping RV in-kernel support negatively affect your current workflow?
About this series, completely converting it to BPF isn't trivial, I can keep on reviewing it and we could upstream it in-kernel, then convert it later (together with the work with other HA).
But I'd prioritise not affecting the in-kernel implementation much to keep it on par with the BPF one.
I'd first like to focus on what should the rv userspace tool support for a monitor like yours to be useful. You'd have all reporting as stdout (or any BPF reactor you'd want to write), then we should probably support some way to set the uprobe location and threshold before loading.
Would you really need dynamic uprobe addition?
What do you think?
Thanks,
Gabriele
Quoted 10 lines · Gabriele Monaco
Quoted 6 lines · On Mon, 2026-10-12 at 03:39 +0800, [email protected] wrote:
From: Wen Yang <[email protected]>
This series introduces tlob (task latency over budget), a per-task hybrid automaton RV monitor that measures elapsed wall-clock time across a user-delimited code section and emits an error when the elapsed time exceeds a configurable budget.
Hi Wen,
Hi Gabriele,
Thanks for the context.
Quoted 5 lines
there's been a conversation at the LPC conference about completely migrating RV to BPF.
While we aren't sure of its feasibility yet, and it will take time, it's something to keep in mind for new contributions.
Understood, and I have been following that direction. I spent the last few days reading the v2 "rv: Add support for BPF monitors" series in detail, so the comparison below is against that code rather than the general idea.
Quoted 3 lines
First of all, would dropping RV in-kernel support negatively affect your current workflow?
Yes, it would. Some concrete points, all from the current tree.
(a) Event ABI. event_da_monitor / error_env_* in kernel/trace/rv/rv_trace.h are plain tracepoints, so trace-cmd, perf and anything else reading tracefs consume them today with no extra userspace. The BPF series delivers events through a ring buffer read by the rv tool, so the two backends are not interchangeable for existing consumers.
(b) Build and deployment. On the images we target, BPF is often not compiled in or not permitted:
CONFIG_BPF_SYSCALL default n
CONFIG_BPF_EVENTS needs BPF_SYSCALL
CONFIG_DEBUG_INFO_BTF needed for CO-RE; depends on
!DEBUG_INFO_REDUCED and pahole
LOCKDOWN_BPF_READ_KERNEL disables the probe_read helpers
Those are product constraints, not local build issues. tracefs/RV works in all of these configurations.
There is also a size cost: BPF implies CONFIG_DEBUG_INFO_BTF, and .BTF is resident in RO_DATA and kept in the boot image - about 5 MB uncompressed on a full kernel, 1.2-1.6 MB compressed, plus roughly 25% per module with DEBUG_INFO_BTF_MODULES. On a 16-32 MB kernel partition that matters.
(c) Timing semantics. tlob arms a per-task hrtimer with HRTIMER_MODE_REL_HARD. bpf_timer is fixed to HRTIMER_MODE_REL_SOFT and the verifier states "bpf_timer callbacks are never sleepable" . On PREEMPT_RT the callback runs in a
threaded
softirq, so the expiry time carries scheduler latency. For a budget monitor the expiry time is the output, so this is a semantic change, not just overhead.
What a BPF tlob would look like once those exist:
- model: still tlob.dot, with rvgen -b extended to the hybrid backend;
- state: one record per task in BPF_MAP_TYPE_TASK_STORAGE (or the per-object hash map the series has today), holding the state, the binding id and the budget;
- window: two uprobe programs attached to PATH:START and PATH:STOP, with the binding id and budget passed through the attach cookie or a small config map populated before load;
- expiry: a bpf_timer in the same record, armed on state transitions and cancelled on STOP, submitting an error entry to the monitor's ringbuf;
- output: the rv tool reads the ringbuf and prints it, and the reactor is linked in with bpf_linker as v2 already does.
The DA engine and the ringbuf/reactor plumbing would carry over unchanged. The three missing pieces are the uprobe attach/configuration path, the timer and env handling in da_monitor_bpf.h, and the hybrid backend in rvgen.
None of this argues against migrating. It argues that the BPF backend has to reach parity on (a)-(c) before the in-kernel path can be removed.
Quoted 3 lines
About this series, completely converting it to BPF isn't trivial, I can keep on reviewing it and we could upstream it in-kernel, then convert it later (together with the work with other HA).
That is the order I would like to take. The series is already split so that a later conversion only replaces the tlob-specific part:
1-5 framework fixes, independent of tlob and needed regardless:
- HA environment reset on monitor start (Patch 1)
- HA timer remainder / invariant underflow (Patch 2)
- HA timer still queued on storage freed via RCU (Patch 4) 3 and 5 are their KUnit coverage.
6-8 small extension points tlob needs, but generic: overridable
reset hook / EVENT_NONE_LBL, the per-object allocation strategy, and da_extra_cleanup.
9-13 uprobe wrapper and tlob itself (DOT model, monitor, KUnit,
verification selftests).
14 ftracetest: find test.d/functions from a nested test
directory. Generic, not tlob-specific.
Could we merge the whole series now rather than staging it:
- 1-5 fix live bugs and should not wait for an unrelated design
decision;
- the BPF work needs the model in tlob.dot and the tracepoint fields in rv_trace.h as a concrete reference now, rather than being designed twice;
- 9-13 are self-contained - one monitor and its tests, plus no shared framework beyond the small hooks in 6-8 - so accepting them does not commit the framework to anything that would make the later conversion harder.
When the BPF backend reaches parity on (a)-(c), 9-13 are replaced without touching 1-8 or 14.
Quoted 3 lines
But I'd prioritise not affecting the in-kernel implementation much to keep it on par with the BPF one.
Agreed, and that is why 6-8 are deliberately small and generic, and why the monitor keeps the same model/event contract as the BPF path instead of growing in-kernel-only semantics.
Quoted 4 lines
I'd first like to focus on what should the rv userspace tool support for a monitor like yours to be useful. You'd have all reporting as stdout (or any BPF reactor you'd want to write), then we should probably support some way to set the uprobe location and threshold before loading.
Reporting already works: rv mon -t tlob creates a trace instance and prints event_tlob / error_env_tlob through ikm_event_handler() and ikm_error_handler(), so a BPF backend could feed the same output.
The missing piece is pre-enable configuration. Proposal:
rv mon tlob --uprobe PATH:START:STOP[:threshold=NS] (repeatable)
rv mon tlob --list-uprobes
rv mon tlob --budget NS default for bindings without one
rv mon tlob --summary per-pid counts instead of transitions
The tool would write the monitor file before enabling. tracefs stays the low-level path for scripts and tests, as uprobe_events coexists with perf.
Quoted 2 lines
Would you really need dynamic uprobe addition?
Minimum: bindings configured before enable. That covers the normal case, including restarting the target, because a binding is per path+offset, not per pid.
Live add/remove is what makes the code large: quiesce/reap of in-flight windows, the TLOB_RETIRED claim, concurrent add/remove versus handlers, and shadowed-binding reporting when two ranges match one task. If you want the smaller interface, I will require configuration while the monitor is disabled and keep only the teardown needed for disable and unload with live tasks; uprobe_unbind_live, uprobe_disable_live and uprobe_multi_instance then move to the tool's restart flow.
Quoted 2 lines
What do you think?
If you're OK with it, I'd suggest merging the series in-kernel now, with 1-8 and 14 kept as the shared infrastructure and 9-13 acting as the reference implementation the BPF backend is measured against. I'd be glad to help with the uprobe and timed-HA pieces on the BPF side once they're on the table.
Thanks,
Wen
Steven Rostedt
maintainerQuoted 5 lines · On Tue, 20 Oct 2026 03:18:48 +0800 Wen Yang <[email protected]> wrote:
If you're OK with it, I'd suggest merging the series in-kernel now, with 1-8 and 14 kept as the shared infrastructure and 9-13 acting as the reference implementation the BPF backend is measured against. I'd be glad to help with the uprobe and timed-HA pieces on the BPF side once they're on the table.
Well not "now" as the merge window has already opened, but perhaps for the next window.
-- Steve
Gabriele Monaco
maintainerQuoted 9 lines · Wen Yang
Quoted 8 lines
there's been a conversation at the LPC conference about completely migrating RV
to BPF.
While we aren't sure of its feasibility yet, and it will take time, it's something to keep in mind for new contributions.
Quoted 4 lines
Understood, and I have been following that direction. I spent the last few days reading the v2 "rv: Add support for BPF monitors" series in detail, so the comparison below is against that code rather than the general idea.
Great, I'm going to review your series when I get some time.
Quoted 11 lines
Quoted 4 lines
First of all, would dropping RV in-kernel support negatively affect your current
workflow?Yes, it would. Some concrete points, all from the current tree.
(a) Event ABI.event_da_monitor/ error_env_* inkernel/trace/rv/rv_trace.hare plain tracepoints, so trace-cmd, perf and anything else reading tracefs consume them today with no extra userspace. The BPF series delivers events through a ring buffer read by the rv tool, so the two backends are not interchangeable for existing consumers.
Good point, you might have noticed a similar discussion is happening also on the RV-BPF thread. We cannot have the absolute same ABI in BPF, but we probably need tracepoints or anything that can easily interact with other tracing tools.
Quoted 10 lines
(b) Build and deployment. On the images we target, BPF is often not compiled in or not permitted:
CONFIG_BPF_SYSCALL default n CONFIG_BPF_EVENTS needsBPF_SYSCALLCONFIG_DEBUG_INFO_BTF needed for CO-RE; depends on !DEBUG_INFO_REDUCED and pahole LOCKDOWN_BPF_READ_KERNEL disables theprobe_readhelpers Those are product constraints, not local build issues. tracefs/RV works in all of these configurations.
Quoted 6 lines
There is also a size cost: BPF implies
CONFIG_DEBUG_INFO_BTF, and .BTF is resident inRO_DATAand kept in the boot image - about 5 MB uncompressed on a full kernel, 1.2-1.6 MB compressed, plus roughly 25% per module with DEBUG_INFO_BTF_MODULES. On a 16-32 MB kernel partition that matters.
I've vaguely heard automotive folks saying they don't want to build BPF support several times. Finally I get a proper technical reason!
Now I think this isn't easy to sort out, if that's a hard requirement, we are probably going to need to keep in-kernel monitors around.
Quoted 6 lines
(c) Timing semantics. tlob arms a per-task hrtimer with HRTIMER_MODE_REL_HARD.
bpf_timeris fixed toHRTIMER_MODE_REL_SOFTand the verifier states "bpf_timercallbacks are never sleepable" . OnPREEMPT_RTthe callback runs in a threaded
softirq, so the expiry time carries scheduler latency.
I haven't looked into this yet, but from what you say, the bpf_timers code doesn't really support PREEMPT_RT.
Quoted 2 lines
For a budget monitor the expiry time is the output, so this is a semantic change, not just overhead.
Well, the expiry time you get from an hrtimer is what you configured + some system jitter. The jitter here is just going to be less predictable but is it really the output you need in your monitor?
Or am I missing your point?
Quoted 7 lines
What a BPF tlob would look like once those exist:
- model: still tlob.dot, with rvgen -b extended to the hybrid backend;
- state: one record per task inBPF_MAP_TYPE_TASK_STORAGE(or the per-object hash map the series has today), holding the state, the binding id and the budget;
Yeah I wasn't aware of BPF_MAP_TYPE_TASK_STORAGE before, I have a draft V3 (coming some time after the merge window) using that.
But I'd assume this type of storage would simplify the design of your monitor, which is currently using a PER_OBJ just to add more things to a PER_TASK. I'm going to keep in mind your use-case where you essentially extend the storage (which is perfectly fine in BPF since we don't use the task_struct slot).
Quoted 9 lines
- window: two uprobe programs attached to PATH:START and PATH:STOP, with the binding id and budget passed through the attach cookie or a small config map populated before load;
- expiry: abpf_timerin the same record, armed on state transitions and cancelled on STOP, submitting an error entry to the monitor's ringbuf;
- output: the rv tool reads the ringbuf and prints it, and the reactor is linked in withbpf_linkeras v2 already does.
Quoted 9 lines
The DA engine and the ringbuf/reactor plumbing would carry over unchanged. The three missing pieces are the uprobe attach/configuration path, the timer and env handling in
da_monitor_bpf.h, and the hybrid backend in rvgen.None of this argues against migrating. It argues that the BPF backend has to reach parity on (a)-(c) before the in-kernel path can be removed.
Thanks for taking time to assess this, it looks like the right direction.
Gabriele
Gabriele Monaco
maintainerQuoted 11 lines · Steven Rostedt
Quoted 5 lines · On Tue, 20 Oct 2026 03:18:48 +0800 Wen Yang <[email protected]> wrote:
If you're OK with it, I'd suggest merging the series in-kernel now, with 1-8 and 14 kept as the shared infrastructure and 9-13 acting as the reference implementation the BPF backend is measured against. I'd be glad to help with the uprobe and timed-HA pieces on the BPF side once they're on the table.
Well not "now" as the merge window has already opened, but perhaps for the next window.
That and I'm going to need to review this series between a few planned holidays. I'll try my best to squeeze it on time for the next merge window.
If things are not ready, I believe we can just upstream the infrastructural changes first.
Gabriele
Re: [PATCH v7 02/14] rv: Fix ha_monitor timer and invariant underflow
In reply to [email protected]Quoted 17 lines · On Mon, 2026-10-12 at 03:39 +0800, [email protected] wrote:
From: Wen Yang <[email protected]>
ha_start_timer_ns()andha_start_timer_jiffy()compute the remaining budget as expire - passed. Once the elapsed time passed exceeds the budget expire (e.g. a state transition after the invariant should have fired, before the timer callback ran), the unsigned subtraction underflows to a huge value and the timer is scheduled far into the future instead of expiring immediately.
ha_check_invariant_ns()computes store >=time_ns-expire_ns, which underflows when the system uptime is still belowexpire_ns(a monitor armed early in boot with a large budget), reporting a false violation.Clamp the timer remainder to 0 (fire immediately) when passed >= expire, and replace the invariant check as store +
expire_ns>=time_nsso neither side can underflow.
Quoted 5 lines
Fixes f5587d1b6ec9 ("rv: Add Hybrid Automata monitor type")Fixes ab2900ae252b ("rv: Simplify hybrid automata monitors's clockvariables")
Signed-off-by Wen Yang <[email protected]>---
Looks good, thanks
Quoted 80 lines
include/rv/ha_monitor.h | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)include/rv/ha_monitor.h +0
-0@@ -351,7 +351,7 @@ static inline void ha_reset_clk_ns(struct ha_monitor *ha_mon, enum envs env, u64static inline bool ha_check_invariant_ns(struct
ha_monitor*ha_mon, enum envs env,
u64time_ns, u64expire_ns) {include/rv/ha_monitor.h +1
-1-return READ_ONCE (ha_mon->env_store[env]) >= time_ns - expire_ns; +return READ_ONCE (ha_mon->env_store[env]) + expire_ns >= time_ns;}
/*
*ha_invariant_passed_ns- prepare the invariant and return the time since resetinclude/rv/ha_monitor.h +0
-1@@ -410,6 +410,11 @@ static inline void ha_inv_to_guard(struct ha_monitor *ha_mon, enum envs env, WRITE_ONCE(ha_mon->env_store[env], READ_ONCE(ha_mon->env_store[env]) - value); }include/rv/ha_monitor.h +5
-0+static inline u64 ha_time_left (u64 passed,u64 expire) +{ +return passed < expire ? expire - passed :0 ; +} +#if
HA_TIMER_TYPE==HA_TIMER_WHEEL
/*
* Helper functions to handle the monitor timer.include/rv/ha_monitor.h +0
-0@@ -439,15 +444,17 @@ static inline void ha_start_timer_jiffy(struct
ha_monitor*ha_mon, enum envs env
{
u64 passed =ha_invariant_passed_jiffy(ha_mon, env, time_ns);include/rv/ha_monitor.h +2
-1-mod_timer (&ha_mon->timer,get_jiffies_64 () + expire - passed); +mod_timer (&ha_mon->timer, + get_jiffies_64() + ha_time_left(passed, expire));}
static inline void ha_start_timer_ns(structha_monitor*ha_mon, enum envs env,
u64 expire, u64time_ns) {
u64 passed =ha_invariant_passed_ns(ha_mon, env, time_ns);include/rv/ha_monitor.h +1
-0+u64 delay =ha_time_left (passed, expire);
ha_start_timer_jiffy(ha_mon,ENV_MAX_STORED,include/rv/ha_monitor.h +0
-1- nsecs_to_jiffies(expire - passed + TICK_NSEC -1),
time_ns);include/rv/ha_monitor.h +1
-0+ nsecs_to_jiffies(delay + TICK_NSEC - 1),
time_ns);
}
/*
*ha_cancel_timer- Cancel the timerinclude/rv/ha_monitor.h +0
-0@@ -494,7 +501,8 @@ static inline void ha_start_timer_ns(struct ha_monitor *ha_mon, enum envs env,
if (RV_MON_TYPE==RV_MON_PER_CPU)
mode |=HRTIMER_MODE_PINNED;include/rv/ha_monitor.h +2
-1-hrtimer_start (&ha_mon->hrtimer,ns_to_ktime (expire - passed), mode); +hrtimer_start (&ha_mon->hrtimer, + ns_to_ktime(ha_time_left(passed, expire)), mode);}
static inline void ha_start_timer_jiffy(structha_monitor*ha_mon, enum envs env,
u64 expire, u64time_ns)include/rv/ha_monitor.h +0
-0@@ -502,7 +510,8 @@ static inline void ha_start_timer_jiffy(struct ha_monitor *ha_mon, enum envs env u64 passed = ha_invariant_passed_jiffy(ha_mon, env, time_ns);
ha_start_timer_ns(ha_mon,ENV_MAX_STORED,include/rv/ha_monitor.h +2
-1- jiffies_to_nsecs(expire - passed), time_ns); + jiffies_to_nsecs(ha_time_left(passed, expire)), + time_ns);}
/*
*ha_cancel_timer- Cancel the timer
Gabriele Monaco
maintainerRe: [PATCH v7 03/14] rv: Add a KUnit test for the HA timer underflow
In reply to [email protected]Quoted 22 lines · On Mon, 2026-10-12 at 03:39 +0800, [email protected] wrote:
From: Wen Yang <[email protected]>
Add rvtest, a minimal per-object HA monitor, and a test for both underflows fixed by the previous patch: an invariant checked while the uptime is below the budget holds, and a timer armed after the budget elapsed expires now.
Signed-off-by Wen Yang <[email protected]>---
kernel/trace/rv/rv_monitors_test.c | 122 +++++++++++++++++++++++++++++#include <
linux/rv.h>
#include "rv.h"kernel/trace/rv/rv_monitors_test.c +8
-0+#include <rv_trace.h> + +/* rvtest: a minimal per-object HA monitor driven directly by the tests. */ +#define MONITOR_NAME rvtest +#define RV_MON_TYPE RV_MON_PER_OBJ +#define HA_TIMER_TYPE HA_TIMER_HRTIMER +typedef void *monitor_target ; +
...
Quoted 1 line
+#include <
rv/ha_monitor.h>
Uhm, this isn't meant to be imported in things that aren't monitors, if we just decided to create another test monitor here it would be a problem.
I'd suggest you put this all in a new C file and build it as a separate object. Then you may export rv_test_rvtest_underflow() (extern function, not export symbol) for rv_monitors_test to use or whatever works best.
Feel free to add a new rv/kunit folder if it looks cleaner, it isn't necessary for now though.
Be mindful that it should still build as a module (trace_rv_retries_error() and rv_monitoring_on() aren't exported, you can shortcircuit them to nothing like you did for the other tracepoints).
I think you should be able to keep it as part of rv_monitors_test.ko but I haven't tried.
It's very good to have unit tests for these things, but in case you cannot get this ready for the next merge window I'd still get the other fix first.
Thanks,
Gabriele
Quoted 83 lines
kernel/trace/rv/rv_monitors_test.c +25
-0+ +#define RVTEST_BUDGET_NS (300ULL * NSEC_PER_MSEC) + +static u64 ha_get_env (struct ha_monitor *ha_mon,enum envs_rvtest env, + u64 time_ns) +{ +return ha_get_clk_ns (ha_mon, env, time_ns); +} + +static void ha_reset_env (struct ha_monitor *ha_mon,enum envs_rvtest env, +u64 time_ns) +{ +ha_reset_clk_ns (ha_mon, env, time_ns); +} + +/* The tests arm the timer themselves. */ +static bool ha_verify_constraint (struct ha_monitor *ha_mon, +enum states curr_state,enum events event, +enum states next_state,u64 time_ns) +{ +return true; +} + +static const struct rv_kunit_mon rvtest_mon =RV_MON_OPS_INIT (); +/*
* An easy way to pass the context is to use kunit_get_current_test()->priv,
* but this doesn't always work (e.g. a reactor running from another contextkernel/trace/rv/rv_monitors_test.c +0
-0@@ -146,6 +227,46 @@ static void rv_test_dummy(struct kunit *test) active_ctx = NULL; }kernel/trace/rv/rv_monitors_test.c +40
-0+/* +* rv_test_rvtest_underflow - the HA clock math must not underflow as u64 +* +* An invariant checked while the uptime is below the budget holds, and a +* timer armed after the budget elapsed expires now. +*/ +static void rv_test_rvtest_underflow (struct kunit *test) +{ +struct ha_monitor *ha_mon; +struct da_monitor *da_mon; +const int id =4242 ; + +prepare_test (test, &rvtest_mon); +da_handle_start_run_event (id,NULL , start_rvtest); + +scoped_guard (rcu) + da_mon =da_get_monitor (id,NULL ); +KUNIT_ASSERT_NOT_NULL (test, da_mon); + ha_mon =to_ha_monitor (da_mon); + +/* Uptime below the budget: time_ns - expire_ns would underflow. */ +ha_reset_clk_ns (ha_mon, clk_rvtest,0 ); +KUNIT_EXPECT_TRUE (test,ha_check_invariant_ns (ha_mon, clk_rvtest,0 , + RVTEST_BUDGET_NS)); + +/* +* Budget already elapsed: expire - passed would underflow and queue +* the timer at KTIME_SEC_MAX. +*/ +da_monitor_reset (&ha_mon->da_mon); +ha_reset_clk_ns (ha_mon, clk_rvtest,0 ); +ha_start_timer_ns (ha_mon, clk_rvtest,RVTEST_BUDGET_NS , + RVTEST_BUDGET_NS + NSEC_PER_SEC); +KUNIT_EXPECT_TRUE (test, + ktime_before(hrtimer_get_expires(&ha_mon->hrtimer), + ktime_get() + NSEC_PER_SEC)); + +ha_cancel_timer_sync (ha_mon); +} +#include "
monitors/sco/sco_kunit.c"
#include "monitors/sssw/sssw_kunit.c"
#include "monitors/sts/sts_kunit.c"kernel/trace/rv/rv_monitors_test.c +1
-0@@ -163,6 +284,7 @@ static struct kunit_case rv_mon_test_cases[] = { KUNIT_CASE(rv_test_nomiss), KUNIT_CASE(rv_test_pagefault), KUNIT_CASE(rv_test_sleep), + KUNIT_CASE(rv_test_rvtest_underflow), {} };
Gabriele Monaco
maintainerRe: [PATCH v7 04/14] rv: Disarm HA timers on the storage free path
In reply to [email protected]Quoted 9 lines · On Mon, 2026-10-12 at 03:39 +0800, [email protected] wrote:
From: Wen Yang <[email protected]>
da_destroy_storage()cancels the HA timer throughda_monitor_reset_hook()and only then unlinks the storage with hash_del_rcu(). A handler on another CPU that looked the storage up before the unlink can still call ha_start_timer_*() after the cancel. The timer then stays queued on a storage thatkfree_rcu()frees a grace period later, and fires on freed memory, reporting a violation for an object that no longer exists.
Quoted 3 lines
A monitor cannot close this from its own handlers: any cancel it issues precedes the unlink. The first point no reader can race with is the RCU callback that frees the storage.
Mmh, looks reasonable, I probably need to check this whole cleanup path again for consistency and duplication.
Quoted 9 lines
Add
da_monitor_free_hook(), called right before a per-object storage is freed, from the RCU callback and from da_monitor_destroy(). HA monitors implement it with ha_cancel_timer_sync(). Replacekfree_rcu()withcall_rcu()so there is a callback to hook.A storage whose monitor never started has a zeroed timer, and
hrtimer_cancel()dereferences timer->base. nomiss pre-creates one such storage per possible CPU. Skip them with the newha_timer_ready().
I struggled with this too, but relying on the internal representation of timers for this is brittle, can you use da_monitoring() there, like we do in ha_monitor_reset_env()? We shouldn't have running timers when not monitoring.
I still want to give a deeper look at this when I get more time.
Thanks,
Gabriele
Quoted 142 lines
Fixes 4a24127bd6cb ("rv: Add support for per-object monitors in DA/HA")Signed-off-by Wen Yang <[email protected]>---
include/rv/da_monitor.h | 23 ++++++++++++++++++++++-include/rv/ha_monitor.h| 26 ++++++++++++++++++++++++++2 files changed, 48 insertions(+), 1 deletion(-)include/rv/da_monitor.h +0
-0@@ -67,6 +67,17 @@ static struct rv_monitor rv_this; #define da_monitor_sync_hook() #endifinclude/rv/da_monitor.h +11
-0+/* +* Called on the last reference to a storage, right before it is freed. +* Monitors embedding a timer disarm it here: an earlier point races with a +* reader that resolved the storage before hash_del_rcu(), and +* tracepoint_synchronize_unregister() only covers tracepoint probes. +* Default: no-op. +*/ +#ifndef da_monitor_free_hook +#define da_monitor_free_hook (da_mon) +#endif +/*
* Type for the target id, default to int but can be overridden.
* A long type can work as hash table key (PER_OBJ) but will be downgraded toinclude/rv/da_monitor.h +0
-0@@ -543,6 +554,15 @@ static inline monitor_target da_get_target_by_id(da_id_type id) return mon_storage->target; }include/rv/da_monitor.h +9
-0+static void da_kfree_cb (struct rcu_head *head) +{ +struct da_monitor_storage *ms = + container_of(head, struct da_monitor_storage, rcu); + +da_monitor_free_hook (&ms->rv.da_mon); +kfree (ms); +} +/*
*da_destroy_storage- destroy the per-object storage *include/rv/da_monitor.h +1
-1@@ -564,7 +584,7 @@ static inline void da_destroy_storage(da_id_type id) return; da_monitor_reset_hook(&mon_storage->rv.da_mon); hash_del_rcu(&mon_storage->node); -kfree_rcu (mon_storage, rcu); +call_rcu (&mon_storage->rcu, da_kfree_cb); }
static void__da_monitor_reset_all(void (*reset)(struct da_monitor *))include/rv/da_monitor.h +1
-0@@ -607,6 +627,7 @@ static inline void da_monitor_destroy(void)* pending, we can safely assume no concurrent user. */ hash_for_each_safe(da_monitor_ht, bkt, tmp, mon_storage, node) { +da_monitor_free_hook (&mon_storage->rv.da_mon); hash_del_rcu(&mon_storage->node); kfree(mon_storage); }include/rv/ha_monitor.h +2
-0@@ -29,6 +29,8 @@ 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 inline void ha_cancel_timer_sync(struct ha_monitor *ha_mon); +static inline bool ha_timer_ready (struct ha_monitor *ha_mon); +static inline void ha_monitor_free_env (struct da_monitor *da_mon); static bool ha_monitor_handle_constraint(struct da_monitor *da_mon, enum states curr_state,enum events event,
include/rv/ha_monitor.h +1
-0@@ -38,6 +40,7 @@ static bool ha_monitor_handle_constraint(struct da_monitor *da_mon, #define da_monitor_init_hook ha_monitor_init_env #define da_monitor_reset_hook ha_monitor_reset_env #define da_monitor_sync_hook() synchronize_rcu() +#define da_monitor_free_hook ha_monitor_free_env
#if !defined(HA_SKIP_AUTO_CLEANUP) &&RV_MON_TYPE==RV_MON_PER_TASK/*include/rv/ha_monitor.h +7
-0@@ -467,8 +470,15 @@ static inline bool ha_cancel_timer(struct ha_monitor *ha_mon) { return timer_delete(&ha_mon->timer); } +/* timer_setup() sets it; the cancel paths here never clear it. */ +static inline bool ha_timer_ready (struct ha_monitor *ha_mon) +{ +return READ_ONCE (ha_mon->timer.function); +} static inline void ha_cancel_timer_sync(struct ha_monitor *ha_mon) { +if (!ha_timer_ready (ha_mon)) +return ; timer_delete_sync(&ha_mon->timer); }#elif
HA_TIMER_TYPE==HA_TIMER_HRTIMERinclude/rv/ha_monitor.h +10
-0@@ -524,8 +534,18 @@ static inline bool ha_cancel_timer(struct ha_monitor *ha_mon) { return hrtimer_try_to_cancel(&ha_mon->hrtimer) == 1; } +/* +* hrtimer_cancel() reads timer->base before anything else; a zeroed timer +* has it NULL and would fault. +*/ +static inline bool ha_timer_ready (struct ha_monitor *ha_mon) +{ +return READ_ONCE (ha_mon->hrtimer.base); +} static inline void ha_cancel_timer_sync(struct ha_monitor *ha_mon) { +if (!ha_timer_ready (ha_mon)) +return ; hrtimer_cancel(&ha_mon->hrtimer); }#else /*
HA_TIMER_NONE*/include/rv/ha_monitor.h +1
-0@@ -538,9 +558,15 @@ static inline bool ha_cancel_timer(struct ha_monitor *ha_mon) { return false; } +static inline bool ha_timer_ready (struct ha_monitor *ha_mon) {return false; } static inline void ha_cancel_timer_sync(struct ha_monitor *ha_mon) { } #endifinclude/rv/ha_monitor.h +5
-0+static inline void ha_monitor_free_env (struct da_monitor *da_mon) +{ +ha_cancel_timer_sync (to_ha_monitor (da_mon)); +} +#if
IS_ENABLED(CONFIG_RV_MONITORS_KUNIT_TEST)
#ifdefRV_MON_OPS_INIT
#undefRV_MON_OPS_INIT
Gabriele Monaco
maintainerQuoted 11 lines · On Mon, 2026-10-12 at 03:39 +0800, [email protected] wrote:
From: Wen Yang <[email protected]>
New in this revision:
- Patches 1 and 2: the HA environment lifecycle and timer fixes, in that order (see above). Patch 3 adds KUnit coverage for both.- Patch 4 (was 1): the HA timer teardown fix, found while auditing tlob's own teardown. The gap is in the framework; a monitor cannot close it. Patch 5 adds KUnit coverage, and Patch 8 splits the per-object teardown hook out of the tlob patch.
Quoted 5 lines
- Dropped
detail_env_tloband the per-state accounting. Which phase consumed the budget is now derived fromevent_tlobin user space;monitor_tlob.rstdocuments the recipe. This removes the per-record lock and the accumulator from the hot path, and tlob no longer deviates from "run a model and spot violations" (Gabriele, v2).
Very well.
You may find [1] of interest as it has some overlap with that accounting, aiming to introduce stats into the DA infrastructure. It probably isn't a direct match and only targets per-cpu for now.
By the way there has been a running discussion about keeping the C files for the monitors as close as possible to the DOT representation, with a long-term goal to extend the DOT to represent everything and have logic represented there too (at least simple trace filters).
Now I haven't been following this religiously either and some things are simply too hard to be represented there. But I'd like you to keep it in mind when writing your model, since yours deviates considerably.
For example I'm still not convinced by the fact that you maintain a parallel state (OPEN/CLOSE/RETIRED). You do a lot of error reporting in tlob_start_task() that is mainly discarded (return value ignored). What prevents you from allowing the start event in running and just not allocate if you have already a target?
I'd really prefer to have the complexity moved to the model. I'm probably missing some background here, but I see an open/closed window as something mostly matching the monitor's state, while the retired state as the result of a race with cleanup.
Where are you seeing such races? When the monitor is stopped while a window is still open? da_extra_cleanup() is supposed to run after everything else settled down, if you have evidence this isn't the case, let's discuss about that first.
If you could drop these states, you'd also no longer need tlob_reset_notify() and many other things will get simplified and closer to what rgven gives you.
And it's also good to keep everything rvgen gives you as is (ha_verify_invariants() and friends). Otherwise you'd never tell your model deviated from the code.
These are general guidelines, the C files cannot yet be a 1-to-1 product of the model, but shouldn't be a completely different story either.
Quoted 8 lines
- The model gives stopped a self-loop for every scheduler event, and the sched handlers call
da_handle_event()unconditionally. The source-side gate that skipped events while stopped is gone; the logic lives in the model (Gabriele, v6).-
tlob_sym.c(a 235-line ELF parser) is replaced by asym_offset()shell helper built on readelf (Gabriele, v3).
Great! Let's keep the code to maintain to a minimum.
Quoted 7 lines
-
tlob_preempt_work, identical totlob_busy_work, is dropped and the busy-wait body is shared (Gabriele, v3).- The KUnit and DOT commit messages were rewritten as requested (Gabriele, v6).
Quoted 7 lines
- Dropped three patches that no longer stand on their own: "rv: Fix
ha_invariant_passed_nssilent bypass of invariant check" (Patch 1 makes it unnecessary), "rv: Check the decoded fields in the tlob parser tests" and "selftests/verification: Fix up the tlob selftests" (folded into the patches they fix, so each patch is complete on its own).
Thanks,
Gabriele
[1] -
https://lore.kernel.org/lkml/[email protected]
Quoted 124 lines
Also folded into the patches they belong to, from earlier review rounds:
theDA_MON_POOL_SIZEdual-macro simplification andDA_ALLOC_MANUAL(Gabriele), the invariant-check rewrite (sashiko-bot), and the tlob teardown/error-path fixes:security_locked_down(LOCKDOWN_TRACEFS)intlob_monitor_open(), the singletlob_record_stateword withTLOB_RETIREDas the only licence to free, tlob_add_uprobe()'s rollback running the full nosync/quiesce/sync/reap sequence, andtlob_remove_all_uprobes()holdingtlob_uprobe_mutexacross the sync and the reap.Testing
=======
Environment:x86_64,CONFIG_PREEMPT_RT=y, kernel built with virtme-ng (vng --build), selftests built on the host and run inside the VM viaftracetest --rv ../verification(the verificationtest-ktap entry point, with logs redirected to /tmp):
- KUnit:rv_monsuite (CONFIG_RV_MONITORS_KUNIT_TEST),rv_test_tlobcase covering the uprobe-line parser
- 11 ftrace-style selftests:uprobe_bind,uprobe_violation,uprobe_no_event,uprobe_multi,uprobe_multi_instance, uprobe_detail_{running,sleeping,waiting},uprobe_restart,uprobe_overlap_register,uprobe_restart_violation
Both suites pass. The pre-existing rv_* tests in the same directory and the rest of therv_monKUnit suite (nomiss, sco, sssw, sts, opid, pagefault, sleep) keep passing as well.v6: https://lore.kernel.org/all/[email protected]/
v4: https://lore.kernel.org/all/[email protected]/
v3: https://lore.kernel.org/all/[email protected]/
v2: https://lore.kernel.org/all/[email protected]/
v1: https://lore.kernel.org/all/[email protected]/Gabriele Monaco (1):
rv: Force environment reset action on HA monitor startWen Yang (13):
rv: Fixha_monitortimer and invariant underflow
rv: Add a KUnit test for the HA timer underflow
rv: Disarm HA timers on the storage free path
rv: Add a KUnit test for the storage free path
rv: Makeda_monitor_reset_hookandEVENT_NONE_LBLoverridable
rv: IntroduceDA_MON_ALLOCATION_STRATEGY
rv: Add per-object teardown hookda_extra_cleanup
rv: Add generic uprobe infrastructure for RV monitors
rv: Add tlob model DOT file
rv: Add tlob hybrid automaton monitor
rv: Add KUnit tests for the tlob monitor selftests/verification: Add tlob selftests selftests/ftrace: Walk up parent dirs to find test.d/functionsDocumentation/trace/rv/index.rst | 1 + Documentation/trace/rv/monitor_tlob.rst | 154 +++ include/rv/da_monitor.h | 154 ++- include/rv/ha_monitor.h | 61 +- include/rv/rv_uprobe.h | 35 + kernel/trace/rv/Kconfig | 5 + kernel/trace/rv/Makefile | 2 + kernel/trace/rv/monitors/nomiss/nomiss.c | 2 +- kernel/trace/rv/monitors/opid/opid.h | 1 + kernel/trace/rv/monitors/tlob/Kconfig | 12 + kernel/trace/rv/monitors/tlob/tlob.c | 1049 +++++++++++++++++ kernel/trace/rv/monitors/tlob/tlob.h | 142 +++
kernel/trace/rv/monitors/tlob/tlob_kunit.c | 151 +++ kernel/trace/rv/monitors/tlob/tlob_kunit.h | 18 + kernel/trace/rv/monitors/tlob/tlob_trace.h | 20 +kernel/trace/rv/rv_monitors_test.c | 162 +++ kernel/trace/rv/rv_trace.h | 1 + kernel/trace/rv/rv_uprobe.c | 56 + tools/testing/selftests/ftrace/ftracetest | 19 +- .../testing/selftests/verification/.gitignore | 1 +
tools/testing/selftests/verification/Makefile| 4 + tools/testing/selftests/verification/config | 3 + .../selftests/verification/test.d/functions | 9 + .../test.d/tlob/run_tlob_tests.sh | 32 + .../verification/test.d/tlob/uprobe_bind.tc | 47 + .../test.d/tlob/uprobe_breakdown.tc | 136 +++ .../test.d/tlob/uprobe_disable_live.tc | 79 ++ .../verification/test.d/tlob/uprobe_multi.tc | 58 + .../test.d/tlob/uprobe_multi_instance.tc | 65 + .../test.d/tlob/uprobe_no_event.tc | 17 + .../test.d/tlob/uprobe_overlap_register.tc | 66 ++ .../test.d/tlob/uprobe_restart.tc | 66 ++ .../test.d/tlob/uprobe_restart_violation.tc | 48 + .../test.d/tlob/uprobe_unbind_live.tc | 65 + .../test.d/tlob/uprobe_violation.tc | 53 + .../selftests/verification/tlob_target.c | 127 ++ tools/verification/models/tlob.dot | 27 + tools/verification/rvgen/rvgen/dot2c.py | 2 +38 files changed, 2930 insertions(+), 20 deletions(-)create mode 100644
Documentation/trace/rv/monitor_tlob.rstcreate mode 100644include/rv/rv_uprobe.h
create mode 100644kernel/trace/rv/monitors/tlob/Kconfig
create mode 100644kernel/trace/rv/monitors/tlob/tlob.c
create mode 100644kernel/trace/rv/monitors/tlob/tlob.h
create mode 100644kernel/trace/rv/monitors/tlob/tlob_kunit.ccreate mode 100644kernel/trace/rv/monitors/tlob/tlob_kunit.hcreate mode 100644kernel/trace/rv/monitors/tlob/tlob_trace.hcreate mode 100644kernel/trace/rv/rv_uprobe.c
create mode 100755tools/testing/selftests/verification/test.d/tlob/run_tlob_tests.shcreate mode 100644tools/testing/selftests/verification/test.d/tlob/uprobe_bind.tccreate mode 100644tools/testing/selftests/verification/test.d/tlob/uprobe_breakdown.tccreate mode 100644tools/testing/selftests/verification/test.d/tlob/uprobe_disable_live.tccreate mode 100644tools/testing/selftests/verification/test.d/tlob/uprobe_multi.tccreate mode 100644tools/testing/selftests/verification/test.d/tlob/uprobe_multi_instance.tccreate mode 100644tools/testing/selftests/verification/test.d/tlob/uprobe_no_event.tccreate mode 100644tools/testing/selftests/verification/test.d/tlob/uprobe_overlap_register.tccreate mode 100644tools/testing/selftests/verification/test.d/tlob/uprobe_restart.tccreate mode 100644tools/testing/selftests/verification/test.d/tlob/uprobe_restart_violation.tccreate mode 100644tools/testing/selftests/verification/test.d/tlob/uprobe_unbind_live.tccreate mode 100644tools/testing/selftests/verification/test.d/tlob/uprobe_violation.tccreate mode 100644tools/testing/selftests/verification/tlob_target.ccreate mode 100644tools/verification/models/tlob.dot
Gabriele Monaco
maintainerQuoted 7 lines · Gabriele Monaco
By the way there has been a running discussion about keeping the C files for the monitors as close as possible to the DOT representation, with a long-term goal to extend the DOT to represent everything and have logic represented there too (at least simple trace filters).
...
These are general guidelines, the C files cannot yet be a 1-to-1 product of the model, but shouldn't be a completely different story either.
To push this even further, why don't we get the framework allocate also the tlob_record ?
I played a bit to adapt the PER_OBJ things to accept non-pointer targets. Now this is far from clean and is /not/ a recommendation over how the API should look like, but it seems to work and simplified tlob even further.
Nothing forbids us to embed data with the monitor state also for other types in the future (besides in-kernel PER_TASK, that must be fixed).
What do you think?
include/rv/da_monitor.h +71 -6
@@ -23,6 +23,17 @@* must run in task context; other handlers only call * da_handle_event() and never allocate. * DA_ALLOC_MANUAL - caller pre-inserts storage; framework links the target. +* +* The target type: a PER_OBJ monitor usually typedefs monitor_target itself, +* as a pointer to the monitored object (borrowed: the object exists +* independently of the monitor and outlives the storage). A monitor whose +* target is monitor-owned data can instead define DA_OBJ_EMBEDDED_TYPE with +* a complete struct type: the framework then derives monitor_target as a +* pointer to it, embeds one instance in each per-object storage, and +* allocates and frees it together with the storage. On storage creation the +* caller-supplied target is shallow-copied into the storage before it is +* published, so it may be a fully initialised stack object. Do not define +* monitor_target by hand when using DA_OBJ_EMBEDDED_TYPE. */ #define DA_ALLOC_AUTO 0#define DA_ALLOC_POOL 1
@@ -61,6 +72,10 @@#include <linux/slab.h> #include <linux/hashtable.h> +#if defined(DA_OBJ_EMBEDDED_TYPE ) &&RV_MON_TYPE !=RV_MON_PER_OBJ +#error "DA_OBJ_EMBEDDED_TYPE is only supported by RV_MON_PER_OBJ monitors" +#endif +/* * Per-cpu variables require a unique name although static in some * configurations (e.g. alpha modules).
@@ -463,9 +478,24 @@ static inline void da_monitor_destroy(void)* Functions to define, init and get a per-object monitor. */ +#ifdef DA_OBJ_EMBEDDED_TYPE +/* +* Framework-owned target: one instance is embedded in each storage and +* allocated/freed with it. monitor_target keeps pointer semantics, but +* da_get_target() points into the storage rather than to a borrowed object. +* The header derives the typedef: do not define monitor_target by hand. +*/ +typedef DA_OBJ_EMBEDDED_TYPE *monitor_target ; +#endif +struct da_monitor_storage {da_id_type id; +#ifdef DA_OBJ_EMBEDDED_TYPE +DA_OBJ_EMBEDDED_TYPE target; +bool filled; +#else monitor_target target; +#endif union rv_task_monitor rv;struct hlist_node node;struct rcu_head rcu;
@@ -476,6 +506,38 @@ struct da_monitor_storage {#endif static DEFINE_HASHTABLE(da_monitor_ht, DA_MONITOR_HT_BITS); +#ifdef DA_OBJ_EMBEDDED_TYPE + +#define TARGET_PTR (storage) &storage->target + +static inline bool da_storage_is_empty (struct da_monitor_storage *mon_storage) +{ +return !smp_load_acquire (&mon_storage->filled); +} +static inline void da_storage_set_target (struct da_monitor_storage *mon_storage, +monitor_target target) +{ +if (WARN_ON_ONCE (!target)) +return ; + +memcpy (&mon_storage->target, target,sizeof (mon_storage->target)); +smp_store_release (&mon_storage->filled, true); +} +#else + +#define TARGET_PTR (storage) storage->target + +static inline bool da_storage_is_empty (struct da_monitor_storage *mon_storage) +{ +return !TARGET_PTR (mon_storage); +} +static inline void da_storage_set_target (struct da_monitor_storage *mon_storage, +monitor_target target) +{ + mon_storage->target = target; +} +#endif +/* * da_create_empty_storage - pre-allocate an empty storage */
@@ -513,7 +575,7 @@ static inline struct da_monitor *da_create_storage(da_id_type id,if (!mon_storage)return NULL ; - mon_storage->target = target; +da_storage_set_target (mon_storage, target);return &mon_storage->rv.da_mon; }
@@ -549,7 +611,7 @@ static struct da_monitor *da_get_monitor(da_id_type id, monitor_target target)*/ static inline monitor_target da_get_target (struct da_monitor *da_mon) { - return container_of(da_mon, struct da_monitor_storage, rv.da_mon)->target; + return TARGET_PTR(container_of(da_mon, struct da_monitor_storage, rv.da_mon)); }/*
@@ -582,8 +644,11 @@ static inline struct da_monitor *da_fill_empty_storage(da_id_type id,monitor_target target, struct da_monitor *da_mon) { - if (unlikely(da_mon && !da_get_target(da_mon))) - container_of(da_mon, struct da_monitor_storage, rv.da_mon)->target = target; + struct da_monitor_storage *mon_storage = + container_of(da_mon, struct da_monitor_storage, rv.da_mon); + +if (unlikely (da_mon &&da_storage_is_empty (mon_storage))) +da_storage_set_target (mon_storage, target);return da_mon; }
@@ -599,7 +664,7 @@ static inline monitor_target da_get_target_by_id(da_id_type id)if (unlikely (!mon_storage))return NULL ; -return mon_storage->target; +return TARGET_PTR (mon_storage); }static void da_kfree_cb (struct rcu_head *head)
@@ -645,7 +710,7 @@ da_create_pool_storage(da_id_type id, monitor_target target,memset (mon_storage,0 ,sizeof (*mon_storage)); mon_storage->id = id; - mon_storage->target = target; +da_storage_set_target (mon_storage, target);hash_add_rcu (da_monitor_ht, &mon_storage->node, id);return &mon_storage->rv.da_mon;
Quoted 66 lines · Gabriele Monaco
Quoted 9 lines · Wen Yang
Quoted 8 lines
there's been a conversation at the LPC conference about completely migrating RV
to BPF.
While we aren't sure of its feasibility yet, and it will take time, it's something to keep in mind for new contributions.Quoted 4 lines
Understood, and I have been following that direction. I spent the last few days reading the v2 "rv: Add support for BPF monitors" series in detail, so the comparison below is against that code rather than the general idea.
Great, I'm going to review your series when I get some time.
Quoted 11 lines
Quoted 4 lines
First of all, would dropping RV in-kernel support negatively affect your current
workflow?Yes, it would. Some concrete points, all from the current tree.
(a) Event ABI.event_da_monitor/ error_env_* inkernel/trace/rv/rv_trace.hare plain tracepoints, so trace-cmd, perf and anything else reading tracefs consume them today with no extra userspace. The BPF series delivers events through a ring buffer read by the rv tool, so the two backends are not interchangeable for existing consumers.Good point, you might have noticed a similar discussion is happening also on the RV-BPF thread. We cannot have the absolute same ABI in BPF, but we probably need tracepoints or anything that can easily interact with other tracing tools.
Quoted 10 lines
(b) Build and deployment. On the images we target, BPF is often not compiled in or not permitted:
CONFIG_BPF_SYSCALL default n CONFIG_BPF_EVENTS needsBPF_SYSCALLCONFIG_DEBUG_INFO_BTF needed for CO-RE; depends on !DEBUG_INFO_REDUCED and pahole LOCKDOWN_BPF_READ_KERNEL disables theprobe_readhelpers Those are product constraints, not local build issues. tracefs/RV works in all of these configurations.Quoted 6 lines
There is also a size cost: BPF implies
CONFIG_DEBUG_INFO_BTF, and .BTF is resident inRO_DATAand kept in the boot image - about 5 MB uncompressed on a full kernel, 1.2-1.6 MB compressed, plus roughly 25% per module with DEBUG_INFO_BTF_MODULES. On a 16-32 MB kernel partition that matters.I've vaguely heard automotive folks saying they don't want to build BPF support several times. Finally I get a proper technical reason!
Now I think this isn't easy to sort out, if that's a hard requirement, we are probably going to need to keep in-kernel monitors around.
Quoted 6 lines
(c) Timing semantics. tlob arms a per-task hrtimer with HRTIMER_MODE_REL_HARD.
bpf_timeris fixed toHRTIMER_MODE_REL_SOFTand the verifier states "bpf_timercallbacks are never sleepable" . OnPREEMPT_RTthe callback runs in a
threaded
softirq, so the expiry time carries scheduler latency.I haven't looked into this yet, but from what you say, the
bpf_timerscode doesn't really supportPREEMPT_RT.
Quoted 8 lines
Quoted 2 lines
For a budget monitor the expiry time is the output, so this is a semantic change, not just overhead.
Well, the expiry time you get from an hrtimer is what you configured + some system jitter. The jitter here is just going to be less predictable but is it really the output you need in your monitor?
Or am I missing your point?
Hi Gabriele,
Thanks for the feedback.
For tlob, the primary goal is detecting deadline overruns on the order of milliseconds or sub-milliseconds (e.g. 10 ms budget). Under HRTIMER_MODE_REL_SOFT on PREEMPT_RT, the timer callback runs in a
threaded softirq (ksoftirqd or threaded irq context). If the system is
under CPU or scheduler pressure, the softirq latency itself can easily add several milliseconds of unpredictable delay. When that happens, it blurs attribution: users cannot distinguish whether the monitored task
exceeded its budget, or whether the task finished on time but the timer
callback was delayed in the scheduler queue. HARD hrtimer avoids this by firing directly in hardirq context at the physical deadline.
Quoted 8 lines
Quoted 7 lines
What a BPF tlob would look like once those exist:
- model: still tlob.dot, with rvgen -b extended to the hybrid backend;
- state: one record per task inBPF_MAP_TYPE_TASK_STORAGE(or the per-object hash map the series has today), holding the state, the binding id and the budget;
Quoted 7 lines
Yeah I wasn't aware of
BPF_MAP_TYPE_TASK_STORAGEbefore, I have a draft V3 (coming some time after the merge window) using that.
But I'd assume this type of storage would simplify the design of your monitor, which is currently using aPER_OBJjust to add more things to aPER_TASK. I'm going to keep in mind your use-case where you essentially extend the storage (which is perfectly fine in BPF since we don't use thetask_structslot).
Thanks.
On the in-kernel side, your DA_OBJ_EMBEDDED_TYPE PoC delivered the exact same simplification.
In v8, we adopted DA_OBJ_EMBEDDED_TYPE so struct tlob_record is embedded directly into struct da_monitor_storage:
- Deleted the private mempool and custom allocator entirely; tlob now relies 100% on the framework pre-allocated pool.
- Deleted the parallel state machine (OPEN / CLOSED / RETIRED) and tlob_binding_lock. The window lifecycle is now driven purely by the automaton states (stopped, running, waiting, sleeping).
- Overall, tlob.c shrunk from ~1065 lines to ~660 lines (-38%), with no locking on the scheduler hot path.
We are currently testing and will send out v8 soon.
--
Best wishes,
Wen
Quoted 24 lines
Quoted 9 lines
- window: two uprobe programs attached to PATH:START and PATH:STOP, with the binding id and budget passed through the attach cookie or a small config map populated before load;
- expiry: abpf_timerin the same record, armed on state transitions and cancelled on STOP, submitting an error entry to the monitor's ringbuf;
- output: the rv tool reads the ringbuf and prints it, and the reactor is linked in withbpf_linkeras v2 already does.Quoted 9 lines
The DA engine and the ringbuf/reactor plumbing would carry over unchanged. The three missing pieces are the uprobe attach/configuration path, the timer and env handling in
da_monitor_bpf.h, and the hybrid backend in rvgen.None of this argues against migrating. It argues that the BPF backend has to reach parity on (a)-(c) before the in-kernel path can be removed.
Thanks for taking time to assess this, it looks like the right direction.
Gabriele