Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] gpu/buddy: 32-bit shift overflow in gpu_buddy_alloc_blocks() on BIT(order) - [Low] gpu/buddy: malformed kernel-doc comment marker above is_power_of_2_u64() --
commit 73704a4c8e819013a9e0b34c59f4447160bbf9ae Author: David Gow <[email protected]> drm_buddy: Fix power-of-two rounding errors This commit introduces u64-specific power-of-two helpers in drivers/gpu/buddy.c and widens page count variables from unsigned long to u64 to avoid 32-bit truncation when handling 64-bit GPU allocation sizes on 32-bit architectures. > diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c > index 503f7d5934e4b..504bd013f4ea2 100644 > --- a/drivers/gpu/buddy.c > +++ b/drivers/gpu/buddy.c > @@ -36,6 +36,35 @@ > #define gpu_buddy_assert(condition) WARN_ON(!(condition)) > #endif > > +/** > + * Implementations of pow_of_2 functions targeting u64: > + * > + * The various pow_of_2 functions in log2.h work on longs, which are 32-bit > on > + * 32-bit systems. Since the GPU address space is 64-bit even on systems with > + * a 32-bit CPU, this causes needless truncation. Work is underway to improve > + * the general implementations to support 64-bit types on 32-bit systems, but > + * in the meantime, we implement our own here. > + */ [Severity: Low] Since drivers/gpu/buddy.c is included by Documentation/gpu/drm-mm.rst for kernel-doc processing, starting this comment block above is_power_of_2_u64() with /** instead of /* causes scripts/kernel-doc to warn during W=1 and documentation builds because the block is not formatted as a kernel-doc comment. Should this comment open with /* instead? [ ... ] > @@ -1970,7 +1999,7 @@ int gpu_buddy_alloc_blocks(struct gpu_buddy *mm, > u64 original_size, original_min_size; > unsigned int min_order, order; > LIST_HEAD(allocated); > - unsigned long pages; > + u64 pages; > int err; [ ... ] > @@ -2017,7 +2046,7 @@ int gpu_buddy_alloc_blocks(struct gpu_buddy *mm, > } > > pages = size >> ilog2(mm->chunk_size); > - order = fls(pages) - 1; > + order = fls64(pages) - 1; > min_order = ilog2(min_block_size) - ilog2(mm->chunk_size); > > if (order > mm->max_order || size > mm->size) { > @@ -2031,7 +2060,7 @@ int gpu_buddy_alloc_blocks(struct gpu_buddy *mm, > } > > do { > - order = min(order, (unsigned int)fls(pages) - 1); > + order = min(order, (unsigned int)fls64(pages) - 1); > BUG_ON(order > mm->max_order); > /* > * Regular allocations must not allocate blocks smaller than > min_block_size. [Severity: Medium] In gpu_buddy_alloc_blocks(), widening pages to u64 and computing order with fls64(pages) - 1 allows order to be 32 or greater on 32-bit architectures when an allocation spans at least 1ULL << 32 chunks. However, at the bottom of this allocation loop in gpu_buddy_alloc_blocks(), pages is still decremented using BIT(order): kmemleak_update_trace(block); list_add_tail(&block->link, &allocated); pages -= BIT(order); if (!pages) break; Because BIT(order) expands to (1UL << (order)), on 32-bit systems an order of 32 or more shifts a 32-bit unsigned long past its width, invoking undefined behavior and corrupting the remaining pages count so the loop does not terminate normally. Should that decrement in gpu_buddy_alloc_blocks() use BIT_ULL(order) instead? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
