[PATCH bpf-next] selftsets/bpf: Retry map update on helper_fill_hashmap()

[PATCH bpf-next] selftsets/bpf: Retry map update on helper_fill_hashmap()

[Gabriele Monaco]

helper_fill_hashmap() is used also on parallel and stress map tests.
Those are consistently failing with ENOMEM on kernels built with
PREEMPT_RT if preallocation is disabled. The failure is transient and
only called by the memory cache refill running in a preemptible
irq_work, which can easily stall in case of contention.

Use a retriable update in those cases to handle transient ENOMEM and
make the test more stable also on PREEMPT_RT.
Also fix the sign of the value printed in case of error (strerror()
expects a positive errno while updates return it negative).

Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 tools/testing/selftests/bpf/test_maps.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index ccc5acd55f..c32da7bd8b 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -260,6 +260,16 @@ static void test_hashmap_percpu(unsigned int task, void *data)
 	close(fd);
 }
 
+#define MAP_RETRIES 20
+
+static bool can_retry(int err)
+{
+	return (err == EAGAIN || err == EBUSY ||
+		((err == ENOMEM || err == E2BIG) &&
+		 map_opts.map_flags == BPF_F_NO_PREALLOC));
+}
+
+
 #define VALUE_SIZE 3
 static int helper_fill_hashmap(int max_entries)
 {
@@ -274,10 +284,11 @@ static int helper_fill_hashmap(int max_entries)
 
 	for (i = 0; i < max_entries; i++) {
 		key = i; value[0] = key;
-		ret = bpf_map_update_elem(fd, &key, value, BPF_NOEXIST);
+		ret = map_update_retriable(fd, &key, value, BPF_NOEXIST,
+					   MAP_RETRIES, can_retry);
 		CHECK(ret != 0,
 		      "can't update hashmap",
-		      "err: %s\n", strerror(ret));
+		      "err: %s\n", strerror(-ret));
 	}
 
 	return fd;
@@ -1392,17 +1403,9 @@ static void test_map_stress(void)
 #define DO_UPDATE 1
 #define DO_DELETE 0
 
-#define MAP_RETRIES 20
 #define MAX_DELAY_US 50000
 #define MIN_DELAY_RANGE_US 5000
 
-static bool can_retry(int err)
-{
-	return (err == EAGAIN || err == EBUSY ||
-		((err == ENOMEM || err == E2BIG) &&
-		 map_opts.map_flags == BPF_F_NO_PREALLOC));
-}
-
 int map_update_retriable(int map_fd, const void *key, const void *value, int flags, int attempts,
 			 retry_for_error_fn need_retry)
 {

base-commit: 2d3090a8aeb596a26935db0955d46c9a5db5c6ce
-- 
2.54.0

Re: [PATCH bpf-next] selftsets/bpf: Retry map update on helper_fill_hashmap()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `can_retry` function incorrectly uses exact equality instead of a bitwise AND to check for `BPF_F_NO_PREALLOC` in `map_opts.map_flags`, preventing retries when other flags are present.
- [Low] The patch modifies a deprecated `CHECK()` macro call without migrating it to modern `ASSERT_*()` macros.
--

commit 39b0e917600bcb89279f4b9e224c7a51b913c4fb
Author: Gabriele Monaco <gmonaco@redhat.com>

selftsets/bpf: Retry map update on helper_fill_hashmap()

This commit enables retrying map updates in helper_fill_hashmap() to handle
transient ENOMEM errors on PREEMPT_RT kernels. It does this by moving the
can_retry() logic up in the file and updating the update loop.

> diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
> index ccc5acd55ff9d..c32da7bd8be27 100644
> --- a/tools/testing/selftests/bpf/test_maps.c
> +++ b/tools/testing/selftests/bpf/test_maps.c
> @@ -260,6 +260,16 @@ static void test_hashmap_percpu(unsigned int task, void *data)
>  	close(fd);
>  }
>  
> +#define MAP_RETRIES 20
> +
> +static bool can_retry(int err)
> +{
> +	return (err == EAGAIN || err == EBUSY ||
> +		((err == ENOMEM || err == E2BIG) &&
> +		 map_opts.map_flags == BPF_F_NO_PREALLOC));
> +}

[Severity: Medium]
Will this exact equality check prevent retries if other bitmask flags are set?

When helper_fill_hashmap() is called by test_hashmap_zero_seed(),
map_opts.map_flags is explicitly modified to include BPF_F_ZERO_SEED. 

This causes the equality check to evaluate to false even when BPF_F_NO_PREALLOC
is actively set, which bypasses the retry mechanism for this test and
leaves it vulnerable to transient ENOMEM failures on PREEMPT_RT.

Could this be updated to use a bitwise AND instead?

> +
> +
>  #define VALUE_SIZE 3
>  static int helper_fill_hashmap(int max_entries)
>  {
> @@ -274,10 +284,11 @@ static int helper_fill_hashmap(int max_entries)
>  
>  	for (i = 0; i < max_entries; i++) {
>  		key = i; value[0] = key;
> -		ret = bpf_map_update_elem(fd, &key, value, BPF_NOEXIST);
> +		ret = map_update_retriable(fd, &key, value, BPF_NOEXIST,
> +					   MAP_RETRIES, can_retry);
>  		CHECK(ret != 0,
>  		      "can't update hashmap",
> -		      "err: %s\n", strerror(ret));
> +		      "err: %s\n", strerror(-ret));

[Severity: Low]
Since this error checking logic and format arguments are being actively
modified, should this deprecated CHECK() macro be migrated to a modern
ASSERT_OK() macro instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611150704.95133-1-gmonaco@redhat.com?part=1

Re: [PATCH bpf-next] selftsets/bpf: Retry map update on helper_fill_hashmap()

[Emil Tsalapatis]

On Thu Jun 11, 2026 at 11:07 AM EDT, Gabriele Monaco wrote:
> helper_fill_hashmap() is used also on parallel and stress map tests.
> Those are consistently failing with ENOMEM on kernels built with
> PREEMPT_RT if preallocation is disabled. The failure is transient and
> only called by the memory cache refill running in a preemptible
> irq_work, which can easily stall in case of contention.
>
> Use a retriable update in those cases to handle transient ENOMEM and
> make the test more stable also on PREEMPT_RT.
> Also fix the sign of the value printed in case of error (strerror()
> expects a positive errno while updates return it negative).
>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>

The bot has a point about changing the code to & BPF_F_PREALLOC, but
we touch test_maps.c twice a year and that only for bugfixes. There will
probably be no other consumer of this function in the foreseeable future.
Similarly, the whole file uses CHECK() so imo it's fine to keep this as-is.

> ---
>  tools/testing/selftests/bpf/test_maps.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
> index ccc5acd55f..c32da7bd8b 100644
> --- a/tools/testing/selftests/bpf/test_maps.c
> +++ b/tools/testing/selftests/bpf/test_maps.c
> @@ -260,6 +260,16 @@ static void test_hashmap_percpu(unsigned int task, void *data)
>  	close(fd);
>  }
>  
> +#define MAP_RETRIES 20
> +
> +static bool can_retry(int err)
> +{
> +	return (err == EAGAIN || err == EBUSY ||
> +		((err == ENOMEM || err == E2BIG) &&
> +		 map_opts.map_flags == BPF_F_NO_PREALLOC));
> +}
> +
> +
>  #define VALUE_SIZE 3
>  static int helper_fill_hashmap(int max_entries)
>  {
> @@ -274,10 +284,11 @@ static int helper_fill_hashmap(int max_entries)
>  
>  	for (i = 0; i < max_entries; i++) {
>  		key = i; value[0] = key;
> -		ret = bpf_map_update_elem(fd, &key, value, BPF_NOEXIST);
> +		ret = map_update_retriable(fd, &key, value, BPF_NOEXIST,
> +					   MAP_RETRIES, can_retry);
>  		CHECK(ret != 0,
>  		      "can't update hashmap",
> -		      "err: %s\n", strerror(ret));
> +		      "err: %s\n", strerror(-ret));
>  	}
>  
>  	return fd;
> @@ -1392,17 +1403,9 @@ static void test_map_stress(void)
>  #define DO_UPDATE 1
>  #define DO_DELETE 0
>  
> -#define MAP_RETRIES 20
>  #define MAX_DELAY_US 50000
>  #define MIN_DELAY_RANGE_US 5000
>  
> -static bool can_retry(int err)
> -{
> -	return (err == EAGAIN || err == EBUSY ||
> -		((err == ENOMEM || err == E2BIG) &&
> -		 map_opts.map_flags == BPF_F_NO_PREALLOC));
> -}
> -
>  int map_update_retriable(int map_fd, const void *key, const void *value, int flags, int attempts,
>  			 retry_for_error_fn need_retry)
>  {
>
> base-commit: 2d3090a8aeb596a26935db0955d46c9a5db5c6ce

Re: [PATCH bpf-next] selftsets/bpf: Retry map update on helper_fill_hashmap()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Thu, 11 Jun 2026 17:07:03 +0200 you wrote:
> helper_fill_hashmap() is used also on parallel and stress map tests.
> Those are consistently failing with ENOMEM on kernels built with
> PREEMPT_RT if preallocation is disabled. The failure is transient and
> only called by the memory cache refill running in a preemptible
> irq_work, which can easily stall in case of contention.
> 
> Use a retriable update in those cases to handle transient ENOMEM and
> make the test more stable also on PREEMPT_RT.
> Also fix the sign of the value printed in case of error (strerror()
> expects a positive errno while updates return it negative).
> 
> [...]

Here is the summary with links:
  - [bpf-next] selftsets/bpf: Retry map update on helper_fill_hashmap()
    https://git.kernel.org/bpf/bpf-next/c/1f32c0d619d9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html