Re: [PATCH] drm/ttm: Account for NULL pages in ttm_pool_backup

[Matthew Brost <matthew.brost@intel.com>]

On Wed, Jul 01, 2026 at 01:57:28PM +0200, Thomas Hellström wrote:
> Hi, Matt
> 
> On Fri, 2026-06-26 at 00:46 -0700, Matthew Brost wrote:
> > Pages in ttm_pool_backup can be NULL, and set_pages_array_wb() cannot
> > handle NULL entries. Switch to set_pages_wb() after checking for NULL
> > pages.
> > 
> > Fixes the following oops:
> > 
> > Oops: general protection fault, kernel NULL pointer dereference 0x0:
> > 0000 [#1] SMP NOPTI
> > RIP: 0010:__cpa_process_fault+0xf8/0x770
> > RSP: 0018:ffffc90000a87718 EFLAGS: 00010287
> > RAX: 0000000000000000 RBX: ffffc90000a87868 RCX: 0000000000000000
> > RDX: 0000000000001000 RSI: 0005088000000000 RDI: ffffffff827c5f34
> > RBP: 0005088000000000 R08: ffffc90000a877cb R09: ffffc90000a877d0
> > R10: 0000000000000000 R11: 000000000000001b R12: 000ffffffffff000
> > R13: ffffc90000a87868 R14: ffffc90000a87868 R15: ffff88815b882ae0
> > FS:  0000000000000000(0000) GS:ffff8884ec840000(0000)
> > knlGS:0000000000000000
> > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 00007f930b844000 CR3: 000000000262e003 CR4: 0000000008f70ef0
> > PKRU: 55555554
> > Call Trace:
> >  <TASK>
> >  __change_page_attr_set_clr+0x989/0xe90
> >  ? __purge_vmap_area_lazy+0x6c/0x3a0
> >  ? _vm_unmap_aliases+0x250/0x2a0
> >  set_pages_array_wb+0x7f/0x120
> >  ttm_pool_backup+0x4c9/0x5b0 [ttm]
> >  ? dma_resv_wait_timeout+0x3b/0xf0
> >  ttm_tt_backup+0x32/0x60 [ttm]
> >  ttm_bo_shrink+0x66/0x110 [ttm]
> >  xe_bo_shrink_purge+0x12b/0x1b0 [xe]
> >  xe_bo_shrink+0xbb/0x270 [xe]
> >  __xe_shrinker_walk+0xf7/0x160 [xe]
> >  xe_shrinker_walk+0x9d/0xc0 [xe]
> >  xe_shrinker_scan+0x11f/0x210 [xe]
> >  do_shrink_slab+0x13b/0x270
> >  shrink_slab+0xf1/0x400
> >  shrink_node+0x352/0x8a0
> >  balance_pgdat+0x32c/0x700
> >  kswapd+0x205/0x2f0
> >  ? __pfx_autoremove_wake_function+0x10/0x10
> >  ? __pfx_kswapd+0x10/0x10
> >  kthread+0xd1/0x110
> >  ? __pfx_kthread+0x10/0x10
> >  ret_from_fork+0x1b1/0x200
> >  ? __pfx_kthread+0x10/0x10
> >  ret_from_fork_asm+0x1a/0x30
> >  </TASK>
> > 
> > Cc: Christian Koenig <christian.koenig@amd.com>
> > Cc: Huang Rui <ray.huang@amd.com>
> > Cc: Matthew Auld <matthew.auld@intel.com>
> > Cc: Matthew Brost <matthew.brost@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Maxime Ripard <mripard@kernel.org>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: David Airlie <airlied@gmail.com>
> > Cc: Simona Vetter <simona@ffwll.ch>
> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: stable@vger.kernel.org
> > Fixes: b63d715b8090 ("drm/ttm/pool, drm/ttm/tt: Provide a helper to
> > shrink pages")
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > ---
> >  drivers/gpu/drm/ttm/ttm_pool.c | 49 +++++++++++++++++---------------
> > --
> >  1 file changed, 24 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c
> > b/drivers/gpu/drm/ttm/ttm_pool.c
> > index 682ae4f40424..ea14447411a6 100644
> > --- a/drivers/gpu/drm/ttm/ttm_pool.c
> > +++ b/drivers/gpu/drm/ttm/ttm_pool.c
> > @@ -1064,34 +1064,33 @@ long ttm_pool_backup(struct ttm_pool *pool,
> > struct ttm_tt *tt,
> >  	    ttm_pool_uses_dma_alloc(pool) ||
> > ttm_tt_is_backed_up(tt))
> >  		return -EBUSY;
> >  
> > -#ifdef CONFIG_X86
> > -	/* Anything returned to the system needs to be cached. */
> > -	if (tt->caching != ttm_cached)
> > -		set_pages_array_wb(tt->pages, tt->num_pages);
> > -#endif
> > +	for (i = 0; i < tt->num_pages; i += num_pages) {
> > +		unsigned int order;
> >  
> > -	if (tt->dma_address || flags->purge) {
> > -		for (i = 0; i < tt->num_pages; i += num_pages) {
> > -			unsigned int order;
> > +		page = tt->pages[i];
> > +		if (unlikely(!page)) {
> > +			num_pages = 1;
> > +			continue;
> > +		}
> >  
> > -			page = tt->pages[i];
> > -			if (unlikely(!page)) {
> > -				num_pages = 1;
> > -				continue;
> > -			}
> > +		order = ttm_pool_page_order(pool, page);
> > +		num_pages = 1UL << order;
> >  
> > -			order = ttm_pool_page_order(pool, page);
> > -			num_pages = 1UL << order;
> > -			if (tt->dma_address)
> > -				ttm_pool_unmap(pool, tt-
> > >dma_address[i],
> > -					       num_pages);
> > -			if (flags->purge) {
> > -				shrunken += num_pages;
> > -				page->private = 0;
> > -				__free_pages_gpu_account(page,
> > order, false);
> > -				memset(tt->pages + i, 0,
> > -				       num_pages * sizeof(*tt-
> > >pages));
> > -			}
> > +#ifdef CONFIG_X86
> > +		/* Anything returned to the system needs to be
> > cached. */
> > +		if (tt->caching != ttm_cached)
> > +			set_pages_wb(page, 1 << order);
> > +#endif
> 
> As discussed otherwise, this causes one IPI per page when TLB flushing,
> whereas set_pages_array_wb() causes one per array. IPIs are quite
> costly, and
> set_pages_array_wb() has been tailor-made to only issue one per array,
> so we would want to call it on subarrays in case there are NULL
> pointers and always call it in case there are no NULL pointers.
> 

Thanks, fixing this another rev. Also is possible for a page to be a
backup ptr, so folding that fix as well into the following revs.

Matt

> /Thomas
> 
> 
> > +
> > +		if (tt->dma_address)
> > +			ttm_pool_unmap(pool, tt->dma_address[i],
> > +				       num_pages);
> > +		if (flags->purge) {
> > +			shrunken += num_pages;
> > +			page->private = 0;
> > +			__free_pages_gpu_account(page, order,
> > false);
> > +			memset(tt->pages + i, 0,
> > +			       num_pages * sizeof(*tt->pages));
> >  		}
> >  	}
> >