Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] gpu/buddy: block_incompatible() incorrectly rejects mixed subtrees during CLEAR allocations - [High] gpu/buddy: range allocator strict pass skips valid subtrees due to stale split block clear bits --
commit 19316e455db24fbfaa4dda04f7d988cae7073da7 Author: Arunpravin Paneer Selvam <[email protected]> gpu/buddy: restore two-pass clear/dirty guard for range-biased allocation This commit introduces a two-pass range search in the buddy allocator's range-biased allocation path. The strict pass uses block_incompatible() to reject opposite-class blocks, while the fallback pass relaxes this constraint when opposite-class space is the only option remaining. > diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c > index 2f2aaadafe351..6566bad4fef15 100644 > --- a/drivers/gpu/buddy.c > +++ b/drivers/gpu/buddy.c > @@ -1240,11 +1240,19 @@ 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; > + > + return needs_clear != gpu_buddy_block_is_clear(block); > +} [Severity: High] Does this evaluation correctly handle intermediate SPLIT blocks? A SPLIT block's GPU_BUDDY_HEADER_CLEAR bit retains the value from when it was last FREE and is not updated after splitting. If a root or intermediate block was dirty when split, its clear bit remains 0 even if it now contains free clear children. For a CLEAR allocation, will this cause the strict pass to read a stale 0 bit and incorrectly skip the entire valid subtree? Additionally, are mixed subtrees inadvertently rejected during CLEAR allocations here? Since mixed subtrees contain both clear and dirty pages, their is_clear state evaluates to false. Consequently, block_incompatible() will evaluate mixed subtrees as incompatible, causing the strict pass to skip them entirely unless the entire root block is fully clear. > 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) > { [ ... ] > @@ -1315,38 +1324,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] Without the has_clear steering logic, how does the fallback pass handle CLEAR allocations when descending? If the strict pass fails (as described above for mixed subtrees), the allocator executes the fallback pass which ignores class preferences. Because the fallback pass now blindly traverses top-down (right child first) after the removal of the has_clear steering logic, could a CLEAR range allocation incorrectly allocate and return dirty blocks if they happen to be right-most? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
