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

[Lucas De Marchi]

On Tue, Jul 09, 2024 at 08:41:49PM GMT, Matthew Brost wrote:
>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.

in the previous version I also pointed at other alternatives that would
allow the inlining you were talking about. Anyway, as we said we may
want to make the pattern uniform across the driver later

>
>>
>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at 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 at intel.com>

agreed with the comments, but the kernel-doc ones don't appear
correct... we document the functions like

/**
  * xe_file_put - Drop a reference to the xe file object
  * @ .... arguments
  *
  * ... additional doc
  *
  * Returns: ...
  */

Lucas De Marchi

>
>> + * @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
>>