[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4BzYE7vYob3ZC8Zyjnfxh0oN3hv1TWFZkjXvi5hTeuqfiwQ@mail.gmail.com>
Date: Thu, 25 Sep 2025 16:18:29 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Tao Chen <chen.dylane@...ux.dev>
Cc: ast@...nel.org, daniel@...earbox.net, john.fastabend@...il.com,
andrii@...nel.org, martin.lau@...ux.dev, eddyz87@...il.com, song@...nel.org,
yonghong.song@...ux.dev, kpsingh@...nel.org, sdf@...ichev.me,
haoluo@...gle.com, jolsa@...nel.org, bpf@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next v6 2/3] selftests/bpf: Refactor stacktrace_map
case with skeleton
On Thu, Sep 25, 2025 at 10:51 AM Tao Chen <chen.dylane@...ux.dev> wrote:
>
> The loading method of the stacktrace_map test case looks too outdated,
> refactor it with skeleton, and we can use global avariable feature in
typo: variable, fixed
> the next patch.
>
> Signed-off-by: Tao Chen <chen.dylane@...ux.dev>
> ---
> .../selftests/bpf/prog_tests/stacktrace_map.c | 52 ++++++++-----------
> .../bpf/prog_tests/stacktrace_map_raw_tp.c | 2 +-
> ...test_stacktrace_map.c => stacktrace_map.c} | 0
> 3 files changed, 22 insertions(+), 32 deletions(-)
> rename tools/testing/selftests/bpf/progs/{test_stacktrace_map.c => stacktrace_map.c} (100%)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
> index 84a7e405e91..0a79bf1d354 100644
> --- a/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
> +++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
> @@ -1,46 +1,38 @@
> // SPDX-License-Identifier: GPL-2.0
> #include <test_progs.h>
> +#include "stacktrace_map.skel.h"
>
> void test_stacktrace_map(void)
> {
> + struct stacktrace_map *skel;
> int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
> - const char *prog_name = "oncpu";
> - int err, prog_fd, stack_trace_len;
> - const char *file = "./test_stacktrace_map.bpf.o";
> + int err, stack_trace_len;
> __u32 key, val, duration = 0;
> - struct bpf_program *prog;
> - struct bpf_object *obj;
> - struct bpf_link *link;
>
> - err = bpf_prog_test_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
> - if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
> + skel = stacktrace_map__open_and_load();
> + if (!ASSERT_OK_PTR(skel, "skel_open_and_load"))
> return;
>
> - prog = bpf_object__find_program_by_name(obj, prog_name);
> - if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
> - goto close_prog;
> -
> - link = bpf_program__attach_tracepoint(prog, "sched", "sched_switch");
> - if (!ASSERT_OK_PTR(link, "attach_tp"))
> - goto close_prog;
> -
> /* find map fds */
> - control_map_fd = bpf_find_map(__func__, obj, "control_map");
> + control_map_fd = bpf_map__fd(skel->maps.control_map);
> if (CHECK_FAIL(control_map_fd < 0))
> - goto disable_pmu;
> + goto out;
>
> - stackid_hmap_fd = bpf_find_map(__func__, obj, "stackid_hmap");
> + stackid_hmap_fd = bpf_map__fd(skel->maps.stackid_hmap);
> if (CHECK_FAIL(stackid_hmap_fd < 0))
> - goto disable_pmu;
> + goto out;
>
> - stackmap_fd = bpf_find_map(__func__, obj, "stackmap");
> + stackmap_fd = bpf_map__fd(skel->maps.stackmap);
> if (CHECK_FAIL(stackmap_fd < 0))
> - goto disable_pmu;
> + goto out;
>
> - stack_amap_fd = bpf_find_map(__func__, obj, "stack_amap");
> + stack_amap_fd = bpf_map__fd(skel->maps.stack_amap);
> if (CHECK_FAIL(stack_amap_fd < 0))
> - goto disable_pmu;
> + goto out;
none of the above CHECK_FAIL checks are necessary anymore, skeleton
loading makes sure all those maps are there. I dropped this section
altogether
>
> + err = stacktrace_map__attach(skel);
> + if (!ASSERT_OK(err, "skel_attach"))
> + goto out;
> /* give some time for bpf program run */
> sleep(1);
>
> @@ -55,21 +47,19 @@ void test_stacktrace_map(void)
> err = compare_map_keys(stackid_hmap_fd, stackmap_fd);
> if (CHECK(err, "compare_map_keys stackid_hmap vs. stackmap",
> "err %d errno %d\n", err, errno))
> - goto disable_pmu;
> + goto out;
>
> err = compare_map_keys(stackmap_fd, stackid_hmap_fd);
> if (CHECK(err, "compare_map_keys stackmap vs. stackid_hmap",
> "err %d errno %d\n", err, errno))
> - goto disable_pmu;
> + goto out;
>
> stack_trace_len = PERF_MAX_STACK_DEPTH * sizeof(__u64);
> err = compare_stack_ips(stackmap_fd, stack_amap_fd, stack_trace_len);
> if (CHECK(err, "compare_stack_ips stackmap vs. stack_amap",
> "err %d errno %d\n", err, errno))
> - goto disable_pmu;
> + goto out;
>
> -disable_pmu:
> - bpf_link__destroy(link);
> -close_prog:
> - bpf_object__close(obj);
> +out:
> + stacktrace_map__destroy(skel);
> }
> diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
> index e0cb4697b4b..e985d51d3d4 100644
> --- a/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
> +++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
> @@ -5,7 +5,7 @@ void test_stacktrace_map_raw_tp(void)
> {
> const char *prog_name = "oncpu";
> int control_map_fd, stackid_hmap_fd, stackmap_fd;
> - const char *file = "./test_stacktrace_map.bpf.o";
> + const char *file = "./stacktrace_map.bpf.o";
> __u32 key, val, duration = 0;
> int err, prog_fd;
> struct bpf_program *prog;
> diff --git a/tools/testing/selftests/bpf/progs/test_stacktrace_map.c b/tools/testing/selftests/bpf/progs/stacktrace_map.c
> similarity index 100%
> rename from tools/testing/selftests/bpf/progs/test_stacktrace_map.c
> rename to tools/testing/selftests/bpf/progs/stacktrace_map.c
> --
> 2.48.1
>
Powered by blists - more mailing lists