Re: [PATCH 2/2] drm/xe: Add debugfs stats for DMA-mapped pages per order

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

On Wed, Nov 04, 2026 at 10:15:05PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] drm/xe: premature xe_device_assert_dma_pages_zero() before ttm_device_fini()
> - [Low] drm/xe: duplicate dma_map_account callbacks in xe_svm.c and xe_userptr.c
> --
> 
> commit d97316f4a4863b594d6196037dc27dd4e278bdd0
> Author: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> 
> drm/xe: Add debugfs stats for DMA-mapped pages per order
> 
> This commit adds per-order DMA-mapped system page counters for TTM buffer
> objects and GPU SVM / userptr ranges, exposes them via debugfs at
> dma_mapped_pages, and asserts that all counters return to zero during device
> teardown.
> 
> > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> > index 273e2c45fea9b..c40bb8f7c697a 100644
> > --- a/drivers/gpu/drm/xe/xe_device.c
> > +++ b/drivers/gpu/drm/xe/xe_device.c
> > @@ -377,12 +377,42 @@ bool xe_device_is_admin_only(const struct xe_device *xe)
> [ ... ]
> >  static void xe_device_destroy(struct drm_device *dev, void *dummy)
> >  {
> >  	struct xe_device *xe = to_xe_device(dev);
> >  
> >  	xe_bo_dev_fini(&xe->bo_device);
> >  
> > +	/*
> > +	 * Assert the DMA-mapped-pages accounting only after xe_bo_dev_fini()
> > +	 * has flushed the async BO-free worker: BOs pending async free still
> > +	 * hold their DMA mappings (and counts) until the worker runs, so
> > +	 * asserting before the flush could trip a false positive.
> > +	 */
> > +	xe_device_assert_dma_pages_zero(xe);
> > +
> >  	if (xe->preempt_fence_wq)
> >  		destroy_workqueue(xe->preempt_fence_wq);
> >  
> >  	if (xe->ordered_wq)
> >  		destroy_workqueue(xe->ordered_wq);
> >  
> >  	if (xe->unordered_wq)
> >  		destroy_workqueue(xe->unordered_wq);
> >  
> >  	if (xe->destroy_wq)
> >  		destroy_workqueue(xe->destroy_wq);
> >  
> >  	ttm_device_fini(&xe->ttm);
> 
> [Severity: Medium]
> Can xe_device_assert_dma_pages_zero() trigger a false-positive assertion here
> because it runs before ttm_device_fini(&xe->ttm)?
> 
> When BOs with a populated ttm_tt and DMA-mapped xe_tt->sg are released via
> ttm_bo_put() -> ttm_bo_release(), TTM defers cleanup to bdev->wq
> (xe->ttm.wq) whenever want_init_on_free() is active or reservation fences or
> locks are busy:
> 
> drivers/gpu/drm/ttm/ttm_bo.c:ttm_bo_release()
> 	if (!dma_resv_test_signaled(&bo->base._resv,
> 				    DMA_RESV_USAGE_BOOKKEEP) ||
> 	    (want_init_on_free() && (bo->ttm != NULL)) ||
> 	    bo->type == ttm_bo_type_sg ||
> 	    !dma_resv_trylock(bo->base.resv)) {
> 		...
> 		INIT_WORK(&bo->delayed_delete, ttm_bo_delayed_delete);
> 		...
> 		queue_work_node(bdev->pool.nid, bdev->wq, &bo->delayed_delete);
> 		return;
> 	}
> 
> In that case, the DMA unmap and counter decrement via:
> 
>   ttm_bo_delayed_delete()
>     -> ttm_bo_cleanup_memtype_use()
>       -> ttm_bo_tt_destroy()
>         -> ttm_tt_unpopulate()
>           -> xe_ttm_tt_unpopulate()
>             -> xe_tt_unmap_sg()
>               -> xe_tt_account_dma_pages(xe, xe_tt, -1)
> 
> will not complete until ttm_device_fini() drains bdev->wq. Should
> xe_device_assert_dma_pages_zero(xe) be called after ttm_device_fini(&xe->ttm)
> instead?
> 

Yes, I can fix this when merging.

Matt

> > diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c
> > index 909a602d091ea..7a49882897495 100644
> > --- a/drivers/gpu/drm/xe/xe_svm.c
> > +++ b/drivers/gpu/drm/xe/xe_svm.c
> > @@ -845,10 +845,50 @@ static int xe_svm_get_pagemaps(struct xe_vm *vm)
> >  }
> >  #endif
> >  
> > +#if IS_ENABLED(CONFIG_DEBUG_FS)
> > +/*
> > + * xe_svm_dma_map_account - drm_gpusvm DMA-mapping accounting callback
> > + * @gpusvm: The GPU SVM the mapping belongs to
> > + * @addr: The address descriptor of the chunk being (un)mapped
> > + * @sign: +1 when @addr was DMA-mapped, -1 when it is being unmapped
> > + *
> > + * Maintain per-order counts of the system-memory pages that the full SVM
> > + * instance (SVM ranges, and userptr VMAs in fault mode) has DMA-mapped for GPU
> > + * access. Only DRM_INTERCONNECT_SYSTEM entries are real DMA maps and counted;
> > + * device interconnect (VRAM, P2P) entries are skipped. The counts are solely
> > + * exposed through debugfs, so the callback is only registered when
> > + * CONFIG_DEBUG_FS is enabled.
> > + */
> > +static void xe_svm_dma_map_account(struct drm_gpusvm *gpusvm,
> > +				   const struct drm_pagemap_addr *addr,
> > +				   int sign)
> > +{
> > +	struct xe_device *xe = gpusvm_to_vm(gpusvm)->xe;
> > +	unsigned int order = addr->order;
> > +
> > +	if (addr->proto != DRM_INTERCONNECT_SYSTEM)
> > +		return;
> > +
> > +	/*
> > +	 * The CPU-PTE map order reported by hmm can exceed MAX_PAGE_ORDER
> > +	 * (e.g. an order-18 1GB mapping), so clamp the array index to avoid an
> > +	 * out-of-bounds access. Clamping is symmetric across the +1/-1 calls,
> > +	 * so the per-order counters stay balanced.
> > +	 */
> > +	order = min(order, (unsigned int)NR_PAGE_ORDERS - 1);
> > +
> > +	atomic_long_add((long)(1UL << addr->order) * sign,
> > +			&xe->mem.dma_mapped_pages_svm[order]);
> > +}
> > +#endif
> > +
> >  static const struct drm_gpusvm_ops gpusvm_ops = {
> >  	.range_alloc = xe_svm_range_alloc,
> >  	.range_free = xe_svm_range_free,
> >  	.invalidate = xe_svm_invalidate,
> > +#if IS_ENABLED(CONFIG_DEBUG_FS)
> > +	.dma_map_account = xe_svm_dma_map_account,
> > +#endif
> >  };
> [ ... ]
> > diff --git a/drivers/gpu/drm/xe/xe_userptr.c b/drivers/gpu/drm/xe/xe_userptr.c
> > index 9c1dac0fce6f1..40f97bd4751dd 100644
> > --- a/drivers/gpu/drm/xe/xe_userptr.c
> > +++ b/drivers/gpu/drm/xe/xe_userptr.c
> > @@ -8,9 +8,72 @@
> >  
> >  #include <linux/mm.h>
> >  
> > +#include <drm/drm_pagemap.h>
> > +
> >  #include "xe_tlb_inval.h"
> >  #include "xe_trace_bo.h"
> >  
> > +#if IS_ENABLED(CONFIG_DEBUG_FS)
> > +/*
> > + * xe_userptr_dma_map_account - drm_gpusvm DMA-mapping accounting callback
> > + * @gpusvm: The GPU SVM the mapping belongs to
> > + * @addr: The address descriptor of the chunk being (un)mapped
> > + * @sign: +1 when @addr was DMA-mapped, -1 when it is being unmapped
> > + *
> > + * Account, per page order, the system-memory pages DMA-mapped for GPU access
> > + * through the drm_gpusvm_pages instance shared by userptr (and, in fault mode,
> > + * SVM). Only DRM_INTERCONNECT_SYSTEM entries are real DMA maps and counted;
> > + * device interconnect (VRAM, P2P) entries are skipped. The counts are solely
> > + * exposed through debugfs, so the callback is only registered when
> > + * CONFIG_DEBUG_FS is enabled.
> > + */
> > +static void xe_userptr_dma_map_account(struct drm_gpusvm *gpusvm,
> > +				       const struct drm_pagemap_addr *addr,
> > +				       int sign)
> > +{
> > +	struct xe_vm *vm = container_of(gpusvm, struct xe_vm, svm.gpusvm);
> > +	unsigned int order = addr->order;
> > +
> > +	if (addr->proto != DRM_INTERCONNECT_SYSTEM)
> > +		return;
> > +
> > +	/*
> > +	 * The CPU-PTE map order reported by hmm can exceed MAX_PAGE_ORDER
> > +	 * (e.g. an order-18 1GB mapping), so clamp the array index to avoid an
> > +	 * out-of-bounds access. Clamping is symmetric across the +1/-1 calls,
> > +	 * so the per-order counters stay balanced.
> > +	 */
> > +	order = min(order, (unsigned int)NR_PAGE_ORDERS - 1);
> > +
> > +	atomic_long_add((long)(1UL << addr->order) * sign,
> > +			&vm->xe->mem.dma_mapped_pages_svm[order]);
> > +}
> 
> [Severity: Low]
> This isn't a bug, but the commit message and comment state that the
> dma_map_account callback is shared between the fault-mode SVM instance and the
> simple userptr instance, whereas xe_svm_dma_map_account() in
> drivers/gpu/drm/xe/xe_svm.c and xe_userptr_dma_map_account() in
> drivers/gpu/drm/xe/xe_userptr.c duplicate the same implementation.
> 
> Since CONFIG_DRM_XE_GPUSVM selects CONFIG_DRM_GPUSVM, could
> xe_userptr_dma_map_account() be exposed and reused in gpusvm_ops instead of
> defining two identical callbacks?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261104215228.1533692-1-matthew.brost@intel.com?part=2