Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] gpu/buddy: strict pass in __alloc_range_bias() skips straddling free blocks - [Medium] gpu/buddy: __alloc_range_bias() drops clear child preference over mixed blocks --
commit 39aa2fd8b0782a6f08d3234bf343e92cab53a75f Author: Arunpravin Paneer Selvam <[email protected]> gpu/buddy: restore two-pass clear/dirty guard for range-biased allocation This commit adds a two-pass clear/dirty compatibility guard to __alloc_range_bias() so range-constrained allocations avoid splitting opposite-class extents unless no compatible space exists in the range. It also adds a KUnit regression test covering range-biased clear/dirty isolation. > diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c > index 28cff97d57dc8..d9749cbaf05d6 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) > { [ ... ] > @@ -1296,8 +1306,19 @@ __alloc_range_bias(struct gpu_buddy *mm, > if (round_down(adjusted_end + 1, req_size) <= > round_up(adjusted_start, req_size)) > continue; > + > + /* > + * Strict pass: don't split a partially-overlapping free > + * block; rejecting its child would strand the split. > Let > + * the fallback pass consume it (either class accepted). > + */ > + if (!fallback && gpu_buddy_block_is_free(block)) > + continue; > } > > + if (!fallback && block_incompatible(block, flags)) > + continue; [Severity: Medium] In gpu_buddy_alloc_blocks() -> __gpu_buddy_alloc_blocks() -> __gpu_buddy_alloc_range_bias() -> __alloc_range_bias(), when a sub-range [start, end) is smaller than a free root or coalesced ancestor block (so block_start < start || block_end > end), doesn't skipping every partially overlapping free block in the strict pass cause the strict pass to skip descending into that block even when it contains compatible space or is itself wholly compatible with the request? Since __gpu_buddy_free() coalesces free buddy pairs across clear and dirty states into mixed free blocks, a coalesced free block or a wholly compatible free block straddling start or end will be skipped by: if (block_start < start || block_end > end) { ... if (!fallback && gpu_buddy_block_is_free(block)) continue; } When that causes the strict pass to return -ENOSPC, __gpu_buddy_alloc_range_bias() retries with fallback = true, which disables block_incompatible() across the entire traversal and allows splitting higher-address opposite-class extents inside [start, end) even when compatible blocks exist in the range. Similarly, when dirty_steer_window() narrows [start, end) to a dirty window inside a larger coalesced mixed free block, won't the strict pass always skip the enclosing free block and fail with -ENOSPC before retrying in the fallback pass? > if (contains(start, end, block_start, block_end) && > order == gpu_buddy_block_order(block)) { > /* > @@ -1315,38 +1336,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: Medium] In __alloc_range_bias(), block_incompatible() treats both wholly clear blocks and mixed blocks (where block->has_clear is true but gpu_buddy_block_is_clear(block) is false) as compatible when GPU_BUDDY_CLEAR_ALLOCATION is set. With the child preference check removed here so block->right is always visited before block->left, if a split parent has a mixed right child and a wholly clear left child at the target order, won't a non-top-down GPU_BUDDY_CLEAR_ALLOCATION request now select the mixed right child (with GPU_BUDDY_HEADER_CLEAR unset, requiring the caller to re-clear it) while leaving the wholly clear left sibling unused? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
