On 25/02/2020 01:35, Andi Shyti wrote:
+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
}
Yes you are right, I confused the two parents again. :I
Okay then, is the extra kobject_get/put on the parent
(kobject_get(gt_get_parent_obj(gt) - this one) needed?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
