Re: [PATCH bpf-next v7 12/12] selftests/bpf: Add resizable hashmap to benchmarks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The bpf-rhashmap-full-update benchmark measures the -E2BIG error path instead of successful map updates, invalidating the commit message's performance claims.
--

commit c016012bca8439800d9763b0e9dbdba68b4e1642
Author: Mykyta Yatsenko <yatsenko@meta.com>

selftests/bpf: Add resizable hashmap to benchmarks

This patch adds support for resizable hashmaps in BPF map benchmarks.
It claims a 5.25x speedup for rhtab over htab during full updates.

>     2. FULL UPDATE  (M events/sec per producer)
>     
>       htab  per-producer:
>         20.33   22.02   19.27   23.61   24.18   23.17   21.07
>         mean  21.94   range  19.27 - 24.18
>     
>       rhtab per-producer:
>        133.51  129.47   74.52  129.29  102.26  129.98  107.64
>         mean 115.24   range  74.52 - 133.51
>     
>       speedup (mean): 5.25x   (+425 %)
>     
>     In-place memcpy avoids the per-update alloc + RCU pointer swap
>     that htab pays.

[Severity: Medium]
Are these benchmark results measuring successful updates, or just the speed of
the -E2BIG error path?

In tools/testing/selftests/bpf/benchs/bench_bpf_hashmap_full_update.c:hashmap_full_update_setup(),
the userspace setup pre-fills the map to its maximum capacity:

    /* fill hash_map */
    map_fd = bpf_map__fd(ctx.skel->maps.hash_map_bench);
    max_entries = bpf_map__max_entries(ctx.skel->maps.hash_map_bench);
    for (i = 0; i < max_entries; i++)
        bpf_map_update_elem(map_fd, &i, &i, BPF_ANY);

The BPF program then attempts to insert a new key that doesn't exist in the map
in tools/testing/selftests/bpf/progs/bpf_hashmap_full_update_bench.c:benchmark():

    u32 cpu = bpf_get_smp_processor_id();
    u32 key = cpu + MAX_ENTRIES;
    ...
    bpf_loop(nr_loops, loop_update_callback, &key, 0);

Because the map is already full, does every update attempt immediately fail with
-E2BIG without performing any updates or memcpy?

For htab (which preallocates), returning -E2BIG requires a slow fallback
traversal of all other CPUs' empty freelists. For rhtab (no prealloc), it is
a fast atomic check against the element count.

Does this regression mean the benchmark is comparing the speed of error paths
rather than successful map updates, which might invalidate the performance
claims?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605-rhash-v7-0-5b8e05f8630d@meta.com?part=12