Re: [PATCH v8 1/2] gpu/buddy: replace dual-tree/force_merge with decoupled dirty tracker

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Deadlock via mempool exhaustion under the buddy allocator lock
- [Medium] has_clear flag is not reset upon allocation, corrupting the free tree's clear-steering heuristic
--

--- Patch [1]: [PATCH v8 1/2] gpu/buddy: replace dual-tree/force_merge with decoupled dirty tracker ---
commit 2bd11b8a5badb80e1216b4be0ddab3e1dbd3cdbb
Author: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>

gpu/buddy: replace dual-tree/force_merge with decoupled dirty tracker

This commit replaces the dual-tree buddy allocator with a single free tree
and a decoupled dirty tracker to avoid merging barriers during mixed
workloads. It removes the O(N) __force_merge scan and instead uses an
augmented interval rbtree for tracking dirty extents.

> diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c
> --- a/drivers/gpu/buddy.c
> +++ b/drivers/gpu/buddy.c
[ ... ]
> +static struct gpu_dirty_extent *extent_alloc(struct gpu_dirty_tracker *dirty_tracker)
> +{
> +	/*
> +	 * The void free/reset paths must record an extent and cannot handle
> +	 * failure, so the mempool reserve guarantees a non-NULL return
> +	 * without __GFP_NOFAIL. GFP_KERNEL is safe under the buddy lock: no
> +	 * driver frees buddy blocks from a shrinker, so reclaim cannot
> +	 * recurse into the lock we hold.
> +	 */
> +	return mempool_alloc(dirty_tracker->extent_pool, GFP_KERNEL);
> +}

[Severity: High]
Could this lead to a deadlock through mempool exhaustion under the buddy
allocator lock?

The comment assumes that because GFP_KERNEL can sleep for reclaim, it will
eventually succeed without __GFP_NOFAIL. However, during an OOM event,
kmalloc can fail and return NULL.

If this happens during a loop in gpu_buddy_reset_clear() or 
__gpu_buddy_free_list() while holding the driver lock, the first allocation
failure consumes the single reserved mempool element.

A subsequent allocation failure in the same loop would block in
mempool_alloc() waiting for a free element. Since the blocking thread holds
the driver lock, no other thread can enter the allocator to free extents,
which seems like it would cause an unrecoverable deadlock.

[ ... ]
> +static inline enum gpu_block_state
> +gpu_block_cached_state(struct gpu_buddy_block *block)
> +{
> +	if (gpu_buddy_block_is_clear(block))
> +		return GPU_BLOCK_CLEAR;
> +	if (block->has_clear)
> +		return GPU_BLOCK_MIXED;
> +	return GPU_BLOCK_DIRTY;
> +}

[Severity: Medium]
Will this read a stale has_clear flag for blocks that were allocated without
being split?

Looking at mark_allocated(), the has_clear flag is not reset when the block
changes to the allocated state. If a block previously evaluated as
GPU_BLOCK_MIXED is allocated at its natural order and given to the driver,
has_clear remains true.

When the block is later freed, __gpu_buddy_free() uses this cached state
instead of querying the dirty tracker, inserting the now fully dirty block
into the free tree as MIXED.

Would it be better to reset has_clear when the block is marked as allocated
to prevent the tree's augmentation from diverging from the actual dirty
tracker state?

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