BPF List
 help / color / mirror / Atom feed
* [PATCH bpf] bpf: Avoid soft lockup in __htab_map_lookup_and_delete_batch()
@ 2026-07-09 23:57 Jose Fernandez (Anthropic)
  2026-07-10  0:05 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Jose Fernandez (Anthropic) @ 2026-07-09 23:57 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Brian Vazquez
  Cc: bpf, linux-kernel, Jose Fernandez (Anthropic)

__htab_map_lookup_and_delete_batch() has no rescheduling point. The
batch count bounds how many entries are copied out, not how many
buckets are visited, so one BPF_MAP_LOOKUP_BATCH call can walk the
map end to end. The empty-bucket fast path is worse: it stays inside
a single rcu_read_lock() / bpf_disable_instrumentation() section for
any run of consecutive empty buckets.

That holds up on small maps, but it falls apart at scale. On a
144-CPU arm64 host running a CONFIG_PREEMPT_NONE kernel, periodic
BPF_MAP_LOOKUP_BATCH calls against an LRU hash map with 16,777,216
buckets held a CPU inside the batch op for 77+ seconds and triggered
the soft lockup watchdog.

Commit 75134f16e7dd ("bpf: Add schedule points in batch ops") fixed this
same problem in the generic batch ops, but not in this htab-native path,
which every htab-based hash map variant uses for its lookup[_and_delete]
batch ops.

Complete that fix here. Leave the critical section after 64 consecutive
empty buckets, call cond_resched(), and resume at the saved bucket
cursor. No locks are held at that point, and resuming from the cursor is
already the function's behavior for non-empty buckets. Add a
cond_resched() to the per-bucket loop after copy_to_user(), where every
lock has been dropped. cond_resched_rcu() is not enough here: sleeping
with bpf_prog_active elevated makes tracing programs on that CPU
silently skip their invocations.

Fixes: 057996380a42 ("bpf: Add batch ops to all htab bpf map")
Assisted-by: Claude:unspecified
Signed-off-by: Jose Fernandez (Anthropic) <jose.fernandez@linux.dev>
---
 kernel/bpf/hashtab.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 9f394e1aa2e8..b208a567892a 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -1769,6 +1769,11 @@ static int htab_lru_percpu_map_lookup_and_delete_elem(struct bpf_map *map,
 						 flags);
 }
 
+/* Max consecutive empty buckets to walk in one RCU +
+ * instrumentation-disabled section before rescheduling.
+ */
+#define HTAB_BATCH_EMPTY_RESCHED 64
+
 static int
 __htab_map_lookup_and_delete_batch(struct bpf_map *map,
 				   const union bpf_attr *attr,
@@ -1790,6 +1795,7 @@ __htab_map_lookup_and_delete_batch(struct bpf_map *map,
 	unsigned long flags = 0;
 	bool locked = false;
 	struct htab_elem *l;
+	u32 empty_cnt = 0;
 	struct bucket *b;
 	int ret = 0;
 
@@ -1969,11 +1975,19 @@ __htab_map_lookup_and_delete_batch(struct bpf_map *map,
 
 next_batch:
 	/* If we are not copying data, we can go to next bucket and avoid
-	 * unlocking the rcu.
+	 * unlocking the rcu. Bound the walk though: after
+	 * HTAB_BATCH_EMPTY_RESCHED consecutive empty buckets, fully exit
+	 * the critical section (no locks are held here) and reschedule.
 	 */
 	if (!bucket_cnt && (batch + 1 < htab->n_buckets)) {
 		batch++;
-		goto again_nocopy;
+		if (++empty_cnt < HTAB_BATCH_EMPTY_RESCHED)
+			goto again_nocopy;
+		empty_cnt = 0;
+		rcu_read_unlock();
+		bpf_enable_instrumentation();
+		cond_resched();
+		goto again;
 	}
 
 	rcu_read_unlock();
@@ -1987,11 +2001,13 @@ __htab_map_lookup_and_delete_batch(struct bpf_map *map,
 	}
 
 	total += bucket_cnt;
+	empty_cnt = 0;
 	batch++;
 	if (batch >= htab->n_buckets) {
 		ret = -ENOENT;
 		goto after_loop;
 	}
+	cond_resched();
 	goto again;
 
 after_loop:

---
base-commit: d2c9a99135da931377240942d44f3dea104cedb8
change-id: 20260708-b4-htab-batch-resched-1bce8304766d

Best regards,
--  
Jose Fernandez (Anthropic) <jose.fernandez@linux.dev>


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH bpf] bpf: Avoid soft lockup in __htab_map_lookup_and_delete_batch()
  2026-07-09 23:57 [PATCH bpf] bpf: Avoid soft lockup in __htab_map_lookup_and_delete_batch() Jose Fernandez (Anthropic)
@ 2026-07-10  0:05 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-07-10  0:05 UTC (permalink / raw)
  To: Jose Fernandez (Anthropic); +Cc: bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Multi-line comments do not follow BPF subsystem guidelines.
--

--- Patch [1]: [PATCH bpf] bpf: Avoid soft lockup in __htab_map_lookup_and_delete_batch() ---
commit a1114e030dd1f8d190db49effffca17e5da09d3f
Author: Jose Fernandez (Anthropic) <jose.fernandez@linux.dev>

bpf: Avoid soft lockup in __htab_map_lookup_and_delete_batch()

This commit limits the number of consecutive empty buckets visited in
__htab_map_lookup_and_delete_batch() before yielding. It drops locks and
calls cond_resched() to prevent soft lockup watchdogs from firing on
large sparse maps.

> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -1769,6 +1769,11 @@ static int htab_lru_percpu_map_lookup_and_delete_elem(struct bpf_map *map,
>  						 flags);
>  }
>  
> +/* Max consecutive empty buckets to walk in one RCU +
> + * instrumentation-disabled section before rescheduling.
> + */
> +#define HTAB_BATCH_EMPTY_RESCHED 64

[Severity: Low]
This isn't a bug, but does this multi-line comment follow the preferred BPF
subsystem comment style? The guidelines specify that the opening /* should
be on its own line, rather than having text on the same line.

[ ... ]

> @@ -1969,11 +1975,19 @@ __htab_map_lookup_and_delete_batch(struct bpf_map *map,
>  
>  next_batch:
> -	/* If we are not copying data, we can go to next bucket and avoid
> -	 * unlocking the rcu.
> +	/* If we are not copying data, we can go to next bucket and avoid
> +	 * unlocking the rcu. Bound the walk though: after
> +	 * HTAB_BATCH_EMPTY_RESCHED consecutive empty buckets, fully exit
> +	 * the critical section (no locks are held here) and reschedule.
>  	 */

[Severity: Low]
Similarly, could this multi-line comment be updated to place the opening /*
on its own line to match the modern BPF comment style?

>  	if (!bucket_cnt && (batch + 1 < htab->n_buckets)) {
>  		batch++;
> -		goto again_nocopy;
> +		if (++empty_cnt < HTAB_BATCH_EMPTY_RESCHED)
> +			goto again_nocopy;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-b4-htab-batch-resched-v1-1-ad7a6b3b4513@linux.dev?part=1

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-07-10  0:05 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-09 23:57 [PATCH bpf] bpf: Avoid soft lockup in __htab_map_lookup_and_delete_batch() Jose Fernandez (Anthropic)
2026-07-10  0:05 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox