BPF List
 help / color / mirror / Atom feed
From: Tao Chen <chen.dylane@linux.dev>
To: Jiri Olsa <olsajiri@gmail.com>
Cc: ast@kernel.org, daniel@iogearbox.net, john.fastabend@gmail.com,
	andrii@kernel.org, martin.lau@linux.dev, eddyz87@gmail.com,
	song@kernel.org, yonghong.song@linux.dev, kpsingh@kernel.org,
	sdf@fomichev.me, haoluo@google.com, bpf@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH bpf-next 2/2] selftests/bpf: Add stacktrace map lookup_and_delete_elem test case
Date: Tue, 9 Sep 2025 21:07:48 +0800	[thread overview]
Message-ID: <eae6de3a-284c-40f4-bdfa-ca4b98bf55e1@linux.dev> (raw)
In-Reply-To: <aMAfAf1JEAcbYOuq@krava>

在 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@linux.dev>
>> ---
>>   .../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

      reply	other threads:[~2025-09-09 13:08 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-08 11:36 [PATCH bpf-next 1/2] bpf: Add lookup_and_delete_elem for BPF_MAP_STACK_TRACE Tao Chen
2025-09-08 11:36 ` [PATCH bpf-next 2/2] selftests/bpf: Add stacktrace map lookup_and_delete_elem test case Tao Chen
2025-09-09 12:35   ` Jiri Olsa
2025-09-09 13:07     ` Tao Chen [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=eae6de3a-284c-40f4-bdfa-ca4b98bf55e1@linux.dev \
    --to=chen.dylane@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=olsajiri@gmail.com \
    --cc=sdf@fomichev.me \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox