Re: [PATCH v4 1/2] drm/gpusvm: Add a DMA-mapping accounting callback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] drm/gpusvm: missing locking and concurrency kernel-doc for
  dma_map_account()
--

commit 21732d5b49daa03b298f4befee97b908a74029e5
Author: Thomas Hellström <thomas.hellstrom@linux.intel.com>

drm/gpusvm: Add a DMA-mapping accounting callback

This commit adds an optional dma_map_account callback to struct
drm_gpusvm_ops so drivers can track DMA-mapped pages symmetrically across
map and unmap paths, and allows restricted ops in drm_gpusvm_pages-only
mode.

> diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
> index 4d31c6bba7455..637c19f7169f1 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);
>  };

[Severity: Low]
Should the kernel-doc for @dma_map_account document its locking and
concurrency rules, like @invalidate above it does?

Both drm_gpusvm_dma_map_pages() and __drm_gpusvm_unmap_pages() invoke
dma_map_account() under gpusvm->notifier_lock, which is annotated with
fs_reclaim_acquire(GFP_KERNEL) in drm_gpusvm_init() and is held only in
read mode during drm_gpusvm_get_pages(), drm_gpusvm_free_pages(), and
non-notifier drm_gpusvm_unmap_pages() calls.  Without noting that the
callback runs under notifier_lock (in read or write mode) and can be
invoked concurrently on the same struct drm_gpusvm, a driver could use
non-atomic counters or take reclaim-unsafe locks inside the callback.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113195212.2124791-1-matthew.brost@intel.com?part=1