lkml.org 
[lkml]   [2004]   [Mar]   [27]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From(Frank A. Uepping)
SubjectRe: struct device::release issue
DateSat, 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
\
 
 \ /
  Last update: 2005-03-22 13:02    [from the cache]
©2003-2008