Hi Tvrtko,
> > > > + }
> > > > +
> > > > + intel_gt_sysfs_pm_remove(gt, root);
> > > > + kobject_put(root);
> > >
> > > Maybe stick to the same terminology regarding root and parent.
> >
> > yes.
> >
> > > Get/put on the parent looks unbalanced. Both register and unregister take a
> > > reference and only unregister releases it. But do you even need a reference?
> >
> > why? I take it here:
> >
> > static inline struct kobject *gt_to_parent_obj(struct intel_gt *gt)
> > {
> > return kobject_get(>->i915->drm.primary->kdev->kobj);
> > }
> >
> > at the beginning (when the driver is loaded) and I release it at
> > the end (when the driver is unloaded). Am I not seeing something?
>
> Gt_to_parent_obj at the top of intel_gt_sysfs_register balances out with the
> put at the end of the same function. What balances out gt_to_parent_obj from
> intel_gt_sysfs_register?
And... you are right!
either nothing or too many :)
> > > I am also tempted by the _once alternative, but then it makes less sense to
> > > include name & pid.
> >
> > It's true, it can be an unrelenting message, and I thought of it,
> > but if the user is resilient at reading out from the wrong
> > directory, why shouldn't I :)
>
> Because we always try to avoid emitting spammy logs when they can be easily
> triggered by userspace. Can we do rate limit? I think that could work well
> with logging the process name & pid.
yes, if two people suggested the same thing, most probably that's
the right thing to do.
> Also, we need an entry in Documentation/ABI/obsolete/.
I was waiting this patch to get in shape before adding the
interface to obsolete.
I will include it in the next patch.
Thanks a lot for the review,
Andi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
