Re: [PATCH V17 07/12] drm/xe/vram: Add page offline data structures and lifecycle

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] RCU-001 Violation: Synchronous freeing of buddy blocks before list removal.
--

--- Patch [7]: [PATCH V17 07/12] drm/xe/vram: Add page offline data structures and lifecycle ---
commit f7feb662373b2ef4fb339b58b4b97a02acc79be4
Author: Tejas Upadhyay <[email protected]>

drm/xe/vram: Add page offline data structures and lifecycle

This commit adds xe_ttm_vram_offline_resource to track individual offlined VRAM
pages, and extends xe_ttm_vram_mgr with offlined_pages/queued_pages lists and
their counters. It also initializes the lists and adds a teardown path to release
all tracked pages.

> diff --git a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> index 16ecea4977809..5513dc014f3e7 100644
> --- a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> +++ b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
[ ... ]
> @@ -338,12 +338,35 @@ static void xe_ttm_vram_mgr_set_unused(struct drm_device *dev, void *arg)
>  	ttm_resource_manager_set_used(man, false);
>  }
>  
> +static void xe_ttm_vram_free_bad_pages(struct xe_ttm_vram_mgr *mgr)
> +{
> +	struct xe_ttm_vram_offline_resource *pos, *n;
> +
> +	list_for_each_entry_safe(pos, n, &mgr->offlined_pages, offlined_link) {
> +		xe_ttm_vram_buddy_free(mgr, &pos->blocks, pos->used_visible_size);
> +		list_del_rcu(&pos->offlined_link);

[Severity: Medium]
Does this violate RCU removal ordering by freeing resources before unlinking?

The buddy blocks are freed synchronously before the object is unlinked from the
RCU-protected offlined_pages list. While this is currently called during the
serialized teardown phase, freeing internal resources before unlinking the
object breaks RCU invariants. A concurrent reader could access pos->blocks
after it has been freed. Should list_del_rcu() happen before
xe_ttm_vram_buddy_free()?

> +		--mgr->n_offlined_pages;
> +		kfree_rcu(pos, rcu);
> +	}
> +	list_for_each_entry_safe(pos, n, &mgr->queued_pages, queued_link) {
> +		/* queued entries have no buddy reservation yet */
> +		xe_ttm_vram_buddy_free(mgr, &pos->blocks, 0);
> +		list_del_rcu(&pos->queued_link);

[Severity: Medium]
Similarly, does this pose the same RCU removal ordering issue?

The buddy blocks are freed before unlinking the object from the RCU-protected
queued_pages list.

> +		--mgr->n_queued_pages;
> +		kfree_rcu(pos, rcu);
> +	}
> +}

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