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

[Lucas De Marchi]

On Mon, Jul 08, 2024 at 01:21:00PM GMT, Umesh Nerlige Ramappa wrote:
>Add ref counting for xe_file.
>
>Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at 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)
>+void xe_file_destroy(struct kref *ref)

why do you export this?  I don't think anybody should be calling
xe_file_destroy() directly and this function being executed should only
be the outcome of kref being 0. Also, if it's exported, it shouldn't
really have struct kref as argument, but rather a struct xe_file


> {
>+	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);
>+
>+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)

I think if you just make this non-inline, passing file_destroy and
leave it in the same place, it should work.

Looking for patterns in the kernel for example, see pinctrl_get(),
pinctrl_put() and pinctrl_release().... or clk_get(), clk_put(),
__clk_release()

Lucas De Marchi

>+{
>+	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
>