Re: [PATCH v6] drm/tests/gpu_buddy: add a new test case for buffer clearance during resume

[sashiko-bot@kernel.org]

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 <Arunpravin.PaneerSelvam@amd.com>

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/20260721114236.507578-1-Arunpravin.PaneerSelvam@amd.com?part=1