Re: [PATCH] drm/pagemap: pass pagemap_addr by reference

[Thomas Hellström]

On Mon, 2026-02-16 at 13:16 +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@xxxxxxxx>
> 
> Passing a structure by reference into a function is sometimes
> problematic,

Do you mean passing by *value* is problematic?

> for a number of reasons. Of of these is a warning from the 32-bit arm
> compiler:
> 
> drivers/gpu/drm/drm_gpusvm.c: In function '__drm_gpusvm_unmap_pages':
> drivers/gpu/drm/drm_gpusvm.c:1152:33: note: parameter passing for
> argument of type 'struct drm_pagemap_addr' changed in GCC 9.1
>  1152 |                                 dpagemap->ops-
> >device_unmap(dpagemap,
>       |                                
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  1153 |                                                            
> dev, *addr);
>       |                                                            
> ~~~~~~~~~~~
> 
> This particular problem is harmless since we are not mixing compiler
> versions
> inside of the compiler. However, passing this by reference avoids the
> warning
> along with providing slightly better calling conventions as it avoids
> an
> extra copy on the stack.
> 
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> ---
>  drivers/gpu/drm/drm_gpusvm.c  | 2 +-
>  drivers/gpu/drm/drm_pagemap.c | 2 +-
>  drivers/gpu/drm/xe/xe_svm.c   | 8 ++++----
>  include/drm/drm_pagemap.h     | 2 +-
>  4 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gpusvm.c
> b/drivers/gpu/drm/drm_gpusvm.c
> index c25f50cad6fe..81626b00b755 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -1150,7 +1150,7 @@ static void __drm_gpusvm_unmap_pages(struct
> drm_gpusvm *gpusvm,
>  					       addr->dir);
>  			else if (dpagemap && dpagemap->ops-
> >device_unmap)
>  				dpagemap->ops-
> >device_unmap(dpagemap,
> -							    dev,
> *addr);
> +							    dev,
> addr);
>  			i += 1 << addr->order;
>  		}
>  
> diff --git a/drivers/gpu/drm/drm_pagemap.c
> b/drivers/gpu/drm/drm_pagemap.c
> index d0041c947a28..22579806c055 100644
> --- a/drivers/gpu/drm/drm_pagemap.c
> +++ b/drivers/gpu/drm/drm_pagemap.c
> @@ -318,7 +318,7 @@ static void
> drm_pagemap_migrate_unmap_pages(struct device *dev,
>  			struct drm_pagemap_zdd *zdd = page-
> >zone_device_data;
>  			struct drm_pagemap *dpagemap = zdd-
> >dpagemap;
>  
> -			dpagemap->ops->device_unmap(dpagemap, dev,
> pagemap_addr[i]);
> +			dpagemap->ops->device_unmap(dpagemap, dev,
> &pagemap_addr[i]);
>  		} else {
>  			dma_unmap_page(dev, pagemap_addr[i].addr,
>  				       PAGE_SIZE <<
> pagemap_addr[i].order, dir);
> diff --git a/drivers/gpu/drm/xe/xe_svm.c
> b/drivers/gpu/drm/xe/xe_svm.c
> index 213f0334518a..184cfaac8baf 100644
> --- a/drivers/gpu/drm/xe/xe_svm.c
> +++ b/drivers/gpu/drm/xe/xe_svm.c
> @@ -1676,13 +1676,13 @@ xe_drm_pagemap_device_map(struct drm_pagemap
> *dpagemap,
>  
>  static void xe_drm_pagemap_device_unmap(struct drm_pagemap
> *dpagemap,
>  					struct device *dev,
> -					struct drm_pagemap_addr
> addr)
> +					struct drm_pagemap_addr
> *addr)
>  {
> -	if (addr.proto != XE_INTERCONNECT_P2P)
> +	if (addr->proto != XE_INTERCONNECT_P2P)
>  		return;
>  
> -	dma_unmap_resource(dev, addr.addr, PAGE_SIZE << addr.order,
> -			   addr.dir, DMA_ATTR_SKIP_CPU_SYNC);
> +	dma_unmap_resource(dev, addr->addr, PAGE_SIZE << addr-
> >order,
> +			   addr->dir, DMA_ATTR_SKIP_CPU_SYNC);
>  }
>  
>  static void xe_pagemap_destroy_work(struct work_struct *work)
> diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h
> index 2baf0861f78f..74a32d0dacd8 100644
> --- a/include/drm/drm_pagemap.h
> +++ b/include/drm/drm_pagemap.h
> @@ -95,7 +95,7 @@ struct drm_pagemap_ops {
>  	 */
>  	void (*device_unmap)(struct drm_pagemap *dpagemap,
>  			     struct device *dev,
> -			     struct drm_pagemap_addr addr);
> +			     struct drm_pagemap_addr *addr);

Makes sense, although this should preferrably be

const struct drm_pagemap_addr *addr;

Thanks,
Thomas




>  
>  	/**
>  	 * @populate_mm: Populate part of the mm with @dpagemap
> memory,