mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] drm/ttm: Account for NULL pages in ttm_pool_backup
@ 2026-06-26  7:46 Matthew Brost
  2026-07-01 11:57 ` Thomas Hellström
  0 siblings, 1 reply; 3+ messages in thread
From: Matthew Brost @ 2026-06-26  7:46 UTC (permalink / raw)
  To: intel-xe, dri-devel
  Cc: Christian Koenig, Huang Rui, Matthew Auld, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	Thomas Hellström, linux-kernel, stable

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
+
+		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));
 		}
 	}
 
-- 
2.34.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/ttm: Account for NULL pages in ttm_pool_backup
  2026-06-26  7:46 [PATCH] drm/ttm: Account for NULL pages in ttm_pool_backup Matthew Brost
@ 2026-07-01 11:57 ` Thomas Hellström
  2026-07-02 22:21   ` Matthew Brost
  0 siblings, 1 reply; 3+ messages in thread
From: Thomas Hellström @ 2026-07-01 11:57 UTC (permalink / raw)
  To: Matthew Brost, intel-xe, dri-devel
  Cc: Christian Koenig, Huang Rui, Matthew Auld, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	linux-kernel, stable

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.

/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));
>  		}
>  	}
>  

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/ttm: Account for NULL pages in ttm_pool_backup
  2026-07-01 11:57 ` Thomas Hellström
@ 2026-07-02 22:21   ` Matthew Brost
  0 siblings, 0 replies; 3+ messages in thread
From: Matthew Brost @ 2026-07-02 22:21 UTC (permalink / raw)
  To: Thomas Hellström
  Cc: intel-xe, dri-devel, Christian Koenig, Huang Rui, Matthew Auld,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, linux-kernel, stable

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));
> >  		}
> >  	}
> >  

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-02 22:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-26  7:46 [PATCH] drm/ttm: Account for NULL pages in ttm_pool_backup Matthew Brost
2026-07-01 11:57 ` Thomas Hellström
2026-07-02 22:21   ` Matthew Brost

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox