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>

diff --git a/Documentation/kobject.txt b/Documentation/kobject.txt
index fc9485d79061874b3f4cc37716a3e3a03ccdabde..ff4c25098119c572d2b590e514342cbdee791f22 100644 (file)
--- a/Documentation/kobject.txt
+++ b/Documentation/kobject.txt
@@ -279,10 +279,14 @@ such a method has a form like::
 One important point cannot be overstated: every kobject must have a
 release() method, and the kobject must persist (in a consistent state)
 until that method is called. If these constraints are not met, the code is
-flawed.  Note that the kernel will warn you if you forget to provide a
+flawed. Note that the kernel will warn you if you forget to provide a
 release() method.  Do not try to get rid of this warning by providing an
-"empty" release function; you will be mocked mercilessly by the kobject
-maintainer if you attempt this.
+"empty" release function.
+
+If all your cleanup function needs to do is call kfree(), then you must
+create a wrapper function which uses container_of() to upcast to the correct
+type (as shown in the example above) and then calls kfree() on the overall
+structure.
 
 Note, the name of the kobject is available in the release function, but it
 must 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 ed145fbfeddf06dcee5b833c0558d35c7fb6325d..e2285059161deb9f010badd96115e89fbbb47f79 100644 (file)
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -897,8 +897,7 @@ static void device_release(struct kobject *kobj)
        else if (dev->class && dev->class->dev_release)
                dev->class->dev_release(dev);
        else
-               WARN(1, KERN_ERR "Device '%s' does not have a release() "
-                       "function, it is broken and must be fixed.\n",
+               WARN(1, KERN_ERR "Device '%s' does not have a release() function, it is broken and must be fixed. See Documentation/kobject.txt.\n",
                        dev_name(dev));
        kfree(p);
 }

diff --git a/include/linux/kref.h b/include/linux/kref.h
index 29220724bf1cb43df4cc6840f4016fe7ba30d54c..cb00a026806197171d8bc9980b810c8fb0dc1cbd 100644 (file)
--- a/include/linux/kref.h
+++ b/include/linux/kref.h
@@ -53,10 +53,7 @@ static inline void kref_get(struct kref *kref)
  * @release: pointer to the function that will clean up the object when the
  *          last reference to the object is released.
  *          This pointer is required, and it is not acceptable to pass kfree
- *          in as this function.  If the caller does pass kfree to this
- *          function, you will be publicly mocked mercilessly by the kref
- *          maintainer, and anyone else who happens to notice it.  You have
- *          been warned.
+ *          in as this function.
  *
  * Decrement the refcount, and if 0, call release().
  * Return 1 if the object was removed, otherwise return 0.  Beware, if this

diff --git a/lib/kobject.c b/lib/kobject.c
index 97d86dc17c42bf09c94dcb5149ff669a7a6d49db..b72e00fd7d09e3d73747ea15e64d921b89f5109d 100644 (file)
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -639,7 +639,7 @@ static void kobject_cleanup(struct kobject *kobj)
                 kobject_name(kobj), kobj, __func__, kobj->parent);
 
        if (t && !t->release)
-               pr_debug("kobject: '%s' (%p): does not have a release() function, it is broken and must be fixed.\n",
+               pr_debug("kobject: '%s' (%p): does not have a release() function, it is broken and must be fixed. See Documentation/kobject.txt.\n",
                         kobject_name(kobj), kobj);
 
        /* send "remove" if the caller did not do it but sent "add" */