> > > > +void intel_gt_sysfs_unregister(struct intel_gt *gt)
> > > > +{
> > > > + struct kobject *parent = gt_get_parent_obj(gt);
> > > > +
> > > > + /*
> > > > + * the name gt tells us wether sysfs_root
> > > > + * object was initialized properly
> > > > + */
> > > > + if (!strcmp(gt->sysfs_root.name, "gt"))
> > > > + kobject_put(>->sysfs_root);
> > >
> > > Slightly nicer would be looking at kobj->state_initialized for this check I
> > > think. Or even kref_get_unless_zero on kobj->kref? Ugliness there is double
> > > put on sucess which makes me ask whether holding a reference on parent is
> > > even needed? It can't go away so perhaps it isn't.
> >
> > I'd rather use the state_initialized, even though I don't trust
> > its value if the kobject has failed to initialise earlier, I
> > trust it only if it's '1', maybe I'm paranoic.
>
> But is the reference even needed?
yes, because I _get it here (i.e. above, during initialization):
> > > > +void intel_gt_sysfs_register(struct intel_gt *gt)
> > > > +{
> > > > + struct kobject *parent = kobject_get(gt_get_parent_obj(gt));
> > > > + int ret;
> > > > +
and if I need to call kobject_put at the end. If for some reason
the files have failed to be initialized, I would have an
unbalanced put and a warning would be printed.
I'll summarize in pseudo code:
intel_gt_sysfs_register()
{
kobject_init_and_add(sysfs_root...); /* which calls kobject_get() inside */
if (fails)
kobject_put(sysfs_root); /* reference goes to '0' */
}
intel_gt_sysfs_unregister()
{
option1: I don't call kobject_put(), I have an unbalanced
situation as you reviewed in patch 1.
option2: I call kobject_put(), if it did fail during init
there is an unbalanced situation, which is
handled but an annoying WARN() is issued.
option3: I check if "state_initialized" which I suppose
has been properly initialised during declaration
(maybe too paranoic?) and call _put()
accordingly
}
Thanks,
Andi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
