* [PATCHv3 bpf-next 2/8] ftrace: Export some of hash related functions
From: Jiri Olsa @ 2025-11-20 21:23 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
In-Reply-To: <20251120212402.466524-1-jolsa@kernel.org>
We are going to use these functions in following changes.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
include/linux/ftrace.h | 16 ++++++++++++++++
kernel/trace/ftrace.c | 7 +++----
kernel/trace/trace.h | 8 --------
3 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 07f8c309e432..5752553bff60 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -405,6 +405,22 @@ enum ftrace_ops_cmd {
typedef int (*ftrace_ops_func_t)(struct ftrace_ops *op, enum ftrace_ops_cmd cmd);
#ifdef CONFIG_DYNAMIC_FTRACE
+
+#define FTRACE_HASH_DEFAULT_BITS 10
+
+struct ftrace_hash {
+ unsigned long size_bits;
+ struct hlist_head *buckets;
+ unsigned long count;
+ unsigned long flags;
+ struct rcu_head rcu;
+};
+
+struct ftrace_hash *alloc_ftrace_hash(int size_bits);
+void free_ftrace_hash(struct ftrace_hash *hash);
+struct ftrace_func_entry *add_hash_entry_direct(struct ftrace_hash *hash,
+ unsigned long ip, unsigned long direct);
+
/* The hash used to know what functions callbacks trace */
struct ftrace_ops_hash {
struct ftrace_hash __rcu *notrace_hash;
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 0e3714b796d9..e6ccf572c5f6 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -68,7 +68,6 @@
})
/* hash bits for specific function selection */
-#define FTRACE_HASH_DEFAULT_BITS 10
#define FTRACE_HASH_MAX_BITS 12
#ifdef CONFIG_DYNAMIC_FTRACE
@@ -1185,7 +1184,7 @@ static void __add_hash_entry(struct ftrace_hash *hash,
hash->count++;
}
-static struct ftrace_func_entry *
+struct ftrace_func_entry *
add_hash_entry_direct(struct ftrace_hash *hash, unsigned long ip, unsigned long direct)
{
struct ftrace_func_entry *entry;
@@ -1265,7 +1264,7 @@ static void clear_ftrace_mod_list(struct list_head *head)
mutex_unlock(&ftrace_lock);
}
-static void free_ftrace_hash(struct ftrace_hash *hash)
+void free_ftrace_hash(struct ftrace_hash *hash)
{
if (!hash || hash == EMPTY_HASH)
return;
@@ -1305,7 +1304,7 @@ void ftrace_free_filter(struct ftrace_ops *ops)
}
EXPORT_SYMBOL_GPL(ftrace_free_filter);
-static struct ftrace_hash *alloc_ftrace_hash(int size_bits)
+struct ftrace_hash *alloc_ftrace_hash(int size_bits)
{
struct ftrace_hash *hash;
int size;
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 85eabb454bee..62e0ac625f65 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -899,14 +899,6 @@ enum {
FTRACE_HASH_FL_MOD = (1 << 0),
};
-struct ftrace_hash {
- unsigned long size_bits;
- struct hlist_head *buckets;
- unsigned long count;
- unsigned long flags;
- struct rcu_head rcu;
-};
-
struct ftrace_func_entry *
ftrace_lookup_ip(struct ftrace_hash *hash, unsigned long ip);
--
2.51.1
^ permalink raw reply related
* [PATCHv3 bpf-next 1/8] ftrace: Make alloc_and_copy_ftrace_hash direct friendly
From: Jiri Olsa @ 2025-11-20 21:23 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
In-Reply-To: <20251120212402.466524-1-jolsa@kernel.org>
Make alloc_and_copy_ftrace_hash to copy also direct address
for each hash entry.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
kernel/trace/ftrace.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 59cfacb8a5bb..0e3714b796d9 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1186,7 +1186,7 @@ static void __add_hash_entry(struct ftrace_hash *hash,
}
static struct ftrace_func_entry *
-add_hash_entry(struct ftrace_hash *hash, unsigned long ip)
+add_hash_entry_direct(struct ftrace_hash *hash, unsigned long ip, unsigned long direct)
{
struct ftrace_func_entry *entry;
@@ -1195,11 +1195,18 @@ add_hash_entry(struct ftrace_hash *hash, unsigned long ip)
return NULL;
entry->ip = ip;
+ entry->direct = direct;
__add_hash_entry(hash, entry);
return entry;
}
+static struct ftrace_func_entry *
+add_hash_entry(struct ftrace_hash *hash, unsigned long ip)
+{
+ return add_hash_entry_direct(hash, ip, 0);
+}
+
static void
free_hash_entry(struct ftrace_hash *hash,
struct ftrace_func_entry *entry)
@@ -1372,7 +1379,7 @@ alloc_and_copy_ftrace_hash(int size_bits, struct ftrace_hash *hash)
size = 1 << hash->size_bits;
for (i = 0; i < size; i++) {
hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
- if (add_hash_entry(new_hash, entry->ip) == NULL)
+ if (add_hash_entry_direct(new_hash, entry->ip, entry->direct) == NULL)
goto free_hash;
}
}
--
2.51.1
^ permalink raw reply related
* [PATCHv3 bpf-next 0/8] ftrace,bpf: Use single direct ops for bpf trampolines
From: Jiri Olsa @ 2025-11-20 21:23 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
hi,
while poking the multi-tracing interface I ended up with just one ftrace_ops
object to attach all trampolines.
This change allows to use less direct API calls during the attachment changes
in the future code, so in effect speeding up the attachment.
In current code we get a speed up from using just a single ftrace_ops object.
- with current code:
Performance counter stats for 'bpftrace -e fentry:vmlinux:ksys_* {} -c true':
6,364,157,902 cycles:k
828,728,902 cycles:u
1,064,803,824 instructions:u # 1.28 insn per cycle
23,797,500,067 instructions:k # 3.74 insn per cycle
4.416004987 seconds time elapsed
0.164121000 seconds user
1.289550000 seconds sys
- with the fix:
Performance counter stats for 'bpftrace -e fentry:vmlinux:ksys_* {} -c true':
6,535,857,905 cycles:k
810,809,429 cycles:u
1,064,594,027 instructions:u # 1.31 insn per cycle
23,962,552,894 instructions:k # 3.67 insn per cycle
1.666961239 seconds time elapsed
0.157412000 seconds user
1.283396000 seconds sys
The speedup seems to be related to the fact that with single ftrace_ops object
we don't call ftrace_shutdown anymore (we use ftrace_update_ops instead) and
we skip the synchronize rcu calls (each ~100ms) at the end of that function.
rfc: https://lore.kernel.org/bpf/20250729102813.1531457-1-jolsa@kernel.org/
v1: https://lore.kernel.org/bpf/20250923215147.1571952-1-jolsa@kernel.org/
v2: https://lore.kernel.org/bpf/20251113123750.2507435-1-jolsa@kernel.org/
v3 changes:
- rebased on top of bpf-next/master
- fixed update_ftrace_direct_del cleanup path
- added missing inline to update_ftrace_direct_* stubs
v2 changes:
- rebased on top fo bpf-next/master plus Song's livepatch fixes [1]
- renamed the API functions [2] [Steven]
- do not export the new api [Steven]
- kept the original direct interface:
I'm not sure if we want to melt both *_ftrace_direct and the new interface
into single one. It's bit different in semantic (hence the name change as
Steven suggested [2]) and I don't think the changes are not that big so
we could easily keep both APIs.
v1 changes:
- make the change x86 specific, after discussing with Mark options for
arm64 [Mark]
thanks,
jirka
[1] https://lore.kernel.org/bpf/20251027175023.1521602-1-song@kernel.org/
[2] https://lore.kernel.org/bpf/20250924050415.4aefcb91@batman.local.home/
---
Jiri Olsa (8):
ftrace: Make alloc_and_copy_ftrace_hash direct friendly
ftrace: Export some of hash related functions
ftrace: Add update_ftrace_direct_add function
ftrace: Add update_ftrace_direct_del function
ftrace: Add update_ftrace_direct_mod function
bpf: Add trampoline ip hash table
ftrace: Factor ftrace_ops ops_func interface
bpf, x86: Use single ftrace_ops for direct calls
arch/x86/Kconfig | 1 +
include/linux/bpf.h | 7 ++-
include/linux/ftrace.h | 37 +++++++++++++-
kernel/bpf/trampoline.c | 205 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------
kernel/trace/Kconfig | 3 ++
kernel/trace/ftrace.c | 337 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
kernel/trace/trace.h | 8 ---
7 files changed, 547 insertions(+), 51 deletions(-)
^ permalink raw reply
* [PATCH v9 1/2] lib: Introduce hierarchical per-cpu counters
From: Mathieu Desnoyers @ 2025-11-20 21:03 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, Mathieu Desnoyers, Paul E. McKenney, Steven Rostedt,
Masami Hiramatsu, Dennis Zhou, Tejun Heo, Christoph Lameter,
Martin Liu, David Rientjes, christian.koenig, Shakeel Butt,
SeongJae Park, Michal Hocko, Johannes Weiner, Sweet Tea Dorminy,
Lorenzo Stoakes, Liam R . Howlett, Mike Rapoport,
Suren Baghdasaryan, Vlastimil Babka, Christian Brauner, Wei Yang,
David Hildenbrand, Miaohe Lin, Al Viro, linux-mm,
linux-trace-kernel, Yu Zhao, Roman Gushchin, Mateusz Guzik,
Matthew Wilcox, Baolin Wang, Aboorva Devarajan
In-Reply-To: <20251120210354.1233994-1-mathieu.desnoyers@efficios.com>
* Motivation
The purpose of this hierarchical split-counter scheme is to:
- Minimize contention when incrementing and decrementing counters,
- Provide fast access to a sum approximation,
- Provide a sum approximation with an acceptable accuracy level when
scaling to many-core systems.
- Provide approximate and precise comparison of two counters, and
between a counter and a value.
It aims at fixing the per-mm RSS tracking which has become too
inaccurate for OOM killer purposes on large many-core systems [1].
* Design
The hierarchical per-CPU counters propagate a sum approximation through
a N-way tree. When reaching the batch size, the carry is propagated
through a binary tree which consists of logN(nr_cpu_ids) levels. The
batch size for each level is twice the batch size of the prior level.
Example propagation diagram with 8 cpus through a binary tree:
Level 0: 0 1 2 3 4 5 6 7
| / | / | / | /
| / | / | / | /
| / | / | / | /
Level 1: 0 1 2 3
| / | /
| / | /
| / | /
Level 2: 0 1
| /
| /
| /
Level 3: 0
For a binary tree, the maximum inaccuracy is bound by:
batch_size * log2(nr_cpus) * nr_cpus
which evolves with O(n*log(n)) as the number of CPUs increases.
For a N-way tree, the maximum inaccuracy can be pre-calculated
based on the the N-arity of each level and the batch size.
Link: https://lore.kernel.org/lkml/20250331223516.7810-2-sweettea-kernel@dorminy.me/ # [1]
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Dennis Zhou <dennis@kernel.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Christoph Lameter <cl@linux.com>
Cc: Martin Liu <liumartin@google.com>
Cc: David Rientjes <rientjes@google.com>
Cc: christian.koenig@amd.com
Cc: Shakeel Butt <shakeel.butt@linux.dev>
Cc: SeongJae Park <sj@kernel.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: "Liam R . Howlett" <liam.howlett@oracle.com>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Miaohe Lin <linmiaohe@huawei.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-mm@kvack.org
Cc: linux-trace-kernel@vger.kernel.org
Cc: Yu Zhao <yuzhao@google.com>
Cc: Roman Gushchin <roman.gushchin@linux.dev>
Cc: Mateusz Guzik <mjguzik@gmail.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Aboorva Devarajan <aboorvad@linux.ibm.com>
---
Changes since v8:
- Remove migrate guard from the fast path. It does not
matter through which path the carry is propagated up
the tree.
- Rebase on top of v6.18-rc6.
- Introduce percpu_counter_tree_init_many and
percpu_counter_tree_destroy_many APIs.
- Move tree items allocation to the caller.
- Introduce percpu_counter_tree_items_size().
- Move percpu_counter_tree_subsystem_init() call before mm_core_init()
so percpu_counter_tree_items_size() is initialized before it is used.
Changes since v7:
- Explicitly initialize the subsystem from start_kernel() right
after mm_core_init() so it is up and running before the creation of
the first mm at boot.
- Remove the module.h include which is not needed with the explicit
initialization.
- Only consider levels>0 items for order={0,1} nr_items. No
functional change except to allocate only the amount of memory
which is strictly needed.
- Introduce positive precise sum API to handle a scenario where an
unlucky precise sum iteration would hit negative counter values
concurrently with counter updates.
Changes since v5:
- Introduce percpu_counter_tree_approximate_sum_positive.
- Introduce !CONFIG_SMP static inlines for UP build.
- Remove percpu_counter_tree_set_bias from the public API and make it
static.
Changes since v3:
- Add gfp flags to init function.
Changes since v2:
- Introduce N-way tree to reduce tree depth on larger systems.
Changes since v1:
- Remove percpu_counter_tree_precise_sum_unbiased from public header,
make this function static,
- Introduce precise and approximate comparisons between two counters,
- Reorder the struct percpu_counter_tree fields,
- Introduce approx_sum field, which points to the approximate sum
for the percpu_counter_tree_approximate_sum() fast path.
---
include/linux/percpu_counter_tree.h | 239 +++++++++++++++
init/main.c | 2 +
lib/Makefile | 1 +
lib/percpu_counter_tree.c | 443 ++++++++++++++++++++++++++++
4 files changed, 685 insertions(+)
create mode 100644 include/linux/percpu_counter_tree.h
create mode 100644 lib/percpu_counter_tree.c
diff --git a/include/linux/percpu_counter_tree.h b/include/linux/percpu_counter_tree.h
new file mode 100644
index 000000000000..1f4938b67730
--- /dev/null
+++ b/include/linux/percpu_counter_tree.h
@@ -0,0 +1,239 @@
+/* SPDX-License-Identifier: GPL-2.0+ OR MIT */
+/* SPDX-FileCopyrightText: 2025 Mathieu Desnoyers <mathieu.desnoyers@efficios.com> */
+
+#ifndef _PERCPU_COUNTER_TREE_H
+#define _PERCPU_COUNTER_TREE_H
+
+#include <linux/preempt.h>
+#include <linux/atomic.h>
+#include <linux/percpu.h>
+
+#ifdef CONFIG_SMP
+
+struct percpu_counter_tree_level_item {
+ atomic_t count;
+} ____cacheline_aligned_in_smp;
+
+struct percpu_counter_tree {
+ /* Fast-path fields. */
+ unsigned int __percpu *level0;
+ unsigned int level0_bit_mask;
+ union {
+ unsigned int *i;
+ atomic_t *a;
+ } approx_sum;
+ int bias; /* bias for counter_set */
+
+ /* Slow-path fields. */
+ struct percpu_counter_tree_level_item *items;
+ unsigned int batch_size;
+ unsigned int inaccuracy; /* approximation imprecise within ± inaccuracy */
+};
+
+size_t percpu_counter_tree_items_size(void);
+int percpu_counter_tree_init_many(struct percpu_counter_tree *counters, struct percpu_counter_tree_level_item *items,
+ unsigned int nr_counters, unsigned int batch_size, gfp_t gfp_flags);
+int percpu_counter_tree_init(struct percpu_counter_tree *counter, struct percpu_counter_tree_level_item *items,
+ unsigned int batch_size, gfp_t gfp_flags);
+void percpu_counter_tree_destroy_many(struct percpu_counter_tree *counter, unsigned int nr_counters);
+void percpu_counter_tree_destroy(struct percpu_counter_tree *counter);
+void percpu_counter_tree_add_slowpath(struct percpu_counter_tree *counter, int inc);
+int percpu_counter_tree_precise_sum(struct percpu_counter_tree *counter);
+int percpu_counter_tree_approximate_compare(struct percpu_counter_tree *a, struct percpu_counter_tree *b);
+int percpu_counter_tree_approximate_compare_value(struct percpu_counter_tree *counter, int v);
+int percpu_counter_tree_precise_compare(struct percpu_counter_tree *a, struct percpu_counter_tree *b);
+int percpu_counter_tree_precise_compare_value(struct percpu_counter_tree *counter, int v);
+void percpu_counter_tree_set(struct percpu_counter_tree *counter, int v);
+unsigned int percpu_counter_tree_inaccuracy(struct percpu_counter_tree *counter);
+int percpu_counter_tree_subsystem_init(void);
+
+/* Fast paths */
+
+static inline
+int percpu_counter_tree_carry(int orig, int res, int inc, unsigned int bit_mask)
+{
+ if (inc < 0) {
+ inc = -(-inc & ~(bit_mask - 1));
+ /*
+ * xor bit_mask: underflow.
+ *
+ * If inc has bit set, decrement an additional bit if
+ * there is _no_ bit transition between orig and res.
+ * Else, inc has bit cleared, decrement an additional
+ * bit if there is a bit transition between orig and
+ * res.
+ */
+ if ((inc ^ orig ^ res) & bit_mask)
+ inc -= bit_mask;
+ } else {
+ inc &= ~(bit_mask - 1);
+ /*
+ * xor bit_mask: overflow.
+ *
+ * If inc has bit set, increment an additional bit if
+ * there is _no_ bit transition between orig and res.
+ * Else, inc has bit cleared, increment an additional
+ * bit if there is a bit transition between orig and
+ * res.
+ */
+ if ((inc ^ orig ^ res) & bit_mask)
+ inc += bit_mask;
+ }
+ return inc;
+}
+
+static inline
+void percpu_counter_tree_add(struct percpu_counter_tree *counter, int inc)
+{
+ unsigned int bit_mask = counter->level0_bit_mask, orig, res;
+
+ res = this_cpu_add_return(*counter->level0, inc);
+ orig = res - inc;
+ inc = percpu_counter_tree_carry(orig, res, inc, bit_mask);
+ if (likely(!inc))
+ return;
+ percpu_counter_tree_add_slowpath(counter, inc);
+}
+
+static inline
+int percpu_counter_tree_approximate_sum(struct percpu_counter_tree *counter)
+{
+ unsigned int v;
+
+ if (!counter->level0_bit_mask)
+ v = READ_ONCE(*counter->approx_sum.i);
+ else
+ v = atomic_read(counter->approx_sum.a);
+ return (int) (v + (unsigned int)READ_ONCE(counter->bias));
+}
+
+#else /* !CONFIG_SMP */
+
+struct percpu_counter_tree_level_item;
+
+struct percpu_counter_tree {
+ atomic_t count;
+};
+
+static inline
+size_t percpu_counter_tree_items_size(void)
+{
+ return 0;
+}
+
+static inline
+int percpu_counter_tree_init_many(struct percpu_counter_tree *counters, struct percpu_counter_tree_level_item *items,
+ unsigned int nr_counters, unsigned int batch_size, gfp_t gfp_flags)
+{
+ for (unsigned int i = 0; i < nr_counters; i++)
+ atomic_set(&counters[i].count, 0);
+ return 0;
+}
+
+static inline
+int percpu_counter_tree_init(struct percpu_counter_tree *counter, struct percpu_counter_tree_level_item *items,
+ unsigned int batch_size, gfp_t gfp_flags)
+{
+ return percpu_counter_tree_init_many(counter, items, 1, batch_size, gfp_flags);
+}
+
+static inline
+void percpu_counter_tree_destroy_many(struct percpu_counter_tree *counter, unsigned int nr_counters)
+{
+}
+
+static inline
+void percpu_counter_tree_destroy(struct percpu_counter_tree *counter)
+{
+}
+
+static inline
+int percpu_counter_tree_precise_sum(struct percpu_counter_tree *counter)
+{
+ return atomic_read(&counter->count);
+}
+
+static inline
+int percpu_counter_tree_precise_compare(struct percpu_counter_tree *a, struct percpu_counter_tree *b)
+{
+ int count_a = percpu_counter_tree_precise_sum(a),
+ count_b = percpu_counter_tree_precise_sum(b);
+
+ if (count_a == count_b)
+ return 0;
+ if (count_a < count_b)
+ return -1;
+ return 1;
+}
+
+static inline
+int percpu_counter_tree_precise_compare_value(struct percpu_counter_tree *counter, int v)
+{
+ int count = percpu_counter_tree_precise_sum(counter);
+
+ if (count == v)
+ return 0;
+ if (count < v)
+ return -1;
+ return 1;
+}
+
+static inline
+int percpu_counter_tree_approximate_compare(struct percpu_counter_tree *a, struct percpu_counter_tree *b)
+{
+ return percpu_counter_tree_precise_compare(a, b);
+}
+
+static inline
+int percpu_counter_tree_approximate_compare_value(struct percpu_counter_tree *counter, int v)
+{
+ return percpu_counter_tree_precise_compare_value(counter, v);
+}
+
+static inline
+void percpu_counter_tree_set(struct percpu_counter_tree *counter, int v)
+{
+ atomic_set(&counter->count, v);
+}
+
+static inline
+unsigned int percpu_counter_tree_inaccuracy(struct percpu_counter_tree *counter)
+{
+ return 0;
+}
+
+static inline
+void percpu_counter_tree_add(struct percpu_counter_tree *counter, int inc)
+{
+ atomic_add(inc, &counter->count);
+}
+
+static inline
+int percpu_counter_tree_approximate_sum(struct percpu_counter_tree *counter)
+{
+ return percpu_counter_tree_precise_sum(counter);
+}
+
+static inline
+int percpu_counter_tree_subsystem_init(void)
+{
+ return 0;
+}
+
+#endif /* CONFIG_SMP */
+
+static inline
+int percpu_counter_tree_approximate_sum_positive(struct percpu_counter_tree *counter)
+{
+ int v = percpu_counter_tree_approximate_sum(counter);
+ return v > 0 ? v : 0;
+}
+
+static inline
+int percpu_counter_tree_precise_sum_positive(struct percpu_counter_tree *counter)
+{
+ int v = percpu_counter_tree_precise_sum(counter);
+ return v > 0 ? v : 0;
+}
+
+#endif /* _PERCPU_COUNTER_TREE_H */
diff --git a/init/main.c b/init/main.c
index 07a3116811c5..8487489b9d00 100644
--- a/init/main.c
+++ b/init/main.c
@@ -104,6 +104,7 @@
#include <linux/pidfs.h>
#include <linux/ptdump.h>
#include <linux/time_namespace.h>
+#include <linux/percpu_counter_tree.h>
#include <net/net_namespace.h>
#include <asm/io.h>
@@ -968,6 +969,7 @@ void start_kernel(void)
vfs_caches_init_early();
sort_main_extable();
trap_init();
+ percpu_counter_tree_subsystem_init();
mm_core_init();
maple_tree_init();
poking_init();
diff --git a/lib/Makefile b/lib/Makefile
index 1ab2c4be3b66..767dc178a55c 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -179,6 +179,7 @@ obj-$(CONFIG_TEXTSEARCH_KMP) += ts_kmp.o
obj-$(CONFIG_TEXTSEARCH_BM) += ts_bm.o
obj-$(CONFIG_TEXTSEARCH_FSM) += ts_fsm.o
obj-$(CONFIG_SMP) += percpu_counter.o
+obj-$(CONFIG_SMP) += percpu_counter_tree.o
obj-$(CONFIG_AUDIT_GENERIC) += audit.o
obj-$(CONFIG_AUDIT_COMPAT_GENERIC) += compat_audit.o
diff --git a/lib/percpu_counter_tree.c b/lib/percpu_counter_tree.c
new file mode 100644
index 000000000000..5056d470bdc2
--- /dev/null
+++ b/lib/percpu_counter_tree.c
@@ -0,0 +1,443 @@
+// SPDX-License-Identifier: GPL-2.0+ OR MIT
+// SPDX-FileCopyrightText: 2025 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
+
+/*
+ * Split Counters With Tree Approximation Propagation
+ *
+ * * Propagation diagram when reaching batch size thresholds (± batch size):
+ *
+ * Example diagram for 8 CPUs:
+ *
+ * log2(8) = 3 levels
+ *
+ * At each level, each pair propagates its values to the next level when
+ * reaching the batch size thresholds.
+ *
+ * Counters at levels 0, 1, 2 can be kept on a single byte (±128 range),
+ * although it may be relevant to keep them on 32-bit counters for
+ * simplicity. (complexity vs memory footprint tradeoff)
+ *
+ * Counter at level 3 can be kept on a 32-bit counter.
+ *
+ * Level 0: 0 1 2 3 4 5 6 7
+ * | / | / | / | /
+ * | / | / | / | /
+ * | / | / | / | /
+ * Level 1: 0 1 2 3
+ * | / | /
+ * | / | /
+ * | / | /
+ * Level 2: 0 1
+ * | /
+ * | /
+ * | /
+ * Level 3: 0
+ *
+ * * Approximation inaccuracy:
+ *
+ * BATCH(level N): Level N batch size.
+ *
+ * Example for BATCH(level 0) = 32.
+ *
+ * BATCH(level 0) = 32
+ * BATCH(level 1) = 64
+ * BATCH(level 2) = 128
+ * BATCH(level N) = BATCH(level 0) * 2^N
+ *
+ * per-counter global
+ * inaccuracy inaccuracy
+ * Level 0: [ -32 .. +31] ±256 (8 * 32)
+ * Level 1: [ -64 .. +63] ±256 (4 * 64)
+ * Level 2: [-128 .. +127] ±256 (2 * 128)
+ * Total: ------ ±768 (log2(nr_cpu_ids) * BATCH(level 0) * nr_cpu_ids)
+ *
+ * -----
+ *
+ * Approximate Sum Carry Propagation
+ *
+ * Let's define a number of counter bits for each level, e.g.:
+ *
+ * log2(BATCH(level 0)) = log2(32) = 5
+ *
+ * nr_bit value_mask range
+ * Level 0: 5 bits v 0 .. +31
+ * Level 1: 1 bit (v & ~((1UL << 5) - 1)) 0 .. +63
+ * Level 2: 1 bit (v & ~((1UL << 6) - 1)) 0 .. +127
+ * Level 3: 25 bits (v & ~((1UL << 7) - 1)) 0 .. 2^32-1
+ *
+ * Note: Use a full 32-bit per-cpu counter at level 0 to allow precise sum.
+ *
+ * Note: Use cacheline aligned counters at levels above 0 to prevent false sharing.
+ * If memory footprint is an issue, a specialized allocator could be used
+ * to eliminate padding.
+ *
+ * Example with expanded values:
+ *
+ * counter_add(counter, inc):
+ *
+ * if (!inc)
+ * return;
+ *
+ * res = percpu_add_return(counter @ Level 0, inc);
+ * orig = res - inc;
+ * if (inc < 0) {
+ * inc = -(-inc & ~0b00011111); // Clear used bits
+ * // xor bit 5: underflow
+ * if ((inc ^ orig ^ res) & 0b00100000)
+ * inc -= 0b00100000;
+ * } else {
+ * inc &= ~0b00011111; // Clear used bits
+ * // xor bit 5: overflow
+ * if ((inc ^ orig ^ res) & 0b00100000)
+ * inc += 0b00100000;
+ * }
+ * if (!inc)
+ * return;
+ *
+ * res = atomic_add_return(counter @ Level 1, inc);
+ * orig = res - inc;
+ * if (inc < 0) {
+ * inc = -(-inc & ~0b00111111); // Clear used bits
+ * // xor bit 6: underflow
+ * if ((inc ^ orig ^ res) & 0b01000000)
+ * inc -= 0b01000000;
+ * } else {
+ * inc &= ~0b00111111; // Clear used bits
+ * // xor bit 6: overflow
+ * if ((inc ^ orig ^ res) & 0b01000000)
+ * inc += 0b01000000;
+ * }
+ * if (!inc)
+ * return;
+ *
+ * res = atomic_add_return(counter @ Level 2, inc);
+ * orig = res - inc;
+ * if (inc < 0) {
+ * inc = -(-inc & ~0b01111111); // Clear used bits
+ * // xor bit 7: underflow
+ * if ((inc ^ orig ^ res) & 0b10000000)
+ * inc -= 0b10000000;
+ * } else {
+ * inc &= ~0b01111111; // Clear used bits
+ * // xor bit 7: overflow
+ * if ((inc ^ orig ^ res) & 0b10000000)
+ * inc += 0b10000000;
+ * }
+ * if (!inc)
+ * return;
+ *
+ * atomic_add(counter @ Level 3, inc);
+ */
+
+#include <linux/percpu_counter_tree.h>
+#include <linux/cpumask.h>
+#include <linux/percpu.h>
+#include <linux/atomic.h>
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/math.h>
+
+#define MAX_NR_LEVELS 5
+
+struct counter_config {
+ unsigned int nr_items;
+ unsigned char nr_levels;
+ unsigned char n_arity_order[MAX_NR_LEVELS];
+};
+
+/*
+ * nr_items is the number of items in the tree for levels 1 to and
+ * including the final level (approximate sum). It excludes the level 0
+ * per-cpu counters.
+ */
+static const struct counter_config per_nr_cpu_order_config[] = {
+ [0] = { .nr_items = 0, .nr_levels = 0, .n_arity_order = { 0 } },
+ [1] = { .nr_items = 1, .nr_levels = 1, .n_arity_order = { 1 } },
+ [2] = { .nr_items = 3, .nr_levels = 2, .n_arity_order = { 1, 1 } },
+ [3] = { .nr_items = 7, .nr_levels = 3, .n_arity_order = { 1, 1, 1 } },
+ [4] = { .nr_items = 7, .nr_levels = 3, .n_arity_order = { 2, 1, 1 } },
+ [5] = { .nr_items = 11, .nr_levels = 3, .n_arity_order = { 2, 2, 1 } },
+ [6] = { .nr_items = 21, .nr_levels = 3, .n_arity_order = { 2, 2, 2 } },
+ [7] = { .nr_items = 21, .nr_levels = 3, .n_arity_order = { 3, 2, 2 } },
+ [8] = { .nr_items = 37, .nr_levels = 3, .n_arity_order = { 3, 3, 2 } },
+ [9] = { .nr_items = 73, .nr_levels = 3, .n_arity_order = { 3, 3, 3 } },
+ [10] = { .nr_items = 149, .nr_levels = 4, .n_arity_order = { 3, 3, 2, 2 } },
+ [11] = { .nr_items = 293, .nr_levels = 4, .n_arity_order = { 3, 3, 3, 2 } },
+ [12] = { .nr_items = 585, .nr_levels = 4, .n_arity_order = { 3, 3, 3, 3 } },
+ [13] = { .nr_items = 1173, .nr_levels = 5, .n_arity_order = { 3, 3, 3, 2, 2 } },
+ [14] = { .nr_items = 2341, .nr_levels = 5, .n_arity_order = { 3, 3, 3, 3, 2 } },
+ [15] = { .nr_items = 4681, .nr_levels = 5, .n_arity_order = { 3, 3, 3, 3, 3 } },
+ [16] = { .nr_items = 4681, .nr_levels = 5, .n_arity_order = { 4, 3, 3, 3, 3 } },
+ [17] = { .nr_items = 8777, .nr_levels = 5, .n_arity_order = { 4, 4, 3, 3, 3 } },
+ [18] = { .nr_items = 17481, .nr_levels = 5, .n_arity_order = { 4, 4, 4, 3, 3 } },
+ [19] = { .nr_items = 34953, .nr_levels = 5, .n_arity_order = { 4, 4, 4, 4, 3 } },
+ [20] = { .nr_items = 69905, .nr_levels = 5, .n_arity_order = { 4, 4, 4, 4, 4 } },
+};
+
+static const struct counter_config *counter_config;
+static unsigned int nr_cpus_order, inaccuracy_multiplier;
+
+static
+int __percpu_counter_tree_init(struct percpu_counter_tree *counter,
+ unsigned int batch_size, gfp_t gfp_flags,
+ unsigned int __percpu *level0,
+ struct percpu_counter_tree_level_item *items)
+{
+ /* Batch size must be power of 2 */
+ if (!batch_size || (batch_size & (batch_size - 1)))
+ return -EINVAL;
+ counter->batch_size = batch_size;
+ counter->bias = 0;
+ counter->level0 = level0;
+ counter->items = items;
+ if (!nr_cpus_order) {
+ counter->approx_sum.i = per_cpu_ptr(counter->level0, 0);
+ counter->level0_bit_mask = 0;
+ } else {
+ counter->approx_sum.a = &counter->items[counter_config->nr_items - 1].count;
+ counter->level0_bit_mask = 1UL << get_count_order(batch_size);
+ }
+ counter->inaccuracy = batch_size * inaccuracy_multiplier;
+ return 0;
+}
+
+int percpu_counter_tree_init_many(struct percpu_counter_tree *counters, struct percpu_counter_tree_level_item *items,
+ unsigned int nr_counters, unsigned int batch_size, gfp_t gfp_flags)
+{
+ void __percpu *level0, *level0_iter;
+ size_t counter_size, items_size = 0;
+ void *items_iter;
+ unsigned int i;
+ int ret;
+
+ counter_size = ALIGN(sizeof(*counters), __alignof__(*counters));
+ level0 = __alloc_percpu_gfp(nr_counters * counter_size,
+ __alignof__(*counters), gfp_flags);
+ if (!level0)
+ return -ENOMEM;
+ if (nr_cpus_order) {
+ items_size = percpu_counter_tree_items_size();
+ memset(items, 0, items_size * nr_counters);
+ }
+ level0_iter = level0;
+ items_iter = items;
+ for (i = 0; i < nr_counters; i++) {
+ ret = __percpu_counter_tree_init(&counters[i], batch_size, gfp_flags, level0_iter, items_iter);
+ if (ret)
+ goto free_level0;
+ level0_iter += counter_size;
+ if (nr_cpus_order)
+ items_iter += items_size;
+ }
+ return 0;
+
+free_level0:
+ free_percpu(level0);
+ return ret;
+}
+
+int percpu_counter_tree_init(struct percpu_counter_tree *counter, struct percpu_counter_tree_level_item *items,
+ unsigned int batch_size, gfp_t gfp_flags)
+{
+ return percpu_counter_tree_init_many(counter, items, 1, batch_size, gfp_flags);
+}
+
+void percpu_counter_tree_destroy_many(struct percpu_counter_tree *counters, unsigned int nr_counters)
+{
+ free_percpu(counters->level0);
+}
+
+void percpu_counter_tree_destroy(struct percpu_counter_tree *counter)
+{
+ return percpu_counter_tree_destroy_many(counter, 1);
+}
+
+/*
+ * It does not matter through which path the carry propagates up the
+ * tree, therefore there is no need to disable preemption because the
+ * cpu number is only used to favor cache locality.
+ */
+void percpu_counter_tree_add_slowpath(struct percpu_counter_tree *counter, int inc)
+{
+ unsigned int level_items, nr_levels = counter_config->nr_levels,
+ level, n_arity_order, bit_mask;
+ struct percpu_counter_tree_level_item *item = counter->items;
+ unsigned int cpu = raw_smp_processor_id();
+
+ WARN_ON_ONCE(!nr_cpus_order); /* Should never be called for 1 cpu. */
+
+ n_arity_order = counter_config->n_arity_order[0];
+ bit_mask = counter->level0_bit_mask << n_arity_order;
+ level_items = 1U << (nr_cpus_order - n_arity_order);
+
+ for (level = 1; level < nr_levels; level++) {
+ atomic_t *count = &item[cpu & (level_items - 1)].count;
+ unsigned int orig, res;
+
+ res = atomic_add_return_relaxed(inc, count);
+ orig = res - inc;
+ inc = percpu_counter_tree_carry(orig, res, inc, bit_mask);
+ if (likely(!inc))
+ return;
+ item += level_items;
+ n_arity_order = counter_config->n_arity_order[level];
+ level_items >>= n_arity_order;
+ bit_mask <<= n_arity_order;
+ }
+ atomic_add(inc, counter->approx_sum.a);
+}
+
+/*
+ * Precise sum. Perform the sum of all per-cpu counters.
+ */
+static int percpu_counter_tree_precise_sum_unbiased(struct percpu_counter_tree *counter)
+{
+ unsigned int sum = 0;
+ int cpu;
+
+ for_each_possible_cpu(cpu)
+ sum += *per_cpu_ptr(counter->level0, cpu);
+ return (int) sum;
+}
+
+int percpu_counter_tree_precise_sum(struct percpu_counter_tree *counter)
+{
+ return percpu_counter_tree_precise_sum_unbiased(counter) + READ_ONCE(counter->bias);
+}
+
+/*
+ * Do an approximate comparison of two counters.
+ * Return 0 if counters do not differ by more than the sum of their
+ * respective inaccuracy ranges,
+ * Return -1 if counter @a less than counter @b,
+ * Return 1 if counter @a is greater than counter @b.
+ */
+int percpu_counter_tree_approximate_compare(struct percpu_counter_tree *a, struct percpu_counter_tree *b)
+{
+ int count_a = percpu_counter_tree_approximate_sum(a),
+ count_b = percpu_counter_tree_approximate_sum(b);
+
+ if (abs(count_a - count_b) <= (a->inaccuracy + b->inaccuracy))
+ return 0;
+ if (count_a < count_b)
+ return -1;
+ return 1;
+}
+
+/*
+ * Do an approximate comparison of a counter against a given value.
+ * Return 0 if the value is within the inaccuracy range of the counter,
+ * Return -1 if the value less than counter,
+ * Return 1 if the value is greater than counter.
+ */
+int percpu_counter_tree_approximate_compare_value(struct percpu_counter_tree *counter, int v)
+{
+ int count = percpu_counter_tree_approximate_sum(counter);
+
+ if (abs(v - count) <= counter->inaccuracy)
+ return 0;
+ if (count < v)
+ return -1;
+ return 1;
+}
+
+/*
+ * Do a precise comparison of two counters.
+ * Return 0 if the counters are equal,
+ * Return -1 if counter @a less than counter @b,
+ * Return 1 if counter @a is greater than counter @b.
+ */
+int percpu_counter_tree_precise_compare(struct percpu_counter_tree *a, struct percpu_counter_tree *b)
+{
+ int count_a = percpu_counter_tree_approximate_sum(a),
+ count_b = percpu_counter_tree_approximate_sum(b);
+
+ if (abs(count_a - count_b) <= (a->inaccuracy + b->inaccuracy)) {
+ if (b->inaccuracy < a->inaccuracy) {
+ count_a = percpu_counter_tree_precise_sum(a);
+ if (abs(count_a - count_b) <= b->inaccuracy)
+ count_b = percpu_counter_tree_precise_sum(b);
+ } else {
+ count_b = percpu_counter_tree_precise_sum(b);
+ if (abs(count_a - count_b) <= a->inaccuracy)
+ count_a = percpu_counter_tree_precise_sum(a);
+ }
+ }
+ if (count_a > count_b)
+ return -1;
+ if (count_a > count_b)
+ return 1;
+ return 0;
+}
+
+/*
+ * Do a precise comparision of a counter against a given value.
+ * Return 0 if the value is equal to the counter,
+ * Return -1 if the value less than counter,
+ * Return 1 if the value is greater than counter.
+ */
+int percpu_counter_tree_precise_compare_value(struct percpu_counter_tree *counter, int v)
+{
+ int count = percpu_counter_tree_approximate_sum(counter);
+
+ if (abs(v - count) <= counter->inaccuracy)
+ count = percpu_counter_tree_precise_sum(counter);
+ if (count < v)
+ return -1;
+ if (count > v)
+ return 1;
+ return 0;
+}
+
+static
+void percpu_counter_tree_set_bias(struct percpu_counter_tree *counter, int bias)
+{
+ WRITE_ONCE(counter->bias, bias);
+}
+
+void percpu_counter_tree_set(struct percpu_counter_tree *counter, int v)
+{
+ percpu_counter_tree_set_bias(counter,
+ v - percpu_counter_tree_precise_sum_unbiased(counter));
+}
+
+unsigned int percpu_counter_tree_inaccuracy(struct percpu_counter_tree *counter)
+{
+ return counter->inaccuracy;
+}
+
+size_t percpu_counter_tree_items_size(void)
+{
+ if (!nr_cpus_order)
+ return 0;
+ return counter_config->nr_items * sizeof(struct percpu_counter_tree_level_item);
+}
+
+static unsigned int __init calculate_inaccuracy_multiplier(void)
+{
+ unsigned int nr_levels = counter_config->nr_levels, level;
+ unsigned int level_items = 1U << nr_cpus_order;
+ unsigned int inaccuracy = 0, batch_size = 1;
+
+ for (level = 0; level < nr_levels; level++) {
+ unsigned int n_arity_order = counter_config->n_arity_order[level];
+
+ inaccuracy += batch_size * level_items;
+ batch_size <<= n_arity_order;
+ level_items >>= n_arity_order;
+ }
+ return inaccuracy;
+}
+
+int __init percpu_counter_tree_subsystem_init(void)
+{
+
+ nr_cpus_order = get_count_order(nr_cpu_ids);
+ if (WARN_ON_ONCE(nr_cpus_order >= ARRAY_SIZE(per_nr_cpu_order_config))) {
+ printk(KERN_ERR "Unsupported number of CPUs (%u)\n", nr_cpu_ids);
+ return -1;
+ }
+ counter_config = &per_nr_cpu_order_config[nr_cpus_order];
+ inaccuracy_multiplier = calculate_inaccuracy_multiplier();
+ return 0;
+}
--
2.39.5
^ permalink raw reply related
* [PATCH v9 2/2] mm: Fix OOM killer inaccuracy on large many-core systems
From: Mathieu Desnoyers @ 2025-11-20 21:03 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, Mathieu Desnoyers, Paul E. McKenney, Steven Rostedt,
Masami Hiramatsu, Dennis Zhou, Tejun Heo, Christoph Lameter,
Martin Liu, David Rientjes, christian.koenig, Shakeel Butt,
SeongJae Park, Michal Hocko, Johannes Weiner, Sweet Tea Dorminy,
Lorenzo Stoakes, Liam R . Howlett, Mike Rapoport,
Suren Baghdasaryan, Vlastimil Babka, Christian Brauner, Wei Yang,
David Hildenbrand, Miaohe Lin, Al Viro, linux-mm,
linux-trace-kernel, Yu Zhao, Roman Gushchin, Mateusz Guzik,
Matthew Wilcox, Baolin Wang, Aboorva Devarajan
In-Reply-To: <20251120210354.1233994-1-mathieu.desnoyers@efficios.com>
Use hierarchical per-cpu counters for rss tracking to fix the per-mm RSS
tracking which has become too inaccurate for OOM killer purposes on
large many-core systems.
The following rss tracking issues were noted by Sweet Tea Dorminy [1],
which lead to picking wrong tasks as OOM kill target:
Recently, several internal services had an RSS usage regression as part of a
kernel upgrade. Previously, they were on a pre-6.2 kernel and were able to
read RSS statistics in a backup watchdog process to monitor and decide if
they'd overrun their memory budget. Now, however, a representative service
with five threads, expected to use about a hundred MB of memory, on a 250-cpu
machine had memory usage tens of megabytes different from the expected amount
-- this constituted a significant percentage of inaccuracy, causing the
watchdog to act.
This was a result of commit f1a7941243c1 ("mm: convert mm's rss stats
into percpu_counter") [1]. Previously, the memory error was bounded by
64*nr_threads pages, a very livable megabyte. Now, however, as a result of
scheduler decisions moving the threads around the CPUs, the memory error could
be as large as a gigabyte.
This is a really tremendous inaccuracy for any few-threaded program on a
large machine and impedes monitoring significantly. These stat counters are
also used to make OOM killing decisions, so this additional inaccuracy could
make a big difference in OOM situations -- either resulting in the wrong
process being killed, or in less memory being returned from an OOM-kill than
expected.
Here is a (possibly incomplete) list of the prior approaches that were
used or proposed, along with their downside:
1) Per-thread rss tracking: large error on many-thread processes.
2) Per-CPU counters: up to 12% slower for short-lived processes and 9%
increased system time in make test workloads [1]. Moreover, the
inaccuracy increases with O(n^2) with the number of CPUs.
3) Per-NUMA-node counters: requires atomics on fast-path (overhead),
error is high with systems that have lots of NUMA nodes (32 times
the number of NUMA nodes).
The approach proposed here is to replace this by the hierarchical
per-cpu counters, which bounds the inaccuracy based on the system
topology with O(N*logN).
commit 82241a83cd15 ("mm: fix the inaccurate memory statistics issue for
users") introduced get_mm_counter_sum() for precise /proc memory status
queries. Implement it with percpu_counter_tree_precise_sum() since it
is not a fast path and precision is preferred over speed.
* Testing results:
Test hardware: 2 sockets AMD EPYC 9654 96-Core Processor (384 logical CPUs total)
Methodology:
Comparing the current upstream implementation with the hierarchical
counters is done by keeping both implementations wired up in parallel,
and running a single-process, single-threaded program which hops
randomly across CPUs in the system, calling mmap(2) and munmap(2) on
random CPUs, keeping track of an array of allocated mappings, randomly
choosing entries to either map or unmap.
get_mm_counter() is instrumented to compare the upstream counter
approximation to the precise value, and print the delta when going over
a given threshold. The delta of the hierarchical counter approximation
to the precise value is also printed for comparison.
After a few minutes running this test, the upstream implementation
counter approximation reaches a 1GB delta from the
precise value, compared to 80MB delta with the hierarchical counter.
The hierarchical counter provides a guaranteed maximum approximation
inaccuracy of 192MB on that hardware topology.
* Fast path implementation comparison
The new inline percpu_counter_tree_add() uses a this_cpu_add_return()
for the fast path (under a certain allocation size threshold). Above
that, it calls a slow path which "trickles up" the carry to upper level
counters with atomic_add_return.
In comparison, the upstream counters implementation calls
percpu_counter_add_batch which uses this_cpu_try_cmpxchg() on the fast
path, and does a raw_spin_lock_irqsave above a certain threshold.
The hierarchical implementation is therefore expected to have less
contention on mid-sized allocations than the upstream counters because
the atomic counters tracking those bits are only shared across nearby
CPUs. In comparison, the upstream counters immediately use a global
spinlock when reaching the threshold.
* Benchmarks
Using will-it-scale page_fault1 benchmarks to compare the upstream
counters to the hierarchical counters. This is done with hyperthreading
disabled. The speedup is within the standard deviation of the upstream
runs, so the overhead is not significant.
upstream hierarchical speedup
page_fault1_processes -s 100 -t 1 614783 615558 +0.1%
page_fault1_threads -s 100 -t 1 612788 612447 -0.1%
page_fault1_processes -s 100 -t 96 37994977 37932035 -0.2%
page_fault1_threads -s 100 -t 96 2484130 2504860 +0.8%
page_fault1_processes -s 100 -t 192 71262917 71118830 -0.2%
page_fault1_threads -s 100 -t 192 2446437 2469296 +0.1%
Link: https://lore.kernel.org/lkml/20250331223516.7810-2-sweettea-kernel@dorminy.me/ # [1]
Link: https://lore.kernel.org/lkml/20250704150226.47980-1-mathieu.desnoyers@efficios.com/
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Dennis Zhou <dennis@kernel.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Christoph Lameter <cl@linux.com>
Cc: Martin Liu <liumartin@google.com>
Cc: David Rientjes <rientjes@google.com>
Cc: christian.koenig@amd.com
Cc: Shakeel Butt <shakeel.butt@linux.dev>
Cc: SeongJae Park <sj@kernel.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: "Liam R . Howlett" <liam.howlett@oracle.com>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Miaohe Lin <linmiaohe@huawei.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-mm@kvack.org
Cc: linux-trace-kernel@vger.kernel.org
Cc: Yu Zhao <yuzhao@google.com>
Cc: Roman Gushchin <roman.gushchin@linux.dev>
Cc: Mateusz Guzik <mjguzik@gmail.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Aboorva Devarajan <aboorvad@linux.ibm.com>
---
Changes since v8:
- Use percpu_counter_tree_init_many and
percpu_counter_tree_destroy_many APIs.
- Remove percpu tree items allocation. Extend mm_struct size to include
rss items. Those are handled through the new helpers
get_rss_stat_items() and get_rss_stat_items_size() and passed
as parameter to percpu_counter_tree_init_many().
Changes since v7:
- Use precise sum positive API to handle a scenario where an unlucky
precise sum iteration would observe negative counter values due to
concurrent updates.
Changes since v6:
- Rebased on v6.18-rc3.
- Implement get_mm_counter_sum as percpu_counter_tree_precise_sum for
/proc virtual files memory state queries.
Changes since v5:
- Use percpu_counter_tree_approximate_sum_positive.
Change since v4:
- get_mm_counter needs to return 0 or a positive value.
get_mm_counter_sum -> precise sum positive
---
include/linux/mm.h | 28 +++++++++++++++++++++++-----
include/linux/mm_types.h | 4 ++--
include/trace/events/kmem.h | 2 +-
kernel/fork.c | 24 ++++++++++++++----------
4 files changed, 40 insertions(+), 18 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 7c79b3369b82..cd81fa8924eb 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2681,38 +2681,56 @@ static inline bool get_user_page_fast_only(unsigned long addr,
{
return get_user_pages_fast_only(addr, 1, gup_flags, pagep) == 1;
}
+
+static inline size_t get_rss_stat_items_size(void)
+{
+ return percpu_counter_tree_items_size() * NR_MM_COUNTERS;
+}
+
+static inline struct percpu_counter_tree_level_item *get_rss_stat_items(struct mm_struct *mm)
+{
+ unsigned long ptr = (unsigned long)mm;
+
+ ptr += offsetof(struct mm_struct, cpu_bitmap);
+ /* Skip cpu_bitmap */
+ ptr += cpumask_size();
+ /* Skip mm_cidmask */
+ ptr += mm_cid_size();
+ return (struct percpu_counter_tree_level_item *)ptr;
+}
+
/*
* per-process(per-mm_struct) statistics.
*/
static inline unsigned long get_mm_counter(struct mm_struct *mm, int member)
{
- return percpu_counter_read_positive(&mm->rss_stat[member]);
+ return percpu_counter_tree_approximate_sum_positive(&mm->rss_stat[member]);
}
static inline unsigned long get_mm_counter_sum(struct mm_struct *mm, int member)
{
- return percpu_counter_sum_positive(&mm->rss_stat[member]);
+ return percpu_counter_tree_precise_sum_positive(&mm->rss_stat[member]);
}
void mm_trace_rss_stat(struct mm_struct *mm, int member);
static inline void add_mm_counter(struct mm_struct *mm, int member, long value)
{
- percpu_counter_add(&mm->rss_stat[member], value);
+ percpu_counter_tree_add(&mm->rss_stat[member], value);
mm_trace_rss_stat(mm, member);
}
static inline void inc_mm_counter(struct mm_struct *mm, int member)
{
- percpu_counter_inc(&mm->rss_stat[member]);
+ percpu_counter_tree_add(&mm->rss_stat[member], 1);
mm_trace_rss_stat(mm, member);
}
static inline void dec_mm_counter(struct mm_struct *mm, int member)
{
- percpu_counter_dec(&mm->rss_stat[member]);
+ percpu_counter_tree_add(&mm->rss_stat[member], -1);
mm_trace_rss_stat(mm, member);
}
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 90e5790c318f..adb2f227bac7 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -18,7 +18,7 @@
#include <linux/page-flags-layout.h>
#include <linux/workqueue.h>
#include <linux/seqlock.h>
-#include <linux/percpu_counter.h>
+#include <linux/percpu_counter_tree.h>
#include <linux/types.h>
#include <linux/bitmap.h>
@@ -1119,7 +1119,7 @@ struct mm_struct {
unsigned long saved_e_flags;
#endif
- struct percpu_counter rss_stat[NR_MM_COUNTERS];
+ struct percpu_counter_tree rss_stat[NR_MM_COUNTERS];
struct linux_binfmt *binfmt;
diff --git a/include/trace/events/kmem.h b/include/trace/events/kmem.h
index 7f93e754da5c..91c81c44f884 100644
--- a/include/trace/events/kmem.h
+++ b/include/trace/events/kmem.h
@@ -442,7 +442,7 @@ TRACE_EVENT(rss_stat,
__entry->mm_id = mm_ptr_to_hash(mm);
__entry->curr = !!(current->mm == mm);
__entry->member = member;
- __entry->size = (percpu_counter_sum_positive(&mm->rss_stat[member])
+ __entry->size = (percpu_counter_tree_approximate_sum_positive(&mm->rss_stat[member])
<< PAGE_SHIFT);
),
diff --git a/kernel/fork.c b/kernel/fork.c
index 3da0f08615a9..5430d63c9e2d 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -133,6 +133,11 @@
*/
#define MAX_THREADS FUTEX_TID_MASK
+/*
+ * Batch size of rss stat approximation
+ */
+#define RSS_STAT_BATCH_SIZE 32
+
/*
* Protected counters by write_lock_irq(&tasklist_lock)
*/
@@ -583,14 +588,12 @@ static void check_mm(struct mm_struct *mm)
"Please make sure 'struct resident_page_types[]' is updated as well");
for (i = 0; i < NR_MM_COUNTERS; i++) {
- long x = percpu_counter_sum(&mm->rss_stat[i]);
-
- if (unlikely(x)) {
- pr_alert("BUG: Bad rss-counter state mm:%p type:%s val:%ld Comm:%s Pid:%d\n",
- mm, resident_page_types[i], x,
+ if (unlikely(percpu_counter_tree_precise_compare_value(&mm->rss_stat[i], 0) != 0))
+ pr_alert("BUG: Bad rss-counter state mm:%p type:%s val:%d Comm:%s Pid:%d\n",
+ mm, resident_page_types[i],
+ percpu_counter_tree_precise_sum(&mm->rss_stat[i]),
current->comm,
task_pid_nr(current));
- }
}
if (mm_pgtables_bytes(mm))
@@ -688,7 +691,7 @@ void __mmdrop(struct mm_struct *mm)
put_user_ns(mm->user_ns);
mm_pasid_drop(mm);
mm_destroy_cid(mm);
- percpu_counter_destroy_many(mm->rss_stat, NR_MM_COUNTERS);
+ percpu_counter_tree_destroy_many(mm->rss_stat, NR_MM_COUNTERS);
free_mm(mm);
}
@@ -1083,8 +1086,9 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
if (mm_alloc_cid(mm, p))
goto fail_cid;
- if (percpu_counter_init_many(mm->rss_stat, 0, GFP_KERNEL_ACCOUNT,
- NR_MM_COUNTERS))
+ if (percpu_counter_tree_init_many(mm->rss_stat, get_rss_stat_items(mm),
+ NR_MM_COUNTERS, RSS_STAT_BATCH_SIZE,
+ GFP_KERNEL_ACCOUNT))
goto fail_pcpu;
mm->user_ns = get_user_ns(user_ns);
@@ -2964,7 +2968,7 @@ void __init mm_cache_init(void)
* dynamically sized based on the maximum CPU number this system
* can have, taking hotplug into account (nr_cpu_ids).
*/
- mm_size = sizeof(struct mm_struct) + cpumask_size() + mm_cid_size();
+ mm_size = sizeof(struct mm_struct) + cpumask_size() + mm_cid_size() + get_rss_stat_items_size();
mm_cachep = kmem_cache_create_usercopy("mm_struct",
mm_size, ARCH_MIN_MMSTRUCT_ALIGN,
--
2.39.5
^ permalink raw reply related
* [PATCH v9 0/2] mm: Fix OOM killer inaccuracy on large many-core systems
From: Mathieu Desnoyers @ 2025-11-20 21:03 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, Mathieu Desnoyers, Paul E. McKenney, Steven Rostedt,
Masami Hiramatsu, Dennis Zhou, Tejun Heo, Christoph Lameter,
Martin Liu, David Rientjes, christian.koenig, Shakeel Butt,
SeongJae Park, Michal Hocko, Johannes Weiner, Sweet Tea Dorminy,
Lorenzo Stoakes, Liam R . Howlett, Mike Rapoport,
Suren Baghdasaryan, Vlastimil Babka, Christian Brauner, Wei Yang,
David Hildenbrand, Miaohe Lin, Al Viro, linux-mm,
linux-trace-kernel, Yu Zhao, Roman Gushchin, Mateusz Guzik,
Matthew Wilcox, Baolin Wang, Aboorva Devarajan
Introduce hierarchical per-cpu counters and use them for rss tracking to
fix the per-mm RSS tracking which has become too inaccurate for OOM
killer purposes on large many-core systems.
The approach proposed here is to replace this by the hierarchical
per-cpu counters, which bounds the inaccuracy based on the system
topology with O(N*logN).
Testing and feedback are welcome!
Thanks,
Mathieu
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Dennis Zhou <dennis@kernel.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Christoph Lameter <cl@linux.com>
Cc: Martin Liu <liumartin@google.com>
Cc: David Rientjes <rientjes@google.com>
Cc: christian.koenig@amd.com
Cc: Shakeel Butt <shakeel.butt@linux.dev>
Cc: SeongJae Park <sj@kernel.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: "Liam R . Howlett" <liam.howlett@oracle.com>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Miaohe Lin <linmiaohe@huawei.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-mm@kvack.org
Cc: linux-trace-kernel@vger.kernel.org
Cc: Yu Zhao <yuzhao@google.com>
Cc: Roman Gushchin <roman.gushchin@linux.dev>
Cc: Mateusz Guzik <mjguzik@gmail.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Aboorva Devarajan <aboorvad@linux.ibm.com>
Mathieu Desnoyers (2):
lib: Introduce hierarchical per-cpu counters
mm: Fix OOM killer inaccuracy on large many-core systems
include/linux/mm.h | 28 +-
include/linux/mm_types.h | 4 +-
include/linux/percpu_counter_tree.h | 239 +++++++++++++++
include/trace/events/kmem.h | 2 +-
init/main.c | 2 +
kernel/fork.c | 24 +-
lib/Makefile | 1 +
lib/percpu_counter_tree.c | 443 ++++++++++++++++++++++++++++
8 files changed, 725 insertions(+), 18 deletions(-)
create mode 100644 include/linux/percpu_counter_tree.h
create mode 100644 lib/percpu_counter_tree.c
--
2.39.5
^ permalink raw reply
* [PATCH] tracing: Add system trigger file to enable triggers for all the system's events
From: Steven Rostedt @ 2025-11-20 21:00 UTC (permalink / raw)
To: LKML, Linux Trace Kernel; +Cc: Masami Hiramatsu, Mathieu Desnoyers, Tom Zanussi
From: Steven Rostedt <rostedt@goodmis.org>
Currently a trigger can only be added to individual events. Some triggers
(like stacktrace) can be useful to add as a bulk trigger for a set of
system events (like interrupt or scheduling).
Add a trigger file to the system directories:
/sys/kernel/tracing/events/*/trigger
And allow stacktrace trigger to be enabled for all those events.
Writing into the system/trigger file acts the same as writing into each of
the system event's trigger files individually.
This also allows to remove a trigger from all events in a subsystem (even
if it's not a subsystem trigger!).
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
Note, this is based on top of:
https://patchwork.kernel.org/project/linux-trace-kernel/cover/20251120205600.570673392@kernel.org/
Documentation/trace/events.rst | 25 ++++
kernel/trace/trace.c | 11 +-
kernel/trace/trace.h | 15 +-
kernel/trace/trace_events.c | 70 +++++-----
kernel/trace/trace_events_trigger.c | 205 +++++++++++++++++++++++++++-
5 files changed, 284 insertions(+), 42 deletions(-)
diff --git a/Documentation/trace/events.rst b/Documentation/trace/events.rst
index 18d112963dec..caa4958af43a 100644
--- a/Documentation/trace/events.rst
+++ b/Documentation/trace/events.rst
@@ -416,6 +416,31 @@ way, so beware about making generalizations between the two.
can also enable triggers that are written into
/sys/kernel/tracing/events/ftrace/print/trigger
+The system directory also has a trigger file that allows some triggers to be
+set for all the system's events. This is limited to only a small subset of the
+triggers and does not allow for the count parameter. But it does allow for
+filters. Writing into this file is the same as writing into each of the
+system's event's trigger files individually. Although only a subset of
+triggers may use this file for enabling, all triggers may use this file for
+disabling::
+
+ cd /sys/kernel/tracing
+ cat events/sched/trigger
+ # Available system triggers:
+ # stacktrace
+
+ echo stacktrace > events/sched/trigger
+ cat events/sched/sched_switch/trigger
+ stacktrace:unlimited
+
+ echo snapshot > events/sched/sched_waking/trigger
+ cat events/sched/sched_waking/trigger
+ snapshot:unlimited
+ echo '!snapshot' > events/sched/trigger
+ cat events/sched/sched_waking/trigger
+ # Available triggers:
+ # traceon traceoff snapshot stacktrace enable_event disable_event enable_hist disable_hist hist
+
6.1 Expression syntax
---------------------
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 59cd4ed8af6d..d400c013d42b 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -592,11 +592,12 @@ void trace_set_ring_buffer_expanded(struct trace_array *tr)
LIST_HEAD(ftrace_trace_arrays);
-int trace_array_get(struct trace_array *this_tr)
+int __trace_array_get(struct trace_array *this_tr)
{
struct trace_array *tr;
- guard(mutex)(&trace_types_lock);
+ lockdep_assert_held(&trace_types_lock);
+
list_for_each_entry(tr, &ftrace_trace_arrays, list) {
if (tr == this_tr) {
tr->ref++;
@@ -607,6 +608,12 @@ int trace_array_get(struct trace_array *this_tr)
return -ENODEV;
}
+int trace_array_get(struct trace_array *tr)
+{
+ guard(mutex)(&trace_types_lock);
+ return __trace_array_get(tr);
+}
+
static void __trace_array_put(struct trace_array *this_tr)
{
WARN_ON(!this_tr->ref);
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index fd5a6daa6c25..7379763a057d 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -469,10 +469,14 @@ extern struct list_head ftrace_trace_arrays;
extern struct mutex trace_types_lock;
extern int trace_array_get(struct trace_array *tr);
+extern int __trace_array_get(struct trace_array *tr);
extern int tracing_check_open_get_tr(struct trace_array *tr);
extern struct trace_array *trace_array_find(const char *instance);
extern struct trace_array *trace_array_find_get(const char *instance);
+extern struct trace_subsystem_dir *trace_get_system_dir(struct inode *inode);
+void trace_put_system_dir(struct trace_subsystem_dir *dir);
+
extern u64 tracing_event_time_stamp(struct trace_buffer *buffer, struct ring_buffer_event *rbe);
extern int tracing_set_filter_buffering(struct trace_array *tr, bool set);
extern int tracing_set_clock(struct trace_array *tr, const char *clockstr);
@@ -1774,6 +1778,7 @@ static inline struct trace_event_file *event_file_file(struct file *filp)
}
extern const struct file_operations event_trigger_fops;
+extern const struct file_operations event_system_trigger_fops;
extern const struct file_operations event_hist_fops;
extern const struct file_operations event_hist_debug_fops;
extern const struct file_operations event_inject_fops;
@@ -2057,10 +2062,16 @@ struct event_command {
* regardless of whether or not it has a filter associated with
* it (filters make a trigger require access to the trace record
* but are not always present).
+ *
+ * @SYSTEM: A flag that says whether or not this command can be used
+ * at the event system level. For example, can it be written into
+ * events/sched/trigger file where it will be enabled for all
+ * sched events?
*/
enum event_command_flags {
- EVENT_CMD_FL_POST_TRIGGER = 1,
- EVENT_CMD_FL_NEEDS_REC = 2,
+ EVENT_CMD_FL_POST_TRIGGER = BIT(1),
+ EVENT_CMD_FL_NEEDS_REC = BIT(2),
+ EVENT_CMD_FL_SYSTEM = BIT(3),
};
static inline bool event_command_post_trigger(struct event_command *cmd_ops)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 9b07ad9eb284..f00b41f73fc2 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2168,51 +2168,52 @@ event_filter_write(struct file *filp, const char __user *ubuf, size_t cnt,
static LIST_HEAD(event_subsystems);
-static int subsystem_open(struct inode *inode, struct file *filp)
+struct trace_subsystem_dir *trace_get_system_dir(struct inode *inode)
{
- struct trace_subsystem_dir *dir = NULL, *iter_dir;
- struct trace_array *tr = NULL, *iter_tr;
- struct event_subsystem *system = NULL;
- int ret;
+ struct trace_subsystem_dir *dir;
+ struct trace_array *tr = NULL;
- if (tracing_is_disabled())
- return -ENODEV;
+ guard(mutex)(&event_mutex);
+ guard(mutex)(&trace_types_lock);
/* Make sure the system still exists */
- mutex_lock(&event_mutex);
- mutex_lock(&trace_types_lock);
- list_for_each_entry(iter_tr, &ftrace_trace_arrays, list) {
- list_for_each_entry(iter_dir, &iter_tr->systems, list) {
- if (iter_dir == inode->i_private) {
+ list_for_each_entry(tr, &ftrace_trace_arrays, list) {
+ list_for_each_entry(dir, &tr->systems, list) {
+ if (dir == inode->i_private) {
/* Don't open systems with no events */
- tr = iter_tr;
- dir = iter_dir;
- if (dir->nr_events) {
- __get_system_dir(dir);
- system = dir->subsystem;
- }
- goto exit_loop;
+ if (!dir->nr_events)
+ return NULL;
+ if (__trace_array_get(tr) < 0)
+ return NULL;
+ __get_system_dir(dir);
+ return dir;
}
}
}
- exit_loop:
- mutex_unlock(&trace_types_lock);
- mutex_unlock(&event_mutex);
+ return NULL;
+}
- if (!system)
+void trace_put_system_dir(struct trace_subsystem_dir *dir)
+{
+ trace_array_put(dir->tr);
+ put_system(dir);
+}
+
+static int subsystem_open(struct inode *inode, struct file *filp)
+{
+ struct trace_subsystem_dir *dir;
+ int ret;
+
+ if (tracing_is_disabled())
return -ENODEV;
- /* Still need to increment the ref count of the system */
- if (trace_array_get(tr) < 0) {
- put_system(dir);
+ dir = trace_get_system_dir(inode);
+ if (!dir)
return -ENODEV;
- }
ret = tracing_open_generic(inode, filp);
- if (ret < 0) {
- trace_array_put(tr);
- put_system(dir);
- }
+ if (ret < 0)
+ trace_put_system_dir(dir);
return ret;
}
@@ -2761,6 +2762,9 @@ static int system_callback(const char *name, umode_t *mode, void **data,
else if (strcmp(name, "enable") == 0)
*fops = &ftrace_system_enable_fops;
+ else if (strcmp(name, "trigger") == 0)
+ *fops = &event_system_trigger_fops;
+
else
return 0;
@@ -2784,6 +2788,10 @@ event_subsystem_dir(struct trace_array *tr, const char *name,
{
.name = "enable",
.callback = system_callback,
+ },
+ {
+ .name = "trigger",
+ .callback = system_callback,
}
};
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 1dfe69146a81..b621406054b7 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -329,21 +329,28 @@ int trigger_process_regex(struct trace_event_file *file, char *buff)
return -EINVAL;
}
+static char *get_user_buf(const char __user *ubuf, size_t cnt)
+{
+ if (!cnt)
+ return NULL;
+
+ if (cnt >= PAGE_SIZE)
+ return ERR_PTR(-EINVAL);
+
+ return memdup_user_nul(ubuf, cnt);
+}
+
static ssize_t event_trigger_regex_write(struct file *file,
const char __user *ubuf,
size_t cnt, loff_t *ppos)
{
struct trace_event_file *event_file;
ssize_t ret;
- char *buf __free(kfree) = NULL;
+ char *buf __free(kfree) = get_user_buf(ubuf, cnt);
- if (!cnt)
+ if (!buf)
return 0;
- if (cnt >= PAGE_SIZE)
- return -EINVAL;
-
- buf = memdup_user_nul(ubuf, cnt);
if (IS_ERR(buf))
return PTR_ERR(buf);
@@ -397,6 +404,190 @@ const struct file_operations event_trigger_fops = {
.release = event_trigger_release,
};
+static ssize_t
+event_system_trigger_read(struct file *filp, char __user *ubuf,
+ size_t count, loff_t *ppos)
+{
+ char *buf __free(kfree) = kmalloc(SZ_4K, GFP_KERNEL);
+ struct event_command *p;
+ struct seq_buf s;
+ int len;
+
+ if (!buf)
+ return -ENOMEM;
+
+ seq_buf_init(&s, buf, SZ_4K);
+
+ seq_buf_puts(&s, "# Available system triggers:\n");
+ seq_buf_putc(&s, '#');
+
+ guard(mutex)(&trigger_cmd_mutex);
+ list_for_each_entry_reverse(p, &trigger_commands, list) {
+ if (p->flags & EVENT_CMD_FL_SYSTEM)
+ seq_buf_printf(&s, " %s", p->name);
+ }
+ seq_buf_putc(&s, '\n');
+
+ len = seq_buf_used(&s);
+
+ if (*ppos >= len)
+ return 0;
+
+ len -= *ppos;
+
+ if (count > len)
+ count = len;
+
+ if (copy_to_user(ubuf, buf + *ppos, count))
+ return -EFAULT;
+
+ *ppos += count;
+
+ return count;
+}
+
+static int process_system_events(struct trace_subsystem_dir *dir,
+ struct event_command *p, char *buff,
+ char *command, char *next)
+{
+ struct event_subsystem *system = dir->subsystem;
+ struct trace_event_file *file;
+ struct trace_array *tr = dir->tr;
+ bool remove = false;
+ int ret = 0;
+
+ if (buff[0] == '!')
+ remove = true;
+
+ lockdep_assert_held(&event_mutex);
+
+ list_for_each_entry(file, &tr->events, list) {
+
+ if (strcmp(system->name, file->event_call->class->system) != 0)
+ continue;
+
+ ret = p->parse(p, file, buff, command, next);
+
+ /* Removals and existing events do not error */
+ if (ret < 0 && ret != -EEXIST && !remove) {
+ pr_warn("Failed adding trigger %s on %s\n",
+ command, trace_event_name(file->event_call));
+ }
+ }
+ return 0;
+}
+
+static ssize_t
+event_system_trigger_write(struct file *filp, const char __user *ubuf,
+ size_t cnt, loff_t *ppos)
+{
+ struct trace_subsystem_dir *dir = filp->private_data;
+ struct event_command *p;
+ char *command, *next;
+ char *buf __free(kfree) = get_user_buf(ubuf, cnt);
+ bool remove = false;
+ bool found = false;
+ ssize_t ret;
+ int len;
+
+ if (!buf)
+ return 0;
+
+ if (IS_ERR(buf))
+ return PTR_ERR(buf);
+
+ /* system triggers are not allowed to have counters */
+ if (strchr(buf, ':'))
+ return -EINVAL;
+
+ /* If opened for read too, dir is in the seq_file descriptor */
+ if (filp->f_mode & FMODE_READ) {
+ struct seq_file *m = filp->private_data;
+ dir = m->private;
+ }
+
+ /* Skip added space at beginning of buf */
+ next = buf;
+ strim(next);
+
+ command = strsep(&next, " \t");
+ if (next) {
+ next = skip_spaces(next);
+ if (!*next)
+ next = NULL;
+ }
+ if (command[0] == '!') {
+ remove = true;
+ command++;
+ }
+
+ len = strlen(command);
+ if (next)
+ len += strlen(next) + 1;
+
+ guard(mutex)(&event_mutex);
+ guard(mutex)(&trigger_cmd_mutex);
+
+ list_for_each_entry(p, &trigger_commands, list) {
+ /* Allow to remove any trigger */
+ if (!remove && !(p->flags & EVENT_CMD_FL_SYSTEM))
+ continue;
+ if (strcmp(p->name, command) == 0) {
+ found = true;
+ ret = process_system_events(dir, p, buf, command, next);
+ break;
+ }
+ }
+
+ if (!found)
+ ret = -ENODEV;
+
+ if (!ret)
+ *ppos += cnt;
+
+ if (remove || ret < 0)
+ return ret ? : cnt;
+
+ return cnt;
+}
+
+static int
+event_system_trigger_open(struct inode *inode, struct file *file)
+{
+ struct trace_subsystem_dir *dir;
+ int ret;
+
+ ret = security_locked_down(LOCKDOWN_TRACEFS);
+ if (ret)
+ return ret;
+
+ dir = trace_get_system_dir(inode);
+ if (!dir)
+ return -ENODEV;
+
+ file->private_data = dir;
+
+ return ret;
+}
+
+static int
+event_system_trigger_release(struct inode *inode, struct file *file)
+{
+ struct trace_subsystem_dir *dir = inode->i_private;
+
+ trace_put_system_dir(dir);
+
+ return 0;
+}
+
+const struct file_operations event_system_trigger_fops = {
+ .open = event_system_trigger_open,
+ .read = event_system_trigger_read,
+ .write = event_system_trigger_write,
+ .llseek = tracing_lseek,
+ .release = event_system_trigger_release,
+};
+
/*
* Currently we only register event commands from __init, so mark this
* __init too.
@@ -1587,7 +1778,7 @@ stacktrace_trigger_print(struct seq_file *m, struct event_trigger_data *data)
static struct event_command trigger_stacktrace_cmd = {
.name = "stacktrace",
.trigger_type = ETT_STACKTRACE,
- .flags = EVENT_CMD_FL_POST_TRIGGER,
+ .flags = EVENT_CMD_FL_POST_TRIGGER | EVENT_CMD_FL_SYSTEM,
.parse = event_trigger_parse,
.reg = register_trigger,
.unreg = unregister_trigger,
--
2.51.0
^ permalink raw reply related
* [PATCH 2/3] tracing: Add bulk garbage collection of freeing event_trigger_data
From: Steven Rostedt @ 2025-11-20 20:56 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Tom Zanussi
In-Reply-To: <20251120205600.570673392@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
The event trigger data requires a full tracepoint_synchronize_unregister()
call before freeing. That call can take 100s of milliseconds to complete.
In order to allow for bulk freeing of the trigger data, it can not call
the tracepoint_synchronize_unregister() for every individual trigger data
being free.
Create a kthread that gets created the first time a trigger data is freed,
and have it use the lockless llist to get the list of data to free, run
the tracepoint_synchronize_unregister() then free everything in the list.
By freeing hundreds of event_trigger_data elements together, it only
requires two runs of the synchronization function, and not hundreds of
runs. This speeds up the operation by orders of magnitude (milliseconds
instead of several seconds).
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace.h | 1 +
kernel/trace/trace_events_trigger.c | 56 +++++++++++++++++++++++++++--
2 files changed, 54 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 5863800b1ab3..fd5a6daa6c25 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1808,6 +1808,7 @@ struct event_trigger_data {
char *name;
struct list_head named_list;
struct event_trigger_data *named_data;
+ struct llist_node llist;
};
/* Avoid typos */
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index e5dcfcbb2cd5..16e3449f3cfe 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -6,26 +6,76 @@
*/
#include <linux/security.h>
+#include <linux/kthread.h>
#include <linux/module.h>
#include <linux/ctype.h>
#include <linux/mutex.h>
#include <linux/slab.h>
#include <linux/rculist.h>
+#include <linux/llist.h>
#include "trace.h"
static LIST_HEAD(trigger_commands);
static DEFINE_MUTEX(trigger_cmd_mutex);
+static struct task_struct *trigger_kthread;
+static struct llist_head trigger_data_free_list;
+static DEFINE_MUTEX(trigger_data_kthread_mutex);
+
+/* Bulk garbage collection of event_trigger_data elements */
+static int trigger_kthread_fn(void *ignore)
+{
+ struct event_trigger_data *data, *tmp;
+ struct llist_node *llnodes;
+
+ /* Once this task starts, it lives forever */
+ for (;;) {
+ set_current_state(TASK_INTERRUPTIBLE);
+ if (llist_empty(&trigger_data_free_list))
+ schedule();
+
+ __set_current_state(TASK_RUNNING);
+
+ llnodes = llist_del_all(&trigger_data_free_list);
+
+ /* make sure current triggers exit before free */
+ tracepoint_synchronize_unregister();
+
+ llist_for_each_entry_safe(data, tmp, llnodes, llist)
+ kfree(data);
+ }
+
+ return 0;
+}
+
void trigger_data_free(struct event_trigger_data *data)
{
if (data->cmd_ops->set_filter)
data->cmd_ops->set_filter(NULL, data, NULL);
- /* make sure current triggers exit before free */
- tracepoint_synchronize_unregister();
+ if (unlikely(!trigger_kthread)) {
+ guard(mutex)(&trigger_data_kthread_mutex);
+ /* Check again after taking mutex */
+ if (!trigger_kthread) {
+ struct task_struct *kthread;
+
+ kthread = kthread_create(trigger_kthread_fn, NULL,
+ "trigger_data_free");
+ if (!IS_ERR(kthread))
+ WRITE_ONCE(trigger_kthread, kthread);
+ }
+ }
+
+ if (!trigger_kthread) {
+ /* Do it the slow way */
+ tracepoint_synchronize_unregister();
+ kfree(data);
+ return;
+ }
- kfree(data);
+ llist_add(&data->llist, &trigger_data_free_list);
+ wake_up_process(trigger_kthread);
}
static inline void data_ops_trigger(struct event_trigger_data *data,
--
2.51.0
^ permalink raw reply related
* [PATCH 3/3] tracing: Use strim() in trigger_process_regex() instead of skip_spaces()
From: Steven Rostedt @ 2025-11-20 20:56 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Tom Zanussi
In-Reply-To: <20251120205600.570673392@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
The function trigger_process_regex() is called by a few functions, where
only one calls strim() on the buffer passed to it. That leaves the other
functions not trimming the end of the buffer passed in and making it a
little inconsistent.
Remove the strim() from event_trigger_regex_write() and have
trigger_process_regex() use strim() instead of skip_spaces(). The buff
variable is not passed in as const, so it can be modified.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace_events_trigger.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 16e3449f3cfe..1dfe69146a81 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -309,7 +309,8 @@ int trigger_process_regex(struct trace_event_file *file, char *buff)
char *command, *next;
struct event_command *p;
- next = buff = skip_spaces(buff);
+ next = buff = strim(buff);
+
command = strsep(&next, ": \t");
if (next) {
next = skip_spaces(next);
@@ -346,8 +347,6 @@ static ssize_t event_trigger_regex_write(struct file *file,
if (IS_ERR(buf))
return PTR_ERR(buf);
- strim(buf);
-
guard(mutex)(&event_mutex);
event_file = event_file_file(file);
--
2.51.0
^ permalink raw reply related
* [PATCH 0/3] tracing: More clean ups of triggers
From: Steven Rostedt @ 2025-11-20 20:56 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Tom Zanussi
This is based on top of:
https://lore.kernel.org/linux-trace-kernel/20251119031042.328864818@kernel.org/
Steven Rostedt (3):
tracing: Remove unneeded event_mutex lock in event_trigger_regex_release()
tracing: Add bulk garbage collection of freeing event_trigger_data
tracing: Use strim() in trigger_process_regex() instead of skip_spaces()
----
kernel/trace/trace.h | 1 +
kernel/trace/trace_events_trigger.c | 65 +++++++++++++++++++++++++++++++------
2 files changed, 56 insertions(+), 10 deletions(-)
^ permalink raw reply
* [PATCH 1/3] tracing: Remove unneeded event_mutex lock in event_trigger_regex_release()
From: Steven Rostedt @ 2025-11-20 20:56 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Tom Zanussi
In-Reply-To: <20251120205600.570673392@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
In event_trigger_regex_release(), the only code is:
mutex_lock(&event_mutex);
if (file->f_mode & FMODE_READ)
seq_release(inode, file);
mutex_unlock(&event_mutex);
return 0;
There's nothing special about the file->f_mode or the seq_release() that
requires any locking. Remove the unnecessary locks.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace_events_trigger.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 7795af600466..e5dcfcbb2cd5 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -314,13 +314,9 @@ static ssize_t event_trigger_regex_write(struct file *file,
static int event_trigger_regex_release(struct inode *inode, struct file *file)
{
- mutex_lock(&event_mutex);
-
if (file->f_mode & FMODE_READ)
seq_release(inode, file);
- mutex_unlock(&event_mutex);
-
return 0;
}
--
2.51.0
^ permalink raw reply related
* Re: [PATCH] selftests: tracing: Add tprobe enable/disable testcase
From: Shuah Khan @ 2025-11-20 16:40 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Steven Rostedt, Shuah Khan, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, linux-kselftest, Shuah Khan
In-Reply-To: <20251120102526.e5e9332fcab1db3ce18c6d15@kernel.org>
On 11/19/25 18:25, Masami Hiramatsu (Google) wrote:
> On Wed, 19 Nov 2025 15:56:57 -0700
> Shuah Khan <skhan@linuxfoundation.org> wrote:
>
>> On 11/19/25 15:06, Steven Rostedt wrote:
>>> On Wed, 19 Nov 2025 14:44:22 -0700
>>> Shuah Khan <skhan@linuxfoundation.org> wrote:
>>>
>>>> On 11/7/25 07:35, Masami Hiramatsu (Google) wrote:
>>>>> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>>>>>
>>>>> Commit 2867495dea86 ("tracing: tprobe-events: Register tracepoint when
>>>>> enable tprobe event") caused regression bug and tprobe did not work.
>>>>> To prevent similar problems, add a testcase which enables/disables a
>>>>> tprobe and check the results.
>>>>>
>>>>> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>>>>
>>>> Steve, do you want me to take this through my tree?
>>>
>>> Yes please. Masami's an official maintainer and mostly handles all things
>>> "probe" related. This is his domain ;-)
>>>
>>> Thanks,
>>>
>>> -- Steve
>> Applied to linux-kselftest next for Linux 6.19-rc1.
>
> Thanks Shuah! This and other regression fixes is better to go
> through selftests tree because those are checking existing
> features. Maybe better to add [PATCH -selftests] or something
> like that?
>
Let me know which ones you would like to pick up and apply to my tree.
thanks,
-- Shuah
^ permalink raw reply
* Re: [PATCH v3] mm/memory-failure: remove the selection of RAS
From: Andrew Morton @ 2025-11-20 15:56 UTC (permalink / raw)
To: David Hildenbrand (Red Hat)
Cc: Xie Yuanbin, tony.luck, bp, linmiaohe, nao.horiguchi, rostedt,
mhiramat, mathieu.desnoyers, lorenzo.stoakes, Liam.Howlett,
vbabka, rppt, surenb, mhocko, linux-kernel, linux-edac, linux-mm,
linux-trace-kernel, will, liaohua4, lilinjie8
In-Reply-To: <fc7bae22-0705-475a-be89-8bb3ca12384d@kernel.org>
On Thu, 20 Nov 2025 14:59:54 +0100 "David Hildenbrand (Red Hat)" <david@kernel.org> wrote:
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -11631,10 +11631,11 @@ R: Naoya Horiguchi <nao.horiguchi@gmail.com>
> > L: linux-mm@kvack.org
> > S: Maintained
> > F: include/linux/memory-failure.h
> > F: mm/hwpoison-inject.c
> > F: mm/memory-failure.c
> > +F: include/trace/events/memory-failure.
>
> These are ordered alphabetically, so it should be further up next to the
> other include.
I fixed that up, thanks.
^ permalink raw reply
* [GIT PULL] RTLA updates for v6.19
From: Tomas Glozar @ 2025-11-20 15:35 UTC (permalink / raw)
To: Steven Rostedt
Cc: Costa Shulyupin, Crystal Wood, Ivan Pravdin, John Kacur, LKML,
Linux Trace Kernel, Tomas Glozar
Steven,
Please pull the following changes for RTLA. See the tag description for more
information.
Thanks,
Tomas
The following changes since commit 0d5077c73aceb51ef10d096160dd62a11db2f3e4:
MAINTAINERS: Add Tomas Glozar as a maintainer to RTLA tool (2025-11-14 12:24:41 -0500)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tglozar/linux.git/ tags/rtla-v6.19
for you to fetch changes up to 368434d20d021aac8071bd08b1a471c588ce9ccd:
rtla/timerlat: Exit top main loop on any non-zero wait_retval (2025-11-20 13:26:55 +0100)
----------------------------------------------------------------
RTLA patches for v6.19
Fixes and minor cleanups for RTLA, targetting v6.19 via
linux-trace tools/for-next:
- Add for_each_monitored_cpu() helper
In multiple places, RTLA tools iterate over the list of CPUs running
tracer threads.
Use single helper instead of repeating the for/if combination.
- Remove unused variable option_index in argument parsing
RTLA tools use getopt_long() for argument parsing. For its last
argument, an unused variable "option_index" is passed.
Remove the variable and pass NULL to getopt_long() to shorten
the naturally long parsing functions, and make them more readable.
- Fix unassigned nr_cpus after code consolidation
In recent code consolidation, timerlat tool cleanup, previously
implemented separately for each tool, was moved to a common function
timerlat_free().
The cleanup relies on nr_cpus being set. This was not done in the new
function, leaving the variable uninitialized.
Initialize the variable properly, and remove silencing of compiler
warning for uninitialized variables.
- Stop tracing on user latency in BPF mode
Despite the name, rtla-timerlat's -T/--thread option sets timerlat's
stop_tracing_total_us option, which also stops tracing on
return-from-user latency, not only on thread latency.
Implement the same behavior also in BPF sample collection stop tracing
handler to avoid a discrepancy and restore correspondence of behavior
with the equivalent option of cyclictest.
- Fix threshold actions always triggering
A bug in threshold action logic caused the action to execute even
if tracing did not stop because of threshold.
Fix the logic to stop correctly.
- Fix few minor issues in tests
Extend tests that were shown to need it to 5s, fix osnoise test
calling timerlat by mistake, and use new, more reliable output
checking in timerlat's "top stop at failed action" test.
- Do not print usage on argument parsing error
RTLA prints the entire usage message on encountering errors in
argument parsing, like a malformed CPU list.
The usage message has gotten too long. Instead of printing it,
use newly added fatal() helper function to simply exit with
the error message, excluding the usage.
- Fix unintuitive -C/--cgroup interface
"-C cgroup" and "--cgroup cgroup" are invalid syntax, despite that
being a common way to specify an option with argument. Moreover,
using them fails silently and no cgroup is set.
Create new helper function to unify the handling of all such options
and allow all of:
-Xsomething
-X=something
-X something
as well as the equivalent for the long option.
- Fix -a overriding -t argument filename
Fix a bug where -a following -t custom_file.txt overrides the custom
filename with the default timerlat_trace.txt.
- Stop tracing correctly on multiple events at once
In some race scenarios, RTLA BPF sample collection might send multiple
stop tracing events via the BPF ringbuffer at once.
Compare the number of events for != 0 instead of == 1 to cover for
this scenario and stop tracing properly.
The tag was tested on both a non-RT-tuned and an RT-tuned machine, and
no issues were found besides known cases of the flakiness of tests for
stack dump and auto-analysis, which rely on correct order of events.
The patchset mostly consists of fixes originally targetting one of
the v6.18-rcs rather than linux-next and the v6.19 merge window; one
of them is even Cc: stable. The delay was caused by maintainership
handover of RTLA.
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
----------------------------------------------------------------
Costa Shulyupin (8):
tools/rtla: Add for_each_monitored_cpu() helper
tools/rtla: Remove unused optional option_index
tools/rtla: Fix unassigned nr_cpus
tools/rtla: Add fatal() and replace error handling pattern
tools/rtla: Replace timerlat_top_usage("...") with fatal("...")
tools/rtla: Replace timerlat_hist_usage("...") with fatal("...")
tools/rtla: Replace osnoise_top_usage("...") with fatal("...")
tools/rtla: Replace osnoise_hist_usage("...") with fatal("...")
Crystal Wood (2):
rtla/tests: Don't rely on matching ^1ALL
rtla/timerlat: Exit top main loop on any non-zero wait_retval
Ivan Pravdin (2):
rtla: Fix -C/--cgroup interface
rtla: Fix -a overriding -t argument
Tomas Glozar (4):
rtla/timerlat_bpf: Stop tracing on user latency
tools/rtla: Fix --on-threshold always triggering
rtla/tests: Extend action tests to 5s
rtla/tests: Fix osnoise test calling timerlat
Documentation/tools/rtla/common_options.rst | 2 +-
tools/tracing/rtla/Makefile.rtla | 2 +-
tools/tracing/rtla/src/common.c | 24 +++--
tools/tracing/rtla/src/common.h | 4 +
tools/tracing/rtla/src/osnoise_hist.c | 136 +++++++++----------------
tools/tracing/rtla/src/osnoise_top.c | 106 +++++++-------------
tools/tracing/rtla/src/timerlat.bpf.c | 3 +
tools/tracing/rtla/src/timerlat.c | 12 +--
tools/tracing/rtla/src/timerlat_hist.c | 148 +++++++++-------------------
tools/tracing/rtla/src/timerlat_top.c | 116 ++++++++--------------
tools/tracing/rtla/src/timerlat_u.c | 12 +--
tools/tracing/rtla/src/utils.c | 41 ++++++++
tools/tracing/rtla/src/utils.h | 2 +
tools/tracing/rtla/tests/osnoise.t | 6 +-
tools/tracing/rtla/tests/timerlat.t | 6 +-
15 files changed, 247 insertions(+), 373 deletions(-)
^ permalink raw reply
* Re: (subset) [PATCH 00/44] Change a lot of min_t() that might mask high bits
From: Jens Axboe @ 2025-11-20 14:52 UTC (permalink / raw)
To: linux-kernel, david.laight.linux
Cc: Alan Stern, Alexander Viro, Alexei Starovoitov, Andi Shyti,
Andreas Dilger, Andrew Lunn, Andrew Morton, Andrii Nakryiko,
Andy Shevchenko, Ard Biesheuvel, Arnaldo Carvalho de Melo,
Bjorn Helgaas, Borislav Petkov, Christian Brauner,
Christian König, Christoph Hellwig, Daniel Borkmann,
Dan Williams, Dave Hansen, Dave Jiang, David Ahern,
Davidlohr Bueso, David S. Miller, Dennis Zhou, Eric Dumazet,
Greg Kroah-Hartman, Herbert Xu, Ingo Molnar, Jakub Kicinski,
Jakub Sitnicki, James E.J. Bottomley, Jarkko Sakkinen,
Jason A. Donenfeld, Jiri Slaby, Johannes Weiner, John Allen,
Jonathan Cameron, Juergen Gross, Kees Cook, KP Singh,
Linus Walleij, Martin K. Petersen, Matthew Wilcox (Oracle),
Mika Westerberg, Mike Rapoport, Miklos Szeredi, Namhyung Kim,
Neal Cardwell, nic_swsd, OGAWA Hirofumi, Olivia Mackall,
Paolo Abeni, Paolo Bonzini, Peter Huewe, Peter Zijlstra,
Rafael J. Wysocki, Sean Christopherson, Srinivas Kandagatla,
Stefano Stabellini, Steven Rostedt, Tejun Heo, Theodore Ts'o,
Thomas Gleixner, Tom Lendacky, Willem de Bruijn, x86, Yury Norov,
amd-gfx, bpf, cgroups, dri-devel, io-uring, kvm, linux-acpi,
linux-block, linux-crypto, linux-cxl, linux-efi, linux-ext4,
linux-fsdevel, linux-gpio, linux-i2c, linux-integrity, linux-mm,
linux-nvme, linux-pci, linux-perf-users, linux-scsi, linux-serial,
linux-trace-kernel, linux-usb, mptcp, netdev, usb-storage,
David Hildenbrand
In-Reply-To: <20251119224140.8616-1-david.laight.linux@gmail.com>
On Wed, 19 Nov 2025 22:40:56 +0000, david.laight.linux@gmail.com wrote:
> It in not uncommon for code to use min_t(uint, a, b) when one of a or b
> is 64bit and can have a value that is larger than 2^32;
> This is particularly prevelant with:
> uint_var = min_t(uint, uint_var, uint64_expression);
>
> Casts to u8 and u16 are very likely to discard significant bits.
>
> [...]
Applied, thanks!
[12/44] block: use min() instead of min_t()
commit: 9420e720ad192c53c8d2803c5a2313b2d586adbd
Best regards,
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH v3] mm/memory-failure: remove the selection of RAS
From: David Hildenbrand (Red Hat) @ 2025-11-20 13:59 UTC (permalink / raw)
To: Xie Yuanbin, tony.luck, bp, linmiaohe, nao.horiguchi, rostedt,
mhiramat, mathieu.desnoyers, akpm, lorenzo.stoakes, Liam.Howlett,
vbabka, rppt, surenb, mhocko
Cc: linux-kernel, linux-edac, linux-mm, linux-trace-kernel, will,
liaohua4, lilinjie8
In-Reply-To: <20251119095943.67125-1-xieyuanbin1@huawei.com>
On 11/19/25 10:59, Xie Yuanbin wrote:
> The commit 97f0b13452198290799f ("tracing: add trace event for
> memory-failure") introduces the selection of RAS in memory-failure.
> This commit is just a tracing feature; in reality, there is no dependency
> between memory-failure and RAS. RAS increases the size of the bzImage
> image by 8k, which is very valuable for embedded devices.
>
> Move the memory-failure traceing code from ras_event.h to
> memory-failure.h and remove the selection of RAS.
>
> v2->v3: https://lore.kernel.org/20251104072306.100738-3-xieyuanbin1@huawei.com
> - Change define TRACE_SYSTEM from ras to memory_failure
> - Add include/trace/events/memory-failure.h to
> "HWPOISON MEMORY FAILURE HANDLING" section in MAINTAINERS
> - Rebase to latest linux-next source
>
> v1->v2: https://lore.kernel.org/20251103033536.52234-2-xieyuanbin1@huawei.com
> - Move the memory-failure traceing code from ras_event.h to
> memory-failure.h
>
> Signed-off-by: Xie Yuanbin <xieyuanbin1@huawei.com>
> Cc: David Hildenbrand (Red Hat) <david@kernel.org>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Miaohe Lin <linmiaohe@huawei.com>
> ---
> MAINTAINERS | 1 +
> include/ras/ras_event.h | 87 ------------------------
> include/trace/events/memory-failure.h | 98 +++++++++++++++++++++++++++
> mm/Kconfig | 1 -
> mm/memory-failure.c | 5 +-
> 5 files changed, 103 insertions(+), 89 deletions(-)
> create mode 100644 include/trace/events/memory-failure.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7310d9ca0370..43d6eb95fb05 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11631,10 +11631,11 @@ R: Naoya Horiguchi <nao.horiguchi@gmail.com>
> L: linux-mm@kvack.org
> S: Maintained
> F: include/linux/memory-failure.h
> F: mm/hwpoison-inject.c
> F: mm/memory-failure.c
> +F: include/trace/events/memory-failure.
These are ordered alphabetically, so it should be further up next to the
other include.
With that
Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
--
Cheers
David
^ permalink raw reply
* Re: [PATCH V2 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: David Hildenbrand (Red Hat) @ 2025-11-20 13:35 UTC (permalink / raw)
To: Shivank Garg, Andrew Morton, Lorenzo Stoakes
Cc: Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Zach O'Keefe, linux-mm,
linux-kernel, linux-trace-kernel, Branden Moore
In-Reply-To: <20251120065043.41738-8-shivankg@amd.com>
On 11/20/25 07:50, Shivank Garg wrote:
> When MADV_COLLAPSE is called on file-backed mappings (e.g., executable
> text sections), the pages may still be dirty from recent writes and
> cause collapse to fail with -EINVAL. This is particularly problematic
> for freshly copied executables on filesystems, where page cache folios
> remain dirty until background writeback completes.
>
> The current code in collapse_file() triggers async writeback via
> filemap_flush() and expects khugepaged to revisit the page later.
> However, MADV_COLLAPSE is a synchronous operation where userspace
> expects immediate results.
>
> Perform synchronous writeback in madvise_collapse() before attempting
> collapse to avoid failing on first attempt.
>
> Reported-by: Branden Moore <Branden.Moore@amd.com>
> Closes: https://lore.kernel.org/all/4e26fe5e-7374-467c-a333-9dd48f85d7cc@amd.com
> Fixes: 34488399fa08 ("mm/madvise: add file and shmem support to MADV_COLLAPSE")
> Suggested-by: David Hildenbrand <david@kernel.org>
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
> mm/khugepaged.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 97d1b2824386..066a332c76ad 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -22,6 +22,7 @@
> #include <linux/dax.h>
> #include <linux/ksm.h>
> #include <linux/pgalloc.h>
> +#include <linux/backing-dev.h>
>
> #include <asm/tlb.h>
> #include "internal.h"
> @@ -2784,6 +2785,31 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
> hstart = (start + ~HPAGE_PMD_MASK) & HPAGE_PMD_MASK;
> hend = end & HPAGE_PMD_MASK;
>
> + /*
> + * For file-backed VMAs, perform synchronous writeback to ensure
> + * dirty folios are flushed before attempting collapse. This avoids
> + * failing on the first attempt when freshly-written executable text
> + * is still dirty in the page cache.
> + */
> + if (!vma_is_anonymous(vma) && vma->vm_file) {
> + struct address_space *mapping = vma->vm_file->f_mapping;
> +
> + if (mapping_can_writeback(mapping)) {
> + pgoff_t pgoff_start = linear_page_index(vma, hstart);
> + pgoff_t pgoff_end = linear_page_index(vma, hend);
> + loff_t lstart = (loff_t)pgoff_start << PAGE_SHIFT;
> + loff_t lend = ((loff_t)pgoff_end << PAGE_SHIFT) - 1;
> +
Hm, so we always do that, without any indication that there actually is
something dirty there.
Internally filemap_write_and_wait_range() uses something called
mapping_needs_writeback(), but it also applies to the complete file, not
a range.
Wouldn't it be better do do that only if we detect that there is
actually a dirty folio in the range?
That is, if we find any dirty folio in hpage_collapse_scan_file() and we
are in madvise, do that dance here and retry?
--
Cheers
David
^ permalink raw reply
* Re: [PATCH V2 2/2] mm/khugepaged: map dirty/writeback pages failures to EAGAIN
From: David Hildenbrand (Red Hat) @ 2025-11-20 13:29 UTC (permalink / raw)
To: Lance Yang, Garg, Shivank, Dev Jain
Cc: Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
Barry Song, Andrew Morton, Steven Rostedt, Lorenzo Stoakes,
Masami Hiramatsu, Mathieu Desnoyers, Zach O'Keefe, linux-mm,
linux-kernel, linux-trace-kernel
In-Reply-To: <ef4a0c2c-62d1-46de-9ea2-dd946857a03a@linux.dev>
On 11/20/25 13:24, Lance Yang wrote:
>
>
> On 2025/11/20 16:17, Garg, Shivank wrote:
>>
>>
>> On 11/20/2025 1:33 PM, Dev Jain wrote:
>>>
>>> On 20/11/25 12:20 pm, Shivank Garg wrote:
>>
>>> SCAN_PAGE_NOT_CLEAN is confusing - NOT_CLEAN literally means dirty, so why not SCAN_PAGE_DIRTY?
>>> Or SCAN_PAGE_DIRTY_OR_UNDER_WRITEBACK? Since folio_test_writeback() is true as a result of
>>> the folio being dirty, maybe just SCAN_PAGE_DIRTY can do.
>>>
>>> Reviewed-by: Dev Jain <dev.jain@arm.com>
>>>
>> Thanks for the review.
>>
>> I chose not to use SCAN_PAGE_DIRTY because dirty and writeback have different meanings[1]:
>>
>> Dirty: Memory that is waiting to be written back to disk
>> Writeback: Memory that is actively being written back to disk
>>
>> [1] https://www.kernel.org/doc/Documentation/filesystems/proc.txt
>>
>> IIUC, a page under writeback is no longer dirty, so using SCAN_PAGE_DIRTY would be misleading
>> for pages in the writeback state.
>>
>> I considered SCAN_PAGE_DIRTY_OR_WRITEBACK initially but felt it was too long.
>
> Nit: If SCAN_PAGE_DIRTY_OR_WRITEBACK
I would prefer that here.
--
Cheers
David
^ permalink raw reply
* Re: [PATCH V2 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: Lance Yang @ 2025-11-20 13:01 UTC (permalink / raw)
To: Shivank Garg
Cc: Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Steven Rostedt, David Hildenbrand,
Masami Hiramatsu, Mathieu Desnoyers, Zach O'Keefe, linux-mm,
Andrew Morton, linux-kernel, linux-trace-kernel, Branden Moore,
Lorenzo Stoakes
In-Reply-To: <20251120065043.41738-8-shivankg@amd.com>
On 2025/11/20 14:50, Shivank Garg wrote:
> When MADV_COLLAPSE is called on file-backed mappings (e.g., executable
> text sections), the pages may still be dirty from recent writes and
> cause collapse to fail with -EINVAL. This is particularly problematic
> for freshly copied executables on filesystems, where page cache folios
> remain dirty until background writeback completes.
>
> The current code in collapse_file() triggers async writeback via
> filemap_flush() and expects khugepaged to revisit the page later.
> However, MADV_COLLAPSE is a synchronous operation where userspace
> expects immediate results.
>
> Perform synchronous writeback in madvise_collapse() before attempting
> collapse to avoid failing on first attempt.
Thanks!
>
> Reported-by: Branden Moore <Branden.Moore@amd.com>
> Closes: https://lore.kernel.org/all/4e26fe5e-7374-467c-a333-9dd48f85d7cc@amd.com
> Fixes: 34488399fa08 ("mm/madvise: add file and shmem support to MADV_COLLAPSE")
> Suggested-by: David Hildenbrand <david@kernel.org>
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
> mm/khugepaged.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 97d1b2824386..066a332c76ad 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -22,6 +22,7 @@
> #include <linux/dax.h>
> #include <linux/ksm.h>
> #include <linux/pgalloc.h>
> +#include <linux/backing-dev.h>
>
> #include <asm/tlb.h>
> #include "internal.h"
> @@ -2784,6 +2785,31 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
> hstart = (start + ~HPAGE_PMD_MASK) & HPAGE_PMD_MASK;
> hend = end & HPAGE_PMD_MASK;
>
> + /*
> + * For file-backed VMAs, perform synchronous writeback to ensure
> + * dirty folios are flushed before attempting collapse. This avoids
> + * failing on the first attempt when freshly-written executable text
> + * is still dirty in the page cache.
> + */
> + if (!vma_is_anonymous(vma) && vma->vm_file) {
> + struct address_space *mapping = vma->vm_file->f_mapping;
> +
> + if (mapping_can_writeback(mapping)) {
> + pgoff_t pgoff_start = linear_page_index(vma, hstart);
> + pgoff_t pgoff_end = linear_page_index(vma, hend);
> + loff_t lstart = (loff_t)pgoff_start << PAGE_SHIFT;
> + loff_t lend = ((loff_t)pgoff_end << PAGE_SHIFT) - 1;
It looks like we need to hold a reference to the file here before
dropping the mmap lock :)
file = get_file(vma->vm_file);
Without it, the vma could be destroyed by a concurrent munmap() while
we are waiting in filemap_write_and_wait_range(), leading to a UAF
on mapping, IIUC ...
> +
> + mmap_read_unlock(mm);
> + mmap_locked = false;
> +
> + if (filemap_write_and_wait_range(mapping, lstart, lend)) {
And drop the reference :)
fput(file);
> + last_fail = SCAN_FAIL;
> + goto out_maybelock;
> + }
Same here :)
fput(file);
> + }
> + }
> +
> for (addr = hstart; addr < hend; addr += HPAGE_PMD_SIZE) {
> int result = SCAN_FAIL;
>
Cheers,
Lance
^ permalink raw reply
* Re: [PATCH v8 22/28] KVM: arm64: Add trace remote for the pKVM hyp
From: Vincent Donnefort @ 2025-11-20 12:27 UTC (permalink / raw)
To: Marc Zyngier
Cc: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <86a50irkst.wl-maz@kernel.org>
On Wed, Nov 19, 2025 at 05:31:30PM +0000, Marc Zyngier wrote:
> On Fri, 07 Nov 2025 09:38:34 +0000,
> Vincent Donnefort <vdonnefort@google.com> wrote:
> >
> > When running with KVM protected mode, the hypervisor is able to generate
> > events into tracefs compatible ring-buffers. Create a trace remote so
> > the kernel can read those buffers.
> >
> > This currently doesn't provide any event support which will come later.
> >
> > Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> >
> > diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> > index 580426cdbe77..64db254f0448 100644
> > --- a/arch/arm64/kvm/Kconfig
> > +++ b/arch/arm64/kvm/Kconfig
> > @@ -87,6 +87,7 @@ config PKVM_TRACING
> > bool
> > depends on KVM
> > depends on TRACING
> > + select TRACE_REMOTE
> > select SIMPLE_RING_BUFFER
> > default y
> >
[...]
> > +static void hyp_trace_buffer_free_bpages_backing(struct hyp_trace_buffer *trace_buffer)
> > +{
> > + free_pages_exact((void *)trace_buffer->desc->bpages_backing_start,
> > + trace_buffer->desc->bpages_backing_size);
> > +}
> > +
> > +static int __load_page(unsigned long va)
> > +{
> > + return kvm_call_hyp_nvhe(__pkvm_host_share_hyp, virt_to_pfn((void *)va), 1);
> > +}
>
> I struggle a bit with the nomenclature here. Why is that called
> "load"? Surely this is a "map" operation, right? Is that because this
> is called at "vcpu load" time? Something else?
I called "load" the operation of getting the tracing buffer ready. But for this
implementation specific part, I can use map/unmap here. (same for
hyp_trace_buffer_map_pages/hyp_trace_buffer_map_pages)
>
> Also, how is this working without pKVM, in a normal nVHE environment?
> Being able to trace in nVHE is a basic requirement, and I don't see
> how this works here.
I can probably make it work with nVHE as well.
>
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [PATCH V2 2/2] mm/khugepaged: map dirty/writeback pages failures to EAGAIN
From: Lance Yang @ 2025-11-20 12:24 UTC (permalink / raw)
To: Garg, Shivank, Dev Jain
Cc: Zi Yan, Baolin Wang, Liam R . Howlett, David Hildenbrand,
Nico Pache, Ryan Roberts, Barry Song, Andrew Morton,
Steven Rostedt, Lorenzo Stoakes, Masami Hiramatsu,
Mathieu Desnoyers, Zach O'Keefe, linux-mm, linux-kernel,
linux-trace-kernel
In-Reply-To: <33476cb4-318b-49db-9cc1-a354eca9e883@amd.com>
On 2025/11/20 16:17, Garg, Shivank wrote:
>
>
> On 11/20/2025 1:33 PM, Dev Jain wrote:
>>
>> On 20/11/25 12:20 pm, Shivank Garg wrote:
>
>> SCAN_PAGE_NOT_CLEAN is confusing - NOT_CLEAN literally means dirty, so why not SCAN_PAGE_DIRTY?
>> Or SCAN_PAGE_DIRTY_OR_UNDER_WRITEBACK? Since folio_test_writeback() is true as a result of
>> the folio being dirty, maybe just SCAN_PAGE_DIRTY can do.
>>
>> Reviewed-by: Dev Jain <dev.jain@arm.com>
>>
> Thanks for the review.
>
> I chose not to use SCAN_PAGE_DIRTY because dirty and writeback have different meanings[1]:
>
> Dirty: Memory that is waiting to be written back to disk
> Writeback: Memory that is actively being written back to disk
>
> [1] https://www.kernel.org/doc/Documentation/filesystems/proc.txt
>
> IIUC, a page under writeback is no longer dirty, so using SCAN_PAGE_DIRTY would be misleading
> for pages in the writeback state.
>
> I considered SCAN_PAGE_DIRTY_OR_WRITEBACK initially but felt it was too long.
Nit: If SCAN_PAGE_DIRTY_OR_WRITEBACK is too verbose, how about
SCAN_PAGE_DIRTY_WB?
It keeps the specificity without the length, and is arguably more
descriptive
than NOT_CLEAN ;)
That said, LGTM.
Reviewed-by: Lance Yang <lance.yang@linux.dev>
>
> SCAN_PAGE_NOT_CLEAN covers both states that indicate the page is not in a clean/stable
> state suitable for collapse.
>
> [1] https://www.kernel.org/doc/Documentation/filesystems/proc.txt
>
> Thanks,
> Shivank
^ permalink raw reply
* Re: [RFC PATCH 0/8] xfs: single block atomic writes for buffered IO
From: Ojaswin Mujoo @ 2025-11-20 12:14 UTC (permalink / raw)
To: Dave Chinner
Cc: Ritesh Harjani, Christoph Hellwig, Christian Brauner, djwong,
john.g.garry, tytso, willy, dchinner, linux-xfs, linux-kernel,
linux-ext4, linux-fsdevel, linux-mm, jack, nilay, martin.petersen,
rostedt, axboe, linux-block, linux-trace-kernel
In-Reply-To: <aRmHRk7FGD4nCT0s@dread.disaster.area>
On Sun, Nov 16, 2025 at 07:11:50PM +1100, Dave Chinner wrote:
> On Fri, Nov 14, 2025 at 02:50:25PM +0530, Ojaswin Mujoo wrote:
> > On Thu, Nov 13, 2025 at 09:32:11PM +1100, Dave Chinner wrote:
> > > On Thu, Nov 13, 2025 at 11:12:49AM +0530, Ritesh Harjani wrote:
> > > > Christoph Hellwig <hch@lst.de> writes:
> > > >
> > > > > On Thu, Nov 13, 2025 at 08:56:56AM +1100, Dave Chinner wrote:
> > > > >> On Wed, Nov 12, 2025 at 04:36:03PM +0530, Ojaswin Mujoo wrote:
> > > > >> > This patch adds support to perform single block RWF_ATOMIC writes for
> > > > >> > iomap xfs buffered IO. This builds upon the inital RFC shared by John
> > > > >> > Garry last year [1]. Most of the details are present in the respective
> > > > >> > commit messages but I'd mention some of the design points below:
> > > > >>
> > > > >> What is the use case for this functionality? i.e. what is the
> > > > >> reason for adding all this complexity?
> > > > >
> > > > > Seconded. The atomic code has a lot of complexity, and further mixing
> > > > > it with buffered I/O makes this even worse. We'd need a really important
> > > > > use case to even consider it.
> > > >
> > > > I agree this should have been in the cover letter itself.
> > > >
> > > > I believe the reason for adding this functionality was also discussed at
> > > > LSFMM too...
> > > >
> > > > For e.g. https://lwn.net/Articles/974578/ goes in depth and talks about
> > > > Postgres folks looking for this, since PostgreSQL databases uses
> > > > buffered I/O for their database writes.
> > >
> > > Pointing at a discussion about how "this application has some ideas
> > > on how it can maybe use it someday in the future" isn't a
> > > particularly good justification. This still sounds more like a
> > > research project than something a production system needs right now.
> >
> > Hi Dave, Christoph,
> >
> > There were some discussions around use cases for buffered atomic writes
> > in the previous LSFMM covered by LWN here [1]. AFAIK, there are
> > databases that recommend/prefer buffered IO over direct IO. As mentioned
> > in the article, MongoDB being one that supports both but recommends
> > buffered IO. Further, many DBs support both direct IO and buffered IO
> > well and it may not be fair to force them to stick to direct IO to get
> > the benefits of atomic writes.
> >
> > [1] https://lwn.net/Articles/1016015/
>
> You are quoting a discussion about atomic writes that was
> held without any XFS developers present. Given how XFS has driven
> atomic write functionality so far, XFS developers might have some
> ..... opinions about how buffered atomic writes in XFS...
>
> Indeed, go back to the 2024 buffered atomic IO LSFMM discussion,
> where there were XFS developers present. That's the discussion that
> Ritesh referenced, so you should be aware of it.
>
> https://lwn.net/Articles/974578/
>
> Back then I talked about how atomic writes made no sense as
> -writeback IO- given the massive window for anything else to modify
> the data in the page cache. There is no guarantee that what the
> application wrote in the syscall is what gets written to disk with
> writeback IO. i.e. anything that can access the page cache can
> "tear" application data that is staged as "atomic data" for later
> writeback.
>
> IOWs, the concept of atomic writes for writeback IO makes almost no
> sense at all - dirty data at rest in the page cache is not protected
> against 3rd party access or modification. The "atomic data IO"
> semantics can only exist in the submitting IO context where
> exclusive access to the user data can be guaranteed.
>
> IMO, the only way semantics that makes sense for buffered atomic
> writes through the page cache is write-through IO. The "atomic"
> context is related directly to user data provided at IO submission,
> and so IO submitted must guarantee exactly that data is being
> written to disk in that IO.
>
> IOWs, we have to guarantee exclusive access between the data copy-in
> and the pages being marked for writeback. The mapping needs to be
> marked as using stable pages to prevent anyone else changing the
> cached data whilst it has an atomic IO pending on it.
>
> That means folios covering atomic IO ranges do not sit in the page
> cache in a dirty state - they *must* immediately transition to the
> writeback state before the folio is unlocked so that *nothing else
> can modify them* before the physical REQ_ATOMIC IO is submitted and
> completed.
>
> If we've got the folios marked as writeback, we can pack them
> immediately into a bio and submit the IO (e.g. via the iomap DIO
> code). There is no need to involve the buffered IO writeback path
> here; we've already got the folios at hand and in the right state
> for IO. Once the IO is done, we end writeback on them and they
> remain clean in the page caceh for anyone else to access and
> modify...
Hi Dave,
I believe the essenece of your comment is that the data in the page
cache can be modified between the write and the writeback time and hence
it makes sense to have a write-through only semantic for RWF_ATOMIC
buffered IO.
However, as per various discussions around this on the mailing list, it
is my understanding that protecting tearing against an application
changing a data range that was previously written atomically is
something that falls out of scope of RWF_ATOMIC.
As John pointed out in [1], even with dio, RWF_ATOMIC writes can be torn
if the application does parallel overlaps. The only thing we guarantee
is the data doesn't tear when the actualy IO happens, and from there its
the userspace's responsibility to not change the data till IO [2]. I
believe userspace changing data between write and writeback time falls
in the same category.
[1] https://lore.kernel.org/fstests/0af205d9-6093-4931-abe9-f236acae8d44@oracle.com/
[2] https://lore.kernel.org/fstests/20250729144526.GB2672049@frogsfrogsfrogs/
>
> This gives us the same physical IO semantics for buffered and direct
> atomic IO, and it allows the same software fallbacks for larger IO
> to be used as well.
>
> > > Why didn't you use the existing COW buffered write IO path to
> > > implement atomic semantics for buffered writes? The XFS
> > > functionality is already all there, and it doesn't require any
> > > changes to the page cache or iomap to support...
> >
> > This patch set focuses on HW accelerated single block atomic writes with
> > buffered IO, to get some early reviews on the core design.
>
> What hardware acceleration? Hardware atomic writes are do not make
> IO faster; they only change IO failure semantics in certain corner
> cases. Making buffered writeback IO use REQ_ATOMIC does not change
> the failure semantics of buffered writeback from the point of view
> of an application; the applicaiton still has no idea just how much
> data or what files lost data whent eh system crashes.
>
> Further, writeback does not retain application write ordering, so
> the application also has no control over the order that structured
> data is updated on physical media. Hence if the application needs
> specific IO ordering for crash recovery (e.g. to avoid using a WAL)
> it cannot use background buffered writeback for atomic writes
> because that does not guarantee ordering.
>
> What happens when you do two atomic buffered writes to the same file
> range? The second on hits the page cache, so now the crash recovery
> semantic is no longer "old or new", it's "some random older version
> or new". If the application rewrites a range frequently enough,
> on-disk updates could skip dozens of versions between "old" and
> "new", whilst other ranges of the file move one version at a time.
> The application has -zero control- of this behaviour because it is
> background writeback that determines when something gets written to
> disk, not the application.
>
> IOWs, the only way to guarantee single version "old or new" atomic
> buffered overwrites for any given write would be to force flushing
> of the data post-write() completion. That means either O_DSYNC,
> fdatasync() or sync_file_range(). And this turns the atomic writes
> into -write-through- IO, not write back IO...
I agree that there is no ordeirng guarantee without calls to sync and
friends, but as with all other IO paths, it has always been the
applicatoin that needs to enforce the ordering. Applications like DBs
are well aware of this however there are still areas where they can
benefit with unordered atomic IO, eg bg write of a bunch of dirty
buffers, which only need to be sync'd once during checkpoint.
>
> > Just like we did for direct IO atomic writes, the software fallback with
> > COW and multi block support can be added eventually.
>
> If the reason for this functionality is "maybe someone
> can use it in future", then you're not implementing this
> functionality to optimise an existing workload. It's a research
> project looking for a user.
>
> Work with the database engineers to build a buffered atomic write
> based engine that implements atomic writes with RWF_DSYNC.
> Make it work, and optimise it to be competitive with existing
> database engines, than then show how much faster it is using
> RWF_ATOMIC buffered writes.
>
> Alternatively - write an algorithm that assumes the filesystem is
> using COW for overwrites, and optimise the data integrity algorithm
> based on this knowledge. e.g. use always-cow mode on XFS, or just
> optimise for normal bcachefs or btrfs buffered writes. Use O_DSYNC
> when completion to submission ordering is required. Now you have
> an application algorithm that is optimised for old-or-new behaviour,
> and that can then be acclerated on overwrite-in-place capable
> filesystems by using a direct-to-hw REQ_ATOMIC overwrite to provide
> old-or-new semantics instead of using COW.
>
> Yes, there are corner cases - partial writeback, fragmented files,
> etc - where data will a mix of old and new when using COW without
> RWF_DSYNC. Those are the the cases that RWF_ATOMIC needs to
> mitigate, but we don't need whacky page cache and writeback stuff to
> implement RWF_ATOMIC semantics in COW capable filesystems.
>
> i.e. enhance the applicaitons to take advantage of native COW
> old-or-new data semantics for buffered writes, then we can look at
> direct-to-hw fast paths to optimise those algorithms.
>
> Trying to go direct-to-hw first without having any clue of how
> applications are going to use such functionality is backwards.
> Design the applicaiton level code that needs highly performant
> old-or-new buffered write guarantees, then we can optimise the data
> paths for it...
Got it, thanks for the pointers Dave, we will look into this.
Regards,
ojaswin
>
> -Dave.
> --
> Dave Chinner
> david@fromorbit.com
^ permalink raw reply
* [PATCH AUTOSEL 6.17-6.1] ftrace: bpf: Fix IPMODIFY + DIRECT in modify_ftrace_direct()
From: Sasha Levin @ 2025-11-20 12:08 UTC (permalink / raw)
To: patches, stable
Cc: Song Liu, Jiri Olsa, Alexei Starovoitov, Steven Rostedt (Google),
Sasha Levin, mhiramat, linux-kernel, linux-trace-kernel, bpf
In-Reply-To: <20251120120838.1754634-1-sashal@kernel.org>
From: Song Liu <song@kernel.org>
[ Upstream commit 3e9a18e1c3e931abecf501cbb23d28d69f85bb56 ]
ftrace_hash_ipmodify_enable() checks IPMODIFY and DIRECT ftrace_ops on
the same kernel function. When needed, ftrace_hash_ipmodify_enable()
calls ops->ops_func() to prepare the direct ftrace (BPF trampoline) to
share the same function as the IPMODIFY ftrace (livepatch).
ftrace_hash_ipmodify_enable() is called in register_ftrace_direct() path,
but not called in modify_ftrace_direct() path. As a result, the following
operations will break livepatch:
1. Load livepatch to a kernel function;
2. Attach fentry program to the kernel function;
3. Attach fexit program to the kernel function.
After 3, the kernel function being used will not be the livepatched
version, but the original version.
Fix this by adding __ftrace_hash_update_ipmodify() to
__modify_ftrace_direct() and adjust some logic around the call.
Signed-off-by: Song Liu <song@kernel.org>
Reviewed-by: Jiri Olsa <jolsa@kernel.org>
Link: https://lore.kernel.org/r/20251027175023.1521602-3-song@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## COMPREHENSIVE ANALYSIS: ftrace: bpf: Fix IPMODIFY + DIRECT in
modify_ftrace_direct()
### 1. COMMIT MESSAGE ANALYSIS
This commit fixes a critical bug in the interaction between ftrace
IPMODIFY (livepatching) and DIRECT (BPF trampolines) operations. The
commit message clearly describes:
**Problem**: `ftrace_hash_ipmodify_enable()` is called in the
`register_ftrace_direct()` path but **NOT** in the
`modify_ftrace_direct()` path. This asymmetry causes livepatch to break
when BPF programs are attached in a specific sequence.
**Bug scenario**:
1. Load livepatch to a kernel function (IPMODIFY set)
2. Attach fentry program (works - register path has the check)
3. Attach fexit program (breaks - modify path missing the check)
4. Result: Function executes original code, **bypassing the livepatch**
**Fix**: Add `__ftrace_hash_update_ipmodify()` to
`__modify_ftrace_direct()` with appropriate logic adjustments.
**Key observations**:
- NO "Fixes:" tag (but bug origin is clear: commit 53cd885bc5c3e from
v6.0)
- NO "Cc: stable@vger.kernel.org" tag (unusual for a bug fix)
- Part of a 2-patch series (companion commit 56b3c85e153b8 **does have**
"Cc: stable@vger.kernel.org # v6.6+")
- Multiple maintainer acknowledgments: Reviewed-by Jiri Olsa, Acked-by
Steven Rostedt, Signed-off-by Alexei Starovoitov
### 2. DEEP CODE RESEARCH
#### Bug Introduction History
I traced the bug to **commit 53cd885bc5c3e** (July 2022, merged in
v6.0-rc1):
- "ftrace: Allow IPMODIFY and DIRECT ops on the same function"
- This commit introduced the mechanism for livepatch and BPF trampolines
to coexist on the same function
- It added `ops_func()` callbacks that get invoked to coordinate between
IPMODIFY and DIRECT operations
- The register path correctly calls `ftrace_hash_ipmodify_enable()`
which invokes this callback
- **The modify path was missing this check from day one**
#### Technical Mechanism of the Bug
**IPMODIFY** (used by livepatching): Modifies function entry points to
redirect to patched code
**DIRECT** (used by BPF): Directly calls custom trampolines (BPF
programs)
When both are on the same function, they must cooperate:
- The ftrace core calls `ops->ops_func(ops,
FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_SELF)`
- This tells the BPF trampoline to set `BPF_TRAMP_F_SHARE_IPMODIFY` flag
- With this flag, BPF generates trampolines with
`BPF_TRAMP_F_ORIG_STACK` to preserve livepatch behavior
- Without this coordination, the BPF trampoline jumps to the original
function, **bypassing the livepatch**
**The bug**: In `__modify_ftrace_direct()` (lines 6107-6154 in current
code):
```c
static int __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long
addr)
{
// ... registers tmp_ops ...
err = register_ftrace_function_nolock(&tmp_ops);
// BUG: No call to __ftrace_hash_update_ipmodify() here!
// ... updates direct call addresses ...
unregister_ftrace_function(&tmp_ops);
}
```
The function modifies where direct calls jump to, but never checks if
there's an IPMODIFY conflict requiring coordination.
#### The Fix in Detail
The patch makes these changes:
1. **Adds new parameter to `__ftrace_hash_update_ipmodify()`**: `bool
update_target`
- When `false`: Only checks differences between old_hash and new_hash
(existing behavior)
- When `true`: Processes all entries even if old_hash == new_hash
(needed for modify case)
2. **Updates the check logic** (lines 2007-2011):
```c
// OLD: Skip if no difference
if (in_old == in_new)
continue;
// NEW: Skip only if not updating target AND no difference
if (!update_target && (in_old == in_new))
continue;
```
3. **Fixes FTRACE_WARN_ON condition** (lines 2024-2034): Makes it aware
that `update_target` scenario is valid
4. **Adds the missing call in `__modify_ftrace_direct()`** (lines
6147-6157):
```c
err = register_ftrace_function_nolock(&tmp_ops);
if (err)
return err;
// NEW: Call the check that was missing!
err = __ftrace_hash_update_ipmodify(ops, hash, hash, true);
if (err)
goto out;
```
5. **Updates all existing callers** to pass `false` for backward
compatibility
#### Why This Works
When `modify_ftrace_direct()` is called:
- It passes `old_hash == new_hash` because it's not changing which
functions to trace, just where to jump
- With `update_target=true`, the code processes all entries anyway
- This triggers `ops->ops_func(ops,
FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_SELF)` if needed
- The BPF trampoline updates its flags and regenerates with proper
livepatch support
- The livepatch is no longer bypassed
### 3. SECURITY ASSESSMENT
**Severity: HIGH**
This is a **security-relevant correctness bug**:
- Livepatching is commonly used to apply **security fixes** without
rebooting
- Silently bypassing a livepatch means security vulnerabilities remain
exploitable
- Users have no indication that their security patch isn't working
- Affects production systems running observability tools (BPF) alongside
security patching
**Attack scenario**:
1. Admin applies critical security livepatch
2. Observability team runs BPF tracing (fentry/fexit)
3. Livepatch silently stops working
4. System remains vulnerable to the security issue the livepatch was
meant to fix
**No CVE mentioned**, but this is the type of issue that could warrant
one.
### 4. FEATURE VS BUG FIX CLASSIFICATION
**Unambiguously a BUG FIX**:
- Fixes broken functionality (livepatch bypass)
- No new features added
- Restores expected behavior (livepatch should work with BPF)
- Keywords: "Fix", "broken", specific bug scenario described
### 5. CODE CHANGE SCOPE ASSESSMENT
**Small and surgical**:
- Single file: `kernel/trace/ftrace.c`
- ~40 lines changed (net +18 lines)
- Changes:
- Add one parameter to existing function
- Update 4 call sites to pass `false` (maintains existing behavior)
- Add one new call in `__modify_ftrace_direct()`
- Update comments and conditional logic
- Add error handling path
- **No new APIs, no exported symbol changes, no ABI impact**
### 6. BUG TYPE AND SEVERITY
**Type**: Logic error / Missing check
**Severity**: **HIGH**
- **Impact**: Security patches silently bypassed
- **Triggering**: Specific but realistic sequence (livepatch + BPF
fentry/fexit)
- **Detection**: Silent failure (no error, no warning to user)
- **Consequences**: System remains vulnerable after applying security
patches
### 7. USER IMPACT EVALUATION
**Affected users**:
- **High-security environments** using livepatching for security updates
- **Cloud providers** running observability (BPF) on production systems
- **Enterprise Linux users** (RHEL, SLES, Ubuntu LTS) who rely on
livepatching
- **Any system** combining livepatch with BPF tracing tools (bpftrace,
bcc, etc.)
**Impact scope**:
- Moderate likelihood: Both livepatch and BPF are common, but the
specific sequence (attach fentry then fexit) is less common
- High consequence: When it happens, security is compromised
- Silent failure: No indication to user that livepatch stopped working
**Affected kernel versions**: v6.0 onwards (when IPMODIFY+DIRECT sharing
was introduced)
### 8. REGRESSION RISK ANALYSIS
**Risk: LOW**
**Why low risk**:
1. **Small, focused change**: Adds a missing check, doesn't refactor
existing code
2. **Parallel to existing code**: The register path already has this
check working fine
3. **Well-tested**: Part of a patch series with selftests
(0e0e608f72422)
4. **Multiple expert reviews**: Reviewed/Acked by ftrace and BPF
maintainers
5. **Conservative approach**: Only affects the modify path when
IPMODIFY+DIRECT interact
6. **Proper error handling**: Returns error if check fails, doesn't
crash
**Potential issues**:
- Could theoretically reject modify operations that were previously
(incorrectly) succeeding
- But those operations were broken anyway (bypassing livepatch)
- Failure mode is explicit error return, not silent corruption or crash
### 9. MAINLINE STABILITY
**Very recent commit**: October 27, 2025 (only in v6.18-rc6)
**Concerning**: Limited time in mainline for testing
**Mitigating factors**:
- Part of a tested patch series
- Multiple maintainer reviews
- Addresses a known issue with clear reproducer
- Similar to existing working code in register path
- Companion commit (56b3c85e153b8) explicitly tagged for stable v6.6+
### 10. COMMIT SERIES CONTEXT
This is part of a **2-commit fix series**:
**Commit 1** (56b3c85e153b8): "ftrace: Fix BPF fexit with livepatch"
- Fixes register path issues
- **Has "Fixes: d05cb470663a"**
- **Has "Cc: stable@vger.kernel.org # v6.6+"**
- Reported by production user at CrowdStrike
- Acked-and-tested-by production user
**Commit 2** (3e9a18e1c3e93): "ftrace: bpf: Fix IPMODIFY + DIRECT in
modify_ftrace_direct()" (THIS COMMIT)
- Fixes modify path issues
- **NO Fixes tag, NO stable tag**
- Part of same series (Link:
.../20251027175023.1521602-3-song@kernel.org)
**Analysis**: The companion commit was explicitly tagged for stable,
making it unusual that this one wasn't. They're addressing the same
problem space (livepatch + BPF interaction), and both should go together
for complete fix.
### 11. STABLE KERNEL APPLICABILITY
**Applies to**: All stable trees from v6.0 onwards
**Reasoning**:
- Bug introduced in v6.0 (commit 53cd885bc5c3e)
- Code exists in all v6.0+ kernels
- Should apply cleanly (no major refactoring in this area)
**Relevant stable trees**:
- v6.0.y through v6.17.y (if still maintained)
- LTS: v6.1.y, v6.6.y, v6.12.y (most important)
### 12. MEETS STABLE KERNEL RULES?
Checking against Documentation/process/stable-kernel-rules.rst:
✅ **Obviously correct**: Yes - adds missing check parallel to existing
code
✅ **Tested**: Yes - part of series with selftests, multiple reviews
✅ **Fixes real bug**: Yes - livepatch bypass is real, has reproducer
✅ **Important issue**: Yes - security implications (patch bypass)
✅ **Small and contained**: Yes - ~40 lines, single file, surgical
❌ **No new features**: Correct - pure bug fix
✅ **Should apply cleanly**: Yes - no dependencies beyond v6.0 feature
**Verdict**: Meets all stable kernel criteria
---
## FINAL DECISION
This commit fixes a serious correctness bug where livepatches can be
silently bypassed when BPF fentry/fexit programs are attached. The bug
has been present since v6.0 (July 2022) when IPMODIFY+DIRECT sharing was
introduced.
**Security Impact**: Livepatches are often used to deploy security fixes
without rebooting. Bypassing them means systems remain vulnerable
despite appearing to be patched. This is a silent failure - users have
no indication their security patches aren't working.
**Technical Quality**: The fix is small (~40 lines), surgical, and
parallels existing code in the register path. It adds the missing
`__ftrace_hash_update_ipmodify()` call to the modify path, ensuring
proper coordination between livepatch (IPMODIFY) and BPF trampolines
(DIRECT).
**Review and Testing**: Multiple expert maintainers reviewed and acked
this (Jiri Olsa, Steven Rostedt, Alexei Starovoitov). It's part of a
tested patch series that includes selftests. The companion commit
(56b3c85e153b8) explicitly has "Cc: stable@vger.kernel.org # v6.6+".
**Regression Risk**: LOW. The change is well-contained, adds a check
that should have existed from the start, and has proper error handling.
The failure mode (if any) is an explicit error return, not silent
corruption.
**Stable Applicability**: Should backport to all v6.0+ stable trees. The
code is present and should apply cleanly. Most critical for LTS trees
(v6.1.y, v6.6.y, v6.12.y) where users depend on both livepatching and
BPF.
**Why No Stable Tag?**: Unclear why the maintainer didn't add "Cc:
stable@vger.kernel.org" when the companion commit has one. This appears
to be an oversight, as both commits address the same problem space and
were submitted together.
**Conclusion**: This is exactly the type of fix stable trees need - it
addresses a real, security-relevant bug affecting production systems
with a small, well-reviewed patch. The lack of an explicit stable tag
appears to be an oversight given that the companion commit has one. Both
commits should be backported together for a complete fix.
**YES**
kernel/trace/ftrace.c | 40 +++++++++++++++++++++++++++++++---------
1 file changed, 31 insertions(+), 9 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 42bd2ba68a821..5ad1138a1894c 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1971,7 +1971,8 @@ static void ftrace_hash_rec_enable_modify(struct ftrace_ops *ops)
*/
static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
struct ftrace_hash *old_hash,
- struct ftrace_hash *new_hash)
+ struct ftrace_hash *new_hash,
+ bool update_target)
{
struct ftrace_page *pg;
struct dyn_ftrace *rec, *end = NULL;
@@ -2006,10 +2007,13 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
if (rec->flags & FTRACE_FL_DISABLED)
continue;
- /* We need to update only differences of filter_hash */
+ /*
+ * Unless we are updating the target of a direct function,
+ * we only need to update differences of filter_hash
+ */
in_old = !!ftrace_lookup_ip(old_hash, rec->ip);
in_new = !!ftrace_lookup_ip(new_hash, rec->ip);
- if (in_old == in_new)
+ if (!update_target && (in_old == in_new))
continue;
if (in_new) {
@@ -2020,7 +2024,16 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
if (is_ipmodify)
goto rollback;
- FTRACE_WARN_ON(rec->flags & FTRACE_FL_DIRECT);
+ /*
+ * If this is called by __modify_ftrace_direct()
+ * then it is only changing where the direct
+ * pointer is jumping to, and the record already
+ * points to a direct trampoline. If it isn't,
+ * then it is a bug to update ipmodify on a direct
+ * caller.
+ */
+ FTRACE_WARN_ON(!update_target &&
+ (rec->flags & FTRACE_FL_DIRECT));
/*
* Another ops with IPMODIFY is already
@@ -2076,7 +2089,7 @@ static int ftrace_hash_ipmodify_enable(struct ftrace_ops *ops)
if (ftrace_hash_empty(hash))
hash = NULL;
- return __ftrace_hash_update_ipmodify(ops, EMPTY_HASH, hash);
+ return __ftrace_hash_update_ipmodify(ops, EMPTY_HASH, hash, false);
}
/* Disabling always succeeds */
@@ -2087,7 +2100,7 @@ static void ftrace_hash_ipmodify_disable(struct ftrace_ops *ops)
if (ftrace_hash_empty(hash))
hash = NULL;
- __ftrace_hash_update_ipmodify(ops, hash, EMPTY_HASH);
+ __ftrace_hash_update_ipmodify(ops, hash, EMPTY_HASH, false);
}
static int ftrace_hash_ipmodify_update(struct ftrace_ops *ops,
@@ -2101,7 +2114,7 @@ static int ftrace_hash_ipmodify_update(struct ftrace_ops *ops,
if (ftrace_hash_empty(new_hash))
new_hash = NULL;
- return __ftrace_hash_update_ipmodify(ops, old_hash, new_hash);
+ return __ftrace_hash_update_ipmodify(ops, old_hash, new_hash, false);
}
static void print_ip_ins(const char *fmt, const unsigned char *p)
@@ -6106,7 +6119,7 @@ EXPORT_SYMBOL_GPL(unregister_ftrace_direct);
static int
__modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
{
- struct ftrace_hash *hash;
+ struct ftrace_hash *hash = ops->func_hash->filter_hash;
struct ftrace_func_entry *entry, *iter;
static struct ftrace_ops tmp_ops = {
.func = ftrace_stub,
@@ -6126,13 +6139,21 @@ __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
if (err)
return err;
+ /*
+ * Call __ftrace_hash_update_ipmodify() here, so that we can call
+ * ops->ops_func for the ops. This is needed because the above
+ * register_ftrace_function_nolock() worked on tmp_ops.
+ */
+ err = __ftrace_hash_update_ipmodify(ops, hash, hash, true);
+ if (err)
+ goto out;
+
/*
* Now the ftrace_ops_list_func() is called to do the direct callers.
* We can safely change the direct functions attached to each entry.
*/
mutex_lock(&ftrace_lock);
- hash = ops->func_hash->filter_hash;
size = 1 << hash->size_bits;
for (i = 0; i < size; i++) {
hlist_for_each_entry(iter, &hash->buckets[i], hlist) {
@@ -6147,6 +6168,7 @@ __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
mutex_unlock(&ftrace_lock);
+out:
/* Removing the tmp_ops will add the updated direct callers to the functions */
unregister_ftrace_function(&tmp_ops);
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v8 21/28] KVM: arm64: Add tracing capability for the pKVM hyp
From: Vincent Donnefort @ 2025-11-20 12:01 UTC (permalink / raw)
To: Marc Zyngier
Cc: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <86bjkyrly9.wl-maz@kernel.org>
On Wed, Nov 19, 2025 at 05:06:38PM +0000, Marc Zyngier wrote:
> On Fri, 07 Nov 2025 09:38:33 +0000,
> Vincent Donnefort <vdonnefort@google.com> wrote:
> >
> > When running with protected mode, the host has very little knowledge
> > about what is happening in the hypervisor. Of course this is an
> > essential feature for security but nonetheless, that piece of code
> > growing with more responsibilities, we need now a way to debug and
> > profile it. Tracefs by its reliability, versatility and support for
> > user-space is the perfect tool.
> >
> > There's no way the hypervisor could log events directly into the host
> > tracefs ring-buffers. So instead let's use our own, where the hypervisor
> > is the writer and the host the reader.
> >
> > Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> >
> > diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> > index 9da54d4ee49e..ad02dee140d3 100644
> > --- a/arch/arm64/include/asm/kvm_asm.h
> > +++ b/arch/arm64/include/asm/kvm_asm.h
> > @@ -89,6 +89,10 @@ enum __kvm_host_smccc_func {
> > __KVM_HOST_SMCCC_FUNC___pkvm_vcpu_load,
> > __KVM_HOST_SMCCC_FUNC___pkvm_vcpu_put,
> > __KVM_HOST_SMCCC_FUNC___pkvm_tlb_flush_vmid,
> > + __KVM_HOST_SMCCC_FUNC___pkvm_load_tracing,
> > + __KVM_HOST_SMCCC_FUNC___pkvm_unload_tracing,
> > + __KVM_HOST_SMCCC_FUNC___pkvm_enable_tracing,
> > + __KVM_HOST_SMCCC_FUNC___pkvm_swap_reader_tracing,
> > };
> >
> > #define DECLARE_KVM_VHE_SYM(sym) extern char sym[]
> > diff --git a/arch/arm64/include/asm/kvm_hyptrace.h b/arch/arm64/include/asm/kvm_hyptrace.h
> > new file mode 100644
> > index 000000000000..9c30a479bc36
> > --- /dev/null
> > +++ b/arch/arm64/include/asm/kvm_hyptrace.h
> > @@ -0,0 +1,13 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +#ifndef __ARM64_KVM_HYPTRACE_H_
> > +#define __ARM64_KVM_HYPTRACE_H_
> > +
> > +#include <linux/ring_buffer.h>
> > +
> > +struct hyp_trace_desc {
> > + unsigned long bpages_backing_start;
>
> Why is this an integer type? You keep casting it all over the place,
> which tells me that's not the ideal type.
That's because it is a kern VA the hyp needs to convert. However it would indeed
make my life easier to declare it as a struct simple_buffer_page * in the
struct hyp_trace_buffer below.
>
> > + size_t bpages_backing_size;
> > + struct trace_buffer_desc trace_buffer_desc;
> > +
> > +};
> > +#endif
> > diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> > index 4f803fd1c99a..580426cdbe77 100644
> > --- a/arch/arm64/kvm/Kconfig
> > +++ b/arch/arm64/kvm/Kconfig
> > @@ -83,4 +83,11 @@ config PTDUMP_STAGE2_DEBUGFS
> >
> > If in doubt, say N.
> >
> > +config PKVM_TRACING
> > + bool
> > + depends on KVM
> > + depends on TRACING
> > + select SIMPLE_RING_BUFFER
> > + default y
>
> I'd rather this is made to depend on NVHE_EL2_DEBUG, just like the
> other debug options.
NVHE_EL2_DEBUG is unsafe for production because of the stage-2 relax on panic.
While this one is. So ideally this should be usable even without NVHE_EL2_DEBUG.
I can remove this hidden PKVM_TRACING option and use everywhere CONFIG_TRACING. But
then I need something to select SIMPLE_RING_BUFFER.
Perhaps with the following?
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index 2ae6bf499236..c561bf9d4754 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -38,6 +38,7 @@ menuconfig KVM
select SCHED_INFO
select GUEST_PERF_EVENTS if PERF_EVENTS
select KVM_GUEST_MEMFD
+ select SIMPLE_RING_BUFFER if CONFIG_TRACING
>
> > +
> > endif # VIRTUALIZATION
> > diff --git a/arch/arm64/kvm/hyp/include/nvhe/trace.h b/arch/arm64/kvm/hyp/include/nvhe/trace.h
> > new file mode 100644
> > index 000000000000..996e90c0974f
> > --- /dev/null
> > +++ b/arch/arm64/kvm/hyp/include/nvhe/trace.h
> > @@ -0,0 +1,23 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +#ifndef __ARM64_KVM_HYP_NVHE_TRACE_H
> > +#define __ARM64_KVM_HYP_NVHE_TRACE_H
> > +#include <asm/kvm_hyptrace.h>
> > +
> > +#ifdef CONFIG_PKVM_TRACING
> > +void *tracing_reserve_entry(unsigned long length);
> > +void tracing_commit_entry(void);
> > +
> > +int __pkvm_load_tracing(unsigned long desc_va, size_t desc_size);
> > +void __pkvm_unload_tracing(void);
> > +int __pkvm_enable_tracing(bool enable);
> > +int __pkvm_swap_reader_tracing(unsigned int cpu);
> > +#else
> > +static inline void *tracing_reserve_entry(unsigned long length) { return NULL; }
> > +static inline void tracing_commit_entry(void) { }
> > +
> > +static inline int __pkvm_load_tracing(unsigned long desc_va, size_t desc_size) { return -ENODEV; }
> > +static inline void __pkvm_unload_tracing(void) { }
> > +static inline int __pkvm_enable_tracing(bool enable) { return -ENODEV; }
> > +static inline int __pkvm_swap_reader_tracing(unsigned int cpu) { return -ENODEV; }
> > +#endif
> > +#endif
> > diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
> > index f55a9a17d38f..504c3b9caef8 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/Makefile
> > +++ b/arch/arm64/kvm/hyp/nvhe/Makefile
> > @@ -29,7 +29,7 @@ hyp-obj-y += ../vgic-v3-sr.o ../aarch32.o ../vgic-v2-cpuif-proxy.o ../entry.o \
> > ../fpsimd.o ../hyp-entry.o ../exception.o ../pgtable.o
> > hyp-obj-y += ../../../kernel/smccc-call.o
> > hyp-obj-$(CONFIG_LIST_HARDENED) += list_debug.o
> > -hyp-obj-$(CONFIG_PKVM_TRACING) += clock.o
> > +hyp-obj-$(CONFIG_PKVM_TRACING) += clock.o trace.o ../../../../../kernel/trace/simple_ring_buffer.o
>
> Can we get something less awful here? Surely there is a way to get an
> absolute path from the kbuild infrastructure? $(objtree) springs to
> mind...
Ack.
[...]
> > +int __pkvm_load_tracing(unsigned long desc_hva, size_t desc_size)
> > +{
> > + struct hyp_trace_desc *desc = (struct hyp_trace_desc *)kern_hyp_va(desc_hva);
> > + int ret;
> > +
> > + if (!desc_size || !PAGE_ALIGNED(desc_hva) || !PAGE_ALIGNED(desc_size))
> > + return -EINVAL;
> > +
> > + ret = __pkvm_host_donate_hyp(hyp_virt_to_pfn((void *)desc),
> > + desc_size >> PAGE_SHIFT);
> > + if (ret)
> > + return ret;
> > +
> > + if (!hyp_trace_desc_validate(desc, desc_size))
> > + goto err_donate_desc;
> > +
> > + hyp_spin_lock(&trace_buffer.lock);
> > +
> > + ret = hyp_trace_buffer_load(&trace_buffer, desc);
> > +
> > + hyp_spin_unlock(&trace_buffer.lock);
> > +
> > +err_donate_desc:
> > + WARN_ON(__pkvm_hyp_donate_host(hyp_virt_to_pfn((void *)desc),
> > + desc_size >> PAGE_SHIFT));
>
> That's basically a guaranteed panic if anything goes wrong. Are you
> sure you want to do that?
A failure would mean a lost page for the kernel. As there's really no reason for
this to happen (the host_donate_hyp worked few lines above), it sounds alright
to panic here in this case.
In reclaim_pgtable_pages() applies the same reasoning: if hyp_donate_host fails,
something really wrong happened.
>
> > + return ret;
> > +}
> > +
> > +void __pkvm_unload_tracing(void)
> > +{
> > + hyp_spin_lock(&trace_buffer.lock);
> > + hyp_trace_buffer_unload(&trace_buffer);
> > + hyp_spin_unlock(&trace_buffer.lock);
> > +}
> > +
> > +int __pkvm_enable_tracing(bool enable)
> > +{
> > + int cpu, ret = enable ? -EINVAL : 0;
> > +
> > + hyp_spin_lock(&trace_buffer.lock);
> > +
> > + if (!hyp_trace_buffer_loaded(&trace_buffer))
> > + goto unlock;
> > +
> > + for (cpu = 0; cpu < hyp_nr_cpus; cpu++)
> > + simple_ring_buffer_enable_tracing(per_cpu_ptr(trace_buffer.simple_rbs, cpu),
> > + enable);
> > +
> > + ret = 0;
> > +
> > +unlock:
> > + hyp_spin_unlock(&trace_buffer.lock);
> > +
> > + return ret;
> > +}
> > +
> > +int __pkvm_swap_reader_tracing(unsigned int cpu)
> > +{
> > + int ret;
> > +
> > + if (cpu >= hyp_nr_cpus)
> > + return -EINVAL;
> > +
> > + hyp_spin_lock(&trace_buffer.lock);
> > +
> > + if (hyp_trace_buffer_loaded(&trace_buffer))
> > + ret = simple_ring_buffer_swap_reader_page(
> > + per_cpu_ptr(trace_buffer.simple_rbs, cpu));
>
> Please keep these things on a single line. I don't care what people
> (of checkpatch) say.
Ack.
>
> > + else
> > + ret = -ENODEV;
> > +
> > + hyp_spin_unlock(&trace_buffer.lock);
> > +
> > + return ret;
> > +}
> > --
> > 2.51.2.1041.gc1ab5b90ca-goog
> >
> >
>
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
^ permalink raw reply related
* Re: [PATCH v8 20/28] KVM: arm64: Add clock support for the pKVM hyp
From: Vincent Donnefort @ 2025-11-20 11:36 UTC (permalink / raw)
To: Marc Zyngier, g
Cc: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <86cy5erprc.wl-maz@kernel.org>
[...]
> > +/* Using host provided data. Do not use for anything else than debugging. */
> > +u64 trace_clock(void)
> > +{
> > + struct clock_data *clock = &trace_clock_data;
> > + u64 bank = smp_load_acquire(&clock->cur);
> > + u64 cyc, ns;
> > +
> > + cyc = __arch_counter_get_cntpct() - clock->data[bank].epoch_cyc;
>
> I can only imagine that you don't care about the broken systems that
> do not have a monotonic counter, and that will happily have their
> counter swing back and forth?
I haven't used the _stable() variant for the affected devices... Hum this will
not be trivial from the hypervisor. I'll see what I can do.
>
> Also, you may want to document why you are using the physical counter
> instead of the virtual one. I guess that you don't want CNTVOFF_EL2 to
> apply when HCR_EL2.E2H==0, but given that you never allow anything
> else for pKVM, this is slightly odd.
You mean I might as well use __arch_counter_get_cntvct() as CNTVOFF_EL2 will
always be ignored in the pKVM case?
>
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox