Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] gpu/buddy: try_harder skips leftward shift on successful out-of-range scan - [High] gpu/buddy: try_harder misses valid range candidates due to strict start and order bounds - [High] gpu/buddy: out-of-window scan inflates filled size, breaking try_harder fallback --
commit 9e12b0182f3fa80f621385a50621b754c45d5811 Author: Arunpravin Paneer Selvam <[email protected]> gpu/buddy: add range-restricted contiguous allocation fallback This commit adds a fallback path for contiguous allocation requests with range restrictions to help prevent false -ENOSPC errors due to fragmented memory. By confining the try_harder scan to the requested range, it attempts to satisfy requests by stitching smaller adjacent blocks. > diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c > --- a/drivers/gpu/buddy.c > +++ b/drivers/gpu/buddy.c > @@ -1685,26 +1685,14 @@ 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) > { > - u64 rhs_offset, lhs_offset, filled; > + u64 rhs_offset, lhs_offset, filled, aligned; [Severity: High] Does this function miss valid range candidates due to its strict starting order bound? __alloc_contig_try_harder() starts its search strictly downwards from order = fls(pages) - 1. If a higher-order block was rejected by the normal allocator because its rounded-up size didn't fit the range, its splits are undone. Because this fallback never searches higher orders, it completely misses any unsplit free blocks that might contain valid range candidates. [ ... ] > @@ -1729,25 +1717,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; [Severity: High] Does this loop termination artificially ignore free space inside a block that starts before the range but extends into it? If a free block begins before range_start but extends well into the valid range, its right half could still satisfy the allocation. Breaking here discards the entire block and terminates the loop, causing the allocator to miss perfectly valid free space. > /* Allocate blocks traversing RHS */ > err = __gpu_buddy_alloc_range(mm, rhs_offset, size, > flags, &filled, blocks); [Severity: High] Does passing size directly here without clamping it to range_end - rhs_offset cause the forward scan to populate filled with space beyond range_end? If the forward scan hits a busy block outside the window, filled will reflect the total contiguous free bytes found, including those beyond range_end. This artificially inflates the filled value. > 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 unconditional goto skip checking if shifting the allocation to the left (to fit within range_end) would succeed, leading to false allocation failures? When err == 0 on the forward scan, we know at least size contiguous bytes are free starting at rhs_offset. If rhs_offset + size > range_end, the allocation could still be satisfied by shifting the window left to end exactly at range_end. However, the code completely bypasses this leftward shift via this goto next, only performing leftward shifts when err == -ENOSPC. This causes the allocator to miss perfectly valid free space that crosses the right boundary but has enough room if shifted left. > } > @@ -1759,15 +1758,17 @@ static int __alloc_contig_try_harder(struct gpu_buddy > *mm, > > /* Allocate the unaligned LHS offset using round_down */ > gpu_buddy_free_list_internal(mm, blocks); > - err = __alloc_contig_aligned_retry(mm, lhs_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(lhs_offset, min_block_size); > + if (aligned >= range_start && aligned + size <= range_end) { [Severity: High] Because filled is artificially inflated from the out-of-window scan above, lhs_offset (calculated prior to this block as rhs_offset - (size - filled)) will be shifted too far to the right. This causes the perfectly valid left-shifted candidate to remain partially out-of-bounds, failing the bounds check here and returning a false -ENOSPC. > + 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; > + } > } > next: > gpu_buddy_free_list_internal(mm, blocks); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
