Re: [PATCH v8 2/2] gpu/tests/buddy: add dirty tracker performance KUnit test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Factual error in documentation and commit message regarding __force_merge() execution path
--

--- Patch [2]: [PATCH v8 2/2] gpu/tests/buddy: add dirty tracker performance KUnit test ---
commit fc0bf373f19d84c8ba58395148cb3dec974d2a81
Author: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>

gpu/tests/buddy: add dirty tracker performance KUnit test

This commit adds a KUnit test to demonstrate the performance advantage of the
decoupled dirty-tracker design over the older dual-tree force_merge approach
during severe fragmentation scenarios.

>      1. Contiguous 4 GiB alloc: the old design requires __force_merge()
>         to rebuild max_order from scratch; the new design coalesces during
>         free() so the alloc is O(log N).
>    
>           old (force_merge) - 71 ms
>           dirty tracker design - 17 ms

[Severity: Medium]
Is this description accurate regarding the execution of __force_merge()?

Looking at gpu_buddy_alloc_blocks(), when start is 0, size is SZ_4G, and
end is SZ_4G, the condition start + size == end evaluates to true:

	/* Actual range allocation */
	if (start + size == end) {
		if (!IS_ALIGNED(start | end, min_block_size))
			return -EINVAL;

		return __gpu_buddy_alloc_range(mm, start, size, flags, NULL, blocks);
	}

This takes the __gpu_buddy_alloc_range() fast-path, which completely bypasses
__alloc_contig_try_harder() and never invokes __force_merge().

Does the 71 ms latency actually come from the O(N) overhead of traversing and
linking the 1,048,576 unmerged 4 KiB blocks covering the range, rather than
the cost of __force_merge()?

> diff --git a/drivers/gpu/tests/gpu_buddy_test.c b/drivers/gpu/tests/gpu_buddy_test.c
> index 198d8dc4e3f0e..4dc54ed33269e 100644
> --- a/drivers/gpu/tests/gpu_buddy_test.c
> +++ b/drivers/gpu/tests/gpu_buddy_test.c
[ ... ]
> @@ -283,6 +283,119 @@ static void gpu_test_buddy_fragmentation_performance(struct kunit *test)
[ ... ]
> +static void gpu_test_buddy_dirty_tracker_performance(struct kunit *test)
> +{
[ ... ]
> +	/*
> +	 * Contiguous alloc latency after alternating clear/dirty fragmentation
> +	 *
> +	 * Fill a 4 GiB pool with 4 KiB allocations, partition them into
> +	 * alternating cleared and dirty sets, then free both.  In the old
> +	 * dual-tree design every adjacent buddy pair has one cleared half and
> +	 * one dirty half, so the pair sits on opposite sides of the clear/dirty
> +	 * merge barrier and cannot be coalesced at free() time.  The pool
> +	 * stays fully fragmented and the subsequent contiguous 4 GiB allocation
> +	 * has to invoke __force_merge() to climb back up to max_order before
> +	 * it can succeed.  With the dirty-tracker design buddy pairs coalesce

[Severity: Medium]
The same documentation mismatch appears here in the inline comment for
gpu_test_buddy_dirty_tracker_performance(). Since this 4 GiB allocation
triggers the exact range allocation path in gpu_buddy_alloc_blocks(),
__force_merge() is not invoked. Could we update the comment to accurately
reflect the source of the latency in the old design?

> +	 * unconditionally during free(), so the pool is already at max_order
> +	 * before the timed alloc begins and __force_merge() is not needed.
> +	 */
> +	KUNIT_ASSERT_FALSE_MSG(test, gpu_buddy_init(&mm, SZ_4G, SZ_4K),
> +			       "buddy_init failed\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810091231.11936-1-Arunpravin.PaneerSelvam@amd.com?part=2