RE: [PATCH V15 08/14] drm/xe/vram: Add page offline data structures and lifecycle

> -----Original Message-----
> From: Ghimiray, Himal Prasad <[email protected]>
> Sent: 12 August 2026 17:31
> To: Upadhyay, Tejas <[email protected]>; intel-
> [email protected]
> Subject: Re: [PATCH V15 08/14] drm/xe/vram: Add page offline data
> structures and lifecycle
> 
> 
> 
> On 11-08-2026 18:10, Tejas Upadhyay wrote:
> > Add xe_ttm_vram_offline_resource to track individual offlined VRAM
> > pages, and extend xe_ttm_vram_mgr with offlined_pages/queued_pages
> > lists and their counters.
> >
> > Initialize the lists in __xe_ttm_vram_mgr_init() and add
> > xe_ttm_vram_free_bad_pages() to release all tracked pages during
> > xe_ttm_vram_mgr_fini() teardown.
> >
> > Signed-off-by: Tejas Upadhyay <[email protected]>
> > ---
> >   drivers/gpu/drm/xe/xe_ttm_vram_mgr.c       | 24
> ++++++++++++++++++++
> >   drivers/gpu/drm/xe/xe_ttm_vram_mgr_types.h | 26
> ++++++++++++++++++++++
> >   2 files changed, 50 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> > b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> > index 49eeec90a470..2813ae68325e 100644
> > --- a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> > +++ b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> > @@ -300,6 +300,24 @@ static const struct ttm_resource_manager_func
> xe_ttm_vram_mgr_func = {
> >   	.debug	= xe_ttm_vram_mgr_debug
> >   };
> >
> > +static void xe_ttm_vram_free_bad_pages(struct drm_device *dev, struct
> > +xe_ttm_vram_mgr *mgr) {
> 
> Don't see dev being used ?

Ok, will remove.

> 
> > +	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(&pos->offlined_link);
> > +		--mgr->n_offlined_pages;
> > +		kfree(pos);
> > +	}
> 
> This can be racy, better to use
> list_del_rcu() + kfree_rcu()

This is already handled in patch 11 where we introduce RCU lock.

> 
> > +	list_for_each_entry_safe(pos, n, &mgr->queued_pages, queued_link)
> {
> > +		xe_ttm_vram_buddy_free(mgr, &pos->blocks, 0);
> 
> Nit: pos->used_visible_size is always 0, hardcoding is Ok. comment it ?

Sure

> 
> > +		list_del(&pos->queued_link);
> > +		--mgr->n_queued_pages;
> > +		kfree(pos);
> > +	}
> > +}
> > +
> >   static void xe_ttm_vram_mgr_fini(struct drm_device *dev, void *arg)
> >   {
> >   	struct xe_device *xe = to_xe_device(dev); @@ -311,6 +329,10 @@
> > static void xe_ttm_vram_mgr_fini(struct drm_device *dev, void *arg)
> >   	if (ttm_resource_manager_evict_all(&xe->ttm, man))
> >   		return;
> >
> 
> 
> potential leak here due to early return ?

Right, offlined page entries are independent of active Bos which above early return is protecting by leaking, so moving it above the early return would be a valid improvement.

Tejas
> 
> > +	mutex_lock(&mgr->lock);
> > +	xe_ttm_vram_free_bad_pages(dev, mgr);
> > +	mutex_unlock(&mgr->lock);
> > +
> >   	WARN_ON_ONCE(mgr->visible_avail != mgr->visible_size);
> >
> >   	gpu_buddy_fini(&mgr->mm);
> > @@ -338,6 +360,8 @@ int __xe_ttm_vram_mgr_init(struct xe_device *xe,
> struct xe_ttm_vram_mgr *mgr,
> >   	err = drmm_mutex_init(&xe->drm, &mgr->lock);
> >   	if (err)
> >   		return err;
> > +	INIT_LIST_HEAD(&mgr->offlined_pages);
> > +	INIT_LIST_HEAD(&mgr->queued_pages);
> >   	mgr->default_page_size = default_page_size;
> >   	mgr->visible_size = io_size;
> >   	mgr->visible_avail = io_size;
> > diff --git a/drivers/gpu/drm/xe/xe_ttm_vram_mgr_types.h
> > b/drivers/gpu/drm/xe/xe_ttm_vram_mgr_types.h
> > index 9106da056b49..bdfdf6ec1218 100644
> > --- a/drivers/gpu/drm/xe/xe_ttm_vram_mgr_types.h
> > +++ b/drivers/gpu/drm/xe/xe_ttm_vram_mgr_types.h
> > @@ -19,6 +19,14 @@ struct xe_ttm_vram_mgr {
> >   	struct ttm_resource_manager manager;
> >   	/** @mm: DRM buddy allocator which manages the VRAM */
> >   	struct gpu_buddy mm;
> > +	/** @offlined_pages: List of offlined pages */
> > +	struct list_head offlined_pages;
> > +	/** @n_offlined_pages: Number of offlined pages */
> > +	u16 n_offlined_pages;
> > +	/** @queued_pages: List of queued pages */
> > +	struct list_head queued_pages;
> > +	/** @n_queued_pages: Number of queued pages */
> > +	u16 n_queued_pages;
> >   	/** @visible_size: Proped size of the CPU visible portion */
> >   	u64 visible_size;
> >   	/** @visible_avail: CPU visible portion still unallocated */ @@
> > -45,4 +53,22 @@ struct xe_ttm_vram_mgr_resource {
> >   	unsigned long flags;
> >   };
> >
> > +/**
> > + * struct xe_ttm_vram_offline_resource - Tracks a single offlined
> > +VRAM page  */ struct xe_ttm_vram_offline_resource {
> > +	/** @offlined_link: Link into mgr->offlined_pages */
> > +	struct list_head offlined_link;
> > +	/** @queued_link: Link into mgr->queued_pages */
> > +	struct list_head queued_link;
> > +	/** @blocks: Buddy blocks reserved for this page */
> > +	struct list_head blocks;
> > +	/** @used_visible_size: CPU-visible bytes consumed */
> > +	u64 used_visible_size;
> > +	/** @addr: Faulty DPA reported by HW */
> > +	u64 addr;
> > +	/** @status: Reservation status (0=pending, 1=fail) */
> > +	bool status;
> > +};
> > +
> >   #endif