On Fri, Apr 17, 2020 at 9:08 AM Randy Dunlap rdunlap@infradead.org wrote:
On 4/17/20 4:39 AM, Heikki Krogerus wrote:
Hi,
An alternative might be to define something like __kobject_del() doing everything that kobject_del() does *without* the kobject_put(kobj->parent).
Then, kobject_del() could be defined as something like (pseudocode):
kobject_del(kobj) { kobject *perent = kobj->parent;
__kobject_del(kobj); kobject_put(parent);}
and kobject_cleanup() could call __kobject_del() instead of kobject_del() and then do the last kobject_put(parent) when it is done with the child.
Would that work?
I think so. Greg, what do you think?
Hm, maybe. Can someone test it out with the reproducer?
Brendan, or Randy! Can you guys test Rafael's proposal? I think it would look like this:
patch is whitespace-damaged. did you copy-paste it from a screen?
Anyway, it works for me. I loaded & unloaded test_printf.ko 5 times without a problem.
I just tried it as well. I also noticed some whitespace corruption, but it otherwise worked against the KUnit reproducer I wrote:
https://lore.kernel.org/patchwork/patch/1224002/
Thanks!
Thanks.
diff --git a/lib/kobject.c b/lib/kobject.c index 83198cb37d8d..2bd631460e18 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -599,14 +599,7 @@ int kobject_move(struct kobject *kobj, struct kobject *new_parent) } EXPORT_SYMBOL_GPL(kobject_move);
-/**
- kobject_del() - Unlink kobject from hierarchy.
- @kobj: object.
- This is the function that should be called to delete an object
- successfully added via kobject_add().
- */
-void kobject_del(struct kobject *kobj) +static void __kobject_del(struct kobject *kobj) { struct kernfs_node *sd; const struct kobj_type *ktype; @@ -625,9 +618,23 @@ void kobject_del(struct kobject *kobj)
kobj->state_in_sysfs = 0; kobj_kset_leave(kobj);
kobject_put(kobj->parent); kobj->parent = NULL;}
+/**
- kobject_del() - Unlink kobject from hierarchy.
- @kobj: object.
- This is the function that should be called to delete an object
- successfully added via kobject_add().
- */
+void kobject_del(struct kobject *kobj) +{
struct kobject *parent = kobj->parent;__kobject_del(kobj);kobject_put(parent);+} EXPORT_SYMBOL(kobject_del);
/** @@ -663,6 +670,7 @@ EXPORT_SYMBOL(kobject_get_unless_zero); */ static void kobject_cleanup(struct kobject *kobj) {
struct kobject *parent = kobj->parent; struct kobj_type *t = get_ktype(kobj); const char *name = kobj->name;@@ -684,7 +692,7 @@ static void kobject_cleanup(struct kobject *kobj) if (kobj->state_in_sysfs) { pr_debug("kobject: '%s' (%p): auto cleanup kobject_del\n", kobject_name(kobj), kobj);
kobject_del(kobj);
__kobject_del(kobj); } if (t && t->release) {@@ -698,6 +706,8 @@ static void kobject_cleanup(struct kobject *kobj) pr_debug("kobject: '%s': free name\n", name); kfree_const(name); }
kobject_put(parent);}
#ifdef CONFIG_DEBUG_KOBJECT_RELEASE
thanks,
-- ~Randy Reported-by: Randy Dunlap rdunlap@infradead.org