* [PATCH] tracing: Clean up use of trace_create_maxlat_file()
From: Steven Rostedt @ 2026-02-06 17:12 UTC (permalink / raw)
To: LKML, Linux Trace Kernel; +Cc: Masami Hiramatsu, Mathieu Desnoyers
From: Steven Rostedt <rostedt@goodmis.org>
In trace.c, the function trace_create_maxlat_file() is defined behind the
#ifdef CONFIG_TRACER_MAX_TRACE block. The #else part defines it as:
#define trace_create_maxlat_file(tr, d_tracer) \
trace_create_file("tracing_max_latency", TRACE_MODE_WRITE, \
d_tracer, tr, &tracing_max_lat_fops)
But the one place that it it used has:
#ifdef CONFIG_TRACER_MAX_TRACE
trace_create_maxlat_file(tr, d_tracer);
#endif
Which is pointless.
Define trace_create_maxlat_file() when CONFIG_TRACER_MAX_TRACE is not
defined as:
static inline void trace_create_maxlat_file(struct trace_array *tr,
struct dentry *d_tracer) { }
And remove the #ifdef's from the code.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index dfa30711c6e2..5bc22d966834 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1547,10 +1547,8 @@ void latency_fsnotify(struct trace_array *tr)
#else /* !LATENCY_FS_NOTIFY */
-#define trace_create_maxlat_file(tr, d_tracer) \
- trace_create_file("tracing_max_latency", TRACE_MODE_WRITE, \
- d_tracer, tr, &tracing_max_lat_fops)
-
+static inline void trace_create_maxlat_file(struct trace_array *tr,
+ struct dentry *d_tracer) { }
#endif
/*
@@ -9909,9 +9907,7 @@ init_tracer_tracefs(struct trace_array *tr, struct dentry *d_tracer)
create_trace_options_dir(tr);
-#ifdef CONFIG_TRACER_MAX_TRACE
trace_create_maxlat_file(tr, d_tracer);
-#endif
if (ftrace_create_function_files(tr, d_tracer))
MEM_FAIL(1, "Could not allocate function filter files");
--
2.51.0
^ permalink raw reply related
* Re: [RFC bpf-next 00/12] bpf: tracing_multi link
From: Andrii Nakryiko @ 2026-02-06 17:03 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, bpf, linux-trace-kernel, Martin KaFai Lau,
Eduard Zingerman, Song Liu, Yonghong Song, Menglong Dong,
Steven Rostedt
In-Reply-To: <aYWjxMeXABGcYqEI@krava>
On Fri, Feb 6, 2026 at 12:18 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Thu, Feb 05, 2026 at 07:55:19AM -0800, Alexei Starovoitov wrote:
> > On Thu, Feb 5, 2026 at 12:55 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > >
> > > On Wed, Feb 04, 2026 at 08:06:50AM -0800, Alexei Starovoitov wrote:
> > > > On Wed, Feb 4, 2026 at 4:36 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > > > >
> > > > > On Tue, Feb 03, 2026 at 03:17:05PM -0800, Alexei Starovoitov wrote:
> > > > > > On Tue, Feb 3, 2026 at 1:38 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > > > > >
> > > > > > > hi,
> > > > > > > as an option to Meglong's change [1] I'm sending proposal for tracing_multi
> > > > > > > link that does not add static trampoline but attaches program to all needed
> > > > > > > trampolines.
> > > > > > >
> > > > > > > This approach keeps the same performance but has some drawbacks:
> > > > > > >
> > > > > > > - when attaching 20k functions we allocate and attach 20k trampolines
> > > > > > > - during attachment we hold each trampoline mutex, so for above
> > > > > > > 20k functions we will hold 20k mutexes during the attachment,
> > > > > > > should be very prone to deadlock, but haven't hit it yet
> > > > > >
> > > > > > If you check that it's sorted and always take them in the same order
> > > > > > then there will be no deadlock.
> > > > > > Or just grab one global mutex first and then grab trampolines mutexes
> > > > > > next in any order. The global one will serialize this attach operation.
> > > > > >
> > > > > > > It looks the trampoline allocations/generation might not be big a problem
> > > > > > > and I'll try to find a solution for holding that many mutexes. If there's
> > > > > > > no better solution I think having one read/write mutex for tracing multi
> > > > > > > link attach/detach should work.
> > > > > >
> > > > > > If you mean to have one global mutex as I proposed above then I don't see
> > > > > > a downside. It only serializes multiple libbpf calls.
> > > > >
> > > > > we also need to serialize it with standard single trampoline attach,
> > > > > because the direct ftrace update is now done under trampoline->mutex:
> > > > >
> > > > > bpf_trampoline_link_prog(tr)
> > > > > {
> > > > > mutex_lock(&tr->mutex);
> > > > > ...
> > > > > update_ftrace_direct_*
> > > > > ...
> > > > > mutex_unlock(&tr->mutex);
> > > > > }
> > > > >
> > > > > for tracing_multi we would link the program first (with tr->mutex)
> > > > > and do the bulk ftrace update later (without tr->mutex)
> > > > >
> > > > > {
> > > > > for each involved trampoline:
> > > > > bpf_trampoline_link_prog
> > > > >
> > > > > --> and here we could race with some other thread doing single
> > > > > trampoline attach
> > > > >
> > > > > update_ftrace_direct_*
> > > > > }
> > > > >
> > > > > note the current version locks all tr->mutex instances all the way
> > > > > through the update_ftrace_direct_* update
> > > > >
> > > > > I think we could use global rwsem and take read lock on single
> > > > > trampoline attach path and write lock on tracing_multi attach,
> > > > >
> > > > > I thought we could take direct_mutex early, but that would mean
> > > > > different order with trampoline mutex than we already have in
> > > > > single attach path
> > > >
> > > > I feel we're talking past each other.
> > > > I meant:
> > > >
> > > > For multi:
> > > > 1. take some global mutex
> > > > 2. take N tramp mutexes in any order
> > > >
> > > > For single:
> > > > 1. take that 1 specific tramp mutex.
> > >
> > > ah ok, I understand, it's to prevent the lockup but keep holding all
> > > the trampolines locks.. the rwsem I mentioned was for the 'fix', where
> > > we do not take all the trampolines locks
> >
> > I don't understand how rwsem would help.
> > All the operations on trampoline are protected by mutex.
> > Switching to rw makes sense only if we can designate certain
> > operations as "read" and others as "write" and number of "reads"
> > dominate. This won't be the case with multi-fentry.
> > And we still need to take all of them as "write" to update trampoline.
>
> this applies to scenario where we do not hold all the trampoline locks,
> in such case we could have race between single and multi attachment,
> while single/single attachment race stays safe
>
> as a fix the single attach would take read lock and multi attach would
> take write lock, so single/single race is allowed and single/multi is
> not ... showed in the patch below
>
> but it might be too much.. in a sense that there's already many locks
> involved in trampoline attach/detach, and simple global lock in multi
> or just sorting the ids would be enough
>
I'll just throw this idea here, but we don't have to do it right away.
What if instead of having a per-trampoline lock, we just have a common
relatively small pool of locks that all trampolines share based on
some hash (i.e., we deterministically map trampoline to one of the
locks). Then multi-attach can just go and grab all of them in
predefined order, while singular trampoline attaches will just get
their own one. We won't need to sort anything, we reduce the amount of
different locks. I don't think lock contention (due to lock sharing
for some trampolines) is a real issue to be worried about either.
> jirka
>
>
> ---
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index b76bb545077b..edbc8f133dda 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -30,6 +30,8 @@ static struct hlist_head trampoline_ip_table[TRAMPOLINE_TABLE_SIZE];
> /* serializes access to trampoline tables */
> static DEFINE_MUTEX(trampoline_mutex);
>
> +static DECLARE_RWSEM(multi_sem);
> +
> struct bpf_trampoline_ops {
> int (*register_fentry)(struct bpf_trampoline *tr, void *new_addr, void *data);
> int (*unregister_fentry)(struct bpf_trampoline *tr, u32 orig_flags, void *old_addr, void *data);
> @@ -367,11 +369,7 @@ static struct bpf_trampoline *bpf_trampoline_lookup(u64 key, unsigned long ip)
> head = &trampoline_ip_table[hash_64(tr->ip, TRAMPOLINE_HASH_BITS)];
> hlist_add_head(&tr->hlist_ip, head);
> refcount_set(&tr->refcnt, 1);
> -#ifdef CONFIG_LOCKDEP
> - mutex_init_with_key(&tr->mutex, &__lockdep_no_track__);
> -#else
> mutex_init(&tr->mutex);
> -#endif
> for (i = 0; i < BPF_TRAMP_MAX; i++)
> INIT_HLIST_HEAD(&tr->progs_hlist[i]);
> out:
> @@ -871,6 +869,8 @@ int bpf_trampoline_link_prog(struct bpf_tramp_node *node,
> {
> int err;
>
> + guard(rwsem_read)(&multi_sem);
> +
> mutex_lock(&tr->mutex);
> err = __bpf_trampoline_link_prog(node, tr, tgt_prog, &trampoline_ops, NULL);
> mutex_unlock(&tr->mutex);
> @@ -916,6 +916,8 @@ int bpf_trampoline_unlink_prog(struct bpf_tramp_node *node,
> {
> int err;
>
> + guard(rwsem_read)(&multi_sem);
> +
> mutex_lock(&tr->mutex);
> err = __bpf_trampoline_unlink_prog(node, tr, tgt_prog, &trampoline_ops, NULL);
> mutex_unlock(&tr->mutex);
> @@ -1463,6 +1465,8 @@ int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
> struct bpf_trampoline *tr;
> u64 key;
>
> + guard(rwsem_write)(&multi_sem);
> +
> data.reg = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
> if (!data.reg)
> return -ENOMEM;
> @@ -1494,12 +1498,10 @@ int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
> tr = mnode->trampoline;
>
> mutex_lock(&tr->mutex);
> -
> err = __bpf_trampoline_link_prog(&mnode->node, tr, NULL, &trampoline_multi_ops, &data);
> - if (err) {
> - mutex_unlock(&tr->mutex);
> + mutex_unlock(&tr->mutex);
> + if (err)
> goto rollback_unlink;
> - }
> }
>
> if (ftrace_hash_count(data.reg)) {
> @@ -1516,11 +1518,6 @@ int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
> }
> }
>
> - for (i = 0; i < cnt; i++) {
> - tr = link->nodes[i].trampoline;
> - mutex_unlock(&tr->mutex);
> - }
> -
> free_fentry_multi_data(&data);
> return 0;
>
> @@ -1528,6 +1525,7 @@ int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
> for (j = 0; j < i; j++) {
> mnode = &link->nodes[j];
> tr = mnode->trampoline;
> + mutex_lock(&tr->mutex);
> WARN_ON_ONCE(__bpf_trampoline_unlink_prog(&mnode->node, tr, NULL,
> &trampoline_multi_ops, &data));
> mutex_unlock(&tr->mutex);
> @@ -1550,6 +1548,8 @@ int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_
> int i, cnt = link->nodes_cnt;
> struct bpf_trampoline *tr;
>
> + guard(rwsem_write)(&multi_sem);
> +
> data.unreg = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
> if (!data.unreg)
> return -ENOMEM;
> @@ -1567,6 +1567,7 @@ int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_
> mutex_lock(&tr->mutex);
> WARN_ON_ONCE(__bpf_trampoline_unlink_prog(&mnode->node, tr, NULL,
> &trampoline_multi_ops, &data));
> + mutex_unlock(&tr->mutex);
> }
>
> if (ftrace_hash_count(data.unreg))
> @@ -1576,7 +1577,6 @@ int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_
>
> for (i = 0; i < cnt; i++) {
> tr = link->nodes[i].trampoline;
> - mutex_unlock(&tr->mutex);
> bpf_trampoline_put(tr);
> }
>
^ permalink raw reply
* Re: [RFC bpf-next 08/12] libbpf: Add btf__find_by_glob_kind function
From: Andrii Nakryiko @ 2026-02-06 16:58 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
linux-trace-kernel, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, Menglong Dong, Steven Rostedt
In-Reply-To: <aYWpotJVLSZdiZgG@krava>
On Fri, Feb 6, 2026 at 12:43 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Thu, Feb 05, 2026 at 02:45:14PM -0800, Andrii Nakryiko wrote:
> > On Thu, Feb 5, 2026 at 12:57 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > >
> > > On Wed, Feb 04, 2026 at 11:04:09AM -0800, Andrii Nakryiko wrote:
> > > > On Tue, Feb 3, 2026 at 1:39 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > > >
> > > > > Adding btf__find_by_glob_kind function that returns array of
> > > > > BTF ids that match given kind and allow/deny patterns.
> > > > >
> > > > > int btf__find_by_glob_kind(const struct btf *btf, __u32 kind,
> > > > > const char *allow_pattern,
> > > > > const char *deny_pattern,
> > > > > __u32 **__ids);
> > > > >
> > > > > The __ids array is allocated and needs to be manually freed.
> > > > >
> > > > > The pattern check is done by glob_match function.
> > > > >
> > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > > > ---
> > > > > tools/lib/bpf/btf.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > > > > tools/lib/bpf/btf.h | 3 +++
> > > > > 2 files changed, 44 insertions(+)
> > > > >
> > > > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > > > > index 83fe79ffcb8f..64502b3ef38a 100644
> > > > > --- a/tools/lib/bpf/btf.c
> > > > > +++ b/tools/lib/bpf/btf.c
> > > > > @@ -1010,6 +1010,47 @@ __s32 btf__find_by_name_kind(const struct btf *btf, const char *type_name,
> > > > > return btf_find_by_name_kind(btf, 1, type_name, kind);
> > > > > }
> > > > >
> > > > > +int btf__find_by_glob_kind(const struct btf *btf, __u32 kind,
> > > > > + const char *allow_pattern, const char *deny_pattern,
> > > > > + __u32 **__ids)
> > > > > +{
> > > > > + __u32 i, nr_types = btf__type_cnt(btf);
> > > > > + int cnt = 0, alloc = 0;
> > > > > + __u32 *ids = NULL;
> > > > > +
> > > > > + for (i = 1; i < nr_types; i++) {
> > > > > + const struct btf_type *t = btf__type_by_id(btf, i);
> > > > > + const char *name;
> > > > > + __u32 *p;
> > > > > +
> > > > > + if (btf_kind(t) != kind)
> > > > > + continue;
> > > > > + name = btf__name_by_offset(btf, t->name_off);
> > > > > + if (!name)
> > > > > + continue;
> > > > > +
> > > > > + if (deny_pattern && glob_match(name, deny_pattern))
> > > > > + continue;
> > > > > + if (allow_pattern && !glob_match(name, allow_pattern))
> > > > > + continue;
> > > > > +
> > > > > + if (cnt == alloc) {
> > > > > + alloc = max(16, alloc * 3 / 2);
> > > > > + p = libbpf_reallocarray(ids, alloc, sizeof(__u32));
> > > > > + if (!p) {
> > > > > + free(ids);
> > > > > + return -ENOMEM;
> > > > > + }
> > > > > + ids = p;
> > > > > + }
> > > > > + ids[cnt] = i;
> > > > > + cnt++;
> > > > > + }
> > > > > +
> > > > > + *__ids = ids;
> > > > > + return cnt;
> > > > > +}
> > > > > +
> > > > > static bool btf_is_modifiable(const struct btf *btf)
> > > > > {
> > > > > return (void *)btf->hdr != btf->raw_data;
> > > > > diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> > > > > index b30008c267c0..d7b47bb0ba99 100644
> > > > > --- a/tools/lib/bpf/btf.h
> > > > > +++ b/tools/lib/bpf/btf.h
> > > > > @@ -661,6 +661,9 @@ static inline struct btf_decl_tag *btf_decl_tag(const struct btf_type *t)
> > > > > return (struct btf_decl_tag *)(t + 1);
> > > > > }
> > > > >
> > > > > +int btf__find_by_glob_kind(const struct btf *btf, __u32 kind,
> > > > > + const char *allow_pattern, const char *deny_pattern,
> > > > > + __u32 **__ids);
> > > >
> > > >
> > > > as AI pointed out, this should be an internal helper, no? Let's also
> > > > not use double underscore pattern here,
> > > > "collect_btf_ids_by_glob_kind()" perhaps?
> > >
> > > ok
> > >
> > > >
> > > > Also, you don't seem to be using deny_pattern, where you planning to?
> > >
> > > the tests are just rudimentary before we agree we want to do it this way
> > >
> > > but I'm not sure I have a usecase for deny_pattern.. I think we added it
> > > just to be complete, I recall we copied that function from somewhere,
> > > it's long time ago ;-)
> > >
> > > >
> > > > Also, are there functions that we'll have BTF for, but they won't be
> > > > attachable? What if I do SEC("fentry.multi/*")? Will it attach or fail
> > > > to attach some functions (and thus fail the overall attachment)?
> > >
> > > yes, for the benchmark tests I had to add is_allowed_func which mimics
> > > btf_distill_func_proto and denies attach for some functions
> > >
> > > also I had to filter out some core kernel functions like rcu*,trace*,..
> > > which seemed to cause trouble when you attach them
> >
> > So the question I'm implying here is if libbpf should do what we do
> > for kprobes: use libbpf_available_kprobes_parse and intersect?
>
> right, I think it's good idea.. and in addition (just for patterns) we would
> filter out functions that:
>
> - won't attach (is_allowed_func == false)
> - might cause problems (rcu*,trace*), maybe for that we could have
> opts config bool
Well, who's going to maintain the "might cause problems" list in
libbpf? Let's not try to be too smart. If some functions are not safe
to be attached, we should mark them as such in the kernel, IMO.
>
> jirka
^ permalink raw reply
* [PATCH 2/2] tracing: Rename trace_array field max_buffer to snapshot_buffer
From: Steven Rostedt @ 2026-02-06 16:35 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton
In-Reply-To: <20260206163554.153428314@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
When tracing was first added, there were latency tracers that would take a
snapshot of the current trace when a new max latency was hit. This
snapshot buffer was called "max_buffer". Since then, a snapshot feature
was added that allowed user space or event triggers to trigger a snapshot
of the current buffer using the same max_buffer of the trace_array.
As this snapshot buffer now has a more generic use case, calling it
"max_buffer" is confusing. Rename it to snapshot_buffer.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace.c | 72 +++++++++++++++++------------------
kernel/trace/trace.h | 13 ++++---
kernel/trace/trace_selftest.c | 10 ++---
3 files changed, 48 insertions(+), 47 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 54851ffbabeb..dfa30711c6e2 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -934,12 +934,12 @@ int tracing_alloc_snapshot_instance(struct trace_array *tr)
/* Make the snapshot buffer have the same order as main buffer */
order = ring_buffer_subbuf_order_get(tr->array_buffer.buffer);
- ret = ring_buffer_subbuf_order_set(tr->max_buffer.buffer, order);
+ ret = ring_buffer_subbuf_order_set(tr->snapshot_buffer.buffer, order);
if (ret < 0)
return ret;
/* allocate spare buffer */
- ret = resize_buffer_duplicate_size(&tr->max_buffer,
+ ret = resize_buffer_duplicate_size(&tr->snapshot_buffer,
&tr->array_buffer, RING_BUFFER_ALL_CPUS);
if (ret < 0)
return ret;
@@ -957,10 +957,10 @@ static void free_snapshot(struct trace_array *tr)
* The max_tr ring buffer has some state (e.g. ring->clock) and
* we want preserve it.
*/
- ring_buffer_subbuf_order_set(tr->max_buffer.buffer, 0);
- ring_buffer_resize(tr->max_buffer.buffer, 1, RING_BUFFER_ALL_CPUS);
- set_buffer_entries(&tr->max_buffer, 1);
- tracing_reset_online_cpus(&tr->max_buffer);
+ ring_buffer_subbuf_order_set(tr->snapshot_buffer.buffer, 0);
+ ring_buffer_resize(tr->snapshot_buffer.buffer, 1, RING_BUFFER_ALL_CPUS);
+ set_buffer_entries(&tr->snapshot_buffer, 1);
+ tracing_reset_online_cpus(&tr->snapshot_buffer);
tr->allocated_snapshot = false;
}
@@ -1562,7 +1562,7 @@ static void
__update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu)
{
struct array_buffer *trace_buf = &tr->array_buffer;
- struct array_buffer *max_buf = &tr->max_buffer;
+ struct array_buffer *max_buf = &tr->snapshot_buffer;
struct trace_array_cpu *data = per_cpu_ptr(trace_buf->data, cpu);
struct trace_array_cpu *max_data = per_cpu_ptr(max_buf->data, cpu);
@@ -1622,9 +1622,9 @@ update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu,
/* Inherit the recordable setting from array_buffer */
if (ring_buffer_record_is_set_on(tr->array_buffer.buffer))
- ring_buffer_record_on(tr->max_buffer.buffer);
+ ring_buffer_record_on(tr->snapshot_buffer.buffer);
else
- ring_buffer_record_off(tr->max_buffer.buffer);
+ ring_buffer_record_off(tr->snapshot_buffer.buffer);
#ifdef CONFIG_TRACER_SNAPSHOT
if (tr->cond_snapshot && !tr->cond_snapshot->update(tr, cond_data)) {
@@ -1632,7 +1632,7 @@ update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu,
return;
}
#endif
- swap(tr->array_buffer.buffer, tr->max_buffer.buffer);
+ swap(tr->array_buffer.buffer, tr->snapshot_buffer.buffer);
__update_max_tr(tr, tsk, cpu);
@@ -1667,7 +1667,7 @@ update_max_tr_single(struct trace_array *tr, struct task_struct *tsk, int cpu)
arch_spin_lock(&tr->max_lock);
- ret = ring_buffer_swap_cpu(tr->max_buffer.buffer, tr->array_buffer.buffer, cpu);
+ ret = ring_buffer_swap_cpu(tr->snapshot_buffer.buffer, tr->array_buffer.buffer, cpu);
if (ret == -EBUSY) {
/*
@@ -1677,7 +1677,7 @@ update_max_tr_single(struct trace_array *tr, struct task_struct *tsk, int cpu)
* and flag that it failed.
* Another reason is resize is in progress.
*/
- trace_array_printk_buf(tr->max_buffer.buffer, _THIS_IP_,
+ trace_array_printk_buf(tr->snapshot_buffer.buffer, _THIS_IP_,
"Failed to swap buffers due to commit or resize in progress\n");
}
@@ -1726,7 +1726,7 @@ static int wait_on_pipe(struct trace_iterator *iter, int full)
* to happen, this would now be the main buffer.
*/
if (iter->snapshot)
- iter->array_buffer = &iter->tr->max_buffer;
+ iter->array_buffer = &iter->tr->snapshot_buffer;
#endif
return ret;
}
@@ -1794,7 +1794,7 @@ static int run_tracer_selftest(struct tracer *type)
if (type->use_max_tr) {
/* If we expanded the buffers, make sure the max is expanded too */
if (tr->ring_buffer_expanded)
- ring_buffer_resize(tr->max_buffer.buffer, trace_buf_size,
+ ring_buffer_resize(tr->snapshot_buffer.buffer, trace_buf_size,
RING_BUFFER_ALL_CPUS);
tr->allocated_snapshot = true;
}
@@ -1821,7 +1821,7 @@ static int run_tracer_selftest(struct tracer *type)
/* Shrink the max buffer again */
if (tr->ring_buffer_expanded)
- ring_buffer_resize(tr->max_buffer.buffer, 1,
+ ring_buffer_resize(tr->snapshot_buffer.buffer, 1,
RING_BUFFER_ALL_CPUS);
}
#endif
@@ -2064,7 +2064,7 @@ void tracing_reset_all_online_cpus_unlocked(void)
tr->clear_trace = false;
tracing_reset_online_cpus(&tr->array_buffer);
#ifdef CONFIG_TRACER_SNAPSHOT
- tracing_reset_online_cpus(&tr->max_buffer);
+ tracing_reset_online_cpus(&tr->snapshot_buffer);
#endif
}
}
@@ -2104,7 +2104,7 @@ static void tracing_start_tr(struct trace_array *tr)
ring_buffer_record_enable(buffer);
#ifdef CONFIG_TRACER_SNAPSHOT
- buffer = tr->max_buffer.buffer;
+ buffer = tr->snapshot_buffer.buffer;
if (buffer)
ring_buffer_record_enable(buffer);
#endif
@@ -2140,7 +2140,7 @@ static void tracing_stop_tr(struct trace_array *tr)
ring_buffer_record_disable(buffer);
#ifdef CONFIG_TRACER_SNAPSHOT
- buffer = tr->max_buffer.buffer;
+ buffer = tr->snapshot_buffer.buffer;
if (buffer)
ring_buffer_record_disable(buffer);
#endif
@@ -3947,7 +3947,7 @@ __tracing_open(struct inode *inode, struct file *file, bool snapshot)
#ifdef CONFIG_TRACER_SNAPSHOT
/* Currently only the top directory has a snapshot */
if (tr->current_trace->print_max || snapshot)
- iter->array_buffer = &tr->max_buffer;
+ iter->array_buffer = &tr->snapshot_buffer;
else
#endif
iter->array_buffer = &tr->array_buffer;
@@ -4150,7 +4150,7 @@ static int tracing_open(struct inode *inode, struct file *file)
#ifdef CONFIG_TRACER_MAX_TRACE
if (tr->current_trace->print_max)
- trace_buf = &tr->max_buffer;
+ trace_buf = &tr->snapshot_buffer;
#endif
if (cpu == RING_BUFFER_ALL_CPUS)
@@ -4363,14 +4363,14 @@ int tracing_set_cpumask(struct trace_array *tr,
!cpumask_test_cpu(cpu, tracing_cpumask_new)) {
ring_buffer_record_disable_cpu(tr->array_buffer.buffer, cpu);
#ifdef CONFIG_TRACER_SNAPSHOT
- ring_buffer_record_disable_cpu(tr->max_buffer.buffer, cpu);
+ ring_buffer_record_disable_cpu(tr->snapshot_buffer.buffer, cpu);
#endif
}
if (!cpumask_test_cpu(cpu, tr->tracing_cpumask) &&
cpumask_test_cpu(cpu, tracing_cpumask_new)) {
ring_buffer_record_enable_cpu(tr->array_buffer.buffer, cpu);
#ifdef CONFIG_TRACER_SNAPSHOT
- ring_buffer_record_enable_cpu(tr->max_buffer.buffer, cpu);
+ ring_buffer_record_enable_cpu(tr->snapshot_buffer.buffer, cpu);
#endif
}
}
@@ -4580,7 +4580,7 @@ int set_tracer_flag(struct trace_array *tr, u64 mask, int enabled)
case TRACE_ITER(OVERWRITE):
ring_buffer_change_overwrite(tr->array_buffer.buffer, enabled);
#ifdef CONFIG_TRACER_SNAPSHOT
- ring_buffer_change_overwrite(tr->max_buffer.buffer, enabled);
+ ring_buffer_change_overwrite(tr->snapshot_buffer.buffer, enabled);
#endif
break;
@@ -5298,7 +5298,7 @@ static int __tracing_resize_ring_buffer(struct trace_array *tr,
if (!tr->allocated_snapshot)
goto out;
- ret = ring_buffer_resize(tr->max_buffer.buffer, size, cpu);
+ ret = ring_buffer_resize(tr->snapshot_buffer.buffer, size, cpu);
if (ret < 0) {
int r = resize_buffer_duplicate_size(&tr->array_buffer,
&tr->array_buffer, cpu);
@@ -5323,7 +5323,7 @@ static int __tracing_resize_ring_buffer(struct trace_array *tr,
goto out_start;
}
- update_buffer_entries(&tr->max_buffer, cpu);
+ update_buffer_entries(&tr->snapshot_buffer, cpu);
out:
#endif /* CONFIG_TRACER_SNAPSHOT */
@@ -7040,9 +7040,9 @@ int tracing_set_clock(struct trace_array *tr, const char *clockstr)
tracing_reset_online_cpus(&tr->array_buffer);
#ifdef CONFIG_TRACER_SNAPSHOT
- if (tr->max_buffer.buffer)
- ring_buffer_set_clock(tr->max_buffer.buffer, trace_clocks[i].func);
- tracing_reset_online_cpus(&tr->max_buffer);
+ if (tr->snapshot_buffer.buffer)
+ ring_buffer_set_clock(tr->snapshot_buffer.buffer, trace_clocks[i].func);
+ tracing_reset_online_cpus(&tr->snapshot_buffer);
#endif
if (tr->scratch && !(tr->flags & TRACE_ARRAY_FL_LAST_BOOT)) {
@@ -7174,7 +7174,7 @@ static int tracing_snapshot_open(struct inode *inode, struct file *file)
ret = 0;
iter->tr = tr;
- iter->array_buffer = &tr->max_buffer;
+ iter->array_buffer = &tr->snapshot_buffer;
iter->cpu_file = tracing_get_cpu(inode);
m->private = iter;
file->private_data = m;
@@ -7237,7 +7237,7 @@ tracing_snapshot_write(struct file *filp, const char __user *ubuf, size_t cnt,
return -EINVAL;
#endif
if (tr->allocated_snapshot)
- ret = resize_buffer_duplicate_size(&tr->max_buffer,
+ ret = resize_buffer_duplicate_size(&tr->snapshot_buffer,
&tr->array_buffer, iter->cpu_file);
ret = tracing_arm_snapshot_locked(tr);
@@ -7258,9 +7258,9 @@ tracing_snapshot_write(struct file *filp, const char __user *ubuf, size_t cnt,
default:
if (tr->allocated_snapshot) {
if (iter->cpu_file == RING_BUFFER_ALL_CPUS)
- tracing_reset_online_cpus(&tr->max_buffer);
+ tracing_reset_online_cpus(&tr->snapshot_buffer);
else
- tracing_reset_cpu(&tr->max_buffer, iter->cpu_file);
+ tracing_reset_cpu(&tr->snapshot_buffer, iter->cpu_file);
}
break;
}
@@ -7316,7 +7316,7 @@ static int snapshot_raw_open(struct inode *inode, struct file *filp)
}
info->iter.snapshot = true;
- info->iter.array_buffer = &info->iter.tr->max_buffer;
+ info->iter.array_buffer = &info->iter.tr->snapshot_buffer;
return ret;
}
@@ -9199,7 +9199,7 @@ buffer_subbuf_size_write(struct file *filp, const char __user *ubuf,
if (!tr->allocated_snapshot)
goto out_max;
- ret = ring_buffer_subbuf_order_set(tr->max_buffer.buffer, order);
+ ret = ring_buffer_subbuf_order_set(tr->snapshot_buffer.buffer, order);
if (ret) {
/* Put back the old order */
cnt = ring_buffer_subbuf_order_set(tr->array_buffer.buffer, old_order);
@@ -9420,7 +9420,7 @@ static int allocate_trace_buffers(struct trace_array *tr, int size)
if (tr->range_addr_start)
return 0;
- ret = allocate_trace_buffer(tr, &tr->max_buffer,
+ ret = allocate_trace_buffer(tr, &tr->snapshot_buffer,
allocate_snapshot ? size : 1);
if (MEM_FAIL(ret, "Failed to allocate trace buffer\n")) {
free_trace_buffer(&tr->array_buffer);
@@ -9443,7 +9443,7 @@ static void free_trace_buffers(struct trace_array *tr)
kfree(tr->module_delta);
#ifdef CONFIG_TRACER_SNAPSHOT
- free_trace_buffer(&tr->max_buffer);
+ free_trace_buffer(&tr->snapshot_buffer);
#endif
}
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index e6f4e10060be..374b7c392f0a 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -331,17 +331,18 @@ struct trace_array {
struct array_buffer array_buffer;
#ifdef CONFIG_TRACER_SNAPSHOT
/*
- * The max_buffer is used to snapshot the trace when a maximum
+ * The snapshot_buffer is used to snapshot the trace when a maximum
* latency is reached, or when the user initiates a snapshot.
* Some tracers will use this to store a maximum trace while
* it continues examining live traces.
*
- * The buffers for the max_buffer are set up the same as the array_buffer
- * When a snapshot is taken, the buffer of the max_buffer is swapped
- * with the buffer of the array_buffer and the buffers are reset for
- * the array_buffer so the tracing can continue.
+ * The buffers for the snapshot_buffer are set up the same as the
+ * array_buffer. When a snapshot is taken, the buffer of the
+ * snapshot_buffer is swapped with the buffer of the array_buffer
+ * and the buffers are reset for the array_buffer so the tracing can
+ * continue.
*/
- struct array_buffer max_buffer;
+ struct array_buffer snapshot_buffer;
bool allocated_snapshot;
spinlock_t snapshot_trigger_lock;
unsigned int snapshot;
diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
index d88c44f1dfa5..be53fe6fee6a 100644
--- a/kernel/trace/trace_selftest.c
+++ b/kernel/trace/trace_selftest.c
@@ -1225,7 +1225,7 @@ trace_selftest_startup_irqsoff(struct tracer *trace, struct trace_array *tr)
/* check both trace buffers */
ret = trace_test_buffer(&tr->array_buffer, NULL);
if (!ret)
- ret = trace_test_buffer(&tr->max_buffer, &count);
+ ret = trace_test_buffer(&tr->snapshot_buffer, &count);
trace->reset(tr);
tracing_start();
@@ -1287,7 +1287,7 @@ trace_selftest_startup_preemptoff(struct tracer *trace, struct trace_array *tr)
/* check both trace buffers */
ret = trace_test_buffer(&tr->array_buffer, NULL);
if (!ret)
- ret = trace_test_buffer(&tr->max_buffer, &count);
+ ret = trace_test_buffer(&tr->snapshot_buffer, &count);
trace->reset(tr);
tracing_start();
@@ -1355,7 +1355,7 @@ trace_selftest_startup_preemptirqsoff(struct tracer *trace, struct trace_array *
if (ret)
goto out;
- ret = trace_test_buffer(&tr->max_buffer, &count);
+ ret = trace_test_buffer(&tr->snapshot_buffer, &count);
if (ret)
goto out;
@@ -1385,7 +1385,7 @@ trace_selftest_startup_preemptirqsoff(struct tracer *trace, struct trace_array *
if (ret)
goto out;
- ret = trace_test_buffer(&tr->max_buffer, &count);
+ ret = trace_test_buffer(&tr->snapshot_buffer, &count);
if (!ret && !count) {
printk(KERN_CONT ".. no entries found ..");
@@ -1513,7 +1513,7 @@ trace_selftest_startup_wakeup(struct tracer *trace, struct trace_array *tr)
/* check both trace buffers */
ret = trace_test_buffer(&tr->array_buffer, NULL);
if (!ret)
- ret = trace_test_buffer(&tr->max_buffer, &count);
+ ret = trace_test_buffer(&tr->snapshot_buffer, &count);
trace->reset(tr);
--
2.51.0
^ permalink raw reply related
* [PATCH 1/2] tracing: Better separate SNAPSHOT and MAX_TRACE options
From: Steven Rostedt @ 2026-02-06 16:35 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton
In-Reply-To: <20260206163554.153428314@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
The latency tracers (scheduler, irqsoff, etc) were created when tracing
was first added. These tracers required a "snapshot" buffer that was the
same size as the ring buffer being written to. When a new max latency was
hit, the main ring buffer would swap with the snapshot buffer so that the
trace leading up to the latency would be saved in the snapshot buffer (The
snapshot buffer is never written to directly and the data within it can be
viewed without fear of being overwritten).
Later, a new feature was added to allow snapshots to be taken by user
space or even event triggers. This created a "snapshot" file that allowed
users to trigger a snapshot from user space to save the current trace.
The config for this new feature (CONFIG_TRACER_SNAPSHOT) would select the
latency tracer config (CONFIG_TRACER_MAX_LATENCY) as it would need all the
functionality from it as it already existed. But this was incorrect. As
the snapshot feature is really what the latency tracers need and not the
other way around.
Have CONFIG_TRACER_MAX_TRACE select CONFIG_TRACER_SNAPSHOT where the
tracers that needs the max latency buffer selects the TRACE_MAX_TRACE
which will then select TRACER_SNAPSHOT.
Also, go through trace.c and trace.h and make the code that only needs the
TRACER_MAX_TRACE protected by that and the code that always requires the
snapshot to be protected by TRACER_SNAPSHOT.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/Kconfig | 8 +++----
kernel/trace/trace.c | 50 ++++++++++++++++++++++----------------------
kernel/trace/trace.h | 11 ++++++----
3 files changed, 36 insertions(+), 33 deletions(-)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index bfa2ec46e075..bedb2f982823 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -133,6 +133,7 @@ config BUILDTIME_MCOUNT_SORT
config TRACER_MAX_TRACE
bool
+ select TRACER_SNAPSHOT
config TRACE_CLOCK
bool
@@ -422,7 +423,6 @@ config IRQSOFF_TRACER
select GENERIC_TRACER
select TRACER_MAX_TRACE
select RING_BUFFER_ALLOW_SWAP
- select TRACER_SNAPSHOT
select TRACER_SNAPSHOT_PER_CPU_SWAP
help
This option measures the time spent in irqs-off critical
@@ -445,7 +445,6 @@ config PREEMPT_TRACER
select GENERIC_TRACER
select TRACER_MAX_TRACE
select RING_BUFFER_ALLOW_SWAP
- select TRACER_SNAPSHOT
select TRACER_SNAPSHOT_PER_CPU_SWAP
select TRACE_PREEMPT_TOGGLE
help
@@ -467,7 +466,6 @@ config SCHED_TRACER
select GENERIC_TRACER
select CONTEXT_SWITCH_TRACER
select TRACER_MAX_TRACE
- select TRACER_SNAPSHOT
help
This tracer tracks the latency of the highest priority task
to be scheduled in, starting from the point it has woken up.
@@ -617,7 +615,6 @@ config TRACE_SYSCALL_BUF_SIZE_DEFAULT
config TRACER_SNAPSHOT
bool "Create a snapshot trace buffer"
- select TRACER_MAX_TRACE
help
Allow tracing users to take snapshot of the current buffer using the
ftrace interface, e.g.:
@@ -625,6 +622,9 @@ config TRACER_SNAPSHOT
echo 1 > /sys/kernel/tracing/snapshot
cat snapshot
+ Note, the latency tracers select this option. To disable it,
+ all the latency tracers need to be disabled.
+
config TRACER_SNAPSHOT_PER_CPU_SWAP
bool "Allow snapshot to swap per CPU"
depends on TRACER_SNAPSHOT
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index abf28f92b678..54851ffbabeb 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1720,7 +1720,7 @@ static int wait_on_pipe(struct trace_iterator *iter, int full)
ret = ring_buffer_wait(iter->array_buffer->buffer, iter->cpu_file, full,
wait_pipe_cond, &pwait);
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
/*
* Make sure this is still the snapshot buffer, as if a snapshot were
* to happen, this would now be the main buffer.
@@ -2063,7 +2063,7 @@ void tracing_reset_all_online_cpus_unlocked(void)
continue;
tr->clear_trace = false;
tracing_reset_online_cpus(&tr->array_buffer);
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
tracing_reset_online_cpus(&tr->max_buffer);
#endif
}
@@ -2103,7 +2103,7 @@ static void tracing_start_tr(struct trace_array *tr)
if (buffer)
ring_buffer_record_enable(buffer);
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
buffer = tr->max_buffer.buffer;
if (buffer)
ring_buffer_record_enable(buffer);
@@ -2139,7 +2139,7 @@ static void tracing_stop_tr(struct trace_array *tr)
if (buffer)
ring_buffer_record_disable(buffer);
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
buffer = tr->max_buffer.buffer;
if (buffer)
ring_buffer_record_disable(buffer);
@@ -3766,7 +3766,7 @@ static void test_ftrace_alive(struct seq_file *m)
"# MAY BE MISSING FUNCTION EVENTS\n");
}
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
static void show_snapshot_main_help(struct seq_file *m)
{
seq_puts(m, "# echo 0 > snapshot : Clears and frees snapshot buffer\n"
@@ -3944,7 +3944,7 @@ __tracing_open(struct inode *inode, struct file *file, bool snapshot)
iter->tr = tr;
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
/* Currently only the top directory has a snapshot */
if (tr->current_trace->print_max || snapshot)
iter->array_buffer = &tr->max_buffer;
@@ -4181,7 +4181,7 @@ static int tracing_open(struct inode *inode, struct file *file)
static bool
trace_ok_for_array(struct tracer *t, struct trace_array *tr)
{
-#ifdef CONFIG_TRACER_SNAPSHOT
+#ifdef CONFIG_TRACER_MAX_TRACE
/* arrays with mapped buffer range do not have snapshots */
if (tr->range_addr_start && t->use_max_tr)
return false;
@@ -4362,14 +4362,14 @@ int tracing_set_cpumask(struct trace_array *tr,
if (cpumask_test_cpu(cpu, tr->tracing_cpumask) &&
!cpumask_test_cpu(cpu, tracing_cpumask_new)) {
ring_buffer_record_disable_cpu(tr->array_buffer.buffer, cpu);
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
ring_buffer_record_disable_cpu(tr->max_buffer.buffer, cpu);
#endif
}
if (!cpumask_test_cpu(cpu, tr->tracing_cpumask) &&
cpumask_test_cpu(cpu, tracing_cpumask_new)) {
ring_buffer_record_enable_cpu(tr->array_buffer.buffer, cpu);
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
ring_buffer_record_enable_cpu(tr->max_buffer.buffer, cpu);
#endif
}
@@ -4579,7 +4579,7 @@ int set_tracer_flag(struct trace_array *tr, u64 mask, int enabled)
case TRACE_ITER(OVERWRITE):
ring_buffer_change_overwrite(tr->array_buffer.buffer, enabled);
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
ring_buffer_change_overwrite(tr->max_buffer.buffer, enabled);
#endif
break;
@@ -5243,7 +5243,7 @@ static void update_buffer_entries(struct array_buffer *buf, int cpu)
}
}
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
/* resize @tr's buffer to the size of @size_tr's entries */
static int resize_buffer_duplicate_size(struct array_buffer *trace_buf,
struct array_buffer *size_buf, int cpu_id)
@@ -5269,7 +5269,7 @@ static int resize_buffer_duplicate_size(struct array_buffer *trace_buf,
return ret;
}
-#endif /* CONFIG_TRACER_MAX_TRACE */
+#endif /* CONFIG_TRACER_SNAPSHOT */
static int __tracing_resize_ring_buffer(struct trace_array *tr,
unsigned long size, int cpu)
@@ -5294,7 +5294,7 @@ static int __tracing_resize_ring_buffer(struct trace_array *tr,
if (ret < 0)
goto out_start;
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
if (!tr->allocated_snapshot)
goto out;
@@ -5326,7 +5326,7 @@ static int __tracing_resize_ring_buffer(struct trace_array *tr,
update_buffer_entries(&tr->max_buffer, cpu);
out:
-#endif /* CONFIG_TRACER_MAX_TRACE */
+#endif /* CONFIG_TRACER_SNAPSHOT */
update_buffer_entries(&tr->array_buffer, cpu);
out_start:
@@ -5583,7 +5583,7 @@ int tracing_set_tracer(struct trace_array *tr, const char *buf)
if (trace == tr->current_trace)
return 0;
-#ifdef CONFIG_TRACER_SNAPSHOT
+#ifdef CONFIG_TRACER_MAX_TRACE
if (trace->use_max_tr) {
local_irq_disable();
arch_spin_lock(&tr->max_lock);
@@ -7039,7 +7039,7 @@ int tracing_set_clock(struct trace_array *tr, const char *clockstr)
*/
tracing_reset_online_cpus(&tr->array_buffer);
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
if (tr->max_buffer.buffer)
ring_buffer_set_clock(tr->max_buffer.buffer, trace_clocks[i].func);
tracing_reset_online_cpus(&tr->max_buffer);
@@ -8190,7 +8190,7 @@ static long tracing_buffers_ioctl(struct file *file, unsigned int cmd, unsigned
return 0;
}
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
static int get_snapshot_map(struct trace_array *tr)
{
int err = 0;
@@ -9194,7 +9194,7 @@ buffer_subbuf_size_write(struct file *filp, const char __user *ubuf,
if (ret)
goto out;
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
if (!tr->allocated_snapshot)
goto out_max;
@@ -9415,7 +9415,7 @@ static int allocate_trace_buffers(struct trace_array *tr, int size)
if (ret)
return ret;
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
/* Fix mapped buffer trace arrays do not have snapshot buffers */
if (tr->range_addr_start)
return 0;
@@ -9442,7 +9442,7 @@ static void free_trace_buffers(struct trace_array *tr)
free_trace_buffer(&tr->array_buffer);
kfree(tr->module_delta);
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
free_trace_buffer(&tr->max_buffer);
#endif
}
@@ -9584,7 +9584,7 @@ trace_array_create_systems(const char *name, const char *systems,
tr->syscall_buf_sz = global_trace.syscall_buf_sz;
tr->max_lock = (arch_spinlock_t)__ARCH_SPIN_LOCK_UNLOCKED;
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
spin_lock_init(&tr->snapshot_trigger_lock);
#endif
tr->current_trace = &nop_trace;
@@ -10540,7 +10540,7 @@ ssize_t trace_parse_run_command(struct file *file, const char __user *buffer,
return done;
}
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
__init static bool tr_needs_alloc_snapshot(const char *name)
{
char *test;
@@ -10730,7 +10730,7 @@ __init static void enable_instances(void)
}
} else {
/* Only non mapped buffers have snapshot buffers */
- if (IS_ENABLED(CONFIG_TRACER_MAX_TRACE))
+ if (IS_ENABLED(CONFIG_TRACER_SNAPSHOT))
do_allocate_snapshot(name);
}
@@ -10857,7 +10857,7 @@ __init static int tracer_alloc_buffers(void)
global_trace.current_trace_flags = nop_trace.flags;
global_trace.max_lock = (arch_spinlock_t)__ARCH_SPIN_LOCK_UNLOCKED;
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
spin_lock_init(&global_trace.snapshot_trigger_lock);
#endif
ftrace_init_global_array_ops(&global_trace);
@@ -10925,7 +10925,7 @@ struct trace_array *trace_get_global_array(void)
void __init ftrace_boot_snapshot(void)
{
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
struct trace_array *tr;
if (!snapshot_at_boot)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 6bd5f4a87909..e6f4e10060be 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -329,7 +329,7 @@ struct trace_array {
struct list_head list;
char *name;
struct array_buffer array_buffer;
-#ifdef CONFIG_TRACER_MAX_TRACE
+#ifdef CONFIG_TRACER_SNAPSHOT
/*
* The max_buffer is used to snapshot the trace when a maximum
* latency is reached, or when the user initiates a snapshot.
@@ -345,13 +345,16 @@ struct trace_array {
bool allocated_snapshot;
spinlock_t snapshot_trigger_lock;
unsigned int snapshot;
+#ifdef CONFIG_TRACER_MAX_TRACE
unsigned long max_latency;
#ifdef CONFIG_FSNOTIFY
struct dentry *d_max_latency;
struct work_struct fsnotify_work;
struct irq_work fsnotify_irqwork;
-#endif
-#endif
+#endif /* CONFIG_FSNOTIFY */
+#endif /* CONFIG_TRACER_MAX_TRACE */
+#endif /* CONFIG_TRACER_SNAPSHOT */
+
/* The below is for memory mapped ring buffer */
unsigned int mapped;
unsigned long range_addr_start;
@@ -377,7 +380,7 @@ struct trace_array {
*
* It is also used in other places outside the update_max_tr
* so it needs to be defined outside of the
- * CONFIG_TRACER_MAX_TRACE.
+ * CONFIG_TRACER_SNAPSHOT.
*/
arch_spinlock_t max_lock;
#ifdef CONFIG_FTRACE_SYSCALLS
--
2.51.0
^ permalink raw reply related
* [PATCH 0/2] tracing: Clean up snapshot config logic and field name
From: Steven Rostedt @ 2026-02-06 16:35 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton
The latency tracers (scheduler, irqsoff, etc) were created when tracing
was first added. These tracers required a "snapshot" buffer that was the
same size as the ring buffer being written to. When a new max latency was
hit, the main ring buffer would swap with the snapshot buffer so that the
trace leading up to the latency would be saved in the snapshot buffer (The
snapshot buffer is never written to directly and the data within it can be
viewed without fear of being overwritten).
Later, a new feature was added to allow snapshots to be taken by user
space or even event triggers. This created a "snapshot" file that allowed
users to trigger a snapshot from user space to save the current trace.
The config for this new feature (CONFIG_TRACER_SNAPSHOT) would select the
latency tracer config (CONFIG_TRACER_MAX_LATENCY) as it would need all the
functionality from it as it already existed. But this was incorrect. As
the snapshot feature is really what the latency tracers need and not the
other way around.
Have CONFIG_TRACER_MAX_TRACE select CONFIG_TRACER_SNAPSHOT where the
tracers that needs the max latency buffer selects the TRACE_MAX_TRACE
which will then select TRACER_SNAPSHOT.
Also, go through trace.c and trace.h and make the code that only needs the
TRACER_MAX_TRACE protected by that and the code that always requires the
snapshot to be protected by TRACER_SNAPSHOT.
As this snapshot buffer now has a more generic use case, calling it
"max_buffer" is confusing. Rename it to "snapshot_buffer".
Head SHA1: 5c478bf3c6e04e6699d66a98bc6cabcc8ef5391c
Steven Rostedt (2):
tracing: Better separate SNAPSHOT and MAX_TRACE options
tracing: Rename trace_array field max_buffer to snapshot_buffer
----
kernel/trace/Kconfig | 8 +--
kernel/trace/trace.c | 122 +++++++++++++++++++++---------------------
kernel/trace/trace.h | 24 +++++----
kernel/trace/trace_selftest.c | 10 ++--
4 files changed, 84 insertions(+), 80 deletions(-)
^ permalink raw reply
* Re: [PATCH 2/2] tracing: resolve enum names for function arguments via BTF
From: Alexei Starovoitov @ 2026-02-06 16:04 UTC (permalink / raw)
To: Donglin Peng
Cc: Steven Rostedt, Andrii Nakryiko, Alexei Starovoitov,
Masami Hiramatsu, LKML, Donglin Peng, linux-trace-kernel, bpf,
Eduard Zingerman
In-Reply-To: <CAErzpmsC0qHL41KoRTxQ2-Xy=t74NnOZQxrv8VMFRSt3eoFrRA@mail.gmail.com>
On Thu, Feb 5, 2026 at 8:10 PM Donglin Peng <dolinux.peng@gmail.com> wrote:
>
> On Thu, Feb 5, 2026 at 11:56 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Thu, Feb 5, 2026 at 1:21 AM Donglin Peng <dolinux.peng@gmail.com> wrote:
> > >
> > > On Wed, Feb 4, 2026 at 10:52 PM Donglin Peng <dolinux.peng@gmail.com> wrote:
> > > >
> > > > On Wed, Feb 4, 2026 at 12:00 AM Alexei Starovoitov
> > > > <alexei.starovoitov@gmail.com> wrote:
> > > > >
> > > > > On Tue, Feb 3, 2026 at 7:16 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> > > > > >
> > > > > > On Tue, 3 Feb 2026 21:50:47 +0800
> > > > > > Donglin Peng <dolinux.peng@gmail.com> wrote:
> > > > > >
> > > > > > > Testing revealed that sorting within resolve_btfids introduces issues with
> > > > > > > btf__dedup. Therefore, I plan to move the sorting logic directly into
> > > > > > > btf__add_enum_value and btf__add_enum64_value in libbpf, which are
> > > > > > > invoked by pahole. However, it means that we need a newer pahole
> > > > > > > version.
> > > > > >
> > > > > > Sorting isn't a requirement just something I wanted to bring up. If it's
> > > > > > too complex and doesn't achieve much benefit then let's not do it.
> > > > > >
> > > > > > My worry is because "cat trace" takes quite a long time just reading the
> > > > > > BTF arguments. I'm worried it will just get worse with enums as well.
> > > > > >
> > > > > > I have trace-cmd reading BTF now (just haven't officially released it) and
> > > > > > doing an extract and reading the trace.dat file is much faster than reading
> > > > > > the trace file with arguments. I'll need to implement the enum logic too in
> > > > > > libtraceevent.
> > > > >
> > > > > If you mean to do pretty printing of the trace in user space then +1 from me.
> > > > >
> > > > > I don't like sorting enums either in resolve_btfid, pahole or kernel.
> > > > > Sorted BTF by name was ok, since it doesn't change original semantics.
> > > > > While sorting enums by value gets us to the grey zone where
> > > > > the sequence of enum names in vmlinux.h becomes different than in dwarf.
> > > >
> > > > Thanks, I agreed.
> > > >
> > > > >
> > > > > Also id->name mapping in general is not precise.
> > > > > There is no requirement for enums to be unique.
> > > > > Just grabbing the first one:
> > > > > ATA_PIO0 = 1,
> > > > > ATA_PIO1 = 3,
> > > > > ATA_PIO2 = 7,
> > > > > ATA_UDMA0 = 1,
> > > > > ATA_UDMA1 = 3,
> > > > > ATA_UDMA2 = 7,
> > > > > ATA_ID_CYLS = 1,
> > > > > ATA_ID_HEADS = 3,
> > > > > SCR_ERROR = 1,
> > > > > SCR_CONTROL = 2,
> > > > > SCR_ACTIVE = 3,
> > > > >
> > > > > All these names are part of the same enum type.
> > > > > Which one to print? First one?
> > >
> > > Another option is to print all matching entries, incurring increased
> > > overhead and extended trace log length. However, I prefer printing
> > > the first matching entry, though it might be inaccurate in rare cases.
> >
> > I disagree. It's not rare.
> > I wouldn't print anything. Let user space deal with it.
>
> Okay, I will implement this in libtraceevent first. By the way, would the first
> patch [1] introducing the for_each_enumand for_each_enum64 helper
> macros be acceptable?
>
> [1] https://lore.kernel.org/lkml/20260202111548.3555306-2-dolinux.peng@gmail.com/
Just that patch alone? What's the point?
Refactor for what? Does it read better? No.
^ permalink raw reply
* [PATCH] tracing: Have all triggers expect a file parameter
From: Steven Rostedt @ 2026-02-06 15:13 UTC (permalink / raw)
To: LKML, Linux Trace Kernel; +Cc: Masami Hiramatsu, Mathieu Desnoyers, Tom Zanussi
From: Steven Rostedt <rostedt@goodmis.org>
When the triggers were first created, they may not have had a file
parameter passed to them and things needed to be done generically.
But today, all triggers have a file parameter passed to them. Remove the
generic code and add a "if (WARN_ON_ONCE(!file))" to each trigger.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace_events_trigger.c | 62 +++++++++++------------------
1 file changed, 24 insertions(+), 38 deletions(-)
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 06b75bcfc7b8..7fa26327c9c7 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -1347,18 +1347,13 @@ traceon_trigger(struct event_trigger_data *data,
{
struct trace_event_file *file = data->private_data;
- if (file) {
- if (tracer_tracing_is_on(file->tr))
- return;
-
- tracer_tracing_on(file->tr);
+ if (WARN_ON_ONCE(!file))
return;
- }
- if (tracing_is_on())
+ if (tracer_tracing_is_on(file->tr))
return;
- tracing_on();
+ tracer_tracing_on(file->tr);
}
static bool
@@ -1368,13 +1363,11 @@ traceon_count_func(struct event_trigger_data *data,
{
struct trace_event_file *file = data->private_data;
- if (file) {
- if (tracer_tracing_is_on(file->tr))
- return false;
- } else {
- if (tracing_is_on())
- return false;
- }
+ if (WARN_ON_ONCE(!file))
+ return false;
+
+ if (tracer_tracing_is_on(file->tr))
+ return false;
if (!data->count)
return false;
@@ -1392,18 +1385,13 @@ traceoff_trigger(struct event_trigger_data *data,
{
struct trace_event_file *file = data->private_data;
- if (file) {
- if (!tracer_tracing_is_on(file->tr))
- return;
-
- tracer_tracing_off(file->tr);
+ if (WARN_ON_ONCE(!file))
return;
- }
- if (!tracing_is_on())
+ if (!tracer_tracing_is_on(file->tr))
return;
- tracing_off();
+ tracer_tracing_off(file->tr);
}
static bool
@@ -1413,13 +1401,11 @@ traceoff_count_func(struct event_trigger_data *data,
{
struct trace_event_file *file = data->private_data;
- if (file) {
- if (!tracer_tracing_is_on(file->tr))
- return false;
- } else {
- if (!tracing_is_on())
- return false;
- }
+ if (WARN_ON_ONCE(!file))
+ return false;
+
+ if (!tracer_tracing_is_on(file->tr))
+ return false;
if (!data->count)
return false;
@@ -1481,10 +1467,10 @@ snapshot_trigger(struct event_trigger_data *data,
{
struct trace_event_file *file = data->private_data;
- if (file)
- tracing_snapshot_instance(file->tr);
- else
- tracing_snapshot();
+ if (WARN_ON_ONCE(!file))
+ return;
+
+ tracing_snapshot_instance(file->tr);
}
static int
@@ -1570,10 +1556,10 @@ stacktrace_trigger(struct event_trigger_data *data,
{
struct trace_event_file *file = data->private_data;
- if (file)
- __trace_stack(file->tr, tracing_gen_ctx_dec(), STACK_SKIP);
- else
- trace_dump_stack(STACK_SKIP);
+ if (WARN_ON_ONCE(!file))
+ return;
+
+ __trace_stack(file->tr, tracing_gen_ctx_dec(), STACK_SKIP);
}
static int
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v7 5/6] x86/vdso: Use CFI macros in __vdso_sgx_enter_enclave()
From: Jens Remus @ 2026-02-06 15:04 UTC (permalink / raw)
To: Josh Poimboeuf, H. Peter Anvin, linux-kernel, linux-trace-kernel,
bpf, x86, linux-mm, Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, Peter Zijlstra, Ingo Molnar,
Jiri Olsa, Arnaldo Carvalho de Melo, Namhyung Kim,
Thomas Gleixner, Andrii Nakryiko, Indu Bhagat, Jose E. Marchesi,
Beau Belgrave, Linus Torvalds, Andrew Morton, Florian Weimer,
Kees Cook, Carlos O'Donell, Sam James, Dylan Hatch,
Borislav Petkov, Dave Hansen, David Hildenbrand, Liam R. Howlett,
Lorenzo Stoakes, Michal Hocko, Mike Rapoport, Suren Baghdasaryan,
Vlastimil Babka, Heiko Carstens, Vasily Gorbik,
Steven Rostedt (Google)
In-Reply-To: <20260203171958.1522030-6-jremus@linux.ibm.com>
On 2/3/2026 6:19 PM, Jens Remus wrote:
> From: Josh Poimboeuf <jpoimboe@kernel.org>
>
> Use the CFI macros instead of the raw .cfi_* directives to be consistent
> with the rest of the VDSO asm. It's also easier on the eyes.
>
> No functional changes.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> ---
> arch/x86/entry/vdso/vdso64/vsgx.S | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/entry/vdso/vdso64/vsgx.S b/arch/x86/entry/vdso/vdso64/vsgx.S
> index c0342238c976..8d7b8eb45c50 100644
> --- a/arch/x86/entry/vdso/vdso64/vsgx.S
> +++ b/arch/x86/entry/vdso/vdso64/vsgx.S
> @@ -24,13 +24,14 @@
> .section .text, "ax"
>
> SYM_FUNC_START(__vdso_sgx_enter_enclave)
> + SYM_F_ALIGN
Somehow I missed Steven's v6 and any of it's review feedback, where Josh
noticed that this added SYM_F_ALIGN somehow creeped in and should be
dropped:
https://lore.kernel.org/all/amh7fzsjx4p5nowy3b3j6blkty5ojqf7jawtlslexwzdbxrhc3@zersgacmcnai/
> push %rbp
> - .cfi_adjust_cfa_offset 8
> - .cfi_rel_offset %rbp, 0
> + CFI_ADJUST_CFA_OFFSET 8
> + CFI_REL_OFFSET %rbp, 0
> mov %rsp, %rbp
> - .cfi_def_cfa_register %rbp
> + CFI_DEF_CFA_REGISTER %rbp
> push %rbx
> - .cfi_rel_offset %rbx, -8
> + CFI_REL_OFFSET %rbx, -8
>
> mov %ecx, %eax
> .Lenter_enclave:
> @@ -77,13 +78,11 @@ SYM_FUNC_START(__vdso_sgx_enter_enclave)
> .Lout:
> pop %rbx
> leave
> - .cfi_def_cfa %rsp, 8
> + CFI_DEF_CFA %rsp, 8
> RET
>
> - /* The out-of-line code runs with the pre-leave stack frame. */
> - .cfi_def_cfa %rbp, 16
> -
> .Linvalid_input:
> + CFI_DEF_CFA %rbp, 16
> mov $(-EINVAL), %eax
> jmp .Lout
>
Regards,
Jens
--
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com
IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
^ permalink raw reply
* Re: [PATCH v7 0/6] x86/vdso: VDSO updates and fixes for sframes
From: Jens Remus @ 2026-02-06 15:02 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel, bpf, x86, linux-mm,
Steven Rostedt, Josh Poimboeuf, H. Peter Anvin
Cc: Masami Hiramatsu, Mathieu Desnoyers, Peter Zijlstra, Ingo Molnar,
Jiri Olsa, Arnaldo Carvalho de Melo, Namhyung Kim,
Thomas Gleixner, Andrii Nakryiko, Indu Bhagat, Jose E. Marchesi,
Beau Belgrave, Linus Torvalds, Andrew Morton, Florian Weimer,
Kees Cook, Carlos O'Donell, Sam James, Dylan Hatch,
Borislav Petkov, Dave Hansen, David Hildenbrand, Liam R. Howlett,
Lorenzo Stoakes, Michal Hocko, Mike Rapoport, Suren Baghdasaryan,
Vlastimil Babka, Heiko Carstens, Vasily Gorbik
In-Reply-To: <20260203171958.1522030-1-jremus@linux.ibm.com>
On 2/3/2026 6:19 PM, Jens Remus wrote:
> This enables generation of SFrame V3 stack trace information for VDSO on
> x86-64. It's a continuation of Josh's and Steve's work:
>
> https://lore.kernel.org/all/cover.1737511963.git.jpoimboe@kernel.org/
> https://lore.kernel.org/all/20250422183439.895236512@goodmis.org/
Above is the link to Steven's v5 of the series on which I based my v6:
https://lore.kernel.org/all/20260129112726.892340-1-jremus@linux.ibm.com/
I just realized that Steven's latest version actually was v6:
https://lore.kernel.org/all/20250425023750.669174660@goodmis.org/
That is why I also messed up the series' versioning and erroneously sent
mine as v6 as well. Sorry for that!
I'll incorporate all of the missing original v6 changes and review
feedback into v8. Following is an excerpt from the range-diff between
Steven's v6 and my v6 showing the only missing change in commit subject
of patch 3:
3: 6ce4adda138b ! 3: 9b59e14424b0 x86/asm: Use CFI_* macros in SYM_FUNC_* macros so they can be added to VDSO
@@ Metadata
Author: Josh Poimboeuf <jpoimboe@kernel.org>
## Commit message ##
- x86/asm: Use CFI_* macros in SYM_FUNC_* macros so they can be added to VDSO
+ x86/asm: Simplify VDSO DWARF generation
Add CFI_STARTPROC and CFI_ENDPROC annotations to the SYM_FUNC_* macros
so the VDSO asm functions don't need to add them manually. Note this
only affects VDSO, the CFI_* macros are empty for the kernel proper.
+ [ Jens Remus: Reword commit subject and message as suggested by Josh. ]
+
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
- [ Fixed rebased issues. ]
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
+ Signed-off-by: Jens Remus <jremus@linux.ibm.com>
## arch/x86/entry/vdso/vdso-layout.lds.S ##
@@
Regards,
Jens
--
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com
IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
^ permalink raw reply
* Re: [PATCH v2] trace/hwlat: prevent false sharing in get_sample()
From: Steven Rostedt @ 2026-02-06 14:53 UTC (permalink / raw)
To: Colin Lord
Cc: linux-kernel, linux-trace-kernel, Masami Hiramatsu,
Mathieu Desnoyers
In-Reply-To: <DG7Q5K72T7AZ.2TGDSMRADSUD9@mykolab.com>
On Fri, 06 Feb 2026 00:09:49 -0800
"Colin Lord" <clord@mykolab.com> wrote:
> > tracing: Fix get_sample() in hwlat from ...
>
> Got it, I can make that adjustment and resend. Would you also like me to
> modify the commit message with any of the above clarifications?
Yes please. Mention the cache sharing issues and that you saw
missed/duplicate seqnums too.
Not everyone knows what "false sharing" is (I forgot what it was ;). You
can use the term, but also explain it in terms of cache like you did in
this email.
Thanks,
-- Steve
^ permalink raw reply
* Re: [PATCH v7 2/6] x86/asm: Avoid emitting DWARF CFI for non-VDSO
From: Jens Remus @ 2026-02-06 14:01 UTC (permalink / raw)
To: H. Peter Anvin, linux-kernel, linux-trace-kernel, bpf, x86,
linux-mm, Steven Rostedt
Cc: Josh Poimboeuf, Masami Hiramatsu, Mathieu Desnoyers,
Peter Zijlstra, Ingo Molnar, Jiri Olsa, Arnaldo Carvalho de Melo,
Namhyung Kim, Thomas Gleixner, Andrii Nakryiko, Indu Bhagat,
Jose E. Marchesi, Beau Belgrave, Linus Torvalds, Andrew Morton,
Florian Weimer, Kees Cook, Carlos O'Donell, Sam James,
Dylan Hatch, Borislav Petkov, Dave Hansen, David Hildenbrand,
Liam R. Howlett, Lorenzo Stoakes, Michal Hocko, Mike Rapoport,
Suren Baghdasaryan, Vlastimil Babka, Heiko Carstens,
Vasily Gorbik, Steven Rostedt (Google)
In-Reply-To: <bd2fdf83-e460-46ad-8617-16319bc55ca5@zytor.com>
On 2/4/2026 1:43 AM, H. Peter Anvin wrote:
> On 2026-02-03 09:19, Jens Remus wrote:
>> From: Josh Poimboeuf <jpoimboe@kernel.org>
>>
>> It was decided years ago that .cfi_* annotations aren't maintainable in
>> the kernel. They were replaced by objtool unwind hints. For the kernel
>> proper, ensure the CFI_* macros don't do anything.
>>
>> On the other hand the VDSO library *does* use them, so user space can
>> unwind through it.
>>
>> Make sure these macros only work for VDSO. They aren't actually being
>> used outside of VDSO anyway, so there's no functional change.
>>
>> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
>> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
>> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
>> ---
>> arch/x86/include/asm/dwarf2.h | 51 ++++++++++++++++++++++++-----------
>> 1 file changed, 35 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/dwarf2.h b/arch/x86/include/asm/dwarf2.h
>> index 09c9684d3ad6..7cc30500c095 100644
>> --- a/arch/x86/include/asm/dwarf2.h
>> +++ b/arch/x86/include/asm/dwarf2.h
>> @@ -6,6 +6,15 @@
>> #warning "asm/dwarf2.h should be only included in pure assembly files"
>> #endif
>>
>> +#ifdef BUILD_VDSO
>> +
>> + /*
>> + * For the vDSO, emit both runtime unwind information and debug
>> + * symbols for the .dbg file.
>> + */
>> +
>> + .cfi_sections .eh_frame, .debug_frame
>> +
>> #define CFI_STARTPROC .cfi_startproc
>> #define CFI_ENDPROC .cfi_endproc
>> #define CFI_DEF_CFA .cfi_def_cfa
>> @@ -22,21 +31,31 @@
>> #define CFI_ESCAPE .cfi_escape
>> #define CFI_SIGNAL_FRAME .cfi_signal_frame
I missed that this is new on tip:x86/entry and thus also needs to be
added below in the !BUILD_VDSO case.
>>
>> -#ifndef BUILD_VDSO
>> - /*
>> - * Emit CFI data in .debug_frame sections, not .eh_frame sections.
>> - * The latter we currently just discard since we don't do DWARF
>> - * unwinding at runtime. So only the offline DWARF information is
>> - * useful to anyone. Note we should not use this directive if we
>> - * ever decide to enable DWARF unwinding at runtime.
>> - */
>> - .cfi_sections .debug_frame
>> -#else
>> - /*
>> - * For the vDSO, emit both runtime unwind information and debug
>> - * symbols for the .dbg file.
>> - */
>> - .cfi_sections .eh_frame, .debug_frame
>> -#endif
>> +#else /* !BUILD_VDSO */
>> +
>> +/*
>> + * On x86, these macros aren't used outside VDSO. As well they shouldn't be:
>> + * they're fragile and very difficult to maintain.
>> + */
>> +
>> +.macro nocfi args:vararg
>> +.endm
>> +
>> +#define CFI_STARTPROC nocfi
>> +#define CFI_ENDPROC nocfi
>> +#define CFI_DEF_CFA nocfi
>> +#define CFI_DEF_CFA_REGISTER nocfi
>> +#define CFI_DEF_CFA_OFFSET nocfi
>> +#define CFI_ADJUST_CFA_OFFSET nocfi
>> +#define CFI_OFFSET nocfi
>> +#define CFI_REL_OFFSET nocfi
>> +#define CFI_REGISTER nocfi
>> +#define CFI_RESTORE nocfi
>> +#define CFI_REMEMBER_STATE nocfi
>> +#define CFI_RESTORE_STATE nocfi
>> +#define CFI_UNDEFINED nocfi
>> +#define CFI_ESCAPE nocfi
#define CFI_SIGNAL_FRAME nocfi
>> +
>> +#endif /* !BUILD_VDSO */
>>
>> #endif /* _ASM_X86_DWARF2_H */
>
> I guess patch 3 justifies this.
>
> Acked-by: H. Peter Anvin (Intel) <hpa@zytor.com>
>
Regards,
Jens
--
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com
IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
^ permalink raw reply
* Re: [PATCH v6 1/4] tracing: Reset last_boot_info if ring buffer is reset
From: Masami Hiramatsu @ 2026-02-06 13:50 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel
In-Reply-To: <20260204204049.0f7dc697@robin>
On Wed, 4 Feb 2026 20:40:49 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Sun, 1 Feb 2026 12:29:07 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
>
> > @@ -8036,6 +8042,7 @@ tracing_snapshot_write(struct file *filp, const char __user *ubuf, size_t cnt,
> > tracing_reset_online_cpus(&tr->max_buffer);
> > else
> > tracing_reset_cpu(&tr->max_buffer, iter->cpu_file);
> > + update_last_data_if_empty(tr);
>
> Is this needed? Memory mapped buffers (which the persistent ring buffer
> is) do not have snapshot buffers.
Yeah, I did this just for consistency. But it is better to just leave
the comment here.
Thank you,
>
> -- Steve
>
>
> > }
> > break;
> > }
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* [bug report] tracing: Properly process error handling in event_hist_trigger_parse()
From: Dan Carpenter @ 2026-02-06 13:40 UTC (permalink / raw)
To: Miaoqian Lin; +Cc: Mathieu Desnoyers, linux-trace-kernel, linux-kernel
In-Reply-To: <caa37f28-a2e8-4e0a-a9ce-a365ce805e4b@stanley.mountain>
[ Smatch checking is paused while we raise funding. #SadFace
https://lore.kernel.org/all/aTaiGSbWZ9DJaGo7@stanley.mountain/ -dan ]
Hello Miaoqian Lin,
Commit 0550069cc25f ("tracing: Properly process error handling in
event_hist_trigger_parse()") from Dec 11, 2025 (linux-next), leads to
the following Smatch static checker warning:
kernel/trace/trace_events_hist.c:6925 event_hist_trigger_parse()
error: we previously assumed 'trigger_data' could be null (see line 6856)
kernel/trace/trace_events_hist.c
6855 trigger_data = trigger_data_alloc(cmd_ops, cmd, param, hist_data);
6856 if (!trigger_data) {
6857 ret = -ENOMEM;
6858 goto out_free;
trigger_data is NULL here.
6859 }
6860
6861 ret = event_trigger_set_filter(cmd_ops, file, filter, trigger_data);
6862 if (ret < 0)
6863 goto out_free;
6864
6865 if (remove) {
6866 if (!have_hist_trigger_match(trigger_data, file))
6867 goto out_free;
6868
6869 if (hist_trigger_check_refs(trigger_data, file)) {
6870 ret = -EBUSY;
6871 goto out_free;
6872 }
6873
6874 event_trigger_unregister(cmd_ops, file, glob+1, trigger_data);
6875 se_name = trace_event_name(file->event_call);
6876 se = find_synth_event(se_name);
6877 if (se)
6878 se->ref--;
6879 ret = 0;
6880 goto out_free;
6881 }
6882
6883 if (existing_hist_update_only(glob, trigger_data, file))
6884 goto out_free;
6885
6886 if (!get_named_trigger_data(trigger_data)) {
6887
6888 ret = create_actions(hist_data);
6889 if (ret)
6890 goto out_free;
6891
6892 if (has_hist_vars(hist_data) || hist_data->n_var_refs) {
6893 ret = save_hist_vars(hist_data);
6894 if (ret)
6895 goto out_free;
6896 }
6897
6898 ret = tracing_map_init(hist_data->map);
6899 if (ret)
6900 goto out_free;
6901 }
6902
6903 ret = event_trigger_register(cmd_ops, file, glob, trigger_data);
6904 if (ret < 0)
6905 goto out_free;
6906
6907 ret = hist_trigger_enable(trigger_data, file);
6908 if (ret)
6909 goto out_unreg;
6910
6911 se_name = trace_event_name(file->event_call);
6912 se = find_synth_event(se_name);
6913 if (se)
6914 se->ref++;
6915 out:
6916 if (ret == 0 && glob[0])
6917 hist_err_clear();
6918
6919 return ret;
6920 out_unreg:
6921 event_trigger_unregister(cmd_ops, file, glob+1, trigger_data);
6922 out_free:
6923 remove_hist_vars(hist_data);
6924
--> 6925 trigger_data_free(trigger_data);
Originally this was kfree(trigger_data) so passing a NULL pointer was
fine, but now it will crash.
6926
6927 destroy_hist_data(hist_data);
6928 goto out;
6929 }
6930
6931 static struct event_command trigger_hist_cmd = {
6932 .name = "hist",
6933 .trigger_type = ETT_EVENT_HIST,
6934 .flags = EVENT_CMD_FL_NEEDS_REC,
6935 .parse = event_hist_trigger_parse,
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] objpool: fix the overestimation of object pooling metadata size
From: wuqiang.matt @ 2026-02-06 11:26 UTC (permalink / raw)
To: zhouwenhao, Masami Hiramatsu
Cc: linux-kernel, linux-trace-kernel, Andrew Morton
On 2026/2/2 21:28, zhouwenhao wrote:
> objpool uses struct objpool_head to store metadata information, and its
> cpu_slots member points to an array of pointers that store the addresses of
> the percpu ring arrays. However, the memory size allocated during the
> initialization of cpu_slots is nr_cpu_ids * sizeof(struct objpool_slot). On
> a 64-bit machine, the size of struct objpool_slot is 16 bytes, which is
> twice the size of the actual pointer required, and the extra memory is
> never be used, resulting in a waste of memory. Therefore, the memory size
> required for cpu_slots needs to be corrected.
>
> Fixes: b4edb8d2d464 ("lib: objpool added: ring-array based lockless MPMC")
>
> Signed-off-by: zhouwenhao <zhouwenhao7600@gmail.com>
> ---
> lib/objpool.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/objpool.c b/lib/objpool.c
> index b998b720c732..d98fadf1de16 100644
> --- a/lib/objpool.c
> +++ b/lib/objpool.c
> @@ -142,7 +142,7 @@ int objpool_init(struct objpool_head *pool, int nr_objs, int object_size,
> pool->gfp = gfp & ~__GFP_ZERO;
> pool->context = context;
> pool->release = release;
> - slot_size = nr_cpu_ids * sizeof(struct objpool_slot);
> + slot_size = nr_cpu_ids * sizeof(struct objpool_slot *);
> pool->cpu_slots = kzalloc(slot_size, pool->gfp);
> if (!pool->cpu_slots)
> return -ENOMEM;
Good catch. It should be "struct objpool_slot *".
Masami, could you please review this fix ? Thank you.
Regards,
Matt Wu
^ permalink raw reply
* Re: [RFC bpf-next 08/12] libbpf: Add btf__find_by_glob_kind function
From: Jiri Olsa @ 2026-02-06 8:43 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
bpf, linux-trace-kernel, Martin KaFai Lau, Eduard Zingerman,
Song Liu, Yonghong Song, Menglong Dong, Steven Rostedt
In-Reply-To: <CAEf4BzbzrPemyBe0xtEwuxG8d-JUC+Dd+B412VBLLAui=88LEw@mail.gmail.com>
On Thu, Feb 05, 2026 at 02:45:14PM -0800, Andrii Nakryiko wrote:
> On Thu, Feb 5, 2026 at 12:57 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Wed, Feb 04, 2026 at 11:04:09AM -0800, Andrii Nakryiko wrote:
> > > On Tue, Feb 3, 2026 at 1:39 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > >
> > > > Adding btf__find_by_glob_kind function that returns array of
> > > > BTF ids that match given kind and allow/deny patterns.
> > > >
> > > > int btf__find_by_glob_kind(const struct btf *btf, __u32 kind,
> > > > const char *allow_pattern,
> > > > const char *deny_pattern,
> > > > __u32 **__ids);
> > > >
> > > > The __ids array is allocated and needs to be manually freed.
> > > >
> > > > The pattern check is done by glob_match function.
> > > >
> > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > > ---
> > > > tools/lib/bpf/btf.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > > > tools/lib/bpf/btf.h | 3 +++
> > > > 2 files changed, 44 insertions(+)
> > > >
> > > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > > > index 83fe79ffcb8f..64502b3ef38a 100644
> > > > --- a/tools/lib/bpf/btf.c
> > > > +++ b/tools/lib/bpf/btf.c
> > > > @@ -1010,6 +1010,47 @@ __s32 btf__find_by_name_kind(const struct btf *btf, const char *type_name,
> > > > return btf_find_by_name_kind(btf, 1, type_name, kind);
> > > > }
> > > >
> > > > +int btf__find_by_glob_kind(const struct btf *btf, __u32 kind,
> > > > + const char *allow_pattern, const char *deny_pattern,
> > > > + __u32 **__ids)
> > > > +{
> > > > + __u32 i, nr_types = btf__type_cnt(btf);
> > > > + int cnt = 0, alloc = 0;
> > > > + __u32 *ids = NULL;
> > > > +
> > > > + for (i = 1; i < nr_types; i++) {
> > > > + const struct btf_type *t = btf__type_by_id(btf, i);
> > > > + const char *name;
> > > > + __u32 *p;
> > > > +
> > > > + if (btf_kind(t) != kind)
> > > > + continue;
> > > > + name = btf__name_by_offset(btf, t->name_off);
> > > > + if (!name)
> > > > + continue;
> > > > +
> > > > + if (deny_pattern && glob_match(name, deny_pattern))
> > > > + continue;
> > > > + if (allow_pattern && !glob_match(name, allow_pattern))
> > > > + continue;
> > > > +
> > > > + if (cnt == alloc) {
> > > > + alloc = max(16, alloc * 3 / 2);
> > > > + p = libbpf_reallocarray(ids, alloc, sizeof(__u32));
> > > > + if (!p) {
> > > > + free(ids);
> > > > + return -ENOMEM;
> > > > + }
> > > > + ids = p;
> > > > + }
> > > > + ids[cnt] = i;
> > > > + cnt++;
> > > > + }
> > > > +
> > > > + *__ids = ids;
> > > > + return cnt;
> > > > +}
> > > > +
> > > > static bool btf_is_modifiable(const struct btf *btf)
> > > > {
> > > > return (void *)btf->hdr != btf->raw_data;
> > > > diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> > > > index b30008c267c0..d7b47bb0ba99 100644
> > > > --- a/tools/lib/bpf/btf.h
> > > > +++ b/tools/lib/bpf/btf.h
> > > > @@ -661,6 +661,9 @@ static inline struct btf_decl_tag *btf_decl_tag(const struct btf_type *t)
> > > > return (struct btf_decl_tag *)(t + 1);
> > > > }
> > > >
> > > > +int btf__find_by_glob_kind(const struct btf *btf, __u32 kind,
> > > > + const char *allow_pattern, const char *deny_pattern,
> > > > + __u32 **__ids);
> > >
> > >
> > > as AI pointed out, this should be an internal helper, no? Let's also
> > > not use double underscore pattern here,
> > > "collect_btf_ids_by_glob_kind()" perhaps?
> >
> > ok
> >
> > >
> > > Also, you don't seem to be using deny_pattern, where you planning to?
> >
> > the tests are just rudimentary before we agree we want to do it this way
> >
> > but I'm not sure I have a usecase for deny_pattern.. I think we added it
> > just to be complete, I recall we copied that function from somewhere,
> > it's long time ago ;-)
> >
> > >
> > > Also, are there functions that we'll have BTF for, but they won't be
> > > attachable? What if I do SEC("fentry.multi/*")? Will it attach or fail
> > > to attach some functions (and thus fail the overall attachment)?
> >
> > yes, for the benchmark tests I had to add is_allowed_func which mimics
> > btf_distill_func_proto and denies attach for some functions
> >
> > also I had to filter out some core kernel functions like rcu*,trace*,..
> > which seemed to cause trouble when you attach them
>
> So the question I'm implying here is if libbpf should do what we do
> for kprobes: use libbpf_available_kprobes_parse and intersect?
right, I think it's good idea.. and in addition (just for patterns) we would
filter out functions that:
- won't attach (is_allowed_func == false)
- might cause problems (rcu*,trace*), maybe for that we could have
opts config bool
jirka
^ permalink raw reply
* [PATCH] libtraceevent: Pretty print function parameters of enum-type
From: Donglin Peng @ 2026-02-06 8:40 UTC (permalink / raw)
To: rostedt; +Cc: mhiramat, linux-trace-kernel, Donglin Peng
From: Donglin Peng <pengdonglin@xiaomi.com>
The tep_btf_print_args() function currently prints values of
enum parameters in decimal format, which reduces readability.
Utilize the BTF information to resolve enum type parameters
and print their symbolic names where possible. This enhances
the trace log readability by displaying meaningful identifiers
instead of raw numbers.
Before:
mod_memcg_lruvec_state(lruvec=0xffff88800c18a540, idx=5, val=320)
After:
mod_memcg_lruvec_state(lruvec=0xffff88800c18a540, idx=5 [NR_SLAB_RECLAIMABLE_B], val=320)
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Donglin Peng <pengdonglin@xiaomi.com>
---
src/trace-btf.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/trace-btf.c b/src/trace-btf.c
index 859b0860ce2a..85f5fefb4d3b 100644
--- a/src/trace-btf.c
+++ b/src/trace-btf.c
@@ -32,6 +32,16 @@ struct tep_btf {
#define REALLOC_SIZE (1 << 10)
#define REALLOC_MASK (REALLOC_SIZE - 1)
+#define for_each_enum(i, enum_type, member) \
+ for (i = 0, member = btf_enum(enum_type); \
+ i < BTF_INFO_VLEN(enum_type->info); \
+ i++, member++)
+
+static inline struct btf_enum *btf_enum(const struct btf_type *t)
+{
+ return (struct btf_enum *)(t + 1);
+}
+
static const char *btf_name(struct tep_btf *btf, int off)
{
if (off < btf->hdr->str_len)
@@ -546,10 +556,11 @@ int tep_btf_print_args(struct tep_handle *tep, struct trace_seq *s, void *args,
struct tep_btf *btf = tep->btf;
struct btf_type *type = tep_btf_find_func(btf, func);
struct btf_param *param;
+ struct btf_enum *enump;
unsigned long long arg;
unsigned int encode;
const char *param_name;
- int a, p, x, nr;
+ int a, p, x, nr, i;
if (!func)
return -1;
@@ -628,6 +639,13 @@ int tep_btf_print_args(struct tep_handle *tep, struct trace_seq *s, void *args,
break;
case BTF_KIND_ENUM:
trace_seq_printf(s, "%lld", arg);
+ for_each_enum(i, t, enump) {
+ if (arg == enump->val) {
+ trace_seq_printf(s, " [%s]",
+ btf_name(btf, enump->name_off));
+ break;
+ }
+ }
break;
default:
/* This does not handle complex arguments */
--
2.34.1
^ permalink raw reply related
* Re: [RFC bpf-next 04/12] bpf: Add struct bpf_tramp_node object
From: Jiri Olsa @ 2026-02-06 8:27 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
bpf, linux-trace-kernel, Martin KaFai Lau, Eduard Zingerman,
Song Liu, Yonghong Song, Menglong Dong, Steven Rostedt
In-Reply-To: <CAEf4BzY=vqpnjrDNtLVvC3_nU5=Z7iROU_He=xbkEJDuGCoqug@mail.gmail.com>
On Thu, Feb 05, 2026 at 02:27:38PM -0800, Andrii Nakryiko wrote:
> On Thu, Feb 5, 2026 at 12:57 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Wed, Feb 04, 2026 at 11:00:57AM -0800, Andrii Nakryiko wrote:
> > > On Tue, Feb 3, 2026 at 1:39 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > >
> > > > Adding struct bpf_tramp_node to decouple the link out of the trampoline
> > > > attachment info.
> > > >
> > > > At the moment the object for attaching bpf program to the trampoline is
> > > > 'struct bpf_tramp_link':
> > > >
> > > > struct bpf_tramp_link {
> > > > struct bpf_link link;
> > > > struct hlist_node tramp_hlist;
> > > > u64 cookie;
> > > > }
> > > >
> > > > The link holds the bpf_prog pointer and forces one link - one program
> > > > binding logic. In following changes we want to attach program to multiple
> > > > trampolines but have just one bpf_link object.
> > > >
> > > > Splitting struct bpf_tramp_link into:
> > > >
> > > > struct bpf_tramp_link {
> > > > struct bpf_link link;
> > > > struct bpf_tramp_node node;
> > > > };
> > > >
> > > > struct bpf_tramp_node {
> > > > struct hlist_node tramp_hlist;
> > > > struct bpf_prog *prog;
> > > > u64 cookie;
> > > > };
> > >
> > > I'm a bit confused here. For singular fentry/fexit attachment we have
> > > one trampoline and one program, right? For multi-fentry, we have
> > > multiple trampoline, but still one program pointer, no? So why put a
> > > prog pointer into tramp_node?.. You do want cookie in tramp_node, yes,
> > > but not the program.
> >
> > yes, but both links:
> > - single link 'struct bpf_tramp_link'
> > - multi link 'struct bpf_tracing_multi_link'
> >
> > are using same code to attach that code needs to have a hlist_node to
> > link the program to the trampoline and be able to reach the bpf_prog
> > (like in invoke_bpf_prog)
> >
> > current code is passing whole bpf_tramp_link object so it has access
> > to both, but multi link needs to keep link to each trampoline (nodes
> > below):
> >
> > struct bpf_tracing_multi_link {
> > struct bpf_link link;
> > enum bpf_attach_type attach_type;
> > int nodes_cnt;
> > struct bpf_tracing_multi_node nodes[] __counted_by(nodes_cnt);
> > };
> >
> > and we can't get get from &nodes[x] to bpf_tracing_multi_link.link.prog
> >
> > it's bit redundant, but not sure what else we can do
>
> invoke_bpf_prog() specifically doesn't have to get prog pointer from
> bpf_tramp_link, it can be passed prog as a separate argument and then
> bpf_tramp_node with cookie separately as well. I haven't looked at
> all other code, but I suspect we can refactor it to accept prog
> explicitly and the relevant parts (node+cookie) separately.
ok, makes sense.. will check on how to refactor that code
for some reason I thought we don't wan't to refactor jit code much,
because it means changes through all the archs code.. but this one
should mostly change just arguments, so it's probably ok
>
> Just at the conceptual level, we have single prog and multiple places
> to patch (trampolines), so we shouldn't be co-locating in the same
> data structure. It feels like a complete hack to duplicate prog just
> to make some internal code access it.
ook
jirka
^ permalink raw reply
* Re: [RFC bpf-next 00/12] bpf: tracing_multi link
From: Jiri Olsa @ 2026-02-06 8:18 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
bpf, linux-trace-kernel, Martin KaFai Lau, Eduard Zingerman,
Song Liu, Yonghong Song, Menglong Dong, Steven Rostedt
In-Reply-To: <CAADnVQLJJn=bSDvQrjNyyeNdLOZABKDjjR_3j6DG3pgbsJ+JXA@mail.gmail.com>
On Thu, Feb 05, 2026 at 07:55:19AM -0800, Alexei Starovoitov wrote:
> On Thu, Feb 5, 2026 at 12:55 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Wed, Feb 04, 2026 at 08:06:50AM -0800, Alexei Starovoitov wrote:
> > > On Wed, Feb 4, 2026 at 4:36 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > > >
> > > > On Tue, Feb 03, 2026 at 03:17:05PM -0800, Alexei Starovoitov wrote:
> > > > > On Tue, Feb 3, 2026 at 1:38 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > > > >
> > > > > > hi,
> > > > > > as an option to Meglong's change [1] I'm sending proposal for tracing_multi
> > > > > > link that does not add static trampoline but attaches program to all needed
> > > > > > trampolines.
> > > > > >
> > > > > > This approach keeps the same performance but has some drawbacks:
> > > > > >
> > > > > > - when attaching 20k functions we allocate and attach 20k trampolines
> > > > > > - during attachment we hold each trampoline mutex, so for above
> > > > > > 20k functions we will hold 20k mutexes during the attachment,
> > > > > > should be very prone to deadlock, but haven't hit it yet
> > > > >
> > > > > If you check that it's sorted and always take them in the same order
> > > > > then there will be no deadlock.
> > > > > Or just grab one global mutex first and then grab trampolines mutexes
> > > > > next in any order. The global one will serialize this attach operation.
> > > > >
> > > > > > It looks the trampoline allocations/generation might not be big a problem
> > > > > > and I'll try to find a solution for holding that many mutexes. If there's
> > > > > > no better solution I think having one read/write mutex for tracing multi
> > > > > > link attach/detach should work.
> > > > >
> > > > > If you mean to have one global mutex as I proposed above then I don't see
> > > > > a downside. It only serializes multiple libbpf calls.
> > > >
> > > > we also need to serialize it with standard single trampoline attach,
> > > > because the direct ftrace update is now done under trampoline->mutex:
> > > >
> > > > bpf_trampoline_link_prog(tr)
> > > > {
> > > > mutex_lock(&tr->mutex);
> > > > ...
> > > > update_ftrace_direct_*
> > > > ...
> > > > mutex_unlock(&tr->mutex);
> > > > }
> > > >
> > > > for tracing_multi we would link the program first (with tr->mutex)
> > > > and do the bulk ftrace update later (without tr->mutex)
> > > >
> > > > {
> > > > for each involved trampoline:
> > > > bpf_trampoline_link_prog
> > > >
> > > > --> and here we could race with some other thread doing single
> > > > trampoline attach
> > > >
> > > > update_ftrace_direct_*
> > > > }
> > > >
> > > > note the current version locks all tr->mutex instances all the way
> > > > through the update_ftrace_direct_* update
> > > >
> > > > I think we could use global rwsem and take read lock on single
> > > > trampoline attach path and write lock on tracing_multi attach,
> > > >
> > > > I thought we could take direct_mutex early, but that would mean
> > > > different order with trampoline mutex than we already have in
> > > > single attach path
> > >
> > > I feel we're talking past each other.
> > > I meant:
> > >
> > > For multi:
> > > 1. take some global mutex
> > > 2. take N tramp mutexes in any order
> > >
> > > For single:
> > > 1. take that 1 specific tramp mutex.
> >
> > ah ok, I understand, it's to prevent the lockup but keep holding all
> > the trampolines locks.. the rwsem I mentioned was for the 'fix', where
> > we do not take all the trampolines locks
>
> I don't understand how rwsem would help.
> All the operations on trampoline are protected by mutex.
> Switching to rw makes sense only if we can designate certain
> operations as "read" and others as "write" and number of "reads"
> dominate. This won't be the case with multi-fentry.
> And we still need to take all of them as "write" to update trampoline.
this applies to scenario where we do not hold all the trampoline locks,
in such case we could have race between single and multi attachment,
while single/single attachment race stays safe
as a fix the single attach would take read lock and multi attach would
take write lock, so single/single race is allowed and single/multi is
not ... showed in the patch below
but it might be too much.. in a sense that there's already many locks
involved in trampoline attach/detach, and simple global lock in multi
or just sorting the ids would be enough
jirka
---
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index b76bb545077b..edbc8f133dda 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -30,6 +30,8 @@ static struct hlist_head trampoline_ip_table[TRAMPOLINE_TABLE_SIZE];
/* serializes access to trampoline tables */
static DEFINE_MUTEX(trampoline_mutex);
+static DECLARE_RWSEM(multi_sem);
+
struct bpf_trampoline_ops {
int (*register_fentry)(struct bpf_trampoline *tr, void *new_addr, void *data);
int (*unregister_fentry)(struct bpf_trampoline *tr, u32 orig_flags, void *old_addr, void *data);
@@ -367,11 +369,7 @@ static struct bpf_trampoline *bpf_trampoline_lookup(u64 key, unsigned long ip)
head = &trampoline_ip_table[hash_64(tr->ip, TRAMPOLINE_HASH_BITS)];
hlist_add_head(&tr->hlist_ip, head);
refcount_set(&tr->refcnt, 1);
-#ifdef CONFIG_LOCKDEP
- mutex_init_with_key(&tr->mutex, &__lockdep_no_track__);
-#else
mutex_init(&tr->mutex);
-#endif
for (i = 0; i < BPF_TRAMP_MAX; i++)
INIT_HLIST_HEAD(&tr->progs_hlist[i]);
out:
@@ -871,6 +869,8 @@ int bpf_trampoline_link_prog(struct bpf_tramp_node *node,
{
int err;
+ guard(rwsem_read)(&multi_sem);
+
mutex_lock(&tr->mutex);
err = __bpf_trampoline_link_prog(node, tr, tgt_prog, &trampoline_ops, NULL);
mutex_unlock(&tr->mutex);
@@ -916,6 +916,8 @@ int bpf_trampoline_unlink_prog(struct bpf_tramp_node *node,
{
int err;
+ guard(rwsem_read)(&multi_sem);
+
mutex_lock(&tr->mutex);
err = __bpf_trampoline_unlink_prog(node, tr, tgt_prog, &trampoline_ops, NULL);
mutex_unlock(&tr->mutex);
@@ -1463,6 +1465,8 @@ int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
struct bpf_trampoline *tr;
u64 key;
+ guard(rwsem_write)(&multi_sem);
+
data.reg = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
if (!data.reg)
return -ENOMEM;
@@ -1494,12 +1498,10 @@ int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
tr = mnode->trampoline;
mutex_lock(&tr->mutex);
-
err = __bpf_trampoline_link_prog(&mnode->node, tr, NULL, &trampoline_multi_ops, &data);
- if (err) {
- mutex_unlock(&tr->mutex);
+ mutex_unlock(&tr->mutex);
+ if (err)
goto rollback_unlink;
- }
}
if (ftrace_hash_count(data.reg)) {
@@ -1516,11 +1518,6 @@ int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
}
}
- for (i = 0; i < cnt; i++) {
- tr = link->nodes[i].trampoline;
- mutex_unlock(&tr->mutex);
- }
-
free_fentry_multi_data(&data);
return 0;
@@ -1528,6 +1525,7 @@ int bpf_trampoline_multi_attach(struct bpf_prog *prog, u32 *ids,
for (j = 0; j < i; j++) {
mnode = &link->nodes[j];
tr = mnode->trampoline;
+ mutex_lock(&tr->mutex);
WARN_ON_ONCE(__bpf_trampoline_unlink_prog(&mnode->node, tr, NULL,
&trampoline_multi_ops, &data));
mutex_unlock(&tr->mutex);
@@ -1550,6 +1548,8 @@ int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_
int i, cnt = link->nodes_cnt;
struct bpf_trampoline *tr;
+ guard(rwsem_write)(&multi_sem);
+
data.unreg = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
if (!data.unreg)
return -ENOMEM;
@@ -1567,6 +1567,7 @@ int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_
mutex_lock(&tr->mutex);
WARN_ON_ONCE(__bpf_trampoline_unlink_prog(&mnode->node, tr, NULL,
&trampoline_multi_ops, &data));
+ mutex_unlock(&tr->mutex);
}
if (ftrace_hash_count(data.unreg))
@@ -1576,7 +1577,6 @@ int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_
for (i = 0; i < cnt; i++) {
tr = link->nodes[i].trampoline;
- mutex_unlock(&tr->mutex);
bpf_trampoline_put(tr);
}
^ permalink raw reply related
* Re: [PATCH v2] trace/hwlat: prevent false sharing in get_sample()
From: Colin Lord @ 2026-02-06 8:09 UTC (permalink / raw)
To: Steven Rostedt, Colin Lord
Cc: linux-kernel, linux-trace-kernel, Masami Hiramatsu,
Mathieu Desnoyers
In-Reply-To: <20260205094722.6d21d8b0@gandalf.local.home>
On Thu Feb 5, 2026 at 6:47 AM PST, Steven Rostedt wrote:
> On Sun, 1 Feb 2026 18:58:38 -0800
> Colin Lord <clord@mykolab.com> wrote:
>
> I was about to send this as part of my fixes, but taking a closer look at
> it, I have more questions.
>
>> The get_sample() function in the hwlat tracer assumes the caller holds
>> hwlat_data.lock, but this is not actually happening. The result is
>> unprotected data access to hwlat_data, and in per-cpu mode can result in
>> false sharing. The false sharing can cause false positive latency
>
> BTW, what exactly do you mean by "false sharing"?
>
>> events, since the sample_width member is involved and gets read as part
>> of the main latency detection loop.
>
> I'm trying to figure out why the change in sample_width would cause any
> issue.
>
You described it below, but yes, by false sharing I am referring to
cache latencies that occur when reading sample_width. In this case, it's
primarily due to hwlat_data.count getting modified. Since count and
sample_width are just 8B apart from each other, they'll often share a
cache line, so one thread incrementing count due to a latency event
causes all other running threads to fetch the updated cache line when
reading sample_width.
On most systems I tested, this doesn't cause high enough latency to show
up, but I guess the system mentioned in the commit message has a slow
interconnect between the two sockets. Combined with large numbers of
threads, a single latency event would cause a cascade of cache related
latencies.
Another test I did to verify this is what was happening on that system
was setting tracing_cpumask to only contain CPUs on the same socket. If
I ran 32 threads on the same socket, there were far fewer latency events
compared to running 16 on one socket + 16 on the other (same number of
total threads just different distribution).
>>
>> Convert hwlat_data.count to atomic64_t so it can be safely accessed
>> without locking, and prevent false sharing by pulling sample_width into
>> a local variable.
>
> The above still makes sense, but this only effects the seqnum, which may
> show either duplicate or skipped numbers. But shouldn't affect any of the
> other data.
>
Yes, the modifications made in this commit to hwlat_data.count do not
affect the latency. The change is related to the rest of the commit in
the sense that it is addressing the incorrect assumption that
hwlat_data.lock is held during get_sample(), but it would also make
sense to put the hwlat_data.count changes in a separate commit. I could
split it up if you'd prefer that.
For what it's worth, I did also see many cases of duplicate/skipped
sequence numbers on the machine that was affected by this latency issue,
but I wasn't sure how detailed I should make my commit message.
>>
>> One system this was tested on was a dual socket server with 32 CPUs on
>> each numa node. With settings of 1us threshold, 1000us width, and
>> 2000us window, this change reduced the number of latency events from
>> 500 per second down to approximately 1 event per minute. Some machines
>> tested did not exhibit measurable latency from the false sharing.
>
> Is this because the read of hwlat_data.sample_width is a global variable
> and could possibly be causing a cache hit latency that shows up on large
> machines?
>
> Is that what you mean by "false sharing"?
>
> Oh, and subjects for the tracing subsystem should start with a capital, and
> should be something like:
>
> tracing: Fix get_sample() in hwlat from ...
Got it, I can make that adjustment and resend. Would you also like me to
modify the commit message with any of the above clarifications?
thanks,
Colin
> -- Steve
>
>
>>
>> Signed-off-by: Colin Lord <clord@mykolab.com>
>> ---
>> Changes in v2:
>> - convert hwlat_data.count to atomic64_t
>> - leave irqs_disabled block where it originally was, outside of
>> get_sample()
>>
>> Thanks for the v1 review Steve, have updated and retested.
>>
>> cheers,
>> Colin
>>
>> kernel/trace/trace_hwlat.c | 15 +++++++--------
>> 1 file changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
>> index 2f7b94e98317..3fe274b84f1c 100644
>> --- a/kernel/trace/trace_hwlat.c
>> +++ b/kernel/trace/trace_hwlat.c
>> @@ -102,9 +102,9 @@ struct hwlat_sample {
>> /* keep the global state somewhere. */
>> static struct hwlat_data {
>>
>> - struct mutex lock; /* protect changes */
>> + struct mutex lock; /* protect changes */
>>
>> - u64 count; /* total since reset */
>> + atomic64_t count; /* total since reset */
>>
>> u64 sample_window; /* total sampling window (on+off) */
>> u64 sample_width; /* active sampling portion of window */
>> @@ -193,8 +193,7 @@ void trace_hwlat_callback(bool enter)
>> * get_sample - sample the CPU TSC and look for likely hardware latencies
>> *
>> * Used to repeatedly capture the CPU TSC (or similar), looking for potential
>> - * hardware-induced latency. Called with interrupts disabled and with
>> - * hwlat_data.lock held.
>> + * hardware-induced latency. Called with interrupts disabled.
>> */
>> static int get_sample(void)
>> {
>> @@ -204,6 +203,7 @@ static int get_sample(void)
>> time_type start, t1, t2, last_t2;
>> s64 diff, outer_diff, total, last_total = 0;
>> u64 sample = 0;
>> + u64 sample_width = READ_ONCE(hwlat_data.sample_width);
>> u64 thresh = tracing_thresh;
>> u64 outer_sample = 0;
>> int ret = -1;
>> @@ -267,7 +267,7 @@ static int get_sample(void)
>> if (diff > sample)
>> sample = diff; /* only want highest value */
>>
>> - } while (total <= hwlat_data.sample_width);
>> + } while (total <= sample_width);
>>
>> barrier(); /* finish the above in the view for NMIs */
>> trace_hwlat_callback_enabled = false;
>> @@ -285,8 +285,7 @@ static int get_sample(void)
>> if (kdata->nmi_total_ts)
>> do_div(kdata->nmi_total_ts, NSEC_PER_USEC);
>>
>> - hwlat_data.count++;
>> - s.seqnum = hwlat_data.count;
>> + s.seqnum = atomic64_inc_return(&hwlat_data.count);
>> s.duration = sample;
>> s.outer_duration = outer_sample;
>> s.nmi_total_ts = kdata->nmi_total_ts;
>> @@ -832,7 +831,7 @@ static int hwlat_tracer_init(struct trace_array *tr)
>>
>> hwlat_trace = tr;
>>
>> - hwlat_data.count = 0;
>> + atomic64_set(&hwlat_data.count, 0);
>> tr->max_latency = 0;
>> save_tracing_thresh = tracing_thresh;
>>
>>
>> base-commit: 24d479d26b25bce5faea3ddd9fa8f3a6c3129ea7
^ permalink raw reply
* Re: [PATCH 2/2] tracing: resolve enum names for function arguments via BTF
From: Donglin Peng @ 2026-02-06 4:09 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Steven Rostedt, Andrii Nakryiko, Alexei Starovoitov,
Masami Hiramatsu, LKML, Donglin Peng, linux-trace-kernel, bpf,
Eduard Zingerman
In-Reply-To: <CAADnVQJgM0W3aQVGi5_35xWc=D7Ruzt1xV0REM+RnnvtooeC5w@mail.gmail.com>
On Thu, Feb 5, 2026 at 11:56 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Thu, Feb 5, 2026 at 1:21 AM Donglin Peng <dolinux.peng@gmail.com> wrote:
> >
> > On Wed, Feb 4, 2026 at 10:52 PM Donglin Peng <dolinux.peng@gmail.com> wrote:
> > >
> > > On Wed, Feb 4, 2026 at 12:00 AM Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > > >
> > > > On Tue, Feb 3, 2026 at 7:16 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> > > > >
> > > > > On Tue, 3 Feb 2026 21:50:47 +0800
> > > > > Donglin Peng <dolinux.peng@gmail.com> wrote:
> > > > >
> > > > > > Testing revealed that sorting within resolve_btfids introduces issues with
> > > > > > btf__dedup. Therefore, I plan to move the sorting logic directly into
> > > > > > btf__add_enum_value and btf__add_enum64_value in libbpf, which are
> > > > > > invoked by pahole. However, it means that we need a newer pahole
> > > > > > version.
> > > > >
> > > > > Sorting isn't a requirement just something I wanted to bring up. If it's
> > > > > too complex and doesn't achieve much benefit then let's not do it.
> > > > >
> > > > > My worry is because "cat trace" takes quite a long time just reading the
> > > > > BTF arguments. I'm worried it will just get worse with enums as well.
> > > > >
> > > > > I have trace-cmd reading BTF now (just haven't officially released it) and
> > > > > doing an extract and reading the trace.dat file is much faster than reading
> > > > > the trace file with arguments. I'll need to implement the enum logic too in
> > > > > libtraceevent.
> > > >
> > > > If you mean to do pretty printing of the trace in user space then +1 from me.
> > > >
> > > > I don't like sorting enums either in resolve_btfid, pahole or kernel.
> > > > Sorted BTF by name was ok, since it doesn't change original semantics.
> > > > While sorting enums by value gets us to the grey zone where
> > > > the sequence of enum names in vmlinux.h becomes different than in dwarf.
> > >
> > > Thanks, I agreed.
> > >
> > > >
> > > > Also id->name mapping in general is not precise.
> > > > There is no requirement for enums to be unique.
> > > > Just grabbing the first one:
> > > > ATA_PIO0 = 1,
> > > > ATA_PIO1 = 3,
> > > > ATA_PIO2 = 7,
> > > > ATA_UDMA0 = 1,
> > > > ATA_UDMA1 = 3,
> > > > ATA_UDMA2 = 7,
> > > > ATA_ID_CYLS = 1,
> > > > ATA_ID_HEADS = 3,
> > > > SCR_ERROR = 1,
> > > > SCR_CONTROL = 2,
> > > > SCR_ACTIVE = 3,
> > > >
> > > > All these names are part of the same enum type.
> > > > Which one to print? First one?
> >
> > Another option is to print all matching entries, incurring increased
> > overhead and extended trace log length. However, I prefer printing
> > the first matching entry, though it might be inaccurate in rare cases.
>
> I disagree. It's not rare.
> I wouldn't print anything. Let user space deal with it.
Okay, I will implement this in libtraceevent first. By the way, would the first
patch [1] introducing the for_each_enumand for_each_enum64 helper
macros be acceptable?
[1] https://lore.kernel.org/lkml/20260202111548.3555306-2-dolinux.peng@gmail.com/
^ permalink raw reply
* Re: [PATCH 2/2] tracing: resolve enum names for function arguments via BTF
From: Donglin Peng @ 2026-02-06 4:04 UTC (permalink / raw)
To: Steven Rostedt
Cc: Alexei Starovoitov, Andrii Nakryiko, Alexei Starovoitov,
Masami Hiramatsu, LKML, Donglin Peng, linux-trace-kernel, bpf,
Eduard Zingerman
In-Reply-To: <20260205130459.0ff532f3@robin>
On Fri, Feb 6, 2026 at 2:05 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed, 4 Feb 2026 22:52:11 +0800
> Donglin Peng <dolinux.peng@gmail.com> wrote:
>
> > > > I have trace-cmd reading BTF now (just haven't officially released it) and
> > > > doing an extract and reading the trace.dat file is much faster than reading
> > > > the trace file with arguments. I'll need to implement the enum logic too in
> > > > libtraceevent.
> > >
> > > If you mean to do pretty printing of the trace in user space then +1 from me.
> > >
> > > I don't like sorting enums either in resolve_btfid, pahole or kernel.
> > > Sorted BTF by name was ok, since it doesn't change original semantics.
> > > While sorting enums by value gets us to the grey zone where
> > > the sequence of enum names in vmlinux.h becomes different than in dwarf.
> >
> > Thanks, I agreed.
>
> BTW, I just officially released trace-cmd v3.4 (where you can see whats
> new in that release here[1]).
>
> The biggest change is that it saves the BTF file in the trace.dat file
> and parses it on the report (it requires libtraceevent v1.9):
>
> ~# trace-cmd record -p function_graph -O funcgraph-args -g do_sys_openat2
> [..]
> ~# trace-cmd report
> trace-cmd-50935 [002] ...1. 3490.518138: funcgraph_entry: | do_sys_openat2(dfd=4294967196, filename=0x557bb9e3ee10, how=0xffff88815220fea8) {
> trace-cmd-50935 [002] ...1. 3490.518141: funcgraph_entry: | getname_flags(filename=0x557bb9e3ee10, flags=0) {
> trace-cmd-50935 [002] ...1. 3490.518142: funcgraph_entry: | getname_flags.part.0(filename=0x557bb9e3ee10, flags=0) {
> trace-cmd-50935 [002] ...1. 3490.518143: funcgraph_entry: | kmem_cache_alloc_noprof(s=0xffff888106c7e000, gfpflags=0xcc0) {
> trace-cmd-50935 [002] ...1. 3490.518145: funcgraph_entry: | stack_trace_save(store=0xffff88815220fac8, size=0x40, skipnr=0x0) {
> trace-cmd-50935 [002] ...1. 3490.518147: funcgraph_entry: | arch_stack_walk(consume_entry=0xffffffff94d9dfe0, cookie=0xffff88815220fa58, task=0xffff88812d4c3580, regs=0x0) {
> trace-cmd-50935 [002] ...1. 3490.518148: funcgraph_entry: | __unwind_start(state=0xffff88815220f988, task=0xffff88812d4c3580, regs=0x0, first_frame=0xffff88815220fa28) {
> trace-cmd-50935 [002] ...1. 3490.518149: funcgraph_entry: 1.518 us | get_stack_info(stack=0xffff88815220f938, task=0xffff88812d4c3580, info=0xffff88815220f988, visit_mask=0xffff88815220f9a8); (ret=0x0)
> trace-cmd-50935 [002] ...1. 3490.518152: funcgraph_entry: | unwind_next_frame(state=0xffff88815220f988) {
> trace-cmd-50935 [002] ...1. 3490.518153: funcgraph_entry: 0.951 us | __rcu_read_lock(); (ret=0xffff88812d4c3580)
Thanks, I will implement this feature in libtraceevent.
>
> -- Steve
>
>
>
> [1] https://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git/tag/?h=trace-cmd-v3.4
^ permalink raw reply
* Re: [PATCH 2/2] tracing: resolve enum names for function arguments via BTF
From: Masami Hiramatsu @ 2026-02-06 0:12 UTC (permalink / raw)
To: Donglin Peng
Cc: rostedt, ast, mhiramat, andrii.nakryiko, linux-kernel,
Donglin Peng, linux-trace-kernel
In-Reply-To: <20260202111548.3555306-3-dolinux.peng@gmail.com>
On Mon, 2 Feb 2026 19:15:48 +0800
Donglin Peng <dolinux.peng@gmail.com> wrote:
> From: Donglin Peng <pengdonglin@xiaomi.com>
>
> Use BTF to print symbolic names for enum-type function arguments,
> improving trace readability.
>
> Before:
> count_memcg_events(memcg=0xffff..., idx=20, count=0x1) {
>
> After:
> count_memcg_events(memcg=0xffff..., idx=20 [PGFAULT], count=0x1) {
Hmm, it is interesting idea. Maybe I can also introduce the same
feature to fetcharg for fprobe/kprobes as a new type.
Since the enum type should be solved in parsing the event definition,
for fprobe/kprobes the performance will not be a performance problem.
Thanks,
>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: linux-trace-kernel@vger.kernel.org
> Signed-off-by: Donglin Peng <pengdonglin@xiaomi.com>
> ---
> kernel/trace/trace_output.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index cc2d3306bb60..c395f768c3b8 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -695,12 +695,13 @@ void print_function_args(struct trace_seq *s, unsigned long *args,
> {
> const struct btf_param *param;
> const struct btf_type *t;
> + const struct btf_enum *enump;
> const char *param_name;
> char name[KSYM_NAME_LEN];
> unsigned long arg;
> struct btf *btf;
> s32 tid, nr = 0;
> - int a, p, x;
> + int a, p, x, i;
> u16 encode;
>
> trace_seq_printf(s, "(");
> @@ -754,6 +755,14 @@ void print_function_args(struct trace_seq *s, unsigned long *args,
> break;
> case BTF_KIND_ENUM:
> trace_seq_printf(s, "%ld", arg);
> + for_each_enum(i, t, enump) {
> + if (arg == enump->val) {
> + trace_seq_printf(s, " [%s]",
> + btf_name_by_offset(btf,
> + enump->name_off));
> + break;
> + }
> + }
> break;
> default:
> /* This does not handle complex arguments */
> --
> 2.34.1
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [RFC bpf-next 08/12] libbpf: Add btf__find_by_glob_kind function
From: Andrii Nakryiko @ 2026-02-05 22:45 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
linux-trace-kernel, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, Menglong Dong, Steven Rostedt
In-Reply-To: <aYRbbL-QCnAZ6PGD@krava>
On Thu, Feb 5, 2026 at 12:57 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Wed, Feb 04, 2026 at 11:04:09AM -0800, Andrii Nakryiko wrote:
> > On Tue, Feb 3, 2026 at 1:39 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > Adding btf__find_by_glob_kind function that returns array of
> > > BTF ids that match given kind and allow/deny patterns.
> > >
> > > int btf__find_by_glob_kind(const struct btf *btf, __u32 kind,
> > > const char *allow_pattern,
> > > const char *deny_pattern,
> > > __u32 **__ids);
> > >
> > > The __ids array is allocated and needs to be manually freed.
> > >
> > > The pattern check is done by glob_match function.
> > >
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > > tools/lib/bpf/btf.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > > tools/lib/bpf/btf.h | 3 +++
> > > 2 files changed, 44 insertions(+)
> > >
> > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > > index 83fe79ffcb8f..64502b3ef38a 100644
> > > --- a/tools/lib/bpf/btf.c
> > > +++ b/tools/lib/bpf/btf.c
> > > @@ -1010,6 +1010,47 @@ __s32 btf__find_by_name_kind(const struct btf *btf, const char *type_name,
> > > return btf_find_by_name_kind(btf, 1, type_name, kind);
> > > }
> > >
> > > +int btf__find_by_glob_kind(const struct btf *btf, __u32 kind,
> > > + const char *allow_pattern, const char *deny_pattern,
> > > + __u32 **__ids)
> > > +{
> > > + __u32 i, nr_types = btf__type_cnt(btf);
> > > + int cnt = 0, alloc = 0;
> > > + __u32 *ids = NULL;
> > > +
> > > + for (i = 1; i < nr_types; i++) {
> > > + const struct btf_type *t = btf__type_by_id(btf, i);
> > > + const char *name;
> > > + __u32 *p;
> > > +
> > > + if (btf_kind(t) != kind)
> > > + continue;
> > > + name = btf__name_by_offset(btf, t->name_off);
> > > + if (!name)
> > > + continue;
> > > +
> > > + if (deny_pattern && glob_match(name, deny_pattern))
> > > + continue;
> > > + if (allow_pattern && !glob_match(name, allow_pattern))
> > > + continue;
> > > +
> > > + if (cnt == alloc) {
> > > + alloc = max(16, alloc * 3 / 2);
> > > + p = libbpf_reallocarray(ids, alloc, sizeof(__u32));
> > > + if (!p) {
> > > + free(ids);
> > > + return -ENOMEM;
> > > + }
> > > + ids = p;
> > > + }
> > > + ids[cnt] = i;
> > > + cnt++;
> > > + }
> > > +
> > > + *__ids = ids;
> > > + return cnt;
> > > +}
> > > +
> > > static bool btf_is_modifiable(const struct btf *btf)
> > > {
> > > return (void *)btf->hdr != btf->raw_data;
> > > diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> > > index b30008c267c0..d7b47bb0ba99 100644
> > > --- a/tools/lib/bpf/btf.h
> > > +++ b/tools/lib/bpf/btf.h
> > > @@ -661,6 +661,9 @@ static inline struct btf_decl_tag *btf_decl_tag(const struct btf_type *t)
> > > return (struct btf_decl_tag *)(t + 1);
> > > }
> > >
> > > +int btf__find_by_glob_kind(const struct btf *btf, __u32 kind,
> > > + const char *allow_pattern, const char *deny_pattern,
> > > + __u32 **__ids);
> >
> >
> > as AI pointed out, this should be an internal helper, no? Let's also
> > not use double underscore pattern here,
> > "collect_btf_ids_by_glob_kind()" perhaps?
>
> ok
>
> >
> > Also, you don't seem to be using deny_pattern, where you planning to?
>
> the tests are just rudimentary before we agree we want to do it this way
>
> but I'm not sure I have a usecase for deny_pattern.. I think we added it
> just to be complete, I recall we copied that function from somewhere,
> it's long time ago ;-)
>
> >
> > Also, are there functions that we'll have BTF for, but they won't be
> > attachable? What if I do SEC("fentry.multi/*")? Will it attach or fail
> > to attach some functions (and thus fail the overall attachment)?
>
> yes, for the benchmark tests I had to add is_allowed_func which mimics
> btf_distill_func_proto and denies attach for some functions
>
> also I had to filter out some core kernel functions like rcu*,trace*,..
> which seemed to cause trouble when you attach them
So the question I'm implying here is if libbpf should do what we do
for kprobes: use libbpf_available_kprobes_parse and intersect?
>
> jirka
^ permalink raw reply
* Re: [RFC bpf-next 04/12] bpf: Add struct bpf_tramp_node object
From: Andrii Nakryiko @ 2026-02-05 22:27 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
linux-trace-kernel, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, Menglong Dong, Steven Rostedt
In-Reply-To: <aYRbYY8f_wWoiZhk@krava>
On Thu, Feb 5, 2026 at 12:57 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Wed, Feb 04, 2026 at 11:00:57AM -0800, Andrii Nakryiko wrote:
> > On Tue, Feb 3, 2026 at 1:39 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > Adding struct bpf_tramp_node to decouple the link out of the trampoline
> > > attachment info.
> > >
> > > At the moment the object for attaching bpf program to the trampoline is
> > > 'struct bpf_tramp_link':
> > >
> > > struct bpf_tramp_link {
> > > struct bpf_link link;
> > > struct hlist_node tramp_hlist;
> > > u64 cookie;
> > > }
> > >
> > > The link holds the bpf_prog pointer and forces one link - one program
> > > binding logic. In following changes we want to attach program to multiple
> > > trampolines but have just one bpf_link object.
> > >
> > > Splitting struct bpf_tramp_link into:
> > >
> > > struct bpf_tramp_link {
> > > struct bpf_link link;
> > > struct bpf_tramp_node node;
> > > };
> > >
> > > struct bpf_tramp_node {
> > > struct hlist_node tramp_hlist;
> > > struct bpf_prog *prog;
> > > u64 cookie;
> > > };
> >
> > I'm a bit confused here. For singular fentry/fexit attachment we have
> > one trampoline and one program, right? For multi-fentry, we have
> > multiple trampoline, but still one program pointer, no? So why put a
> > prog pointer into tramp_node?.. You do want cookie in tramp_node, yes,
> > but not the program.
>
> yes, but both links:
> - single link 'struct bpf_tramp_link'
> - multi link 'struct bpf_tracing_multi_link'
>
> are using same code to attach that code needs to have a hlist_node to
> link the program to the trampoline and be able to reach the bpf_prog
> (like in invoke_bpf_prog)
>
> current code is passing whole bpf_tramp_link object so it has access
> to both, but multi link needs to keep link to each trampoline (nodes
> below):
>
> struct bpf_tracing_multi_link {
> struct bpf_link link;
> enum bpf_attach_type attach_type;
> int nodes_cnt;
> struct bpf_tracing_multi_node nodes[] __counted_by(nodes_cnt);
> };
>
> and we can't get get from &nodes[x] to bpf_tracing_multi_link.link.prog
>
> it's bit redundant, but not sure what else we can do
invoke_bpf_prog() specifically doesn't have to get prog pointer from
bpf_tramp_link, it can be passed prog as a separate argument and then
bpf_tramp_node with cookie separately as well. I haven't looked at
all other code, but I suspect we can refactor it to accept prog
explicitly and the relevant parts (node+cookie) separately.
Just at the conceptual level, we have single prog and multiple places
to patch (trampolines), so we shouldn't be co-locating in the same
data structure. It feels like a complete hack to duplicate prog just
to make some internal code access it.
>
> > Because then there is also a question what is
> > bpf_link's prog pointing to?...
>
> bpf_link.prog is still keeping the prog, I don't think we can remove that
>
> jirka
>
> >
> >
> > >
> > > where 'struct bpf_tramp_link' defines standard single trampoline link,
> > > and 'struct bpf_tramp_node' is the attachment trampoline object. This
> > > will allow us to define link for multiple trampolines, like:
> > >
> > > struct bpf_tracing_multi_link {
> > > struct bpf_link link;
> > > ...
> > > int nodes_cnt;
> > > struct bpf_tracing_multi_node nodes[] __counted_by(nodes_cnt);
> > > };
> > >
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > > arch/arm64/net/bpf_jit_comp.c | 58 +++++++++----------
> > > arch/s390/net/bpf_jit_comp.c | 42 +++++++-------
> > > arch/x86/net/bpf_jit_comp.c | 54 ++++++++---------
> > > include/linux/bpf.h | 47 ++++++++-------
> > > kernel/bpf/bpf_struct_ops.c | 24 ++++----
> > > kernel/bpf/syscall.c | 25 ++++----
> > > kernel/bpf/trampoline.c | 102 ++++++++++++++++-----------------
> > > net/bpf/bpf_dummy_struct_ops.c | 11 ++--
> > > 8 files changed, 185 insertions(+), 178 deletions(-)
> > >
> >
> > [...]
^ 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