Re: [PATCH bpf-next] selftests/bpf: Fix map_kptr test failure

[Yonghong Song <yonghong.song@linux.dev>]

On 1/14/26 11:54 PM, Kumar Kartikeya Dwivedi wrote:
> On Thu, 15 Jan 2026 at 07:16, Yonghong Song <yonghong.song@linux.dev> wrote:
>> On my arm64 machine, I get the following failure:
>>    ...
>>    tester_init:PASS:tester_log_buf 0 nsec
>>    process_subtest:PASS:obj_open_mem 0 nsec
>>    process_subtest:PASS:specs_alloc 0 nsec
>>    serial_test_map_kptr:PASS:rcu_tasks_trace_gp__open_and_load 0 nsec
>>    ...
>>    test_map_kptr_success:PASS:map_kptr__open_and_load 0 nsec
>>    test_map_kptr_success:PASS:test_map_kptr_ref1 refcount 0 nsec
>>    test_map_kptr_success:FAIL:test_map_kptr_ref1 retval unexpected error: 2 (errno 2)
>>    test_map_kptr_success:PASS:test_map_kptr_ref2 refcount 0 nsec
>>    test_map_kptr_success:FAIL:test_map_kptr_ref2 retval unexpected error: 1 (errno 2)
>>    ...
>>    #201/21  map_kptr/success-map:FAIL
>>
>> In serial_test_map_kptr(), before test_map_kptr_success(), one
>> kern_sync_rcu() is used to have some delay for freeing the map.
>> But in my environment, one kern_sync_rcu() seems not enough and
>> caused the test failure.
>>
>> In bpf_map_free_in_work() in syscall.c, the queue time for
>>    queue_work(system_dfl_wq, &map->work)
>> may be longer than expected. This may cause the test failure
>> since test_map_kptr_success() expects all previous maps having been freed.
>>
>> In stead of one kern_sync_rcu() before test_map_kptr_success(),
>> I added two more kern_sync_rcu() to have a longer delay and
>> the test succeeded.
>>
>> Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
> This is still not a proper fix, right? Maybe two works in this case,
> but it isn't guaranteed to be enough either.
> RCU gp wait won't have any synchronization with when wq items are executed.
> I forgot why I used kern_sync_rcu() originally, but I feel the right
> way to fix this would be to count when all maps have finished their
> bpf_map_free through an fexit hook. Thoughts?

Agree that this is still not to guarantee it won't break due to queue_work().
One possibility is to count the references in a separate bpf program and until
all references are gone then we can do subsequent test_map_kptr_success().
Let me give a try.

>
>>   tools/testing/selftests/bpf/prog_tests/map_kptr.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/map_kptr.c b/tools/testing/selftests/bpf/prog_tests/map_kptr.c
>> index 8743df599567..f9cfc4d3153c 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/map_kptr.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/map_kptr.c
>> @@ -148,11 +148,15 @@ void serial_test_map_kptr(void)
>>
>>                  ASSERT_OK(kern_sync_rcu_tasks_trace(skel), "sync rcu_tasks_trace");
>>                  ASSERT_OK(kern_sync_rcu(), "sync rcu");
>> +               ASSERT_OK(kern_sync_rcu(), "sync rcu");
>> +               ASSERT_OK(kern_sync_rcu(), "sync rcu");
>>                  /* Observe refcount dropping to 1 on bpf_map_free_deferred */
>>                  test_map_kptr_success(false);
>>
>>                  ASSERT_OK(kern_sync_rcu_tasks_trace(skel), "sync rcu_tasks_trace");
>>                  ASSERT_OK(kern_sync_rcu(), "sync rcu");
>> +               ASSERT_OK(kern_sync_rcu(), "sync rcu");
>> +               ASSERT_OK(kern_sync_rcu(), "sync rcu");
>>                  /* Observe refcount dropping to 1 on synchronous delete elem */
>>                  test_map_kptr_success(true);
>>          }
>> --
>> 2.47.3
>>
>>