Re: [PATCH bpf-next v6 2/3] bpf, arena: fix range_tree_set inconsistency on kmalloc_nolock failure

[bot+bpf-ci@kernel.org]

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

> diff --git a/kernel/bpf/range_tree.c b/kernel/bpf/range_tree.c
> index 15b588377a769..0420ab715f206 100644
> --- a/kernel/bpf/range_tree.c
> +++ b/kernel/bpf/range_tree.c
> @@ -199,6 +199,7 @@ int is_range_tree_set(struct range_tree *rt, u32 start, u32 len)
>  int range_tree_set(struct range_tree *rt, u32 start, u32 len)
>  {
>  	u32 last = start + len - 1;
> +	struct range_node *new_rn = NULL;
>  	struct range_node *right;
>  	struct range_node *left;
>  	int err;
> @@ -208,20 +209,40 @@ int range_tree_set(struct range_tree *rt, u32 start, u32 len)
>  	if (left && left->rn_start <= start && left->rn_last >= last)
>  		return 0;
>
> +	/*
> +	 * The pre-clear probe only decides whether a fresh node is needed;
> +	 * adjacency on either side is invariant across the clear, so it can
> +	 * run before the tree is modified and -ENOMEM leaves it untouched.
> +	 * The merge below re-fetches its handles after the clear instead of
> +	 * relying on the overlapping nodes being updated in place.
> +	 */

This isn't a bug, but would it read closer to the rest of the file if
this block were trimmed to the one non-obvious point - that a node
covering start-1 or last+1 cannot disappear across the clear, so the
probe is safe to do first?

Looking at other comments in this file, they're all single terse lines
like "/* Clear out everything in the range we want to set. */" or
"/* Do we have a left-adjacent range ? */", and the middle two
sentences here describe what the code already shows (the two
range_it_iter_first() calls below and the "/* No adjacent ranges; use
the pre-allocated node */" comment later).

> +	left = range_it_iter_first(rt, start - 1, start - 1);
> +	right = range_it_iter_first(rt, last + 1, last + 1);
> +	if (!left && !right) {
> +		new_rn = kmalloc_nolock(sizeof(struct range_node),
> +					__GFP_ACCOUNT, NUMA_NO_NODE);
> +		if (!new_rn)
> +			return -ENOMEM;
> +	}
> +
>  	/* Clear out everything in the range we want to set. */

[ ... ]


---
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/35700555218