Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] gpu/buddy: leaked split on range-straddling mixed block in 
__alloc_range_bias()
--

commit caf65a7be15ef66d98faecfc731749477d23b658
Author: Arunpravin Paneer Selvam <[email protected]>

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

This commit introduces a two-pass clear/dirty eligibility check in
__alloc_range_bias() via block_incompatible() so that range-biased
allocations avoid splitting opposite-class extents during the initial strict
pass before falling back when -ENOSPC is returned. It also accepts mixed
blocks at the requested order as-is and adds a KUnit regression test for
range-biased clear/dirty separation.

> diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c
> index 28cff97d57dc8..f2914f919b04a 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)
>  {

[ ... ]

> @@ -1298,6 +1308,9 @@ __alloc_range_bias(struct gpu_buddy *mm,
>                               continue;
>               }
>  
> +             if (!fallback && block_incompatible(block, flags))
> +                     continue;
> +
>               if (contains(start, end, block_start, block_end) &&
>                   order == gpu_buddy_block_order(block)) {
>                       /*
> @@ -1315,38 +1328,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]
Can __alloc_range_bias() still leave an unmerged split block (and leak the
child gpu_buddy_block allocations on teardown) when a free mixed block of
order greater than the target order straddles the start or end boundary of
the requested [start, end) range?

In __alloc_range_bias(), block_incompatible() returns false for any free
mixed block because block->has_clear is true and gpu_buddy_block_is_clear()
is false across the whole block span. Dropping unsplit_block() relies on the
assumption that splitting a mixed block during the strict pass
(fallback == false) will always find at least one compatible descendant at
the target order.

However, when a free mixed block with order > target order only partially
overlaps [start, end), it passes the adjusted range check in
__alloc_range_bias():

        if (block_start < start || block_end > end) {
                u64 adjusted_start = max(block_start, start);
                u64 adjusted_end = min(block_end, end);

                if (round_down(adjusted_end + 1, req_size) <=
                    round_up(adjusted_start, req_size))
                        continue;
        }

        if (!fallback && block_incompatible(block, flags))
                continue;

        ...

        if (!gpu_buddy_block_is_split(block)) {
                err = split_block(mm, block);
                if (unlikely(err))
                        goto err_undo;
        }

If the compatible portion that made the parent mixed lies in the child
outside [start, end) while the child inside [start, end) is purely
opposite-class (for example, clear when a dirty allocation is requested),
split_block() splits the parent into two free children, the out-of-range
child is skipped by the range check, and the in-range child is rejected by
block_incompatible().

When the allocation is then satisfied elsewhere in the tree, the split parent
is left in GPU_BUDDY_SPLIT state with both children simultaneously in
GPU_BUDDY_FREE state:

1. If the mixed block straddles end (e.g. a 4 MiB allocator with 1 MiB
   chunks where [0, 1M) is dirty free, [1M, 2M) is allocated, and [2M, 4M)
   is a coalesced free mixed block with [2M, 3M) clear and [3M, 4M) dirty),
   a 1 MiB dirty range allocation over [0, 3M) visits [2M, 4M) first, splits
   it, rejects [3M, 4M) as out of range and [2M, 3M) via
   block_incompatible(), and then succeeds at [0, 1M) during the strict pass
   without running the fallback pass.

2. If the mixed block straddles start (e.g. [0, 2M) is a free mixed block
   with [0, 1M) dirty and [1M, 2M) clear, and [2M, 4M) is clear free), a
   1 MiB dirty range allocation over [1M, 4M) skips [2M, 4M) in the strict
   pass, splits [0, 2M) and rejects both halves, fails the strict pass with
   -ENOSPC, and then satisfies the request from [3M, 4M) in the fallback
   pass before visiting [0, 2M).

Because neither child of the abandoned split node is allocated, freeing
other blocks never coalesces across the GPU_BUDDY_SPLIT parent in
__gpu_buddy_free(), leaving the buddy tree permanently fragmented and
triggering gpu_buddy_assert(gpu_buddy_block_is_free(mm->roots[i])) in
gpu_buddy_fini() while leaking the descendant gpu_buddy_block structures in
slab_blocks.

Should __alloc_range_bias() still undo rejected splits when descending
blocks that are not fully contained within [start, end), or check compatibility
against the in-range portion?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to