Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Calling `__fls()` with 0 causes undefined behavior when an allocation fulfills the exact remaining pages, potentially causing an infinite loop. -- --- Patch [4]: [PATCH v2 04/33] drm/ttm: Introduce ttm_pool_alloc_iter for __ttm_pool_alloc() --- commit 5c371f076caffdc9b759a99aaeaf1a30e3ac5dd7 Author: Matthew Brost <[email protected]> drm/ttm: Introduce ttm_pool_alloc_iter for __ttm_pool_alloc() This commit factors the working state of the __ttm_pool_alloc loop into a struct ttm_pool_alloc_iter. It pulls the per-order page acquisition into smaller helpers to simplify the nested block and improve readability. > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c > index 370d991c9882b..6826c8ae6fc56 100644 > --- a/drivers/gpu/drm/ttm/ttm_pool.c > +++ b/drivers/gpu/drm/ttm/ttm_pool.c [ ... ] > @@ -819,59 +903,28 @@ static int __ttm_pool_alloc(struct ttm_pool *pool, > struct ttm_tt *tt, > else > gfp_flags |= GFP_HIGHUSER; > > - page_caching = tt->caching; > - allow_pools = true; > - for (order = ttm_pool_alloc_find_order(MAX_PAGE_ORDER, alloc); > + it.gfp_flags = gfp_flags; > + > + for (it.order = ttm_pool_alloc_find_order(MAX_PAGE_ORDER, alloc); > alloc->remaining_pages; > - order = ttm_pool_alloc_find_order(order, alloc)) { > + it.order = ttm_pool_alloc_find_order(it.order, alloc)) { [Severity: High] This is a pre-existing issue, but does this loop increment expression evaluate __fls(0) when the allocation exactly fulfills the request? If the loop successfully allocates the final needed pages during an iteration, alloc->remaining_pages drops to 0 inside the loop body. The C for-loop evaluates the increment expression before checking the loop condition, which calls ttm_pool_alloc_find_order(). Inside ttm_pool_alloc_find_order(), this directly executes: min_t(unsigned int, highest, __fls(alloc->remaining_pages)) Calling __fls(0) translates to __builtin_clzl(0), which is explicitly undefined behavior in GCC and Clang. Could this undefined behavior allow the compiler to assume alloc->remaining_pages is always non-zero because it is passed to __fls, potentially optimizing away the loop termination condition and causing an infinite loop? [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
