* Re: [RFC bpf-next 00/12] bpf: tracing_multi link
From: Alexei Starovoitov @ 2026-02-04 16:06 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: <aYM9YUl6xf9I2x4c@krava>
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.
^ permalink raw reply
* [PATCH v2] tracing: Fix ftrace event field alignments
From: Steven Rostedt @ 2026-02-04 16:36 UTC (permalink / raw)
To: LKML, Linux Trace Kernel
Cc: Masami Hiramatsu, Mathieu Desnoyers, Mark Rutland, jempty.liang
From: Steven Rostedt <rostedt@goodmis.org>
The fields of ftrace specific events (events used to save ftrace internal
events like function traces and trace_printk) are generated similarly to
how normal trace event fields are generated. That is, the fields are added
to a trace_events_fields array that saves the name, offset, size,
alignment and signness of the field. It is used to produce the output in
the format file in tracefs so that tooling knows how to parse the binary
data of the trace events.
The issue is that some of the ftrace event structures are packed. The
function graph exit event structures are one of them. The 64 bit calltime
and rettime fields end up 4 byte aligned, but the algorithm to show to
userspace shows them as 8 byte aligned.
The macros that create the ftrace events has one for embedded structure
fields. There's two macros for theses fields:
__field_desc() and __field_packed()
The difference of the latter macro is that it treats the field as packed.
Rename that field to __field_desc_packed() and create replace the
__field_packed() to be a normal field that is packed and have the calltime
and rettime use those.
This showed up on 32bit architectures for function graph time fields. It
had:
~# cat /sys/kernel/tracing/events/ftrace/funcgraph_exit/format
[..]
field:unsigned long func; offset:8; size:4; signed:0;
field:unsigned int depth; offset:12; size:4; signed:0;
field:unsigned int overrun; offset:16; size:4; signed:0;
field:unsigned long long calltime; offset:24; size:8; signed:0;
field:unsigned long long rettime; offset:32; size:8; signed:0;
Notice that overrun is at offset 16 with size 4, where in the structure
calltime is at offset 20 (16 + 4), but it shows the offset at 24. That's
because it used the alignment of unsigned long long when used as a
declaration and not as a member of a structure where it would be aligned
by word size (in this case 4).
By using the proper structure alignment, the format has it at the correct
offset:
~# cat /sys/kernel/tracing/events/ftrace/funcgraph_exit/format
[..]
field:unsigned long func; offset:8; size:4; signed:0;
field:unsigned int depth; offset:12; size:4; signed:0;
field:unsigned int overrun; offset:16; size:4; signed:0;
field:unsigned long long calltime; offset:20; size:8; signed:0;
field:unsigned long long rettime; offset:28; size:8; signed:0;
Cc: stable@vger.kernel.org
Fixes: 04ae87a52074e ("ftrace: Rework event_create_dir()")
Reported-by: "jempty.liang" <imntjempty@163.com>
Closes: https://lore.kernel.org/all/20260130015740.212343-1-imntjempty@163.com/
Closes: https://lore.kernel.org/all/20260202123342.2544795-1-imntjempty@163.com/
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
Changes since v1: https://patch.msgid.link/20260202113024.61d5c1fd@gandalf.local.home
- Instead of using an alignment for structures, create a new macro
called __field_desc_packed() to use for packed structure fields
and have __field_packed() be used for normal packed fields.
kernel/trace/trace.h | 7 +++++--
kernel/trace/trace_entries.h | 32 ++++++++++++++++----------------
kernel/trace/trace_export.c | 21 +++++++++++++++------
3 files changed, 36 insertions(+), 24 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index b6d42fe06115..c11edec5d8f5 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -68,14 +68,17 @@ enum trace_type {
#undef __field_fn
#define __field_fn(type, item) type item;
+#undef __field_packed
+#define __field_packed(type, item) type item;
+
#undef __field_struct
#define __field_struct(type, item) __field(type, item)
#undef __field_desc
#define __field_desc(type, container, item)
-#undef __field_packed
-#define __field_packed(type, container, item)
+#undef __field_desc_packed
+#define __field_desc_packed(type, container, item)
#undef __array
#define __array(type, item, size) type item[size];
diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h
index f6a8d29c0d76..54417468fdeb 100644
--- a/kernel/trace/trace_entries.h
+++ b/kernel/trace/trace_entries.h
@@ -79,8 +79,8 @@ FTRACE_ENTRY(funcgraph_entry, ftrace_graph_ent_entry,
F_STRUCT(
__field_struct( struct ftrace_graph_ent, graph_ent )
- __field_packed( unsigned long, graph_ent, func )
- __field_packed( unsigned long, graph_ent, depth )
+ __field_desc_packed(unsigned long, graph_ent, func )
+ __field_desc_packed(unsigned long, graph_ent, depth )
__dynamic_array(unsigned long, args )
),
@@ -96,9 +96,9 @@ FTRACE_ENTRY_PACKED(fgraph_retaddr_entry, fgraph_retaddr_ent_entry,
F_STRUCT(
__field_struct( struct fgraph_retaddr_ent, graph_rent )
- __field_packed( unsigned long, graph_rent.ent, func )
- __field_packed( unsigned long, graph_rent.ent, depth )
- __field_packed( unsigned long, graph_rent, retaddr )
+ __field_desc_packed( unsigned long, graph_rent.ent, func )
+ __field_desc_packed( unsigned long, graph_rent.ent, depth )
+ __field_desc_packed( unsigned long, graph_rent, retaddr )
__dynamic_array(unsigned long, args )
),
@@ -123,12 +123,12 @@ FTRACE_ENTRY_PACKED(funcgraph_exit, ftrace_graph_ret_entry,
F_STRUCT(
__field_struct( struct ftrace_graph_ret, ret )
- __field_packed( unsigned long, ret, func )
- __field_packed( unsigned long, ret, retval )
- __field_packed( unsigned int, ret, depth )
- __field_packed( unsigned int, ret, overrun )
- __field(unsigned long long, calltime )
- __field(unsigned long long, rettime )
+ __field_desc_packed( unsigned long, ret, func )
+ __field_desc_packed( unsigned long, ret, retval )
+ __field_desc_packed( unsigned int, ret, depth )
+ __field_desc_packed( unsigned int, ret, overrun )
+ __field_packed(unsigned long long, calltime)
+ __field_packed(unsigned long long, rettime )
),
F_printk("<-- %ps (%u) (start: %llx end: %llx) over: %u retval: %lx",
@@ -146,11 +146,11 @@ FTRACE_ENTRY_PACKED(funcgraph_exit, ftrace_graph_ret_entry,
F_STRUCT(
__field_struct( struct ftrace_graph_ret, ret )
- __field_packed( unsigned long, ret, func )
- __field_packed( unsigned int, ret, depth )
- __field_packed( unsigned int, ret, overrun )
- __field(unsigned long long, calltime )
- __field(unsigned long long, rettime )
+ __field_desc_packed( unsigned long, ret, func )
+ __field_desc_packed( unsigned int, ret, depth )
+ __field_desc_packed( unsigned int, ret, overrun )
+ __field_packed(unsigned long long, calltime )
+ __field_packed(unsigned long long, rettime )
),
F_printk("<-- %ps (%u) (start: %llx end: %llx) over: %u",
diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index 1698fc22afa0..32a42ef31855 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -42,11 +42,14 @@ static int ftrace_event_register(struct trace_event_call *call,
#undef __field_fn
#define __field_fn(type, item) type item;
+#undef __field_packed
+#define __field_packed(type, item) type item;
+
#undef __field_desc
#define __field_desc(type, container, item) type item;
-#undef __field_packed
-#define __field_packed(type, container, item) type item;
+#undef __field_desc_packed
+#define __field_desc_packed(type, container, item) type item;
#undef __array
#define __array(type, item, size) type item[size];
@@ -104,11 +107,14 @@ static void __always_unused ____ftrace_check_##name(void) \
#undef __field_fn
#define __field_fn(_type, _item) __field_ext(_type, _item, FILTER_TRACE_FN)
+#undef __field_packed
+#define __field_packed(_type, _item) __field_ext_packed(_type, _item, FILTER_OTHER)
+
#undef __field_desc
#define __field_desc(_type, _container, _item) __field_ext(_type, _item, FILTER_OTHER)
-#undef __field_packed
-#define __field_packed(_type, _container, _item) __field_ext_packed(_type, _item, FILTER_OTHER)
+#undef __field_desc_packed
+#define __field_desc_packed(_type, _container, _item) __field_ext_packed(_type, _item, FILTER_OTHER)
#undef __array
#define __array(_type, _item, _len) { \
@@ -146,11 +152,14 @@ static struct trace_event_fields ftrace_event_fields_##name[] = { \
#undef __field_fn
#define __field_fn(type, item)
+#undef __field_packed
+#define __field_packed(type, item)
+
#undef __field_desc
#define __field_desc(type, container, item)
-#undef __field_packed
-#define __field_packed(type, container, item)
+#undef __field_desc_packed
+#define __field_desc_packed(type, container, item)
#undef __array
#define __array(type, item, len)
--
2.51.0
^ permalink raw reply related
* Re: [RFC bpf-next 04/12] bpf: Add struct bpf_tramp_node object
From: Andrii Nakryiko @ 2026-02-04 19:00 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: <20260203093819.2105105-5-jolsa@kernel.org>
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. Because then there is also a question what is
bpf_link's prog pointing to?...
>
> 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
* Re: [RFC bpf-next 08/12] libbpf: Add btf__find_by_glob_kind function
From: Andrii Nakryiko @ 2026-02-04 19:04 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: <20260203093819.2105105-9-jolsa@kernel.org>
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?
Also, you don't seem to be using deny_pattern, where you planning to?
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)?
> #ifdef __cplusplus
> } /* extern "C" */
> #endif
> --
> 2.52.0
>
^ permalink raw reply
* Re: [RFC bpf-next 07/12] bpf: Add support to create tracing multi link
From: Andrii Nakryiko @ 2026-02-04 19:05 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: <20260203093819.2105105-8-jolsa@kernel.org>
On Tue, Feb 3, 2026 at 1:39 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding new link to allow to attach program to multiple function
> BTF IDs. The link is represented by struct bpf_tracing_multi_link.
>
> To configure the link, new fields are added to bpf_attr::link_create
> to pass array of BTF IDs;
>
> struct {
> __aligned_u64 btf_ids; /* addresses to attach */
> __u32 btf_ids_cnt; /* addresses count */
cookies suspiciously missing?
> } tracing_multi;
>
> Each BTF ID represents function (BTF_KIND_FUNC) that the link will
> attach bpf program to.
>
> We use previously added bpf_trampoline_multi_attach/detach functions
> to attach/detach the link.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> include/linux/trace_events.h | 6 ++
> include/uapi/linux/bpf.h | 5 ++
> kernel/bpf/syscall.c | 2 +
> kernel/trace/bpf_trace.c | 105 +++++++++++++++++++++++++++++++++
> tools/include/uapi/linux/bpf.h | 5 ++
> 5 files changed, 123 insertions(+)
>
[...]
^ permalink raw reply
* Re: [RFC bpf-next 09/12] libbpf: Add support to create tracing multi link
From: Andrii Nakryiko @ 2026-02-04 19:05 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: <20260203093819.2105105-10-jolsa@kernel.org>
On Tue, Feb 3, 2026 at 1:40 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding new interface function to attach programs with tracing
> multi link:
>
> bpf_program__attach_tracing_multi(const struct bpf_program *prog,
> const char *pattern,
> const struct bpf_tracing_multi_opts *opts);
>
> The program is attach to functions specified by pattern or by
> btf IDs specified in bpf_tracing_multi_opts object.
>
> Adding support for new sections to attach programs with above
> functions:
>
> fentry.multi/pattern
> fexit.multi/pattern
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/lib/bpf/bpf.c | 7 ++++
> tools/lib/bpf/bpf.h | 4 ++
> tools/lib/bpf/libbpf.c | 87 ++++++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 14 +++++++
> tools/lib/bpf/libbpf.map | 1 +
> 5 files changed, 113 insertions(+)
[...]
> static const char * const map_type_name[] = {
> @@ -9814,6 +9817,7 @@ static int attach_kprobe_session(const struct bpf_program *prog, long cookie, st
> static int attach_uprobe_multi(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> static int attach_lsm(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> static int attach_iter(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> +static int attach_tracing_multi(const struct bpf_program *prog, long cookie, struct bpf_link **link);
>
> static const struct bpf_sec_def section_defs[] = {
> SEC_DEF("socket", SOCKET_FILTER, 0, SEC_NONE),
> @@ -9862,6 +9866,8 @@ static const struct bpf_sec_def section_defs[] = {
> SEC_DEF("fexit.s+", TRACING, BPF_TRACE_FEXIT, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_trace),
> SEC_DEF("fsession+", TRACING, BPF_TRACE_FSESSION, SEC_ATTACH_BTF, attach_trace),
> SEC_DEF("fsession.s+", TRACING, BPF_TRACE_FSESSION, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_trace),
> + SEC_DEF("fentry.multi+", TRACING, BPF_TRACE_FENTRY_MULTI, 0, attach_tracing_multi),
> + SEC_DEF("fexit.multi+", TRACING, BPF_TRACE_FEXIT_MULTI, 0, attach_tracing_multi),
> SEC_DEF("freplace+", EXT, 0, SEC_ATTACH_BTF, attach_trace),
> SEC_DEF("lsm+", LSM, BPF_LSM_MAC, SEC_ATTACH_BTF, attach_lsm),
> SEC_DEF("lsm.s+", LSM, BPF_LSM_MAC, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_lsm),
> @@ -12237,6 +12243,87 @@ static int attach_uprobe_multi(const struct bpf_program *prog, long cookie, stru
> return ret;
> }
>
> +struct bpf_link *
> +bpf_program__attach_tracing_multi(const struct bpf_program *prog, const char *pattern,
> + const struct bpf_tracing_multi_opts *opts)
> +{
> + LIBBPF_OPTS(bpf_link_create_opts, lopts);
> + __u32 *btf_ids, cnt, *free_ids = NULL;
> + int prog_fd, link_fd, err;
> + struct bpf_link *link;
> +
> + btf_ids = OPTS_GET(opts, btf_ids, false);
> + cnt = OPTS_GET(opts, cnt, false);
> +
> + if (!pattern && !btf_ids && !cnt)
let's check that either both btf_ids and cnt are specified or none
then we can check that either pattern or btf_ids are specified
still two checks, but will capture all the bad cases
> + return libbpf_err_ptr(-EINVAL);
> + if (pattern && (btf_ids || cnt))
> + return libbpf_err_ptr(-EINVAL);
> +
[...]
> struct bpf_uprobe_opts {
> /* size of this struct, for forward/backward compatibility */
> size_t sz;
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index d18fbcea7578..a3ffb21270e9 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -358,6 +358,7 @@ LIBBPF_1.0.0 {
> bpf_program__attach_ksyscall;
> bpf_program__autoattach;
> bpf_program__set_autoattach;
> + bpf_program__attach_tracing_multi;
stuck in the past? ;) we are in 1.7 cycle
> btf__add_enum64;
> btf__add_enum64_value;
> libbpf_bpf_attach_type_str;
> --
> 2.52.0
>
^ permalink raw reply
* [ANNOUNCE] libtraceevent 1.9 released!
From: Steven Rostedt @ 2026-02-04 19:23 UTC (permalink / raw)
To: Linux Trace Devel, linux-trace-users@vger.kernel.org,
Linux Trace Kernel
libtraceevent: 1.9.0
Changes since 1.8.6:
New APIs:
Addition of loading the BTF file (/sys/kernel/btf/vmlinux) into the tep
handle to allow it to parse function parameters:
cat-33501 [005] ..... 136155.767950: function: __x64_sys_execve(regs=0xffffc9000e3eff58)
cat-33501 [005] ..... 136155.767951: function: getname_flags(filename=0x7ffe7d33f3d0, flags=0)
cat-33501 [005] ..... 136155.767951: function: getname_flags.part.0(7ffe7d33f3d0, 0, 0, 0, 0, 0)
cat-33501 [005] ..... 136155.767951: function: kmem_cache_alloc_noprof(s=0xffff8881001d3800, gfpflags=0xcc0)
cat-33501 [005] ..... 136155.767951: function: fs_reclaim_acquire(gfp_mask=0xcc0)
cat-33501 [005] ..... 136155.767952: function: fs_reclaim_release(gfp_mask=0xcc0)
tep_load_btf() - loads the raw BTF file and parses it.
tep_btf_print_args() - parses the arguments of a given function with the
given parameters.
tep_btf_list_args() - list the arguments of a given function to produce
its prototype.
- Add API for persistent ring buffer processing:
tep_parse_last_boot_info() - Reads and parses the last_boot_info file of a
persistent ring buffer from a previous boot. This
has the information to convert the addresses of
functions from the previous boot to the addresses of
the corresponding function of the current boot.
tep_load_modules() - If last_boot_info is loaded, then /proc/modules
should also be loaded to compare where the addresses
of the modules were loaded in the last boot (found in
the last_boot_info) to where they are loaded in the
current boot. This is used to convert the addresses
of module functions from the last boot to the address
of their corresponding function in the current boot.
The above functions are used to be able to convert the addresses of the
functions from the previous boot to their corresponding functions of the
current boot that the kallsyms came from. (see tep_parse_kallsyms())
Updates:
- Handle __get_stacktrace() in the print-fmt field that synthetic events
can add when they have a stacktrace field.
^ permalink raw reply
* Re: [PATCH v2 1/2] bootconfig: Terminate value search if it hits a newline
From: Julius Werner @ 2026-02-04 19:25 UTC (permalink / raw)
To: Masami Hiramatsu (Google); +Cc: Steven Rostedt, linux-trace-kernel, LKML
In-Reply-To: <177019402883.80694.10977484286951965499.stgit@devnote2>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Thank you!
^ permalink raw reply
* Re: [PATCH mm-unstable v14 00/16] khugepaged: mTHP support
From: Nico Pache @ 2026-02-04 21:35 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: linux-mm, linux-doc, linux-kernel, linux-trace-kernel, akpm,
david, ziy, baolin.wang, Liam.Howlett, ryan.roberts, dev.jain,
baohua, lance.yang, vbabka, rppt, surenb, mhocko, corbet, rostedt,
mhiramat, mathieu.desnoyers, matthew.brost, joshua.hahnjy,
rakie.kim, byungchul, gourry, ying.huang, apopple, jannh,
pfalcato, jackmanb, hannes, willy, peterx, wangkefeng.wang,
usamaarif642, sunnanyong, vishal.moola, thomas.hellstrom, yang,
kas, aarcange, raquini, anshuman.khandual, catalin.marinas, tiwai,
will, dave.hansen, jack, cl, jglisse, zokeefe, rientjes, rdunlap,
hughd, richard.weiyang
In-Reply-To: <8067c5c9-491e-453a-be1b-33603744758e@lucifer.local>
On Mon, Jan 26, 2026 at 4:34 AM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
>
> On Thu, Jan 22, 2026 at 12:28:25PM -0700, Nico Pache wrote:
> > V14 Changes:
> > - Added review tags
> > - refactored is_mthp_order() to is_pmd_order(), utilized it in more places, and
> > moved it to the first commit of the series
> > - squashed fixup sent with v13
> > - rebased and handled conflicts with new madvise_collapse writeback retry logic [3]
> > - handled conflict with khugepaged cleanup series [4]
>
> Hmm no mention of change to 3/16, unless it's folded into one of the above?
It's the line with [3], but yeah my bad, i'll try to be more detailed
with these change logs in the future. Was particularly lazy on this
one.
Thanks for the reviews :)
-- Nico
>
> Very important to make reviewers aware of this stuff.
>
> It's also worth separating out things at a fine-grained level, really
> everything. More detail is good. See [0] for example - I practice what I preach
> :)
>
> Thanks, Lorenzo
>
> [0]:https://lore.kernel.org/linux-mm/cover.1769198904.git.lorenzo.stoakes@oracle.com/
>
^ permalink raw reply
* Re: [PATCH mm-unstable v14 07/16] khugepaged: introduce collapse_max_ptes_none helper function
From: Nico Pache @ 2026-02-04 21:39 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: linux-mm, linux-doc, linux-kernel, linux-trace-kernel, akpm,
david, ziy, baolin.wang, Liam.Howlett, ryan.roberts, dev.jain,
baohua, lance.yang, vbabka, rppt, surenb, mhocko, corbet, rostedt,
mhiramat, mathieu.desnoyers, matthew.brost, joshua.hahnjy,
rakie.kim, byungchul, gourry, ying.huang, apopple, jannh,
pfalcato, jackmanb, hannes, willy, peterx, wangkefeng.wang,
usamaarif642, sunnanyong, vishal.moola, thomas.hellstrom, yang,
kas, aarcange, raquini, anshuman.khandual, catalin.marinas, tiwai,
will, dave.hansen, jack, cl, jglisse, zokeefe, rientjes, rdunlap,
hughd, richard.weiyang
In-Reply-To: <db10946c-9743-49e0-a845-7f53a60778a6@lucifer.local>
On Tue, Feb 3, 2026 at 5:09 AM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
>
> On Thu, Jan 22, 2026 at 12:28:32PM -0700, Nico Pache wrote:
> > The current mechanism for determining mTHP collapse scales the
> > khugepaged_max_ptes_none value based on the target order. This
> > introduces an undesirable feedback loop, or "creep", when max_ptes_none
> > is set to a value greater than HPAGE_PMD_NR / 2.
> >
> > With this configuration, a successful collapse to order N will populate
> > enough pages to satisfy the collapse condition on order N+1 on the next
> > scan. This leads to unnecessary work and memory churn.
> >
> > To fix this issue introduce a helper function that will limit mTHP
> > collapse support to two max_ptes_none values, 0 and HPAGE_PMD_NR - 1.
> > This effectively supports two modes:
> >
> > - max_ptes_none=0: never introduce new none-pages for mTHP collapse.
> > - max_ptes_none=511 (on 4k pagesz): Always collapse to the highest
> > available mTHP order.
> >
> > This removes the possiblilty of "creep", while not modifying any uAPI
> > expectations. A warning will be emitted if any non-supported
> > max_ptes_none value is configured with mTHP enabled.
> >
> > The limits can be ignored by passing full_scan=true, this is useful for
> > madvise_collapse (which ignores limits), or in the case of
> > collapse_scan_pmd(), allows the full PMD to be scanned when mTHP
> > collapse is available.
>
> Thanks, great commit msg!
>
> >
> > Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> > Signed-off-by: Nico Pache <npache@redhat.com>
>
> This LGTM in terms of logic, some nits below, with those addressed feel
> free to add:
>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Thanks :)
>
> Cheers, Lorenzo
>
> > ---
> > mm/khugepaged.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 42 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index 0f68902edd9a..9b7e05827749 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -460,6 +460,44 @@ void __khugepaged_enter(struct mm_struct *mm)
> > wake_up_interruptible(&khugepaged_wait);
> > }
> >
> > +/**
> > + * collapse_max_ptes_none - Calculate maximum allowed empty PTEs for collapse
> > + * @order: The folio order being collapsed to
> > + * @full_scan: Whether this is a full scan (ignore limits)
> > + *
> > + * For madvise-triggered collapses (full_scan=true), all limits are bypassed
> > + * and allow up to HPAGE_PMD_NR - 1 empty PTEs.
> > + *
> > + * For PMD-sized collapses (order == HPAGE_PMD_ORDER), use the configured
> > + * khugepaged_max_ptes_none value.
> > + *
> > + * For mTHP collapses, we currently only support khugepaged_max_pte_none values
> > + * of 0 or (HPAGE_PMD_NR - 1). Any other value will emit a warning and no mTHP
> > + * collapse will be attempted
> > + *
> > + * Return: Maximum number of empty PTEs allowed for the collapse operation
> > + */
> > +static unsigned int collapse_max_ptes_none(unsigned int order, bool full_scan)
> > +{
> > + /* ignore max_ptes_none limits */
> > + if (full_scan)
> > + return HPAGE_PMD_NR - 1;
>
> I wonder if, given we are effectively doing:
>
> const unsigned int nr_pages = collapse_max_ptes_none(order, /*full_scan=*/true);
>
> ...
>
> foo(nr_pages);
>
> In places where we ignore limits, whether we would be better off putting
> HPAGE_PMD_NR - 1 into a define and just using that in these cases, like:
>
> #define COLLAPSE_MAX_PTES_LIM (HPAGE_PMD_NR - 1)
>
> Then instead doing:
>
> foo(COLLAPSE_MAX_PTES_LIM);
>
> ?
>
> Seems somewhat silly to pass in a boolean that makes it return a set value in
> cases where you know that should be the case at the outset.
>
> > +
> > + if (is_pmd_order(order))
> > + return khugepaged_max_ptes_none;
> > +
> > + /* Zero/non-present collapse disabled. */
> > + if (!khugepaged_max_ptes_none)
> > + return 0;
> > +
> > + if (khugepaged_max_ptes_none == HPAGE_PMD_NR - 1)
>
> Having a define for HPAGE_PMD_NR - 1 would also be handy here...
>
> > + return (1 << order) - 1;
> > +
> > + pr_warn_once("mTHP collapse only supports max_ptes_none values of 0 or %d\n",
> > + HPAGE_PMD_NR - 1);
>
> ...and here.
>
> Also a MICRO nit here - the function returns unsigned int and thus we
> express PTEs in this unit, so maybe use %u rather than %d?
>
> > + return -EINVAL;
> > +}
>
> Logic of this function looks correct though!
>
> > +
> > void khugepaged_enter_vma(struct vm_area_struct *vma,
> > vm_flags_t vm_flags)
> > {
> > @@ -548,7 +586,10 @@ static enum scan_result __collapse_huge_page_isolate(struct vm_area_struct *vma,
> > int none_or_zero = 0, shared = 0, referenced = 0;
> > enum scan_result result = SCAN_FAIL;
> > const unsigned long nr_pages = 1UL << order;
> > - int max_ptes_none = khugepaged_max_ptes_none >> (HPAGE_PMD_ORDER - order);
> > + int max_ptes_none = collapse_max_ptes_none(order, !cc->is_khugepaged);
>
> Yeah, the !cc->is_khugepaged is a bit gross here, so as per the above, maybe do:
Ok sounds good! I'll make the recommended changes.
Thanks!
-- Nico
>
> int max_ptes_none;
>
> if (cc->is_khugepaged)
> max_ptes_none = collapse_max_ptes_none(order);
> else /* MADV_COLLAPSE is not limited. */
> max_ptes_none = COLLAPSE_MAX_PTES_LIM;
>
> > +
> > + if (max_ptes_none == -EINVAL)
> > + return result;
> >
> > for (_pte = pte; _pte < pte + nr_pages;
> > _pte++, addr += PAGE_SIZE) {
> > --
> > 2.52.0
> >
>
^ permalink raw reply
* Re: [PATCH] [v2] tracing: move __printf() attribute on __ftrace_vbprintk()
From: David Laight @ 2026-02-04 21:48 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Steven Rostedt, Masami Hiramatsu, Anna Schumaker, Jeff Layton,
Chuck Lever, Simon Horman, Arnd Bergmann, Mathieu Desnoyers,
Andrew Morton, Andy Shevchenko, Yury Norov, Randy Dunlap,
linux-kernel, linux-trace-kernel
In-Reply-To: <20260203164545.3174910-1-arnd@kernel.org>
On Tue, 3 Feb 2026 17:45:29 +0100
Arnd Bergmann <arnd@kernel.org> wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The sunrpc change to use trace_printk() for debugging caused
> a new warning for every instance of dprintk() in some configurations,
> when -Wformat-security is enabled:
>
> fs/nfs/getroot.c: In function 'nfs_get_root':
> fs/nfs/getroot.c:90:17: error: format not a string literal and no format arguments [-Werror=format-security]
> 90 | nfs_errorf(fc, "NFS: Couldn't getattr on root");
>
> I've been slowly chipping away at those warnings over time with the
> intention of enabling them by default in the future. While I could not
> figure out why this only happens for this one instance, I see that the
> __trace_bprintk() function is always called with a local variable as
> the format string, rather than a literal.
>
> Move the __printf(2,3) annotation on this function from the declaration
> to the caller. As this is can only be validated for literals, the
^ definition ?
David
> attribute on the declaration causes the warnings every time, but
> removing it entirely introduces a new warning on the __ftrace_vbprintk()
> definition.
>
> The format strings still get checked because the underlying literal keeps
> getting passed into __trace_printk() in the "else" branch, which is not
> taken but still evaluated for compile-time warnings.
>
> Fixes: ec7d8e68ef0e ("sunrpc: add a Kconfig option to redirect dfprintk() output to trace buffer")
> Acked-by: Jeff Layton <jlayton@kernel.org>
> Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v2: included fix for regression reported by kernel test robot <lkp@intel.com>
> ---
> include/linux/trace_printk.h | 1 -
> kernel/trace/trace_printk.c | 1 +
> 2 files changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/trace_printk.h b/include/linux/trace_printk.h
> index bb5874097f24..2670ec7f4262 100644
> --- a/include/linux/trace_printk.h
> +++ b/include/linux/trace_printk.h
> @@ -107,7 +107,6 @@ do { \
> __trace_printk(_THIS_IP_, fmt, ##args); \
> } while (0)
>
> -extern __printf(2, 3)
> int __trace_bprintk(unsigned long ip, const char *fmt, ...);
>
> extern __printf(2, 3)
> diff --git a/kernel/trace/trace_printk.c b/kernel/trace/trace_printk.c
> index 29f6e95439b6..48c085fcae7a 100644
> --- a/kernel/trace/trace_printk.c
> +++ b/kernel/trace/trace_printk.c
> @@ -197,6 +197,7 @@ struct notifier_block module_trace_bprintk_format_nb = {
> .notifier_call = module_trace_bprintk_format_notify,
> };
>
> +__printf(2, 3)
> int __trace_bprintk(unsigned long ip, const char *fmt, ...)
> {
> int ret;
^ permalink raw reply
* Re: [PATCH mm-unstable v14 08/16] khugepaged: generalize collapse_huge_page for mTHP collapse
From: Nico Pache @ 2026-02-04 22:00 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: linux-mm, linux-doc, linux-kernel, linux-trace-kernel, akpm,
david, ziy, baolin.wang, Liam.Howlett, ryan.roberts, dev.jain,
baohua, lance.yang, vbabka, rppt, surenb, mhocko, corbet, rostedt,
mhiramat, mathieu.desnoyers, matthew.brost, joshua.hahnjy,
rakie.kim, byungchul, gourry, ying.huang, apopple, jannh,
pfalcato, jackmanb, hannes, willy, peterx, wangkefeng.wang,
usamaarif642, sunnanyong, vishal.moola, thomas.hellstrom, yang,
kas, aarcange, raquini, anshuman.khandual, catalin.marinas, tiwai,
will, dave.hansen, jack, cl, jglisse, zokeefe, rientjes, rdunlap,
hughd, richard.weiyang
In-Reply-To: <599ebe0a-086a-4701-b797-dcd801ad02fb@lucifer.local>
On Tue, Feb 3, 2026 at 6:13 AM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
>
> On Thu, Jan 22, 2026 at 12:28:33PM -0700, Nico Pache wrote:
> > Pass an order and offset to collapse_huge_page to support collapsing anon
> > memory to arbitrary orders within a PMD. order indicates what mTHP size we
> > are attempting to collapse to, and offset indicates were in the PMD to
> > start the collapse attempt.
> >
> > For non-PMD collapse we must leave the anon VMA write locked until after
> > we collapse the mTHP-- in the PMD case all the pages are isolated, but in
> > the mTHP case this is not true, and we must keep the lock to prevent
> > changes to the VMA from occurring.
> >
> > Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> > Tested-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> > Signed-off-by: Nico Pache <npache@redhat.com>
> > ---
> > mm/khugepaged.c | 111 +++++++++++++++++++++++++++++++-----------------
> > 1 file changed, 71 insertions(+), 40 deletions(-)
> >
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index 9b7e05827749..76cb17243793 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -1151,44 +1151,54 @@ static enum scan_result alloc_charge_folio(struct folio **foliop, struct mm_stru
> > return SCAN_SUCCEED;
> > }
> >
> > -static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long address,
> > - int referenced, int unmapped, struct collapse_control *cc)
> > +static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long start_addr,
> > + int referenced, int unmapped, struct collapse_control *cc,
> > + bool *mmap_locked, unsigned int order)
> > {
> > LIST_HEAD(compound_pagelist);
> > pmd_t *pmd, _pmd;
> > - pte_t *pte;
> > + pte_t *pte = NULL;
> > pgtable_t pgtable;
> > struct folio *folio;
> > spinlock_t *pmd_ptl, *pte_ptl;
> > enum scan_result result = SCAN_FAIL;
> > struct vm_area_struct *vma;
> > struct mmu_notifier_range range;
> > + bool anon_vma_locked = false;
> > + const unsigned long nr_pages = 1UL << order;
> > + const unsigned long pmd_address = start_addr & HPAGE_PMD_MASK;
> >
> > - VM_BUG_ON(address & ~HPAGE_PMD_MASK);
> > + VM_WARN_ON_ONCE(pmd_address & ~HPAGE_PMD_MASK);
> >
> > /*
> > * Before allocating the hugepage, release the mmap_lock read lock.
> > * The allocation can take potentially a long time if it involves
> > * sync compaction, and we do not need to hold the mmap_lock during
> > * that. We will recheck the vma after taking it again in write mode.
> > + * If collapsing mTHPs we may have already released the read_lock.
> > */
> > - mmap_read_unlock(mm);
> > + if (*mmap_locked) {
> > + mmap_read_unlock(mm);
> > + *mmap_locked = false;
> > + }
> >
> > - result = alloc_charge_folio(&folio, mm, cc, HPAGE_PMD_ORDER);
> > + result = alloc_charge_folio(&folio, mm, cc, order);
> > if (result != SCAN_SUCCEED)
> > goto out_nolock;
> >
> > mmap_read_lock(mm);
> > - result = hugepage_vma_revalidate(mm, address, true, &vma, cc,
> > - HPAGE_PMD_ORDER);
> > + *mmap_locked = true;
> > + result = hugepage_vma_revalidate(mm, pmd_address, true, &vma, cc, order);
>
> Why would we use the PMD address here rather than the actual start address?
The revalidation relies on the pmd_addr not the start_addr. It (only)
uses this to make sure the VMA is still at least PMD sized, and it
uses the order to validate that the target order is allowed. I left a
small comment about this in the revalidate function.
>
> Also please add /*expect_anon=*/ before the 'true' because it's hard to
> understand what that references.
ack
>
> > if (result != SCAN_SUCCEED) {
> > mmap_read_unlock(mm);
> > + *mmap_locked = false;
> > goto out_nolock;
> > }
> >
> > - result = find_pmd_or_thp_or_none(mm, address, &pmd);
> > + result = find_pmd_or_thp_or_none(mm, pmd_address, &pmd);
> > if (result != SCAN_SUCCEED) {
> > mmap_read_unlock(mm);
> > + *mmap_locked = false;
> > goto out_nolock;
> > }
> >
> > @@ -1198,13 +1208,16 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long a
> > * released when it fails. So we jump out_nolock directly in
> > * that case. Continuing to collapse causes inconsistency.
> > */
> > - result = __collapse_huge_page_swapin(mm, vma, address, pmd,
> > - referenced, HPAGE_PMD_ORDER);
> > - if (result != SCAN_SUCCEED)
> > + result = __collapse_huge_page_swapin(mm, vma, start_addr, pmd,
> > + referenced, order);
> > + if (result != SCAN_SUCCEED) {
> > + *mmap_locked = false;
> > goto out_nolock;
> > + }
> > }
> >
> > mmap_read_unlock(mm);
> > + *mmap_locked = false;
> > /*
> > * Prevent all access to pagetables with the exception of
> > * gup_fast later handled by the ptep_clear_flush and the VM
> > @@ -1214,20 +1227,20 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long a
> > * mmap_lock.
> > */
> > mmap_write_lock(mm);
> > - result = hugepage_vma_revalidate(mm, address, true, &vma, cc,
> > - HPAGE_PMD_ORDER);
> > + result = hugepage_vma_revalidate(mm, pmd_address, true, &vma, cc, order);
> > if (result != SCAN_SUCCEED)
> > goto out_up_write;
> > /* check if the pmd is still valid */
> > vma_start_write(vma);
> > - result = check_pmd_still_valid(mm, address, pmd);
> > + result = check_pmd_still_valid(mm, pmd_address, pmd);
> > if (result != SCAN_SUCCEED)
> > goto out_up_write;
> >
> > anon_vma_lock_write(vma->anon_vma);
> > + anon_vma_locked = true;
> >
> > - mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, mm, address,
> > - address + HPAGE_PMD_SIZE);
> > + mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, mm, start_addr,
> > + start_addr + (PAGE_SIZE << order));
> > mmu_notifier_invalidate_range_start(&range);
> >
> > pmd_ptl = pmd_lock(mm, pmd); /* probably unnecessary */
> > @@ -1239,24 +1252,21 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long a
> > * Parallel GUP-fast is fine since GUP-fast will back off when
> > * it detects PMD is changed.
> > */
> > - _pmd = pmdp_collapse_flush(vma, address, pmd);
> > + _pmd = pmdp_collapse_flush(vma, pmd_address, pmd);
> > spin_unlock(pmd_ptl);
> > mmu_notifier_invalidate_range_end(&range);
> > tlb_remove_table_sync_one();
> >
> > - pte = pte_offset_map_lock(mm, &_pmd, address, &pte_ptl);
> > + pte = pte_offset_map_lock(mm, &_pmd, start_addr, &pte_ptl);
> > if (pte) {
> > - result = __collapse_huge_page_isolate(vma, address, pte, cc,
> > - HPAGE_PMD_ORDER,
> > - &compound_pagelist);
> > + result = __collapse_huge_page_isolate(vma, start_addr, pte, cc,
> > + order, &compound_pagelist);
> > spin_unlock(pte_ptl);
> > } else {
> > result = SCAN_NO_PTE_TABLE;
> > }
> >
> > if (unlikely(result != SCAN_SUCCEED)) {
> > - if (pte)
> > - pte_unmap(pte);
> > spin_lock(pmd_ptl);
> > BUG_ON(!pmd_none(*pmd));
> > /*
> > @@ -1266,21 +1276,21 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long a
> > */
> > pmd_populate(mm, pmd, pmd_pgtable(_pmd));
> > spin_unlock(pmd_ptl);
> > - anon_vma_unlock_write(vma->anon_vma);
> > goto out_up_write;
> > }
> >
> > /*
> > - * All pages are isolated and locked so anon_vma rmap
> > - * can't run anymore.
> > + * For PMD collapse all pages are isolated and locked so anon_vma
> > + * rmap can't run anymore. For mTHP collapse we must hold the lock
> > */
> > - anon_vma_unlock_write(vma->anon_vma);
> > + if (is_pmd_order(order)) {
> > + anon_vma_unlock_write(vma->anon_vma);
> > + anon_vma_locked = false;
> > + }
> >
> > result = __collapse_huge_page_copy(pte, folio, pmd, _pmd,
> > - vma, address, pte_ptl,
> > - HPAGE_PMD_ORDER,
> > - &compound_pagelist);
> > - pte_unmap(pte);
> > + vma, start_addr, pte_ptl,
> > + order, &compound_pagelist);
> > if (unlikely(result != SCAN_SUCCEED))
> > goto out_up_write;
> >
> > @@ -1290,20 +1300,42 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long a
> > * write.
> > */
> > __folio_mark_uptodate(folio);
> > - pgtable = pmd_pgtable(_pmd);
> > + if (is_pmd_order(order)) { /* PMD collapse */
> > + pgtable = pmd_pgtable(_pmd);
> >
> > - spin_lock(pmd_ptl);
> > - BUG_ON(!pmd_none(*pmd));
> > - pgtable_trans_huge_deposit(mm, pmd, pgtable);
> > - map_anon_folio_pmd_nopf(folio, pmd, vma, address);
> > + spin_lock(pmd_ptl);
> > + WARN_ON_ONCE(!pmd_none(*pmd));
> > + pgtable_trans_huge_deposit(mm, pmd, pgtable);
> > + map_anon_folio_pmd_nopf(folio, pmd, vma, pmd_address);
> > + } else { /* mTHP collapse */
> > + pte_t mthp_pte = mk_pte(folio_page(folio, 0), vma->vm_page_prot);
> > +
> > + mthp_pte = maybe_mkwrite(pte_mkdirty(mthp_pte), vma);
> > + spin_lock(pmd_ptl);
> > + WARN_ON_ONCE(!pmd_none(*pmd));
> > + folio_ref_add(folio, nr_pages - 1);
> > + folio_add_new_anon_rmap(folio, vma, start_addr, RMAP_EXCLUSIVE);
> > + folio_add_lru_vma(folio, vma);
> > + set_ptes(vma->vm_mm, start_addr, pte, mthp_pte, nr_pages);
> > + update_mmu_cache_range(NULL, vma, start_addr, pte, nr_pages);
> > +
> > + smp_wmb(); /* make PTEs visible before PMD. See pmd_install() */
> > + pmd_populate(mm, pmd, pmd_pgtable(_pmd));
>
> I seriously hate this being open-coded, can we separate it out into another
> function?
Yeah I think we've discussed this before. I started to generalize
this, and apply it to other parts of the kernel that maintain a
similar pattern, but each potential user of the helper was slightly
different in its approach and I was unable to find a quick solution to
make it apply to all. I think it will require a lot more thought to
cleanly refactor this. I figured I could leave this to the later
cleanup work, or I could just create a static function just for
khugepaged for now?
>
> > + }
> > spin_unlock(pmd_ptl);
> >
> > folio = NULL;
> >
> > result = SCAN_SUCCEED;
> > out_up_write:
> > + if (anon_vma_locked)
> > + anon_vma_unlock_write(vma->anon_vma);
>
> Thanks it's much better tracking this specifically.
>
> The whole damn thing needs refactoring (by this I mean - khugepaged and really
> THP in general to be clear :) but it's not your fault.
Yeah it has not been the prettiest code to try and understand/work on!
>
> Could I ask though whether you might help out with some cleanups after this
> lands :)
>
> I feel like we all need to do our bit to pay down some technical debt!
Yes ofc! I had already planned on doing so. I have some in mind, and I
believe others have already tackled some. After this land, let's
discuss further plans (discussion thread or THP meeting).
Cheers,
-- Nico
>
> > + if (pte)
> > + pte_unmap(pte);
> > mmap_write_unlock(mm);
> > + *mmap_locked = false;
> > out_nolock:
> > + WARN_ON_ONCE(*mmap_locked);
> > if (folio)
> > folio_put(folio);
> > trace_mm_collapse_huge_page(mm, result == SCAN_SUCCEED, result);
> > @@ -1471,9 +1503,8 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> > pte_unmap_unlock(pte, ptl);
> > if (result == SCAN_SUCCEED) {
> > result = collapse_huge_page(mm, start_addr, referenced,
> > - unmapped, cc);
> > - /* collapse_huge_page will return with the mmap_lock released */
> > - *mmap_locked = false;
> > + unmapped, cc, mmap_locked,
> > + HPAGE_PMD_ORDER);
> > }
> > out:
> > trace_mm_khugepaged_scan_pmd(mm, folio, referenced,
> > --
> > 2.52.0
> >
>
> Cheers, Lorenzo
>
^ permalink raw reply
* Re: [PATCH v2 1/2] bootconfig: Terminate value search if it hits a newline
From: Steven Rostedt @ 2026-02-04 22:20 UTC (permalink / raw)
To: Masami Hiramatsu (Google); +Cc: linux-trace-kernel, Julius Werner, LKML
In-Reply-To: <177019402883.80694.10977484286951965499.stgit@devnote2>
On Wed, 4 Feb 2026 17:33:48 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> Terminate the value search for a key if it hits a newline and make
> the value empty.
>
> When we pass a bootconfig with an empty value terminated by the
> newline, like below::
>
> foo =
> bar = value
>
> Current bootconfig interprets it as a single entry::
>
> foo = "bar = value";
>
> The Documentation/admin-guide/bootconfig.rst defines the value
> itself is terminated by newline:
>
> The value has to be terminated by semi-colon (``;``) or newline (``\n``).
>
> but it does not define when the value search is terminated.
> This changes the behavior to more line-oriented, so that it can
to be more line-oriented
> clear how it is working.
so that it is clearer in how it works.
>
> - The value search of key-value pair will be terminated by a comment
> or newline.
> - The value search of an array will continue beyond comments and
> newlines.
>
> Thus, with this update, the above example is interpreted as::
>
> foo = "";
> bar = "value";
>
> And the below example will cause a syntax error because "bar" is expected
> as a key but it has ','.
>
> foo =
> bar, buz
>
> According to this change, one wrong example config is updated.
>
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
> Changes in v2:
> - Fix to handle multi-line array case correctly.
> - Make this as a spec update, not fix.
> ---
> .../samples/good-array-space-comment.bconf | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/Documentation/admin-guide/bootconfig.rst b/Documentation/admin-guide/bootconfig.rst
> index 7a86042c9b6d..843b24b8de88 100644
> --- a/Documentation/admin-guide/bootconfig.rst
> +++ b/Documentation/admin-guide/bootconfig.rst
> @@ -20,18 +20,26 @@ Config File Syntax
>
> The boot config syntax is a simple structured key-value. Each key consists
> of dot-connected-words, and key and value are connected by ``=``. The value
> -has to be terminated by semi-colon (``;``) or newline (``\n``).
> -For array value, array entries are separated by comma (``,``). ::
> -
> - KEY[.WORD[...]] = VALUE[, VALUE2[...]][;]
> -
> -Unlike the kernel command line syntax, spaces are OK around the comma and ``=``.
> +string has to be terminated by the following delimiters described below.
>
> Each key word must contain only alphabets, numbers, dash (``-``) or underscore
> (``_``). And each value only contains printable characters or spaces except
> for delimiters such as semi-colon (``;``), new-line (``\n``), comma (``,``),
> hash (``#``) and closing brace (``}``).
>
> +If the ``=`` is followed by whitespace up to one of these delimiters, the
> +key is assigned an empty value.
> +
> +For arrays, the array values are comma (``,``) separated, and comments and
> +line breaks with newline (``\n``) are allowed between array values for
> +readability. Thus the first entry of the array must be the same line of the
must be on the same line as the key.
> +key.::
> +
> + KEY[.WORD[...]] = VALUE[, VALUE2[...]][;]
> +
> +Unlike the kernel command line syntax, white spaces (including tabs) are
> +ignored around the comma and ``=``.
> +
> If you want to use those delimiters in a value, you can use either double-
> quotes (``"VALUE"``) or single-quotes (``'VALUE'``) to quote it. Note that
> you can not escape these quotes.
> @@ -138,8 +146,8 @@ This is parsed as below::
> foo = value
> bar = 1, 2, 3
>
> -Note that you can not put a comment between value and delimiter(``,`` or
> -``;``). This means following config has a syntax error ::
> +Note that you can NOT put a comment or a newline between value and delimiter
> +(``,`` or ``;``). This means following config has a syntax error ::
>
> key = 1 # comment
> ,2
> diff --git a/lib/bootconfig.c b/lib/bootconfig.c
> index 81f29c29f47b..c210fb8b1e85 100644
> --- a/lib/bootconfig.c
> +++ b/lib/bootconfig.c
> @@ -557,17 +557,13 @@ static int __init __xbc_close_brace(char *p)
> /*
> * Return delimiter or error, no node added. As same as lib/cmdline.c,
> * you can use " around spaces, but can't escape " for value.
> + * *@__v must point real value string. (not including spaces before value.)
> */
> static int __init __xbc_parse_value(char **__v, char **__n)
> {
> char *p, *v = *__v;
> int c, quotes = 0;
>
> - v = skip_spaces(v);
> - while (*v == '#') {
> - v = skip_comment(v);
> - v = skip_spaces(v);
> - }
> if (*v == '"' || *v == '\'') {
> quotes = *v;
> v++;
> @@ -617,6 +613,13 @@ static int __init xbc_parse_array(char **__v)
> last_parent = xbc_node_get_child(last_parent);
>
> do {
> + /* Search the next array value beyond comments and empty lines */
> + next = skip_spaces(*__v);
> + while (*next == '#') {
> + next = skip_comment(next);
> + next = skip_spaces(next);
> + }
> + *__v = next;
> c = __xbc_parse_value(__v, &next);
> if (c < 0)
> return c;
> @@ -701,9 +704,17 @@ static int __init xbc_parse_kv(char **k, char *v, int op)
> if (ret)
> return ret;
>
> - c = __xbc_parse_value(&v, &next);
> - if (c < 0)
> - return c;
> + v = skip_spaces_until_newline(v);
> + /* If there is a comment, this has an mpty value. */
empty value
-- Steve
> + if (*v == '#') {
> + next = skip_comment(v);
> + *v = '\0';
> + c = '\n';
> + } else {
> + c = __xbc_parse_value(&v, &next);
> + if (c < 0)
> + return c;
> + }
>
> child = xbc_node_get_child(last_parent);
> if (child && xbc_node_is_value(child)) {
> diff --git a/tools/bootconfig/samples/good-array-space-comment.bconf b/tools/bootconfig/samples/good-array-space-comment.bconf
> index 45b938dc0695..416fa2ed4109 100644
> --- a/tools/bootconfig/samples/good-array-space-comment.bconf
> +++ b/tools/bootconfig/samples/good-array-space-comment.bconf
> @@ -1,4 +1,3 @@
> -key = # comment
> - "value1", # comment1
> +key = "value1", # comment1
> "value2" , # comment2
> "value3"
^ permalink raw reply
* Re: [PATCH v2 2/2] bootconfig: Check the parsed output of the good examples
From: Steven Rostedt @ 2026-02-04 22:24 UTC (permalink / raw)
To: Masami Hiramatsu (Google); +Cc: linux-trace-kernel, Julius Werner, LKML
In-Reply-To: <177019403805.80694.10065992068449824435.stgit@devnote2>
On Wed, 4 Feb 2026 17:33:58 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> Check whether the parsed output of the good example configs are
> the same as expected.
>
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Tested-by: Steven Rostedt (Google) <rostedt@goodmis.org>
-- Steve
^ permalink raw reply
* Re: [PATCH v11 00/30] Tracefs support for pKVM
From: Steven Rostedt @ 2026-02-04 22:45 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <20260131132848.254084-1-vdonnefort@google.com>
On Sat, 31 Jan 2026 13:28:18 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:
> changes since v10
BTW, it's always nice to add a link to the previous version here. That way
it makes it easy to backtrace from one version to the next (and reviewers
to see what they said the last time).
Changes since v10: https://lore.kernel.org/all/20260126104419.1649811-1-vdonnefort@google.com/
-- Steve
>
> - Move kerneldoc to .c files (Steven)
> - Return EBUSY on buffer_size_kb write if buffer is loaded (Steven)
> - Remove rb_iter/rb_iters union in trace_remote_iterator (Steven)
> - Rename a refactor trace file seq_operations (Steven)
> - Make trace_get_cpu() accessible to trace_remote.c (Steven)
> - Remove unnecessary cpus_read_unlock() (Steven)
> - !preempt on remote_test driver buffer writing (Steven)
> - Do not fail selftest if cpu/online is unavailable (Steven)
> - Add rational for trace_remote into documentation (Steven)
^ permalink raw reply
* Re: [PATCH v2 1/2] bootconfig: Terminate value search if it hits a newline
From: Masami Hiramatsu @ 2026-02-04 23:13 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-trace-kernel, Julius Werner, LKML
In-Reply-To: <20260204172057.5bcbfcfd@gandalf.local.home>
On Wed, 4 Feb 2026 17:20:57 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed, 4 Feb 2026 17:33:48 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
>
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> >
> > Terminate the value search for a key if it hits a newline and make
> > the value empty.
> >
> > When we pass a bootconfig with an empty value terminated by the
> > newline, like below::
> >
> > foo =
> > bar = value
> >
> > Current bootconfig interprets it as a single entry::
> >
> > foo = "bar = value";
> >
> > The Documentation/admin-guide/bootconfig.rst defines the value
> > itself is terminated by newline:
> >
> > The value has to be terminated by semi-colon (``;``) or newline (``\n``).
> >
> > but it does not define when the value search is terminated.
> > This changes the behavior to more line-oriented, so that it can
>
> to be more line-oriented
>
> > clear how it is working.
>
> so that it is clearer in how it works.
>
OK,
>
> >
> > - The value search of key-value pair will be terminated by a comment
> > or newline.
> > - The value search of an array will continue beyond comments and
> > newlines.
> >
> > Thus, with this update, the above example is interpreted as::
> >
> > foo = "";
> > bar = "value";
> >
> > And the below example will cause a syntax error because "bar" is expected
> > as a key but it has ','.
> >
> > foo =
> > bar, buz
> >
> > According to this change, one wrong example config is updated.
> >
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > ---
> > Changes in v2:
> > - Fix to handle multi-line array case correctly.
> > - Make this as a spec update, not fix.
> > ---
> > .../samples/good-array-space-comment.bconf | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/Documentation/admin-guide/bootconfig.rst b/Documentation/admin-guide/bootconfig.rst
> > index 7a86042c9b6d..843b24b8de88 100644
> > --- a/Documentation/admin-guide/bootconfig.rst
> > +++ b/Documentation/admin-guide/bootconfig.rst
> > @@ -20,18 +20,26 @@ Config File Syntax
> >
> > The boot config syntax is a simple structured key-value. Each key consists
> > of dot-connected-words, and key and value are connected by ``=``. The value
> > -has to be terminated by semi-colon (``;``) or newline (``\n``).
> > -For array value, array entries are separated by comma (``,``). ::
> > -
> > - KEY[.WORD[...]] = VALUE[, VALUE2[...]][;]
> > -
> > -Unlike the kernel command line syntax, spaces are OK around the comma and ``=``.
> > +string has to be terminated by the following delimiters described below.
> >
> > Each key word must contain only alphabets, numbers, dash (``-``) or underscore
> > (``_``). And each value only contains printable characters or spaces except
> > for delimiters such as semi-colon (``;``), new-line (``\n``), comma (``,``),
> > hash (``#``) and closing brace (``}``).
> >
> > +If the ``=`` is followed by whitespace up to one of these delimiters, the
> > +key is assigned an empty value.
> > +
> > +For arrays, the array values are comma (``,``) separated, and comments and
> > +line breaks with newline (``\n``) are allowed between array values for
> > +readability. Thus the first entry of the array must be the same line of the
>
> must be on the same line as the key.
OK.
>
> > +key.::
> > +
> > + KEY[.WORD[...]] = VALUE[, VALUE2[...]][;]
> > +
> > +Unlike the kernel command line syntax, white spaces (including tabs) are
> > +ignored around the comma and ``=``.
> > +
> > If you want to use those delimiters in a value, you can use either double-
> > quotes (``"VALUE"``) or single-quotes (``'VALUE'``) to quote it. Note that
> > you can not escape these quotes.
> > @@ -138,8 +146,8 @@ This is parsed as below::
> > foo = value
> > bar = 1, 2, 3
> >
> > -Note that you can not put a comment between value and delimiter(``,`` or
> > -``;``). This means following config has a syntax error ::
> > +Note that you can NOT put a comment or a newline between value and delimiter
> > +(``,`` or ``;``). This means following config has a syntax error ::
> >
> > key = 1 # comment
> > ,2
> > diff --git a/lib/bootconfig.c b/lib/bootconfig.c
> > index 81f29c29f47b..c210fb8b1e85 100644
> > --- a/lib/bootconfig.c
> > +++ b/lib/bootconfig.c
> > @@ -557,17 +557,13 @@ static int __init __xbc_close_brace(char *p)
> > /*
> > * Return delimiter or error, no node added. As same as lib/cmdline.c,
> > * you can use " around spaces, but can't escape " for value.
> > + * *@__v must point real value string. (not including spaces before value.)
> > */
> > static int __init __xbc_parse_value(char **__v, char **__n)
> > {
> > char *p, *v = *__v;
> > int c, quotes = 0;
> >
> > - v = skip_spaces(v);
> > - while (*v == '#') {
> > - v = skip_comment(v);
> > - v = skip_spaces(v);
> > - }
> > if (*v == '"' || *v == '\'') {
> > quotes = *v;
> > v++;
> > @@ -617,6 +613,13 @@ static int __init xbc_parse_array(char **__v)
> > last_parent = xbc_node_get_child(last_parent);
> >
> > do {
> > + /* Search the next array value beyond comments and empty lines */
> > + next = skip_spaces(*__v);
> > + while (*next == '#') {
> > + next = skip_comment(next);
> > + next = skip_spaces(next);
> > + }
> > + *__v = next;
> > c = __xbc_parse_value(__v, &next);
> > if (c < 0)
> > return c;
> > @@ -701,9 +704,17 @@ static int __init xbc_parse_kv(char **k, char *v, int op)
> > if (ret)
> > return ret;
> >
> > - c = __xbc_parse_value(&v, &next);
> > - if (c < 0)
> > - return c;
> > + v = skip_spaces_until_newline(v);
> > + /* If there is a comment, this has an mpty value. */
>
> empty value
Thanks!
>
> -- Steve
>
> > + if (*v == '#') {
> > + next = skip_comment(v);
> > + *v = '\0';
> > + c = '\n';
> > + } else {
> > + c = __xbc_parse_value(&v, &next);
> > + if (c < 0)
> > + return c;
> > + }
> >
> > child = xbc_node_get_child(last_parent);
> > if (child && xbc_node_is_value(child)) {
> > diff --git a/tools/bootconfig/samples/good-array-space-comment.bconf b/tools/bootconfig/samples/good-array-space-comment.bconf
> > index 45b938dc0695..416fa2ed4109 100644
> > --- a/tools/bootconfig/samples/good-array-space-comment.bconf
> > +++ b/tools/bootconfig/samples/good-array-space-comment.bconf
> > @@ -1,4 +1,3 @@
> > -key = # comment
> > - "value1", # comment1
> > +key = "value1", # comment1
> > "value2" , # comment2
> > "value3"
>
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v11 07/30] tracing: Add non-consuming read to trace remotes
From: Steven Rostedt @ 2026-02-04 23:52 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <20260131132848.254084-8-vdonnefort@google.com>
On Sat, 31 Jan 2026 13:28:25 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:
> -static struct trace_remote_iterator *trace_remote_iter(struct trace_remote *remote, int cpu)
> +static void __free_ring_buffer_iter(struct trace_remote_iterator *iter, int cpu)
> +{
> + if (!iter->rb_iter)
> + return;
Hmm, can't iter->rb_iter be NULL when iter->rb_iters[] is used?
> +
> + if (cpu != RING_BUFFER_ALL_CPUS) {
> + ring_buffer_read_finish(iter->rb_iter);
> + return;
> + }
> +
> + for_each_possible_cpu(cpu) {
> + if (iter->rb_iters[cpu])
> + ring_buffer_read_finish(iter->rb_iters[cpu]);
> + }
> +
> + kfree(iter->rb_iters);
> +}
> +
> +static int __alloc_ring_buffer_iter(struct trace_remote_iterator *iter, int cpu)
> +{
> + if (cpu != RING_BUFFER_ALL_CPUS) {
> + iter->rb_iter = ring_buffer_read_start(iter->remote->trace_buffer, cpu, GFP_KERNEL);
> +
> + return iter->rb_iter ? 0 : -ENOMEM;
> + }
> +
> + iter->rb_iters = kcalloc(nr_cpu_ids, sizeof(*iter->rb_iters), GFP_KERNEL);
> + if (!iter->rb_iters)
> + return -ENOMEM;
> +
> + for_each_possible_cpu(cpu) {
> + iter->rb_iters[cpu] = ring_buffer_read_start(iter->remote->trace_buffer, cpu,
> + GFP_KERNEL);
> + if (!iter->rb_iters[cpu]) {
> + __free_ring_buffer_iter(iter, RING_BUFFER_ALL_CPUS);
For instance, we call __free_ring_buffer_iter() here, but I don't see
iter->rb_iter being set.
-- Steve
> + return -ENOMEM;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static struct trace_remote_iterator
> +*trace_remote_iter(struct trace_remote *remote, int cpu, enum tri_type type)
> {
> struct trace_remote_iterator *iter = NULL;
> int ret;
>
> lockdep_assert_held(&remote->lock);
>
> + if (type == TRI_NONCONSUMING && !trace_remote_loaded(remote))
> + return NULL;
>
> ret = trace_remote_get(remote, cpu);
> if (ret)
> @@ -279,9 +352,21 @@ static struct trace_remote_iterator *trace_remote_iter(struct trace_remote *remo
> if (iter) {
> iter->remote = remote;
> iter->cpu = cpu;
> + iter->type = type;
> trace_seq_init(&iter->seq);
> - INIT_DELAYED_WORK(&iter->poll_work, __poll_remote);
> - schedule_delayed_work(&iter->poll_work, msecs_to_jiffies(remote->poll_ms));
> +
> + switch (type) {
> + case TRI_CONSUMING:
> + INIT_DELAYED_WORK(&iter->poll_work, __poll_remote);
> + schedule_delayed_work(&iter->poll_work, msecs_to_jiffies(remote->poll_ms));
> + break;
> + case TRI_NONCONSUMING:
> + ret = __alloc_ring_buffer_iter(iter, cpu);
> + break;
> + }
> +
> + if (ret)
> + goto err;
>
> return iter;
> }
> @@ -305,10 +390,100 @@ static void trace_remote_iter_free(struct trace_remote_iterator *iter)
>
> lockdep_assert_held(&remote->lock);
>
> + switch (iter->type) {
> + case TRI_CONSUMING:
> + cancel_delayed_work_sync(&iter->poll_work);
> + break;
> + case TRI_NONCONSUMING:
> + __free_ring_buffer_iter(iter, iter->cpu);
> + break;
> + }
> +
> kfree(iter);
> trace_remote_put(remote);
> }
>
> +static void trace_remote_iter_read_start(struct trace_remote_iterator *iter)
> +{
> + struct trace_remote *remote = iter->remote;
> + int cpu = iter->cpu;
> +
> + /* Acquire global reader lock */
> + if (cpu == RING_BUFFER_ALL_CPUS && iter->type == TRI_CONSUMING)
> + down_write(&remote->reader_lock);
> + else
> + down_read(&remote->reader_lock);
> +
> + if (cpu == RING_BUFFER_ALL_CPUS)
> + return;
> +
> + /*
> + * No need for the remote lock here, iter holds a reference on
> + * remote->nr_readers
> + */
> +
> + /* Get the per-CPU one */
> + if (WARN_ON_ONCE(!remote->pcpu_reader_locks))
> + return;
> +
> + if (iter->type == TRI_CONSUMING)
> + down_write(&remote->pcpu_reader_locks[cpu]);
> + else
> + down_read(&remote->pcpu_reader_locks[cpu]);
> +}
> +
> +static void trace_remote_iter_read_finished(struct trace_remote_iterator *iter)
> +{
> + struct trace_remote *remote = iter->remote;
> + int cpu = iter->cpu;
> +
> + /* Release per-CPU reader lock */
> + if (cpu != RING_BUFFER_ALL_CPUS) {
> + /*
> + * No need for the remote lock here, iter holds a reference on
> + * remote->nr_readers
> + */
> + if (iter->type == TRI_CONSUMING)
> + up_write(&remote->pcpu_reader_locks[cpu]);
> + else
> + up_read(&remote->pcpu_reader_locks[cpu]);
> + }
> +
> + /* Release global reader lock */
> + if (cpu == RING_BUFFER_ALL_CPUS && iter->type == TRI_CONSUMING)
> + up_write(&remote->reader_lock);
> + else
> + up_read(&remote->reader_lock);
> +}
> +
> +static struct ring_buffer_iter *__get_rb_iter(struct trace_remote_iterator *iter, int cpu)
> +{
> + return iter->cpu != RING_BUFFER_ALL_CPUS ? iter->rb_iter : iter->rb_iters[cpu];
> +}
> +
> +static struct ring_buffer_event *
> +__peek_event(struct trace_remote_iterator *iter, int cpu, u64 *ts, unsigned long *lost_events)
> +{
> + struct ring_buffer_event *rb_evt;
> + struct ring_buffer_iter *rb_iter;
> +
> + switch (iter->type) {
> + case TRI_CONSUMING:
> + return ring_buffer_peek(iter->remote->trace_buffer, cpu, ts, lost_events);
> + case TRI_NONCONSUMING:
> + rb_iter = __get_rb_iter(iter, cpu);
> + rb_evt = ring_buffer_iter_peek(rb_iter, ts);
> + if (!rb_evt)
> + return NULL;
> +
> + *lost_events = ring_buffer_iter_dropped(rb_iter);
> +
> + return rb_evt;
> + }
> +
> + return NULL;
> +}
> +
> static bool trace_remote_iter_read_event(struct trace_remote_iterator *iter)
> {
> struct trace_buffer *trace_buffer = iter->remote->trace_buffer;
> @@ -318,7 +493,7 @@ static bool trace_remote_iter_read_event(struct trace_remote_iterator *iter)
> if (ring_buffer_empty_cpu(trace_buffer, cpu))
> return false;
>
> - if (!ring_buffer_peek(trace_buffer, cpu, &iter->ts, &iter->lost_events))
> + if (!__peek_event(iter, cpu, &iter->ts, &iter->lost_events))
> return false;
>
> iter->evt_cpu = cpu;
> @@ -333,7 +508,7 @@ static bool trace_remote_iter_read_event(struct trace_remote_iterator *iter)
> if (ring_buffer_empty_cpu(trace_buffer, cpu))
> continue;
>
> - if (!ring_buffer_peek(trace_buffer, cpu, &ts, &lost_events))
> + if (!__peek_event(iter, cpu, &ts, &lost_events))
> continue;
>
> if (ts >= iter->ts)
> @@ -347,6 +522,20 @@ static bool trace_remote_iter_read_event(struct trace_remote_iterator *iter)
> return iter->ts != U64_MAX;
> }
>
> +static void trace_remote_iter_move(struct trace_remote_iterator *iter)
> +{
> + struct trace_buffer *trace_buffer = iter->remote->trace_buffer;
> +
> + switch (iter->type) {
> + case TRI_CONSUMING:
> + ring_buffer_consume(trace_buffer, iter->evt_cpu, NULL, NULL);
> + break;
> + case TRI_NONCONSUMING:
> + ring_buffer_iter_advance(__get_rb_iter(iter, iter->evt_cpu));
> + break;
> + }
> +}
> +
> static int trace_remote_iter_print_event(struct trace_remote_iterator *iter)
> {
> unsigned long usecs_rem;
> @@ -369,13 +558,14 @@ static int trace_pipe_open(struct inode *inode, struct file *filp)
> {
> struct trace_remote *remote = inode->i_private;
> struct trace_remote_iterator *iter;
> - int cpu = RING_BUFFER_ALL_CPUS;
> -
> - if (inode->i_cdev)
> - cpu = (long)inode->i_cdev - 1;
> + int cpu = tracing_get_cpu(inode);
>
> guard(mutex)(&remote->lock);
> - iter = trace_remote_iter(remote, cpu);
> +
> + iter = trace_remote_iter(remote, cpu, TRI_CONSUMING);
> + if (IS_ERR(iter))
> + return PTR_ERR(iter);
> +
> filp->private_data = iter;
>
> return IS_ERR(iter) ? PTR_ERR(iter) : 0;
> @@ -410,6 +600,8 @@ static ssize_t trace_pipe_read(struct file *filp, char __user *ubuf, size_t cnt,
> if (ret < 0)
> return ret;
>
> + trace_remote_iter_read_start(iter);
> +
> while (trace_remote_iter_read_event(iter)) {
> int prev_len = iter->seq.seq.len;
>
> @@ -418,9 +610,11 @@ static ssize_t trace_pipe_read(struct file *filp, char __user *ubuf, size_t cnt,
> break;
> }
>
> - ring_buffer_consume(trace_buffer, iter->evt_cpu, NULL, NULL);
> + trace_remote_iter_move(iter);
> }
>
> + trace_remote_iter_read_finished(iter);
> +
> goto copy_to_user;
> }
>
> @@ -430,14 +624,123 @@ static const struct file_operations trace_pipe_fops = {
> .release = trace_pipe_release,
> };
>
> +static void *trace_next(struct seq_file *m, void *v, loff_t *pos)
> +{
> + struct trace_remote_iterator *iter = m->private;
> +
> + ++*pos;
> +
> + if (!iter || !trace_remote_iter_read_event(iter))
> + return NULL;
> +
> + trace_remote_iter_move(iter);
> + iter->pos++;
> +
> + return iter;
> +}
> +
> +static void *trace_start(struct seq_file *m, loff_t *pos)
> +{
> + struct trace_remote_iterator *iter = m->private;
> + loff_t i;
> +
FYI, this is where you take locks for iteration of files.
> + if (!iter)
> + return NULL;
> +
> + if (!*pos) {
> + iter->pos = -1;
> + return trace_next(m, NULL, &i);
> + }
> +
> + i = iter->pos;
> + while (i < *pos) {
> + iter = trace_next(m, NULL, &i);
> + if (!iter)
> + return NULL;
> + }
> +
> + return iter;
> +}
> +
> +static int trace_show(struct seq_file *m, void *v)
> +{
> + struct trace_remote_iterator *iter = v;
> +
> + trace_seq_init(&iter->seq);
> +
> + if (trace_remote_iter_print_event(iter)) {
> + seq_printf(m, "[EVENT %d PRINT TOO BIG]\n", iter->evt->id);
> + return 0;
> + }
> +
> + return trace_print_seq(m, &iter->seq);
> +}
> +
> +static void trace_stop(struct seq_file *s, void *v) { }
And stop is where you release the locks.
> +
> +static const struct seq_operations trace_sops = {
> + .start = trace_start,
> + .next = trace_next,
> + .show = trace_show,
> + .stop = trace_stop,
> +};
> +
> +static int trace_open(struct inode *inode, struct file *filp)
> +{
> + struct trace_remote *remote = inode->i_private;
> + struct trace_remote_iterator *iter = NULL;
> + int cpu = tracing_get_cpu(inode);
> + int ret;
> +
> + if (!(filp->f_mode & FMODE_READ))
> + return 0;
> +
> + guard(mutex)(&remote->lock);
> +
> + iter = trace_remote_iter(remote, cpu, TRI_NONCONSUMING);
> + if (IS_ERR(iter))
> + return PTR_ERR(iter);
So if iter is bad we exit out here.
> +
> + ret = seq_open(filp, &trace_sops);
> + if (ret) {
> + trace_remote_iter_free(iter);
> + return ret;
> + }
> +
> + if (iter)
Why test if iter exists here?
> + trace_remote_iter_read_start(iter);
But still, the above grabs locks in the open, where it can return to user
space while still holding the locks? That's a no-no.
You can use the seq file start and stop for locking.
-- Steve
> +
> + ((struct seq_file *)filp->private_data)->private = (void *)iter;
> +
> + return 0;
> +}
> +
> +static int trace_release(struct inode *inode, struct file *filp)
> +{
> + struct trace_remote_iterator *iter;
> +
> + if (!(filp->f_mode & FMODE_READ))
> + return 0;
> +
> + iter = ((struct seq_file *)filp->private_data)->private;
> + seq_release(inode, filp);
> +
> + if (!iter)
> + return 0;
> +
> + guard(mutex)(&iter->remote->lock);
> +
> + trace_remote_iter_read_finished(iter);
> + trace_remote_iter_free(iter);
> +
> + return 0;
> +}
> +
> static ssize_t trace_write(struct file *filp, const char __user *ubuf, size_t cnt, loff_t *ppos)
> {
> struct inode *inode = file_inode(filp);
> struct trace_remote *remote = inode->i_private;
> - int cpu = RING_BUFFER_ALL_CPUS;
> -
> - if (inode->i_cdev)
> - cpu = (long)inode->i_cdev - 1;
> + int cpu = tracing_get_cpu(inode);
>
> guard(mutex)(&remote->lock);
>
> @@ -447,7 +750,11 @@ static ssize_t trace_write(struct file *filp, const char __user *ubuf, size_t cn
> }
>
> static const struct file_operations trace_fops = {
> + .open = trace_open,
> .write = trace_write,
> + .read = seq_read,
> + .read_iter = seq_read_iter,
> + .release = trace_release,
> };
>
> static int trace_remote_init_tracefs(const char *name, struct trace_remote *remote)
> @@ -566,6 +873,7 @@ int trace_remote_register(const char *name, struct trace_remote_callbacks *cbs,
> remote->trace_buffer_size = 7 << 10;
> remote->poll_ms = 100;
> mutex_init(&remote->lock);
> + init_rwsem(&remote->reader_lock);
>
> if (trace_remote_init_tracefs(name, remote)) {
> kfree(remote);
^ permalink raw reply
* Re: [PATCH v11 09/30] tracing: Add events to trace remotes
From: Steven Rostedt @ 2026-02-05 0:40 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <20260131132848.254084-10-vdonnefort@google.com>
On Sat, 31 Jan 2026 13:28:27 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:
> @@ -487,16 +494,19 @@ __peek_event(struct trace_remote_iterator *iter, int cpu, u64 *ts, unsigned long
> static bool trace_remote_iter_read_event(struct trace_remote_iterator *iter)
> {
> struct trace_buffer *trace_buffer = iter->remote->trace_buffer;
> + struct ring_buffer_event *rb_evt;
> int cpu = iter->cpu;
>
> if (cpu != RING_BUFFER_ALL_CPUS) {
> if (ring_buffer_empty_cpu(trace_buffer, cpu))
> return false;
>
> - if (!__peek_event(iter, cpu, &iter->ts, &iter->lost_events))
> + rb_evt = __peek_event(iter, cpu, &iter->ts, &iter->lost_events);
> + if (!rb_evt)
> return false;
>
> iter->evt_cpu = cpu;
> + iter->evt = (struct remote_event_hdr *)ring_buffer_event_data(rb_evt);
BTW, you don't need to typecast the return of ring_buffer_event_data()
as that returns a void pointer.
> return true;
> }
>
> @@ -508,7 +518,8 @@ static bool trace_remote_iter_read_event(struct trace_remote_iterator *iter)
> if (ring_buffer_empty_cpu(trace_buffer, cpu))
> continue;
>
> - if (!__peek_event(iter, cpu, &ts, &lost_events))
> + rb_evt = __peek_event(iter, cpu, &ts, &lost_events);
> + if (!rb_evt)
> continue;
>
> if (ts >= iter->ts)
> @@ -516,6 +527,7 @@ static bool trace_remote_iter_read_event(struct trace_remote_iterator *iter)
>
> iter->ts = ts;
> iter->evt_cpu = cpu;
> + iter->evt = (struct remote_event_hdr *)ring_buffer_event_data(rb_evt);
ditto.
> iter->lost_events = lost_events;
> }
>
Other than that...
Review-by: Steven Rostedt (Google) <rostedt@goodmis.org>
-- Steve
> @@ -536,8 +548,11 @@ static void trace_remote_iter_move(struct trace_remote_iterator *iter)
> }
> }
>
^ permalink raw reply
* Re: [PATCH v6 0/4] tracing: Remove backup instance after read all
From: Masami Hiramatsu @ 2026-02-05 0:41 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Steven Rostedt, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <176991653525.4025429.12655335935351822711.stgit@mhiramat.tok.corp.google.com>
Hi Steve,
Can you review this series?
On Sun, 1 Feb 2026 12:28:55 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> Hi,
>
> Here is the v6 of the series to improve backup instances of
> the persistent ring buffer. The previous version is here:
>
> https://lore.kernel.org/all/176955897718.2786091.11948759407196200082.stgit@mhiramat.tok.corp.google.com/
>
> This version cleanup code and fix typo, according to Steve's
> comments. Also fix to init autoremove_wq only when the readonly
> instance has been made. A major UI change is that the tracing_on
> file is also removed from readonly instance.
>
> Since backup instances are a kind of snapshot of the persistent
> ring buffer, it should be readonly. And if it is readonly
> there is no reason to keep it after reading all data via trace_pipe
> because the data has been consumed. But user should be able to remove
> the readonly instance by rmdir or truncating `trace` file.
>
> Thus, [2/4] makes backup instances readonly (not able to write any
> events, cleanup trace, change buffer size). Also, [3/4] removes the
> backup instance after consuming all data via trace_pipe.
> With this improvements, even if we makes a backup instance (using
> the same amount of memory of the persistent ring buffer), it will
> be removed after reading the data automatically.
>
> ---
>
> Masami Hiramatsu (Google) (4):
> tracing: Reset last_boot_info if ring buffer is reset
> tracing: Make the backup instance non-reusable
> tracing: Remove the backup instance automatically after read
> tracing/Documentation: Add a section about backup instance
>
>
> Documentation/trace/debugging.rst | 19 ++++
> kernel/trace/trace.c | 162 ++++++++++++++++++++++++++++++-------
> kernel/trace/trace.h | 13 +++
> kernel/trace/trace_boot.c | 5 +
> kernel/trace/trace_events.c | 76 ++++++++++-------
> 5 files changed, 209 insertions(+), 66 deletions(-)
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH v3 0/2] bootconfig: Handle an empty value
From: Masami Hiramatsu (Google) @ 2026-02-05 0:46 UTC (permalink / raw)
To: Steven Rostedt, linux-trace-kernel; +Cc: Julius Werner, Masami Hiramatsu, LKML
This is the 3rd version of the update of the bootconfig parser to
make it handle an empty value which is terminated by a newline or
a comment. The previous version is here:
https://lore.kernel.org/all/177019401833.80694.554894321526842218.stgit@devnote2/
This version fixes typos and add 2 bad examples which were
previously good[1/2].
Thank you,
---
Masami Hiramatsu (Google) (2):
bootconfig: Terminate value search if it hits a newline
bootconfig: Check the parsed output of the good examples
.../samples/bad-array-after-comment.bconf | 4 ++++
.../samples/bad-array-in-next-line.bconf | 4 ++++
.../samples/exp-good-array-space-comment.bconf | 1 +
.../samples/exp-good-comment-after-value.bconf | 1 +
.../bootconfig/samples/exp-good-mixed-append.bconf | 2 ++
tools/bootconfig/samples/exp-good-mixed-kv1.bconf | 2 ++
tools/bootconfig/samples/exp-good-mixed-kv2.bconf | 2 ++
tools/bootconfig/samples/exp-good-mixed-kv3.bconf | 5 +++++
.../samples/exp-good-mixed-override.bconf | 2 ++
tools/bootconfig/samples/exp-good-override.bconf | 4 ++++
tools/bootconfig/samples/exp-good-printables.bconf | 2 ++
tools/bootconfig/samples/exp-good-simple.bconf | 8 ++++++++
tools/bootconfig/samples/exp-good-single.bconf | 3 +++
.../samples/exp-good-space-after-value.bconf | 1 +
tools/bootconfig/samples/exp-good-tree.bconf | 8 ++++++++
.../samples/good-array-space-comment.bconf | 3 +--
tools/bootconfig/test-bootconfig.sh | 3 +++
17 files changed, 53 insertions(+), 2 deletions(-)
create mode 100644 tools/bootconfig/samples/bad-array-after-comment.bconf
create mode 100644 tools/bootconfig/samples/bad-array-in-next-line.bconf
create mode 100644 tools/bootconfig/samples/exp-good-array-space-comment.bconf
create mode 100644 tools/bootconfig/samples/exp-good-comment-after-value.bconf
create mode 100644 tools/bootconfig/samples/exp-good-mixed-append.bconf
create mode 100644 tools/bootconfig/samples/exp-good-mixed-kv1.bconf
create mode 100644 tools/bootconfig/samples/exp-good-mixed-kv2.bconf
create mode 100644 tools/bootconfig/samples/exp-good-mixed-kv3.bconf
create mode 100644 tools/bootconfig/samples/exp-good-mixed-override.bconf
create mode 100644 tools/bootconfig/samples/exp-good-override.bconf
create mode 100644 tools/bootconfig/samples/exp-good-printables.bconf
create mode 100644 tools/bootconfig/samples/exp-good-simple.bconf
create mode 100644 tools/bootconfig/samples/exp-good-single.bconf
create mode 100644 tools/bootconfig/samples/exp-good-space-after-value.bconf
create mode 100644 tools/bootconfig/samples/exp-good-tree.bconf
--
Masami Hiramatsu (Google) <mhiramat@google.com>
^ permalink raw reply
* [PATCH v3 1/2] bootconfig: Terminate value search if it hits a newline
From: Masami Hiramatsu (Google) @ 2026-02-05 0:46 UTC (permalink / raw)
To: Steven Rostedt, linux-trace-kernel; +Cc: Julius Werner, Masami Hiramatsu, LKML
In-Reply-To: <177025237564.14982.14899224362754775890.stgit@devnote2>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Terminate the value search for a key if it hits a newline and make
the value empty.
When we pass a bootconfig with an empty value terminated by the
newline, like below::
foo =
bar = value
Current bootconfig interprets it as a single entry::
foo = "bar = value";
The Documentation/admin-guide/bootconfig.rst defines the value
itself is terminated by newline:
The value has to be terminated by semi-colon (``;``) or newline (``\n``).
but it does not define when the value search is terminated.
This changes the behavior to be more line-oriented, so that it is
clearer in how it works.
- The value search of key-value pair will be terminated by a comment
or newline.
- The value search of an array will continue beyond comments and
newlines.
Thus, with this update, the above example is interpreted as::
foo = "";
bar = "value";
And the below example will cause a syntax error because "bar" is expected
as a key but it has ','.
foo =
bar, buz
According to this change, one wrong example config is updated.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
---
Changes in v3:
- Fix typos.
- Add bad examples (which was good previously).
Changes in v2:
- Fix to handle multi-line array case correctly.
- Make this as a spec update, not fix.
---
| 4 ++++
.../samples/bad-array-in-next-line.bconf | 4 ++++
| 3 +--
3 files changed, 9 insertions(+), 2 deletions(-)
create mode 100644 tools/bootconfig/samples/bad-array-after-comment.bconf
create mode 100644 tools/bootconfig/samples/bad-array-in-next-line.bconf
diff --git a/Documentation/admin-guide/bootconfig.rst b/Documentation/admin-guide/bootconfig.rst
index 7a86042c9b6d..f712758472d5 100644
--- a/Documentation/admin-guide/bootconfig.rst
+++ b/Documentation/admin-guide/bootconfig.rst
@@ -20,18 +20,26 @@ Config File Syntax
The boot config syntax is a simple structured key-value. Each key consists
of dot-connected-words, and key and value are connected by ``=``. The value
-has to be terminated by semi-colon (``;``) or newline (``\n``).
-For array value, array entries are separated by comma (``,``). ::
-
- KEY[.WORD[...]] = VALUE[, VALUE2[...]][;]
-
-Unlike the kernel command line syntax, spaces are OK around the comma and ``=``.
+string has to be terminated by the following delimiters described below.
Each key word must contain only alphabets, numbers, dash (``-``) or underscore
(``_``). And each value only contains printable characters or spaces except
for delimiters such as semi-colon (``;``), new-line (``\n``), comma (``,``),
hash (``#``) and closing brace (``}``).
+If the ``=`` is followed by whitespace up to one of these delimiters, the
+key is assigned an empty value.
+
+For arrays, the array values are comma (``,``) separated, and comments and
+line breaks with newline (``\n``) are allowed between array values for
+readability. Thus the first entry of the array must be on the same line as
+the key.::
+
+ KEY[.WORD[...]] = VALUE[, VALUE2[...]][;]
+
+Unlike the kernel command line syntax, white spaces (including tabs) are
+ignored around the comma and ``=``.
+
If you want to use those delimiters in a value, you can use either double-
quotes (``"VALUE"``) or single-quotes (``'VALUE'``) to quote it. Note that
you can not escape these quotes.
@@ -138,8 +146,8 @@ This is parsed as below::
foo = value
bar = 1, 2, 3
-Note that you can not put a comment between value and delimiter(``,`` or
-``;``). This means following config has a syntax error ::
+Note that you can NOT put a comment or a newline between value and delimiter
+(``,`` or ``;``). This means following config has a syntax error ::
key = 1 # comment
,2
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 81f29c29f47b..449369a60846 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -557,17 +557,13 @@ static int __init __xbc_close_brace(char *p)
/*
* Return delimiter or error, no node added. As same as lib/cmdline.c,
* you can use " around spaces, but can't escape " for value.
+ * *@__v must point real value string. (not including spaces before value.)
*/
static int __init __xbc_parse_value(char **__v, char **__n)
{
char *p, *v = *__v;
int c, quotes = 0;
- v = skip_spaces(v);
- while (*v == '#') {
- v = skip_comment(v);
- v = skip_spaces(v);
- }
if (*v == '"' || *v == '\'') {
quotes = *v;
v++;
@@ -617,6 +613,13 @@ static int __init xbc_parse_array(char **__v)
last_parent = xbc_node_get_child(last_parent);
do {
+ /* Search the next array value beyond comments and empty lines */
+ next = skip_spaces(*__v);
+ while (*next == '#') {
+ next = skip_comment(next);
+ next = skip_spaces(next);
+ }
+ *__v = next;
c = __xbc_parse_value(__v, &next);
if (c < 0)
return c;
@@ -701,9 +704,17 @@ static int __init xbc_parse_kv(char **k, char *v, int op)
if (ret)
return ret;
- c = __xbc_parse_value(&v, &next);
- if (c < 0)
- return c;
+ v = skip_spaces_until_newline(v);
+ /* If there is a comment, this has an empty value. */
+ if (*v == '#') {
+ next = skip_comment(v);
+ *v = '\0';
+ c = '\n';
+ } else {
+ c = __xbc_parse_value(&v, &next);
+ if (c < 0)
+ return c;
+ }
child = xbc_node_get_child(last_parent);
if (child && xbc_node_is_value(child)) {
diff --git a/tools/bootconfig/samples/bad-array-after-comment.bconf b/tools/bootconfig/samples/bad-array-after-comment.bconf
new file mode 100644
index 000000000000..fdb6d4e04447
--- /dev/null
+++ b/tools/bootconfig/samples/bad-array-after-comment.bconf
@@ -0,0 +1,4 @@
+# the first array value must be on the same line as the key
+key = # comment
+ value1,
+ value2
diff --git a/tools/bootconfig/samples/bad-array-in-next-line.bconf b/tools/bootconfig/samples/bad-array-in-next-line.bconf
new file mode 100644
index 000000000000..95a99a3bde8c
--- /dev/null
+++ b/tools/bootconfig/samples/bad-array-in-next-line.bconf
@@ -0,0 +1,4 @@
+# the first array value must be on the same line as the key
+key =
+ value1,
+ value2
diff --git a/tools/bootconfig/samples/good-array-space-comment.bconf b/tools/bootconfig/samples/good-array-space-comment.bconf
index 45b938dc0695..416fa2ed4109 100644
--- a/tools/bootconfig/samples/good-array-space-comment.bconf
+++ b/tools/bootconfig/samples/good-array-space-comment.bconf
@@ -1,4 +1,3 @@
-key = # comment
- "value1", # comment1
+key = "value1", # comment1
"value2" , # comment2
"value3"
^ permalink raw reply related
* [PATCH v3 2/2] bootconfig: Check the parsed output of the good examples
From: Masami Hiramatsu (Google) @ 2026-02-05 0:46 UTC (permalink / raw)
To: Steven Rostedt, linux-trace-kernel; +Cc: Julius Werner, Masami Hiramatsu, LKML
In-Reply-To: <177025237564.14982.14899224362754775890.stgit@devnote2>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Check whether the parsed output of the good example configs are
the same as expected.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Tested-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
| 1 +
| 1 +
.../bootconfig/samples/exp-good-mixed-append.bconf | 2 ++
tools/bootconfig/samples/exp-good-mixed-kv1.bconf | 2 ++
tools/bootconfig/samples/exp-good-mixed-kv2.bconf | 2 ++
tools/bootconfig/samples/exp-good-mixed-kv3.bconf | 5 +++++
.../samples/exp-good-mixed-override.bconf | 2 ++
tools/bootconfig/samples/exp-good-override.bconf | 4 ++++
tools/bootconfig/samples/exp-good-printables.bconf | 2 ++
tools/bootconfig/samples/exp-good-simple.bconf | 8 ++++++++
tools/bootconfig/samples/exp-good-single.bconf | 3 +++
.../samples/exp-good-space-after-value.bconf | 1 +
tools/bootconfig/samples/exp-good-tree.bconf | 8 ++++++++
tools/bootconfig/test-bootconfig.sh | 3 +++
14 files changed, 44 insertions(+)
create mode 100644 tools/bootconfig/samples/exp-good-array-space-comment.bconf
create mode 100644 tools/bootconfig/samples/exp-good-comment-after-value.bconf
create mode 100644 tools/bootconfig/samples/exp-good-mixed-append.bconf
create mode 100644 tools/bootconfig/samples/exp-good-mixed-kv1.bconf
create mode 100644 tools/bootconfig/samples/exp-good-mixed-kv2.bconf
create mode 100644 tools/bootconfig/samples/exp-good-mixed-kv3.bconf
create mode 100644 tools/bootconfig/samples/exp-good-mixed-override.bconf
create mode 100644 tools/bootconfig/samples/exp-good-override.bconf
create mode 100644 tools/bootconfig/samples/exp-good-printables.bconf
create mode 100644 tools/bootconfig/samples/exp-good-simple.bconf
create mode 100644 tools/bootconfig/samples/exp-good-single.bconf
create mode 100644 tools/bootconfig/samples/exp-good-space-after-value.bconf
create mode 100644 tools/bootconfig/samples/exp-good-tree.bconf
--git a/tools/bootconfig/samples/exp-good-array-space-comment.bconf b/tools/bootconfig/samples/exp-good-array-space-comment.bconf
new file mode 100644
index 000000000000..8d3278fa6af5
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-array-space-comment.bconf
@@ -0,0 +1 @@
+key = "value1", "value2", "value3";
--git a/tools/bootconfig/samples/exp-good-comment-after-value.bconf b/tools/bootconfig/samples/exp-good-comment-after-value.bconf
new file mode 100644
index 000000000000..a8e8450db3c0
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-comment-after-value.bconf
@@ -0,0 +1 @@
+key = "value";
diff --git a/tools/bootconfig/samples/exp-good-mixed-append.bconf b/tools/bootconfig/samples/exp-good-mixed-append.bconf
new file mode 100644
index 000000000000..c2b407901ddd
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-mixed-append.bconf
@@ -0,0 +1,2 @@
+key = "foo", "bar";
+keyx.subkey = "value";
diff --git a/tools/bootconfig/samples/exp-good-mixed-kv1.bconf b/tools/bootconfig/samples/exp-good-mixed-kv1.bconf
new file mode 100644
index 000000000000..8346287d9251
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-mixed-kv1.bconf
@@ -0,0 +1,2 @@
+key = "value";
+key.subkey = "another-value";
diff --git a/tools/bootconfig/samples/exp-good-mixed-kv2.bconf b/tools/bootconfig/samples/exp-good-mixed-kv2.bconf
new file mode 100644
index 000000000000..40c6232c7cdd
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-mixed-kv2.bconf
@@ -0,0 +1,2 @@
+key = "another-value";
+key.subkey = "value";
diff --git a/tools/bootconfig/samples/exp-good-mixed-kv3.bconf b/tools/bootconfig/samples/exp-good-mixed-kv3.bconf
new file mode 100644
index 000000000000..8368a7bef60a
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-mixed-kv3.bconf
@@ -0,0 +1,5 @@
+key = "value";
+key {
+ subkey1;
+ subkey2 = "foo";
+}
diff --git a/tools/bootconfig/samples/exp-good-mixed-override.bconf b/tools/bootconfig/samples/exp-good-mixed-override.bconf
new file mode 100644
index 000000000000..58757712ca45
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-mixed-override.bconf
@@ -0,0 +1,2 @@
+key = "value2";
+key.foo = "bar";
diff --git a/tools/bootconfig/samples/exp-good-override.bconf b/tools/bootconfig/samples/exp-good-override.bconf
new file mode 100644
index 000000000000..00bbd30e99ae
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-override.bconf
@@ -0,0 +1,4 @@
+key {
+ word = "2", "3";
+ new.word = "new";
+}
diff --git a/tools/bootconfig/samples/exp-good-printables.bconf b/tools/bootconfig/samples/exp-good-printables.bconf
new file mode 100644
index 000000000000..5981d304eacb
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-printables.bconf
@@ -0,0 +1,2 @@
+key = "
+\v\f !#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz{|}~";
diff --git a/tools/bootconfig/samples/exp-good-simple.bconf b/tools/bootconfig/samples/exp-good-simple.bconf
new file mode 100644
index 000000000000..d17f39421c86
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-simple.bconf
@@ -0,0 +1,8 @@
+key {
+ word1 = "1";
+ word2 = "2";
+ word3 = "3";
+ word4 = "4";
+ word5 = "5";
+ word6 = "6";
+}
diff --git a/tools/bootconfig/samples/exp-good-single.bconf b/tools/bootconfig/samples/exp-good-single.bconf
new file mode 100644
index 000000000000..01196910d7f4
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-single.bconf
@@ -0,0 +1,3 @@
+key = "1";
+key2 = "2";
+key3 = "alpha", "beta";
diff --git a/tools/bootconfig/samples/exp-good-space-after-value.bconf b/tools/bootconfig/samples/exp-good-space-after-value.bconf
new file mode 100644
index 000000000000..a8e8450db3c0
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-space-after-value.bconf
@@ -0,0 +1 @@
+key = "value";
diff --git a/tools/bootconfig/samples/exp-good-tree.bconf b/tools/bootconfig/samples/exp-good-tree.bconf
new file mode 100644
index 000000000000..b711d38d86fd
--- /dev/null
+++ b/tools/bootconfig/samples/exp-good-tree.bconf
@@ -0,0 +1,8 @@
+key {
+ word.tree.value = "0";
+ word2.tree.value = "1", "2";
+}
+other.tree {
+ value = "2";
+ value2 = "3";
+}
diff --git a/tools/bootconfig/test-bootconfig.sh b/tools/bootconfig/test-bootconfig.sh
index 7594659af1e1..be9bd18b1d56 100755
--- a/tools/bootconfig/test-bootconfig.sh
+++ b/tools/bootconfig/test-bootconfig.sh
@@ -179,6 +179,9 @@ done
echo "=== expected success cases ==="
for i in samples/good-* ; do
xpass $BOOTCONF -a $i $INITRD
+ x="samples/exp-"`basename $i`
+ $BOOTCONF $i > $TEMPCONF
+ xpass diff $x $TEMPCONF
done
^ permalink raw reply related
* Re: [PATCH v2] tracing: Fix ftrace event field alignments
From: Masami Hiramatsu @ 2026-02-05 0:49 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Linux Trace Kernel, Masami Hiramatsu, Mathieu Desnoyers,
Mark Rutland, jempty.liang
In-Reply-To: <20260204113628.53faec78@gandalf.local.home>
On Wed, 4 Feb 2026 11:36:28 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> The fields of ftrace specific events (events used to save ftrace internal
> events like function traces and trace_printk) are generated similarly to
> how normal trace event fields are generated. That is, the fields are added
> to a trace_events_fields array that saves the name, offset, size,
> alignment and signness of the field. It is used to produce the output in
> the format file in tracefs so that tooling knows how to parse the binary
> data of the trace events.
>
> The issue is that some of the ftrace event structures are packed. The
> function graph exit event structures are one of them. The 64 bit calltime
> and rettime fields end up 4 byte aligned, but the algorithm to show to
> userspace shows them as 8 byte aligned.
>
> The macros that create the ftrace events has one for embedded structure
> fields. There's two macros for theses fields:
>
> __field_desc() and __field_packed()
>
> The difference of the latter macro is that it treats the field as packed.
>
> Rename that field to __field_desc_packed() and create replace the
> __field_packed() to be a normal field that is packed and have the calltime
> and rettime use those.
>
> This showed up on 32bit architectures for function graph time fields. It
> had:
>
> ~# cat /sys/kernel/tracing/events/ftrace/funcgraph_exit/format
> [..]
> field:unsigned long func; offset:8; size:4; signed:0;
> field:unsigned int depth; offset:12; size:4; signed:0;
> field:unsigned int overrun; offset:16; size:4; signed:0;
> field:unsigned long long calltime; offset:24; size:8; signed:0;
> field:unsigned long long rettime; offset:32; size:8; signed:0;
>
> Notice that overrun is at offset 16 with size 4, where in the structure
> calltime is at offset 20 (16 + 4), but it shows the offset at 24. That's
> because it used the alignment of unsigned long long when used as a
> declaration and not as a member of a structure where it would be aligned
> by word size (in this case 4).
>
> By using the proper structure alignment, the format has it at the correct
> offset:
>
> ~# cat /sys/kernel/tracing/events/ftrace/funcgraph_exit/format
> [..]
> field:unsigned long func; offset:8; size:4; signed:0;
> field:unsigned int depth; offset:12; size:4; signed:0;
> field:unsigned int overrun; offset:16; size:4; signed:0;
> field:unsigned long long calltime; offset:20; size:8; signed:0;
> field:unsigned long long rettime; offset:28; size:8; signed:0;
>
> Cc: stable@vger.kernel.org
> Fixes: 04ae87a52074e ("ftrace: Rework event_create_dir()")
> Reported-by: "jempty.liang" <imntjempty@163.com>
> Closes: https://lore.kernel.org/all/20260130015740.212343-1-imntjempty@163.com/
> Closes: https://lore.kernel.org/all/20260202123342.2544795-1-imntjempty@163.com/
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thanks,
> ---
> Changes since v1: https://patch.msgid.link/20260202113024.61d5c1fd@gandalf.local.home
>
> - Instead of using an alignment for structures, create a new macro
> called __field_desc_packed() to use for packed structure fields
> and have __field_packed() be used for normal packed fields.
>
> kernel/trace/trace.h | 7 +++++--
> kernel/trace/trace_entries.h | 32 ++++++++++++++++----------------
> kernel/trace/trace_export.c | 21 +++++++++++++++------
> 3 files changed, 36 insertions(+), 24 deletions(-)
>
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index b6d42fe06115..c11edec5d8f5 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -68,14 +68,17 @@ enum trace_type {
> #undef __field_fn
> #define __field_fn(type, item) type item;
>
> +#undef __field_packed
> +#define __field_packed(type, item) type item;
> +
> #undef __field_struct
> #define __field_struct(type, item) __field(type, item)
>
> #undef __field_desc
> #define __field_desc(type, container, item)
>
> -#undef __field_packed
> -#define __field_packed(type, container, item)
> +#undef __field_desc_packed
> +#define __field_desc_packed(type, container, item)
>
> #undef __array
> #define __array(type, item, size) type item[size];
> diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h
> index f6a8d29c0d76..54417468fdeb 100644
> --- a/kernel/trace/trace_entries.h
> +++ b/kernel/trace/trace_entries.h
> @@ -79,8 +79,8 @@ FTRACE_ENTRY(funcgraph_entry, ftrace_graph_ent_entry,
>
> F_STRUCT(
> __field_struct( struct ftrace_graph_ent, graph_ent )
> - __field_packed( unsigned long, graph_ent, func )
> - __field_packed( unsigned long, graph_ent, depth )
> + __field_desc_packed(unsigned long, graph_ent, func )
> + __field_desc_packed(unsigned long, graph_ent, depth )
> __dynamic_array(unsigned long, args )
> ),
>
> @@ -96,9 +96,9 @@ FTRACE_ENTRY_PACKED(fgraph_retaddr_entry, fgraph_retaddr_ent_entry,
>
> F_STRUCT(
> __field_struct( struct fgraph_retaddr_ent, graph_rent )
> - __field_packed( unsigned long, graph_rent.ent, func )
> - __field_packed( unsigned long, graph_rent.ent, depth )
> - __field_packed( unsigned long, graph_rent, retaddr )
> + __field_desc_packed( unsigned long, graph_rent.ent, func )
> + __field_desc_packed( unsigned long, graph_rent.ent, depth )
> + __field_desc_packed( unsigned long, graph_rent, retaddr )
> __dynamic_array(unsigned long, args )
> ),
>
> @@ -123,12 +123,12 @@ FTRACE_ENTRY_PACKED(funcgraph_exit, ftrace_graph_ret_entry,
>
> F_STRUCT(
> __field_struct( struct ftrace_graph_ret, ret )
> - __field_packed( unsigned long, ret, func )
> - __field_packed( unsigned long, ret, retval )
> - __field_packed( unsigned int, ret, depth )
> - __field_packed( unsigned int, ret, overrun )
> - __field(unsigned long long, calltime )
> - __field(unsigned long long, rettime )
> + __field_desc_packed( unsigned long, ret, func )
> + __field_desc_packed( unsigned long, ret, retval )
> + __field_desc_packed( unsigned int, ret, depth )
> + __field_desc_packed( unsigned int, ret, overrun )
> + __field_packed(unsigned long long, calltime)
> + __field_packed(unsigned long long, rettime )
> ),
>
> F_printk("<-- %ps (%u) (start: %llx end: %llx) over: %u retval: %lx",
> @@ -146,11 +146,11 @@ FTRACE_ENTRY_PACKED(funcgraph_exit, ftrace_graph_ret_entry,
>
> F_STRUCT(
> __field_struct( struct ftrace_graph_ret, ret )
> - __field_packed( unsigned long, ret, func )
> - __field_packed( unsigned int, ret, depth )
> - __field_packed( unsigned int, ret, overrun )
> - __field(unsigned long long, calltime )
> - __field(unsigned long long, rettime )
> + __field_desc_packed( unsigned long, ret, func )
> + __field_desc_packed( unsigned int, ret, depth )
> + __field_desc_packed( unsigned int, ret, overrun )
> + __field_packed(unsigned long long, calltime )
> + __field_packed(unsigned long long, rettime )
> ),
>
> F_printk("<-- %ps (%u) (start: %llx end: %llx) over: %u",
> diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
> index 1698fc22afa0..32a42ef31855 100644
> --- a/kernel/trace/trace_export.c
> +++ b/kernel/trace/trace_export.c
> @@ -42,11 +42,14 @@ static int ftrace_event_register(struct trace_event_call *call,
> #undef __field_fn
> #define __field_fn(type, item) type item;
>
> +#undef __field_packed
> +#define __field_packed(type, item) type item;
> +
> #undef __field_desc
> #define __field_desc(type, container, item) type item;
>
> -#undef __field_packed
> -#define __field_packed(type, container, item) type item;
> +#undef __field_desc_packed
> +#define __field_desc_packed(type, container, item) type item;
>
> #undef __array
> #define __array(type, item, size) type item[size];
> @@ -104,11 +107,14 @@ static void __always_unused ____ftrace_check_##name(void) \
> #undef __field_fn
> #define __field_fn(_type, _item) __field_ext(_type, _item, FILTER_TRACE_FN)
>
> +#undef __field_packed
> +#define __field_packed(_type, _item) __field_ext_packed(_type, _item, FILTER_OTHER)
> +
> #undef __field_desc
> #define __field_desc(_type, _container, _item) __field_ext(_type, _item, FILTER_OTHER)
>
> -#undef __field_packed
> -#define __field_packed(_type, _container, _item) __field_ext_packed(_type, _item, FILTER_OTHER)
> +#undef __field_desc_packed
> +#define __field_desc_packed(_type, _container, _item) __field_ext_packed(_type, _item, FILTER_OTHER)
>
> #undef __array
> #define __array(_type, _item, _len) { \
> @@ -146,11 +152,14 @@ static struct trace_event_fields ftrace_event_fields_##name[] = { \
> #undef __field_fn
> #define __field_fn(type, item)
>
> +#undef __field_packed
> +#define __field_packed(type, item)
> +
> #undef __field_desc
> #define __field_desc(type, container, item)
>
> -#undef __field_packed
> -#define __field_packed(type, container, item)
> +#undef __field_desc_packed
> +#define __field_desc_packed(type, container, item)
>
> #undef __array
> #define __array(type, item, len)
> --
> 2.51.0
>
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v11 13/30] tracing: Introduce simple_ring_buffer
From: Steven Rostedt @ 2026-02-05 1:06 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <20260131132848.254084-14-vdonnefort@google.com>
On Sat, 31 Jan 2026 13:28:31 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:
\
> +/**
> + * simple_ring_buffer_swap_reader_page - Swap ring-buffer head with the reader
> + *
> + * This function enables consuming reading. It ensures the current head page will not be overwritten
> + * and can be safely read.
> + *
> + * @cpu_buffer: A simple_rb_per_cpu
And if you're going to do kerneldoc, you need to do it correctly ;-)
You put the description before the parameters.
> + *
> + * Returns 0 on success, -ENODEV if @cpu_buffer was unloaded or -EBUSY if we failed to catch the
> + * head page.
> + */
> +int simple_ring_buffer_swap_reader_page(struct simple_rb_per_cpu *cpu_buffer)
> +{
> + struct simple_buffer_page *last, *head, *reader;
> + unsigned long overrun;
> + int retry = 8;
> + int ret;
> +
> + if (!simple_rb_loaded(cpu_buffer))
> + return -ENODEV;
> +
> + reader = cpu_buffer->reader_page;
> +
> + do {
> + /* Run after the writer to find the head */
> + ret = simple_rb_find_head(cpu_buffer);
> + if (ret)
> + return ret;
> +
> + head = cpu_buffer->head_page;
> +
> + /* Connect the reader page around the header page */
> + reader->link.next = head->link.next;
> + reader->link.prev = head->link.prev;
> +
> + /* The last page before the head */
> + last = simple_bpage_from_link(head->link.prev);
> +
> + /* The reader page points to the new header page */
> + simple_bpage_set_head_link(reader);
> +
> + overrun = cpu_buffer->meta->overrun;
> + } while (!simple_bpage_unset_head_link(last, reader, SIMPLE_RB_LINK_NORMAL) && retry--);
> +
> + if (!retry)
> + return -EINVAL;
> +
> + cpu_buffer->head_page = simple_bpage_from_link(reader->link.next);
> + cpu_buffer->head_page->link.prev = &reader->link;
> + cpu_buffer->reader_page = head;
> + cpu_buffer->meta->reader.lost_events = overrun - cpu_buffer->last_overrun;
> + cpu_buffer->meta->reader.id = cpu_buffer->reader_page->id;
> + cpu_buffer->last_overrun = overrun;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(simple_ring_buffer_swap_reader_page);
> +
> +static struct simple_buffer_page *simple_rb_move_tail(struct simple_rb_per_cpu *cpu_buffer)
> +{
> + struct simple_buffer_page *tail, *new_tail;
> +
> + tail = cpu_buffer->tail_page;
> + new_tail = simple_bpage_next_page(tail);
> +
> + if (simple_bpage_unset_head_link(tail, new_tail, SIMPLE_RB_LINK_HEAD_MOVING)) {
> + /*
> + * Oh no! we've caught the head. There is none anymore and
> + * swap_reader will spin until we set the new one. Overrun must
> + * be written first, to make sure we report the correct number
> + * of lost events.
> + */
> + simple_rb_meta_inc(cpu_buffer->meta->overrun, new_tail->entries);
> + simple_rb_meta_inc(cpu_buffer->meta->pages_lost, 1);
> +
> + simple_bpage_set_head_link(new_tail);
> + simple_bpage_set_normal_link(tail);
> + }
> +
> + simple_bpage_reset(new_tail);
> + cpu_buffer->tail_page = new_tail;
> +
> + simple_rb_meta_inc(cpu_buffer->meta->pages_touched, 1);
> +
> + return new_tail;
> +}
> +
> +static unsigned long rb_event_size(unsigned long length)
> +{
> + struct ring_buffer_event *event;
> +
> + return length + RB_EVNT_HDR_SIZE + sizeof(event->array[0]);
> +}
> +
> +static struct ring_buffer_event *
> +rb_event_add_ts_extend(struct ring_buffer_event *event, u64 delta)
> +{
> + event->type_len = RINGBUF_TYPE_TIME_EXTEND;
> + event->time_delta = delta & TS_MASK;
> + event->array[0] = delta >> TS_SHIFT;
> +
> + return (struct ring_buffer_event *)((unsigned long)event + 8);
> +}
> +
> +static struct ring_buffer_event *
> +simple_rb_reserve_next(struct simple_rb_per_cpu *cpu_buffer, unsigned long length, u64 timestamp)
> +{
> + unsigned long ts_ext_size = 0, event_size = rb_event_size(length);
> + struct simple_buffer_page *tail = cpu_buffer->tail_page;
> + struct ring_buffer_event *event;
> + u32 write, prev_write;
> + u64 time_delta;
> +
> + time_delta = timestamp - cpu_buffer->write_stamp;
> +
> + if (test_time_stamp(time_delta))
> + ts_ext_size = 8;
> +
> + prev_write = tail->write;
> + write = prev_write + event_size + ts_ext_size;
> +
> + if (unlikely(write > (PAGE_SIZE - BUF_PAGE_HDR_SIZE)))
> + tail = simple_rb_move_tail(cpu_buffer);
> +
> + if (!tail->entries) {
> + tail->page->time_stamp = timestamp;
> + time_delta = 0;
> + ts_ext_size = 0;
> + write = event_size;
> + prev_write = 0;
> + }
> +
> + tail->write = write;
> + tail->entries++;
> +
> + cpu_buffer->write_stamp = timestamp;
> +
> + event = (struct ring_buffer_event *)(tail->page->data + prev_write);
> + if (ts_ext_size) {
> + event = rb_event_add_ts_extend(event, time_delta);
> + time_delta = 0;
> + }
> +
> + event->type_len = 0;
> + event->time_delta = time_delta;
> + event->array[0] = event_size - RB_EVNT_HDR_SIZE;
> +
> + return event;
> +}
> +
> +/**
> + * simple_ring_buffer_reserve - Reserve an entry in @cpu_buffer
> + *
And you don't leave a space between the one line description and the
arguments.
> + * @cpu_buffer: A simple_rb_per_cpu
> + * @length: Size of the entry in bytes
> + * @timestamp: Timestamp of the entry
> + *
> + * Returns the address of the entry where to write data or NULL
> + */
> +void *simple_ring_buffer_reserve(struct simple_rb_per_cpu *cpu_buffer, unsigned long length,
> + u64 timestamp)
> +{
> + struct ring_buffer_event *rb_event;
> +
> + if (cmpxchg(&cpu_buffer->status, SIMPLE_RB_READY, SIMPLE_RB_WRITING) != SIMPLE_RB_READY)
> + return NULL;
> +
> + rb_event = simple_rb_reserve_next(cpu_buffer, length, timestamp);
> +
> + return &rb_event->array[1];
> +}
> +EXPORT_SYMBOL_GPL(simple_ring_buffer_reserve);
> +
Other than that:
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
-- Steve
^ permalink raw reply
* Re: [PATCH v11 14/30] tracing: Add a trace remote module for testing
From: Steven Rostedt @ 2026-02-05 1:32 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <20260131132848.254084-15-vdonnefort@google.com>
On Sat, 31 Jan 2026 13:28:32 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:
> Add a module to help testing the tracefs support for trace remotes. This
> module:
>
> * Use simple_ring_buffer to write into a ring-buffer.
> * Declare a single "selftest" event that can be triggered from
> user-space.
> * Register a "test" trace remote.
>
> This is intended to be used by trace remote selftests.
>
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
-- Steve
^ 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