Re: [PATCH v2] gpu: buddy: avoid repeated builds of root dfs when alloc range

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Spurious -ENOSPC failure in __alloc_range() for fully free memory ranges due to unmerged SPLIT blocks.
--

--- Patch [1]: [PATCH v2] gpu: buddy: avoid repeated builds of root dfs when alloc range ---
commit 87b87afa0112d6b6bc23de2294a961d8ef1fb83e
Author: Zhaoyu Liu <liuzhaoyu.zackary@picoheart.com>

gpu: buddy: avoid repeated builds of root dfs when alloc range

This commit optimizes the buddy allocator by avoiding wasteful repeated builds
of the DFS list during range allocations. It introduces __get_candidate_block()
to unify block acquisition, falling back to lazily fetching the next root block
only when the DFS list is exhausted.

> diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c
> index 3c73ae87f3c5b..72f521c51b3fa 100644
> --- a/drivers/gpu/buddy.c
> +++ b/drivers/gpu/buddy.c

[ ... ]

> @@ -1030,14 +1059,10 @@ static int __alloc_range(struct gpu_buddy *mm,
>  		u64 block_start;
>  		u64 block_end;
>  
> -		block = list_first_entry_or_null(dfs,
> -						 struct gpu_buddy_block,
> -						 tmp_link);
> -		if (!block)
> +		block = __get_candidate_block(mm, dfs, &cursor);
> +		if (IS_ERR(block))
>  			break;
>  
> -		list_del(&block->tmp_link);
> -
>  		block_start = gpu_buddy_block_offset(block);
>  		block_end = block_start + gpu_buddy_block_size(mm, block) - 1;

[Severity: High]
This is a pre-existing issue, but could allocations fail spuriously in
__alloc_range() due to unmerged SPLIT blocks?

Looking further down in this same loop in __alloc_range():

    if (contains(start, end, block_start, block_end)) {
        if (gpu_buddy_block_is_free(block)) {
            ...
        } else if (!mm->clear_avail) {
            err = -ENOSPC;
            goto err_free;
        }
    }

If a previous allocation attempt splits a large free block but fails and does
not undo the splits, the parent block is left marked as SPLIT with fully FREE
children.

If the system globally has no available clear memory (mm->clear_avail == 0), a
subsequent call to __alloc_range() requesting a range that completely
encompasses this SPLIT block will satisfy the contains() check. However,
because the parent block is SPLIT rather than FREE, it hits the
!mm->clear_avail condition and immediately aborts the entire allocation with
-ENOSPC.

Should this logic instead traverse down to allocate the perfectly free
children instead of aborting?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/alTlW8TmTXLocWKP@hostpc?part=1