Re: [PATCH V2 3/3] drm/xe/vram: stop decay worker from rescheduling itself forever

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/xe/vram: soft-offlined pages via queued_pages never arm
  decay_gc_work
--

commit 22ff2c502172a757420f34aa8acb6ee7e2f6e05c
Author: Tejas Upadhyay <tejas.upadhyay@intel.com>

drm/xe/vram: stop decay worker from rescheduling itself forever

This commit changes xe_ttm_vram_decay_gc_worker() to only reschedule
mgr->decay_gc_work when XE_PAGE_OFFLINE_SOFT entries remain on
mgr->offlined_pages after a sweep.

> diff --git a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> index 07360ab7b0722..7613024a1410a 100644
> --- a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> +++ b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> @@ -373,17 +373,20 @@ static void xe_ttm_vram_decay_gc_worker(struct work_struct *work)
>  	struct xe_device *xe = ttm_to_xe_device(mgr->manager.bdev);
>  	unsigned long now = jiffies;
>  	unsigned long decay;
> +	bool soft_remaining = false;
>  
>  	mutex_lock(&mgr->lock);
>  	decay = mgr->decay_interval_jiffies;
>  	if (!decay)
> -		goto reschedule;
> +		goto unlock;
>  
>  	list_for_each_entry_safe(pos, n, &mgr->offlined_pages, offlined_link) {
>  		if (pos->mode != XE_PAGE_OFFLINE_SOFT)
>  			continue;
> -		if (!time_after(now, pos->last_seen + decay))
> +		if (!time_after(now, pos->last_seen + decay)) {
> +			soft_remaining = true;
>  			continue;
> +		}
>  
>  		list_del(&pos->offlined_link);
>  		xe_ttm_vram_buddy_free(mgr, &pos->blocks, pos->used_visible_size);
> @@ -394,9 +397,11 @@ static void xe_ttm_vram_decay_gc_worker(struct work_struct *work)
>  		kfree(pos);
>  	}
>  
> -reschedule:
> -	schedule_delayed_work(&mgr->decay_gc_work,
> -			      xe_ttm_vram_decay_gc_period(mgr));
> +	/* Nothing left to track; re-armed on demand when a new SOFT entry appears */
> +	if (soft_remaining)
> +		schedule_delayed_work(&mgr->decay_gc_work,
> +				      xe_ttm_vram_decay_gc_period(mgr));
> +unlock:
>  	mutex_unlock(&mgr->lock);
>  }

[Severity: Medium]
Can soft-offlined pages that go through mgr->queued_pages be left on
mgr->offlined_pages forever without arming mgr->decay_gc_work?

In xe_ttm_vram_reserve_page_at_addr(), schedule_delayed_work() is only
called when block is NULL and xe_ttm_vram_buddy_alloc() succeeds
immediately.  When a transient fault hits an allocated page, nentry is
placed on vram_mgr->queued_pages while the owning BO is purged, and
xe_ttm_vram_retry_queued_pages() later moves it to mgr->offlined_pages
without scheduling the worker:

drivers/gpu/drm/xe/xe_ttm_vram_mgr.c:xe_ttm_vram_retry_queued_pages() {
	list_for_each_entry_safe(pos, n, &mgr->queued_pages, queued_link) {
		if (xe_ttm_vram_buddy_alloc(mgr, pos->addr, pos->addr + PAGE_SIZE,
					    PAGE_SIZE, PAGE_SIZE, &pos->blocks,
					    GPU_BUDDY_RANGE_ALLOCATION, NULL,
					    &pos->used_visible_size)) {
			pos->status = XE_PAGE_RESERVE_FAIL;
			continue;
		}
		--mgr->n_queued_pages;
		list_del(&pos->queued_link);
		++mgr->n_offlined_pages;
		list_add(&pos->offlined_link, &mgr->offlined_pages);
	}
}

Once mgr->decay_gc_work is idle (either because no free page was offlined
earlier, or because xe_ttm_vram_decay_gc_worker() ran while the entry was
still on mgr->queued_pages), any XE_PAGE_OFFLINE_SOFT entry promoted via
xe_ttm_vram_retry_queued_pages() will never decay and remains offlined
until driver unbind.  Arming mgr->decay_gc_work in
xe_ttm_vram_retry_queued_pages() when a XE_PAGE_OFFLINE_SOFT entry is
moved to mgr->offlined_pages would avoid this.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112124329.2960272-5-tejas.upadhyay@intel.com?part=3