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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm: gpu/buddy try-harder fails range allocation when free space exceeds range_end
- [High] mm: gpu/buddy try-harder misses higher-order blocks for range allocations
--

commit 3089b8dadc6718a57b0162914961a76a654e9d6b
Author: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Subject: gpu/buddy: add range-restricted contiguous allocation fallback

This commit adds support for range-restricted contiguous allocation
fallback in the GPU buddy allocator. It extends __alloc_contig_try_harder
to confine candidate placements to a specified [range_start, range_end)
window.

> diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c
> index 6566bad4fef15..1f648bcfce574 100644
> --- a/drivers/gpu/buddy.c
> +++ b/drivers/gpu/buddy.c
> @@ -1682,26 +1682,14 @@ static int __gpu_buddy_alloc_range(struct gpu_buddy *mm,
[ ... ]
>  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)
>  {

[Severity: High]
Does the try-harder fallback miss higher-order blocks for range allocations?

When this function is called as a fallback, it derives an order based on the
unrounded size and searches exclusively in mm->free_tree[order]:

        modify_size = rounddown_pow_of_two(size);
        pages = modify_size >> ilog2(mm->chunk_size);
        order = fls(pages) - 1;
        ...
        root = &mm->free_tree[order];
        if (RB_EMPTY_ROOT(root))
                return -ENOSPC;

However, for range contiguous allocations, the normal allocation pass rounds up
the requested size. If a higher-order block intersects the range but cannot
fit the rounded-up size within the bounds, it gets skipped and is not split.

If this occurs, the valid contiguous space remains in a higher-order tree.

Since the try-harder fallback only checks the single derived order's tree,
will it fail immediately with -ENOSPC, missing the free space that resides in
the unsplit, higher-order blocks?

> -	u64 rhs_offset, lhs_offset, filled;
> +	u64 rhs_offset, lhs_offset, filled, aligned;
>  	struct gpu_buddy_block *block;
>  	struct rb_root *root;
>  	struct rb_node *iter;
> @@ -1726,25 +1714,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;
> +
>  		/* Allocate blocks traversing RHS */
>  		err =  __gpu_buddy_alloc_range(mm, rhs_offset, size,
>  					       flags, &filled, blocks);
>  		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 bypass the left-shift fallback when free space exceeds range_end?

__alloc_contig_try_harder() attempts to place a contiguous block by calling
__gpu_buddy_alloc_range() without passing range_end.

If the contiguous free space extends beyond range_end, this allocation fully
succeeds and returns err == 0.

Because the allocation succeeded, the condition aligned + size <= range_end
will evaluate to false if the memory exceeds the requested range, and the code
hits goto next:

                        if (aligned >= range_start &&
                            aligned + size <= range_end) {
                                ...
                        }
                        goto next;
                }

This completely skips the lhs_offset fallback path that normally follows:

                if (size - filled > rhs_offset)
                        goto next;
                lhs_offset = rhs_offset - (size - filled);
                ...

Had the allocator been bounded by range_end, it would have returned -ENOSPC
with filled set, triggering the left-shift fallback to perfectly slide the
allocation into the requested range.

Will the presence of extra free memory outside the range cause valid contiguous
allocations to spuriously fail?

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