On Wed, 20 Apr 2022 05:17:57 -0700, Andrzej Hajda wrote:
>
> Hi Ashutosh,
Hi Andrzej,
> 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")
/snip/
> > +void intel_gt_sysfs_unregister(struct intel_gt *gt)
> > +{
> > + kobject_put(>->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?
Because we are explicitly doing a kobject_put() in
intel_gt_sysfs_unregister(). Which is exactly what we are *not* doing in
the previous code.
Let me explain a bit about the previous code (but feel free to skip since
the patch should speak for itself):
* Previously we kzalloc a 'struct kobj_gt'
* But we don't save a pointer to the 'struct kobj_gt' so we don't have the
pointer to the kobject to be able to do a kobject_put() on it later
* Therefore we need to store the pointer in 'struct intel_gt'
* But if we have to put the pointer in 'struct intel_gt' we might as well
put the kobject as part of 'struct intel_gt' and that also removes the
need to have a 'struct kobj_gt' (kobj_to_gt() can just use container_of()
to get gt from kobj).
* So I think this patch simpler/cleaner than the original code if you take
the requirement for kobject_put() into account.
Thanks.
--
Ashutosh
