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

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Massive memory footprint in KUnit test triggers OOM in constrained environments
- [Medium] Benchmark logic bypasses the measured path in the baseline allocator
--

--- Patch [2]: [PATCH v7 2/2] gpu/tests/buddy: add dirty tracker performance KUnit test ---
commit 7af49c21f49c14e81ea1bc47365402c2ec976aec
Author: Arunpravin Paneer Selvam <[email protected]>

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

This commit adds a KUnit test `gpu_test_buddy_dirty_tracker_performance`
to demonstrate the performance benefits of the decoupled dirty-tracker
design compared to the dual-tree approach. It measures contiguous
allocation latency and repeated allocation throughput after fragmentation.

> diff --git a/drivers/gpu/tests/gpu_buddy_test.c b/drivers/gpu/tests/gpu_buddy_test.c
> index f8e56da5058e6..3fc2bfa66ee15 100644
> --- a/drivers/gpu/tests/gpu_buddy_test.c
> +++ b/drivers/gpu/tests/gpu_buddy_test.c

[ ... ]

> +static void gpu_test_buddy_dirty_tracker_performance(struct kunit *test)
> +{
> +	struct gpu_buddy_block *block, *tmp;
> +	unsigned long elapsed_ms;

[ ... ]

> +	KUNIT_ASSERT_FALSE_MSG(test, gpu_buddy_init(&mm, SZ_4G, SZ_4K),
> +			       "buddy_init failed\n");
> +
> +	for (i = 0; i < SZ_4G / SZ_4K; i++)
> +		KUNIT_ASSERT_FALSE_MSG(test,
> +				       gpu_buddy_alloc_blocks(&mm, 0, SZ_4G, SZ_4K, SZ_4K,
> +							      &allocated, 0),
> +				       "buddy_alloc hit an error size=%u\n", SZ_4K);

[Severity: Medium]
Will this massive memory footprint trigger an Out-Of-Memory panic in
constrained environments?

Allocating a 4 GiB pool using 4 KiB block granularity requires creating over
1 million struct gpu_buddy_block nodes dynamically from the slab. This will
consume well over 100 MiB of kernel memory.

Default UML environments for KUnit often have a memory limit of 256 MiB or 
less, and this exhaustive allocation is likely to crash the test suite.

[ ... ]

> +	/*
> +	 * Repeated alloc throughput from a maximally fragmented pool
> +	 *
> +	 * Fill a 4 GiB pool with 4 KiB allocations, free even-indexed blocks
> +	 * as cleared and odd-indexed blocks as dirty.  The alternating pattern
> +	 * ensures every adjacent buddy pair has one cleared half and one dirty
> +	 * half, so each pair lands on opposite sides of the old merge barrier.
> +	 * Each of the 16 384 x 256 KiB allocations in the timed loop has to
> +	 * pay the __force_merge() cost on the alloc path under the old design.
> +	 * With the dirty-tracker design the pool collapses to one max_order
> +	 * block during free(), so each alloc is a simple O(log N) split.
> +	 */

[ ... ]

> +	start = ktime_get();
> +	for (i = 0; i < SZ_4G / SZ_256K; i++)
> +		KUNIT_ASSERT_FALSE_MSG(test,
> +				       gpu_buddy_alloc_blocks(&mm, 0, SZ_4G, SZ_256K, SZ_4K,
> +							      &results, 0),
> +				       "buddy_alloc hit an error size=%u\n", SZ_256K);

[Severity: Medium]
Does this benchmark logic bypass the measured path in the baseline allocator?

The commit message states this measures the __force_merge() cost on the
alloc path under the old design. However, the flags parameter is passed as 0
here, which omits GPU_BUDDY_CONTIGUOUS_ALLOCATION.

Without the contiguous flag, the baseline allocator does not enforce that
the 256 KiB allocation must be a single block. Instead, it will fall back to
satisfying the 256 KiB request with 64 independent 4 KiB blocks.

Because 4 KiB blocks are readily available in the fragmented pool, the
allocation succeeds without ever invoking the __force_merge() fallback.

Are we actually comparing the cost of 64 tree lookups against 1 tree lookup,
rather than measuring the __force_merge() overhead?

> +	end = ktime_get();
> +	elapsed_ms = ktime_to_ms(ktime_sub(end, start));
> +
> +	kunit_info(test, "Repeated 256 KiB allocs from fragmented pool: %lu ms\n",
> +		   elapsed_ms);
> +
> +	gpu_buddy_free_list(&mm, &results, 0);
> +	gpu_buddy_fini(&mm);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2