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

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

On Mon, Jul 08, 2024 at 01:21:00PM -0700, Umesh Nerlige Ramappa wrote:
> Add ref counting for xe_file.
> 

Patch LGTM, but a nit I'd make this first patch in the series.

> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_device.c       |  7 +++++--
>  drivers/gpu/drm/xe/xe_device.h       | 12 ++++++++++++
>  drivers/gpu/drm/xe/xe_device_types.h |  3 +++
>  3 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index babb697652d5..e6eacf1bcce0 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -87,11 +87,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)

Kernel doc as this is exported.

> +void xe_file_destroy(struct kref *ref)
>  {
> +	struct xe_file *xef = container_of(ref, struct xe_file, refcount);
>  	struct xe_device *xe = xef->xe;
>  	struct xe_vm *vm;
>  	unsigned long idx;
> @@ -130,7 +133,7 @@ static void xe_file_close(struct drm_device *dev, struct drm_file *file)
>  		xe_exec_queue_put(q);
>  	}
>  
> -	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 bb07f5669dbb..2a8b370b1fda 100644
> --- a/drivers/gpu/drm/xe/xe_device.h
> +++ b/drivers/gpu/drm/xe/xe_device.h
> @@ -169,5 +169,17 @@ static inline bool xe_device_wedged(struct xe_device *xe)
>  }
>  
>  void xe_device_declare_wedged(struct xe_device *xe);
> +void xe_file_destroy(struct kref *ref);
> +

I forget if this is a hard rule, but public static inlines likely should
have kernl doc too.

Matt

> +static inline struct xe_file *xe_file_get(struct xe_file *xef)
> +{
> +	kref_get(&xef->refcount);
> +	return xef;
> +}
> +
> +static inline void xe_file_put(struct xe_file *xef)
> +{
> +	kref_put(&xef->refcount, xe_file_destroy);
> +}
>  
>  #endif
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index c37be471d11c..ec2b0fb6afe1 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -566,6 +566,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
>