Re: [PATCH v4 1/3] 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:
- [High] gpu/buddy: block_incompatible() incorrectly rejects mixed subtrees during CLEAR allocations
- [High] gpu/buddy: range allocator strict pass skips valid subtrees due to stale split block clear bits
--

commit 19316e455db24fbfaa4dda04f7d988cae7073da7
Author: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>

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

This commit introduces a two-pass range search in the buddy allocator's
range-biased allocation path. The strict pass uses block_incompatible() to
reject opposite-class blocks, while the fallback pass relaxes this constraint
when opposite-class space is the only option remaining.

> diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c
> index 2f2aaadafe351..6566bad4fef15 100644
> --- a/drivers/gpu/buddy.c
> +++ b/drivers/gpu/buddy.c
> @@ -1240,11 +1240,19 @@ 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;
> +
> +	return needs_clear != gpu_buddy_block_is_clear(block);
> +}

[Severity: High]
Does this evaluation correctly handle intermediate SPLIT blocks?

A SPLIT block's GPU_BUDDY_HEADER_CLEAR bit retains the value from when it
was last FREE and is not updated after splitting. If a root or intermediate
block was dirty when split, its clear bit remains 0 even if it now contains
free clear children.

For a CLEAR allocation, will this cause the strict pass to read a stale 0 bit
and incorrectly skip the entire valid subtree?

Additionally, are mixed subtrees inadvertently rejected during CLEAR
allocations here?

Since mixed subtrees contain both clear and dirty pages, their is_clear
state evaluates to false. Consequently, block_incompatible() will evaluate
mixed subtrees as incompatible, causing the strict pass to skip them entirely
unless the entire root block is fully clear.

>  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)
>  {

[ ... ]

> @@ -1315,38 +1324,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: High]
Without the has_clear steering logic, how does the fallback pass handle
CLEAR allocations when descending?

If the strict pass fails (as described above for mixed subtrees), the
allocator executes the fallback pass which ignores class preferences.
Because the fallback pass now blindly traverses top-down (right child first)
after the removal of the has_clear steering logic, could a CLEAR range
allocation incorrectly allocate and return dirty blocks if they happen to
be right-most?

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