* Re: struct device::release issue
2004-03-27 1:14 ` Greg KH
@ 2004-03-27 14:26 ` Frank A. Uepping
2004-06-04 21:52 ` Greg KH
0 siblings, 1 reply; 4+ messages in thread
From: Frank A. Uepping @ 2004-03-27 14:26 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel, Patrick Mochel
[-- Attachment #1: Type: text/plain, Size: 1094 bytes --]
On Saturday 27 March 2004 02:14, Greg KH wrote:
> On Sat, Mar 06, 2004 at 12:47:24PM +0100, Frank A. Uepping wrote:
> > Hi,
> > if device_add fails (e.g. bus_add_device returns an error) then the release
> > method will be called for the device. Is this a bug or a feature?
>
> Are you sure this will happen? device_initialize() gets a reference
> that is still present after device_add() fails, right? So release()
> will not get called.
At the label PMError, kobject_unregister is called, which decrements the
recount by 2, which will result in calling release at label Done (put_device).
kobject_unregister should be superseded by kobject_del.
Here is a patch:
--- linux-2.6.4/drivers/base/core.c.orig Sat Mar 13 10:33:40 2004
+++ linux-2.6.4/drivers/base/core.c Sat Mar 27 15:04:27 2004
@@ -249,7 +249,7 @@
BusError:
device_pm_remove(dev);
PMError:
- kobject_unregister(&dev->kobj);
+ kobject_del(&dev->kobj);
Error:
if (parent)
put_device(parent);
I have attached a simple module, which demonstrates the behavior.
/FAU
[-- Attachment #2: device_add_bug.c --]
[-- Type: text/x-csrc, Size: 1839 bytes --]
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/device.h>
#include <../drivers/base/power/power.h>
#define DBG(format, arg...) do { printk(KERN_DEBUG "foo: %s(): " format "\n", __FUNCTION__ , ## arg); } while (0)
int device_add(struct device *dev) // Defined in drivers/base/core.c
{
struct device * parent;
int error;
DBG("1. refcount=%d", atomic_read(&dev->kobj.refcount));
dev = get_device(dev);
if (!dev || !strlen(dev->bus_id))
return -EINVAL;
DBG("2. refcount=%d", atomic_read(&dev->kobj.refcount));
parent = get_device(dev->parent);
pr_debug("DEV: registering device: ID = '%s'\n", dev->bus_id);
/* first, register with generic layer. */
kobject_set_name(&dev->kobj,dev->bus_id);
if (parent)
dev->kobj.parent = &parent->kobj;
if ((error = kobject_add(&dev->kobj)))
goto Error;
DBG("3. refcount=%d", atomic_read(&dev->kobj.refcount));
// e.g. device_pm_add failed
error = -EBUSY;
goto PMError;
/*
if ((error = device_pm_add(dev)))
goto PMError;
if ((error = bus_add_device(dev)))
goto BusError;
down_write(&devices_subsys.rwsem);
if (parent)
list_add_tail(&dev->node,&parent->children);
up_write(&devices_subsys.rwsem);
if (platform_notify)
platform_notify(dev);
*/
Done:
put_device(dev);
DBG("5. refcount=%d", atomic_read(&dev->kobj.refcount));
return error;
BusError:
device_pm_remove(dev);
PMError:
kobject_unregister(&dev->kobj);
DBG("4. refcount=%d", atomic_read(&dev->kobj.refcount));
Error:
if (parent)
put_device(parent);
goto Done;
}
void foo_release(struct device* dev)
{
DBG("XXX: called even if device_add fails?");
}
struct device foo_dev = {
.bus_id = "bus",
.release = foo_release
};
int __init foo_init(void)
{
device_initialize(&foo_dev);
return device_add(&foo_dev);
}
module_init(foo_init);
[-- Attachment #3: Makefile --]
[-- Type: text/x-makefile, Size: 146 bytes --]
obj-m += device_add_bug.o
KDIR := /lib/modules/$(shell uname -r)/build
PWD := $(shell pwd)
default:
$(MAKE) -C $(KDIR) SUBDIRS="$(PWD)" modules
^ permalink raw reply [flat|nested] 4+ messages in thread