summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorEzequiel Garcia <ezequiel@collabora.com>2018-12-03 11:44:35 -0500
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2018-12-06 07:57:03 -0500
commit186bddb28ff9f61250d1b33554321d0bf5d085f6 (patch)
tree3bd5f8a73b4932fdddce73a9f628f055e7496182
parent3f8e9178538189215b59f726f2449a08362e7074 (diff)
kref/kobject: Improve documentation
The current kref and kobject documentation may be insufficient to understand these common pitfalls regarding object lifetime and object releasing. Add a bit more documentation and improve the warnings seen by the user, pointing to the right piece of documentation. Also, it's important to understand that making fun of people publicly is not at all helpful, doesn't provide any value, and it's not a healthy way of encouraging developers to do better. "Mocking mercilessly" will, if anything, make developers feel bad and go away. This kind of behavior should not be encouraged or justified. Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> Signed-off-by: Matthias Brugger <mbrugger@suse.com> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Acked-by: Guenter Roeck <linux@roeck-us.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r--Documentation/kobject.txt10
-rw-r--r--drivers/base/core.c3
-rw-r--r--include/linux/kref.h5
-rw-r--r--lib/kobject.c2
4 files changed, 10 insertions, 10 deletions
diff --git a/Documentation/kobject.txt b/Documentation/kobject.txt
index fc9485d79061..ff4c25098119 100644
--- a/Documentation/kobject.txt
+++ b/Documentation/kobject.txt
@@ -279,10 +279,14 @@ such a method has a form like::
279One important point cannot be overstated: every kobject must have a 279One important point cannot be overstated: every kobject must have a
280release() method, and the kobject must persist (in a consistent state) 280release() method, and the kobject must persist (in a consistent state)
281until that method is called. If these constraints are not met, the code is 281until that method is called. If these constraints are not met, the code is
282flawed. Note that the kernel will warn you if you forget to provide a 282flawed. Note that the kernel will warn you if you forget to provide a
283release() method. Do not try to get rid of this warning by providing an 283release() method. Do not try to get rid of this warning by providing an
284"empty" release function; you will be mocked mercilessly by the kobject 284"empty" release function.
285maintainer if you attempt this. 285
286If all your cleanup function needs to do is call kfree(), then you must
287create a wrapper function which uses container_of() to upcast to the correct
288type (as shown in the example above) and then calls kfree() on the overall
289structure.
286 290
287Note, the name of the kobject is available in the release function, but it 291Note, the name of the kobject is available in the release function, but it
288must NOT be changed within this callback. Otherwise there will be a memory 292must NOT be changed within this callback. Otherwise there will be a memory
diff --git a/drivers/base/core.c b/drivers/base/core.c
index ed145fbfeddf..e2285059161d 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -897,8 +897,7 @@ static void device_release(struct kobject *kobj)
897 else if (dev->class && dev->class->dev_release) 897 else if (dev->class && dev->class->dev_release)
898 dev->class->dev_release(dev); 898 dev->class->dev_release(dev);
899 else 899 else
900 WARN(1, KERN_ERR "Device '%s' does not have a release() " 900 WARN(1, KERN_ERR "Device '%s' does not have a release() function, it is broken and must be fixed. See Documentation/kobject.txt.\n",
901 "function, it is broken and must be fixed.\n",
902 dev_name(dev)); 901 dev_name(dev));
903 kfree(p); 902 kfree(p);
904} 903}
diff --git a/include/linux/kref.h b/include/linux/kref.h
index 29220724bf1c..cb00a0268061 100644
--- a/include/linux/kref.h
+++ b/include/linux/kref.h
@@ -53,10 +53,7 @@ static inline void kref_get(struct kref *kref)
53 * @release: pointer to the function that will clean up the object when the 53 * @release: pointer to the function that will clean up the object when the
54 * last reference to the object is released. 54 * last reference to the object is released.
55 * This pointer is required, and it is not acceptable to pass kfree 55 * This pointer is required, and it is not acceptable to pass kfree
56 * in as this function. If the caller does pass kfree to this 56 * in as this function.
57 * function, you will be publicly mocked mercilessly by the kref
58 * maintainer, and anyone else who happens to notice it. You have
59 * been warned.
60 * 57 *
61 * Decrement the refcount, and if 0, call release(). 58 * Decrement the refcount, and if 0, call release().
62 * Return 1 if the object was removed, otherwise return 0. Beware, if this 59 * Return 1 if the object was removed, otherwise return 0. Beware, if this
diff --git a/lib/kobject.c b/lib/kobject.c
index 97d86dc17c42..b72e00fd7d09 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -639,7 +639,7 @@ static void kobject_cleanup(struct kobject *kobj)
639 kobject_name(kobj), kobj, __func__, kobj->parent); 639 kobject_name(kobj), kobj, __func__, kobj->parent);
640 640
641 if (t && !t->release) 641 if (t && !t->release)
642 pr_debug("kobject: '%s' (%p): does not have a release() function, it is broken and must be fixed.\n", 642 pr_debug("kobject: '%s' (%p): does not have a release() function, it is broken and must be fixed. See Documentation/kobject.txt.\n",
643 kobject_name(kobj), kobj); 643 kobject_name(kobj), kobj);
644 644
645 /* send "remove" if the caller did not do it but sent "add" */ 645 /* send "remove" if the caller did not do it but sent "add" */