Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: <intel-xe@lists.freedesktop.org>
Subject: Re: [RFC PATCH 1/2] drm/gpusvm: Add a DMA-mapping accounting callback
Date: Wed, 10 Jun 2026 08:37:41 -0700	[thread overview]
Message-ID: <aimExeYtw8KZBsUq@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <20260610133451.8930-1-thomas.hellstrom@linux.intel.com>

On Wed, Jun 10, 2026 at 03:34:50PM +0200, Thomas Hellström wrote:
> Drivers that use the GPU SVM core to DMA-map system memory for GPU
> access may want to keep track of how many pages they have mapped, for
> example to expose statistics or to detect leaks. Doing this accounting
> in the driver around drm_gpusvm_get_pages() / drm_gpusvm_unmap_pages()
> is error prone: the number, order and kind of the dma_addr[] entries can
> change between map and unmap (migration, partial unmaps, the iova vs
> non-iova paths), which makes symmetric accounting hard to get right.
> 
> Add an optional @dma_map_account callback to struct drm_gpusvm_ops and
> invoke it once per &drm_pagemap_addr entry at the exact points where the
> entry is DMA-mapped (sign == +1) in drm_gpusvm_get_pages() and unmapped
> (sign == -1) in __drm_gpusvm_unmap_pages(). Since the map error path and
> every unmap/free path funnel through __drm_gpusvm_unmap_pages() for the
> entries that were actually mapped, the accounting is symmetric by
> construction. The callback receives the full &drm_pagemap_addr so the
> driver can use the order and the proto field to distinguish system
> memory mappings from device interconnect mappings.
> 
> The callback must also be usable by the core drm_gpusvm_pages-only API
> (mm == NULL), as used e.g. for userptr-style mappings. Relax
> drm_gpusvm_init() to allow such users to pass a restricted @ops that
> only implements the page-level hooks; the full-SVM hooks (@invalidate)
> and the chunk configuration must still be unset in that mode.
> 
> Assisted-by: GitHub_Copilot:claude-opus-4.8
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_gpusvm.c | 17 +++++++++++++++--

I believe this makes sense and is useful for debugging/profiling — the
same applies to the next patch.

I do have a question, though: drm_pagemap also performs DMA mapping for
migrations. Should we include those in the accounting as well, or was
there a reason this was intentionally omitted?

Matt

>  include/drm/drm_gpusvm.h     | 19 +++++++++++++++++++
>  2 files changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index 958cb605aedd..c18f15c77e94 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -393,8 +393,15 @@ int drm_gpusvm_init(struct drm_gpusvm *gpusvm,
>  			return -EINVAL;
>  		mmgrab(mm);
>  	} else {
> -		/* No full SVM mode, only core drm_gpusvm_pages API. */
> -		if (ops || num_chunks || mm_range || notifier_size)
> +		/*
> +		 * No full SVM mode, only core drm_gpusvm_pages API. A
> +		 * restricted @ops that only implements the page-level hooks
> +		 * (e.g. @dma_map_account) is allowed; the full-SVM hooks must
> +		 * not be set.
> +		 */
> +		if (num_chunks || mm_range || notifier_size)
> +			return -EINVAL;
> +		if (ops && ops->invalidate)
>  			return -EINVAL;
>  	}
>  
> @@ -1162,6 +1169,8 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
>  			else if (dpagemap && dpagemap->ops->device_unmap)
>  				dpagemap->ops->device_unmap(dpagemap,
>  							    dev, addr);
> +			if (gpusvm->ops && gpusvm->ops->dma_map_account)
> +				gpusvm->ops->dma_map_account(gpusvm, addr, -1);
>  			i += 1 << addr->order;
>  		}
>  
> @@ -1584,6 +1593,10 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
>  				(addr, DRM_INTERCONNECT_SYSTEM, order,
>  				 dma_dir);
>  		}
> +		if (gpusvm->ops && gpusvm->ops->dma_map_account)
> +			gpusvm->ops->dma_map_account(gpusvm,
> +						     &svm_pages->dma_addr[j],
> +						     1);
>  		i += 1 << order;
>  		num_dma_mapped = i;
>  		flags.has_dma_mapping = true;
> diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
> index 8a4d7134a9a7..87ad2dcaa7c4 100644
> --- a/include/drm/drm_gpusvm.h
> +++ b/include/drm/drm_gpusvm.h
> @@ -75,6 +75,25 @@ struct drm_gpusvm_ops {
>  	void (*invalidate)(struct drm_gpusvm *gpusvm,
>  			   struct drm_gpusvm_notifier *notifier,
>  			   const struct mmu_notifier_range *mmu_range);
> +
> +	/**
> +	 * @dma_map_account: Account a DMA-mapping change (optional)
> +	 * @gpusvm: Pointer to the GPU SVM
> +	 * @addr: The address descriptor of the chunk being (un)mapped
> +	 * @sign: +1 when @addr has just been DMA-mapped, -1 when it is being
> +	 *        unmapped
> +	 *
> +	 * Called once per &drm_pagemap_addr entry, when the entry is
> +	 * DMA-mapped by drm_gpusvm_get_pages() (@sign == +1) and when it is
> +	 * unmapped (@sign == -1), so the driver can keep symmetric accounting
> +	 * of the pages it has mapped for GPU access. The @addr->proto field
> +	 * can be used to distinguish system-memory mappings from device
> +	 * interconnect mappings. May be provided in both full SVM mode and the
> +	 * core drm_gpusvm_pages-only mode.
> +	 */
> +	void (*dma_map_account)(struct drm_gpusvm *gpusvm,
> +				const struct drm_pagemap_addr *addr,
> +				int sign);
>  };
>  
>  /**
> -- 
> 2.54.0
> 

  parent reply	other threads:[~2026-06-10 15:37 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-10 13:34 [RFC PATCH 1/2] drm/gpusvm: Add a DMA-mapping accounting callback Thomas Hellström
2026-06-10 13:34 ` [RFC PATCH 2/2] drm/xe: Add debugfs stats for DMA-mapped pages per order Thomas Hellström
2026-06-10 13:43 ` ✓ CI.KUnit: success for series starting with [RFC,1/2] drm/gpusvm: Add a DMA-mapping accounting callback Patchwork
2026-06-10 14:23 ` ✓ Xe.CI.BAT: " Patchwork
2026-06-10 15:37 ` Matthew Brost [this message]
2026-06-10 17:55 ` ✗ Xe.CI.FULL: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aimExeYtw8KZBsUq@gsse-cloud1.jf.intel.com \
    --to=matthew.brost@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=thomas.hellstrom@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox