* [RFC PATCH v2 00/10] rv/tlob: Add task latency over budget RV monitor
From: wen.yang @ 2026-05-11 18:24 UTC (permalink / raw)
To: Gabriele Monaco, Steven Rostedt
Cc: linux-trace-kernel, linux-kernel, Wen Yang
From: Wen Yang <wen.yang@linux.dev>
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 fires when the time exceeds a
configurable budget.
Background
----------
The existing wwnr monitor uses a two-state DA to detect tasks that are
woken but never run. tlob extends the RV framework to a three-state
hybrid automaton:
running (initial) -- on CPU
waiting -- in the scheduler runqueue, not yet on CPU
sleeping -- blocked on a lock, I/O, or similar resource
A single HA clock invariant, clk_elapsed < BUDGET_NS(), is active in
all states. The framework enforces it via a per-task hrtimer. On
expiry, error_env_tlob is emitted, followed by detail_env_tlob which
carries a per-state time breakdown (running_ns, waiting_ns, sleeping_ns)
that pinpoints whether the overrun occurred in the running, waiting, or
sleeping state.
Two userspace interfaces are provided:
ioctl (/dev/rv): TLOB_IOCTL_TRACE_START / TLOB_IOCTL_TRACE_STOP for
in-process self-instrumentation; TRACE_STOP returns -EOVERFLOW if
the budget was exceeded during the window.
tracefs (monitors/tlob/monitor): write "p PATH:START STOP threshold=NS"
to attach uprobes to an unmodified binary. The start uprobe calls
tlob_start_task() and the stop uprobe calls tlob_stop_task().
Infrastructure additions
------------------------
The series also includes several prerequisite additions to the RV
framework that are useful beyond tlob:
rv/da: two bug fixes for race conditions in da_monitor_destroy() and
the monitoring flag memory ordering that affect existing per-task
monitors (patches 1-2).
rv/da: pre-allocated storage pool (da_monitor_init_prealloc) to avoid
kmalloc on the scheduler tracepoint hot path and to support
PREEMPT_RT configurations (patch 4).
rv: generic uprobe infrastructure (rv_uprobe) providing a thin wrapper
around uprobe_consumer with a per-binding priv pointer and fully
synchronous detach semantics (patch 5).
rvgen: support for reset() annotations on the __init arrow, enabling
monitors where a clock is reset once at object creation and active
in all states (patch 6).
Testing
-------
KUnit (five suites, 19 test cases):
tlob_task_api -- start/stop lifecycle, error paths, -ENOSPC,
deadline firing in each state
tlob_sched_integration -- context-switch accounting, monitoring a
kthread other than current
tlob_uprobe_format -- add/remove format acceptance and rejection
via tlob_create_or_delete_uprobe()
tlob_trace_output -- event_tlob and error_env_tlob field values
tlob_violation_react -- error count per budget expiry; per-state
ns breakdown dominance
kselftest (12 tests, run via verificationtest-ktap on x86_64 with vng):
TAP version 13
1..12
ok 1 Test monitor enable/disable
ok 2 Test monitor reactor setting
ok 3 Check available monitors
ok 4 Test wwnr monitor with printk reactor
ok 5 Test tlob ioctl self-instrumentation (within/over-budget, error paths)
ok 6 Test tlob monitor tracefs interface (enable/disable and files)
ok 7 uprobe binding: visible in monitor file, removable, duplicate offset rejected
ok 8 uprobe detail sleeping: sleeping_ns dominates when task blocks between probes
ok 9 uprobe detail waiting: waiting_ns dominates when task is preempted between probes
ok 10 Two bindings on same binary with different offsets and budgets fire independently
ok 11 Verify no spurious error_env_tlob events without an active uprobe binding
ok 12 uprobe violation: error_env_tlob and detail_env_tlob fire with correct fields
# Totals: pass:12 fail:0 xfail:0 xpass:0 skip:0 error:0
Changes in v2 (addressing review by Gabriele)
----------------------------------------------------------------
- Switch from a custom hash table to RV_MON_PER_OBJ with
"typedef struct tlob_task_state *monitor_target"; the per-invocation
threshold is carried in the target struct and accessed via
ha_get_target(ha_mon)->threshold_us, following the nomiss monitor
pattern.
- Replace the explicit unmonitored state and da_monitor_init_hook()
timer arming with da_handle_start_event() for initial state setup,
then ha_reset_clk_ns() + ha_start_timer_ns() directly. The HA clock
invariant clk_elapsed < BUDGET_NS() is now expressed in the DOT model
and enforced by the HA framework.
- Drop the v1 KUnit test suite that validated every DA transition matrix
entry; replace with five suites that test meaningful behaviours:
start/stop lifecycle, context-switch accounting, uprobe format
validation, trace event field values, and violation reaction with
per-state ns breakdown.
- Move selftests from tools/testing/selftests/rv/ into the existing
tools/testing/selftests/verification/ harness; test logic is expressed
as ftracetest .tc scripts under test.d/tlob/, with helper binaries
(tlob_ioctl, tlob_target) built by tlob/Makefile and located via PATH.
Wen Yang (10):
rv/da: fix monitor start ordering and memory ordering for monitoring
flag
rv/da: fix per-task da_monitor_destroy() ordering and sync
selftests/verification: fix verificationtest-ktap for out-of-tree
execution
rv/da: add pre-allocated storage pool for per-object monitors
rv: add generic uprobe infrastructure for RV monitors
rvgen: support reset() on the __init arrow for global-window HA clocks
rv/tlob: add tlob model DOT file
rv/tlob: add tlob hybrid automaton monitor
rv/tlob: add KUnit tests for the tlob monitor
selftests/verification: add tlob selftests
Documentation/trace/rv/index.rst | 1 +
Documentation/trace/rv/monitor_tlob.rst | 213 +++
MAINTAINERS | 3 +
include/linux/rv.h | 45 +
include/rv/automata.h | 15 +
include/rv/da_monitor.h | 234 ++-
include/rv/ha_monitor.h | 33 +-
include/rv/rv_uprobe.h | 119 ++
include/uapi/linux/rv.h | 86 ++
kernel/trace/rv/Kconfig | 6 +
kernel/trace/rv/Makefile | 5 +-
kernel/trace/rv/monitors/tlob/.kunitconfig | 5 +
kernel/trace/rv/monitors/tlob/Kconfig | 69 +
kernel/trace/rv/monitors/tlob/tlob.c | 1333 +++++++++++++++++
kernel/trace/rv/monitors/tlob/tlob.h | 171 +++
kernel/trace/rv/monitors/tlob/tlob_kunit.c | 881 +++++++++++
kernel/trace/rv/monitors/tlob/tlob_trace.h | 58 +
kernel/trace/rv/rv.c | 38 +
kernel/trace/rv/rv.h | 2 +
kernel/trace/rv/rv_chardev.c | 201 +++
kernel/trace/rv/rv_trace.h | 1 +
kernel/trace/rv/rv_uprobe.c | 189 +++
tools/include/uapi/linux/rv.h | 86 ++
tools/testing/selftests/verification/Makefile | 21 +-
.../verification/test.d/tlob/ioctl.tc | 36 +
.../verification/test.d/tlob/tracefs.tc | 17 +
.../verification/test.d/tlob/uprobe_bind.tc | 34 +
.../test.d/tlob/uprobe_detail_sleeping.tc | 47 +
.../test.d/tlob/uprobe_detail_waiting.tc | 60 +
.../verification/test.d/tlob/uprobe_multi.tc | 60 +
.../test.d/tlob/uprobe_no_event.tc | 19 +
.../test.d/tlob/uprobe_violation.tc | 60 +
.../selftests/verification/tlob/Makefile | 21 +
.../selftests/verification/tlob/tlob_ioctl.c | 626 ++++++++
.../selftests/verification/tlob/tlob_target.c | 138 ++
.../verification/verificationtest-ktap | 4 +-
tools/verification/models/tlob.dot | 21 +
tools/verification/rvgen/rvgen/automata.py | 26 +
tools/verification/rvgen/rvgen/dot2k.py | 100 +-
39 files changed, 5043 insertions(+), 41 deletions(-)
create mode 100644 Documentation/trace/rv/monitor_tlob.rst
create mode 100644 include/rv/rv_uprobe.h
create mode 100644 include/uapi/linux/rv.h
create mode 100644 kernel/trace/rv/monitors/tlob/.kunitconfig
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_trace.h
create mode 100644 kernel/trace/rv/rv_chardev.c
create mode 100644 kernel/trace/rv/rv_uprobe.c
create mode 100644 tools/include/uapi/linux/rv.h
create mode 100644 tools/testing/selftests/verification/test.d/tlob/ioctl.tc
create mode 100644 tools/testing/selftests/verification/test.d/tlob/tracefs.tc
create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_bind.tc
create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_detail_sleeping.tc
create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_detail_waiting.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_no_event.tc
create mode 100644 tools/testing/selftests/verification/test.d/tlob/uprobe_violation.tc
create mode 100644 tools/testing/selftests/verification/tlob/Makefile
create mode 100644 tools/testing/selftests/verification/tlob/tlob_ioctl.c
create mode 100644 tools/testing/selftests/verification/tlob/tlob_target.c
create mode 100644 tools/verification/models/tlob.dot
--
2.25.1
^ permalink raw reply
* [RFC PATCH v2 04/10] rv/da: add pre-allocated storage pool for per-object monitors
From: wen.yang @ 2026-05-11 18:24 UTC (permalink / raw)
To: Gabriele Monaco, Steven Rostedt
Cc: linux-trace-kernel, linux-kernel, Wen Yang
In-Reply-To: <cover.1778522945.git.wen.yang@linux.dev>
From: Wen Yang <wen.yang@linux.dev>
da_create_empty_storage() uses kmalloc_nolock(), which requires
CONFIG_HAVE_ALIGNED_STRUCT_PAGE; on UML and some PREEMPT_RT
configurations it always returns NULL. Calling kmalloc from scheduler
tracepoint handlers also adds unwanted latency and can fail under
memory pressure.
Add da_monitor_init_prealloc(N) as an opt-in alternative to
da_monitor_init(). It allocates N da_monitor_storage slots with
GFP_KERNEL up-front and manages them on a LIFO free-stack protected
by a spinlock, so da_create_or_get() never calls kmalloc on the hot
path.
Monitors that do not call da_monitor_init_prealloc() are unaffected.
Signed-off-by: Wen Yang <wen.yang@linux.dev>
---
include/rv/da_monitor.h | 208 +++++++++++++++++++++++++++++++++++-----
1 file changed, 186 insertions(+), 22 deletions(-)
diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index d04bb3229c75..7d6f62766251 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -433,18 +433,6 @@ static inline da_id_type da_get_id(struct da_monitor *da_mon)
return container_of(da_mon, struct da_monitor_storage, rv.da_mon)->id;
}
-/*
- * da_create_or_get - create the per-object storage if not already there
- *
- * This needs a lookup so should be guarded by RCU, the condition is checked
- * directly in da_create_storage()
- */
-static inline void da_create_or_get(da_id_type id, monitor_target target)
-{
- guard(rcu)();
- da_create_storage(id, target, da_get_monitor(id, target));
-}
-
/*
* da_fill_empty_storage - store the target in a pre-allocated storage
*
@@ -475,15 +463,121 @@ static inline monitor_target da_get_target_by_id(da_id_type id)
return mon_storage->target;
}
+/*
+ * Per-object pool state.
+ *
+ * Zero-initialised by default (storage == NULL ⟹ kmalloc mode). A monitor
+ * opts into pool mode by calling da_monitor_init_prealloc(N) instead of
+ * da_monitor_init(), which sets storage to a non-NULL kcalloc'd array.
+ *
+ * Because every field is wrapped in this struct and the struct itself is a
+ * per-TU static, each monitor that includes this header gets a completely
+ * independent pool. A kmalloc monitor (e.g. nomiss) and a pool monitor
+ * (e.g. tlob) therefore coexist without any interference.
+ *
+ * da_pool_return_cb runs from softirq on non-PREEMPT_RT, so irqsave is
+ * required to prevent deadlock with task-context callers. On PREEMPT_RT
+ * it runs from an rcuc kthread where spinlock_t is a sleeping lock.
+ */
+struct da_per_obj_pool {
+ struct da_monitor_storage *storage; /* non-NULL ⟹ pool mode */
+ struct da_monitor_storage **free; /* kmalloc'd pointer stack */
+ unsigned int free_top;
+ spinlock_t lock;
+};
+
+static struct da_per_obj_pool da_pool = {
+ .lock = __SPIN_LOCK_UNLOCKED(da_pool.lock),
+};
+
+static void da_pool_return_cb(struct rcu_head *head)
+{
+ struct da_monitor_storage *ms =
+ container_of(head, struct da_monitor_storage, rcu);
+ unsigned long flags;
+
+ spin_lock_irqsave(&da_pool.lock, flags);
+ da_pool.free[da_pool.free_top++] = ms;
+ spin_unlock_irqrestore(&da_pool.lock, flags);
+}
+
+/* Pops a slot from the pre-allocated pool; returns -ENOSPC if exhausted. */
+static inline int da_create_or_get_pool(da_id_type id, monitor_target target)
+{
+ struct da_monitor_storage *mon_storage;
+ unsigned long flags;
+
+ spin_lock_irqsave(&da_pool.lock, flags);
+ if (!da_pool.free_top) {
+ spin_unlock_irqrestore(&da_pool.lock, flags);
+ return -ENOSPC;
+ }
+ mon_storage = da_pool.free[--da_pool.free_top];
+ spin_unlock_irqrestore(&da_pool.lock, flags);
+
+ mon_storage->id = id;
+ mon_storage->target = target;
+ guard(rcu)();
+ hash_add_rcu(da_monitor_ht, &mon_storage->node, id);
+ return 0;
+}
+
+/*
+ * Tries da_create_storage() first (lock-free via kmalloc_nolock); falls back
+ * to kmalloc(GFP_KERNEL). Must be called from task context.
+ */
+static inline int da_create_or_get_kmalloc(da_id_type id, monitor_target target)
+{
+ struct da_monitor_storage *mon_storage;
+
+ scoped_guard(rcu) {
+ if (da_create_storage(id, target, da_get_monitor(id, target)))
+ return 0;
+ }
+
+ /*
+ * da_create_storage() failed because kmalloc_nolock() returned NULL.
+ * Allocate with GFP_KERNEL outside the RCU read section: GFP_KERNEL
+ * may sleep for memory reclaim, which is illegal while the RCU read
+ * lock is held (preemption disabled on !PREEMPT_RT).
+ */
+ mon_storage = kmalloc_obj(*mon_storage, GFP_KERNEL | __GFP_ZERO);
+ if (!mon_storage)
+ return -ENOMEM;
+ mon_storage->id = id;
+ mon_storage->target = target;
+
+ /*
+ * Re-check for a concurrent insertion before linking: another
+ * caller may have succeeded while we slept in kmalloc().
+ * Discard our allocation and let the winner's entry stand.
+ */
+ scoped_guard(rcu) {
+ if (da_get_monitor(id, target)) {
+ kfree(mon_storage);
+ return 0;
+ }
+ hash_add_rcu(da_monitor_ht, &mon_storage->node, id);
+ }
+ return 0;
+}
+
+/* Create the per-object storage if not already there. */
+static inline int da_create_or_get(da_id_type id, monitor_target target)
+{
+ if (da_pool.storage)
+ return da_create_or_get_pool(id, target);
+ return da_create_or_get_kmalloc(id, target);
+}
+
/*
* da_destroy_storage - destroy the per-object storage
*
- * The caller is responsible to synchronise writers, either with locks or
- * implicitly. For instance, if da_destroy_storage is called at sched_exit and
- * da_create_storage can never occur after that, it's safe to call this without
- * locks.
- * This function includes an RCU read-side critical section to synchronise
- * against da_monitor_destroy().
+ * Pool mode: removes from hash and returns the slot via call_rcu().
+ * Kmalloc mode: removes from hash and frees via kfree_rcu().
+ *
+ * Includes an RCU read-side critical section to synchronise against
+ * da_monitor_destroy().
*/
static inline void da_destroy_storage(da_id_type id)
{
@@ -491,15 +585,17 @@ static inline void da_destroy_storage(da_id_type id)
guard(rcu)();
mon_storage = __da_get_mon_storage(id);
-
if (!mon_storage)
return;
da_monitor_reset_hook(&mon_storage->rv.da_mon);
hash_del_rcu(&mon_storage->node);
- kfree_rcu(mon_storage, rcu);
+ if (da_pool.storage)
+ call_rcu(&mon_storage->rcu, da_pool_return_cb);
+ else
+ kfree_rcu(mon_storage, rcu);
}
-static void da_monitor_reset_all(void)
+static __maybe_unused void da_monitor_reset_all(void)
{
struct da_monitor_storage *mon_storage;
int bkt;
@@ -510,13 +606,65 @@ static void da_monitor_reset_all(void)
rcu_read_unlock();
}
+/*
+ * da_monitor_init_prealloc - initialise with a pre-allocated storage pool
+ *
+ * Allocates @prealloc_count storage slots up-front so that da_create_or_get()
+ * and da_destroy_storage() never call kmalloc/kfree. Must be called instead
+ * of da_monitor_init() for monitors that require pool mode.
+ */
+static inline int da_monitor_init_prealloc(unsigned int prealloc_count)
+{
+ hash_init(da_monitor_ht);
+
+ da_pool.storage = kcalloc(prealloc_count, sizeof(*da_pool.storage),
+ GFP_KERNEL);
+ if (!da_pool.storage)
+ return -ENOMEM;
+
+ da_pool.free = kmalloc_array(prealloc_count, sizeof(*da_pool.free),
+ GFP_KERNEL);
+ if (!da_pool.free) {
+ kfree(da_pool.storage);
+ da_pool.storage = NULL;
+ return -ENOMEM;
+ }
+
+ da_pool.free_top = 0;
+ for (unsigned int i = 0; i < prealloc_count; i++)
+ da_pool.free[da_pool.free_top++] = &da_pool.storage[i];
+ return 0;
+}
+
+/*
+ * da_monitor_init - initialise in kmalloc mode (no pre-allocation)
+ */
static inline int da_monitor_init(void)
{
hash_init(da_monitor_ht);
return 0;
}
-static inline void da_monitor_destroy(void)
+static inline void da_monitor_destroy_pool(void)
+{
+ WARN_ON_ONCE(!hash_empty(da_monitor_ht));
+ /*
+ * Wait for all in-flight da_pool_return_cb() callbacks to
+ * complete before freeing da_pool.free. synchronize_rcu() is
+ * not sufficient: it only waits for callbacks registered before
+ * it was called, but call_rcu() from concurrent da_destroy_storage()
+ * calls may have been enqueued later. rcu_barrier() drains every
+ * pending callback.
+ */
+ rcu_barrier();
+ kfree(da_pool.storage);
+ da_pool.storage = NULL;
+ kfree(da_pool.free);
+ da_pool.free = NULL;
+ da_pool.free_top = 0;
+}
+
+static inline void da_monitor_destroy_kmalloc(void)
{
struct da_monitor_storage *mon_storage;
struct hlist_node *tmp;
@@ -534,6 +682,22 @@ static inline void da_monitor_destroy(void)
}
}
+/*
+ * da_monitor_destroy - tear down the per-object monitor
+ *
+ * Pool mode: the hash must already be empty (caller must have drained all
+ * tasks first); calls rcu_barrier() to drain all pending da_pool_return_cb()
+ * callbacks before freeing pool arrays.
+ * Kmalloc mode: drains any remaining entries after synchronize_rcu().
+ */
+static inline void da_monitor_destroy(void)
+{
+ if (da_pool.storage)
+ da_monitor_destroy_pool();
+ else
+ da_monitor_destroy_kmalloc();
+}
+
/*
* Allow the per-object monitors to run allocation manually, necessary if the
* start condition is in a context problematic for allocation (e.g. scheduling).
--
2.25.1
^ permalink raw reply related
* [RFC PATCH v2 03/10] selftests/verification: fix verificationtest-ktap for out-of-tree execution
From: wen.yang @ 2026-05-11 18:24 UTC (permalink / raw)
To: Gabriele Monaco, Steven Rostedt
Cc: linux-trace-kernel, linux-kernel, Wen Yang
In-Reply-To: <cover.1778522945.git.wen.yang@linux.dev>
From: Wen Yang <wen.yang@linux.dev>
verificationtest-ktap used a CWD-relative path (../ftrace/ftracetest)
and a relative argument (../verification) for --rv. This works when
the shell changes into the verification directory first, but breaks
when the script is invoked directly - e.g. by the kselftest runner or
vng - because the working directory is the kernel source root, not the
script's own directory.
Fix this by computing the script's directory from $0 with cd/dirname/pwd
and using absolute paths for both the ftracetest invocation and the --rv
argument. Also export the directory to PATH so that check_requires in
the ftracetest framework can locate helper binaries.
Signed-off-by: Wen Yang <wen.yang@linux.dev>
---
tools/testing/selftests/verification/verificationtest-ktap | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/verification/verificationtest-ktap b/tools/testing/selftests/verification/verificationtest-ktap
index 18f7fe324e2f..456b8578a307 100755
--- a/tools/testing/selftests/verification/verificationtest-ktap
+++ b/tools/testing/selftests/verification/verificationtest-ktap
@@ -5,4 +5,6 @@
#
# Copyright (C) Arm Ltd., 2023
-../ftrace/ftracetest -K -v --rv ../verification
+dir=$(cd "$(dirname "$0")" && pwd)
+export PATH="$dir:$PATH"
+"$dir/../ftrace/ftracetest" -K -v --rv "$dir"
--
2.25.1
^ permalink raw reply related
* [RFC PATCH v2 02/10] rv/da: fix per-task da_monitor_destroy() ordering and sync
From: wen.yang @ 2026-05-11 18:24 UTC (permalink / raw)
To: Gabriele Monaco, Steven Rostedt
Cc: linux-trace-kernel, linux-kernel, Wen Yang
In-Reply-To: <cover.1778522945.git.wen.yang@linux.dev>
From: Wen Yang <wen.yang@linux.dev>
The following two paths race:
CPU 0 (disable_stall/__rv_disable_monitor) CPU 1 (wwnr probe handler)
------------------------------------------ -----------------------------
disable_stall()
da_monitor_destroy()
da_monitor_reset_all() <------ [task T: monitoring=0]
da_monitor_start(&T->rv[n])
/* no timer_setup */
monitoring=1 <----
tracepoint_synchronize_unregister()
// CPU 1 probe has already returned; sync returns
Later, enable_stall() acquires the same slot and calls da_monitor_init():
da_monitor_reset_all()
da_monitor_reset(&T->rv[slot]) // monitoring=1, timer.function==0
ha_monitor_reset_env()
ha_cancel_timer()
timer_delete(&ha_mon->timer) // ODEBUG: timer never initialised
ODEBUG: assert_init not available (active state 0)
object type: timer_list
Call trace: timer_delete <- da_monitor_reset_all <- enable_stall
Call tracepoint_synchronize_unregister() inside da_monitor_destroy()
before da_monitor_reset_all(). The unregister_trace_xxx() calls in the
monitor's disable() have already disconnected the tracepoints; the sync
here drains any handler still in flight, so no new monitoring=1 can
appear after da_monitor_reset_all() clears the slot.
Also fix the slot release ordering: release the slot only after
reset_all() to avoid accessing rv[] with an out-of-bounds index.
Fixes: f5587d1b6ec9 ("rv: Add Hybrid Automata monitor type")
Signed-off-by: Wen Yang <wen.yang@linux.dev>
---
include/rv/da_monitor.h | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index 00ded3d5ab3f..d04bb3229c75 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -304,6 +304,20 @@ static int da_monitor_init(void)
/*
* da_monitor_destroy - return the allocated slot
+ *
+ * Call tracepoint_synchronize_unregister() before reset_all() to close
+ * the race where an in-flight non-HA probe handler sets monitoring=1
+ * (without calling timer_setup()) after da_monitor_reset_all() has
+ * already cleared the slot but before the caller's own sync completes.
+ * Without this barrier, an HA_TIMER_WHEEL monitor that later acquires
+ * the same slot would call timer_delete() on a never-initialised
+ * timer_list, triggering ODEBUG warnings.
+ *
+ * Note: tracepoint_synchronize_unregister() is a system-wide barrier
+ * that waits for all CPUs to finish any in-flight tracepoint handlers.
+ * The caller's own __rv_disable_monitor() issues a second sync after
+ * returning from disable(); that redundant call is harmless on the
+ * infrequent admin (enable/disable) path.
*/
static inline void da_monitor_destroy(void)
{
@@ -311,10 +325,10 @@ static inline void da_monitor_destroy(void)
WARN_ONCE(1, "Disabling a disabled monitor: " __stringify(MONITOR_NAME));
return;
}
+ tracepoint_synchronize_unregister();
+ da_monitor_reset_all();
rv_put_task_monitor_slot(task_mon_slot);
task_mon_slot = RV_PER_TASK_MONITOR_INIT;
-
- da_monitor_reset_all();
}
#elif RV_MON_TYPE == RV_MON_PER_OBJ
--
2.25.1
^ permalink raw reply related
* [RFC PATCH v2 01/10] rv/da: fix monitor start ordering and memory ordering for monitoring flag
From: wen.yang @ 2026-05-11 18:24 UTC (permalink / raw)
To: Gabriele Monaco, Steven Rostedt
Cc: linux-trace-kernel, linux-kernel, Wen Yang
In-Reply-To: <cover.1778522945.git.wen.yang@linux.dev>
From: Wen Yang <wen.yang@linux.dev>
da_monitor_start() set monitoring=1 before calling da_monitor_init_hook(),
may racing with the sched_switch handler:
da_monitor_start() sched_switch handler
------------------------- ---------------------------------
da_mon->monitoring = 1;
if (da_monitoring(da_mon)) /* true */
ha_start_timer_ns(...);
/* hrtimer->base == NULL, crash */
da_monitor_init_hook(da_mon);
/* hrtimer_setup() sets base */
Fix the ordering and pair with release/acquire semantics:
da_monitor_init_hook(da_mon);
smp_store_release(&da_mon->monitoring, 1); /* da_monitor_start() */
return smp_load_acquire(&da_mon->monitoring); /* da_monitoring() */
On ARM64 a plain STR + LDR does not form a release-acquire pair, so
the load can observe monitoring=1 while hrtimer->base is still NULL.
The plain accesses are also data races under KCSAN.
Use WRITE_ONCE for the monitoring=0 store in da_monitor_reset() to
cover the reset path.
Fixes: 792575348ff7 ("rv/include: Add deterministic automata monitor definition via C macros")
Signed-off-by: Wen Yang <wen.yang@linux.dev>
---
include/rv/da_monitor.h | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index 39765ff6f098..00ded3d5ab3f 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -82,7 +82,7 @@ static void react(enum states curr_state, enum events event)
static inline void da_monitor_reset(struct da_monitor *da_mon)
{
da_monitor_reset_hook(da_mon);
- da_mon->monitoring = 0;
+ WRITE_ONCE(da_mon->monitoring, 0);
da_mon->curr_state = model_get_initial_state();
}
@@ -95,8 +95,9 @@ static inline void da_monitor_reset(struct da_monitor *da_mon)
static inline void da_monitor_start(struct da_monitor *da_mon)
{
da_mon->curr_state = model_get_initial_state();
- da_mon->monitoring = 1;
da_monitor_init_hook(da_mon);
+ /* Pairs with smp_load_acquire in da_monitoring(). */
+ smp_store_release(&da_mon->monitoring, 1);
}
/*
@@ -104,7 +105,8 @@ static inline void da_monitor_start(struct da_monitor *da_mon)
*/
static inline bool da_monitoring(struct da_monitor *da_mon)
{
- return da_mon->monitoring;
+ /* Pairs with smp_store_release in da_monitor_start(). */
+ return smp_load_acquire(&da_mon->monitoring);
}
/*
--
2.25.1
^ permalink raw reply related
* Re: [PATCH bpf 1/2] uprobes/x86: Fix red zone clobbering in nop5 optimization
From: Oleg Nesterov @ 2026-05-11 17:24 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, bpf, linux-trace-kernel, jolsa, peterz, mingo,
mhiramat
In-Reply-To: <CAEf4BzZSSsUKGje1mOTOxrRxGfw4sVfyVmKrono1rOdDq1iFHQ@mail.gmail.com>
On 05/11, Andrii Nakryiko wrote:
>
> On Mon, May 11, 2026 at 7:45 AM Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > On 05/08, Andrii Nakryiko wrote:
> > >
> > > +static bool resolve_uprobe_addr(unsigned long ip, unsigned long *probe_addr)
> > > {
> > > - struct vm_area_struct *vma = vma_lookup(current->mm, ip);
> > > + struct uprobes_state *state = ¤t->mm->uprobes_state;
> >
> > it seems that there is a problem, with or without this change...
> >
> > Lets forget about this patch for the moment, I am still trying to understand it.
> >
> > What if register_for_each_vma() calls install_breakpoint(vma) and
> > vma->mm != current->mm ?
> >
> > In this case install_breakpoint() path will call __is_optimized() and then
> > __in_uprobe_trampoline() which does vma_lookup(current->mm). This looks
> > obviously wrong ?
> >
> > And unless I am totally confused, this patch "inherits" the problem...
> >
> > No?
> >
>
> Yes, sashiko called this out as well. I'm going to pass mm explicitly
> into is_optimized and all the way to resolve_uprobe_addr(), easy fix.
Aah... found it: https://sashiko.dev/#/patchset/20260509003146.976844-1-andrii%40kernel.org
And note that sashiko is more clever than me; I didn't realize that _unregister()
has more problems with current->mm :/
> But we should have a stand-alone easily backportable fix for this even
> without my changes, yes. I can add it as pre-patch or send separately.
Agreed. This deserves another/simple fix. If nothing else, for documentation.
Oleg.
^ permalink raw reply
* Re: [PATCH bpf 1/2] uprobes/x86: Fix red zone clobbering in nop5 optimization
From: Andrii Nakryiko @ 2026-05-11 16:56 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andrii Nakryiko, bpf, linux-trace-kernel, jolsa, peterz, mingo,
mhiramat
In-Reply-To: <agHreV1m7MRKUtbh@redhat.com>
On Mon, May 11, 2026 at 7:45 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 05/08, Andrii Nakryiko wrote:
> >
> > +static bool resolve_uprobe_addr(unsigned long ip, unsigned long *probe_addr)
> > {
> > - struct vm_area_struct *vma = vma_lookup(current->mm, ip);
> > + struct uprobes_state *state = ¤t->mm->uprobes_state;
>
> it seems that there is a problem, with or without this change...
>
> Lets forget about this patch for the moment, I am still trying to understand it.
>
> What if register_for_each_vma() calls install_breakpoint(vma) and
> vma->mm != current->mm ?
>
> In this case install_breakpoint() path will call __is_optimized() and then
> __in_uprobe_trampoline() which does vma_lookup(current->mm). This looks
> obviously wrong ?
>
> And unless I am totally confused, this patch "inherits" the problem...
>
> No?
>
Yes, sashiko called this out as well. I'm going to pass mm explicitly
into is_optimized and all the way to resolve_uprobe_addr(), easy fix.
But we should have a stand-alone easily backportable fix for this even
without my changes, yes. I can add it as pre-patch or send separately.
> Oleg.
>
^ permalink raw reply
* Re: [PATCH bpf 1/2] uprobes/x86: Fix red zone clobbering in nop5 optimization
From: Andrii Nakryiko @ 2026-05-11 16:41 UTC (permalink / raw)
To: Jiri Olsa
Cc: Andrii Nakryiko, bpf, linux-trace-kernel, oleg, peterz, mingo,
mhiramat
In-Reply-To: <agD3xq2MUyskd7X-@krava>
On Sun, May 10, 2026 at 2:25 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Fri, May 08, 2026 at 05:30:56PM -0700, Andrii Nakryiko wrote:
> > The x86 uprobe nop5 optimization currently replaces a 5-byte NOP at the
> > probe site with a CALL into a uprobe trampoline. CALL pushes a return
> > address to [rsp-8]. On x86-64 this is inside the 128-byte red zone, where
> > user code may keep temporary data without adjusting rsp.
> >
> > Use a 5-byte JMP instead. JMP does not write to the user stack, but it
> > also does not provide a return address. Replace the single trampoline
> > entry with a page of 16-byte slots. Each optimized probe jumps to its
> > assigned slot, the slot moves rsp below the red zone, saves the registers
> > clobbered by syscall, and invokes the uprobe syscall:
> >
> > Probe site: jmp slot_N (5B, replaces nop5)
> >
> > Slot N: lea -128(%rsp), %rsp (5B) skip red zone
> > push %rcx (1B) save (syscall clobbers)
> > push %r11 (2B) save (syscall clobbers)
> > push %rax (1B) save (syscall uses for nr)
> > mov $336, %eax (5B) uprobe syscall number
> > syscall (2B)
> >
> > All slots contain identical code at different offsets, so the trampoline
> > page is generated once at boot and mapped read-execute into each process.
> > The syscall handler identifies the slot from regs->ip, which points just
> > after the syscall instruction, and uses a per-mm slot table to recover the
> > original probe address.
> >
> > The uprobe syscall does not return to the trampoline slot. The handler
> > restores the probe-site register state, runs the uprobe consumers, sets
> > pt_regs to continue at probe_addr + 5 unless a consumer redirected
> > execution, and returns directly through the IRET path. This preserves
> > general purpose registers, including rcx and r11, without requiring any
> > post-syscall cleanup code in the trampoline and avoids call/ret, RSB, and
> > shadow stack concerns.
> >
> > Protect the per-mm trampoline list with RCU and free trampoline metadata
> > with kfree_rcu(). This lets the syscall path resolve trampoline slots
> > without taking mmap_lock. The optimized-instruction detection path also
> > walks the trampoline list under an RCU read-side lock. Since that path
> > starts from the JMP target, it translates the slot start to the post-syscall
> > IP expected by the shared resolver before checking the trampoline mapping.
> >
> > Each trampoline page provides 256 slots. Slots stay permanently assigned
> > to their first probe address and are reused only when the same address is
> > probed again. Reassigning detached slots is deliberately avoided because a
> > thread can remain in a trampoline for an unbounded time due to ptrace,
> > interrupts, or scheduling delays. If a reachable trampoline page runs out
> > of slots, probes that cannot allocate a slot fall back to the slower INT3
> > path.
> >
> > Require the entire trampoline page to be reachable by a rel32 JMP before
> > reusing it for a probe. This keeps every slot in the page within the range
> > that can be encoded at the probe site.
> >
> > Change the error code returned when the uprobe syscall is invoked outside
> > a kernel-generated trampoline from -ENXIO to -EPROTO. This lets libbpf and
> > similar libraries distinguish fixed kernels from kernels with the
> > red-zone-clobbering implementation and enable nop5 optimization only on
> > fixed kernels.
> >
> > Performance (usdt single-thread, M/s):
> >
> > usdt-nop usdt-nop5-base usdt-nop5-fix nop5-change iret%
> > Skylake 3.149 6.422 4.865 -24.3% 39.1%
> > Milan 2.910 3.443 3.820 +11.0% 24.3%
> > Sapphire Rapids 1.896 4.023 3.693 -8.2% 24.9%
> > Bergamo 3.393 3.895 3.849 -1.2% 24.5%
> >
> > The fixed nop5 path remains faster than the non-optimized INT3 path on all
> > measured systems. The regression relative to the old CALL-based trampoline
> > comes from IRET being more expensive than SYSRET, most noticeably on older
> > Intel Skylake. Newer Intel CPUs and tested AMD CPUs have lower IRET cost,
> > and AMD Milan improves because removing mmap_lock from the hot path more
> > than offsets the IRET cost.
> >
> > Multi-threaded throughput scales nearly linearly with the number of CPUs, like
> > it used to, thanks to lockless RCU-protected uprobe trampoline lookup.
>
> hi,
> thanks a lot for the fix
>
> FWIW we discussed also an option to have 10-bytes nop and do:
> [rsp+0x80, call trampoline]
>
> we would not need the slots re-use logic, but not sure what other
> surprises there are with 10-bytes nop
>
> I tried that change [1], it seems to work, but it has other
> difficulties, like I think the unoptimized path needs to do:
> [rsp+0x80, call trampoline] -> [jmp end of 10-bytes nop]
> instead of patching back the 10-byte nop, because some thread
> could be inside the nop area already.
>
Yeah, nop10 and this jump-over-nop10 approach is an alternative. I
don't have strong feelings apart from the ridiculousness of a 10-byte
nop :)
did you get a chance to benchmark your nop10 approach, curious how do
the number look like
> jirka
>
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git/commit/?h=redzone_fix&id=74b09240289dba8368c2783b771e678b2cc31574
>
> >
> > Fixes: ba2bfc97b462 ("uprobes/x86: Add support to optimize uprobes")
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> > arch/x86/include/asm/uprobes.h | 18 ++
> > arch/x86/kernel/uprobes.c | 262 ++++++++++--------
> > tools/lib/bpf/features.c | 8 +-
> > .../selftests/bpf/prog_tests/uprobe_syscall.c | 5 +-
> > tools/testing/selftests/bpf/prog_tests/usdt.c | 2 +-
> > 5 files changed, 181 insertions(+), 114 deletions(-)
> >
[...]
^ permalink raw reply
* Re: [PATCH 0/2] tools/bootconfig: render kernel.* subtree as a cmdline string
From: Breno Leitao @ 2026-05-11 16:38 UTC (permalink / raw)
To: Andrew Morton
Cc: Masami Hiramatsu, linux-kernel, linux-trace-kernel, paulmck, oss,
kernel-team
In-Reply-To: <20260508145641.754e4c7f73eabad0b5f89a1e@linux-foundation.org>
On Fri, May 08, 2026 at 02:56:41PM -0700, Andrew Morton wrote:
> On Fri, 08 May 2026 06:55:02 -0700 Breno Leitao <leitao@debian.org> wrote:
>
> > Add a bootconfig -> kernel cmdline rendering capability shared between
> > the kernel parser library and the userspace tools/bootconfig binary.
> >
> > The new userspace mode "tools/bootconfig -C <file>" walks a bootconfig
> > file's "kernel" subtree and prints it as a flat, space-separated
> > cmdline string suitable for direct use as (or appending to) a kernel
> > command line.
> >
> > This series prepares tools/bootconfig and lib/bootconfig.c for an
> > upcoming feature that lets the kernel build render an embedded
> > bootconfig file's "kernel" subtree to a flat cmdline string and embed
> > it in the kernel image.
> >
> > The follow-up series (sent separately) wires this into setup_arch() so
> > early_param() handlers see values supplied via CONFIG_BOOT_CONFIG_EMBED_FILE,
> > following Masami suggestion in [1]
> >
> > These two patches are pure groundwork. They add no kernel feature,
> > change no runtime behavior, and are useful on their own (the new
> > "tools/bootconfig -C" mode lets anyone render a .bootconfig file to
> > a cmdline string from the shell).
> >
> > Landing them independently lets the follow-up series focus on the
> > kernel-side plumbing without dragging the refactor and tool addition
> > through the same review cycle.
>
> I'll assume that Masami will process this, although
> `scripts/get_maintainer.pl lib/bootconfig.c' doesn't mention a git
> tree.
>
> https://sashiko.dev/#/patchset/20260508-bootconfig_using_tools-v1-0-1132219aa773@debian.org
> says a bunch of picky things which seem pretty ignorable to me. Your
> call ;)
Well, these are some warnings about not checking that the output was
properly set. From my view, these are not new to this patch, it is just the
pattern we see, which is fire-and-forget writes to stdout, which seems
quite reasonable.
If we decide to fix this, it would make more sense to do it file-wide
instead of just in this patch code.
^ permalink raw reply
* Re: [PATCH v19 0/7] ring-buffer: Making persistent ring buffers robust
From: Steven Rostedt @ 2026-05-11 16:29 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Catalin Marinas, Will Deacon, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, Ian Rogers, linux-arm-kernel
In-Reply-To: <20260507131416.35bc304d89f974ae3b33d20b@kernel.org>
On Thu, 7 May 2026 13:14:16 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > I'll test this some more, and make a proper patch.
>
> Ah, indeed. Thanks for fixing!
>
> BTW, shouldn't we unify common logic of those functions?
Hmm, there's not much common between the two. One is a consuming read and
the other is a non-consuming read that needs to test for a bunch of race
conditions.
If you see something that can be shared, I'm all for it.
-- Steve
^ permalink raw reply
* Re: [PATCH v14 05/19] unwind_user/sframe: Add support for reading .sframe contents
From: Jens Remus @ 2026-05-11 16:16 UTC (permalink / raw)
To: Steven Rostedt, Josh Poimboeuf, Indu Bhagat, Dylan Hatch
Cc: bpf, linux-mm, linux-kernel, linux-trace-kernel, x86,
Namhyung Kim, Andrii Nakryiko, Jose E. Marchesi, Beau Belgrave,
Florian Weimer, Carlos O'Donell, Masami Hiramatsu, Jiri Olsa,
Arnaldo Carvalho de Melo, Andrew Morton, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Heiko Carstens, Vasily Gorbik,
Ilya Leoshkevich, Steven Rostedt (Google), Peter Zijlstra,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Mathieu Desnoyers, Kees Cook, Sam James
In-Reply-To: <20260505121718.3572346-6-jremus@linux.ibm.com>
On 5/5/2026 2:17 PM, Jens Remus wrote:
> From: Josh Poimboeuf <jpoimboe@kernel.org>
>
> In preparation for using sframe to unwind user space stacks, add an
> sframe_find() interface for finding the sframe information associated
> with a given text address.
>
> For performance, use user_read_access_begin() and the corresponding
> unsafe_*() accessors. Note that use of pr_debug() in uaccess-enabled
> regions would break noinstr validation, so there aren't any debug
> messages yet. That will be added in a subsequent commit.
>
> Link: https://lore.kernel.org/all/77c0d1ec143bf2a53d66c4ecb190e7e0a576fbfd.1737511963.git.jpoimboe@kernel.org/
> Link: https://lore.kernel.org/all/b35ca3a3-8de5-4d32-8d30-d4e562f6b0de@linux.ibm.com/
>
> [ Jens Remus: Add initial support for SFrame V3 (limited to regular
> FDEs). Add support for PC-relative FDE function start offset. Simplify
> logic by using an internal FDE representation. Rename struct sframe_fre
> to sframe_fre_internal to align with struct sframe_fde_internal.
> Cleanup includes. Fix checkpatch errors "spaces required around that
> ':'". ]
> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
It just occurred to me that that the use of the return values -EINVAL
and -EFAULT is inconsistent across __read_fde() and __read_fre():
-EINVAL is to be used when a lookup for an IP fails (i.e. cannot be
satisfied) and
-EFAULT is to be used when the .sframe is corrupted/inconsistent and
should be unregistered.
> +static __always_inline int __read_fde(struct sframe_section *sec,
> + unsigned int fde_num,
> + struct sframe_fde_internal *fde)
> +{
> + unsigned long fde_addr, fda_addr, func_addr;
> + struct sframe_fde_v3 _fde;
> + struct sframe_fda_v3 _fda;
> +
> + fde_addr = sec->fdes_start + (fde_num * sizeof(struct sframe_fde_v3));
> + unsafe_copy_from_user(&_fde, (void __user *)fde_addr,
> + sizeof(struct sframe_fde_v3), Efault);
> +
> + func_addr = fde_addr + _fde.func_start_off;
> + if (func_addr < sec->text_start || func_addr >= sec->text_end)
> + return -EINVAL;
return -EFAULT;
> +
> + fda_addr = sec->fres_start + _fde.fres_off;
> + if (fda_addr + sizeof(struct sframe_fda_v3) > sec->fres_end)
> + return -EINVAL;
return -EFAULT;
> + unsafe_copy_from_user(&_fda, (void __user *)fda_addr,
> + sizeof(struct sframe_fda_v3), Efault);
> +
> + fde->func_addr = func_addr;
> + fde->func_size = _fde.func_size;
> + fde->fda_off = _fde.fres_off;
> + fde->fres_off = _fde.fres_off + sizeof(struct sframe_fda_v3);
> + fde->fres_num = _fda.fres_num;
> + fde->info = _fda.info;
> + fde->info2 = _fda.info2;
> + fde->rep_size = _fda.rep_size;
> +
> + return 0;
> +
> +Efault:
> + return -EFAULT;
> +}
> +static __always_inline int __read_fre(struct sframe_section *sec,
> + struct sframe_fde_internal *fde,
> + unsigned long fre_addr,
> + struct sframe_fre_internal *fre)
> +{
> + unsigned char fde_type = SFRAME_V3_FDE_TYPE(fde->info2);
> + unsigned char fde_pctype = SFRAME_V3_FDE_PCTYPE(fde->info);
> + unsigned char fre_type = SFRAME_V3_FDE_FRE_TYPE(fde->info);
> + unsigned char dataword_count, dataword_size;
> + s32 cfa_off, ra_off, fp_off;
> + unsigned long cur = fre_addr;
> + unsigned char addr_size;
> + u32 ip_off;
> + u8 info;
> +
> + addr_size = fre_type_to_size(fre_type);
> + if (!addr_size)
> + return -EFAULT;
> +
> + if (fre_addr + addr_size + 1 > sec->fres_end)
> + return -EFAULT;
> +
> + UNSAFE_GET_USER_INC(ip_off, cur, addr_size, Efault);
> + if (fde_pctype == SFRAME_FDE_PCTYPE_INC && ip_off > fde->func_size)
> + return -EFAULT;
> +
> + UNSAFE_GET_USER_INC(info, cur, 1, Efault);
> + dataword_count = SFRAME_V3_FRE_DATAWORD_COUNT(info);
> + dataword_size = dataword_size_enum_to_size(SFRAME_V3_FRE_DATAWORD_SIZE(info));
> + if (!dataword_count || !dataword_size)
> + return -EFAULT;
> +
> + if (cur + (dataword_count * dataword_size) > sec->fres_end)
> + return -EFAULT;
> +
> + /* TODO: Support for flexible FDEs not implemented yet. */
> + if (fde_type != SFRAME_FDE_TYPE_DEFAULT)
> + return -EFAULT;
> +
> + UNSAFE_GET_USER_INC(cfa_off, cur, dataword_size, Efault);
> + dataword_count--;
> +
> + ra_off = sec->ra_off;
> + if (!ra_off) {
> + if (!dataword_count--)
> + return -EFAULT;
> +
> + UNSAFE_GET_USER_INC(ra_off, cur, dataword_size, Efault);
> + }
> +
> + fp_off = sec->fp_off;
> + if (!fp_off && dataword_count) {
> + dataword_count--;
> + UNSAFE_GET_USER_INC(fp_off, cur, dataword_size, Efault);
> + }
> +
> + if (dataword_count)
> + return -EFAULT;
> +
> + fre->size = addr_size + 1 + (dataword_count * dataword_size);
> + fre->ip_off = ip_off;
> + fre->cfa_off = cfa_off;
> + fre->ra_off = ra_off;
> + fre->fp_off = fp_off;
> + fre->info = info;
> +
> + return 0;
> +
> +Efault:
> + return -EFAULT;
> +}
Similar for sframe_init_cfa_rule_data() (and sframe_init_rule_data())
that get introduced with subsequent commits.
Regards,
Jens
--
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com
IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
^ permalink raw reply
* [PATCH v6 4/4] Documentation: document panic_on_unrecoverable_memory_failure sysctl
From: Breno Leitao @ 2026-05-11 15:38 UTC (permalink / raw)
To: Miaohe Lin, Naoya Horiguchi, Andrew Morton, Jonathan Corbet,
Shuah Khan, David Hildenbrand, Lorenzo Stoakes, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Shuah Khan,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Liam R. Howlett
Cc: linux-mm, linux-kernel, linux-doc, linux-kselftest, Breno Leitao,
linux-trace-kernel, kernel-team
In-Reply-To: <20260511-ecc_panic-v6-0-183012ba7d4b@debian.org>
Add documentation for the new vm.panic_on_unrecoverable_memory_failure
sysctl, describing which failures trigger a panic (kernel-owned pages
the handler cannot recover) and which are intentionally left out
(transient allocator races and unclassified pages).
Signed-off-by: Breno Leitao <leitao@debian.org>
---
Documentation/admin-guide/sysctl/vm.rst | 70 +++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/Documentation/admin-guide/sysctl/vm.rst b/Documentation/admin-guide/sysctl/vm.rst
index 97e12359775c9..802c51ba8c43b 100644
--- a/Documentation/admin-guide/sysctl/vm.rst
+++ b/Documentation/admin-guide/sysctl/vm.rst
@@ -67,6 +67,7 @@ Currently, these files are in /proc/sys/vm:
- page-cluster
- page_lock_unfairness
- panic_on_oom
+- panic_on_unrecoverable_memory_failure
- percpu_pagelist_high_fraction
- stat_interval
- stat_refresh
@@ -925,6 +926,75 @@ panic_on_oom=2+kdump gives you very strong tool to investigate
why oom happens. You can get snapshot.
+panic_on_unrecoverable_memory_failure
+======================================
+
+When a hardware memory error (e.g. multi-bit ECC) hits a kernel page
+that cannot be recovered by the memory failure handler, the default
+behaviour is to ignore the error and continue operation. This is
+dangerous because the corrupted data remains accessible to the kernel,
+risking silent data corruption or a delayed crash when the poisoned
+memory is next accessed.
+
+When enabled, this sysctl triggers a panic on kernel-owned pages that
+the memory failure handler cannot recover: reserved pages
+(``PageReserved``) and stable kernel pages that hwpoison cannot handle
+(slab, vmalloc, page tables, kernel stacks, and similar non-LRU,
+non-buddy pages).
+
+Other failure paths are intentionally left out because they can be
+reached by transient races with the page allocator (an in-flight
+buddy allocation has refcount 0 and is no longer on the buddy free
+list, briefly), and panicking on them would risk killing the box for
+a page that was actually destined for userspace where the standard
+SIGBUS recovery path applies. Pages whose state could not be
+classified at all are also not covered, since an unknown state is
+not a sound basis for a panic decision.
+
+For many environments it is preferable to panic immediately with a clean
+crash dump that captures the original error context, rather than to
+continue and face a random crash later whose cause is difficult to
+diagnose.
+
+Use cases
+---------
+
+This option is most useful in environments where unattributed crashes
+are expensive to debug or where data integrity must take precedence
+over availability:
+
+* Large fleets, where multi-bit ECC errors on kernel pages are observed
+ regularly and post-mortem analysis of an unrelated downstream crash
+ (often seconds to minutes after the original error) consumes
+ significant engineering effort.
+
+* Systems configured with kdump, where panicking at the moment of the
+ hardware error produces a vmcore that still contains the faulting
+ address, the affected page state, and the originating MCE/GHES
+ record — context that is typically lost by the time a delayed crash
+ occurs.
+
+* High-availability clusters that rely on fast, deterministic node
+ failure for failover, and prefer an immediate panic over silent data
+ corruption propagating to replicas or persistent storage.
+
+* Kernel and platform developers reproducing hwpoison issues with
+ tools such as ``mce-inject`` or error-injection debugfs interfaces,
+ where panicking on the unrecoverable path makes regressions
+ immediately visible instead of surfacing as later, unrelated
+ failures.
+
+= =====================================================================
+0 Try to continue operation (default).
+1 Panic immediately. If the ``panic`` sysctl is also non-zero then the
+ machine will be rebooted.
+= =====================================================================
+
+Example::
+
+ echo 1 > /proc/sys/vm/panic_on_unrecoverable_memory_failure
+
+
percpu_pagelist_high_fraction
=============================
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v6 3/4] mm/memory-failure: add panic option for unrecoverable pages
From: Breno Leitao @ 2026-05-11 15:38 UTC (permalink / raw)
To: Miaohe Lin, Naoya Horiguchi, Andrew Morton, Jonathan Corbet,
Shuah Khan, David Hildenbrand, Lorenzo Stoakes, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Shuah Khan,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Liam R. Howlett
Cc: linux-mm, linux-kernel, linux-doc, linux-kselftest, Breno Leitao,
linux-trace-kernel, kernel-team
In-Reply-To: <20260511-ecc_panic-v6-0-183012ba7d4b@debian.org>
Add a sysctl panic_on_unrecoverable_memory_failure that triggers a
kernel panic when memory_failure() encounters pages that cannot be
recovered. This provides a clean crash with useful debug information
rather than allowing silent data corruption or a delayed crash at an
unrelated code path.
Panic eligibility is intentionally narrow: only MF_MSG_KERNEL with
result == MF_IGNORED panics. That covers reserved pages (PageReserved)
and the kernel page types that the prior patch promotes from
MF_MSG_GET_HWPOISON via MF_GET_PAGE_UNHANDLABLE — slab, vmalloc, page
tables, kernel stacks, and similar non-LRU/non-buddy kernel-owned pages.
All other action types are excluded:
- MF_MSG_GET_HWPOISON and MF_MSG_KERNEL_HIGH_ORDER can be reached by
transient refcount races with the page allocator (an in-flight buddy
allocation has refcount 0 and is no longer on the buddy free list,
briefly), and panicking on them would risk killing the box for what
is actually a recoverable userspace page.
- MF_MSG_UNKNOWN means identify_page_state() could not classify the
page; that is precisely the wrong basis for a panic decision.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
mm/memory-failure.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 4210173060aac..e4a9ceacaf36b 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -74,6 +74,8 @@ static int sysctl_memory_failure_recovery __read_mostly = 1;
static int sysctl_enable_soft_offline __read_mostly = 1;
+static int sysctl_panic_on_unrecoverable_mf __read_mostly;
+
atomic_long_t num_poisoned_pages __read_mostly = ATOMIC_LONG_INIT(0);
static bool hw_memory_failure __read_mostly = false;
@@ -155,6 +157,15 @@ static const struct ctl_table memory_failure_table[] = {
.proc_handler = proc_dointvec_minmax,
.extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_ONE,
+ },
+ {
+ .procname = "panic_on_unrecoverable_memory_failure",
+ .data = &sysctl_panic_on_unrecoverable_mf,
+ .maxlen = sizeof(sysctl_panic_on_unrecoverable_mf),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
}
};
@@ -1281,6 +1292,18 @@ static void update_per_node_mf_stats(unsigned long pfn,
++mf_stats->total;
}
+static bool panic_on_unrecoverable_mf(enum mf_action_page_type type,
+ enum mf_result result)
+{
+ if (!sysctl_panic_on_unrecoverable_mf || result != MF_IGNORED)
+ return false;
+
+ if (type == MF_MSG_KERNEL)
+ return true;
+
+ return false;
+}
+
/*
* "Dirty/Clean" indication is not 100% accurate due to the possibility of
* setting PG_dirty outside page lock. See also comment above set_page_dirty().
@@ -1298,6 +1321,9 @@ static int action_result(unsigned long pfn, enum mf_action_page_type type,
pr_err("%#lx: recovery action for %s: %s\n",
pfn, action_page_types[type], action_name[result]);
+ if (panic_on_unrecoverable_mf(type, result))
+ panic("Memory failure: %#lx: unrecoverable page", pfn);
+
return (result == MF_RECOVERED || result == MF_DELAYED) ? 0 : -EBUSY;
}
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v6 2/4] mm/memory-failure: classify get_any_page() failures by reason
From: Breno Leitao @ 2026-05-11 15:38 UTC (permalink / raw)
To: Miaohe Lin, Naoya Horiguchi, Andrew Morton, Jonathan Corbet,
Shuah Khan, David Hildenbrand, Lorenzo Stoakes, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Shuah Khan,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Liam R. Howlett
Cc: linux-mm, linux-kernel, linux-doc, linux-kselftest, Breno Leitao,
linux-trace-kernel, kernel-team, Lance Yang
In-Reply-To: <20260511-ecc_panic-v6-0-183012ba7d4b@debian.org>
When get_any_page() fails to grab a page reference, the *reason* it
failed is known at the call site but is not surfaced to callers: the
HWPoisonHandlable() rejection path (a stable kernel page hwpoison cannot
handle — slab, vmalloc, page tables, kernel stacks, ...) and the
page_count() / put_page race paths (a transient page-allocator lifecycle
race) all collapse to a single negative errno by the time
memory_failure() sees them. memory_failure() can only observe the
conflated result and reports both as MF_MSG_GET_HWPOISON.
Surface the diagnosis explicitly. Add an mf_get_page_status enum,
plumbed out through get_any_page() and get_hwpoison_page() (NULL is
accepted by callers that do not care — unpoison_memory() and
soft_offline_page() pass NULL). get_any_page() sets the status at the
moment it gives up:
MF_GET_PAGE_UNHANDLABLE — HWPoisonHandlable() rejected the page
after retries.
MF_GET_PAGE_RACE — exhausted retries on a refcount /
lifecycle race with the allocator.
memory_failure() then promotes the unhandlable case to MF_MSG_KERNEL
alongside the existing PageReserved branch, and leaves the
transient-race case as MF_MSG_GET_HWPOISON. This forms the foundation
a later patch will rely on to decide whether an unrecoverable failure
should panic.
Drop the "reserved" qualifier from action_page_types[MF_MSG_KERNEL]
and the matching tracepoint string in MF_PAGE_TYPE: the enum value
now covers both PageReserved pages and unhandlable kernel pages
(slab, vmalloc, page tables, kernel stacks, ...), so "kernel page"
is the accurate label for both populations.
Suggested-by: Lance Yang <lance.yang@linux.dev>
Signed-off-by: Breno Leitao <leitao@debian.org>
---
include/trace/events/memory-failure.h | 2 +-
mm/memory-failure.c | 46 +++++++++++++++++++++++++++++------
2 files changed, 39 insertions(+), 9 deletions(-)
diff --git a/include/trace/events/memory-failure.h b/include/trace/events/memory-failure.h
index aa57cc8f896be..8a860e6fcb4e9 100644
--- a/include/trace/events/memory-failure.h
+++ b/include/trace/events/memory-failure.h
@@ -24,7 +24,7 @@
EMe ( MF_RECOVERED, "Recovered" )
#define MF_PAGE_TYPE \
- EM ( MF_MSG_KERNEL, "reserved kernel page" ) \
+ EM ( MF_MSG_KERNEL, "kernel page" ) \
EM ( MF_MSG_KERNEL_HIGH_ORDER, "high-order kernel page" ) \
EM ( MF_MSG_HUGE, "huge page" ) \
EM ( MF_MSG_FREE_HUGE, "free huge page" ) \
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index f112fb27a8ff6..4210173060aac 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -878,7 +878,7 @@ static const char *action_name[] = {
};
static const char * const action_page_types[] = {
- [MF_MSG_KERNEL] = "reserved kernel page",
+ [MF_MSG_KERNEL] = "kernel page",
[MF_MSG_KERNEL_HIGH_ORDER] = "high-order kernel page",
[MF_MSG_HUGE] = "huge page",
[MF_MSG_FREE_HUGE] = "free huge page",
@@ -1389,11 +1389,29 @@ static int __get_hwpoison_page(struct page *page, unsigned long flags)
#define GET_PAGE_MAX_RETRY_NUM 3
-static int get_any_page(struct page *p, unsigned long flags)
+enum mf_get_page_status {
+ MF_GET_PAGE_OK = 0,
+ MF_GET_PAGE_RACE,
+ MF_GET_PAGE_UNHANDLABLE,
+};
+
+static void set_mf_get_page_status(enum mf_get_page_status *gp_status,
+ enum mf_get_page_status value)
+{
+ if (!gp_status)
+ return;
+
+ *gp_status = value;
+}
+
+static int get_any_page(struct page *p, unsigned long flags,
+ enum mf_get_page_status *gp_status)
{
int ret = 0, pass = 0;
bool count_increased = false;
+ set_mf_get_page_status(gp_status, MF_GET_PAGE_OK);
+
if (flags & MF_COUNT_INCREASED)
count_increased = true;
@@ -1406,11 +1424,13 @@ static int get_any_page(struct page *p, unsigned long flags)
if (pass++ < GET_PAGE_MAX_RETRY_NUM)
goto try_again;
ret = -EBUSY;
+ set_mf_get_page_status(gp_status, MF_GET_PAGE_RACE);
} else if (!PageHuge(p) && !is_free_buddy_page(p)) {
/* We raced with put_page, retry. */
if (pass++ < GET_PAGE_MAX_RETRY_NUM)
goto try_again;
ret = -EIO;
+ set_mf_get_page_status(gp_status, MF_GET_PAGE_RACE);
}
goto out;
} else if (ret == -EBUSY) {
@@ -1423,6 +1443,7 @@ static int get_any_page(struct page *p, unsigned long flags)
goto try_again;
}
ret = -EIO;
+ set_mf_get_page_status(gp_status, MF_GET_PAGE_UNHANDLABLE);
goto out;
}
}
@@ -1442,6 +1463,7 @@ static int get_any_page(struct page *p, unsigned long flags)
}
put_page(p);
ret = -EIO;
+ set_mf_get_page_status(gp_status, MF_GET_PAGE_UNHANDLABLE);
}
out:
if (ret == -EIO)
@@ -1480,6 +1502,7 @@ static int __get_unpoison_page(struct page *page)
* get_hwpoison_page() - Get refcount for memory error handling
* @p: Raw error page (hit by memory error)
* @flags: Flags controlling behavior of error handling
+ * @gp_status: Optional output for the reason get_any_page() failed
*
* get_hwpoison_page() takes a page refcount of an error page to handle memory
* error on it, after checking that the error page is in a well-defined state
@@ -1503,7 +1526,8 @@ static int __get_unpoison_page(struct page *page)
* operations like allocation and free,
* -EHWPOISON when the page is hwpoisoned and taken off from buddy.
*/
-static int get_hwpoison_page(struct page *p, unsigned long flags)
+static int get_hwpoison_page(struct page *p, unsigned long flags,
+ enum mf_get_page_status *gp_status)
{
int ret;
@@ -1511,7 +1535,7 @@ static int get_hwpoison_page(struct page *p, unsigned long flags)
if (flags & MF_UNPOISON)
ret = __get_unpoison_page(p);
else
- ret = get_any_page(p, flags);
+ ret = get_any_page(p, flags, gp_status);
zone_pcp_enable(page_zone(p));
return ret;
@@ -2349,6 +2373,7 @@ int memory_failure(unsigned long pfn, int flags)
bool retry = true;
int hugetlb = 0;
bool is_reserved;
+ enum mf_get_page_status gp_status = MF_GET_PAGE_OK;
if (!sysctl_memory_failure_recovery)
panic("Memory failure on page %lx", pfn);
@@ -2424,7 +2449,7 @@ int memory_failure(unsigned long pfn, int flags)
*/
is_reserved = PageReserved(p);
- res = get_hwpoison_page(p, flags);
+ res = get_hwpoison_page(p, flags, &gp_status);
if (!res) {
if (is_free_buddy_page(p)) {
if (take_page_off_buddy(p)) {
@@ -2445,7 +2470,12 @@ int memory_failure(unsigned long pfn, int flags)
}
goto unlock_mutex;
} else if (res < 0) {
- if (is_reserved)
+ /*
+ * Promote a stable unhandlable kernel page diagnosed by
+ * get_hwpoison_page() to MF_MSG_KERNEL alongside reserved
+ * pages; transient lifecycle races stay as MF_MSG_GET_HWPOISON.
+ */
+ if (is_reserved || gp_status == MF_GET_PAGE_UNHANDLABLE)
res = action_result(pfn, MF_MSG_KERNEL, MF_IGNORED);
else
res = action_result(pfn, MF_MSG_GET_HWPOISON,
@@ -2750,7 +2780,7 @@ int unpoison_memory(unsigned long pfn)
goto unlock_mutex;
}
- ghp = get_hwpoison_page(p, MF_UNPOISON);
+ ghp = get_hwpoison_page(p, MF_UNPOISON, NULL);
if (!ghp) {
if (folio_test_hugetlb(folio)) {
huge = true;
@@ -2957,7 +2987,7 @@ int soft_offline_page(unsigned long pfn, int flags)
retry:
get_online_mems();
- ret = get_hwpoison_page(page, flags | MF_SOFT_OFFLINE);
+ ret = get_hwpoison_page(page, flags | MF_SOFT_OFFLINE, NULL);
put_online_mems();
if (hwpoison_filter(page)) {
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v6 1/4] mm/memory-failure: report MF_MSG_KERNEL for reserved pages
From: Breno Leitao @ 2026-05-11 15:38 UTC (permalink / raw)
To: Miaohe Lin, Naoya Horiguchi, Andrew Morton, Jonathan Corbet,
Shuah Khan, David Hildenbrand, Lorenzo Stoakes, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Shuah Khan,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Liam R. Howlett
Cc: linux-mm, linux-kernel, linux-doc, linux-kselftest, Breno Leitao,
linux-trace-kernel, kernel-team, Lance Yang
In-Reply-To: <20260511-ecc_panic-v6-0-183012ba7d4b@debian.org>
When get_hwpoison_page() returns a negative value, distinguish
reserved pages from other failure cases by reporting MF_MSG_KERNEL
instead of MF_MSG_GET_HWPOISON. Reserved pages belong to the kernel
and should be classified accordingly for proper handling.
Sample PG_reserved before the get_hwpoison_page() call. In the
MF_COUNT_INCREASED path get_any_page() can drop the caller's
reference before returning -EIO, after which the underlying page may
have been freed and reallocated with page->flags reset; reading
PageReserved(p) at that point would observe stale or unrelated state.
The pre-call snapshot reflects what the page actually was at the
time of the failure event.
Acked-by: Miaohe Lin <linmiaohe@huawei.com>
Reviewed-by: Lance Yang <lance.yang@linux.dev>
Signed-off-by: Breno Leitao <leitao@debian.org>
---
mm/memory-failure.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 866c4428ac7ef..f112fb27a8ff6 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2348,6 +2348,7 @@ int memory_failure(unsigned long pfn, int flags)
unsigned long page_flags;
bool retry = true;
int hugetlb = 0;
+ bool is_reserved;
if (!sysctl_memory_failure_recovery)
panic("Memory failure on page %lx", pfn);
@@ -2411,6 +2412,18 @@ int memory_failure(unsigned long pfn, int flags)
* In fact it's dangerous to directly bump up page count from 0,
* that may make page_ref_freeze()/page_ref_unfreeze() mismatch.
*/
+ /*
+ * Pages with PG_reserved set are not currently managed by the
+ * page allocator (memblock-reserved memory, driver reservations,
+ * etc.), so classify them as kernel-owned for reporting.
+ *
+ * Sample the flag before get_hwpoison_page(): in the
+ * MF_COUNT_INCREASED path, get_any_page() can drop the caller's
+ * reference before returning -EIO, after which page->flags may
+ * have been reset by the allocator.
+ */
+ is_reserved = PageReserved(p);
+
res = get_hwpoison_page(p, flags);
if (!res) {
if (is_free_buddy_page(p)) {
@@ -2432,7 +2445,11 @@ int memory_failure(unsigned long pfn, int flags)
}
goto unlock_mutex;
} else if (res < 0) {
- res = action_result(pfn, MF_MSG_GET_HWPOISON, MF_IGNORED);
+ if (is_reserved)
+ res = action_result(pfn, MF_MSG_KERNEL, MF_IGNORED);
+ else
+ res = action_result(pfn, MF_MSG_GET_HWPOISON,
+ MF_IGNORED);
goto unlock_mutex;
}
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v6 0/4] mm/memory-failure: add panic option for unrecoverable pages
From: Breno Leitao @ 2026-05-11 15:38 UTC (permalink / raw)
To: Miaohe Lin, Naoya Horiguchi, Andrew Morton, Jonathan Corbet,
Shuah Khan, David Hildenbrand, Lorenzo Stoakes, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Shuah Khan,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Liam R. Howlett
Cc: linux-mm, linux-kernel, linux-doc, linux-kselftest, Breno Leitao,
linux-trace-kernel, kernel-team, Lance Yang
Changes from previouis version
* Replaced the late, action-time recheck for MF_MSG_KERNEL_HIGH_ORDER
with early failure classification inside get_any_page() (new patch
2/4).
* David Hildenbrand pointed out that the recheck inspected refcount and
folio mapping without holding a folio reference, which is unsafe
(concurrent split can trigger VM_WARN_ON_FOLIO).
* Lance Yang suggested moving the disambiguation to the call site that
still knows *why* the page reference could not be taken, which is what
this version does via a new enum mf_get_page_status (MF_GET_PAGE_OK
/ RACE / UNHANDLABLE) plumbed out through get_hwpoison_page().
Signed-off-by: Breno Leitao <leitao@debian.org>
---
Changes in v6:
- Dropped the selftest given the value was not clear
- Get the status of the failure from get_any_page()
- Small nits from different people/AIs.
- Link to v5: https://patch.msgid.link/20260424-ecc_panic-v5-0-a35f4b50425c@debian.org
Changes in v5:
- Add vm.panic_on_unrecoverable_memory_failure sysctl to panic on
unrecoverable kernel page hwpoison events (reserved pages, refcount-0
non-buddy pages, unknown state), with a recheck to avoid racing with
concurrent buddy allocations. (Miaohe)
- Distinguish reserved pages as MF_MSG_KERNEL in memory_failure(),
document the new sysctl in Documentation/admin-guide/sysctl/vm.rst,
and add a selftest verifying SIGBUS recovery on userspace pages still
works when the sysctl is enabled. (Miaohe)
- Added a selftest
- Link to v4:
https://patch.msgid.link/20260415-ecc_panic-v4-0-2d0277f8f601@debian.org
Changes in v4:
- Drop CONFIG_BOOTPARAM_MEMORY_FAILURE_PANIC kernel configuration option.
- Split the reserved page classification (MF_MSG_KERNEL) into its own
patch, separate from the panic mechanism.
- Document why the buddy allocator TOCTOU race (between
get_hwpoison_page() and is_free_buddy_page()) cannot cause false
positives: PG_hwpoison is set beforehand and check_new_page() in the
page allocator rejects hwpoisoned pages.
- Document the narrow LRU isolation race window for MF_MSG_UNKNOWN and
its mitigation via identify_page_state()'s two-pass design.
- Explicitly document why MF_MSG_GET_HWPOISON is excluded from the
panic conditions (shared path with transient races and non-reserved
kernel memory).
- Link to v3: https://patch.msgid.link/20260413-ecc_panic-v3-0-1dcbb2f12bc4@debian.org
Changes in v3:
- Rename is_unrecoverable_memory_failure() to panic_on_unrecoverable_mf()
as suggested by maintainer.
- Add CONFIG_BOOTPARAM_MEMORY_FAILURE_PANIC kernel configuration option,
similar to CONFIG_BOOTPARAM_HARDLOCKUP_PANIC.
- Add documentation for the sysctl and CONFIG option.
- Add code comments documenting the panic condition design rationale and
how the retry mechanism mitigates false positives from buddy allocator
races.
- Link to v2: https://patch.msgid.link/20260331-ecc_panic-v2-0-9e40d0f64f7a@debian.org
Changes in v2:
- Panic on MF_MSG_KERNEL, MF_MSG_KERNEL_HIGH_ORDER and MF_MSG_UNKNOWN
instead of MF_MSG_GET_HWPOISON.
- Report MF_MSG_KERNEL for reserved pages when get_hwpoison_page() fails
instead of MF_MSG_GET_HWPOISON.
- Link to v1: https://patch.msgid.link/20260323-ecc_panic-v1-0-72a1921726c5@debian.org
To: Miaohe Lin <linmiaohe@huawei.com>
To: Naoya Horiguchi <nao.horiguchi@gmail.com>
To: Andrew Morton <akpm@linux-foundation.org>
To: Steven Rostedt <rostedt@goodmis.org>
To: Masami Hiramatsu <mhiramat@kernel.org>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Jonathan Corbet <corbet@lwn.net>
To: Shuah Khan <skhan@linuxfoundation.org>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-trace-kernel@vger.kernel.org
Cc: linux-doc@vger.kernel.org
---
Breno Leitao (4):
mm/memory-failure: report MF_MSG_KERNEL for reserved pages
mm/memory-failure: classify get_any_page() failures by reason
mm/memory-failure: add panic option for unrecoverable pages
Documentation: document panic_on_unrecoverable_memory_failure sysctl
Documentation/admin-guide/sysctl/vm.rst | 70 ++++++++++++++++++++++++++
include/trace/events/memory-failure.h | 2 +-
mm/memory-failure.c | 89 ++++++++++++++++++++++++++++++---
3 files changed, 152 insertions(+), 9 deletions(-)
---
base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
change-id: 20260323-ecc_panic-4e473b83087c
Best regards,
--
Breno Leitao <leitao@debian.org>
^ permalink raw reply
* Re: [PATCH bpf 1/2] uprobes/x86: Fix red zone clobbering in nop5 optimization
From: Oleg Nesterov @ 2026-05-11 14:45 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, linux-trace-kernel, jolsa, peterz, mingo, mhiramat
In-Reply-To: <20260509003146.976844-1-andrii@kernel.org>
On 05/08, Andrii Nakryiko wrote:
>
> +static bool resolve_uprobe_addr(unsigned long ip, unsigned long *probe_addr)
> {
> - struct vm_area_struct *vma = vma_lookup(current->mm, ip);
> + struct uprobes_state *state = ¤t->mm->uprobes_state;
it seems that there is a problem, with or without this change...
Lets forget about this patch for the moment, I am still trying to understand it.
What if register_for_each_vma() calls install_breakpoint(vma) and
vma->mm != current->mm ?
In this case install_breakpoint() path will call __is_optimized() and then
__in_uprobe_trampoline() which does vma_lookup(current->mm). This looks
obviously wrong ?
And unless I am totally confused, this patch "inherits" the problem...
No?
Oleg.
^ permalink raw reply
* Re: [PATCH 07/13] rv: Simply hybrid automata monitors's clock variables
From: Nam Cao @ 2026-05-11 11:55 UTC (permalink / raw)
To: Gabriele Monaco
Cc: Steven Rostedt, Wander Lairson Costa, linux-trace-kernel,
linux-kernel
In-Reply-To: <967f1cd8cd6e27aaa65d68194487306bc312caa0.camel@redhat.com>
Gabriele Monaco <gmonaco@redhat.com> writes:
> Well, this is roughly what we discussed in [1].
> Now, I didn't submit the throttle monitor yet because it depends on unacked
> tracepoints.
Thanks for bringing that up. I had no memory of that discussion.
> In short, this works with the assumption that the expires value you pass to
> ha_check_invariant() is the same you used to arm the timer.
>
> That's true for constant values only (the deadline) but not for something like
> the runtime. I couldn't think of a way to rearrange that model not to require
> the runtime left field.
I believe you are referring to this:
|
|
dl_replenish;reset(clk) v
sched_switch_in #=========================# sched_switch_in;
+--------------- H H reset(clk)
| H H <----------------+
+--------------> H running H |
dl_throttle;reset(clk) H clk < runtime_left_ns() H |
+--------------------------- H H sched_switch_out |
| +------------------> H H -------------+ |
| dl_replenish;reset(clk) #=========================# | |
| | | ^ | |
v | dl_defer_arm | | |
Now that I stared at this again, I think we already deviate from theory
here. Our documentation mentions that the invariant must be in the form
g = v < c | true
with "c [being] a numerical value".
The invariant "clk < runtime_left_ns()" means clk must not exceed the
remaining runtime, which is sampled by calling runtime_left_ns() when
the state is entered. This is not in the theory. Additionally, I think
this interpretation is ambiguous; one could also interpret that as "the
clk value must never exceed the *current* value returned by
runtime_left_ns()".
I digged into the cited academic papers, but couldn't find anything that
can describe this. The closest I see is the "init" label for states, but
that is the condition for entering the states.
> Otherwise.. We could read the remaining time in the timer, but we wouldn't be
> able to simulate ns precision when using the timer wheel.
>
> Now if we really wanted to go down that path, we are using a union to allocate
> either timer or hrtimer, the latter is larger, so we /could/ add a u64 expire_ns
> field to the ha_monitor struct without needing additional memory.
>
> If that doesn't sound too wild to you, I may try and sketch something up to see
> if that's viable. Then this patch could go through as is and I would add the
> extension together with the submission of throttle.
That can work, but not ideal, because hrtimer will not be usable.
Looking at the throttle monitor again, is it possible to rewrite
runtime_left_ns() to read .dl_runtime instead of .runtime? I don't know
the deadline schedule very well, but I think .dl_runtime is not changing
like .runtime?
Nam
^ permalink raw reply
* [PATCH] tracing/probes: Ensure the uprobe buffer size is bigger than event size
From: Masami Hiramatsu (Google) @ 2026-05-11 10:03 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Add BUILD_BUG_ON() to ensure the uprobe per-CPU working buffer
size is bigger than the event size.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
kernel/trace/trace_uprobe.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 2cabf8a23ec5..c5ee7920dec6 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -979,6 +979,7 @@ static struct uprobe_cpu_buffer *prepare_uprobe_buffer(struct trace_uprobe *tu,
ucb = uprobe_buffer_get();
ucb->dsize = tu->tp.size + dsize;
+ BUILD_BUG_ON(MAX_UCB_BUFFER_SIZE < MAX_PROBE_EVENT_SIZE);
if (WARN_ON_ONCE(ucb->dsize > MAX_UCB_BUFFER_SIZE)) {
ucb->dsize = MAX_UCB_BUFFER_SIZE;
dsize = MAX_UCB_BUFFER_SIZE - tu->tp.size;
^ permalink raw reply related
* [RFC v7 7/7] ext4: fast commit: export snapshot stats in fc_info
From: Li Chen @ 2026-05-11 8:43 UTC (permalink / raw)
To: Zhang Yi, Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
Ojaswin Mujoo, Ritesh Harjani (IBM), Zhang Yi, linux-ext4,
linux-kernel
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
linux-trace-kernel
In-Reply-To: <20260511084304.1559557-1-me@linux.beauty>
Snapshot-based fast commit can fall back when the commit-time snapshot
cannot be built (e.g. extent status cache misses). It is useful to
quantify the updates-locked window and to see why snapshotting failed.
Add best-effort snapshot counters to the ext4 superblock and extend
/proc/fs/ext4/<sb_id>/fc_info to report the number of snapshotted
inodes and ranges, snapshot failure reasons, and the average/max time
spent with journal updates locked.
Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
Changes in v7:
- Address Sashiko review by using READ_ONCE() + div64_u64() for the fc_info
lock_updates average.
Changes in v6:
- Start consuming locked_ns in fc_info, so this patch intentionally moves
lock_updates_ns_{total,max,samples} accounting here.
- Guard the tracepoint call with trace_ext4_fc_lock_updates_enabled() and
use trace_call__ext4_fc_lock_updates() to avoid the double static_branch
at the guarded call site.
- Keep the stats unconditionally while avoiding extra tracepoint
overhead when ext4_fc_lock_updates is disabled.
fs/ext4/ext4.h | 31 +++++++++++++++++
fs/ext4/fast_commit.c | 78 +++++++++++++++++++++++++++++++++++++------
fs/ext4/super.c | 1 +
3 files changed, 100 insertions(+), 10 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index df30f8705c98..3457b4950c02 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1550,6 +1550,36 @@ struct ext4_orphan_info {
* file blocks */
};
+/*
+ * Ext4 fast commit snapshot statistics.
+ *
+ * These are best-effort counters intended for debugging / performance
+ * introspection; they are not exact under concurrent updates.
+ */
+struct ext4_fc_snap_stats {
+ u64 lock_updates_ns_total;
+ u64 lock_updates_ns_max;
+ u64 lock_updates_samples;
+
+ u64 snap_inodes;
+ u64 snap_ranges;
+
+ u64 snap_fail_es_miss;
+ u64 snap_fail_es_delayed;
+ u64 snap_fail_es_other;
+
+ u64 snap_fail_inodes_cap;
+ u64 snap_fail_ranges_cap;
+ u64 snap_fail_nomem;
+ u64 snap_fail_inode_loc;
+
+ /*
+ * Missing inode snapshots during log writing should never happen.
+ * Keep this counter to help catch unexpected regressions.
+ */
+ u64 snap_fail_no_snap;
+};
+
/*
* fourth extended-fs super-block data in memory
*/
@@ -1824,6 +1854,7 @@ struct ext4_sb_info {
struct mutex s_fc_lock;
struct buffer_head *s_fc_bh;
struct ext4_fc_stats s_fc_stats;
+ struct ext4_fc_snap_stats s_fc_snap_stats;
tid_t s_fc_ineligible_tid;
#ifdef CONFIG_EXT4_DEBUG
int s_fc_debug_max_replay;
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index c24984d8df83..1dfcccf4179e 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -874,13 +874,17 @@ static int ext4_fc_write_inode(struct inode *inode, u32 *crc)
int inode_len;
int ret;
- if (!snap)
+ if (!snap) {
+ EXT4_SB(inode->i_sb)->s_fc_snap_stats.snap_fail_no_snap++;
return -ECANCELED;
+ }
src = snap->inode_buf;
inode_len = snap->inode_len;
- if (!src || inode_len == 0)
+ if (!src || inode_len == 0) {
+ EXT4_SB(inode->i_sb)->s_fc_snap_stats.snap_fail_no_snap++;
return -ECANCELED;
+ }
fc_inode.fc_ino = cpu_to_le32(inode->i_ino);
tl.fc_tag = cpu_to_le16(EXT4_FC_TAG_INODE);
@@ -915,8 +919,10 @@ static int ext4_fc_write_inode_data(struct inode *inode, u32 *crc)
struct ext4_extent *ex;
struct ext4_fc_range *range;
- if (!snap)
+ if (!snap) {
+ EXT4_SB(inode->i_sb)->s_fc_snap_stats.snap_fail_no_snap++;
return -ECANCELED;
+ }
list_for_each_entry(range, &snap->data_list, list) {
if (range->tag == EXT4_FC_TAG_DEL_RANGE) {
@@ -977,6 +983,8 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
int *snap_err)
{
struct ext4_inode_info *ei = EXT4_I(inode);
+ struct ext4_fc_snap_stats *stats =
+ &EXT4_SB(inode->i_sb)->s_fc_snap_stats;
ext4_lblk_t start_lblk, end_lblk, cur_lblk;
unsigned int nr_ranges = 0;
@@ -1004,11 +1012,13 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
u64 remaining = (u64)end_lblk - cur_lblk + 1;
if (!ext4_es_lookup_extent(inode, cur_lblk, NULL, &es, NULL)) {
+ stats->snap_fail_es_miss++;
ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_ES_MISS);
return -EAGAIN;
}
if (ext4_es_is_delayed(&es)) {
+ stats->snap_fail_es_delayed++;
ext4_fc_set_snap_err(snap_err,
EXT4_FC_SNAP_ERR_ES_DELAYED);
return -EAGAIN;
@@ -1023,6 +1033,7 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
}
if (nr_ranges_total + nr_ranges >= EXT4_FC_SNAPSHOT_MAX_RANGES) {
+ stats->snap_fail_ranges_cap++;
ext4_fc_set_snap_err(snap_err,
EXT4_FC_SNAP_ERR_RANGES_CAP);
return -E2BIG;
@@ -1030,6 +1041,7 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
range = kmem_cache_alloc(ext4_fc_range_cachep, GFP_NOFS);
if (!range) {
+ stats->snap_fail_nomem++;
ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_NOMEM);
return -ENOMEM;
}
@@ -1057,6 +1069,7 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
range->len = max;
} else {
kmem_cache_free(ext4_fc_range_cachep, range);
+ stats->snap_fail_es_other++;
ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_ES_OTHER);
return -EAGAIN;
}
@@ -1080,6 +1093,8 @@ static int ext4_fc_snapshot_inode(struct inode *inode,
unsigned int *nr_rangesp, int *snap_err)
{
struct ext4_inode_info *ei = EXT4_I(inode);
+ struct ext4_fc_snap_stats *stats =
+ &EXT4_SB(inode->i_sb)->s_fc_snap_stats;
struct ext4_fc_inode_snap *snap;
int inode_len = EXT4_GOOD_OLD_INODE_SIZE;
struct ext4_iloc iloc;
@@ -1090,6 +1105,7 @@ static int ext4_fc_snapshot_inode(struct inode *inode,
ret = ext4_get_inode_loc_noio(inode, &iloc);
if (ret) {
+ stats->snap_fail_inode_loc++;
ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_INODE_LOC);
return ret;
}
@@ -1101,6 +1117,7 @@ static int ext4_fc_snapshot_inode(struct inode *inode,
snap = kmalloc(struct_size(snap, inode_buf, inode_len), GFP_NOFS);
if (!snap) {
+ stats->snap_fail_nomem++;
ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_NOMEM);
brelse(iloc.bh);
return -ENOMEM;
@@ -1125,6 +1142,8 @@ static int ext4_fc_snapshot_inode(struct inode *inode,
list_splice_tail_init(&ranges, &snap->data_list);
ext4_fc_unlock(inode->i_sb, alloc_ctx);
+ stats->snap_inodes++;
+ stats->snap_ranges += nr_ranges;
if (nr_rangesp)
*nr_rangesp = nr_ranges;
return 0;
@@ -1234,6 +1253,7 @@ static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes,
alloc_ctx = ext4_fc_lock(sb);
list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
if (i >= inodes_size) {
+ sbi->s_fc_snap_stats.snap_fail_inodes_cap++;
ext4_fc_set_snap_err(snap_err,
EXT4_FC_SNAP_ERR_INODES_CAP);
ret = -E2BIG;
@@ -1259,6 +1279,7 @@ static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes,
continue;
if (i >= inodes_size) {
+ sbi->s_fc_snap_stats.snap_fail_inodes_cap++;
ext4_fc_set_snap_err(snap_err,
EXT4_FC_SNAP_ERR_INODES_CAP);
ret = -E2BIG;
@@ -1302,6 +1323,7 @@ static int ext4_fc_perform_commit(journal_t *journal, tid_t commit_tid)
{
struct super_block *sb = journal->j_private;
struct ext4_sb_info *sbi = EXT4_SB(sb);
+ struct ext4_fc_snap_stats *snap_stats = &sbi->s_fc_snap_stats;
struct ext4_inode_info *iter;
struct ext4_fc_head head;
struct inode *inode;
@@ -1364,8 +1386,13 @@ static int ext4_fc_perform_commit(journal_t *journal, tid_t commit_tid)
return ret;
ret = ext4_fc_alloc_snapshot_inodes(sb, &inodes, &inodes_size);
- if (ret)
+ if (ret) {
+ if (ret == -E2BIG)
+ snap_stats->snap_fail_inodes_cap++;
+ else if (ret == -ENOMEM)
+ snap_stats->snap_fail_nomem++;
return ret;
+ }
/* Step 4: Mark all inodes as being committed. */
jbd2_journal_lock_updates(journal);
@@ -1386,12 +1413,15 @@ static int ext4_fc_perform_commit(journal_t *journal, tid_t commit_tid)
ret = ext4_fc_snapshot_inodes(journal, inodes, inodes_size,
&snap_inodes, &snap_ranges, &snap_err);
jbd2_journal_unlock_updates(journal);
- if (trace_ext4_fc_lock_updates_enabled()) {
- locked_ns = ktime_to_ns(ktime_sub(ktime_get(), lock_start));
- trace_ext4_fc_lock_updates(sb, commit_tid, locked_ns,
- snap_inodes, snap_ranges, ret,
- snap_err);
- }
+ locked_ns = ktime_to_ns(ktime_sub(ktime_get(), lock_start));
+ snap_stats->lock_updates_ns_total += locked_ns;
+ snap_stats->lock_updates_samples++;
+ if (locked_ns > snap_stats->lock_updates_ns_max)
+ snap_stats->lock_updates_ns_max = locked_ns;
+ if (trace_ext4_fc_lock_updates_enabled())
+ trace_call__ext4_fc_lock_updates(sb, commit_tid, locked_ns,
+ snap_inodes, snap_ranges,
+ ret, snap_err);
kvfree(inodes);
if (ret)
return ret;
@@ -2667,11 +2697,23 @@ int ext4_fc_info_show(struct seq_file *seq, void *v)
{
struct ext4_sb_info *sbi = EXT4_SB((struct super_block *)seq->private);
struct ext4_fc_stats *stats = &sbi->s_fc_stats;
+ struct ext4_fc_snap_stats *snap_stats = &sbi->s_fc_snap_stats;
+ u64 lock_avg_ns = 0;
+ u64 lock_updates_samples;
+ u64 lock_updates_ns_total;
+ u64 lock_updates_ns_max;
int i;
if (v != SEQ_START_TOKEN)
return 0;
+ lock_updates_samples = READ_ONCE(snap_stats->lock_updates_samples);
+ lock_updates_ns_total = READ_ONCE(snap_stats->lock_updates_ns_total);
+ lock_updates_ns_max = READ_ONCE(snap_stats->lock_updates_ns_max);
+ if (lock_updates_samples)
+ lock_avg_ns = div64_u64(lock_updates_ns_total,
+ lock_updates_samples);
+
seq_printf(seq,
"fc stats:\n%ld commits\n%ld ineligible\n%ld numblks\n%lluus avg_commit_time\n",
stats->fc_num_commits, stats->fc_ineligible_commits,
@@ -2682,6 +2724,22 @@ int ext4_fc_info_show(struct seq_file *seq, void *v)
seq_printf(seq, "\"%s\":\t%d\n", fc_ineligible_reasons[i],
stats->fc_ineligible_reason_count[i]);
+ seq_printf(seq,
+ "Snapshot stats:\n%llu inodes\n%llu ranges\n%lluus lock_updates_avg\n%lluus lock_updates_max\n",
+ snap_stats->snap_inodes, snap_stats->snap_ranges,
+ div_u64(lock_avg_ns, 1000),
+ div_u64(lock_updates_ns_max, 1000));
+ seq_printf(seq,
+ "Snapshot failures:\n%llu es_miss\n%llu es_delayed\n%llu es_other\n%llu inodes_cap\n%llu ranges_cap\n%llu nomem\n%llu inode_loc\n%llu no_snap\n",
+ snap_stats->snap_fail_es_miss,
+ snap_stats->snap_fail_es_delayed,
+ snap_stats->snap_fail_es_other,
+ snap_stats->snap_fail_inodes_cap,
+ snap_stats->snap_fail_ranges_cap,
+ snap_stats->snap_fail_nomem,
+ snap_stats->snap_fail_inode_loc,
+ snap_stats->snap_fail_no_snap);
+
return 0;
}
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 3c869f0001c5..f1f8819a2a23 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4544,6 +4544,7 @@ static void ext4_fast_commit_init(struct super_block *sb)
sbi->s_fc_ineligible_tid = 0;
mutex_init(&sbi->s_fc_lock);
memset(&sbi->s_fc_stats, 0, sizeof(sbi->s_fc_stats));
+ memset(&sbi->s_fc_snap_stats, 0, sizeof(sbi->s_fc_snap_stats));
sbi->s_fc_replay_state.fc_regions = NULL;
sbi->s_fc_replay_state.fc_regions_size = 0;
sbi->s_fc_replay_state.fc_regions_used = 0;
--
2.53.0
^ permalink raw reply related
* [RFC v7 6/7] ext4: fast commit: add lock_updates tracepoint
From: Li Chen @ 2026-05-11 8:43 UTC (permalink / raw)
To: Zhang Yi, Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
Ojaswin Mujoo, Ritesh Harjani (IBM), Zhang Yi, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, linux-ext4, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260511084304.1559557-1-me@linux.beauty>
Commit-time fast commit snapshots run under jbd2_journal_lock_updates(),
so it is useful to quantify the time spent with updates locked and to
understand why snapshotting can fail.
Add a new tracepoint, ext4_fc_lock_updates, reporting the time spent in
the updates-locked window along with the number of snapshotted inodes
and ranges. Record the first snapshot failure reason in a stable snap_err
field for tooling.
Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
Changes in v7:
- Address Sashiko review by reporting successfully snapshotted inode counts
in ext4_fc_lock_updates when snapshotting stops early.
Changes in v6:
- Drop explicit ext4_fc_snap_err assignments and rely on enum
auto-increment.
- Treat locked_ns as trace-only in this patch and calculate it only when
ext4_fc_lock_updates is enabled, as suggested by Steven Rostedt.
fs/ext4/ext4.h | 15 ++++++++
fs/ext4/fast_commit.c | 74 +++++++++++++++++++++++++++++--------
include/trace/events/ext4.h | 61 ++++++++++++++++++++++++++++++
3 files changed, 135 insertions(+), 15 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 2a706acdfaf8..df30f8705c98 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1027,6 +1027,21 @@ enum {
struct ext4_fc_inode_snap;
+/*
+ * Snapshot failure reasons for ext4_fc_lock_updates tracepoint.
+ * Keep these stable for tooling.
+ */
+enum ext4_fc_snap_err {
+ EXT4_FC_SNAP_ERR_NONE = 0,
+ EXT4_FC_SNAP_ERR_ES_MISS,
+ EXT4_FC_SNAP_ERR_ES_DELAYED,
+ EXT4_FC_SNAP_ERR_ES_OTHER,
+ EXT4_FC_SNAP_ERR_INODES_CAP,
+ EXT4_FC_SNAP_ERR_RANGES_CAP,
+ EXT4_FC_SNAP_ERR_NOMEM,
+ EXT4_FC_SNAP_ERR_INODE_LOC,
+};
+
/*
* fourth extended file system inode data in memory
*/
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 9fc17c1fa7af..c24984d8df83 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -194,6 +194,12 @@ static struct kmem_cache *ext4_fc_range_cachep;
#define EXT4_FC_SNAPSHOT_MAX_INODES 1024
#define EXT4_FC_SNAPSHOT_MAX_RANGES 2048
+static inline void ext4_fc_set_snap_err(int *snap_err, int err)
+{
+ if (snap_err && *snap_err == EXT4_FC_SNAP_ERR_NONE)
+ *snap_err = err;
+}
+
static void ext4_end_buffer_io_sync(struct buffer_head *bh, int uptodate)
{
BUFFER_TRACE(bh, "");
@@ -967,11 +973,12 @@ static void ext4_fc_free_inode_snap(struct inode *inode)
static int ext4_fc_snapshot_inode_data(struct inode *inode,
struct list_head *ranges,
unsigned int nr_ranges_total,
- unsigned int *nr_rangesp)
+ unsigned int *nr_rangesp,
+ int *snap_err)
{
struct ext4_inode_info *ei = EXT4_I(inode);
- unsigned int nr_ranges = 0;
ext4_lblk_t start_lblk, end_lblk, cur_lblk;
+ unsigned int nr_ranges = 0;
spin_lock(&ei->i_fc_lock);
if (ei->i_fc_lblk_len == 0) {
@@ -996,11 +1003,16 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
ext4_lblk_t len;
u64 remaining = (u64)end_lblk - cur_lblk + 1;
- if (!ext4_es_lookup_extent(inode, cur_lblk, NULL, &es, NULL))
+ if (!ext4_es_lookup_extent(inode, cur_lblk, NULL, &es, NULL)) {
+ ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_ES_MISS);
return -EAGAIN;
+ }
- if (ext4_es_is_delayed(&es))
+ if (ext4_es_is_delayed(&es)) {
+ ext4_fc_set_snap_err(snap_err,
+ EXT4_FC_SNAP_ERR_ES_DELAYED);
return -EAGAIN;
+ }
len = es.es_len - (cur_lblk - es.es_lblk);
if (len > remaining)
@@ -1010,12 +1022,17 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
continue;
}
- if (nr_ranges_total + nr_ranges >= EXT4_FC_SNAPSHOT_MAX_RANGES)
+ if (nr_ranges_total + nr_ranges >= EXT4_FC_SNAPSHOT_MAX_RANGES) {
+ ext4_fc_set_snap_err(snap_err,
+ EXT4_FC_SNAP_ERR_RANGES_CAP);
return -E2BIG;
+ }
range = kmem_cache_alloc(ext4_fc_range_cachep, GFP_NOFS);
- if (!range)
+ if (!range) {
+ ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_NOMEM);
return -ENOMEM;
+ }
nr_ranges++;
range->lblk = cur_lblk;
@@ -1040,6 +1057,7 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
range->len = max;
} else {
kmem_cache_free(ext4_fc_range_cachep, range);
+ ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_ES_OTHER);
return -EAGAIN;
}
@@ -1059,7 +1077,7 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
static int ext4_fc_snapshot_inode(struct inode *inode,
unsigned int nr_ranges_total,
- unsigned int *nr_rangesp)
+ unsigned int *nr_rangesp, int *snap_err)
{
struct ext4_inode_info *ei = EXT4_I(inode);
struct ext4_fc_inode_snap *snap;
@@ -1071,8 +1089,10 @@ static int ext4_fc_snapshot_inode(struct inode *inode,
int alloc_ctx;
ret = ext4_get_inode_loc_noio(inode, &iloc);
- if (ret)
+ if (ret) {
+ ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_INODE_LOC);
return ret;
+ }
if (ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA))
inode_len = EXT4_INODE_SIZE(inode->i_sb);
@@ -1081,6 +1101,7 @@ static int ext4_fc_snapshot_inode(struct inode *inode,
snap = kmalloc(struct_size(snap, inode_buf, inode_len), GFP_NOFS);
if (!snap) {
+ ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_NOMEM);
brelse(iloc.bh);
return -ENOMEM;
}
@@ -1091,7 +1112,7 @@ static int ext4_fc_snapshot_inode(struct inode *inode,
brelse(iloc.bh);
ret = ext4_fc_snapshot_inode_data(inode, &ranges, nr_ranges_total,
- &nr_ranges);
+ &nr_ranges, snap_err);
if (ret) {
kfree(snap);
ext4_fc_free_ranges(&ranges);
@@ -1192,7 +1213,10 @@ static int ext4_fc_alloc_snapshot_inodes(struct super_block *sb,
unsigned int *nr_inodesp);
static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes,
- unsigned int inodes_size)
+ unsigned int inodes_size,
+ unsigned int *nr_inodesp,
+ unsigned int *nr_rangesp,
+ int *snap_err)
{
struct super_block *sb = journal->j_private;
struct ext4_sb_info *sbi = EXT4_SB(sb);
@@ -1210,6 +1234,8 @@ static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes,
alloc_ctx = ext4_fc_lock(sb);
list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
if (i >= inodes_size) {
+ ext4_fc_set_snap_err(snap_err,
+ EXT4_FC_SNAP_ERR_INODES_CAP);
ret = -E2BIG;
goto unlock;
}
@@ -1233,6 +1259,8 @@ static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes,
continue;
if (i >= inodes_size) {
+ ext4_fc_set_snap_err(snap_err,
+ EXT4_FC_SNAP_ERR_INODES_CAP);
ret = -E2BIG;
goto unlock;
}
@@ -1257,16 +1285,20 @@ static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes,
unsigned int inode_ranges = 0;
ret = ext4_fc_snapshot_inode(inodes[idx], nr_ranges,
- &inode_ranges);
+ &inode_ranges, snap_err);
if (ret)
break;
nr_ranges += inode_ranges;
}
+ if (nr_inodesp)
+ *nr_inodesp = idx;
+ if (nr_rangesp)
+ *nr_rangesp = nr_ranges;
return ret;
}
-static int ext4_fc_perform_commit(journal_t *journal)
+static int ext4_fc_perform_commit(journal_t *journal, tid_t commit_tid)
{
struct super_block *sb = journal->j_private;
struct ext4_sb_info *sbi = EXT4_SB(sb);
@@ -1275,10 +1307,15 @@ static int ext4_fc_perform_commit(journal_t *journal)
struct inode *inode;
struct inode **inodes;
unsigned int inodes_size;
+ unsigned int snap_inodes = 0;
+ unsigned int snap_ranges = 0;
+ int snap_err = EXT4_FC_SNAP_ERR_NONE;
struct blk_plug plug;
int ret = 0;
u32 crc = 0;
int alloc_ctx;
+ ktime_t lock_start;
+ u64 locked_ns;
/*
* Step 1: Mark all inodes on s_fc_q[MAIN] with
@@ -1326,13 +1363,13 @@ static int ext4_fc_perform_commit(journal_t *journal)
if (ret)
return ret;
-
ret = ext4_fc_alloc_snapshot_inodes(sb, &inodes, &inodes_size);
if (ret)
return ret;
/* Step 4: Mark all inodes as being committed. */
jbd2_journal_lock_updates(journal);
+ lock_start = ktime_get();
/*
* The journal is now locked. No more handles can start and all the
* previous handles are now drained. Snapshotting happens in this
@@ -1346,8 +1383,15 @@ static int ext4_fc_perform_commit(journal_t *journal)
}
ext4_fc_unlock(sb, alloc_ctx);
- ret = ext4_fc_snapshot_inodes(journal, inodes, inodes_size);
+ ret = ext4_fc_snapshot_inodes(journal, inodes, inodes_size,
+ &snap_inodes, &snap_ranges, &snap_err);
jbd2_journal_unlock_updates(journal);
+ if (trace_ext4_fc_lock_updates_enabled()) {
+ locked_ns = ktime_to_ns(ktime_sub(ktime_get(), lock_start));
+ trace_ext4_fc_lock_updates(sb, commit_tid, locked_ns,
+ snap_inodes, snap_ranges, ret,
+ snap_err);
+ }
kvfree(inodes);
if (ret)
return ret;
@@ -1552,7 +1596,7 @@ int ext4_fc_commit(journal_t *journal, tid_t commit_tid)
journal_ioprio = EXT4_DEF_JOURNAL_IOPRIO;
set_task_ioprio(current, journal_ioprio);
fc_bufs_before = (sbi->s_fc_bytes + bsize - 1) / bsize;
- ret = ext4_fc_perform_commit(journal);
+ ret = ext4_fc_perform_commit(journal, commit_tid);
if (ret < 0) {
if (ret == -EAGAIN || ret == -E2BIG || ret == -ECANCELED)
status = EXT4_FC_STATUS_INELIGIBLE;
diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
index f493642cf121..7028a28316fa 100644
--- a/include/trace/events/ext4.h
+++ b/include/trace/events/ext4.h
@@ -107,6 +107,26 @@ TRACE_DEFINE_ENUM(EXT4_FC_REASON_VERITY);
TRACE_DEFINE_ENUM(EXT4_FC_REASON_MOVE_EXT);
TRACE_DEFINE_ENUM(EXT4_FC_REASON_MAX);
+#undef EM
+#undef EMe
+#define EM(a) TRACE_DEFINE_ENUM(EXT4_FC_SNAP_ERR_##a);
+#define EMe(a) TRACE_DEFINE_ENUM(EXT4_FC_SNAP_ERR_##a);
+
+#define TRACE_SNAP_ERR \
+ EM(NONE) \
+ EM(ES_MISS) \
+ EM(ES_DELAYED) \
+ EM(ES_OTHER) \
+ EM(INODES_CAP) \
+ EM(RANGES_CAP) \
+ EM(NOMEM) \
+ EMe(INODE_LOC)
+
+TRACE_SNAP_ERR
+
+#undef EM
+#undef EMe
+
#define show_fc_reason(reason) \
__print_symbolic(reason, \
{ EXT4_FC_REASON_XATTR, "XATTR"}, \
@@ -2818,6 +2838,47 @@ TRACE_EVENT(ext4_fc_commit_stop,
__entry->num_fc_ineligible, __entry->nblks_agg, __entry->tid)
);
+#define EM(a) { EXT4_FC_SNAP_ERR_##a, #a },
+#define EMe(a) { EXT4_FC_SNAP_ERR_##a, #a }
+
+TRACE_EVENT(ext4_fc_lock_updates,
+ TP_PROTO(struct super_block *sb, tid_t commit_tid, u64 locked_ns,
+ unsigned int nr_inodes, unsigned int nr_ranges, int err,
+ int snap_err),
+
+ TP_ARGS(sb, commit_tid, locked_ns, nr_inodes, nr_ranges, err, snap_err),
+
+ TP_STRUCT__entry(/* entry */
+ __field(dev_t, dev)
+ __field(tid_t, tid)
+ __field(u64, locked_ns)
+ __field(unsigned int, nr_inodes)
+ __field(unsigned int, nr_ranges)
+ __field(int, err)
+ __field(int, snap_err)
+ ),
+
+ TP_fast_assign(/* assign */
+ __entry->dev = sb->s_dev;
+ __entry->tid = commit_tid;
+ __entry->locked_ns = locked_ns;
+ __entry->nr_inodes = nr_inodes;
+ __entry->nr_ranges = nr_ranges;
+ __entry->err = err;
+ __entry->snap_err = snap_err;
+ ),
+
+ TP_printk("dev %d,%d tid %u locked_ns %llu nr_inodes %u nr_ranges %u err %d snap_err %s",
+ MAJOR(__entry->dev), MINOR(__entry->dev), __entry->tid,
+ __entry->locked_ns, __entry->nr_inodes, __entry->nr_ranges,
+ __entry->err, __print_symbolic(__entry->snap_err,
+ TRACE_SNAP_ERR))
+);
+
+#undef EM
+#undef EMe
+#undef TRACE_SNAP_ERR
+
#define FC_REASON_NAME_STAT(reason) \
show_fc_reason(reason), \
__entry->fc_ineligible_rc[reason]
--
2.53.0
^ permalink raw reply related
* [RFC v7 5/7] ext4: fast commit: avoid i_data_sem by dropping ext4_map_blocks() in snapshots
From: Li Chen @ 2026-05-11 8:43 UTC (permalink / raw)
To: Zhang Yi, Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
Ojaswin Mujoo, Ritesh Harjani (IBM), Zhang Yi, linux-ext4,
linux-kernel
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
linux-trace-kernel
In-Reply-To: <20260511084304.1559557-1-me@linux.beauty>
Commit-time snapshots run under jbd2_journal_lock_updates(), so the work
done there must stay bounded.
The snapshot path still used ext4_map_blocks() to build data ranges. This
can take i_data_sem and pulls the mapping code into the snapshot logic.
Build inode data range snapshots from the extent status tree instead.
The extent status tree is a cache, not an authoritative source. If the
needed information is missing or unstable (e.g. delayed allocation), treat
the transaction as fast commit ineligible and fall back to full commit.
Also cap the number of inodes and ranges snapshotted per fast commit and
allocate range records from a dedicated slab cache. The inode pointer
array is allocated outside the updates-locked window.
Testing: QEMU/KVM guest, virtio-pmem + dax, ext4 -O fast_commit, mounted
dax,noatime. Ran python3 500x {4K write + fsync}, fallocate 256M, and
python3 500x {creat + fsync(dir)} without lockdep splats or errors.
Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
Changes in v7:
- Address Sashiko review by guarding snapshot range arithmetic near
EXT_MAX_BLOCKS to avoid cur_lblk / remaining-range wraparound in the
snapshot walk.
fs/ext4/fast_commit.c | 257 +++++++++++++++++++++++++++++-------------
1 file changed, 181 insertions(+), 76 deletions(-)
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index f9bb18c0b549..9fc17c1fa7af 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -184,6 +184,15 @@
#include <trace/events/ext4.h>
static struct kmem_cache *ext4_fc_dentry_cachep;
+static struct kmem_cache *ext4_fc_range_cachep;
+
+/*
+ * Avoid spending unbounded time/memory snapshotting highly fragmented files
+ * under jbd2_journal_lock_updates(). If we exceed this limit, fall back to
+ * full commit.
+ */
+#define EXT4_FC_SNAPSHOT_MAX_INODES 1024
+#define EXT4_FC_SNAPSHOT_MAX_RANGES 2048
static void ext4_end_buffer_io_sync(struct buffer_head *bh, int uptodate)
{
@@ -938,7 +947,7 @@ static void ext4_fc_free_ranges(struct list_head *head)
list_for_each_entry_safe(range, range_n, head, list) {
list_del(&range->list);
- kfree(range);
+ kmem_cache_free(ext4_fc_range_cachep, range);
}
}
@@ -956,16 +965,19 @@ static void ext4_fc_free_inode_snap(struct inode *inode)
}
static int ext4_fc_snapshot_inode_data(struct inode *inode,
- struct list_head *ranges)
+ struct list_head *ranges,
+ unsigned int nr_ranges_total,
+ unsigned int *nr_rangesp)
{
struct ext4_inode_info *ei = EXT4_I(inode);
+ unsigned int nr_ranges = 0;
ext4_lblk_t start_lblk, end_lblk, cur_lblk;
- struct ext4_map_blocks map;
- int ret;
spin_lock(&ei->i_fc_lock);
if (ei->i_fc_lblk_len == 0) {
spin_unlock(&ei->i_fc_lock);
+ if (nr_rangesp)
+ *nr_rangesp = 0;
return 0;
}
start_lblk = ei->i_fc_lblk_start;
@@ -979,61 +991,82 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode,
(unsigned long long)inode->i_ino);
while (cur_lblk <= end_lblk) {
+ struct extent_status es;
struct ext4_fc_range *range;
+ ext4_lblk_t len;
+ u64 remaining = (u64)end_lblk - cur_lblk + 1;
- map.m_lblk = cur_lblk;
- map.m_len = end_lblk - cur_lblk + 1;
- ret = ext4_map_blocks(NULL, inode, &map,
- EXT4_GET_BLOCKS_IO_SUBMIT |
- EXT4_EX_NOCACHE);
- if (ret < 0)
- return -ECANCELED;
+ if (!ext4_es_lookup_extent(inode, cur_lblk, NULL, &es, NULL))
+ return -EAGAIN;
+
+ if (ext4_es_is_delayed(&es))
+ return -EAGAIN;
- if (map.m_len == 0) {
+ len = es.es_len - (cur_lblk - es.es_lblk);
+ if (len > remaining)
+ len = remaining;
+ if (len == 0) {
cur_lblk++;
continue;
}
- range = kmalloc(sizeof(*range), GFP_NOFS);
+ if (nr_ranges_total + nr_ranges >= EXT4_FC_SNAPSHOT_MAX_RANGES)
+ return -E2BIG;
+
+ range = kmem_cache_alloc(ext4_fc_range_cachep, GFP_NOFS);
if (!range)
return -ENOMEM;
+ nr_ranges++;
- range->lblk = map.m_lblk;
- range->len = map.m_len;
+ range->lblk = cur_lblk;
+ range->len = len;
range->pblk = 0;
range->unwritten = false;
- if (ret == 0) {
+ if (ext4_es_is_hole(&es)) {
range->tag = EXT4_FC_TAG_DEL_RANGE;
- } else {
- unsigned int max = (map.m_flags & EXT4_MAP_UNWRITTEN) ?
- EXT_UNWRITTEN_MAX_LEN : EXT_INIT_MAX_LEN;
-
- /* Limit the number of blocks in one extent */
- map.m_len = min(max, map.m_len);
+ } else if (ext4_es_is_written(&es) ||
+ ext4_es_is_unwritten(&es)) {
+ unsigned int max;
range->tag = EXT4_FC_TAG_ADD_RANGE;
- range->len = map.m_len;
- range->pblk = map.m_pblk;
- range->unwritten = !!(map.m_flags & EXT4_MAP_UNWRITTEN);
+ range->pblk = ext4_es_pblock(&es) +
+ (cur_lblk - es.es_lblk);
+ range->unwritten = ext4_es_is_unwritten(&es);
+
+ max = range->unwritten ? EXT_UNWRITTEN_MAX_LEN :
+ EXT_INIT_MAX_LEN;
+ if (range->len > max)
+ range->len = max;
+ } else {
+ kmem_cache_free(ext4_fc_range_cachep, range);
+ return -EAGAIN;
}
INIT_LIST_HEAD(&range->list);
list_add_tail(&range->list, ranges);
- cur_lblk += map.m_len;
+ if ((u64)range->len > (u64)end_lblk - cur_lblk)
+ break;
+
+ cur_lblk += range->len;
}
+ if (nr_rangesp)
+ *nr_rangesp = nr_ranges;
return 0;
}
-static int ext4_fc_snapshot_inode(struct inode *inode)
+static int ext4_fc_snapshot_inode(struct inode *inode,
+ unsigned int nr_ranges_total,
+ unsigned int *nr_rangesp)
{
struct ext4_inode_info *ei = EXT4_I(inode);
struct ext4_fc_inode_snap *snap;
int inode_len = EXT4_GOOD_OLD_INODE_SIZE;
struct ext4_iloc iloc;
LIST_HEAD(ranges);
+ unsigned int nr_ranges = 0;
int ret;
int alloc_ctx;
@@ -1057,7 +1090,8 @@ static int ext4_fc_snapshot_inode(struct inode *inode)
memcpy(snap->inode_buf, (u8 *)ext4_raw_inode(&iloc), inode_len);
brelse(iloc.bh);
- ret = ext4_fc_snapshot_inode_data(inode, &ranges);
+ ret = ext4_fc_snapshot_inode_data(inode, &ranges, nr_ranges_total,
+ &nr_ranges);
if (ret) {
kfree(snap);
ext4_fc_free_ranges(&ranges);
@@ -1070,10 +1104,11 @@ static int ext4_fc_snapshot_inode(struct inode *inode)
list_splice_tail_init(&ranges, &snap->data_list);
ext4_fc_unlock(inode->i_sb, alloc_ctx);
+ if (nr_rangesp)
+ *nr_rangesp = nr_ranges;
return 0;
}
-
/* Flushes data of all the inodes in the commit queue. */
static int ext4_fc_flush_data(journal_t *journal)
{
@@ -1152,49 +1187,32 @@ static int ext4_fc_commit_dentry_updates(journal_t *journal, u32 *crc)
return 0;
}
-static int ext4_fc_snapshot_inodes(journal_t *journal)
+static int ext4_fc_alloc_snapshot_inodes(struct super_block *sb,
+ struct inode ***inodesp,
+ unsigned int *nr_inodesp);
+
+static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes,
+ unsigned int inodes_size)
{
struct super_block *sb = journal->j_private;
struct ext4_sb_info *sbi = EXT4_SB(sb);
struct ext4_inode_info *iter;
struct ext4_fc_dentry_update *fc_dentry;
- struct inode **inodes;
- unsigned int nr_inodes = 0;
unsigned int i = 0;
+ unsigned int idx;
+ unsigned int nr_ranges = 0;
int ret = 0;
int alloc_ctx;
- alloc_ctx = ext4_fc_lock(sb);
- list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list)
- nr_inodes++;
-
- list_for_each_entry(fc_dentry, &sbi->s_fc_dentry_q[FC_Q_MAIN], fcd_list) {
- struct ext4_inode_info *ei;
-
- if (fc_dentry->fcd_op != EXT4_FC_TAG_CREAT)
- continue;
- if (list_empty(&fc_dentry->fcd_dilist))
- continue;
-
- /* See the comment in ext4_fc_commit_dentry_updates(). */
- ei = list_first_entry(&fc_dentry->fcd_dilist,
- struct ext4_inode_info, i_fc_dilist);
- if (!list_empty(&ei->i_fc_list))
- continue;
-
- nr_inodes++;
- }
- ext4_fc_unlock(sb, alloc_ctx);
-
- if (!nr_inodes)
+ if (!inodes_size)
return 0;
- inodes = kvcalloc(nr_inodes, sizeof(*inodes), GFP_NOFS);
- if (!inodes)
- return -ENOMEM;
-
alloc_ctx = ext4_fc_lock(sb);
list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
+ if (i >= inodes_size) {
+ ret = -E2BIG;
+ goto unlock;
+ }
inodes[i++] = &iter->vfs_inode;
}
@@ -1214,6 +1232,10 @@ static int ext4_fc_snapshot_inodes(journal_t *journal)
if (!list_empty(&ei->i_fc_list))
continue;
+ if (i >= inodes_size) {
+ ret = -E2BIG;
+ goto unlock;
+ }
/*
* Create-only inodes may only be referenced via fcd_dilist and
* not appear on s_fc_q[MAIN]. They may hit the last iput while
@@ -1225,15 +1247,22 @@ static int ext4_fc_snapshot_inodes(journal_t *journal)
ext4_set_inode_state(inode, EXT4_STATE_FC_COMMITTING);
inodes[i++] = inode;
}
+unlock:
ext4_fc_unlock(sb, alloc_ctx);
- for (nr_inodes = 0; nr_inodes < i; nr_inodes++) {
- ret = ext4_fc_snapshot_inode(inodes[nr_inodes]);
+ if (ret)
+ return ret;
+
+ for (idx = 0; idx < i; idx++) {
+ unsigned int inode_ranges = 0;
+
+ ret = ext4_fc_snapshot_inode(inodes[idx], nr_ranges,
+ &inode_ranges);
if (ret)
break;
+ nr_ranges += inode_ranges;
}
- kvfree(inodes);
return ret;
}
@@ -1244,6 +1273,8 @@ static int ext4_fc_perform_commit(journal_t *journal)
struct ext4_inode_info *iter;
struct ext4_fc_head head;
struct inode *inode;
+ struct inode **inodes;
+ unsigned int inodes_size;
struct blk_plug plug;
int ret = 0;
u32 crc = 0;
@@ -1296,6 +1327,10 @@ static int ext4_fc_perform_commit(journal_t *journal)
return ret;
+ ret = ext4_fc_alloc_snapshot_inodes(sb, &inodes, &inodes_size);
+ if (ret)
+ return ret;
+
/* Step 4: Mark all inodes as being committed. */
jbd2_journal_lock_updates(journal);
/*
@@ -1311,8 +1346,9 @@ static int ext4_fc_perform_commit(journal_t *journal)
}
ext4_fc_unlock(sb, alloc_ctx);
- ret = ext4_fc_snapshot_inodes(journal);
+ ret = ext4_fc_snapshot_inodes(journal, inodes, inodes_size);
jbd2_journal_unlock_updates(journal);
+ kvfree(inodes);
if (ret)
return ret;
@@ -1368,6 +1404,64 @@ static int ext4_fc_perform_commit(journal_t *journal)
return ret;
}
+static unsigned int ext4_fc_count_snapshot_inodes(struct super_block *sb)
+{
+ struct ext4_sb_info *sbi = EXT4_SB(sb);
+ struct ext4_inode_info *iter;
+ struct ext4_fc_dentry_update *fc_dentry;
+ unsigned int nr_inodes = 0;
+ int alloc_ctx;
+
+ alloc_ctx = ext4_fc_lock(sb);
+ list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list)
+ nr_inodes++;
+
+ list_for_each_entry(fc_dentry, &sbi->s_fc_dentry_q[FC_Q_MAIN], fcd_list) {
+ struct ext4_inode_info *ei;
+
+ if (fc_dentry->fcd_op != EXT4_FC_TAG_CREAT)
+ continue;
+ if (list_empty(&fc_dentry->fcd_dilist))
+ continue;
+
+ /* See the comment in ext4_fc_commit_dentry_updates(). */
+ ei = list_first_entry(&fc_dentry->fcd_dilist,
+ struct ext4_inode_info, i_fc_dilist);
+ if (!list_empty(&ei->i_fc_list))
+ continue;
+
+ nr_inodes++;
+ }
+ ext4_fc_unlock(sb, alloc_ctx);
+
+ return nr_inodes;
+}
+
+static int ext4_fc_alloc_snapshot_inodes(struct super_block *sb,
+ struct inode ***inodesp,
+ unsigned int *nr_inodesp)
+{
+ unsigned int nr_inodes = ext4_fc_count_snapshot_inodes(sb);
+ struct inode **inodes;
+
+ *inodesp = NULL;
+ *nr_inodesp = 0;
+
+ if (!nr_inodes)
+ return 0;
+
+ if (nr_inodes > EXT4_FC_SNAPSHOT_MAX_INODES)
+ return -E2BIG;
+
+ inodes = kvcalloc(nr_inodes, sizeof(*inodes), GFP_NOFS);
+ if (!inodes)
+ return -ENOMEM;
+
+ *inodesp = inodes;
+ *nr_inodesp = nr_inodes;
+ return 0;
+}
+
static void ext4_fc_update_stats(struct super_block *sb, int status,
u64 commit_time, int nblks, tid_t commit_tid)
{
@@ -1460,7 +1554,10 @@ int ext4_fc_commit(journal_t *journal, tid_t commit_tid)
fc_bufs_before = (sbi->s_fc_bytes + bsize - 1) / bsize;
ret = ext4_fc_perform_commit(journal);
if (ret < 0) {
- status = EXT4_FC_STATUS_FAILED;
+ if (ret == -EAGAIN || ret == -E2BIG || ret == -ECANCELED)
+ status = EXT4_FC_STATUS_INELIGIBLE;
+ else
+ status = EXT4_FC_STATUS_FAILED;
goto fallback;
}
nblks = (sbi->s_fc_bytes + bsize - 1) / bsize - fc_bufs_before;
@@ -1544,34 +1641,35 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid)
while (!list_empty(&sbi->s_fc_dentry_q[FC_Q_MAIN])) {
fc_dentry = list_first_entry(&sbi->s_fc_dentry_q[FC_Q_MAIN],
- struct ext4_fc_dentry_update,
- fcd_list);
+ struct ext4_fc_dentry_update,
+ fcd_list);
list_del_init(&fc_dentry->fcd_list);
if (fc_dentry->fcd_op == EXT4_FC_TAG_CREAT &&
- !list_empty(&fc_dentry->fcd_dilist)) {
+ !list_empty(&fc_dentry->fcd_dilist)) {
/* See the comment in ext4_fc_commit_dentry_updates(). */
ei = list_first_entry(&fc_dentry->fcd_dilist,
- struct ext4_inode_info,
- i_fc_dilist);
+ struct ext4_inode_info,
+ i_fc_dilist);
ext4_fc_free_inode_snap(&ei->vfs_inode);
spin_lock(&ei->i_fc_lock);
ext4_clear_inode_state(&ei->vfs_inode,
- EXT4_STATE_FC_REQUEUE);
+ EXT4_STATE_FC_REQUEUE);
ext4_clear_inode_state(&ei->vfs_inode,
- EXT4_STATE_FC_COMMITTING);
+ EXT4_STATE_FC_COMMITTING);
spin_unlock(&ei->i_fc_lock);
/*
* Make sure clearing of EXT4_STATE_FC_COMMITTING is
- * visible before we send the wakeup. Pairs with implicit
- * barrier in prepare_to_wait() in ext4_fc_del().
+ * visible before we send the wakeup. Pairs with
+ * implicit barrier in prepare_to_wait() in
+ * ext4_fc_del().
*/
smp_mb();
#if (BITS_PER_LONG < 64)
wake_up_bit(&ei->i_state_flags,
- EXT4_STATE_FC_COMMITTING);
+ EXT4_STATE_FC_COMMITTING);
#else
wake_up_bit(&ei->i_flags,
- EXT4_STATE_FC_COMMITTING);
+ EXT4_STATE_FC_COMMITTING);
#endif
}
list_del_init(&fc_dentry->fcd_dilist);
@@ -2548,13 +2646,20 @@ int __init ext4_fc_init_dentry_cache(void)
ext4_fc_dentry_cachep = KMEM_CACHE(ext4_fc_dentry_update,
SLAB_RECLAIM_ACCOUNT);
- if (ext4_fc_dentry_cachep == NULL)
+ if (!ext4_fc_dentry_cachep)
return -ENOMEM;
+ ext4_fc_range_cachep = KMEM_CACHE(ext4_fc_range, SLAB_RECLAIM_ACCOUNT);
+ if (!ext4_fc_range_cachep) {
+ kmem_cache_destroy(ext4_fc_dentry_cachep);
+ return -ENOMEM;
+ }
+
return 0;
}
void ext4_fc_destroy_dentry_cache(void)
{
+ kmem_cache_destroy(ext4_fc_range_cachep);
kmem_cache_destroy(ext4_fc_dentry_cachep);
}
--
2.53.0
^ permalink raw reply related
* [RFC v7 4/7] ext4: fast commit: avoid self-deadlock in inode snapshotting
From: Li Chen @ 2026-05-11 8:42 UTC (permalink / raw)
To: Zhang Yi, Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
Ojaswin Mujoo, Ritesh Harjani (IBM), Zhang Yi, linux-ext4,
linux-kernel
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
linux-trace-kernel
In-Reply-To: <20260511084304.1559557-1-me@linux.beauty>
ext4_fc_snapshot_inodes() used igrab()/iput() to pin inodes while building
commit-time snapshots. With ext4_fc_del() waiting for
EXT4_STATE_FC_COMMITTING, iput() can trigger
ext4_clear_inode()->ext4_fc_del() in the commit thread and deadlock waiting
for the fast commit to finish.
Avoid taking extra references. Collect inode pointers under s_fc_lock and
rely on EXT4_STATE_FC_COMMITTING to pin inodes until ext4_fc_cleanup()
clears the bit.
Also set EXT4_STATE_FC_COMMITTING for create-only inodes referenced
from the dentry update queue, and wake up waiters when ext4_fc_cleanup()
clears the bit.
Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
fs/ext4/fast_commit.c | 47 ++++++++++++++++++++++++++++++++-----------
1 file changed, 35 insertions(+), 12 deletions(-)
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 273bf34031ae..f9bb18c0b549 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -1195,13 +1195,12 @@ static int ext4_fc_snapshot_inodes(journal_t *journal)
alloc_ctx = ext4_fc_lock(sb);
list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
- inodes[i] = igrab(&iter->vfs_inode);
- if (inodes[i])
- i++;
+ inodes[i++] = &iter->vfs_inode;
}
list_for_each_entry(fc_dentry, &sbi->s_fc_dentry_q[FC_Q_MAIN], fcd_list) {
struct ext4_inode_info *ei;
+ struct inode *inode;
if (fc_dentry->fcd_op != EXT4_FC_TAG_CREAT)
continue;
@@ -1211,12 +1210,20 @@ static int ext4_fc_snapshot_inodes(journal_t *journal)
/* See the comment in ext4_fc_commit_dentry_updates(). */
ei = list_first_entry(&fc_dentry->fcd_dilist,
struct ext4_inode_info, i_fc_dilist);
+ inode = &ei->vfs_inode;
if (!list_empty(&ei->i_fc_list))
continue;
- inodes[i] = igrab(&ei->vfs_inode);
- if (inodes[i])
- i++;
+ /*
+ * Create-only inodes may only be referenced via fcd_dilist and
+ * not appear on s_fc_q[MAIN]. They may hit the last iput while
+ * we are snapshotting, but inode eviction calls ext4_fc_del(),
+ * which waits for FC_COMMITTING to clear. Mark them FC_COMMITTING
+ * so the inode stays pinned and the snapshot stays valid until
+ * ext4_fc_cleanup().
+ */
+ ext4_set_inode_state(inode, EXT4_STATE_FC_COMMITTING);
+ inodes[i++] = inode;
}
ext4_fc_unlock(sb, alloc_ctx);
@@ -1226,10 +1233,6 @@ static int ext4_fc_snapshot_inodes(journal_t *journal)
break;
}
- for (nr_inodes = 0; nr_inodes < i; nr_inodes++) {
- if (inodes[nr_inodes])
- iput(inodes[nr_inodes]);
- }
kvfree(inodes);
return ret;
}
@@ -1297,8 +1300,9 @@ static int ext4_fc_perform_commit(journal_t *journal)
jbd2_journal_lock_updates(journal);
/*
* The journal is now locked. No more handles can start and all the
- * previous handles are now drained. We now mark the inodes on the
- * commit queue as being committed.
+ * previous handles are now drained. Snapshotting happens in this
+ * window so log writing can consume only stable snapshots without
+ * doing logical-to-physical mapping.
*/
alloc_ctx = ext4_fc_lock(sb);
list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
@@ -1550,6 +1554,25 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid)
struct ext4_inode_info,
i_fc_dilist);
ext4_fc_free_inode_snap(&ei->vfs_inode);
+ spin_lock(&ei->i_fc_lock);
+ ext4_clear_inode_state(&ei->vfs_inode,
+ EXT4_STATE_FC_REQUEUE);
+ ext4_clear_inode_state(&ei->vfs_inode,
+ EXT4_STATE_FC_COMMITTING);
+ spin_unlock(&ei->i_fc_lock);
+ /*
+ * Make sure clearing of EXT4_STATE_FC_COMMITTING is
+ * visible before we send the wakeup. Pairs with implicit
+ * barrier in prepare_to_wait() in ext4_fc_del().
+ */
+ smp_mb();
+#if (BITS_PER_LONG < 64)
+ wake_up_bit(&ei->i_state_flags,
+ EXT4_STATE_FC_COMMITTING);
+#else
+ wake_up_bit(&ei->i_flags,
+ EXT4_STATE_FC_COMMITTING);
+#endif
}
list_del_init(&fc_dentry->fcd_dilist);
--
2.53.0
^ permalink raw reply related
* [RFC v7 3/7] ext4: fast commit: avoid waiting for FC_COMMITTING
From: Li Chen @ 2026-05-11 8:42 UTC (permalink / raw)
To: Zhang Yi, Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
Ojaswin Mujoo, Ritesh Harjani (IBM), Zhang Yi, linux-ext4,
linux-kernel
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
linux-trace-kernel
In-Reply-To: <20260511084304.1559557-1-me@linux.beauty>
ext4_fc_track_inode() can be called while holding i_data_sem (e.g.
fallocate). Waiting for EXT4_STATE_FC_COMMITTING in that case risks an
ABBA deadlock: i_data_sem -> wait(FC_COMMITTING) vs FC_COMMITTING ->
wait(i_data_sem) in the commit task.
Now that fast commit snapshots inode state at commit time, updates during
log writing do not need to block. Drop the wait and lockdep assertion in
ext4_fc_track_inode(), and make ext4_fc_del() wait for FC_COMMITTING so an
inode cannot be removed while the commit thread is still using it.
When an inode is modified during a fast commit, mark it with
EXT4_STATE_FC_REQUEUE so cleanup keeps it queued for the next fast commit.
This is needed because jbd2_fc_end_commit() invokes the cleanup callback
with tid == 0, so tid-based requeue logic would requeue every inode.
Testing: tracepoint ext4:ext4_fc_commit_stop with two fsyncs in the same
transaction. nblks is the number of journal blocks written for that fast
commit. Before this change, the second fsync still wrote almost the same
fast commit log (nblks 10->9), because tid == 0 in jbd2_fc_end_commit()
caused the tid-based requeue logic to keep all inodes queued. After this
change, only inodes modified during the commit are requeued, and the
second fsync wrote a nearly empty fast commit (nblks 10->1).
Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
fs/ext4/ext4.h | 1 +
fs/ext4/fast_commit.c | 111 ++++++++++++++++++++----------------------
2 files changed, 53 insertions(+), 59 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 05c8f67625b4..2a706acdfaf8 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1991,6 +1991,7 @@ enum {
EXT4_STATE_FC_COMMITTING, /* Fast commit ongoing */
EXT4_STATE_FC_FLUSHING_DATA, /* Fast commit flushing data */
EXT4_STATE_ORPHAN_FILE, /* Inode orphaned in orphan file */
+ EXT4_STATE_FC_REQUEUE, /* Inode modified during fast commit */
};
#define EXT4_INODE_BIT_FNS(name, field, offset) \
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index cd4eac4e7dcb..273bf34031ae 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -62,9 +62,8 @@
* setting "EXT4_STATE_FC_COMMITTING" state, and snapshot the inode state
* needed for log writing.
* [5] Unlock the journal by calling jbd2_journal_unlock_updates(). This allows
- * starting of new handles. If new handles try to start an update on
- * any of the inodes that are being committed, ext4_fc_track_inode()
- * will block until those inodes have finished the fast commit.
+ * starting of new handles. Updates to inodes being fast committed are
+ * tracked for requeue rather than blocking.
* [6] Commit all the directory entry updates in the fast commit space.
* [7] Commit all the changed inodes in the fast commit space.
* [8] Write tail tag (this tag ensures the atomicity, please read the following
@@ -218,6 +217,7 @@ void ext4_fc_init_inode(struct inode *inode)
ext4_fc_reset_inode(inode);
ext4_clear_inode_state(inode, EXT4_STATE_FC_COMMITTING);
+ ext4_clear_inode_state(inode, EXT4_STATE_FC_REQUEUE);
INIT_LIST_HEAD(&ei->i_fc_list);
INIT_LIST_HEAD(&ei->i_fc_dilist);
ei->i_fc_snap = NULL;
@@ -257,22 +257,30 @@ void ext4_fc_del(struct inode *inode)
}
/*
- * Since ext4_fc_del is called from ext4_evict_inode while having a
- * handle open, there is no need for us to wait here even if a fast
- * commit is going on. That is because, if this inode is being
- * committed, ext4_mark_inode_dirty would have waited for inode commit
- * operation to finish before we come here. So, by the time we come
- * here, inode's EXT4_STATE_FC_COMMITTING would have been cleared. So,
- * we shouldn't see EXT4_STATE_FC_COMMITTING to be set on this inode
- * here.
- *
- * We may come here without any handles open in the "no_delete" case of
- * ext4_evict_inode as well. However, if that happens, we first mark the
- * file system as fast commit ineligible anyway. So, even in that case,
- * it is okay to remove the inode from the fc list.
+ * Wait for ongoing fast commit to finish. We cannot remove the inode
+ * from fast commit lists while it is being committed.
*/
- WARN_ON(ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)
- && !ext4_test_mount_flag(inode->i_sb, EXT4_MF_FC_INELIGIBLE));
+ while (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) {
+#if (BITS_PER_LONG < 64)
+ DEFINE_WAIT_BIT(wait, &ei->i_state_flags,
+ EXT4_STATE_FC_COMMITTING);
+ wq = bit_waitqueue(&ei->i_state_flags,
+ EXT4_STATE_FC_COMMITTING);
+#else
+ DEFINE_WAIT_BIT(wait, &ei->i_flags,
+ EXT4_STATE_FC_COMMITTING);
+ wq = bit_waitqueue(&ei->i_flags,
+ EXT4_STATE_FC_COMMITTING);
+#endif
+ prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE);
+ if (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) {
+ ext4_fc_unlock(inode->i_sb, alloc_ctx);
+ schedule();
+ alloc_ctx = ext4_fc_lock(inode->i_sb);
+ }
+ finish_wait(wq, &wait.wq_entry);
+ }
+
while (ext4_test_inode_state(inode, EXT4_STATE_FC_FLUSHING_DATA)) {
#if (BITS_PER_LONG < 64)
DEFINE_WAIT_BIT(wait, &ei->i_state_flags,
@@ -293,19 +301,22 @@ void ext4_fc_del(struct inode *inode)
}
finish_wait(wq, &wait.wq_entry);
}
+
ext4_fc_free_inode_snap(inode);
list_del_init(&ei->i_fc_list);
/*
- * Since this inode is getting removed, let's also remove all FC
- * dentry create references, since it is not needed to log it anyways.
+ * Since this inode is getting removed, let's also remove all FC dentry
+ * create references, since it is not needed to log it anyways.
*/
if (list_empty(&ei->i_fc_dilist)) {
ext4_fc_unlock(inode->i_sb, alloc_ctx);
return;
}
- fc_dentry = list_first_entry(&ei->i_fc_dilist, struct ext4_fc_dentry_update, fcd_dilist);
+ fc_dentry = list_first_entry(&ei->i_fc_dilist,
+ struct ext4_fc_dentry_update,
+ fcd_dilist);
WARN_ON(fc_dentry->fcd_op != EXT4_FC_TAG_CREAT);
list_del_init(&fc_dentry->fcd_list);
list_del_init(&fc_dentry->fcd_dilist);
@@ -377,6 +388,8 @@ static int ext4_fc_track_template(
tid = handle->h_transaction->t_tid;
spin_lock(&ei->i_fc_lock);
+ if (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING))
+ ext4_set_inode_state(inode, EXT4_STATE_FC_REQUEUE);
if (tid == ei->i_sync_tid) {
update = true;
} else {
@@ -547,8 +560,6 @@ static int __track_inode(handle_t *handle, struct inode *inode, void *arg,
void ext4_fc_track_inode(handle_t *handle, struct inode *inode)
{
- struct ext4_inode_info *ei = EXT4_I(inode);
- wait_queue_head_t *wq;
int ret;
if (S_ISDIR(inode->i_mode))
@@ -564,29 +575,11 @@ void ext4_fc_track_inode(handle_t *handle, struct inode *inode)
return;
/*
- * If we come here, we may sleep while waiting for the inode to
- * commit. We shouldn't be holding i_data_sem when we go to sleep since
- * the commit path needs to grab the lock while committing the inode.
+ * Fast commit snapshots inode state at commit time, so there's no need
+ * to wait for EXT4_STATE_FC_COMMITTING here. If the inode is already
+ * on the commit queue, ext4_fc_cleanup() will requeue it for the new
+ * transaction once the current commit finishes.
*/
- lockdep_assert_not_held(&ei->i_data_sem);
-
- while (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) {
-#if (BITS_PER_LONG < 64)
- DEFINE_WAIT_BIT(wait, &ei->i_state_flags,
- EXT4_STATE_FC_COMMITTING);
- wq = bit_waitqueue(&ei->i_state_flags,
- EXT4_STATE_FC_COMMITTING);
-#else
- DEFINE_WAIT_BIT(wait, &ei->i_flags,
- EXT4_STATE_FC_COMMITTING);
- wq = bit_waitqueue(&ei->i_flags,
- EXT4_STATE_FC_COMMITTING);
-#endif
- prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE);
- if (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING))
- schedule();
- finish_wait(wq, &wait.wq_entry);
- }
/*
* From this point on, this inode will not be committed either
@@ -1510,32 +1503,32 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid)
alloc_ctx = ext4_fc_lock(sb);
while (!list_empty(&sbi->s_fc_q[FC_Q_MAIN])) {
+ bool requeue;
+
ei = list_first_entry(&sbi->s_fc_q[FC_Q_MAIN],
struct ext4_inode_info,
i_fc_list);
list_del_init(&ei->i_fc_list);
ext4_fc_free_inode_snap(&ei->vfs_inode);
+ spin_lock(&ei->i_fc_lock);
+ if (full)
+ requeue = !tid_geq(tid, ei->i_sync_tid);
+ else
+ requeue = ext4_test_inode_state(&ei->vfs_inode,
+ EXT4_STATE_FC_REQUEUE);
+ if (!requeue)
+ ext4_fc_reset_inode(&ei->vfs_inode);
+ ext4_clear_inode_state(&ei->vfs_inode, EXT4_STATE_FC_REQUEUE);
ext4_clear_inode_state(&ei->vfs_inode,
EXT4_STATE_FC_COMMITTING);
- if (tid_geq(tid, ei->i_sync_tid)) {
- ext4_fc_reset_inode(&ei->vfs_inode);
- } else if (full) {
- /*
- * We are called after a full commit, inode has been
- * modified while the commit was running. Re-enqueue
- * the inode into STAGING, which will then be splice
- * back into MAIN. This cannot happen during
- * fastcommit because the journal is locked all the
- * time in that case (and tid doesn't increase so
- * tid check above isn't reliable).
- */
+ spin_unlock(&ei->i_fc_lock);
+ if (requeue)
list_add_tail(&ei->i_fc_list,
&sbi->s_fc_q[FC_Q_STAGING]);
- }
/*
* Make sure clearing of EXT4_STATE_FC_COMMITTING is
* visible before we send the wakeup. Pairs with implicit
- * barrier in prepare_to_wait() in ext4_fc_track_inode().
+ * barrier in prepare_to_wait() in ext4_fc_del().
*/
smp_mb();
#if (BITS_PER_LONG < 64)
--
2.53.0
^ permalink raw reply related
* [RFC v7 2/7] ext4: lockdep: handle i_data_sem subclassing for special inodes
From: Li Chen @ 2026-05-11 8:42 UTC (permalink / raw)
To: Zhang Yi, Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
Ojaswin Mujoo, Ritesh Harjani (IBM), Zhang Yi, linux-ext4,
linux-kernel
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
linux-trace-kernel
In-Reply-To: <20260511084304.1559557-1-me@linux.beauty>
Fast commit can hold s_fc_lock while writing journal blocks. Mapping the
journal inode can take its i_data_sem. Normal inode update paths can take a
data inode i_data_sem and then s_fc_lock, which makes lockdep report a
circular dependency.
lockdep treats all i_data_sem instances as one lock class and cannot
distinguish the journal inode i_data_sem from a regular inode i_data_sem.
The journal inode is not tracked by fast commit and no FC waiters ever
depend on it, so this is not a real ABBA deadlock. Assign the journal inode
a dedicated i_data_sem lockdep subclass to avoid the false positive.
Inode cache objects can be recycled, so also reset i_data_sem to
I_DATA_SEM_NORMAL when allocating an ext4 inode. Otherwise a new inode may
inherit an old subclass (journal/quota/ea) and trigger lockdep warnings.
Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
Changes in v6:
- Rebase onto linux-next master as of 2026-04-08.
- Refresh the patch context around upstream ext4_alloc_inode() changes,
without changing the subclassing logic.
fs/ext4/ext4.h | 4 +++-
fs/ext4/super.c | 8 ++++++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index e01d00dbc077..05c8f67625b4 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1015,12 +1015,14 @@ do { \
* than the first
* I_DATA_SEM_QUOTA - Used for quota inodes only
* I_DATA_SEM_EA - Used for ea_inodes only
+ * I_DATA_SEM_JOURNAL - Used for journal inode only
*/
enum {
I_DATA_SEM_NORMAL = 0,
I_DATA_SEM_OTHER,
I_DATA_SEM_QUOTA,
- I_DATA_SEM_EA
+ I_DATA_SEM_EA,
+ I_DATA_SEM_JOURNAL
};
struct ext4_fc_inode_snap;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 6a77db4d3124..3c869f0001c5 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1431,6 +1431,9 @@ static struct inode *ext4_alloc_inode(struct super_block *sb)
ext4_fc_init_inode(&ei->vfs_inode);
spin_lock_init(&ei->i_fc_lock);
mmb_init(&ei->i_metadata_bhs, &ei->vfs_inode.i_data);
+#ifdef CONFIG_LOCKDEP
+ lockdep_set_subclass(&ei->i_data_sem, I_DATA_SEM_NORMAL);
+#endif
return &ei->vfs_inode;
}
@@ -5910,6 +5913,11 @@ static struct inode *ext4_get_journal_inode(struct super_block *sb,
return ERR_PTR(-EFSCORRUPTED);
}
+#ifdef CONFIG_LOCKDEP
+ lockdep_set_subclass(&EXT4_I(journal_inode)->i_data_sem,
+ I_DATA_SEM_JOURNAL);
+#endif
+
ext4_debug("Journal inode found at %p: %lld bytes\n",
journal_inode, journal_inode->i_size);
return journal_inode;
--
2.53.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox