* Re: [PATCH bpf-next 2/2] selftests/bpf: test bpf_get_func_arg() for tp_btf
From: bot+bpf-ci @ 2026-01-16 4:09 UTC (permalink / raw)
To: menglong8.dong, ast
Cc: daniel, john.fastabend, andrii, martin.lau, eddyz87, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, mattbobrowski,
rostedt, mhiramat, mathieu.desnoyers, bpf, linux-kernel,
linux-trace-kernel, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
In-Reply-To: <20260116035024.98214-3-dongml2@chinatelecom.cn>
[-- Attachment #1: Type: text/plain, Size: 1997 bytes --]
> diff --git a/tools/testing/selftests/bpf/prog_tests/get_func_args_test.c b/tools/testing/selftests/bpf/prog_tests/get_func_args_test.c
> index 64a9c95d4acf..848fab952719 100644
> --- a/tools/testing/selftests/bpf/prog_tests/get_func_args_test.c
> +++ b/tools/testing/selftests/bpf/prog_tests/get_func_args_test.c
> @@ -33,6 +33,7 @@ void test_get_func_args_test(void)
>
> ASSERT_EQ(topts.retval >> 16, 1, "test_run");
> ASSERT_EQ(topts.retval & 0xffff, 1234 + 29, "test_run");
> + ASSERT_OK(trigger_module_test_read(1), "trigger_read");
>
> ASSERT_EQ(skel->bss->test1_result, 1, "test1_result");
> ASSERT_EQ(skel->bss->test2_result, 1, "test2_result");
> ASSERT_EQ(skel->bss->test3_result, 1, "test3_result");
> ASSERT_EQ(skel->bss->test4_result, 1, "test4_result");
The trigger_module_test_read() call fires the new tracepoints which run the
tp_test1 and tp_test2 BPF programs. These programs set test5_result and
test6_result respectively. However, there are no assertions here to verify
that test5_result and test6_result equal 1.
Should there be:
ASSERT_EQ(skel->bss->test5_result, 1, "test5_result");
ASSERT_EQ(skel->bss->test6_result, 1, "test6_result");
Without these assertions, the test will pass even if the tp_btf
bpf_get_func_arg() functionality is broken.
> diff --git a/tools/testing/selftests/bpf/progs/get_func_args_test.c b/tools/testing/selftests/bpf/progs/get_func_args_test.c
[ ... ]
> +__u64 test5_result = 0;
> +SEC("tp_btf/bpf_testmod_fentry_test1_tp")
> +int BPF_PROG(tp_test1)
> +{
[ ... ]
> +__u64 test6_result = 0;
> +SEC("tp_btf/bpf_testmod_fentry_test2_tp")
> +int BPF_PROG(tp_test2)
> +{
These result variables are set by the BPF programs but never checked in the
test driver, as noted above.
---
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/21055137351
^ permalink raw reply
* [PATCH bpf-next 2/2] selftests/bpf: test bpf_get_func_arg() for tp_btf
From: Menglong Dong @ 2026-01-16 3:50 UTC (permalink / raw)
To: ast
Cc: daniel, john.fastabend, andrii, martin.lau, eddyz87, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, mattbobrowski,
rostedt, mhiramat, mathieu.desnoyers, bpf, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260116035024.98214-1-dongml2@chinatelecom.cn>
Test bpf_get_func_arg() and bpf_get_func_arg_cnt() for tp_btf. The code
is most copied from test1 and test2.
Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
---
.../bpf/prog_tests/get_func_args_test.c | 1 +
.../selftests/bpf/progs/get_func_args_test.c | 44 +++++++++++++++++++
.../bpf/test_kmods/bpf_testmod-events.h | 10 +++++
.../selftests/bpf/test_kmods/bpf_testmod.c | 4 ++
4 files changed, 59 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/get_func_args_test.c b/tools/testing/selftests/bpf/prog_tests/get_func_args_test.c
index 64a9c95d4acf..848fab952719 100644
--- a/tools/testing/selftests/bpf/prog_tests/get_func_args_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/get_func_args_test.c
@@ -33,6 +33,7 @@ void test_get_func_args_test(void)
ASSERT_EQ(topts.retval >> 16, 1, "test_run");
ASSERT_EQ(topts.retval & 0xffff, 1234 + 29, "test_run");
+ ASSERT_OK(trigger_module_test_read(1), "trigger_read");
ASSERT_EQ(skel->bss->test1_result, 1, "test1_result");
ASSERT_EQ(skel->bss->test2_result, 1, "test2_result");
diff --git a/tools/testing/selftests/bpf/progs/get_func_args_test.c b/tools/testing/selftests/bpf/progs/get_func_args_test.c
index e0f34a55e697..5b7233afef05 100644
--- a/tools/testing/selftests/bpf/progs/get_func_args_test.c
+++ b/tools/testing/selftests/bpf/progs/get_func_args_test.c
@@ -121,3 +121,47 @@ int BPF_PROG(fexit_test, int _a, int *_b, int _ret)
test4_result &= err == 0 && ret == 1234;
return 0;
}
+
+__u64 test5_result = 0;
+SEC("tp_btf/bpf_testmod_fentry_test1_tp")
+int BPF_PROG(tp_test1)
+{
+ __u64 cnt = bpf_get_func_arg_cnt(ctx);
+ __u64 a = 0, z = 0;
+ __s64 err;
+
+ test5_result = cnt == 1;
+
+ err = bpf_get_func_arg(ctx, 0, &a);
+ test5_result &= err == 0 && ((int) a == 1);
+
+ /* not valid argument */
+ err = bpf_get_func_arg(ctx, 1, &z);
+ test5_result &= err == -EINVAL;
+
+ return 0;
+}
+
+__u64 test6_result = 0;
+SEC("tp_btf/bpf_testmod_fentry_test2_tp")
+int BPF_PROG(tp_test2)
+{
+ __u64 cnt = bpf_get_func_arg_cnt(ctx);
+ __u64 a = 0, b = 0, z = 0;
+ __s64 err;
+
+ test6_result = cnt == 2;
+
+ /* valid arguments */
+ err = bpf_get_func_arg(ctx, 0, &a);
+ test6_result &= err == 0 && (int) a == 2;
+
+ err = bpf_get_func_arg(ctx, 1, &b);
+ test6_result &= err == 0 && b == 3;
+
+ /* not valid argument */
+ err = bpf_get_func_arg(ctx, 2, &z);
+ test6_result &= err == -EINVAL;
+
+ return 0;
+}
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h b/tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h
index aeef86b3da74..45a5e41f3a92 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h
@@ -63,6 +63,16 @@ BPF_TESTMOD_DECLARE_TRACE(bpf_testmod_test_writable_bare,
sizeof(struct bpf_testmod_test_writable_ctx)
);
+DECLARE_TRACE(bpf_testmod_fentry_test1,
+ TP_PROTO(int a),
+ TP_ARGS(a)
+);
+
+DECLARE_TRACE(bpf_testmod_fentry_test2,
+ TP_PROTO(int a, u64 b),
+ TP_ARGS(a, b)
+);
+
#endif /* _BPF_TESTMOD_EVENTS_H */
#undef TRACE_INCLUDE_PATH
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
index bc07ce9d5477..f3698746f033 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
@@ -396,11 +396,15 @@ __weak noinline struct file *bpf_testmod_return_ptr(int arg)
noinline int bpf_testmod_fentry_test1(int a)
{
+ trace_bpf_testmod_fentry_test1_tp(a);
+
return a + 1;
}
noinline int bpf_testmod_fentry_test2(int a, u64 b)
{
+ trace_bpf_testmod_fentry_test2_tp(a, b);
+
return a + b;
}
--
2.52.0
^ permalink raw reply related
* [PATCH bpf-next 1/2] bpf: support bpf_get_func_arg() for BPF_TRACE_RAW_TP
From: Menglong Dong @ 2026-01-16 3:50 UTC (permalink / raw)
To: ast
Cc: daniel, john.fastabend, andrii, martin.lau, eddyz87, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, mattbobrowski,
rostedt, mhiramat, mathieu.desnoyers, bpf, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260116035024.98214-1-dongml2@chinatelecom.cn>
For now, bpf_get_func_arg() and bpf_get_func_arg_cnt() is not supported by
the BPF_TRACE_RAW_TP, which is not convenient to get the argument of the
tracepoint, especially for the case that the position of the arguments in
a tracepoint can change.
The target tracepoint BTF type id is specified during loading time,
therefore we can get the function argument conut from the function
prototype instead of the stack.
Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
---
kernel/bpf/verifier.c | 28 ++++++++++++++++++++++++----
kernel/trace/bpf_trace.c | 4 ++--
2 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index faa1ecc1fe9d..6dee0defa291 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -23316,8 +23316,18 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
/* Implement bpf_get_func_arg inline. */
if (prog_type == BPF_PROG_TYPE_TRACING &&
insn->imm == BPF_FUNC_get_func_arg) {
- /* Load nr_args from ctx - 8 */
- insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
+ if (eatype == BPF_TRACE_RAW_TP) {
+ int nr_args;
+
+ if (!prog->aux->attach_func_proto)
+ return -EINVAL;
+ nr_args = btf_type_vlen(prog->aux->attach_func_proto);
+ /* Save nr_args to reg0 */
+ insn_buf[0] = BPF_MOV64_IMM(BPF_REG_0, nr_args);
+ } else {
+ /* Load nr_args from ctx - 8 */
+ insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
+ }
insn_buf[1] = BPF_JMP32_REG(BPF_JGE, BPF_REG_2, BPF_REG_0, 6);
insn_buf[2] = BPF_ALU64_IMM(BPF_LSH, BPF_REG_2, 3);
insn_buf[3] = BPF_ALU64_REG(BPF_ADD, BPF_REG_2, BPF_REG_1);
@@ -23369,8 +23379,18 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
/* Implement get_func_arg_cnt inline. */
if (prog_type == BPF_PROG_TYPE_TRACING &&
insn->imm == BPF_FUNC_get_func_arg_cnt) {
- /* Load nr_args from ctx - 8 */
- insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
+ if (eatype == BPF_TRACE_RAW_TP) {
+ int nr_args;
+
+ if (!prog->aux->attach_func_proto)
+ return -EINVAL;
+ nr_args = btf_type_vlen(prog->aux->attach_func_proto);
+ /* Save nr_args to reg0 */
+ insn_buf[0] = BPF_MOV64_IMM(BPF_REG_0, nr_args);
+ } else {
+ /* Load nr_args from ctx - 8 */
+ insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
+ }
new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, 1);
if (!new_prog)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 6e076485bf70..9b1b56851d26 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1734,11 +1734,11 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
case BPF_FUNC_d_path:
return &bpf_d_path_proto;
case BPF_FUNC_get_func_arg:
- return bpf_prog_has_trampoline(prog) ? &bpf_get_func_arg_proto : NULL;
+ return &bpf_get_func_arg_proto;
case BPF_FUNC_get_func_ret:
return bpf_prog_has_trampoline(prog) ? &bpf_get_func_ret_proto : NULL;
case BPF_FUNC_get_func_arg_cnt:
- return bpf_prog_has_trampoline(prog) ? &bpf_get_func_arg_cnt_proto : NULL;
+ return &bpf_get_func_arg_cnt_proto;
case BPF_FUNC_get_attach_cookie:
if (prog->type == BPF_PROG_TYPE_TRACING &&
prog->expected_attach_type == BPF_TRACE_RAW_TP)
--
2.52.0
^ permalink raw reply related
* [PATCH bpf-next 0/2] bpf: support bpf_get_func_arg() for BPF_TRACE_RAW_TP
From: Menglong Dong @ 2026-01-16 3:50 UTC (permalink / raw)
To: ast
Cc: daniel, john.fastabend, andrii, martin.lau, eddyz87, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, mattbobrowski,
rostedt, mhiramat, mathieu.desnoyers, bpf, linux-kernel,
linux-trace-kernel
Support bpf_get_func_arg() for BPF_TRACE_RAW_TP by getting the function
argument count from tracepoint prototype during verifier inline.
Menglong Dong (2):
bpf: support bpf_get_func_arg() for BPF_TRACE_RAW_TP
selftests/bpf: test bpf_get_func_arg() for tp_btf
kernel/bpf/verifier.c | 28 ++++++++++--
kernel/trace/bpf_trace.c | 4 +-
.../bpf/prog_tests/get_func_args_test.c | 1 +
.../selftests/bpf/progs/get_func_args_test.c | 44 +++++++++++++++++++
.../bpf/test_kmods/bpf_testmod-events.h | 10 +++++
.../selftests/bpf/test_kmods/bpf_testmod.c | 4 ++
6 files changed, 85 insertions(+), 6 deletions(-)
--
2.52.0
^ permalink raw reply
* Re: [PATCHv3 bpf-next 1/2] arm64/ftrace,bpf: Fix partial regs after bpf_prog_run
From: patchwork-bot+netdevbpf @ 2026-01-16 0:40 UTC (permalink / raw)
To: Jiri Olsa
Cc: mhiramat, rostedt, will, mahe.tardy, peterz, bpf,
linux-trace-kernel, linux-arm-kernel, x86, yhs, songliubraving,
andrii, mark.rutland
In-Reply-To: <20260112121157.854473-1-jolsa@kernel.org>
Hello:
This series was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:
On Mon, 12 Jan 2026 13:11:56 +0100 you wrote:
> Mahe reported issue with bpf_override_return helper not working when
> executed from kprobe.multi bpf program on arm.
>
> The problem is that on arm we use alternate storage for pt_regs object
> that is passed to bpf_prog_run and if any register is changed (which
> is the case of bpf_override_return) it's not propagated back to actual
> pt_regs object.
>
> [...]
Here is the summary with links:
- [PATCHv3,bpf-next,1/2] arm64/ftrace,bpf: Fix partial regs after bpf_prog_run
https://git.kernel.org/bpf/bpf-next/c/276f3b6daf60
- [PATCHv3,bpf-next,2/2] selftests/bpf: Add test for bpf_override_return helper
https://git.kernel.org/bpf/bpf-next/c/934d9746ed02
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v3 0/2] mm/vmscan: mitigate spurious kswapd_failures reset and add tracepoints
From: Andrew Morton @ 2026-01-15 23:39 UTC (permalink / raw)
To: Jiayuan Chen
Cc: linux-mm, shakeel.butt, 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,
Brendan Jackman, Johannes Weiner, Zi Yan, Qi Zheng, Jiayuan Chen,
linux-kernel, linux-trace-kernel
In-Reply-To: <20260114074049.229935-1-jiayuan.chen@linux.dev>
On Wed, 14 Jan 2026 15:40:34 +0800 Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
> == Problem ==
>
> We observed an issue in production on a multi-NUMA system where kswapd
> runs endlessly, causing sustained heavy IO READ pressure across the
> entire system.
>
> == Solution ==
>
> ...
>
Thanks, I'll update mm.git to this version to keep the series under
test while Michal and Shakeel continue their review (please!).
^ permalink raw reply
* Re: [PATCH v3 1/2] cpufreq: Replace trace_cpu_frequency with trace_policy_frequency
From: Samuel Wu @ 2026-01-15 23:35 UTC (permalink / raw)
To: Steven Rostedt
Cc: Rafael J. Wysocki, Christian Loehle, Huang Rui, Gautham R. Shenoy,
Mario Limonciello, Perry Yuan, Jonathan Corbet, Viresh Kumar,
Masami Hiramatsu, Mathieu Desnoyers, Srinivas Pandruvada,
Len Brown, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Ian Rogers,
Adrian Hunter, James Clark, kernel-team, linux-pm, linux-doc,
linux-kernel, linux-trace-kernel, bpf, linux-perf-users
In-Reply-To: <20251229170021.71cc5425@gandalf.local.home>
On Mon, Dec 29, 2025 at 2:00 PM Steven Rostedt <steven@rostedt.org> wrote:
>
> On Mon, 29 Dec 2025 16:52:12 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > On Thu, 4 Dec 2025 18:24:57 +0100
> > "Rafael J. Wysocki" <rafael@kernel.org> wrote:
> >
> > > My concern is that the patch effectively removes one trace point
> > > (cpu_frequency) and adds another one with a different format
> > > (policy_frequency), updates one utility in the kernel tree and expects
> > > everyone else to somehow know that they should switch over.
> > >
> > > I know about at least several people who have their own scripts using
> > > this tracepoint though.
> >
> > Hi Rafael,
> >
> > Can you reach out to those that have scripts that use this trace event to
> > see if it can be changed?
> >
> > Thanks,
>
> I got a bunch of "Undelivered Mail Returned to Sender". It seems that gmail
> thinks my goodmis.org account is now spam :-p
>
> -- Steve
>
Hi Rafael,
Bumping thread since it's unclear if Steven's email has gone through.
Are you able to reach out to those with the scripts using this
tracepoint to see if they can be changed? Hopefully this update can be
proactive, but I'm optimistic even a reactive update would be
straightforward.
-- Sam
^ permalink raw reply
* Re: [PATCH v3 03/18] rtla: Simplify argument parsing
From: Costa Shulyupin @ 2026-01-15 21:47 UTC (permalink / raw)
To: Wander Lairson Costa
Cc: Steven Rostedt, Tomas Glozar, Crystal Wood, Ivan Pravdin,
John Kacur, Haiyong Sun, Tiezhu Yang, Daniel Wagner,
Daniel Bristot de Oliveira,
open list:Real-time Linux Analysis (RTLA) tools,
open list:Real-time Linux Analysis (RTLA) tools,
open list:BPF [MISC]:Keyword:(?:b|_)bpf(?:b|_)
In-Reply-To: <20260115163650.118910-4-wander@redhat.com>
On Thu, 15 Jan 2026 at 19:25, Wander Lairson Costa <wander@redhat.com> wrote:
> To simplify and improve the robustness of argument parsing, introduce a
> new extract_arg() helper macro. This macro extracts the value from a
> "key=value" pair, making the code more concise and readable.
Would you consider using getsubopt?
Costa
^ permalink raw reply
* Re: [PATCHv6 bpf-next 0/9] ftrace,bpf: Use single direct ops for bpf trampolines
From: Andrii Nakryiko @ 2026-01-15 18:54 UTC (permalink / raw)
To: Jiri Olsa, Steven Rostedt
Cc: Florent Revest, Mark Rutland, bpf, linux-kernel,
linux-trace-kernel, linux-arm-kernel, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Menglong Dong, Song Liu
In-Reply-To: <20251230145010.103439-1-jolsa@kernel.org>
On Tue, Dec 30, 2025 at 6:50 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> hi,
> while poking the multi-tracing interface I ended up with just one ftrace_ops
> object to attach all trampolines.
>
> This change allows to use less direct API calls during the attachment changes
> in the future code, so in effect speeding up the attachment.
>
> In current code we get a speed up from using just a single ftrace_ops object.
>
> - with current code:
>
> Performance counter stats for 'bpftrace -e fentry:vmlinux:ksys_* {} -c true':
>
> 6,364,157,902 cycles:k
> 828,728,902 cycles:u
> 1,064,803,824 instructions:u # 1.28 insn per cycle
> 23,797,500,067 instructions:k # 3.74 insn per cycle
>
> 4.416004987 seconds time elapsed
>
> 0.164121000 seconds user
> 1.289550000 seconds sys
>
>
> - with the fix:
>
> Performance counter stats for 'bpftrace -e fentry:vmlinux:ksys_* {} -c true':
>
> 6,535,857,905 cycles:k
> 810,809,429 cycles:u
> 1,064,594,027 instructions:u # 1.31 insn per cycle
> 23,962,552,894 instructions:k # 3.67 insn per cycle
>
> 1.666961239 seconds time elapsed
>
> 0.157412000 seconds user
> 1.283396000 seconds sys
>
>
>
> The speedup seems to be related to the fact that with single ftrace_ops object
> we don't call ftrace_shutdown anymore (we use ftrace_update_ops instead) and
> we skip the synchronize rcu calls (each ~100ms) at the end of that function.
>
> rfc: https://lore.kernel.org/bpf/20250729102813.1531457-1-jolsa@kernel.org/
> v1: https://lore.kernel.org/bpf/20250923215147.1571952-1-jolsa@kernel.org/
> v2: https://lore.kernel.org/bpf/20251113123750.2507435-1-jolsa@kernel.org/
> v3: https://lore.kernel.org/bpf/20251120212402.466524-1-jolsa@kernel.org/
> v4: https://lore.kernel.org/bpf/20251203082402.78816-1-jolsa@kernel.org/
> v5: https://lore.kernel.org/bpf/20251215211402.353056-10-jolsa@kernel.org/
>
> v6 changes:
> - rename add_hash_entry_direct to add_ftrace_hash_entry_direct [Steven]
> - factor hash_add/hash_sub [Steven]
> - add kerneldoc header for update_ftrace_direct_* functions [Steven]
> - few assorted smaller fixes [Steven]
> - added missing direct_ops wrappers for !CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> case [Steven]
>
So this looks good from BPF side, I think. Steven, if you don't mind
giving this patch set another look and if everything is to your liking
giving your ack, we can then apply it to bpf-next. Thanks!
> v5 changes:
> - do not export ftrace_hash object [Steven]
> - fix update_ftrace_direct_add new_filter_hash leak [ci]
>
> v4 changes:
> - rebased on top of bpf-next/master (with jmp attach changes)
> added patch 1 to deal with that
> - added extra checks for update_ftrace_direct_del/mod to address
> the ci bot review
>
> v3 changes:
> - rebased on top of bpf-next/master
> - fixed update_ftrace_direct_del cleanup path
> - added missing inline to update_ftrace_direct_* stubs
>
> v2 changes:
> - rebased on top fo bpf-next/master plus Song's livepatch fixes [1]
> - renamed the API functions [2] [Steven]
> - do not export the new api [Steven]
> - kept the original direct interface:
>
> I'm not sure if we want to melt both *_ftrace_direct and the new interface
> into single one. It's bit different in semantic (hence the name change as
> Steven suggested [2]) and I don't think the changes are not that big so
> we could easily keep both APIs.
>
> v1 changes:
> - make the change x86 specific, after discussing with Mark options for
> arm64 [Mark]
>
> thanks,
> jirka
>
>
> [1] https://lore.kernel.org/bpf/20251027175023.1521602-1-song@kernel.org/
> [2] https://lore.kernel.org/bpf/20250924050415.4aefcb91@batman.local.home/
> ---
> Jiri Olsa (9):
> ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace_ops flag
> ftrace: Make alloc_and_copy_ftrace_hash direct friendly
> ftrace: Export some of hash related functions
> ftrace: Add update_ftrace_direct_add function
> ftrace: Add update_ftrace_direct_del function
> ftrace: Add update_ftrace_direct_mod function
> bpf: Add trampoline ip hash table
> ftrace: Factor ftrace_ops ops_func interface
> bpf,x86: Use single ftrace_ops for direct calls
>
> arch/x86/Kconfig | 1 +
> include/linux/bpf.h | 7 ++-
> include/linux/ftrace.h | 31 +++++++++-
> kernel/bpf/trampoline.c | 259 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------
> kernel/trace/Kconfig | 3 +
> kernel/trace/ftrace.c | 406 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------
> 6 files changed, 632 insertions(+), 75 deletions(-)
>
^ permalink raw reply
* Re: [PATCH bpf-next 2/4] x86/fgraph,bpf: Switch kprobe_multi program stack unwind to hw_regs path
From: Andrii Nakryiko @ 2026-01-15 18:52 UTC (permalink / raw)
To: Jiri Olsa
Cc: Steven Rostedt, Masami Hiramatsu, Josh Poimboeuf, Mahe Tardy,
Peter Zijlstra, bpf, linux-trace-kernel, x86, Yonghong Song,
Song Liu, Andrii Nakryiko
In-Reply-To: <aWYv6864cdO2PWbb@krava>
On Tue, Jan 13, 2026 at 3:43 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Mon, Jan 12, 2026 at 05:07:57PM -0500, Steven Rostedt wrote:
> > On Mon, 12 Jan 2026 22:49:38 +0100
> > Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > > To recreate same stack setup for return probe as we have for entry
> > > probe, we set the instruction pointer to the attached function address,
> > > which gets us the same unwind setup and same stack trace.
> > >
> > > With the fix, entry probe:
> > >
> > > # bpftrace -e 'kprobe:__x64_sys_newuname* { print(kstack)}'
> > > Attaching 1 probe...
> > >
> > > __x64_sys_newuname+9
> > > do_syscall_64+134
> > > entry_SYSCALL_64_after_hwframe+118
> > >
> > > return probe:
> > >
> > > # bpftrace -e 'kretprobe:__x64_sys_newuname* { print(kstack)}'
> > > Attaching 1 probe...
> > >
> > > __x64_sys_newuname+4
> > > do_syscall_64+134
> > > entry_SYSCALL_64_after_hwframe+118
> >
> > But is this really correct?
> >
> > The stack trace of the return from __x86_sys_newuname is from offset "+4".
> >
> > The stack trace from entry is offset "+9". Isn't it confusing that the
> > offset is likely not from the return portion of that function?
>
> right, makes sense.. so standard kprobe actualy skips attached function
> (__x86_sys_newuname) on return probe stacktrace.. perhaps we should do
> the same for kprobe_multi
but it is quite nice to see what function we were kretprobing,
actually... How hard would it be to support that for singular kprobe
as well? And what does fexit's stack trace show for such case?
>
> I managed to get that with the change below, but it's wrong wrt arch code,
> note the ftrace_regs_set_stack_pointer(fregs, stack + 8) .. will try to
> figure out better way when we agree on the solution
>
> thanks,
> jirka
>
>
> ---
> diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
> index c56e1e63b893..b0e8ce4934e7 100644
> --- a/arch/x86/include/asm/ftrace.h
> +++ b/arch/x86/include/asm/ftrace.h
> @@ -71,6 +71,9 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs)
> #define ftrace_regs_set_instruction_pointer(fregs, _ip) \
> do { arch_ftrace_regs(fregs)->regs.ip = (_ip); } while (0)
>
> +#define ftrace_regs_set_stack_pointer(fregs, _sp) \
> + do { arch_ftrace_regs(fregs)->regs.sp = (_sp); } while (0)
> +
>
> static __always_inline unsigned long
> ftrace_regs_get_return_address(struct ftrace_regs *fregs)
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index 6279e0a753cf..b1510c412dcb 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -717,7 +717,8 @@ int function_graph_enter_regs(unsigned long ret, unsigned long func,
> /* Retrieve a function return address to the trace stack on thread info.*/
> static struct ftrace_ret_stack *
> ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
> - unsigned long frame_pointer, int *offset)
> + unsigned long *stack, unsigned long frame_pointer,
> + int *offset)
> {
> struct ftrace_ret_stack *ret_stack;
>
> @@ -762,6 +763,7 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
>
> *offset += FGRAPH_FRAME_OFFSET;
> *ret = ret_stack->ret;
> + *stack = (unsigned long) ret_stack->retp;
> trace->func = ret_stack->func;
> trace->overrun = atomic_read(¤t->trace_overrun);
> trace->depth = current->curr_ret_depth;
> @@ -810,12 +812,13 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
> struct ftrace_ret_stack *ret_stack;
> struct ftrace_graph_ret trace;
> unsigned long bitmap;
> + unsigned long stack;
> unsigned long ret;
> int offset;
> int bit;
> int i;
>
> - ret_stack = ftrace_pop_return_trace(&trace, &ret, frame_pointer, &offset);
> + ret_stack = ftrace_pop_return_trace(&trace, &ret, &stack, frame_pointer, &offset);
>
> if (unlikely(!ret_stack)) {
> ftrace_graph_stop();
> @@ -824,8 +827,11 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
> return (unsigned long)panic;
> }
>
> - if (fregs)
> - ftrace_regs_set_instruction_pointer(fregs, trace.func);
> + if (fregs) {
> + ftrace_regs_set_instruction_pointer(fregs, ret);
> + ftrace_regs_set_stack_pointer(fregs, stack + 8);
> + }
> +
>
> bit = ftrace_test_recursion_trylock(trace.func, ret);
> /*
> diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_ips.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_ips.c
> index e1a9b55e07cb..852830536109 100644
> --- a/tools/testing/selftests/bpf/prog_tests/stacktrace_ips.c
> +++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_ips.c
> @@ -74,12 +74,20 @@ static void test_stacktrace_ips_kprobe_multi(bool retprobe)
>
> load_kallsyms();
>
> - check_stacktrace_ips(bpf_map__fd(skel->maps.stackmap), skel->bss->stack_key, 5,
> - ksym_get_addr("bpf_testmod_stacktrace_test"),
> - ksym_get_addr("bpf_testmod_stacktrace_test_3"),
> - ksym_get_addr("bpf_testmod_stacktrace_test_2"),
> - ksym_get_addr("bpf_testmod_stacktrace_test_1"),
> - ksym_get_addr("bpf_testmod_test_read"));
> + if (retprobe) {
> + check_stacktrace_ips(bpf_map__fd(skel->maps.stackmap), skel->bss->stack_key, 4,
> + ksym_get_addr("bpf_testmod_stacktrace_test_3"),
> + ksym_get_addr("bpf_testmod_stacktrace_test_2"),
> + ksym_get_addr("bpf_testmod_stacktrace_test_1"),
> + ksym_get_addr("bpf_testmod_test_read"));
> + } else {
> + check_stacktrace_ips(bpf_map__fd(skel->maps.stackmap), skel->bss->stack_key, 5,
> + ksym_get_addr("bpf_testmod_stacktrace_test"),
> + ksym_get_addr("bpf_testmod_stacktrace_test_3"),
> + ksym_get_addr("bpf_testmod_stacktrace_test_2"),
> + ksym_get_addr("bpf_testmod_stacktrace_test_1"),
> + ksym_get_addr("bpf_testmod_test_read"));
> + }
>
> cleanup:
> stacktrace_ips__destroy(skel);
^ permalink raw reply
* Re: [PATCH bpf-next 4/4] selftests/bpf: Allow to benchmark trigger with stacktrace
From: Andrii Nakryiko @ 2026-01-15 18:50 UTC (permalink / raw)
To: Jiri Olsa
Cc: Masami Hiramatsu, Steven Rostedt, Josh Poimboeuf, Peter Zijlstra,
bpf, linux-trace-kernel, x86, Yonghong Song, Song Liu,
Andrii Nakryiko, Mahe Tardy
In-Reply-To: <CAEf4BzaXhGpkycs-TO_1V81-irq3d8Mjfyk=LMc0OC-NW-FnRg@mail.gmail.com>
On Thu, Jan 15, 2026 at 10:48 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Jan 12, 2026 at 1:50 PM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding support to call bpf_get_stackid helper from trigger programs,
> > so far added for kprobe multi.
> >
> > Adding the --stacktrace/-g option to enable it.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > tools/testing/selftests/bpf/bench.c | 4 ++++
> > tools/testing/selftests/bpf/bench.h | 1 +
> > .../selftests/bpf/benchs/bench_trigger.c | 1 +
> > .../selftests/bpf/progs/trigger_bench.c | 18 ++++++++++++++++++
> > 4 files changed, 24 insertions(+)
> >
>
> This now actually becomes a stack trace benchmark :) But I don't mind,
> I think it would be good to be able to benchmark this. But I think we
> should then implement it for all different tracing programs (tp,
> raw_tp, fentry/fexit/fmod_ret) for consistency and so we can compare
> and contrast?...
>
> > diff --git a/tools/testing/selftests/bpf/bench.c b/tools/testing/selftests/bpf/bench.c
> > index bd29bb2e6cb5..8dadd9c928ec 100644
> > --- a/tools/testing/selftests/bpf/bench.c
> > +++ b/tools/testing/selftests/bpf/bench.c
> > @@ -265,6 +265,7 @@ static const struct argp_option opts[] = {
> > { "verbose", 'v', NULL, 0, "Verbose debug output"},
> > { "affinity", 'a', NULL, 0, "Set consumer/producer thread affinity"},
> > { "quiet", 'q', NULL, 0, "Be more quiet"},
> > + { "stacktrace", 'g', NULL, 0, "Get stack trace"},
>
> bikeshedding time: why "g"? why not -S or something like that?
>
> > { "prod-affinity", ARG_PROD_AFFINITY_SET, "CPUSET", 0,
> > "Set of CPUs for producer threads; implies --affinity"},
> > { "cons-affinity", ARG_CONS_AFFINITY_SET, "CPUSET", 0,
> > @@ -350,6 +351,9 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
> > case 'q':
> > env.quiet = true;
> > break;
> > + case 'g':
> > + env.stacktrace = true;
> > + break;
> > case ARG_PROD_AFFINITY_SET:
> > env.affinity = true;
> > if (parse_num_list(arg, &env.prod_cpus.cpus,
> > diff --git a/tools/testing/selftests/bpf/bench.h b/tools/testing/selftests/bpf/bench.h
> > index bea323820ffb..7cf21936e7ed 100644
> > --- a/tools/testing/selftests/bpf/bench.h
> > +++ b/tools/testing/selftests/bpf/bench.h
> > @@ -26,6 +26,7 @@ struct env {
> > bool list;
> > bool affinity;
> > bool quiet;
> > + bool stacktrace;
> > int consumer_cnt;
> > int producer_cnt;
> > int nr_cpus;
> > diff --git a/tools/testing/selftests/bpf/benchs/bench_trigger.c b/tools/testing/selftests/bpf/benchs/bench_trigger.c
> > index 34018fc3927f..aeec9edd3851 100644
> > --- a/tools/testing/selftests/bpf/benchs/bench_trigger.c
> > +++ b/tools/testing/selftests/bpf/benchs/bench_trigger.c
> > @@ -146,6 +146,7 @@ static void setup_ctx(void)
> > bpf_program__set_autoload(ctx.skel->progs.trigger_driver, true);
> >
> > ctx.skel->rodata->batch_iters = args.batch_iters;
> > + ctx.skel->rodata->stacktrace = env.stacktrace;
> > }
> >
> > static void load_ctx(void)
> > diff --git a/tools/testing/selftests/bpf/progs/trigger_bench.c b/tools/testing/selftests/bpf/progs/trigger_bench.c
> > index 2898b3749d07..479400d96fa4 100644
> > --- a/tools/testing/selftests/bpf/progs/trigger_bench.c
> > +++ b/tools/testing/selftests/bpf/progs/trigger_bench.c
> > @@ -25,6 +25,23 @@ static __always_inline void inc_counter(void)
> > __sync_add_and_fetch(&hits[cpu & CPU_MASK].value, 1);
> > }
> >
> > +volatile const int stacktrace;
> > +
> > +typedef __u64 stack_trace_t[128];
> > +
> > +struct {
> > + __uint(type, BPF_MAP_TYPE_STACK_TRACE);
> > + __uint(max_entries, 16384);
> > + __type(key, __u32);
> > + __type(value, stack_trace_t);
> > +} stackmap SEC(".maps");
oh, and why bother with STACK_TRACE map, just call bpf_get_stack() API
and have maybe per-CPU scratch array for stack trace (per-CPU so that
in multi-cpu benchmarks they don't just contend on the same cache
lines)
> > +
> > +static __always_inline void do_stacktrace(void *ctx)
> > +{
> > + if (stacktrace)
> > + bpf_get_stackid(ctx, &stackmap, 0);
> > +}
> > +
> > SEC("?uprobe")
> > int bench_trigger_uprobe(void *ctx)
> > {
> > @@ -96,6 +113,7 @@ SEC("?kprobe.multi/bpf_get_numa_node_id")
> > int bench_trigger_kprobe_multi(void *ctx)
> > {
> > inc_counter();
> > + do_stacktrace(ctx);
> > return 0;
> > }
> >
> > --
> > 2.52.0
> >
^ permalink raw reply
* Re: [PATCH bpf-next 4/4] selftests/bpf: Allow to benchmark trigger with stacktrace
From: Andrii Nakryiko @ 2026-01-15 18:48 UTC (permalink / raw)
To: Jiri Olsa
Cc: Masami Hiramatsu, Steven Rostedt, Josh Poimboeuf, Peter Zijlstra,
bpf, linux-trace-kernel, x86, Yonghong Song, Song Liu,
Andrii Nakryiko, Mahe Tardy
In-Reply-To: <20260112214940.1222115-5-jolsa@kernel.org>
On Mon, Jan 12, 2026 at 1:50 PM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding support to call bpf_get_stackid helper from trigger programs,
> so far added for kprobe multi.
>
> Adding the --stacktrace/-g option to enable it.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/testing/selftests/bpf/bench.c | 4 ++++
> tools/testing/selftests/bpf/bench.h | 1 +
> .../selftests/bpf/benchs/bench_trigger.c | 1 +
> .../selftests/bpf/progs/trigger_bench.c | 18 ++++++++++++++++++
> 4 files changed, 24 insertions(+)
>
This now actually becomes a stack trace benchmark :) But I don't mind,
I think it would be good to be able to benchmark this. But I think we
should then implement it for all different tracing programs (tp,
raw_tp, fentry/fexit/fmod_ret) for consistency and so we can compare
and contrast?...
> diff --git a/tools/testing/selftests/bpf/bench.c b/tools/testing/selftests/bpf/bench.c
> index bd29bb2e6cb5..8dadd9c928ec 100644
> --- a/tools/testing/selftests/bpf/bench.c
> +++ b/tools/testing/selftests/bpf/bench.c
> @@ -265,6 +265,7 @@ static const struct argp_option opts[] = {
> { "verbose", 'v', NULL, 0, "Verbose debug output"},
> { "affinity", 'a', NULL, 0, "Set consumer/producer thread affinity"},
> { "quiet", 'q', NULL, 0, "Be more quiet"},
> + { "stacktrace", 'g', NULL, 0, "Get stack trace"},
bikeshedding time: why "g"? why not -S or something like that?
> { "prod-affinity", ARG_PROD_AFFINITY_SET, "CPUSET", 0,
> "Set of CPUs for producer threads; implies --affinity"},
> { "cons-affinity", ARG_CONS_AFFINITY_SET, "CPUSET", 0,
> @@ -350,6 +351,9 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
> case 'q':
> env.quiet = true;
> break;
> + case 'g':
> + env.stacktrace = true;
> + break;
> case ARG_PROD_AFFINITY_SET:
> env.affinity = true;
> if (parse_num_list(arg, &env.prod_cpus.cpus,
> diff --git a/tools/testing/selftests/bpf/bench.h b/tools/testing/selftests/bpf/bench.h
> index bea323820ffb..7cf21936e7ed 100644
> --- a/tools/testing/selftests/bpf/bench.h
> +++ b/tools/testing/selftests/bpf/bench.h
> @@ -26,6 +26,7 @@ struct env {
> bool list;
> bool affinity;
> bool quiet;
> + bool stacktrace;
> int consumer_cnt;
> int producer_cnt;
> int nr_cpus;
> diff --git a/tools/testing/selftests/bpf/benchs/bench_trigger.c b/tools/testing/selftests/bpf/benchs/bench_trigger.c
> index 34018fc3927f..aeec9edd3851 100644
> --- a/tools/testing/selftests/bpf/benchs/bench_trigger.c
> +++ b/tools/testing/selftests/bpf/benchs/bench_trigger.c
> @@ -146,6 +146,7 @@ static void setup_ctx(void)
> bpf_program__set_autoload(ctx.skel->progs.trigger_driver, true);
>
> ctx.skel->rodata->batch_iters = args.batch_iters;
> + ctx.skel->rodata->stacktrace = env.stacktrace;
> }
>
> static void load_ctx(void)
> diff --git a/tools/testing/selftests/bpf/progs/trigger_bench.c b/tools/testing/selftests/bpf/progs/trigger_bench.c
> index 2898b3749d07..479400d96fa4 100644
> --- a/tools/testing/selftests/bpf/progs/trigger_bench.c
> +++ b/tools/testing/selftests/bpf/progs/trigger_bench.c
> @@ -25,6 +25,23 @@ static __always_inline void inc_counter(void)
> __sync_add_and_fetch(&hits[cpu & CPU_MASK].value, 1);
> }
>
> +volatile const int stacktrace;
> +
> +typedef __u64 stack_trace_t[128];
> +
> +struct {
> + __uint(type, BPF_MAP_TYPE_STACK_TRACE);
> + __uint(max_entries, 16384);
> + __type(key, __u32);
> + __type(value, stack_trace_t);
> +} stackmap SEC(".maps");
> +
> +static __always_inline void do_stacktrace(void *ctx)
> +{
> + if (stacktrace)
> + bpf_get_stackid(ctx, &stackmap, 0);
> +}
> +
> SEC("?uprobe")
> int bench_trigger_uprobe(void *ctx)
> {
> @@ -96,6 +113,7 @@ SEC("?kprobe.multi/bpf_get_numa_node_id")
> int bench_trigger_kprobe_multi(void *ctx)
> {
> inc_counter();
> + do_stacktrace(ctx);
> return 0;
> }
>
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH v1] tools/rtla: Generate optstring from long options
From: Wander Lairson Costa @ 2026-01-15 18:06 UTC (permalink / raw)
To: Costa Shulyupin
Cc: Steven Rostedt, Tomas Glozar, Crystal Wood, John Kacur,
Ivan Pravdin, linux-trace-kernel, linux-kernel
In-Reply-To: <20260108095011.2115719-1-costa.shul@redhat.com>
On Thu, Jan 08, 2026 at 11:49:55AM +0200, Costa Shulyupin wrote:
> getopt_long() processes short and long options independently.
> RTLA, like the majority of applications, uses both short and long
> variants for each logical option.
>
> Since the val member of struct option holds the letter of the short
> variant, the string of short options can be reconstructed from the
> array of long options.
>
> Add getopt_auto() to generate optstring from an array of long options,
> eliminating the need to maintain separate short option strings.
>
> Signed-off-by: Costa Shulyupin <costa.shul@redhat.com>
...
That's very clever.
Reviewed-by: Wander Lairson Costa <wander@redhat.com>
^ permalink raw reply
* [PATCH v3 18/18] rtla/utils: Fix loop condition in PID validation
From: Wander Lairson Costa @ 2026-01-15 16:32 UTC (permalink / raw)
To: Steven Rostedt, Tomas Glozar, Wander Lairson Costa, Crystal Wood,
Ivan Pravdin, Costa Shulyupin, John Kacur, Tiezhu Yang,
Daniel Wagner, Daniel Bristot de Oliveira,
open list:Real-time Linux Analysis (RTLA) tools,
open list:Real-time Linux Analysis (RTLA) tools,
open list:BPF [MISC]:Keyword:(?:b|_)bpf(?:b|_)
In-Reply-To: <20260115163650.118910-1-wander@redhat.com>
The procfs_is_workload_pid() function iterates through a directory
entry name to validate if it represents a process ID. The loop
condition checks if the pointer t_name is non-NULL, but since
incrementing a pointer never makes it NULL, this condition is always
true within the loop's context. Although the inner isdigit() check
catches the NUL terminator and breaks out of the loop, the condition
is semantically misleading and not idiomatic for C string processing.
Correct the loop condition from checking the pointer (t_name) to
checking the character it points to (*t_name). This ensures the loop
terminates when the NUL terminator is reached, aligning with standard
C string iteration practices. While the original code functioned
correctly due to the existing character validation, this change
improves code clarity and maintainability.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/utils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
index 1ea9980d8ecd3..3d47f3ed52dee 100644
--- a/tools/tracing/rtla/src/utils.c
+++ b/tools/tracing/rtla/src/utils.c
@@ -296,7 +296,7 @@ static int procfs_is_workload_pid(const char *comm_prefix, struct dirent *proc_e
return 0;
/* check if the string is a pid */
- for (t_name = proc_entry->d_name; t_name; t_name++) {
+ for (t_name = proc_entry->d_name; *t_name; t_name++) {
if (!isdigit(*t_name))
break;
}
--
2.52.0
^ permalink raw reply related
* [PATCH v3 17/18] rtla/utils: Fix resource leak in set_comm_sched_attr()
From: Wander Lairson Costa @ 2026-01-15 16:32 UTC (permalink / raw)
To: Steven Rostedt, Tomas Glozar, Wander Lairson Costa, Ivan Pravdin,
Crystal Wood, Costa Shulyupin, John Kacur, Haiyong Sun,
Tiezhu Yang, Daniel Wagner, Daniel Bristot de Oliveira,
open list:Real-time Linux Analysis (RTLA) tools,
open list:Real-time Linux Analysis (RTLA) tools,
open list:BPF [MISC]:Keyword:(?:b|_)bpf(?:b|_)
In-Reply-To: <20260115163650.118910-1-wander@redhat.com>
The set_comm_sched_attr() function opens the /proc directory via
opendir() but fails to call closedir() on its successful exit path.
If the function iterates through all processes without error, it
returns 0 directly, leaking the DIR stream pointer.
Fix this by refactoring the function to use a single exit path. A
retval variable is introduced to track the success or failure status.
All exit points now jump to a unified out label that calls closedir()
before the function returns, ensuring the resource is always freed.
Fixes: dada03db9bb19 ("rtla: Remove procps-ng dependency")
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/utils.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
index b029fe5970c31..1ea9980d8ecd3 100644
--- a/tools/tracing/rtla/src/utils.c
+++ b/tools/tracing/rtla/src/utils.c
@@ -362,22 +362,23 @@ int set_comm_sched_attr(const char *comm_prefix, struct sched_attr *attr)
if (strtoi(proc_entry->d_name, &pid)) {
err_msg("'%s' is not a valid pid", proc_entry->d_name);
- goto out_err;
+ retval = 1;
+ goto out;
}
/* procfs_is_workload_pid confirmed it is a pid */
retval = __set_sched_attr(pid, attr);
if (retval) {
err_msg("Error setting sched attributes for pid:%s\n", proc_entry->d_name);
- goto out_err;
+ goto out;
}
debug_msg("Set sched attributes for pid:%s\n", proc_entry->d_name);
}
- return 0;
-out_err:
+ retval = 0;
+out:
closedir(procfs);
- return 1;
+ return retval;
}
#define INVALID_VAL (~0L)
--
2.52.0
^ permalink raw reply related
* [PATCH v3 16/18] rtla/trace: Fix I/O handling in save_trace_to_file()
From: Wander Lairson Costa @ 2026-01-15 16:31 UTC (permalink / raw)
To: Steven Rostedt, Tomas Glozar, Wander Lairson Costa, Ivan Pravdin,
Crystal Wood, Costa Shulyupin, John Kacur, Haiyong Sun,
Tiezhu Yang, Daniel Wagner, Daniel Bristot de Oliveira,
open list:Real-time Linux Analysis (RTLA) tools,
open list:Real-time Linux Analysis (RTLA) tools,
open list:BPF [MISC]:Keyword:(?:b|_)bpf(?:b|_)
In-Reply-To: <20260115163650.118910-1-wander@redhat.com>
The read/write loop in save_trace_to_file() does not correctly handle
errors from the read() and write() system calls. If either call is
interrupted by a signal, it returns -1 with errno set to EINTR, but
the code treats this as a fatal error and aborts the save operation.
Additionally, write() may perform a partial write, returning fewer
bytes than requested, which the code does not handle.
Fix the I/O loop by introducing proper error handling. The return
value of read() is now stored in a ssize_t variable and checked for
errors, with EINTR causing a retry. For write(), an inner loop ensures
all bytes are written, handling both EINTR and partial writes. Error
messages now include strerror() output for better debugging.
This follows the same pattern established in the previous commit that
fixed trace_event_save_hist(), ensuring consistent and robust I/O
handling throughout the trace saving code.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/trace.c | 30 +++++++++++++++++++++++-------
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/tools/tracing/rtla/src/trace.c b/tools/tracing/rtla/src/trace.c
index fed3362527b08..8e93b48d33ef8 100644
--- a/tools/tracing/rtla/src/trace.c
+++ b/tools/tracing/rtla/src/trace.c
@@ -73,6 +73,7 @@ int save_trace_to_file(struct tracefs_instance *inst, const char *filename)
char buffer[4096];
int out_fd, in_fd;
int retval = -1;
+ ssize_t n_read;
if (!inst || !filename)
return 0;
@@ -90,15 +91,30 @@ int save_trace_to_file(struct tracefs_instance *inst, const char *filename)
goto out_close_in;
}
- do {
- retval = read(in_fd, buffer, sizeof(buffer));
- if (retval <= 0)
+ for (;;) {
+ n_read = read(in_fd, buffer, sizeof(buffer));
+ if (n_read < 0) {
+ if (errno == EINTR)
+ continue;
+ err_msg("Error reading trace file: %s\n", strerror(errno));
goto out_close;
+ }
+ if (n_read == 0)
+ break;
- retval = write(out_fd, buffer, retval);
- if (retval < 0)
- goto out_close;
- } while (retval > 0);
+ ssize_t n_written = 0;
+ while (n_written < n_read) {
+ ssize_t w = write(out_fd, buffer + n_written, n_read - n_written);
+
+ if (w < 0) {
+ if (errno == EINTR)
+ continue;
+ err_msg("Error writing trace file: %s\n", strerror(errno));
+ goto out_close;
+ }
+ n_written += w;
+ }
+ }
retval = 0;
out_close:
--
2.52.0
^ permalink raw reply related
* [PATCH v3 15/18] rtla/trace: Fix write loop in trace_event_save_hist()
From: Wander Lairson Costa @ 2026-01-15 16:31 UTC (permalink / raw)
To: Steven Rostedt, Tomas Glozar, Wander Lairson Costa, Crystal Wood,
Ivan Pravdin, Costa Shulyupin, John Kacur, Haiyong Sun,
Tiezhu Yang, Daniel Wagner, Daniel Bristot de Oliveira,
open list:Real-time Linux Analysis (RTLA) tools,
open list:Real-time Linux Analysis (RTLA) tools,
open list:BPF [MISC]:Keyword:(?:b|_)bpf(?:b|_)
In-Reply-To: <20260115163650.118910-1-wander@redhat.com>
The write loop in trace_event_save_hist() does not correctly handle
errors from the write() system call. If write() returns -1, this value
is added to the loop index, leading to an incorrect memory access on
the next iteration and potentially an infinite loop. The loop also
fails to handle EINTR.
Fix the write loop by introducing proper error handling. The return
value of write() is now stored in a ssize_t variable and checked for
errors. The loop retries the call if interrupted by a signal and breaks
on any other error after logging it with strerror().
Additionally, change the index variable type from int to size_t to
match the type used for buffer sizes and by strlen(), improving type
safety.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/trace.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/tools/tracing/rtla/src/trace.c b/tools/tracing/rtla/src/trace.c
index ed7db5f4115ce..fed3362527b08 100644
--- a/tools/tracing/rtla/src/trace.c
+++ b/tools/tracing/rtla/src/trace.c
@@ -342,11 +342,11 @@ static void trace_event_disable_filter(struct trace_instance *instance,
static void trace_event_save_hist(struct trace_instance *instance,
struct trace_events *tevent)
{
- int index, out_fd;
+ size_t index, hist_len;
mode_t mode = 0644;
char path[MAX_PATH];
char *hist;
- size_t hist_len;
+ int out_fd;
if (!tevent)
return;
@@ -378,7 +378,15 @@ static void trace_event_save_hist(struct trace_instance *instance,
index = 0;
hist_len = strlen(hist);
do {
- index += write(out_fd, &hist[index], hist_len - index);
+ const ssize_t written = write(out_fd, &hist[index], hist_len - index);
+
+ if (written < 0) {
+ if (errno == EINTR)
+ continue;
+ err_msg(" Error writing hist file: %s\n", strerror(errno));
+ break;
+ }
+ index += written;
} while (index < hist_len);
free(hist);
--
2.52.0
^ permalink raw reply related
* [PATCH v3 14/18] rtla/timerlat: Simplify RTLA_NO_BPF environment variable check
From: Wander Lairson Costa @ 2026-01-15 16:31 UTC (permalink / raw)
To: Steven Rostedt, Tomas Glozar, Wander Lairson Costa, Crystal Wood,
Ivan Pravdin, Costa Shulyupin, John Kacur, Tiezhu Yang,
Daniel Wagner, Daniel Bristot de Oliveira,
open list:Real-time Linux Analysis (RTLA) tools,
open list:Real-time Linux Analysis (RTLA) tools,
open list:BPF [MISC]:Keyword:(?:b|_)bpf(?:b|_)
In-Reply-To: <20260115163650.118910-1-wander@redhat.com>
The code that checks the RTLA_NO_BPF environment variable calls
getenv() twice and uses strncmp() with a length of 2 to compare
against the single-character string "1". This is inefficient and
the comparison length is unnecessarily long.
Store the result of getenv() in a local variable to avoid the
redundant call, and replace strncmp() with strcmp() for the exact
match comparison. This follows the same pattern established in
recent commits that improved string comparison consistency
throughout the rtla codebase.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/timerlat.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/tracing/rtla/src/timerlat.c b/tools/tracing/rtla/src/timerlat.c
index 84577daadd668..d7aeaad975386 100644
--- a/tools/tracing/rtla/src/timerlat.c
+++ b/tools/tracing/rtla/src/timerlat.c
@@ -28,12 +28,13 @@ int
timerlat_apply_config(struct osnoise_tool *tool, struct timerlat_params *params)
{
int retval;
+ const char *const rtla_no_bpf = getenv("RTLA_NO_BPF");
/*
* Try to enable BPF, unless disabled explicitly.
* If BPF enablement fails, fall back to tracefs mode.
*/
- if (getenv("RTLA_NO_BPF") && strncmp(getenv("RTLA_NO_BPF"), "1", 2) == 0) {
+ if (rtla_no_bpf && strcmp(rtla_no_bpf, "1") == 0) {
debug_msg("RTLA_NO_BPF set, disabling BPF\n");
params->mode = TRACING_MODE_TRACEFS;
} else if (!tep_find_event_by_name(tool->trace.tep, "osnoise", "timerlat_sample")) {
--
2.52.0
^ permalink raw reply related
* [PATCH v3 13/18] rtla: Use str_has_prefix() for option prefix check
From: Wander Lairson Costa @ 2026-01-15 16:31 UTC (permalink / raw)
To: Steven Rostedt, Tomas Glozar, Wander Lairson Costa, Ivan Pravdin,
Crystal Wood, Costa Shulyupin, John Kacur, Tiezhu Yang,
Daniel Wagner, Daniel Bristot de Oliveira,
open list:Real-time Linux Analysis (RTLA) tools,
open list:Real-time Linux Analysis (RTLA) tools,
open list:BPF [MISC]:Keyword:(?:b|_)bpf(?:b|_)
In-Reply-To: <20260115163650.118910-1-wander@redhat.com>
The argument parsing code in timerlat_main() and osnoise_main() uses
strncmp() with a length of 1 to check if the first argument starts
with a dash, indicating an option flag was passed.
Replace this pattern with str_has_prefix() for consistency with the
rest of the codebase. While character comparison would be slightly
more efficient, using str_has_prefix() provides better readability
and maintains a uniform coding style throughout the rtla tool.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/osnoise.c | 2 +-
tools/tracing/rtla/src/timerlat.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/tracing/rtla/src/osnoise.c b/tools/tracing/rtla/src/osnoise.c
index 4890a9a9d6466..5b342d945c5f6 100644
--- a/tools/tracing/rtla/src/osnoise.c
+++ b/tools/tracing/rtla/src/osnoise.c
@@ -1210,7 +1210,7 @@ int osnoise_main(int argc, char *argv[])
if ((strcmp(argv[1], "-h") == 0) || (strcmp(argv[1], "--help") == 0)) {
osnoise_usage(0);
- } else if (strncmp(argv[1], "-", 1) == 0) {
+ } else if (str_has_prefix(argv[1], "-")) {
/* the user skipped the tool, call the default one */
run_tool(&osnoise_top_ops, argc, argv);
exit(0);
diff --git a/tools/tracing/rtla/src/timerlat.c b/tools/tracing/rtla/src/timerlat.c
index 8f8811f7a13bd..84577daadd668 100644
--- a/tools/tracing/rtla/src/timerlat.c
+++ b/tools/tracing/rtla/src/timerlat.c
@@ -272,7 +272,7 @@ int timerlat_main(int argc, char *argv[])
if ((strcmp(argv[1], "-h") == 0) || (strcmp(argv[1], "--help") == 0)) {
timerlat_usage(0);
- } else if (strncmp(argv[1], "-", 1) == 0) {
+ } else if (str_has_prefix(argv[1], "-")) {
/* the user skipped the tool, call the default one */
run_tool(&timerlat_top_ops, argc, argv);
exit(0);
--
2.52.0
^ permalink raw reply related
* [PATCH v3 12/18] rtla: Enforce exact match for time unit suffixes
From: Wander Lairson Costa @ 2026-01-15 16:31 UTC (permalink / raw)
To: Steven Rostedt, Tomas Glozar, Wander Lairson Costa, Crystal Wood,
Ivan Pravdin, Costa Shulyupin, John Kacur, Tiezhu Yang,
Daniel Wagner, Daniel Bristot de Oliveira,
open list:Real-time Linux Analysis (RTLA) tools,
open list:Real-time Linux Analysis (RTLA) tools,
open list:BPF [MISC]:Keyword:(?:b|_)bpf(?:b|_)
In-Reply-To: <20260115163650.118910-1-wander@redhat.com>
The parse_ns_duration() function currently uses prefix matching for
detecting time units. This approach is problematic as it silently
accepts malformed strings such as "100nsx" or "100us_invalid" by
ignoring the trailing characters, leading to potential configuration
errors.
Switch to using strcmp() for suffix comparison to enforce exact matches.
This ensures that the parser strictly validates the time unit and
rejects any input containing invalid trailing characters, thereby
improving the robustness of the configuration parsing.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/utils.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
index 486d96e8290fb..b029fe5970c31 100644
--- a/tools/tracing/rtla/src/utils.c
+++ b/tools/tracing/rtla/src/utils.c
@@ -211,15 +211,15 @@ long parse_ns_duration(char *val)
t = strtol(val, &end, 10);
if (end) {
- if (!strncmp(end, "ns", 2)) {
+ if (strcmp(end, "ns") == 0) {
return t;
- } else if (!strncmp(end, "us", 2)) {
+ } else if (strcmp(end, "us") == 0) {
t *= 1000;
return t;
- } else if (!strncmp(end, "ms", 2)) {
+ } else if (strcmp(end, "ms") == 0) {
t *= 1000 * 1000;
return t;
- } else if (!strncmp(end, "s", 1)) {
+ } else if (strcmp(end, "s") == 0) {
t *= 1000 * 1000 * 1000;
return t;
}
--
2.52.0
^ permalink raw reply related
* [PATCH v3 11/18] rtla: Use str_has_prefix() for prefix checks
From: Wander Lairson Costa @ 2026-01-15 16:31 UTC (permalink / raw)
To: Steven Rostedt, Tomas Glozar, Wander Lairson Costa, Crystal Wood,
Ivan Pravdin, Costa Shulyupin, John Kacur, Tiezhu Yang,
Daniel Wagner, Daniel Bristot de Oliveira,
open list:Real-time Linux Analysis (RTLA) tools,
open list:Real-time Linux Analysis (RTLA) tools,
open list:BPF [MISC]:Keyword:(?:b|_)bpf(?:b|_)
In-Reply-To: <20260115163650.118910-1-wander@redhat.com>
The code currently uses strncmp() combined with strlen() to check if a
string starts with a specific prefix. This pattern is verbose and prone
to errors if the length does not match the prefix string.
Replace this pattern with the str_has_prefix() helper function in both
trace.c and utils.c. This improves code readability and safety by
handling the prefix length calculation automatically.
In addition, remove the unused retval variable from
trace_event_save_hist() in trace.c to clean up the function and
silence potential compiler warnings.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/trace.c | 5 ++---
tools/tracing/rtla/src/utils.c | 3 +--
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/tools/tracing/rtla/src/trace.c b/tools/tracing/rtla/src/trace.c
index 2f529aaf8deef..ed7db5f4115ce 100644
--- a/tools/tracing/rtla/src/trace.c
+++ b/tools/tracing/rtla/src/trace.c
@@ -342,7 +342,7 @@ static void trace_event_disable_filter(struct trace_instance *instance,
static void trace_event_save_hist(struct trace_instance *instance,
struct trace_events *tevent)
{
- int retval, index, out_fd;
+ int index, out_fd;
mode_t mode = 0644;
char path[MAX_PATH];
char *hist;
@@ -356,8 +356,7 @@ static void trace_event_save_hist(struct trace_instance *instance,
return;
/* is this a hist: trigger? */
- retval = strncmp(tevent->trigger, "hist:", strlen("hist:"));
- if (retval)
+ if (!str_has_prefix(tevent->trigger, "hist:"))
return;
snprintf(path, ARRAY_SIZE(path), "%s_%s_hist.txt", tevent->system, tevent->event);
diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
index e98288e55db15..486d96e8290fb 100644
--- a/tools/tracing/rtla/src/utils.c
+++ b/tools/tracing/rtla/src/utils.c
@@ -318,8 +318,7 @@ static int procfs_is_workload_pid(const char *comm_prefix, struct dirent *proc_e
return 0;
buffer[MAX_PATH-1] = '\0';
- retval = strncmp(comm_prefix, buffer, strlen(comm_prefix));
- if (retval)
+ if (!str_has_prefix(buffer, comm_prefix))
return 0;
/* comm already have \n */
--
2.52.0
^ permalink raw reply related
* [PATCH v3 10/18] rtla: Add str_has_prefix() helper function
From: Wander Lairson Costa @ 2026-01-15 16:31 UTC (permalink / raw)
To: Steven Rostedt, Tomas Glozar, Wander Lairson Costa, Crystal Wood,
Ivan Pravdin, Costa Shulyupin, John Kacur, Tiezhu Yang,
Haiyong Sun, Daniel Wagner, Daniel Bristot de Oliveira,
open list:Real-time Linux Analysis (RTLA) tools,
open list:Real-time Linux Analysis (RTLA) tools,
open list:BPF [MISC]:Keyword:(?:b|_)bpf(?:b|_)
In-Reply-To: <20260115163650.118910-1-wander@redhat.com>
Add a str_has_prefix() helper function that tests whether a string
starts with a given prefix. This function provides a cleaner interface
for prefix matching compared to using strncmp() with strlen() directly.
The function returns a boolean value indicating whether the string
starts with the specified prefix. This helper will be used in
subsequent changes to simplify prefix matching code throughout rtla.
Also add the missing string.h include which is needed for the strlen()
and strncmp() functions used by str_has_prefix() and the existing
strncmp_static() macro.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/utils.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
index 25b08fc5e199a..1235d0f3a7bfd 100644
--- a/tools/tracing/rtla/src/utils.h
+++ b/tools/tracing/rtla/src/utils.h
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <stdint.h>
+#include <string.h>
#include <time.h>
#include <sched.h>
#include <stdbool.h>
@@ -24,6 +25,18 @@
/* Compare string with static string, length determined at compile time */
#define strncmp_static(s1, s2) strncmp(s1, s2, ARRAY_SIZE(s2))
+/**
+ * str_has_prefix - Test if a string has a given prefix
+ * @str: The string to test
+ * @prefix: The string to see if @str starts with
+ *
+ * Returns: true if @str starts with @prefix, false otherwise
+ */
+static inline bool str_has_prefix(const char *str, const char *prefix)
+{
+ return strncmp(str, prefix, strlen(prefix)) == 0;
+}
+
#define container_of(ptr, type, member)({ \
const typeof(((type *)0)->member) * __mptr = (ptr); \
(type *)((char *)__mptr - offsetof(type, member)) ; })
--
2.52.0
^ permalink raw reply related
* [PATCH v3 09/18] rtla: Handle pthread_create() failure properly
From: Wander Lairson Costa @ 2026-01-15 16:31 UTC (permalink / raw)
To: Steven Rostedt, Tomas Glozar, Wander Lairson Costa, Ivan Pravdin,
Crystal Wood, Costa Shulyupin, John Kacur, Tiezhu Yang,
Daniel Wagner, Daniel Bristot de Oliveira,
open list:Real-time Linux Analysis (RTLA) tools,
open list:Real-time Linux Analysis (RTLA) tools,
open list:BPF [MISC]:Keyword:(?:b|_)bpf(?:b|_)
In-Reply-To: <20260115163650.118910-1-wander@redhat.com>
Add proper error handling when pthread_create() fails to create the
timerlat user-space dispatcher thread. Previously, the code only logged
an error message but continued execution, which could lead to undefined
behavior when the tool later expects the thread to be running.
When pthread_create() returns an error, the function now jumps to the
out_trace error path to properly clean up resources and exit. This
ensures consistent error handling and prevents the tool from running
in an invalid state without the required user-space thread.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/common.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/tracing/rtla/src/common.c b/tools/tracing/rtla/src/common.c
index cbc207fa58707..73906065e7772 100644
--- a/tools/tracing/rtla/src/common.c
+++ b/tools/tracing/rtla/src/common.c
@@ -303,8 +303,10 @@ int run_tool(struct tool_ops *ops, int argc, char *argv[])
params->user.cgroup_name = params->cgroup_name;
retval = pthread_create(&user_thread, NULL, timerlat_u_dispatcher, ¶ms->user);
- if (retval)
+ if (retval) {
err_msg("Error creating timerlat user-space threads\n");
+ goto out_trace;
+ }
}
retval = ops->enable(tool);
--
2.52.0
^ permalink raw reply related
* [PATCH v3 08/18] rtla/timerlat: Add bounds check for softirq vector
From: Wander Lairson Costa @ 2026-01-15 16:31 UTC (permalink / raw)
To: Steven Rostedt, Tomas Glozar, Wander Lairson Costa, Ivan Pravdin,
Crystal Wood, Costa Shulyupin, John Kacur, Tiezhu Yang,
Haiyong Sun, Daniel Wagner, Daniel Bristot de Oliveira,
open list:Real-time Linux Analysis (RTLA) tools,
open list:Real-time Linux Analysis (RTLA) tools,
open list:BPF [MISC]:Keyword:(?:b|_)bpf(?:b|_)
In-Reply-To: <20260115163650.118910-1-wander@redhat.com>
Add bounds checking when accessing the softirq_name array using the
vector value from kernel trace data. The vector field from the
osnoise:softirq_noise event is used directly as an array index without
validation, which could cause an out-of-bounds read if the kernel
provides an unexpected vector value.
The softirq_name array contains 10 elements corresponding to the
standard Linux softirq vectors. While the kernel should only provide
valid vector values in the range 0-9, defensive programming requires
validating untrusted input before using it as an array index. If an
out-of-range vector is encountered, display the word UNKNOWN instead
of attempting to read beyond the array bounds.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/timerlat_aa.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/tracing/rtla/src/timerlat_aa.c b/tools/tracing/rtla/src/timerlat_aa.c
index 30ef56d644f9c..bc421637cc19b 100644
--- a/tools/tracing/rtla/src/timerlat_aa.c
+++ b/tools/tracing/rtla/src/timerlat_aa.c
@@ -417,8 +417,8 @@ static int timerlat_aa_softirq_handler(struct trace_seq *s, struct tep_record *r
taa_data->thread_softirq_sum += duration;
trace_seq_printf(taa_data->softirqs_seq, " %24s:%-3llu %.*s %9.2f us\n",
- softirq_name[vector], vector,
- 24, spaces,
+ vector < ARRAY_SIZE(softirq_name) ? softirq_name[vector] : "UNKNOWN",
+ vector, 24, spaces,
ns_to_usf(duration));
return 0;
}
--
2.52.0
^ permalink raw reply related
* [PATCH v3 07/18] rtla: Add strscpy() and replace strncpy() calls
From: Wander Lairson Costa @ 2026-01-15 16:31 UTC (permalink / raw)
To: Steven Rostedt, Tomas Glozar, Wander Lairson Costa, Crystal Wood,
Ivan Pravdin, Costa Shulyupin, John Kacur, Tiezhu Yang,
Haiyong Sun, Daniel Wagner, Daniel Bristot de Oliveira,
open list:Real-time Linux Analysis (RTLA) tools,
open list:Real-time Linux Analysis (RTLA) tools,
open list:BPF [MISC]:Keyword:(?:b|_)bpf(?:b|_)
In-Reply-To: <20260115163650.118910-1-wander@redhat.com>
Introduce a userspace strscpy() implementation that matches the Linux
kernel's strscpy() semantics. The function is built on top of glibc's
strlcpy() and provides guaranteed NUL-termination along with proper
truncation detection through its return value.
The previous strncpy() calls had potential issues: strncpy() does not
guarantee NUL-termination when the source string length equals or
exceeds the destination buffer size. This required defensive patterns
like pre-zeroing buffers or manually setting the last byte to NUL.
The new strscpy() function always NUL-terminates the destination buffer
unless the size is zero, and returns -E2BIG on truncation, making error
handling cleaner and more consistent with kernel code.
Note that unlike the kernel's strscpy(), this implementation uses
strlcpy() internally, which reads the entire source string to determine
its length. The kernel avoids this to prevent potential DoS attacks from
extremely long untrusted strings. This is harmless for a userspace CLI
tool like rtla where input sources are bounded and trusted.
Replace all strncpy() calls in rtla with strscpy(), using sizeof() for
buffer sizes instead of magic constants to ensure the sizes stay in
sync with the actual buffer declarations. Also remove a now-redundant
memset() call that was previously needed to work around strncpy()
behavior.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/timerlat_aa.c | 6 ++---
tools/tracing/rtla/src/utils.c | 34 ++++++++++++++++++++++++++--
tools/tracing/rtla/src/utils.h | 1 +
3 files changed, 36 insertions(+), 5 deletions(-)
diff --git a/tools/tracing/rtla/src/timerlat_aa.c b/tools/tracing/rtla/src/timerlat_aa.c
index 31e66ea2b144c..30ef56d644f9c 100644
--- a/tools/tracing/rtla/src/timerlat_aa.c
+++ b/tools/tracing/rtla/src/timerlat_aa.c
@@ -455,9 +455,9 @@ static int timerlat_aa_thread_handler(struct trace_seq *s, struct tep_record *re
taa_data->thread_blocking_duration = duration;
if (comm)
- strncpy(taa_data->run_thread_comm, comm, MAX_COMM);
+ strscpy(taa_data->run_thread_comm, comm, sizeof(taa_data->run_thread_comm));
else
- sprintf(taa_data->run_thread_comm, "<...>");
+ strscpy(taa_data->run_thread_comm, "<...>", sizeof(taa_data->run_thread_comm));
} else {
taa_data->thread_thread_sum += duration;
@@ -519,7 +519,7 @@ static int timerlat_aa_sched_switch_handler(struct trace_seq *s, struct tep_reco
tep_get_field_val(s, event, "next_pid", record, &taa_data->current_pid, 1);
comm = tep_get_field_raw(s, event, "next_comm", record, &val, 1);
- strncpy(taa_data->current_comm, comm, MAX_COMM);
+ strscpy(taa_data->current_comm, comm, sizeof(taa_data->current_comm));
/*
* If this was a kworker, clean the last kworkers that ran.
diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
index b5a6007b108d2..e98288e55db15 100644
--- a/tools/tracing/rtla/src/utils.c
+++ b/tools/tracing/rtla/src/utils.c
@@ -722,8 +722,7 @@ static const int find_mount(const char *fs, char *mp, int sizeof_mp)
if (!found)
return 0;
- memset(mp, 0, sizeof_mp);
- strncpy(mp, mount_point, sizeof_mp - 1);
+ strscpy(mp, mount_point, sizeof_mp);
debug_msg("Fs %s found at %s\n", fs, mp);
return 1;
@@ -1036,6 +1035,37 @@ int strtoi(const char *s, int *res)
return 0;
}
+/**
+ * strscpy - Copy a C-string into a sized buffer
+ * @dst: Where to copy the string to
+ * @src: Where to copy the string from
+ * @count: Size of destination buffer
+ *
+ * Copy the source string @src, or as much of it as fits, into the destination
+ * @dst buffer. The destination @dst buffer is always NUL-terminated, unless
+ * it's zero-sized.
+ *
+ * This is a userspace implementation matching the kernel's strscpy() semantics,
+ * built on top of glibc's strlcpy().
+ *
+ * Returns the number of characters copied (not including the trailing NUL)
+ * or -E2BIG if @count is 0 or the copy was truncated.
+ */
+ssize_t strscpy(char *dst, const char *src, size_t count)
+{
+ size_t len;
+
+ if (count == 0)
+ return -E2BIG;
+
+ len = strlcpy(dst, src, count);
+
+ if (len >= count)
+ return -E2BIG;
+
+ return (ssize_t) len;
+}
+
static inline void fatal_alloc(void)
{
fatal("Error allocating memory\n");
diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
index 8323c999260c2..25b08fc5e199a 100644
--- a/tools/tracing/rtla/src/utils.h
+++ b/tools/tracing/rtla/src/utils.h
@@ -97,6 +97,7 @@ static inline int have_libcpupower_support(void) { return 0; }
#endif /* HAVE_LIBCPUPOWER_SUPPORT */
int auto_house_keeping(cpu_set_t *monitored_cpus);
__attribute__((__warn_unused_result__)) int strtoi(const char *s, int *res);
+ssize_t strscpy(char *dst, const char *src, size_t count);
#define ns_to_usf(x) (((double)x/1000))
#define ns_to_per(total, part) ((part * 100) / (double)total)
--
2.52.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox