Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The test fails to create the intended alternating block fragmentation, allocating contiguous blocks instead. --
--- Patch [1]: [PATCH v6] drm/tests/gpu_buddy: add a new test case for buffer clearance during resume --- commit 8231e2c55481150c6a9aaa5025aef00267d3c249 Author: Arunpravin Paneer Selvam <[email protected]> drm/tests/gpu_buddy: add a new test case for buffer clearance during resume This commit adds a new unit test case for buffer clearance issues during resume. It aims to allocate alternating blocks of 4KiB in an even and odd sequence, then free them as cleared, to verify that split blocks are properly merged. > diff --git a/drivers/gpu/tests/gpu_buddy_test.c > b/drivers/gpu/tests/gpu_buddy_test.c > --- a/drivers/gpu/tests/gpu_buddy_test.c > +++ b/drivers/gpu/tests/gpu_buddy_test.c [ ... ] > + /* > + * Using a non-power-of-two mm size, allocate alternating blocks of > 4KiB in an > + * even sequence and free them as cleared. All blocks should be marked > as > + * dirty and the split blocks should be merged back to their original > + * size when the blocks clear reset function is called. > + */ > + KUNIT_EXPECT_FALSE(test, gpu_buddy_init(&mm, mm_size, ps)); > + KUNIT_EXPECT_EQ(test, mm.max_order, max_order); > + > + i = 0; > + n_pages = mm_size / ps; > + do { > + if (i % 2 == 0) > + KUNIT_ASSERT_FALSE_MSG(test, > gpu_buddy_alloc_blocks(&mm, 0, mm_size, > + ps, > ps, &allocated, 0), > + "buddy_alloc hit an error size=%lu\n", > ps); > + } while (++i < n_pages); [Severity: Medium] Does this loop actually create an alternating fragmented pattern? Since gpu_buddy_alloc_blocks() simply returns the next available contiguous free block, skipping loop iterations with if (i % 2 == 0) might merely halve the total number of sequential allocations. This would result in a single contiguous chunk of allocated blocks rather than the intended interleaved checkerboard pattern, bypassing the fragmentation scenario the test relies on. To properly interleave blocks, shouldn't the test allocate all pages into two different lists and selectively free one of them, similar to what is done earlier in this same test function? > + > + gpu_buddy_free_list(&mm, &allocated, GPU_BUDDY_CLEARED); > + gpu_buddy_reset_clear(&mm, false); > + KUNIT_EXPECT_EQ(test, mm.clear_avail, 0); > + > + /* > + * Using a non-power-of-two mm size, allocate alternating blocks of > 4KiB in an > + * odd sequence and free them as cleared. All blocks should be marked as > + * cleared and the split blocks should be merged back to their original > + * size when the blocks clear reset function is called. > + */ > + i = 0; > + do { > + if (i % 2 != 0) > + KUNIT_ASSERT_FALSE_MSG(test, > gpu_buddy_alloc_blocks(&mm, 0, mm_size, > + ps, > ps, &allocated, 0), > + "buddy_alloc hit an error size=%lu\n", > ps); > + } while (++i < n_pages); [Severity: Medium] Does this odd sequence suffer from the same contiguous allocation issue as the even sequence loop above? > + > + gpu_buddy_free_list(&mm, &allocated, GPU_BUDDY_CLEARED); > + gpu_buddy_reset_clear(&mm, true); > + KUNIT_EXPECT_EQ(test, mm.clear_avail, mm_size); > + gpu_buddy_fini(&mm); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
