Messages in this thread Patch in this message | | | From | (Frank A. Uepping) | Subject | Re: struct device::release issue | Date | Sat, 27 Mar 2004 15:26:25 +0100 |
| |
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 #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); obj-m += device_add_bug.o KDIR := /lib/modules/$(shell uname -r)/build PWD := $(shell pwd)
default: $(MAKE) -C $(KDIR) SUBDIRS="$(PWD)" modules
| |