[PATCH bpf-next v2 0/1] selftests: bpf: fix test_lru_sanity3

[PATCH bpf-next v2 0/1] selftests: bpf: fix test_lru_sanity3

[Eva Kurchatova]

Changes in v2:
- Skip the test where __tgt_size(tgt_free) is 0 instead of forcing
  batch_size to 1. With one element per refill the forced shrink on the
  last insert evicts a live key, so v1 failed above 128 cpus, where the
  original test still passed by degenerating.
- Drop assert(__tgt_size(map_size) == batch_size), it holds for every
  batch_size and cpu count and checked nothing.
- Cut the comment above map_size to one line.
- Reword to better explain the cause of the failure.
- Sent separately from rest of v1 patchset.

Link to v1: https://lore.kernel.org/bpf/20260917015605.2464043-2-eva.kurchatova@virtuozzo.com/

Eva Kurchatova (1):
  selftests: bpf: size the map in test_lru_sanity3 to whole refills

 tools/testing/selftests/bpf/test_lru_map.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

-- 
2.55.0

[PATCH bpf-next v2 1/1] selftests: bpf: size the map in test_lru_sanity3 to whole refills

[Eva Kurchatova]

test_lru_sanity3 fills a map of tgt_free * 2 elements, and then reads
back all but the last few. Whether that passes depends on the cpu
count alone; on a machine with six it does not:

  test_lru_sanity3 (map_type:9 map_flags:0x0): test_lru_map.c:463:
  test_lru_sanity3: Assertion `!bpf_map_lookup_elem_with_ref_bit(
  lru_map_fd, key, value)' failed.

The kernel hands elements out in refills of lru->target_free, which is
clamp((size / nr_cpus) / 2, 1, LOCAL_FREE_TARGET), that is 128 / nr_cpus
rounded down. A refill the global free list cannot satisfy in full
calls __bpf_lru_list_shrink() for the remainder, and that evicts live
elements. Filling the map therefore evicts nothing only when computed
128 / nr_cpus is a power of two. Any other nr_cpus variation fails.

batch_size is already __tgt_size(tgt_free), the refill size of a map
is __map_size(batch_size) elements, so size the map that way and the
fill consumes whole refills. Start the keys of the last insert at
map_size + 1, just past the new size.

Above 128 cpus __tgt_size(tgt_free) is 0 and there is no size to build
on. Skip the test there.

Fixes: 5e9388f7984a ("selftests/bpf: adapt one more case in test_lru_map to the new target_free")
Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
---
 tools/testing/selftests/bpf/test_lru_map.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_lru_map.c b/tools/testing/selftests/bpf/test_lru_map.c
index 0921939532c6..84e9a930471a 100644
--- a/tools/testing/selftests/bpf/test_lru_map.c
+++ b/tools/testing/selftests/bpf/test_lru_map.c
@@ -441,8 +441,15 @@ static void test_lru_sanity3(int map_type, int map_flags, unsigned int tgt_free)
 	assert(sched_next_online(0, &next_cpu) != -1);
 
 	batch_size = __tgt_size(tgt_free);
+	if (!batch_size) {
+		/* More cpus than this tgt_free gives a refill for */
+		printf("Skip\n");
+		return;
+	}
+
+	/* A partial refill shrinks the LRU and evicts live elements */
+	map_size = __map_size(batch_size);
 
-	map_size = tgt_free * 2;
 	lru_map_fd = create_map(map_type, map_flags, map_size);
 	assert(lru_map_fd != -1);
 
@@ -466,7 +473,7 @@ static void test_lru_sanity3(int map_type, int map_flags, unsigned int tgt_free)
 	}
 
 	/* Insert new batch_size: replaces the non-referenced elements */
-	key = 2 * tgt_free + 1;
+	key = 1 + map_size;
 	end_key = key + batch_size;
 	for (; key < end_key; key++) {
 		assert(!bpf_map_update_elem(lru_map_fd, &key, value,
-- 
2.55.0

Re: [PATCH bpf-next v2 1/1] selftests: bpf: size the map in test_lru_sanity3 to whole refills

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 4094 bytes --]

> selftests: bpf: size the map in test_lru_sanity3 to whole refills
>
> test_lru_sanity3 fills a map of tgt_free * 2 elements, and then reads
> back all but the last few. Whether that passes depends on the cpu
> count alone; on a machine with six it does not:
>
>   test_lru_sanity3 (map_type:9 map_flags:0x0): test_lru_map.c:463:
>   test_lru_sanity3: Assertion `!bpf_map_lookup_elem_with_ref_bit(
>   lru_map_fd, key, value)' failed.
>
> The kernel hands elements out in refills of lru->target_free, which is
> clamp((size / nr_cpus) / 2, 1, LOCAL_FREE_TARGET), that is 128 / nr_cpus
> rounded down. A refill the global free list cannot satisfy in full
> calls __bpf_lru_list_shrink() for the remainder, and that evicts live
> elements. Filling the map therefore evicts nothing only when computed
> 128 / nr_cpus is a power of two. Any other nr_cpus variation fails.
>
> batch_size is already __tgt_size(tgt_free), the refill size of a map
> is __map_size(batch_size) elements, so size the map that way and the
> fill consumes whole refills. Start the keys of the last insert at
> map_size + 1, just past the new size.
>
> Above 128 cpus __tgt_size(tgt_free) is 0 and there is no size to build
> on. Skip the test there.

The changelog states the skip triggers "Above 128 cpus", but the code
skips from 65 CPUs up. main() passes tgt_free = LOCAL_FREE_TARGET (128),
and __tgt_size() computes:

    batch_size = (128 / nr_cpus) / 2 = floor(64 / nr_cpus)

which is 0 for any nr_cpus >= 65 (since 128 / 65 = 1, and 1 / 2 = 0).
The new `if (!batch_size)` check fires on 65..128 CPU machines, not only
"above 128 cpus".

The sentence "batch_size is already __tgt_size(tgt_free), the refill
size of a map is __map_size(batch_size) elements" is also hard to parse.
The refill size is batch_size (not tgt_free); the map is sized to make
the kernel's target_free equal batch_size.

Could the last paragraph say "Above 64 cpus", and could the sentence be
reworded to clarify that batch_size becomes the refill size after the
map is resized?

> diff --git a/tools/testing/selftests/bpf/test_lru_map.c b/tools/testing/selftests/bpf/test_lru_map.c
> index 0921939532c6..84e9a930471a 100644
> --- a/tools/testing/selftests/bpf/test_lru_map.c
> +++ b/tools/testing/selftests/bpf/test_lru_map.c
> @@ -441,8 +441,15 @@ static void test_lru_sanity3(int map_type, int map_flags, unsigned int tgt_free)
>  	assert(sched_next_online(0, &next_cpu) != -1);
>
>  	batch_size = __tgt_size(tgt_free);
> +	if (!batch_size) {
> +		/* More cpus than this tgt_free gives a refill for */
> +		printf("Skip\n");
> +		return;
> +	}

A subsystem pattern flags this as potentially concerning: the new skip
reports with printf("Skip\n") and then returns, so the binary exits 0
and CI counts it as a pass. The selftests guide asks for skip results to
go through kselftest.h (ksft_test_result_skip() with a reason string),
and says skip messages should always include a reason.

test_lru_map.c is a legacy binary that never includes kselftest.h, and
converting just this line would be inconsistent with the existing
printf("Pass\n") pattern used elsewhere in the file. The concern is
narrow: could the message at least say why the test was skipped, for
example printf("Skip: nr_cpus %d > %u\n", nr_cpus, tgt_free / 2)?

> +
> +	/* A partial refill shrinks the LRU and evicts live elements */
> +	map_size = __map_size(batch_size);
>
> -	map_size = tgt_free * 2;
>  	lru_map_fd = create_map(map_type, map_flags, map_size);

[ ... ]

> @@ -466,7 +473,7 @@ static void test_lru_sanity3(int map_type, int map_flags, unsigned int tgt_free)
>  	}
>
>  	/* Insert new batch_size: replaces the non-referenced elements */
> -	key = 2 * tgt_free + 1;
> +	key = 1 + map_size;
>  	end_key = key + batch_size;
>  	for (; key < end_key; key++) {
>  		assert(!bpf_map_update_elem(lru_map_fd, &key, value,


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/36157128898