Re: [PATCH v4] gpu/buddy: restore two-pass clear/dirty guard for range-biased allocation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] gpu/buddy: strict pass in __alloc_range_bias() skips straddling free blocks
- [Medium] gpu/buddy: __alloc_range_bias() drops clear child preference over mixed blocks
--

commit 39aa2fd8b0782a6f08d3234bf343e92cab53a75f
Author: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>

gpu/buddy: restore two-pass clear/dirty guard for range-biased allocation

This commit adds a two-pass clear/dirty compatibility guard to
__alloc_range_bias() so range-constrained allocations avoid splitting
opposite-class extents unless no compatible space exists in the range. It
also adds a KUnit regression test covering range-biased clear/dirty
isolation.

> diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c
> index 28cff97d57dc8..d9749cbaf05d6 100644
> --- a/drivers/gpu/buddy.c
> +++ b/drivers/gpu/buddy.c
> @@ -1240,11 +1240,23 @@ static void __gpu_buddy_undo_splits(struct gpu_buddy *mm,
>  	}
>  }
>  
> +static bool block_incompatible(struct gpu_buddy_block *block, unsigned int flags)
> +{
> +	bool needs_clear = flags & GPU_BUDDY_CLEAR_ALLOCATION;
> +
> +	if (!gpu_buddy_block_is_free(block))
> +		return false;
> +
> +	return needs_clear ? !block->has_clear :
> +			     gpu_buddy_block_is_clear(block);
> +}
> +
>  static struct gpu_buddy_block *
>  __alloc_range_bias(struct gpu_buddy *mm,
>  		   u64 start, u64 end,
>  		   unsigned int order,
> -		   unsigned long flags)
> +		   unsigned long flags,
> +		   bool fallback)
>  {

[ ... ]

> @@ -1296,8 +1306,19 @@ __alloc_range_bias(struct gpu_buddy *mm,
>  			if (round_down(adjusted_end + 1, req_size) <=
>  			    round_up(adjusted_start, req_size))
>  				continue;
> +
> +			/*
> +			 * Strict pass: don't split a partially-overlapping free
> +			 * block; rejecting its child would strand the split. Let
> +			 * the fallback pass consume it (either class accepted).
> +			 */
> +			if (!fallback && gpu_buddy_block_is_free(block))
> +				continue;
>  		}
>  
> +		if (!fallback && block_incompatible(block, flags))
> +			continue;

[Severity: Medium]
In gpu_buddy_alloc_blocks() -> __gpu_buddy_alloc_blocks() ->
__gpu_buddy_alloc_range_bias() -> __alloc_range_bias(), when a sub-range
[start, end) is smaller than a free root or coalesced ancestor block (so
block_start < start || block_end > end), doesn't skipping every partially
overlapping free block in the strict pass cause the strict pass to skip
descending into that block even when it contains compatible space or is
itself wholly compatible with the request?

Since __gpu_buddy_free() coalesces free buddy pairs across clear and dirty
states into mixed free blocks, a coalesced free block or a wholly compatible
free block straddling start or end will be skipped by:

	if (block_start < start || block_end > end) {
		...
		if (!fallback && gpu_buddy_block_is_free(block))
			continue;
	}

When that causes the strict pass to return -ENOSPC,
__gpu_buddy_alloc_range_bias() retries with fallback = true, which disables
block_incompatible() across the entire traversal and allows splitting
higher-address opposite-class extents inside [start, end) even when
compatible blocks exist in the range.

Similarly, when dirty_steer_window() narrows [start, end) to a dirty window
inside a larger coalesced mixed free block, won't the strict pass always
skip the enclosing free block and fail with -ENOSPC before retrying in the
fallback pass?

>  		if (contains(start, end, block_start, block_end) &&
>  		    order == gpu_buddy_block_order(block)) {
>  			/*
> @@ -1315,38 +1336,8 @@ __alloc_range_bias(struct gpu_buddy *mm,
>  				goto err_undo;
>  		}
>  
> -		/*
> -		 * Top-down is a strict address-placement policy, so when it is
> -		 * requested we ignore clear steering and simply descend the
> -		 * right (higher-address) child first. Only a non-top-down clear
> -		 * request biases the descent towards clear/has_clear subtrees.
> -		 */
> -		if ((flags & GPU_BUDDY_CLEAR_ALLOCATION) &&
> -		    !(flags & GPU_BUDDY_TOPDOWN_ALLOCATION)) {
> -			struct gpu_buddy_block *prefer;
> -
> -			if (gpu_buddy_block_is_clear(block->right))
> -				prefer = block->right;
> -			else if (gpu_buddy_block_is_clear(block->left))
> -				prefer = block->left;
> -			else if (block->right->has_clear)
> -				prefer = block->right;
> -			else if (block->left->has_clear)
> -				prefer = block->left;
> -			else
> -				prefer = block->right;
> -
> -			if (prefer == block->right) {
> -				list_add(&block->left->tmp_link, &dfs);
> -				list_add(&block->right->tmp_link, &dfs);
> -			} else {
> -				list_add(&block->right->tmp_link, &dfs);
> -				list_add(&block->left->tmp_link, &dfs);
> -			}
> -		} else {
> -			list_add(&block->left->tmp_link, &dfs);
> -			list_add(&block->right->tmp_link, &dfs);
> -		}
> +		list_add(&block->left->tmp_link, &dfs);
> +		list_add(&block->right->tmp_link, &dfs);
>  	} while (1);

[Severity: Medium]
In __alloc_range_bias(), block_incompatible() treats both wholly clear
blocks and mixed blocks (where block->has_clear is true but
gpu_buddy_block_is_clear(block) is false) as compatible when
GPU_BUDDY_CLEAR_ALLOCATION is set.

With the child preference check removed here so block->right is always
visited before block->left, if a split parent has a mixed right child and a
wholly clear left child at the target order, won't a non-top-down
GPU_BUDDY_CLEAR_ALLOCATION request now select the mixed right child (with
GPU_BUDDY_HEADER_CLEAR unset, requiring the caller to re-clear it) while
leaving the wholly clear left sibling unused?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104072847.2617886-1-arunpravin.paneerselvam@amd.com?part=1