Re: [PATCH v3 1/2] gpu/buddy: add range-restricted contiguous allocation fallback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] gpu/buddy: try_harder skips leftward shift on successful out-of-range scan
- [High] gpu/buddy: try_harder misses valid range candidates due to strict start and order bounds
- [High] gpu/buddy: out-of-window scan inflates filled size, breaking try_harder fallback
--

commit 9e12b0182f3fa80f621385a50621b754c45d5811
Author: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>

gpu/buddy: add range-restricted contiguous allocation fallback

This commit adds a fallback path for contiguous allocation requests with
range restrictions to help prevent false -ENOSPC errors due to fragmented
memory. By confining the try_harder scan to the requested range, it attempts
to satisfy requests by stitching smaller adjacent blocks.

> diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c
> --- a/drivers/gpu/buddy.c
> +++ b/drivers/gpu/buddy.c
> @@ -1685,26 +1685,14 @@ static int __alloc_contig_try_harder(struct gpu_buddy *mm,
> +				     u64 range_start, u64 range_end,
>  				     u64 size,
>  				     u64 min_block_size,
>  				     unsigned long flags,
>  				     struct list_head *blocks)
>  {
> -	u64 rhs_offset, lhs_offset, filled;
> +	u64 rhs_offset, lhs_offset, filled, aligned;

[Severity: High]
Does this function miss valid range candidates due to its strict starting
order bound?

__alloc_contig_try_harder() starts its search strictly downwards from
order = fls(pages) - 1. If a higher-order block was rejected by the normal
allocator because its rounded-up size didn't fit the range, its splits are
undone. Because this fallback never searches higher orders, it completely
misses any unsplit free blocks that might contain valid range candidates.

[ ... ]

> @@ -1729,25 +1717,36 @@ static int __alloc_contig_try_harder(struct gpu_buddy *mm,
>  
>  		rhs_offset = gpu_buddy_block_offset(block);
>  
> +		if (rhs_offset >= range_end) {
> +			iter = rb_prev(iter);
> +			continue;
> +		}
> +		if (rhs_offset < range_start)
> +			break;

[Severity: High]
Does this loop termination artificially ignore free space inside a block
that starts before the range but extends into it?

If a free block begins before range_start but extends well into the valid
range, its right half could still satisfy the allocation. Breaking here
discards the entire block and terminates the loop, causing the allocator
to miss perfectly valid free space.

>  		/* Allocate blocks traversing RHS */
>  		err =  __gpu_buddy_alloc_range(mm, rhs_offset, size,
>  					       flags, &filled, blocks);

[Severity: High]
Does passing size directly here without clamping it to range_end - rhs_offset
cause the forward scan to populate filled with space beyond range_end?

If the forward scan hits a busy block outside the window, filled will
reflect the total contiguous free bytes found, including those beyond
range_end. This artificially inflates the filled value.

>  		if (err && err != -ENOSPC)
>  			return err;
> -		if (!err && IS_ALIGNED(rhs_offset, min_block_size))
> +		if (!err && IS_ALIGNED(rhs_offset, min_block_size) &&
> +		    rhs_offset >= range_start && rhs_offset + size <= range_end)
>  			return 0;
>  		if (!err) {
>  			/* Allocate the unaligned RHS offset using round_down */
>  			gpu_buddy_free_list_internal(mm, blocks);
> -			err = __alloc_contig_aligned_retry(mm, rhs_offset,
> -							   size,
> -							   min_block_size,
> -							   flags, blocks);
> -			if (!err)
> -				return 0;
> -			if (err != -ENOSPC) {
> -				gpu_buddy_free_list_internal(mm, blocks);
> -				return err;
> +
> +			aligned = round_down(rhs_offset, min_block_size);
> +			if (aligned >= range_start &&
> +			    aligned + size <= range_end) {
> +				err = __gpu_buddy_alloc_range(mm, aligned, size,
> +							      flags, NULL, blocks);
> +				if (!err)
> +					return 0;
> +				if (err != -ENOSPC) {
> +					gpu_buddy_free_list_internal(mm, blocks);
> +					return err;
> +				}
>  			}
>  			goto next;

[Severity: High]
Does this unconditional goto skip checking if shifting the allocation to the
left (to fit within range_end) would succeed, leading to false allocation
failures?

When err == 0 on the forward scan, we know at least size contiguous bytes
are free starting at rhs_offset. If rhs_offset + size > range_end, the
allocation could still be satisfied by shifting the window left to end exactly
at range_end.

However, the code completely bypasses this leftward shift via this goto next,
only performing leftward shifts when err == -ENOSPC. This causes the allocator
to miss perfectly valid free space that crosses the right boundary but has
enough room if shifted left.

>  		}
> @@ -1759,15 +1758,17 @@ static int __alloc_contig_try_harder(struct gpu_buddy *mm,
>  
>  		/* Allocate the unaligned LHS offset using round_down */
>  		gpu_buddy_free_list_internal(mm, blocks);
> -		err = __alloc_contig_aligned_retry(mm, lhs_offset,
> -						   size,
> -						   min_block_size,
> -						   flags, blocks);
> -		if (!err)
> -			return 0;
> -		if (err != -ENOSPC) {
> -			gpu_buddy_free_list_internal(mm, blocks);
> -			return err;
> +
> +		aligned = round_down(lhs_offset, min_block_size);
> +		if (aligned >= range_start && aligned + size <= range_end) {

[Severity: High]
Because filled is artificially inflated from the out-of-window scan above,
lhs_offset (calculated prior to this block as rhs_offset - (size - filled))
will be shifted too far to the right.

This causes the perfectly valid left-shifted candidate to remain partially
out-of-bounds, failing the bounds check here and returning a false -ENOSPC.

> +			err = __gpu_buddy_alloc_range(mm, aligned, size,
> +						      flags, NULL, blocks);
> +			if (!err)
> +				return 0;
> +			if (err != -ENOSPC) {
> +				gpu_buddy_free_list_internal(mm, blocks);
> +				return err;
> +			}
>  		}
>  next:
>  		gpu_buddy_free_list_internal(mm, blocks);

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