Re: [PATCH v2 2/4] drm/xe: Add ref counting for xe_file

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

On Mon, Jul 08, 2024 at 05:28:33PM -0700, Umesh Nerlige Ramappa wrote:
> Add ref counting for xe_file.
> 
> v2:
> - Add kernel doc for exported functions (Matt)
> - Instead of xe_file_destroy, export the get/put helpers (Lucas)

Won't get into this bikeshed here.

> 
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_device.c       | 31 ++++++++++++++++++++++++++--
>  drivers/gpu/drm/xe/xe_device.h       |  3 +++
>  drivers/gpu/drm/xe/xe_device_types.h |  3 +++
>  3 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index c5464ad5d908..61dc0a37a4d0 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -90,11 +90,14 @@ static int xe_file_open(struct drm_device *dev, struct drm_file *file)
>  	spin_unlock(&xe->clients.lock);
>  
>  	file->driver_priv = xef;
> +	kref_init(&xef->refcount);
> +
>  	return 0;
>  }
>  
> -static void xe_file_destroy(struct xe_file *xef)
> +static void xe_file_destroy(struct kref *ref)
>  {
> +	struct xe_file *xef = container_of(ref, struct xe_file, refcount);
>  	struct xe_device *xe = xef->xe;
>  
>  	xa_destroy(&xef->exec_queue.xa);
> @@ -110,6 +113,30 @@ static void xe_file_destroy(struct xe_file *xef)
>  	kfree(xef);
>  }
>  
> +/**
> + * xe_file_get: Take a reference to the xe file object

s/xe_file_get/xe_file_get()

> + * @xef: Pointer to the xe file
> + *
> + * Anyone making a copy of xef must take a reference to the xe file

s/'Anyone making a copy'/'Anyone with a pointer'

> + * object using this call.


Returns: xe file pointer

> + */
> +struct xe_file *xe_file_get(struct xe_file *xef)
> +{
> +	kref_get(&xef->refcount);
> +	return xef;
> +}
> +
> +/**
> + * xe_file_put: Drop a reference to the xe file object

s/xe_file_put/xe_file_put()

With the nits fixed:
Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> + * @xef: Pointer to the xe file
> + *
> + * Used to drop reference to the xef object
> + */
> +void xe_file_put(struct xe_file *xef)
> +{
> +	kref_put(&xef->refcount, xe_file_destroy);
> +}
> +
>  static void xe_file_close(struct drm_device *dev, struct drm_file *file)
>  {
>  	struct xe_file *xef = file->driver_priv;
> @@ -132,7 +159,7 @@ static void xe_file_close(struct drm_device *dev, struct drm_file *file)
>  		xe_vm_close_and_put(vm);
>  	mutex_unlock(&xef->vm.lock);
>  
> -	xe_file_destroy(xef);
> +	xe_file_put(xef);
>  }
>  
>  static const struct drm_ioctl_desc xe_ioctls[] = {
> diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> index 0a2a3e7fd402..533ccfb2567a 100644
> --- a/drivers/gpu/drm/xe/xe_device.h
> +++ b/drivers/gpu/drm/xe/xe_device.h
> @@ -171,4 +171,7 @@ static inline bool xe_device_wedged(struct xe_device *xe)
>  
>  void xe_device_declare_wedged(struct xe_device *xe);
>  
> +struct xe_file *xe_file_get(struct xe_file *xef);
> +void xe_file_put(struct xe_file *xef);
> +
>  #endif
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index f0cf9020e463..70115fb8d806 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -578,6 +578,9 @@ struct xe_file {
>  
>  	/** @client: drm client */
>  	struct xe_drm_client *client;
> +
> +	/** @refcount: ref count of this xe file */
> +	struct kref refcount;
>  };
>  
>  #endif
> -- 
> 2.38.1
>