* Re: [PATCHv2 bpf-next 22/23] selftests/bpf: Add tracing multi attach benchmark test
From: Jiri Olsa @ 2026-03-05 14:01 UTC (permalink / raw)
To: Leon Hwang
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: <30ebc83a-8f17-44fc-89a6-037e826f6c70@linux.dev>
On Thu, Mar 05, 2026 at 03:30:01PM +0800, Leon Hwang wrote:
SNIP
> > +/*
> > + * Skip several kernel symbols that might not be safe or could cause delays.
> > + */
> > +static bool skip_symbol(char *name)
> > +{
> > + if (!strcmp(name, "arch_cpu_idle"))
> > + return true;
> > + if (!strcmp(name, "default_idle"))
> > + return true;
> > + if (!strncmp(name, "rcu_", 4))
> > + return true;
> > + if (!strcmp(name, "bpf_dispatcher_xdp_func"))
> > + return true;
> > + if (strstr(name, "rcu"))
> > + return true;
> > + if (strstr(name, "trace"))
> > + return true;
> > + if (strstr(name, "irq"))
> > + return true;
> > + if (strstr(name, "bpf_lsm_"))
> > + return true;
> > + if (!strcmp(name, "migrate_enable"))
> > + return true;
> > + if (!strcmp(name, "migrate_disable"))
> > + return true;
> > + if (!strcmp(name, "preempt_count_sub"))
> > + return true;
> > + if (!strcmp(name, "preempt_count_add"))
> > + return true;
> > + return false;
> > +}
>
> These names intersect with the list in trace_helpers.c::skip_entry().
>
> It would be better to move this list to trace_helpers.c.
ah right, will check
>
> > +
> > +#define MAX_BPF_FUNC_ARGS 12
> > +
> > +static bool btf_type_is_modifier(const struct btf_type *t)
> > +{
> > + switch (BTF_INFO_KIND(t->info)) {
> > + case BTF_KIND_TYPEDEF:
> > + case BTF_KIND_VOLATILE:
> > + case BTF_KIND_CONST:
> > + case BTF_KIND_RESTRICT:
> > + case BTF_KIND_TYPE_TAG:
> > + return true;
> > + }
> > + return false;
> > +}
> > +
> > +static bool is_allowed_func(const struct btf *btf, const struct btf_type *t)
> > +{
> > + const struct btf_type *proto;
> > + const struct btf_param *args;
> > + __u32 i, nargs;
> > + __s64 ret;
> > +
> > + proto = btf_type_by_id(btf, t->type);
> > + if (BTF_INFO_KIND(proto->info) != BTF_KIND_FUNC_PROTO)
> > + return false;
> > +
> > + args = (const struct btf_param *)(proto + 1);
> > + nargs = btf_vlen(proto);
> > + if (nargs > MAX_BPF_FUNC_ARGS)
> > + return false;
> > +
> > + t = btf__type_by_id(btf, proto->type);
> > + while (t && btf_type_is_modifier(t))
> > + t = btf__type_by_id(btf, t->type);
> > +
> > + if (btf_is_struct(t) || btf_is_union(t))
> > + return false;
> > +
> > + for (i = 0; i < nargs; i++) {
> > + /* No support for variable args */
> > + if (i == nargs - 1 && args[i].type == 0)
> > + return false;
> > +
> > + /* No support of struct argument size greater than 16 bytes */
> > + ret = btf__resolve_size(btf, args[i].type);
> > + if (ret < 0 || ret > 16)
> > + return false;
> > + }
> > +
> > + return true;
> > +}
>
> NIT: can we avoid copy/pasting the code snippet from libbpf.c?
maybe we could make it global and use it through libbpf_internal.h,
will check
thanks,
jirka
^ permalink raw reply
* Re: [PATCHv2 bpf-next 14/23] libbpf: Add bpf_link_create support for tracing_multi link
From: Jiri Olsa @ 2026-03-05 14:01 UTC (permalink / raw)
To: bot+bpf-ci
Cc: ast, daniel, andrii, bpf, linux-trace-kernel, kafai, eddyz87,
songliubraving, yhs, menglong8.dong, rostedt, martin.lau,
yonghong.song, clm, ihor.solodrai
In-Reply-To: <ac763f388094863ef5c4d3a60d5606ea40406f66c91006fc9dd0e47573154a72@mail.kernel.org>
On Wed, Mar 04, 2026 at 11:02:23PM +0000, bot+bpf-ci@kernel.org wrote:
> > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> > --- a/tools/lib/bpf/bpf.c
> > +++ b/tools/lib/bpf/bpf.c
>
> [ ... ]
>
> > + case BPF_TRACE_FENTRY_MULTI:
> > + case BPF_TRACE_FEXIT_MULTI:
> > + case BPF_TRACE_FSESSION_MULTI:
> > + attr.link_create.tracing_multi.ids = (__u64) OPTS_GET(opts, tracing_multi.ids, 0);
> > + attr.link_create.tracing_multi.cookies = (__u64) OPTS_GET(opts, tracing_multi.cookies, 0);
>
> Should these use ptr_to_u64() instead of a raw (__u64) cast?
> The ids and cookies fields are pointers (__u32 * and __u64 *
> respectively), and every other pointer-to-u64 conversion in
> bpf_link_create() goes through ptr_to_u64():
>
> attr.link_create.kprobe_multi.syms = ptr_to_u64(OPTS_GET(...));
> attr.link_create.kprobe_multi.addrs = ptr_to_u64(OPTS_GET(...));
> attr.link_create.uprobe_multi.path = ptr_to_u64(OPTS_GET(...));
>
> The ptr_to_u64() helper casts through (unsigned long) first,
> which avoids implementation-defined behavior and compiler
> warnings on 32-bit builds.
yep, should be ptr_to_u64, will fix, thnx
jirka
>
> > + attr.link_create.tracing_multi.cnt = OPTS_GET(opts, tracing_multi.cnt, 0);
> > + if (!OPTS_ZEROED(opts, tracing_multi))
> > + return libbpf_err(-EINVAL);
> > + break;
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/22692622038
^ permalink raw reply
* Re: [PATCHv2 bpf-next 12/23] bpf: Add support for tracing_multi link fdinfo
From: Jiri Olsa @ 2026-03-05 14:01 UTC (permalink / raw)
To: bot+bpf-ci
Cc: ast, daniel, andrii, bpf, linux-trace-kernel, kafai, eddyz87,
songliubraving, yhs, menglong8.dong, rostedt, martin.lau,
yonghong.song, clm, ihor.solodrai
In-Reply-To: <77554f42f56834088eca284c1a787f51e4402907c616f8194942c82530a9ce6b@mail.kernel.org>
On Wed, Mar 04, 2026 at 11:02:26PM +0000, bot+bpf-ci@kernel.org wrote:
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
>
> [ ... ]
>
> > + seq_printf(seq, "%18s %10s %s\n", "cookie", "BTF-id", "func");
> > + for (int i = 0; i < tr_link->nodes_cnt; i++) {
> > + struct bpf_tracing_multi_node *mnode = &tr_link->nodes[i];
> > + u32 btf_id;
> > +
> > + bpf_trampoline_unpack_key(mnode->trampoline->key, NULL, &btf_id);
> > + seq_printf(seq, "%18llx %10u %pS\n",
> > + has_cookies ? tr_link->cookies[i] : 0,
> > + btf_id, (void *) mnode->trampoline->ip);
> ^^^^
>
> Should this be %18llu instead of %18llx? Every other BPF link fdinfo
> displays cookies as decimal:
>
> bpf_kprobe_multi_show_fdinfo():
> seq_printf(seq, "%llu\t %pS\n",
> has_cookies ? kmulti_link->cookies[i] : 0, ...);
>
> bpf_uprobe_multi_show_fdinfo():
> seq_printf(seq, "%llu\t %#llx\t %#lx\n",
> umulti_link->uprobes[i].cookie, ...);
>
> bpf_tracing_link_show_fdinfo():
> seq_printf(seq, ... "cookie:\t%llu\n", ...);
>
> With %llx, a cookie value like 255 would print as "ff" here but "255"
> in every other BPF link fdinfo. The hex output also lacks a 0x prefix,
> making it ambiguous whether values like "10" are decimal or hex.
yep, should be consistent, will make it decimal, thnx
jirka
>
> > + }
> > +}
>
> [ ... ]
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/22692622038
^ permalink raw reply
* Re: [PATCHv2 bpf-next 02/23] bpf: Use mutex lock pool for bpf trampolines
From: Jiri Olsa @ 2026-03-05 14:01 UTC (permalink / raw)
To: bot+bpf-ci
Cc: ast, daniel, andrii, bpf, linux-trace-kernel, kafai, eddyz87,
songliubraving, yhs, menglong8.dong, rostedt, martin.lau,
yonghong.song, clm, ihor.solodrai
In-Reply-To: <28376b19ac32833bc93e5688dbb271724f84f19aece2f828c7ca56bbd9706a0f@mail.kernel.org>
On Wed, Mar 04, 2026 at 11:02:28PM +0000, bot+bpf-ci@kernel.org wrote:
> > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> > --- a/kernel/bpf/trampoline.c
> > +++ b/kernel/bpf/trampoline.c
>
> [ ... ]
>
> > @@ -99,12 +127,12 @@ static int bpf_tramp_ftrace_ops_func(struct ftrace_ops *ops, unsigned long ip,
> > * cleanup_direct_functions_after_ipmodify
> > *
> > * In both cases, direct_mutex is already locked. Use
> > - * mutex_trylock(&tr->mutex) to avoid deadlock in race condition
> > + * mutex_trylock(select_trampoline_lock(tr)) to avoid deadlock in race condition
> > * (something else is making changes to this same trampoline).
> > */
> > - if (!mutex_trylock(&tr->mutex)) {
> > - /* sleep 1 ms to make sure whatever holding tr->mutex makes
> > - * some progress.
> > + if (!mutex_trylock(select_trampoline_lock(tr))) {
> > + /* sleep 1 ms to make sure whatever holding select_trampoline_lock(tr)
> > + * makes some progress.
> > */
>
> With a pool of 32 mutexes, the trylock can now also fail because a
> different trampoline that hashes to the same pool entry holds the lock.
> Should the comment say "something else holds the same pool lock" instead
> of "something else is making changes to this same trampoline"?
right, will change the comment, thnx
>
> Similarly, the sleep comment says the holder "makes some progress" but
> with a pool lock the holder could be operating on an entirely unrelated
> trampoline.
this one seems ok to me.. holder is 'whatever holding select_trampoline_lock'
which is correct
jirka
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/22692622038
^ permalink raw reply
* Re: [PATCHv2 bpf-next 09/23] bpf: Add support for tracing multi link
From: Jiri Olsa @ 2026-03-05 14:02 UTC (permalink / raw)
To: bot+bpf-ci
Cc: ast, daniel, andrii, bpf, linux-trace-kernel, kafai, eddyz87,
songliubraving, yhs, menglong8.dong, rostedt, martin.lau,
yonghong.song, clm, ihor.solodrai
In-Reply-To: <e0a34750677c3cfbc461c9653f39b1d8388c1e3ca8a95e918e380e61999adf51@mail.kernel.org>
On Wed, Mar 04, 2026 at 11:02:30PM +0000, bot+bpf-ci@kernel.org wrote:
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
>
> [ ... ]
>
> > +int bpf_tracing_multi_attach(struct bpf_prog *prog, const union bpf_attr *attr)
> > +{
> > + struct bpf_tracing_multi_link *link = NULL;
> > + struct bpf_link_primer link_primer;
> > + u32 cnt, *ids = NULL;
> > + u32 __user *uids;
> > + int err;
> > +
> > + uids = u64_to_user_ptr(attr->link_create.tracing_multi.ids);
> > + cnt = attr->link_create.tracing_multi.cnt;
> > +
> > + if (!cnt || !uids)
> > + return -EINVAL;
>
> Should attr->link_create.flags be validated here as well?
>
> Both bpf_kprobe_multi_link_attach() and bpf_uprobe_multi_link_attach()
> reject non-zero flags:
>
> kernel/trace/bpf_trace.c:bpf_kprobe_multi_link_attach() {
> ...
> if (attr->link_create.flags)
> return -EINVAL;
> ...
> }
argh right, that's missing, will fix, thnx
>
> Without this check, userspace passing flags != 0 will be silently
> accepted, which would prevent using the flags field for future
> extensions since old kernels could not be distinguished from new
> ones.
>
> > + if (cnt > MAX_TRACING_MULTI_CNT)
> > + return -E2BIG;
>
> [ ... ]
>
> > +#else
> > +
> > +int bpf_tracing_multi_attach(struct bpf_prog *prog, const union bpf_attr *attr)
> > +{
> > + return -EOPNOTSUPP;
> > +}
> > +
> > +#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS) && CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS */
>
> Minor: there is a stray ')' after CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> in this comment.
yep, will remove, thnx
jirka
^ permalink raw reply
* Re: [PATCHv2 bpf-next 08/23] bpf: Add bpf_trampoline_multi_attach/detach functions
From: Jiri Olsa @ 2026-03-05 14:02 UTC (permalink / raw)
To: bot+bpf-ci
Cc: ast, daniel, andrii, bpf, linux-trace-kernel, kafai, eddyz87,
songliubraving, yhs, menglong8.dong, rostedt, martin.lau,
yonghong.song, clm, ihor.solodrai
In-Reply-To: <bb15f21a9af1489fe2cee94afb78dd423be2a7a77b3dee5ed8b286dcef45f034@mail.kernel.org>
On Wed, Mar 04, 2026 at 11:02:31PM +0000, bot+bpf-ci@kernel.org wrote:
> > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> > index e2f4a15886b0..c42bf16b6807 100644
> > --- a/kernel/bpf/trampoline.c
> > +++ b/kernel/bpf/trampoline.c
>
> [ ... ]
>
> > +int bpf_trampoline_multi_detach(struct bpf_prog *prog, struct bpf_tracing_multi_link *link)
> > +{
> > + struct bpf_tracing_multi_node *mnode;
> > + struct fentry_multi_data data = {};
> > + int i, cnt = link->nodes_cnt;
> > +
> > + data.unreg = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
> > + data.modify = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
> > +
> > + if (!data.unreg || !data.modify) {
> > + free_ftrace_hash(data.unreg);
> > + return -ENOMEM;
> > + }
>
> If data.unreg allocation fails but data.modify succeeds, only
> free_ftrace_hash(data.unreg) is called (which is a no-op on NULL).
> Wouldn't data.modify be leaked in that case?
>
> The attach function handles this correctly by using
> free_fentry_multi_data() which frees all three hashes. Could the
> detach error path also free data.modify?
should call free_fentry_multi_data, will fix, thnx
jirka
^ permalink raw reply
* Re: [PATCHv2 bpf-next 06/23] bpf: Add multi tracing attach types
From: Jiri Olsa @ 2026-03-05 14:02 UTC (permalink / raw)
To: bot+bpf-ci
Cc: ast, daniel, andrii, bpf, linux-trace-kernel, kafai, eddyz87,
songliubraving, yhs, menglong8.dong, rostedt, martin.lau,
yonghong.song, clm, ihor.solodrai
In-Reply-To: <90246a2d7a691e7374b18d699e0778f10b750b85fd0739d485c96960925a822b@mail.kernel.org>
On Wed, Mar 04, 2026 at 11:02:33PM +0000, bot+bpf-ci@kernel.org wrote:
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index d92cf2821657..832a87da5a86 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
>
> [ ... ]
>
> > @@ -25173,6 +25181,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> > case BPF_TRACE_FENTRY:
> > case BPF_TRACE_FEXIT:
> > case BPF_TRACE_FSESSION:
> > + case BPF_TRACE_FENTRY_MULTI:
> > + case BPF_TRACE_FEXIT_MULTI:
>
> The new multi types are added to this switch, but earlier in the same
> function bpf_check_attach_target() there is a check that prevents
> BPF_PROG_TYPE_EXT from targeting fentry/fexit/fsession programs:
>
> kernel/bpf/verifier.c:bpf_check_attach_target() {
> ...
> if (tgt_prog->type == BPF_PROG_TYPE_TRACING &&
> prog_extension &&
> (tgt_prog->expected_attach_type == BPF_TRACE_FENTRY ||
> tgt_prog->expected_attach_type == BPF_TRACE_FEXIT ||
> tgt_prog->expected_attach_type == BPF_TRACE_FSESSION)) {
> bpf_log(log, "Cannot extend fentry/fexit/fsession\n");
> return -EINVAL;
> }
> ...
> }
>
> Should BPF_TRACE_FENTRY_MULTI and BPF_TRACE_FEXIT_MULTI be included
> in that check as well? The comment above it explains that extending
> fentry/fexit programs is blocked to prevent
> fentry->extension->fentry->extension call chains. Multi programs map
> to BPF_TRAMP_FENTRY/BPF_TRAMP_FEXIT in bpf_attach_type_to_tramp(),
> so the same concern would apply.
but multi programs would never have tgt_prog/prog->aux->dst_prog set,
and that check above is under 'if (tgt_prog) {' check, so I think we
are ok, but I will double check, thnx
jirka
>
> > @@ -25202,7 +25212,17 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
>
> [ ... ]
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/22692622038
^ permalink raw reply
* Re: [PATCHv2 bpf-next 15/23] libbpf: Add support to create tracing multi link
From: Jiri Olsa @ 2026-03-05 14:02 UTC (permalink / raw)
To: bot+bpf-ci
Cc: ast, daniel, andrii, bpf, linux-trace-kernel, kafai, eddyz87,
songliubraving, yhs, menglong8.dong, rostedt, martin.lau,
yonghong.song, clm, ihor.solodrai
In-Reply-To: <625b9bd61d08db7a0cdff51817fe64752c1901d360d73455b609516a0b611d3d@mail.kernel.org>
On Wed, Mar 04, 2026 at 11:02:25PM +0000, bot+bpf-ci@kernel.org wrote:
> > The provided <pattern> is used as 'pattern' argument in
> > bpf_program__attach_kprobe_multi_opts function.
>
> The function referenced here should be
> bpf_program__attach_tracing_multi, not
> bpf_program__attach_kprobe_multi_opts. This was noted in v1
> review.
I'm refering to bpf_program__attach_kprobe_multi_opts intentionaly
>
> > - bpf_program__attach_kprobe_multi_opts must either have
> > pattern with module spec or BTF ids from the module
>
> Same here, should be bpf_program__attach_tracing_multi.
yep, that's typo, thnx
>
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
>
> [ ... ]
>
> > +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 *ids, *free_ids = NULL;
> > + __u64 *cookies;
> > + int prog_fd, link_fd, err, cnt;
> > + struct bpf_link *link;
> > +
> > + cnt = OPTS_GET(opts, cnt, 0);
> > + ids = OPTS_GET(opts, ids, NULL);
> > + cookies = OPTS_GET(opts, cookies, NULL);
>
> Should this function validate opts with OPTS_VALID() before
> calling OPTS_GET()? The comparable
> bpf_program__attach_kprobe_multi_opts() does:
>
> if (!OPTS_VALID(opts, bpf_kprobe_multi_opts))
> return libbpf_err_ptr(-EINVAL);
>
> Without OPTS_VALID(), a caller passing opts with sz=0 would have
> ids, cookies and cnt silently ignored rather than getting -EINVAL
> back.
yep, should be there, will fix, thnx
jirka
^ permalink raw reply
* Re: [PATCHv2 bpf-next 15/23] libbpf: Add support to create tracing multi link
From: Jiri Olsa @ 2026-03-05 14:02 UTC (permalink / raw)
To: Leon Hwang
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: <0f3ec422-4b98-4311-96de-ccfc86bb22ad@linux.dev>
On Thu, Mar 05, 2026 at 02:59:48PM +0800, Leon Hwang wrote:
SNIP
> > +static int tracing_multi_mod_fd(struct bpf_program *prog, int *btf_obj_fd)
> > +{
> > + const char *attach_name, *sep, *mod_name = NULL;
> > + int i, err, mod_len = 0;
> > +
> > + *btf_obj_fd = 0;
> > + attach_name = strchr(prog->sec_name, '/');
> > +
> > + /* Program with no details in spec, using kernel btf. */
> > + if (!attach_name)
> > + return 0;
> > +
> > + attach_name++;
> > + sep = strchr(attach_name, ':');
> > + if (sep) {
> > + mod_name = attach_name;
> > + mod_len = sep - mod_name;
> > + }
> > +
> > + /* Program with no module section, using kernel btf. */
> > + if (!mod_name)
> > + return 0;
> > +
> > + err = load_module_btfs(prog->obj);
> > + if (err)
> > + return err;
> > +
> > + for (i = 0; i < prog->obj->btf_module_cnt; i++) {
> > + const struct module_btf *mod = &prog->obj->btf_modules[i];
> > +
> > + if (mod_name && strncmp(mod->name, mod_name, mod_len) == 0) {
>
> strncmp() is not enough to exactly compare two module names, as
> strncmp() only compares the first 'mod_len' bytes.
>
> So, mod_name "foo" could match mod "foobar", since 'obj->btf_modules'
> are not sorted by name after loading from kernel.
>
> To find the exact module via module name, "strncmp() == 0 &&
> mod->name[mod_len] == '\0'" would be better.
yes, good catch, will fix
SNIP
> > +static int collect_func_ids_by_glob(struct bpf_object *obj, const char *pattern, __u32 **ids)
> > +{
> > + const char *mod_name = NULL, *sep;
> > + int i, err, mod_len = 0;
> > + struct btf *btf = NULL;
> > +
> > + err = bpf_object__load_vmlinux_btf(obj, true);
> > + if (err)
> > + return err;
> > +
> > + /* In case we have module specified, we will find its btf and use that. */
> > + sep = strchr(pattern, ':');
> > + if (sep) {
> > + mod_name = pattern;
> > + mod_len = sep - pattern;
> > + pattern = sep + 1;
> > +
> > + err = load_module_btfs(obj);
> > + if (err)
> > + goto cleanup;
> > +
> > + for (i = 0; i < obj->btf_module_cnt; i++) {
> > + const struct module_btf *mod = &obj->btf_modules[i];
> > +
> > + if (!strncmp(mod->name, mod_name, mod_len)) {
>
> Same concern here.
>
> Would it be better to factor out a helper to find module via name?
we have similar issue in find_kernel_btf_id, but we get away with
that with just continuing to loop modules if the lookup fails
I agree some helper would be good hear, will try to add it
thanks,
jirka
^ permalink raw reply
* Re: [PATCH v3 01/12] vfs: widen inode hash/lookup functions to u64
From: Christoph Hellwig @ 2026-03-05 14:24 UTC (permalink / raw)
To: Jeff Layton
Cc: Alexander Viro, Christian Brauner, Jan Kara, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Dan Williams, Eric Biggers,
Theodore Y. Ts'o, Muchun Song, Oscar Salvador,
David Hildenbrand, David Howells, Paulo Alcantara, Andreas Dilger,
Jan Kara, Jaegeuk Kim, Chao Yu, Trond Myklebust, Anna Schumaker,
Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Steve French, Ronnie Sahlberg, Shyam Prasad N, Bharath SM,
Alexander Aring, Ryusuke Konishi, Viacheslav Dubeyko,
Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
Christian Schoenebeck, David Sterba, Marc Dionne, Ian Kent,
Luis de Bethencourt, Salah Triki, Tigran A. Aivazian,
Ilya Dryomov, Alex Markuze, Jan Harkes, coda, Nicolas Pitre,
Tyler Hicks, Amir Goldstein, Christoph Hellwig,
John Paul Adrian Glaubitz, Yangtao Li, Mikulas Patocka,
David Woodhouse, Richard Weinberger, Dave Kleikamp,
Konstantin Komarov, Mark Fasheh, Joel Becker, Joseph Qi,
Mike Marshall, Martin Brandenburg, Miklos Szeredi, Anders Larsen,
Zhihao Cheng, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Fan Wu,
Stephen Smalley, Ondrej Mosnacek, Casey Schaufler, Alex Deucher,
Christian König, David Airlie, Simona Vetter, Sumit Semwal,
Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
David S. Miller, Jakub Kicinski, Simon Horman, Oleg Nesterov,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, James Clark, Darrick J. Wong,
Martin Schiller, Eric Paris, Joerg Reuter, Marcel Holtmann,
Johan Hedberg, Luiz Augusto von Dentz, Oliver Hartkopp,
Marc Kleine-Budde, David Ahern, Neal Cardwell, Steffen Klassert,
Herbert Xu, Remi Denis-Courmont, Marcelo Ricardo Leitner,
Xin Long, Magnus Karlsson, Maciej Fijalkowski, Stanislav Fomichev,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, linux-fsdevel, linux-kernel, linux-trace-kernel,
nvdimm, fsverity, linux-mm, netfs, linux-ext4, linux-f2fs-devel,
linux-nfs, linux-cifs, samba-technical, linux-nilfs, v9fs,
linux-afs, autofs, ceph-devel, codalist, ecryptfs, linux-mtd,
jfs-discussion, ntfs3, ocfs2-devel, devel, linux-unionfs,
apparmor, linux-security-module, linux-integrity, selinux,
amd-gfx, dri-devel, linux-media, linaro-mm-sig, netdev,
linux-perf-users, linux-fscrypt, linux-xfs, linux-hams, linux-x25,
audit, linux-bluetooth, linux-can, linux-sctp, bpf
In-Reply-To: <20260304-iino-u64-v3-1-2257ad83d372@kernel.org>
> extern struct inode *ilookup5_nowait(struct super_block *sb,
> - unsigned long hashval, int (*test)(struct inode *, void *),
> + u64 hashval, int (*test)(struct inode *, void *),
> void *data, bool *isnew);
> -extern struct inode *ilookup5(struct super_block *sb, unsigned long hashval,
> +extern struct inode *ilookup5(struct super_block *sb, u64 hashval,
> int (*test)(struct inode *, void *), void *data);
...
Can you please drop all these pointless externs while you're at it?
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH v3 12/12] treewide: change inode->i_ino from unsigned long to u64
From: Christoph Hellwig @ 2026-03-05 14:25 UTC (permalink / raw)
To: Jeff Layton
Cc: Alexander Viro, Christian Brauner, Jan Kara, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Dan Williams, Eric Biggers,
Theodore Y. Ts'o, Muchun Song, Oscar Salvador,
David Hildenbrand, David Howells, Paulo Alcantara, Andreas Dilger,
Jan Kara, Jaegeuk Kim, Chao Yu, Trond Myklebust, Anna Schumaker,
Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Steve French, Ronnie Sahlberg, Shyam Prasad N, Bharath SM,
Alexander Aring, Ryusuke Konishi, Viacheslav Dubeyko,
Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
Christian Schoenebeck, David Sterba, Marc Dionne, Ian Kent,
Luis de Bethencourt, Salah Triki, Tigran A. Aivazian,
Ilya Dryomov, Alex Markuze, Jan Harkes, coda, Nicolas Pitre,
Tyler Hicks, Amir Goldstein, Christoph Hellwig,
John Paul Adrian Glaubitz, Yangtao Li, Mikulas Patocka,
David Woodhouse, Richard Weinberger, Dave Kleikamp,
Konstantin Komarov, Mark Fasheh, Joel Becker, Joseph Qi,
Mike Marshall, Martin Brandenburg, Miklos Szeredi, Anders Larsen,
Zhihao Cheng, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Fan Wu,
Stephen Smalley, Ondrej Mosnacek, Casey Schaufler, Alex Deucher,
Christian König, David Airlie, Simona Vetter, Sumit Semwal,
Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
David S. Miller, Jakub Kicinski, Simon Horman, Oleg Nesterov,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, James Clark, Darrick J. Wong,
Martin Schiller, Eric Paris, Joerg Reuter, Marcel Holtmann,
Johan Hedberg, Luiz Augusto von Dentz, Oliver Hartkopp,
Marc Kleine-Budde, David Ahern, Neal Cardwell, Steffen Klassert,
Herbert Xu, Remi Denis-Courmont, Marcelo Ricardo Leitner,
Xin Long, Magnus Karlsson, Maciej Fijalkowski, Stanislav Fomichev,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, linux-fsdevel, linux-kernel, linux-trace-kernel,
nvdimm, fsverity, linux-mm, netfs, linux-ext4, linux-f2fs-devel,
linux-nfs, linux-cifs, samba-technical, linux-nilfs, v9fs,
linux-afs, autofs, ceph-devel, codalist, ecryptfs, linux-mtd,
jfs-discussion, ntfs3, ocfs2-devel, devel, linux-unionfs,
apparmor, linux-security-module, linux-integrity, selinux,
amd-gfx, dri-devel, linux-media, linaro-mm-sig, netdev,
linux-perf-users, linux-fscrypt, linux-xfs, linux-hams, linux-x25,
audit, linux-bluetooth, linux-can, linux-sctp, bpf
In-Reply-To: <20260304-iino-u64-v3-12-2257ad83d372@kernel.org>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH v4 4/5] mm: rename zone->lock to zone->_lock
From: SeongJae Park @ 2026-03-05 14:55 UTC (permalink / raw)
To: Vlastimil Babka (SUSE)
Cc: SeongJae Park, Dmitry Ilvokhin, Andrew Morton, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Rafael J. Wysocki, Pavel Machek, Len Brown, Brendan Jackman,
Johannes Weiner, Zi Yan, Oscar Salvador, Qi Zheng, Shakeel Butt,
linux-kernel, linux-mm, linux-trace-kernel, linux-pm
In-Reply-To: <ebd994ca-eb04-4dff-a0a8-47aef0934c2c@kernel.org>
On Thu, 5 Mar 2026 10:27:07 +0100 "Vlastimil Babka (SUSE)" <vbabka@kernel.org> wrote:
> On 3/4/26 16:13, SeongJae Park wrote:
> > On Wed, 4 Mar 2026 13:01:45 +0000 Dmitry Ilvokhin <d@ilvokhin.com> wrote:
> >
> >> On Tue, Mar 03, 2026 at 05:50:34PM -0800, SeongJae Park wrote:
> >> > On Tue, 3 Mar 2026 14:25:55 +0000 Dmitry Ilvokhin <d@ilvokhin.com> wrote:
> >> >
> >> > > On Mon, Mar 02, 2026 at 02:37:43PM -0800, Andrew Morton wrote:
> >> > > > On Mon, 2 Mar 2026 15:10:03 +0100 "Vlastimil Babka (SUSE)" <vbabka@kernel.org> wrote:
> >> > > >
> >> > > > > On 2/27/26 17:00, Dmitry Ilvokhin wrote:
> >> > > > > > This intentionally breaks direct users of zone->lock at compile time so
> >> > > > > > all call sites are converted to the zone lock wrappers. Without the
> >> > > > > > rename, present and future out-of-tree code could continue using
> >> > > > > > spin_lock(&zone->lock) and bypass the wrappers and tracing
> >> > > > > > infrastructure.
> >> > > > > >
> >> > > > > > No functional change intended.
> >> > > > > >
> >> > > > > > Suggested-by: Andrew Morton <akpm@linux-foundation.org>
> >> > > > > > Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
> >> > > > > > Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
> >> > > > > > Acked-by: SeongJae Park <sj@kernel.org>
> >> > > > >
> >> > > > > I see some more instances of 'zone->lock' in comments in
> >> > > > > include/linux/mmzone.h and under Documentation/ but otherwise LGTM.
> >> > > > >
> >> > > >
> >> > > > I fixed (most of) that in the previous version but my fix was lost.
> >> > >
> >> > > Thanks for the fixups, Andrew.
> >> > >
> >> > > I still see a few 'zone->lock' references in Documentation remain on
> >> > > mm-new. This patch cleans them up, as noted by Vlastimil.
> >> > >
> >> > > I'm happy to adjust this patch if anything else needs attention.
> >> > >
> >> > > From 9142d5a8b60038fa424a6033253960682e5a51f4 Mon Sep 17 00:00:00 2001
> >> > > From: Dmitry Ilvokhin <d@ilvokhin.com>
> >> > > Date: Tue, 3 Mar 2026 06:13:13 -0800
> >> > > Subject: [PATCH] mm: fix remaining zone->lock references
> >> > >
> >> > > Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
> >> > > ---
> >> > > Documentation/mm/physical_memory.rst | 4 ++--
> >> > > Documentation/trace/events-kmem.rst | 8 ++++----
> >> > > 2 files changed, 6 insertions(+), 6 deletions(-)
> >> > >
> >> > > diff --git a/Documentation/mm/physical_memory.rst b/Documentation/mm/physical_memory.rst
> >> > > index b76183545e5b..e344f93515b6 100644
> >> > > --- a/Documentation/mm/physical_memory.rst
> >> > > +++ b/Documentation/mm/physical_memory.rst
> >> > > @@ -500,11 +500,11 @@ General
> >> > > ``nr_isolate_pageblock``
> >> > > Number of isolated pageblocks. It is used to solve incorrect freepage counting
> >> > > problem due to racy retrieving migratetype of pageblock. Protected by
> >> > > - ``zone->lock``. Defined only when ``CONFIG_MEMORY_ISOLATION`` is enabled.
> >> > > + ``zone_lock``. Defined only when ``CONFIG_MEMORY_ISOLATION`` is enabled.
> >> >
> >> > Dmitry's original patch [1] was doing 's/zone->lock/zone->_lock/', which aligns
> >> > to my expectation. But this patch is doing 's/zone->lock/zone_lock/'. Same
> >> > for the rest of this patch.
> >> >
> >> > I was initially thinking this is just a mistake, but I also found Andrew is
> >> > doing same change [2], so I'm bit confused. Is this an intentional change?
> >> >
> >> > [1] https://lore.kernel.org/d61500c5784c64e971f4d328c57639303c475f81.1772206930.git.d@ilvokhin.com
> >> > [2] https://lore.kernel.org/20260302143743.220eed4feb36d7572fe726cc@linux-foundation.org
> >> >
> >>
> >> Good catch, thanks for pointing this out, SJ.
> >>
> >> Originally the mechanical rename was indeed zone->lock -> zone->_lock.
> >> However, in Documentation I intentionally switched references to
> >> zone_lock instead of zone->_lock. The reasoning is that _lock is now an
> >> internal implementation detail, and direct access is discouraged. The
> >> intended interface is via the zone_lock_*() / zone_unlock_*() wrappers,
> >> so referencing zone_lock in documentation felt more appropriate than
> >> mentioning the private struct field (zone->_lock).
> >
> > Thank you for this nice and kind clarification, Dmitry! I agree mentioning
> > zone_[un]lock_*() helpers instead of the hidden member (zone->_lock) can be
> > better.
> >
> > But, I'm concerned if people like me might not aware the intention under
> > 'zone_lock'. If there is a well-known convention that allows people to know it
> > is for 'zone_[un]lock_*()' helpers, making it more clear would be nice, in my
> > humble opinion. If there is such a convention but I'm just missing it, please
> > ignore. If I'm not, for eaxmaple,
> >
> > "protected by ``zone->lock``" could be re-wrote to
> > "protected by ``zone_[un]lock_*()`` locking helpers" or,
> > "protected by zone lock helper functions (``zone_[un]lock_*()``)" ?
> >
> >>
> >> That said, I agree this creates inconsistency with the mechanical
> >> rename, and I'm happy to adjust either way: either consistently refer
> >> to the wrapper API, or keep documentation aligned with zone->_lock.
> >>
> >> I slightly prefer referring to the wrapper API, but don't have a strong
> >> preference as long as we're consistent.
> >
> > I also think both approaches are good. But for the wrapper approach, I think
> > giving more contexts rather than just ``zone_lock`` to readers would be nice.
>
> Grep tells me that we also have comments mentioning simply "zone lock", btw.
> And it's also a term used often in informal conversations. Maybe we could
> just standardize on that in comments/documentations as it's easier to read.
> Discovering that the field is called _lock and that wrappers should be used,
> is hopefully not that difficult.
Sounds good, that also works for me.
Thanks,
SJ
^ permalink raw reply
* Re: [RFC PATCH] tracing: Revert "tracing: Remove pid in task_rename tracing output"
From: Steven Rostedt @ 2026-03-05 15:08 UTC (permalink / raw)
To: Xuewen Yan
Cc: mhiramat, mathieu.desnoyers, elver, kees, lorenzo.stoakes,
brauner, schuster.simon, david, linux-kernel, linux-trace-kernel,
guohua.yan, ke.wang, xuewen.yan94
In-Reply-To: <20260305080809.3245-1-xuewen.yan@unisoc.com>
On Thu, 5 Mar 2026 16:08:09 +0800
Xuewen Yan <xuewen.yan@unisoc.com> wrote:
> This reverts commit e3f6a42272e028c46695acc83fc7d7c42f2750ad.
>
> The commit says that the tracepoint only deals with the current task,
> however the following case is not current task:
>
> comm_write
> set_task_comm
> __set_task_comm
> trace_task_rename
I'm fine with the patch, but the change log can use a bit of work:
comm_write() {
p = get_proc_task(inode);
if (!p)
return -ESRCH;
if (same_thread_group(current, p)) {
set_task_comm(p, buffer);
where set_task_comm() calls __set_task_comm() which records the update of
p and not current.
The above states exactly why current isn't the pid that should be recorded.
Other than that.
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
-- Steve
>
> So revert the patch to show pid.
>
> Fixes: e3f6a42272e0 ("tracing: Remove pid in task_rename tracing output")
> Reported-by: Guohua Yan <guohua.yan@unisoc.com>
> Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
> ---
> include/trace/events/task.h | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/include/trace/events/task.h b/include/trace/events/task.h
> index 4f0759634306..b9a129eb54d9 100644
> --- a/include/trace/events/task.h
> +++ b/include/trace/events/task.h
> @@ -38,19 +38,22 @@ TRACE_EVENT(task_rename,
> TP_ARGS(task, comm),
>
> TP_STRUCT__entry(
> + __field( pid_t, pid)
> __array( char, oldcomm, TASK_COMM_LEN)
> __array( char, newcomm, TASK_COMM_LEN)
> __field( short, oom_score_adj)
> ),
>
> TP_fast_assign(
> + __entry->pid = task->pid;
> memcpy(entry->oldcomm, task->comm, TASK_COMM_LEN);
> strscpy(entry->newcomm, comm, TASK_COMM_LEN);
> __entry->oom_score_adj = task->signal->oom_score_adj;
> ),
>
> - TP_printk("oldcomm=%s newcomm=%s oom_score_adj=%hd",
> - __entry->oldcomm, __entry->newcomm, __entry->oom_score_adj)
> + TP_printk("pid=%d oldcomm=%s newcomm=%s oom_score_adj=%hd",
> + __entry->pid, __entry->oldcomm,
> + __entry->newcomm, __entry->oom_score_adj)
> );
>
> /**
^ permalink raw reply
* Re: [PATCH RFC 2/3] locking/percpu-rwsem: Extract __percpu_up_read_slowpath()
From: Steven Rostedt @ 2026-03-05 15:47 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Dmitry Ilvokhin, Dennis Zhou, Tejun Heo, Christoph Lameter,
Masami Hiramatsu, Mathieu Desnoyers, Ingo Molnar, Will Deacon,
Boqun Feng, Waiman Long, linux-mm, linux-kernel,
linux-trace-kernel, kernel-team, Christoph Hellwig
In-Reply-To: <20260304220223.GS606826@noisy.programming.kicks-ass.net>
On Wed, 4 Mar 2026 23:02:23 +0100
Peter Zijlstra <peterz@infradead.org> wrote:
> > diff --git a/include/linux/percpu-rwsem.h b/include/linux/percpu-rwsem.h
> > index c8cb010d655e..89506895365c 100644
> > --- a/include/linux/percpu-rwsem.h
> > +++ b/include/linux/percpu-rwsem.h
> > @@ -107,6 +107,8 @@ static inline bool percpu_down_read_trylock(struct percpu_rw_semaphore *sem)
> > return ret;
> > }
> >
> > +void __percpu_up_read_slowpath(struct percpu_rw_semaphore *sem);
> > +
>
> extern for consistency with all the other declarations in this header.
I wonder if a cleanup patch should be added to remove the "extern" from the
other functions, as that tends to be the way things are going (hch just
recommended it elsewhere).
>
> s/_slowpath//, the corresponding down function also doesn't have
> _slowpath on.
>
> > static inline void percpu_up_read(struct percpu_rw_semaphore *sem)
> > {
> > rwsem_release(&sem->dep_map, _RET_IP_);
And since "slowpath" is more descriptive (and used in the rtmutex code),
should that be added too?
-- Steve
^ permalink raw reply
* Re: [PATCH RFC 3/3] locking: Wire up contended_release tracepoint
From: Steven Rostedt @ 2026-03-05 15:59 UTC (permalink / raw)
To: Dmitry Ilvokhin
Cc: Dennis Zhou, Tejun Heo, Christoph Lameter, Masami Hiramatsu,
Mathieu Desnoyers, Peter Zijlstra, Ingo Molnar, Will Deacon,
Boqun Feng, Waiman Long, linux-mm, linux-kernel,
linux-trace-kernel, kernel-team
In-Reply-To: <8298e098d3418cb446ef396f119edac58a3414e9.1772642407.git.d@ilvokhin.com>
On Wed, 4 Mar 2026 16:56:17 +0000
Dmitry Ilvokhin <d@ilvokhin.com> wrote:
> @@ -204,6 +206,8 @@ static inline void rwbase_write_unlock(struct rwbase_rt *rwb)
> unsigned long flags;
>
> raw_spin_lock_irqsave(&rtm->wait_lock, flags);
> + if (rt_mutex_has_waiters(rtm))
> + trace_contended_release(rwb);
Hmm, if statements should never be used just for tracepoints without a
static branch. The above should be:
if (trace_contended_release_enabled() && rt_mutex_has_waiters(rtm))
trace_contended_release(rwb);
The above "trace_contened_release_enabled()" is a static_branch where it
turns the if statement into a nop when the tracepoint is not enabled, and a
jmp when it is.
> __rwbase_write_unlock(rwb, WRITER_BIAS, flags);
> }
>
> @@ -213,6 +217,8 @@ static inline void rwbase_write_downgrade(struct rwbase_rt *rwb)
> unsigned long flags;
>
> raw_spin_lock_irqsave(&rtm->wait_lock, flags);
> + if (rt_mutex_has_waiters(rtm))
> + trace_contended_release(rwb);
Same here.
-- Steve
> /* Release it and account current as reader */
> __rwbase_write_unlock(rwb, WRITER_BIAS - 1, flags);
> }
> diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
> index 24df4d98f7d2..4e61dc0bb045 100644
> --- a/kernel/locking/rwsem.c
> +++ b/kernel/locking/rwsem.c
> @@ -1360,6 +1360,7 @@ static inline void __up_read(struct rw_semaphore *sem)
> if (unlikely((tmp & (RWSEM_LOCK_MASK|RWSEM_FLAG_WAITERS)) ==
> RWSEM_FLAG_WAITERS)) {
> clear_nonspinnable(sem);
> + trace_contended_release(sem);
> rwsem_wake(sem);
> }
> preempt_enable();
> @@ -1383,8 +1384,10 @@ static inline void __up_write(struct rw_semaphore *sem)
> preempt_disable();
> rwsem_clear_owner(sem);
> tmp = atomic_long_fetch_add_release(-RWSEM_WRITER_LOCKED, &sem->count);
> - if (unlikely(tmp & RWSEM_FLAG_WAITERS))
> + if (unlikely(tmp & RWSEM_FLAG_WAITERS)) {
> + trace_contended_release(sem);
> rwsem_wake(sem);
> + }
> preempt_enable();
> }
>
> @@ -1407,8 +1410,10 @@ static inline void __downgrade_write(struct rw_semaphore *sem)
> tmp = atomic_long_fetch_add_release(
> -RWSEM_WRITER_LOCKED+RWSEM_READER_BIAS, &sem->count);
> rwsem_set_reader_owned(sem);
> - if (tmp & RWSEM_FLAG_WAITERS)
> + if (tmp & RWSEM_FLAG_WAITERS) {
> + trace_contended_release(sem);
> rwsem_downgrade_wake(sem);
> + }
> preempt_enable();
> }
>
> diff --git a/kernel/locking/semaphore.c b/kernel/locking/semaphore.c
> index 3ef032e22f7e..3cef5ba88f7e 100644
> --- a/kernel/locking/semaphore.c
> +++ b/kernel/locking/semaphore.c
> @@ -231,8 +231,10 @@ void __sched up(struct semaphore *sem)
> else
> __up(sem, &wake_q);
> raw_spin_unlock_irqrestore(&sem->lock, flags);
> - if (!wake_q_empty(&wake_q))
> + if (!wake_q_empty(&wake_q)) {
> + trace_contended_release(sem);
> wake_up_q(&wake_q);
> + }
> }
> EXPORT_SYMBOL(up);
>
^ permalink raw reply
* Re: [PATCH] tracing: Documentation: Update histogram-design.rst for fn() handling
From: Steven Rostedt @ 2026-03-05 16:03 UTC (permalink / raw)
To: linux-doc, Jonathan Corbet
Cc: LKML, Linux Trace Kernel, Masami Hiramatsu, Mathieu Desnoyers,
Tom Zanussi
In-Reply-To: <20260126181742.03e8f0d5@gandalf.local.home>
Hi Jon,
Can you take this through your tree?
Thanks,
-- Steve
On Mon, 26 Jan 2026 18:17:42 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> The histogram documentation describes the old method of the histogram
> triggers using the fn() field of the histogram field structure to process
> the field. But due to Spectre mitigation, the function pointer to handle
> the fields at runtime caused a noticeable overhead. It was converted over
> to a fn_num and hist_fn_call() is now used to call the specific functions
> for the fields via a switch statement based on the field's fn_num value.
>
> Update the documentation to reflect this change.
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> Documentation/trace/histogram-design.rst | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/trace/histogram-design.rst b/Documentation/trace/histogram-design.rst
> index ae71b5bf97c6..e92f56ebd0b5 100644
> --- a/Documentation/trace/histogram-design.rst
> +++ b/Documentation/trace/histogram-design.rst
> @@ -69,7 +69,8 @@ So in this histogram, there's a separate bucket for each pid, and each
> bucket contains a value for that bucket, counting the number of times
> sched_waking was called for that pid.
>
> -Each histogram is represented by a hist_data struct.
> +Each histogram is represented by a hist_data struct
> +(struct hist_trigger_data).
>
> To keep track of each key and value field in the histogram, hist_data
> keeps an array of these fields named fields[]. The fields[] array is
> @@ -82,7 +83,7 @@ value or not, which the above histogram does not.
>
> Each struct hist_field contains a pointer to the ftrace_event_field
> from the event's trace_event_file along with various bits related to
> -that such as the size, offset, type, and a hist_field_fn_t function,
> +that such as the size, offset, type, and a hist field function,
> which is used to grab the field's data from the ftrace event buffer
> (in most cases - some hist_fields such as hitcount don't directly map
> to an event field in the trace buffer - in these cases the function
> @@ -241,28 +242,33 @@ it, event_hist_trigger() is called. event_hist_trigger() first deals
> with the key: for each subkey in the key (in the above example, there
> is just one subkey corresponding to pid), the hist_field that
> represents that subkey is retrieved from hist_data.fields[] and the
> -hist_field_fn_t fn() associated with that field, along with the
> +hist field function associated with that field, along with the
> field's size and offset, is used to grab that subkey's data from the
> current trace record.
>
> +Note, the hist field function use to be a function pointer in the
> +hist_field stucture. Due to spectre mitigation, it was converted into
> +a fn_num and hist_fn_call() is used to call the associated hist field
> +function that corresponds to the fn_num of the hist_field structure.
> +
> Once the complete key has been retrieved, it's used to look that key
> up in the tracing_map. If there's no tracing_map_elt associated with
> that key, an empty one is claimed and inserted in the map for the new
> key. In either case, the tracing_map_elt associated with that key is
> returned.
>
> -Once a tracing_map_elt available, hist_trigger_elt_update() is called.
> +Once a tracing_map_elt is available, hist_trigger_elt_update() is called.
> As the name implies, this updates the element, which basically means
> updating the element's fields. There's a tracing_map_field associated
> with each key and value in the histogram, and each of these correspond
> to the key and value hist_fields created when the histogram was
> created. hist_trigger_elt_update() goes through each value hist_field
> -and, as for the keys, uses the hist_field's fn() and size and offset
> +and, as for the keys, uses the hist_field's function and size and offset
> to grab the field's value from the current trace record. Once it has
> that value, it simply adds that value to that field's
> continually-updated tracing_map_field.sum member. Some hist_field
> -fn()s, such as for the hitcount, don't actually grab anything from the
> -trace record (the hitcount fn() just increments the counter sum by 1),
> +functions, such as for the hitcount, don't actually grab anything from the
> +trace record (the hitcount function just increments the counter sum by 1),
> but the idea is the same.
>
> Once all the values have been updated, hist_trigger_elt_update() is
^ permalink raw reply
* Re: [PATCH RFC 2/3] locking/percpu-rwsem: Extract __percpu_up_read_slowpath()
From: Peter Zijlstra @ 2026-03-05 16:05 UTC (permalink / raw)
To: Steven Rostedt
Cc: Dmitry Ilvokhin, Dennis Zhou, Tejun Heo, Christoph Lameter,
Masami Hiramatsu, Mathieu Desnoyers, Ingo Molnar, Will Deacon,
Boqun Feng, Waiman Long, linux-mm, linux-kernel,
linux-trace-kernel, kernel-team, Christoph Hellwig
In-Reply-To: <20260305104703.2a1e8151@gandalf.local.home>
On Thu, Mar 05, 2026 at 10:47:03AM -0500, Steven Rostedt wrote:
> On Wed, 4 Mar 2026 23:02:23 +0100
> Peter Zijlstra <peterz@infradead.org> wrote:
>
> > > diff --git a/include/linux/percpu-rwsem.h b/include/linux/percpu-rwsem.h
> > > index c8cb010d655e..89506895365c 100644
> > > --- a/include/linux/percpu-rwsem.h
> > > +++ b/include/linux/percpu-rwsem.h
> > > @@ -107,6 +107,8 @@ static inline bool percpu_down_read_trylock(struct percpu_rw_semaphore *sem)
> > > return ret;
> > > }
> > >
> > > +void __percpu_up_read_slowpath(struct percpu_rw_semaphore *sem);
> > > +
> >
> > extern for consistency with all the other declarations in this header.
>
> I wonder if a cleanup patch should be added to remove the "extern" from the
> other functions, as that tends to be the way things are going (hch just
> recommended it elsewhere).
Well, I rather like the extern. But yeah, I know hch does not agree.
> >
> > s/_slowpath//, the corresponding down function also doesn't have
> > _slowpath on.
> >
> > > static inline void percpu_up_read(struct percpu_rw_semaphore *sem)
> > > {
> > > rwsem_release(&sem->dep_map, _RET_IP_);
>
> And since "slowpath" is more descriptive (and used in the rtmutex code),
> should that be added too?
It already has __ prefix, no point in making the name even longer for no
real benefit.
^ permalink raw reply
* Re: [PATCH] tracing: Documentation: Update histogram-design.rst for fn() handling
From: Jonathan Corbet @ 2026-03-05 16:12 UTC (permalink / raw)
To: Steven Rostedt, linux-doc
Cc: LKML, Linux Trace Kernel, Masami Hiramatsu, Mathieu Desnoyers,
Tom Zanussi
In-Reply-To: <20260305110347.31d6bae5@gandalf.local.home>
Steven Rostedt <rostedt@goodmis.org> writes:
> Hi Jon,
>
> Can you take this through your tree?
Sure, will do.
Thanks,
jon
^ permalink raw reply
* Re: [PATCH v12 00/30] Tracefs support for pKVM
From: Steven Rostedt @ 2026-03-05 16:17 UTC (permalink / raw)
To: Vincent Donnefort
Cc: Marc Zyngier, mhiramat, mathieu.desnoyers, linux-trace-kernel,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <87cy20eeg6.wl-maz@kernel.org>
On Thu, 19 Feb 2026 19:11:21 +0000
Marc Zyngier <maz@kernel.org> wrote:
> >
> > Then the arm/kvm folks could start with that branch and add the arm/KVM
> > portion of this series on top of it. This will prevent major merge
> > conflicts in linux-next.
> >
> > Does that sound OK?
>
> That works. Just send us a link to the branch after -rc2 and we'll get
> that sorted.
Vincent,
There's a few minor conflicts with this series and v7.0-rc2, can you rebase
and send again?
Thanks,
-- Steve
^ permalink raw reply
* [PATCH] tracing: Do not call trigger_data_free with NULL data pointer
From: Guenter Roeck @ 2026-03-05 16:36 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, Guenter Roeck, Miaoqian Lin
If trigger_data_alloc() fails and returns NULL, event_hist_trigger_parse()
jumps to the out_free error path. While kfree() safely handles a NULL
pointer, trigger_data_free() does not. This causes a NULL pointer
dereference in trigger_data_free() when evaluating
data->cmd_ops->set_filter.
Fix the problem by adding a new goto label and jumping to it if
trigger_data_alloc() returns NULL.
The problem was found by an experimental code review agent based on
gemini-3.1-pro while reviewing backports into v6.18.y.
Assisted-by: Gemini:gemini-3.1-pro
Cc: Miaoqian Lin <linmq006@gmail.com>
Cc: Steven Rostedt (Google) <rostedt@goodmis.org>
Fixes: 0550069cc25f ("tracing: Properly process error handling in event_hist_trigger_parse()")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
kernel/trace/trace_events_hist.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 73ea180cad55..a2abdfe19281 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -6874,7 +6874,7 @@ static int event_hist_trigger_parse(struct event_command *cmd_ops,
trigger_data = trigger_data_alloc(cmd_ops, cmd, param, hist_data);
if (!trigger_data) {
ret = -ENOMEM;
- goto out_free;
+ goto out_destroy;
}
ret = event_trigger_set_filter(cmd_ops, file, filter, trigger_data);
@@ -6942,7 +6942,7 @@ static int event_hist_trigger_parse(struct event_command *cmd_ops,
remove_hist_vars(hist_data);
trigger_data_free(trigger_data);
-
+out_destroy:
destroy_hist_data(hist_data);
goto out;
}
--
2.45.2
^ permalink raw reply related
* Re: [PATCH] tracing: Do not call trigger_data_free with NULL data pointer
From: Steven Rostedt @ 2026-03-05 16:43 UTC (permalink / raw)
To: Guenter Roeck
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, Miaoqian Lin
In-Reply-To: <20260305163633.2782210-1-linux@roeck-us.net>
On Thu, 5 Mar 2026 08:36:33 -0800
Guenter Roeck <linux@roeck-us.net> wrote:
> If trigger_data_alloc() fails and returns NULL, event_hist_trigger_parse()
> jumps to the out_free error path. While kfree() safely handles a NULL
> pointer, trigger_data_free() does not. This causes a NULL pointer
> dereference in trigger_data_free() when evaluating
> data->cmd_ops->set_filter.
>
> Fix the problem by adding a new goto label and jumping to it if
> trigger_data_alloc() returns NULL.
>
> The problem was found by an experimental code review agent based on
> gemini-3.1-pro while reviewing backports into v6.18.y.
>
> Assisted-by: Gemini:gemini-3.1-pro
> Cc: Miaoqian Lin <linmq006@gmail.com>
> Cc: Steven Rostedt (Google) <rostedt@goodmis.org>
> Fixes: 0550069cc25f ("tracing: Properly process error handling in event_hist_trigger_parse()")
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> kernel/trace/trace_events_hist.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index 73ea180cad55..a2abdfe19281 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -6874,7 +6874,7 @@ static int event_hist_trigger_parse(struct event_command *cmd_ops,
> trigger_data = trigger_data_alloc(cmd_ops, cmd, param, hist_data);
> if (!trigger_data) {
> ret = -ENOMEM;
> - goto out_free;
> + goto out_destroy;
> }
>
> ret = event_trigger_set_filter(cmd_ops, file, filter, trigger_data);
> @@ -6942,7 +6942,7 @@ static int event_hist_trigger_parse(struct event_command *cmd_ops,
> remove_hist_vars(hist_data);
>
> trigger_data_free(trigger_data);
I rather make trigger_data_free() more robust by starting it with:
if (!data)
return;
-- Steve
> -
> +out_destroy:
> destroy_hist_data(hist_data);
> goto out;
> }
^ permalink raw reply
* Re: [PATCH] tracing: Do not call trigger_data_free with NULL data pointer
From: Guenter Roeck @ 2026-03-05 16:57 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, Miaoqian Lin
In-Reply-To: <20260305114336.0f2710eb@gandalf.local.home>
On Thu, Mar 05, 2026 at 11:43:36AM -0500, Steven Rostedt wrote:
> On Thu, 5 Mar 2026 08:36:33 -0800
> Guenter Roeck <linux@roeck-us.net> wrote:
>
> > If trigger_data_alloc() fails and returns NULL, event_hist_trigger_parse()
> > jumps to the out_free error path. While kfree() safely handles a NULL
> > pointer, trigger_data_free() does not. This causes a NULL pointer
> > dereference in trigger_data_free() when evaluating
> > data->cmd_ops->set_filter.
> >
> > Fix the problem by adding a new goto label and jumping to it if
> > trigger_data_alloc() returns NULL.
> >
> > The problem was found by an experimental code review agent based on
> > gemini-3.1-pro while reviewing backports into v6.18.y.
> >
> > Assisted-by: Gemini:gemini-3.1-pro
> > Cc: Miaoqian Lin <linmq006@gmail.com>
> > Cc: Steven Rostedt (Google) <rostedt@goodmis.org>
> > Fixes: 0550069cc25f ("tracing: Properly process error handling in event_hist_trigger_parse()")
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> > kernel/trace/trace_events_hist.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> > index 73ea180cad55..a2abdfe19281 100644
> > --- a/kernel/trace/trace_events_hist.c
> > +++ b/kernel/trace/trace_events_hist.c
> > @@ -6874,7 +6874,7 @@ static int event_hist_trigger_parse(struct event_command *cmd_ops,
> > trigger_data = trigger_data_alloc(cmd_ops, cmd, param, hist_data);
> > if (!trigger_data) {
> > ret = -ENOMEM;
> > - goto out_free;
> > + goto out_destroy;
> > }
> >
> > ret = event_trigger_set_filter(cmd_ops, file, filter, trigger_data);
> > @@ -6942,7 +6942,7 @@ static int event_hist_trigger_parse(struct event_command *cmd_ops,
> > remove_hist_vars(hist_data);
> >
> > trigger_data_free(trigger_data);
>
> I rather make trigger_data_free() more robust by starting it with:
>
> if (!data)
> return;
Sure. No preference on my side. I'll send v2.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH v8 3/6] tracefs: Check file permission even if user has CAP_DAC_OVERRIDE
From: Steven Rostedt @ 2026-03-05 17:33 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel
In-Reply-To: <20260212151515.b384ac24de9b736d10387d21@kernel.org>
On Thu, 12 Feb 2026 15:15:15 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > With this still not working this late in the game, it will need to wait
> > until the next merge window. I'll take the first two patches of this
> > series now though.
>
> OK. I will send the next version without the first 2 patches.
Hi Masami,
Did you send a new version of this yet?
-- Steve
^ permalink raw reply
* Re: [PATCH RFC 3/3] locking: Wire up contended_release tracepoint
From: Peter Zijlstra @ 2026-03-05 17:42 UTC (permalink / raw)
To: Steven Rostedt
Cc: Dmitry Ilvokhin, Dennis Zhou, Tejun Heo, Christoph Lameter,
Masami Hiramatsu, Mathieu Desnoyers, Ingo Molnar, Will Deacon,
Boqun Feng, Waiman Long, linux-mm, linux-kernel,
linux-trace-kernel, kernel-team
In-Reply-To: <20260305105924.7069eb23@gandalf.local.home>
On Thu, Mar 05, 2026 at 10:59:24AM -0500, Steven Rostedt wrote:
> On Wed, 4 Mar 2026 16:56:17 +0000
> Dmitry Ilvokhin <d@ilvokhin.com> wrote:
>
> > @@ -204,6 +206,8 @@ static inline void rwbase_write_unlock(struct rwbase_rt *rwb)
> > unsigned long flags;
> >
> > raw_spin_lock_irqsave(&rtm->wait_lock, flags);
> > + if (rt_mutex_has_waiters(rtm))
> > + trace_contended_release(rwb);
>
> Hmm, if statements should never be used just for tracepoints without a
> static branch. The above should be:
>
> if (trace_contended_release_enabled() && rt_mutex_has_waiters(rtm))
> trace_contended_release(rwb);
>
I still wish you would accept:
if (trace_foo_enabled() && foo)
__do_trace_foo();
The compilers can't optimize the static branches and thus you'll get it
twice for no reason.
I really wish they would just accept __pure, but alas.
^ permalink raw reply
* Re: [PATCH v5 2/3] ring-buffer: Handle RB_MISSED_* flags on commit field correctly
From: Steven Rostedt @ 2026-03-05 18:03 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel
In-Reply-To: <177211312362.419230.15156461178245984273.stgit@mhiramat.tok.corp.google.com>
On Thu, 26 Feb 2026 22:38:43 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> Since the MSBs of rb_data_page::commit are used for storing
> RB_MISSED_EVENTS and RB_MISSED_STORED, we need to mask out those bits
> when it is used for finding the size of data pages.
>
> Fixes: 5f3b6e839f3c ("ring-buffer: Validate boot range memory events")
> Fixes: 5b7be9c709e1 ("ring-buffer: Add test to validate the time stamp deltas")
> Cc: stable@vger.kernel.org
This is unneeded for the current way things work.
The missed events flags are added when a page is read, so the commits in
the write buffer should never have those flags set. If they did, the ring
buffer code itself would break.
But as patch 3 is adding a flag, you should likely merge this and patch 3
together, as the only way that flag would get set is if the validator set
it on a previous boot. And then this would be needed for subsequent boots
that did not reset the buffer.
Hmm, I don't think we even need to do that! Because if it is set, it would
simply warn again that a page is invalid, and I think we *want* that! As it
would preserve that pages were invalid and not be cleared with a simple
reboot.
-- 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