Re: [PATCH 7/9] drm/i915/gt: Fix memory leaks in per-gt sysfs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ashutosh,

On 20.04.2022 07:21, Ashutosh Dixit wrote:
All kmalloc'd kobjects need a kobject_put() to free memory. For example in
previous code, kobj_gt_release() never gets called. The requirement of
kobject_put() now results in a slightly different code organization.

Cc: Andi Shyti <andi.shyti@xxxxxxxxx>
Cc: Andrzej Hajda <andrzej.hajda@xxxxxxxxx>
Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Fixes: b770bcfae9ad ("drm/i915/gt: create per-tile sysfs interface")
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx>
---
  drivers/gpu/drm/i915/gt/intel_gt.c       |  1 +
  drivers/gpu/drm/i915/gt/intel_gt_sysfs.c | 35 ++++++++++--------------
  drivers/gpu/drm/i915/gt/intel_gt_sysfs.h |  6 +---
  drivers/gpu/drm/i915/gt/intel_gt_types.h |  3 ++
  drivers/gpu/drm/i915/i915_sysfs.c        |  2 ++
  5 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index f0014c5072c9..f0c56ca12c0b 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -783,6 +783,7 @@ void intel_gt_driver_unregister(struct intel_gt *gt)
  {
  	intel_wakeref_t wakeref;
+ intel_gt_sysfs_unregister(gt);
  	intel_rps_driver_unregister(&gt->rps);
intel_pxp_fini(&gt->pxp);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
index 8ec8bc660c8c..6f1b081ca5b7 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
@@ -24,7 +24,7 @@ bool is_object_gt(struct kobject *kobj)
static struct intel_gt *kobj_to_gt(struct kobject *kobj)
  {
-	return container_of(kobj, struct kobj_gt, base)->gt;
+	return container_of(kobj, struct intel_gt, sysfs_gtn);
  }
struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
@@ -72,21 +72,19 @@ static struct attribute *id_attrs[] = {
  };
  ATTRIBUTE_GROUPS(id);
-static void kobj_gt_release(struct kobject *kobj)
+/* A kobject needs a release() method even if it does nothing */
+static void kobj_gtn_release(struct kobject *kobj)
  {
-	kfree(kobj);
  }
-static struct kobj_type kobj_gt_type = {
-	.release = kobj_gt_release,
+static struct kobj_type kobj_gtn_type = {
+	.release = kobj_gtn_release,
  	.sysfs_ops = &kobj_sysfs_ops,
  	.default_groups = id_groups,
  };
void intel_gt_sysfs_register(struct intel_gt *gt)
  {
-	struct kobj_gt *kg;
-
  	/*
  	 * We need to make things right with the
  	 * ABI compatibility. The files were originally
@@ -98,25 +96,22 @@ void intel_gt_sysfs_register(struct intel_gt *gt)
  	if (gt_is_root(gt))
  		intel_gt_sysfs_pm_init(gt, gt_get_parent_obj(gt));
- kg = kzalloc(sizeof(*kg), GFP_KERNEL);
-	if (!kg)
+	/* init and xfer ownership to sysfs tree */
+	if (kobject_init_and_add(&gt->sysfs_gtn, &kobj_gtn_type,
+				 gt->i915->sysfs_gt, "gt%d", gt->info.id))
  		goto exit_fail;
- kobject_init(&kg->base, &kobj_gt_type);
-	kg->gt = gt;
-
-	/* xfer ownership to sysfs tree */
-	if (kobject_add(&kg->base, gt->i915->sysfs_gt, "gt%d", gt->info.id))
-		goto exit_kobj_put;
-
-	intel_gt_sysfs_pm_init(gt, &kg->base);
+	intel_gt_sysfs_pm_init(gt, &gt->sysfs_gtn);
return; -exit_kobj_put:
-	kobject_put(&kg->base);
-
  exit_fail:
+	kobject_put(&gt->sysfs_gtn);
  	drm_warn(&gt->i915->drm,
  		 "failed to initialize gt%d sysfs root\n", gt->info.id);
  }
+
+void intel_gt_sysfs_unregister(struct intel_gt *gt)
+{
+	kobject_put(&gt->sysfs_gtn);
+}
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
index 9471b26752cf..a99aa7e8b01a 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
@@ -13,11 +13,6 @@
struct intel_gt; -struct kobj_gt {
-	struct kobject base;
-	struct intel_gt *gt;
-};
-
  bool is_object_gt(struct kobject *kobj);
struct drm_i915_private *kobj_to_i915(struct kobject *kobj);
@@ -28,6 +23,7 @@ intel_gt_create_kobj(struct intel_gt *gt,
  		     const char *name);
void intel_gt_sysfs_register(struct intel_gt *gt);
+void intel_gt_sysfs_unregister(struct intel_gt *gt);
  struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
  					    const char *name);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index 937b2e1a305e..4c72b4f983a6 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -222,6 +222,9 @@ struct intel_gt {
  	} mocs;
struct intel_pxp pxp;
+
+	/* gt/gtN sysfs */
+	struct kobject sysfs_gtn;

If you put kobject as a part of intel_gt what assures you that lifetime of kobject is shorter than intel_gt? Ie its refcounter is 0 on removal of intel_gt?

Regards
Andrzej

  };
enum intel_gt_scratch_field {
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index 8521daba212a..3f06106cdcf5 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -259,4 +259,6 @@ void i915_teardown_sysfs(struct drm_i915_private *dev_priv)
device_remove_bin_file(kdev, &dpf_attrs_1);
  	device_remove_bin_file(kdev,  &dpf_attrs);
+
+	kobject_put(dev_priv->sysfs_gt);
  }




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux