Re: [PATCH bpf-next 2/2] selftests/bpf: Add stacktrace map lookup_and_delete_elem test case

[Tao Chen <chen.dylane@xxxxxxxxx>]

在 2025/9/9 20:35, Jiri Olsa 写道:
> On Mon, Sep 08, 2025 at 07:36:22PM +0800, Tao Chen wrote:
> > ...
> > test_stacktrace_map:PASS:compare_stack_ips stackmap vs. stack_amap 0 nsec
> > test_stacktrace_map:PASS:stack_key_map lookup 0 nsec
> > test_stacktrace_map:PASS:stackmap lookup and detele 0 nsec
> >   #397     stacktrace_map:OK
> > ...
> >
> > Signed-off-by: Tao Chen <chen.dylane@xxxxxxxxx>
> > ---
> >   .../selftests/bpf/prog_tests/stacktrace_map.c  | 18 +++++++++++++++++-
> >   .../selftests/bpf/progs/test_stacktrace_map.c  | 12 +++++++++++-
> >   2 files changed, 28 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
> > index 84a7e405e91..496c4dcf4ea 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
> > @@ -3,7 +3,7 @@
> >   
> >   void test_stacktrace_map(void)
> >   {
> > -	int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
> > +	int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd, stack_key_map_fd;
> >   	const char *prog_name = "oncpu";
> >   	int err, prog_fd, stack_trace_len;
> >   	const char *file = "./test_stacktrace_map.bpf.o";
> > @@ -11,6 +11,9 @@ void test_stacktrace_map(void)
> >   	struct bpf_program *prog;
> >   	struct bpf_object *obj;
> >   	struct bpf_link *link;
> > +	__u32 stackmap_key;
> > +	char val_buf[PERF_MAX_STACK_DEPTH *
> > +		sizeof(struct bpf_stack_build_id)];
> >   
> >   	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))
> > @@ -41,6 +44,10 @@ void test_stacktrace_map(void)
> >   	if (CHECK_FAIL(stack_amap_fd < 0))
> >   		goto disable_pmu;
> >   
> > +	stack_key_map_fd = bpf_find_map(__func__, obj, "stack_key_map");
> > +	if (CHECK_FAIL(stack_key_map_fd < 0))
> > +		goto disable_pmu;
> > +
> >   	/* give some time for bpf program run */
> >   	sleep(1);
> >   
> > @@ -68,6 +75,15 @@ void test_stacktrace_map(void)
> >   		  "err %d errno %d\n", err, errno))
> >   		goto disable_pmu;
> >   
> > +	err = bpf_map_lookup_elem(stack_key_map_fd, &key, &stackmap_key);
> > +	if (CHECK(err, "stack_key_map lookup", "err %d errno %d\n", err, errno))
> > +		goto disable_pmu;
> > +
> > +	err = bpf_map_lookup_and_delete_elem(stackmap_fd, &stackmap_key, &val_buf);
> > +	if (CHECK(err, "stackmap lookup and detele",
>
> nit typo 's/detele/delete/'

my fault, will fix it in v2.

> > +		  "err %d errno %d\n", err, errno))
> > +		goto disable_pmu;
>
> should we also check the record got deleted? like make sure following
> lookup fails with NOENT:

yes we can, will add it in v2. Thanks.

>    bpf_map_lookup_elem(stackmap_fd, &stackmap_key, &val_buf)
>
> > +
> >   disable_pmu:
> >   	bpf_link__destroy(link);
> >   close_prog:
> > diff --git a/tools/testing/selftests/bpf/progs/test_stacktrace_map.c b/tools/testing/selftests/bpf/progs/test_stacktrace_map.c
> > index 47568007b66..d036e8e9c83 100644
> > --- a/tools/testing/selftests/bpf/progs/test_stacktrace_map.c
> > +++ b/tools/testing/selftests/bpf/progs/test_stacktrace_map.c
> > @@ -38,6 +38,13 @@ struct {
> >   	__type(value, stack_trace_t);
> >   } stack_amap SEC(".maps");
> >   
> > +struct {
> > +	__uint(type, BPF_MAP_TYPE_ARRAY);
> > +	__uint(max_entries, 1);
> > +	__type(key, __u32);
> > +	__type(value, __u32);
> > +} stack_key_map SEC(".maps");
> > +
> >   /* taken from /sys/kernel/tracing/events/sched/sched_switch/format */
> >   struct sched_switch_args {
> >   	unsigned long long pad;
> > @@ -54,7 +61,7 @@ SEC("tracepoint/sched/sched_switch")
> >   int oncpu(struct sched_switch_args *ctx)
> >   {
> >   	__u32 max_len = PERF_MAX_STACK_DEPTH * sizeof(__u64);
> > -	__u32 key = 0, val = 0, *value_p;
> > +	__u32 key = 0, val = 0, *value_p, stackmap_key = 0;
> >   	void *stack_p;
> >   
> >   	value_p = bpf_map_lookup_elem(&control_map, &key);
> > @@ -64,6 +71,9 @@ int oncpu(struct sched_switch_args *ctx)
> >   	/* The size of stackmap and stackid_hmap should be the same */
> >   	key = bpf_get_stackid(ctx, &stackmap, 0);
> >   	if ((int)key >= 0) {
> > +		val = key;
> > +		bpf_map_update_elem(&stack_key_map, &stackmap_key, &val, 0);
>
> why not use '&key' directly as the update value?
will clean it in v2.

> jirka
>
>
> > +		val = 0;
> >   		bpf_map_update_elem(&stackid_hmap, &key, &val, 0);
> >   		stack_p = bpf_map_lookup_elem(&stack_amap, &key);
> >   		if (stack_p)
> > --
> > 2.48.1
--
Best Regards
Tao Chen