[PATCH] correct attribute_container list usage

[PATCH] correct attribute_container list usage

[James Bottomley]

One of the changes in the attribute_container code in the scsi-misc tree
was to add a lock to protect the list of devices per container.  This,
unfortunately, leads to potential scheduling while atomic problems if
there's a sleep in the function called by a trigger.

The correct solution is to use the kernel klist infrastructure instead
which allows lockless traversal of a list.

While implementing this patch, some bugs were found in klist, so the
attached patch depends on this patch:

http://marc.theaimsgroup.com/?l=linux-kernel&m=112445733108094

James

diff --git a/drivers/base/attribute_container.c b/drivers/base/attribute_container.c
--- a/drivers/base/attribute_container.c
+++ b/drivers/base/attribute_container.c
@@ -22,7 +22,7 @@
 /* This is a private structure used to tie the classdev and the
  * container .. it should never be visible outside this file */
 struct internal_container {
-	struct list_head node;
+	struct klist_node node;
 	struct attribute_container *cont;
 	struct class_device classdev;
 };
@@ -57,8 +57,7 @@ int
 attribute_container_register(struct attribute_container *cont)
 {
 	INIT_LIST_HEAD(&cont->node);
-	INIT_LIST_HEAD(&cont->containers);
-	spin_lock_init(&cont->containers_lock);
+	klist_init(&cont->containers);
 		
 	down(&attribute_container_mutex);
 	list_add_tail(&cont->node, &attribute_container_list);
@@ -78,13 +77,13 @@ attribute_container_unregister(struct at
 {
 	int retval = -EBUSY;
 	down(&attribute_container_mutex);
-	spin_lock(&cont->containers_lock);
-	if (!list_empty(&cont->containers))
+	spin_lock(&cont->containers.k_lock);
+	if (!list_empty(&cont->containers.k_list))
 		goto out;
 	retval = 0;
 	list_del(&cont->node);
  out:
-	spin_unlock(&cont->containers_lock);
+	spin_unlock(&cont->containers.k_lock);
 	up(&attribute_container_mutex);
 	return retval;
 		
@@ -143,7 +142,6 @@ attribute_container_add_device(struct de
 			continue;
 		}
 		memset(ic, 0, sizeof(struct internal_container));
-		INIT_LIST_HEAD(&ic->node);
 		ic->cont = cont;
 		class_device_initialize(&ic->classdev);
 		ic->classdev.dev = get_device(dev);
@@ -154,13 +152,22 @@ attribute_container_add_device(struct de
 			fn(cont, dev, &ic->classdev);
 		else
 			attribute_container_add_class_device(&ic->classdev);
-		spin_lock(&cont->containers_lock);
-		list_add_tail(&ic->node, &cont->containers);
-		spin_unlock(&cont->containers_lock);
+		klist_add_tail(&ic->node, &cont->containers);
 	}
 	up(&attribute_container_mutex);
 }
 
+/* FIXME: can't break out of this unless klist_iter_exit is also
+ * called before doing the break
+ */
+#define klist_for_each_entry(pos, head, member, iter) \
+	for (klist_iter_init(head, iter); (pos = ({ \
+		struct klist_node *n = klist_next(iter); \
+		n ? ({ klist_iter_exit(iter) ; NULL; }) : \
+			container_of(n, typeof(*pos), member);\
+	}) ) != NULL; )
+			
+
 /**
  * attribute_container_remove_device - make device eligible for removal.
  *
@@ -187,18 +194,19 @@ attribute_container_remove_device(struct
 
 	down(&attribute_container_mutex);
 	list_for_each_entry(cont, &attribute_container_list, node) {
-		struct internal_container *ic, *tmp;
+		struct internal_container *ic;
+		struct klist_iter iter;
 
 		if (attribute_container_no_classdevs(cont))
 			continue;
 
 		if (!cont->match(cont, dev))
 			continue;
-		spin_lock(&cont->containers_lock);
-		list_for_each_entry_safe(ic, tmp, &cont->containers, node) {
+
+		klist_for_each_entry(ic, &cont->containers, node, &iter) {
 			if (dev != ic->classdev.dev)
 				continue;
-			list_del(&ic->node);
+			klist_remove(&ic->node);
 			if (fn)
 				fn(cont, dev, &ic->classdev);
 			else {
@@ -206,7 +214,6 @@ attribute_container_remove_device(struct
 				class_device_unregister(&ic->classdev);
 			}
 		}
-		spin_unlock(&cont->containers_lock);
 	}
 	up(&attribute_container_mutex);
 }
@@ -232,7 +239,8 @@ attribute_container_device_trigger(struc
 
 	down(&attribute_container_mutex);
 	list_for_each_entry(cont, &attribute_container_list, node) {
-		struct internal_container *ic, *tmp;
+		struct internal_container *ic;
+		struct klist_iter iter;
 
 		if (!cont->match(cont, dev))
 			continue;
@@ -242,12 +250,10 @@ attribute_container_device_trigger(struc
 			continue;
 		}
 
-		spin_lock(&cont->containers_lock);
-		list_for_each_entry_safe(ic, tmp, &cont->containers, node) {
+		klist_for_each_entry(ic, &cont->containers, node, &iter) {
 			if (dev == ic->classdev.dev)
 				fn(cont, dev, &ic->classdev);
 		}
-		spin_unlock(&cont->containers_lock);
 	}
 	up(&attribute_container_mutex);
 }
@@ -397,15 +403,16 @@ attribute_container_find_class_device(st
 {
 	struct class_device *cdev = NULL;
 	struct internal_container *ic;
+	struct klist_iter iter;
 
-	spin_lock(&cont->containers_lock);
-	list_for_each_entry(ic, &cont->containers, node) {
+	klist_for_each_entry(ic, &cont->containers, node, &iter) {
 		if (ic->classdev.dev == dev) {
 			cdev = &ic->classdev;
+			/* FIXME: must exit iterator then break */
+			klist_iter_exit(&iter);
 			break;
 		}
 	}
-	spin_unlock(&cont->containers_lock);
 
 	return cdev;
 }
diff --git a/include/linux/attribute_container.h b/include/linux/attribute_container.h
--- a/include/linux/attribute_container.h
+++ b/include/linux/attribute_container.h
@@ -11,12 +11,12 @@
 
 #include <linux/device.h>
 #include <linux/list.h>
+#include <linux/klist.h>
 #include <linux/spinlock.h>
 
 struct attribute_container {
 	struct list_head	node;
-	struct list_head	containers;
-	spinlock_t		containers_lock;
+	struct klist		containers;
 	struct class		*class;
 	struct class_device_attribute **attrs;
 	int (*match)(struct attribute_container *, struct device *);

Re: [PATCH] correct attribute_container list usage

[Patrick Mansfield]

On Mon, Aug 22, 2005 at 10:06:19AM -0500, James Bottomley wrote:

> +/* FIXME: can't break out of this unless klist_iter_exit is also
> + * called before doing the break
> + */
> +#define klist_for_each_entry(pos, head, member, iter) \
> +	for (klist_iter_init(head, iter); (pos = ({ \
> +		struct klist_node *n = klist_next(iter); \
> +		n ? ({ klist_iter_exit(iter) ; NULL; }) : \
> +			container_of(n, typeof(*pos), member);\
> +	}) ) != NULL; )
> +			
> +

> -		spin_lock(&cont->containers_lock);
> -		list_for_each_entry_safe(ic, tmp, &cont->containers, node) {
> +
> +		klist_for_each_entry(ic, &cont->containers, node, &iter) {
>  			if (dev != ic->classdev.dev)
>  				continue;
> -			list_del(&ic->node);
> +			klist_remove(&ic->node);
>  			if (fn)
>  				fn(cont, dev, &ic->classdev);

Did you test with CONFIG_DEBUG_SLAB enabled?

I have a workaround for problems with device_for_each_child() not being
"safe", I'm trying to verify it right now, but the underlying problem is
in klist_next(), I don't have a general solution for it (it looks hard to
fix).

-- Patrick Mansfield

Re: [PATCH] correct attribute_container list usage

[James Bottomley]

On Mon, 2005-08-22 at 14:46 -0700, Patrick Mansfield wrote:
> Did you test with CONFIG_DEBUG_SLAB enabled?

Yes, but only on an ia64.

> I have a workaround for problems with device_for_each_child() not being
> "safe", I'm trying to verify it right now, but the underlying problem is
> in klist_next(), I don't have a general solution for it (it looks hard to
> fix).

Could you elaborate ... the principle (hold refs to the node until
you've extracted the next pointer) looks sound to me, even in the face
of deletion.

James

Re: [PATCH] correct attribute_container list usage

[Patrick Mansfield]

On Mon, Aug 22, 2005 at 04:59:16PM -0500, James Bottomley wrote:
> On Mon, 2005-08-22 at 14:46 -0700, Patrick Mansfield wrote:
> > Did you test with CONFIG_DEBUG_SLAB enabled?
> 
> Yes, but only on an ia64.
> 
> > I have a workaround for problems with device_for_each_child() not being
> > "safe", I'm trying to verify it right now, but the underlying problem is
> > in klist_next(), I don't have a general solution for it (it looks hard to
> > fix).
> 
> Could you elaborate ... the principle (hold refs to the node until
> you've extracted the next pointer) looks sound to me, even in the face
> of deletion.

[based on whatever was in current 2.6.x git tree a couple weeks ago.]

The klist is (effectively) embedded within the struct device.

The klist_next() gets and puts on the klist object, so when the struct
device ref (or kref) counts go to zero, we free up the klist independent
of its ref counts.

Attached is a test module, it oopsed for me with CONFIG_DEBUG_SLAB, on
ppc64. I was trying to complete testing of my hack (on current git tree,
rather than scsi-misc), but have been preempted by other work today.
The patch worked OK for rmmod qla2300.

You could modify it to test similar klist code.

Build and insmod, then test via:

echo 0 > /sys/module/dev_child/parameters/state
echo 1 > /sys/module/dev_child/parameters/state
echo 0 > /sys/module/dev_child/parameters/state
echo 2 > /sys/module/dev_child/parameters/state

0 creates the child, 1 removes it without using dev_for_each_child, 2
oopses when removing the device via dev_for_each_child.

Here is my hack workaround:

diff -uprN -X /home/patman/dontdiff scsi-misc-2.6.git/drivers/base/core.c dev-each-hack-scsi-misc-2.6/drivers/base/core.c
--- scsi-misc-2.6.git/drivers/base/core.c	2005-08-16 15:02:19.000000000 -0700
+++ dev-each-hack-scsi-misc-2.6/drivers/base/core.c	2005-08-22 12:14:27.000000000 -0700
@@ -366,13 +366,6 @@ void device_unregister(struct device * d
 	put_device(dev);
 }
 
-
-static struct device * next_device(struct klist_iter * i)
-{
-	struct klist_node * n = klist_next(i);
-	return n ? container_of(n, struct device, knode_parent) : NULL;
-}
-
 /**
  *	device_for_each_child - device child iterator.
  *	@dev:	parent struct device.
@@ -389,12 +382,27 @@ int device_for_each_child(struct device 
 		     int (*fn)(struct device *, void *))
 {
 	struct klist_iter i;
-	struct device * child;
+	struct device * child, * prev;
 	int error = 0;
+	struct klist_node * n;
 
 	klist_iter_init(&parent->klist_children, &i);
-	while ((child = next_device(&i)) && !error)
-		error = fn(child, data);
+
+	prev = NULL;
+	do {
+		n = klist_next(&i);
+		if (prev)
+			put_device(prev);
+		child = n ? container_of(n, struct device, knode_parent) : NULL;
+		if (child) {
+			prev = child;
+			get_device(prev);
+			error = fn(child, data);
+			if (error)
+				put_device(prev);
+		}
+	} while (child && !error);
+
 	klist_iter_exit(&i);
 	return error;
 }
-- Patrick Mansfield

Re: [PATCH] correct attribute_container list usage

[Patrick Mansfield]

[-- Attachment #1: Type: text/plain, Size: 366 bytes --]

On Mon, Aug 22, 2005 at 03:14:27PM -0700, Patrick Mansfield wrote:

> Attached is a test module, it oopsed for me with CONFIG_DEBUG_SLAB, on
> ppc64. I was trying to complete testing of my hack (on current git tree,
> rather than scsi-misc), but have been preempted by other work today.
> The patch worked OK for rmmod qla2300.

OK ... really attached it this time.

[-- Attachment #2: dev_child.c --]
[-- Type: text/plain, Size: 5031 bytes --]

#include <linux/config.h>
#include <linux/module.h>

#include <linux/kernel.h>
#include <linux/sched.h>
#include <linux/errno.h>
#include <linux/timer.h>
#include <linux/types.h>
#include <linux/string.h>
#include <linux/genhd.h>
#include <linux/fs.h>
#include <linux/init.h>
#include <linux/proc_fs.h>
#include <linux/moduleparam.h>

static int dev_child_state = 1;

MODULE_AUTHOR("Patrick Mansfield");
MODULE_DESCRIPTION("test device_for_each_child");
MODULE_LICENSE("GPL");

static struct device dev_child_main, *dev_child_child;

static void some_release(struct device *dev)
{
	printk(KERN_WARNING "enter %s to kfree %s\n", __FUNCTION__,
	       dev->bus_id);
	kfree(dev);
	printk(KERN_WARNING "exit  %s\n", __FUNCTION__);
}

/*
 * Add one device under dev_child_main.
 */
static void dev_child_create(void)
{
	struct device *dev;

	printk(KERN_WARNING "enter %s\n", __FUNCTION__);
	dev = kmalloc(sizeof (*dev), GFP_KERNEL);
	if (!dev) {
		printk(KERN_WARNING "%s can't get dev\n", __FUNCTION__);
		return;
	}
	memset(dev, 0, sizeof(*dev));
	dev->parent = &dev_child_main;
	dev->release = &some_release;
	sprintf(dev->bus_id, "child-1");
	device_register(dev);
	dev_child_child = dev;
	printk(KERN_WARNING "exit  %s, added %s\n", __FUNCTION__,
	       dev->bus_id);
}

/*
 * Remove dev_child_child.
 */
static void dev_child_destroy_ok(void)
{
	printk(KERN_WARNING "enter %s\n", __FUNCTION__);
	if (dev_child_child) {
		get_device(&dev_child_main);
		device_unregister(dev_child_child);
		dev_child_child = NULL;
		put_device(&dev_child_main);
	}
	printk(KERN_WARNING "exit  %s\n", __FUNCTION__);
}

static int remove_child(struct device *dev, void *data)
{
	printk(KERN_WARNING "enter %s\n", __FUNCTION__);
	device_unregister(dev);
	printk(KERN_WARNING "exit  %s\n", __FUNCTION__);
	return 0;
}

/*
 * Remove all children of dev_child_main
 */
static void dev_child_destroy_fails(void)
{
	printk(KERN_WARNING "enter %s\n", __FUNCTION__);
	get_device(&dev_child_main);
	device_for_each_child(&dev_child_main, NULL, remove_child);
	put_device(&dev_child_main);
	dev_child_child = NULL;
	printk(KERN_WARNING "exit  %s\n", __FUNCTION__);
}

static int dev_child_set_state(const char *val, struct kernel_param *kp)
{
	int new_state;

	switch(val[0]) {
	case '0':
		new_state = 0;
		break;
	case '1':
		new_state = 1;
		break;
	case '2':
		new_state = 2;
		break;
	default:
		return -EINVAL;
	}

	if (new_state != dev_child_state) {
		dev_child_state = new_state;
		if (dev_child_state == 0)
			dev_child_create();
		else if (dev_child_state == 1)
			dev_child_destroy_ok();
		else /* 2 */
			dev_child_destroy_fails();
		return 0;
	} else
		return -E2BIG;

	return 0;
}

module_param_call(state, dev_child_set_state, param_get_int,
		                    &dev_child_state, 0644);

static void dev_child_0_release(struct device * dev)
{
}

static struct device dev_child_primary = {
	.bus_id		= "pseudo-dev_child",
	.release	= dev_child_0_release,
};

static int dev_child_bus_match(struct device *dev,
                          struct device_driver *dev_driver)
{
        return 1;
}

static struct bus_type dev_child_bus = {
        .name = "dev_child_bus",
        .match = dev_child_bus_match,
};

static int dev_child_probe(struct device *dev)
{
	int error = 0;

	return error;
}

static int dev_child_remove(struct device *dev)
{
	int error = 0;

	dev_child_destroy_ok();
	return error;
}

static struct device_driver dev_child_sys_driver = {
	.name           = "dev_child",
	.bus            = &dev_child_bus,
	.probe          = dev_child_probe,
	.remove         = dev_child_remove,
};

static void dev_child_release(struct device *dev)
{
}

static void add_main(void)
{
	struct device *dev = &dev_child_main;
	int error;

	/* add one device to our pseudo bus */
	device_initialize(dev);
	dev->bus = &dev_child_bus;
	dev->parent = &dev_child_primary;
	dev->release = &dev_child_release;
	strcpy(dev->bus_id, "dev_child_main");

	error = device_register(dev);
	if (error)
		printk(KERN_WARNING "%s device_register failed %d\n",
		       __FUNCTION__, error);
}

static void remove_main(void)
{
	device_unregister(&dev_child_main);
}

static int __init dev_child_init(void)
{
	int err;

	err = device_register(&dev_child_primary);
	if (err) {
		printk(KERN_WARNING "%s: device_register failed %d\n",
		      __FUNCTION__, err);
		goto fail1;
	}
	err = bus_register(&dev_child_bus);
	if (err) {
		printk(KERN_WARNING "%s: bus_register failed %d\n",
		      __FUNCTION__, err);
		goto fail2;
	}
	err = driver_register(&dev_child_sys_driver);
	if (err) {
		printk(KERN_WARNING "%s: driver_register failed %d\n",
		      __FUNCTION__, err);
		goto fail3;
	}

	add_main();
	return 0;

fail3:
	bus_unregister(&dev_child_bus);
fail2:
	device_unregister(&dev_child_primary);
fail1:
	return err;
}

static void __exit dev_child_exit(void)
{
	remove_main();
	driver_unregister(&dev_child_sys_driver);
	bus_unregister(&dev_child_bus);
	device_unregister(&dev_child_primary);
}

device_initcall(dev_child_init);
module_exit(dev_child_exit);

Re: [PATCH] correct attribute_container list usage

[James Bottomley]

On Mon, 2005-08-22 at 15:14 -0700, Patrick Mansfield wrote:
> based on whatever was in current 2.6.x git tree a couple weeks ago.]
> 
> The klist is (effectively) embedded within the struct device.
> 
> The klist_next() gets and puts on the klist object, so when the struct
> device ref (or kref) counts go to zero, we free up the klist independent
> of its ref counts.

Aarg, yet another object lifetime problem.

That somewhat lessens klist's utility, I think.  To fix this, the node
kref and the object that embeds it have to be tied together somehow.

One apparent, but rather nasty, solution would be to embed object get
and put into the klist head as functions that take the node, so
klist_next would take the object reference as well as the list kref,
then drop it on klist_release.

James

Re: [PATCH] correct attribute_container list usage

[James Bottomley]

On Mon, 2005-08-22 at 17:47 -0500, James Bottomley wrote:
> One apparent, but rather nasty, solution would be to embed object get
> and put into the klist head as functions that take the node, so
> klist_next would take the object reference as well as the list kref,
> then drop it on klist_release.

Well, I'm not enormously fond of this, but it's not as downright nasty
as I thought.  Patrick, could you try this (assuming you have a fast
machine ... I'll be done with the complete kernel rebuild that touching
klist.h requires eventually) you'll have to encode klist to device get
and put functions and feed them to klist_init_embedded().

James

diff --git a/include/linux/klist.h b/include/linux/klist.h
--- a/include/linux/klist.h
+++ b/include/linux/klist.h
@@ -14,14 +14,24 @@
 #include <linux/kref.h>
 #include <linux/list.h>
 
-
+struct klist_node;
 struct klist {
 	spinlock_t		k_lock;
 	struct list_head	k_list;
+	void			(*get)(struct klist_node *);
+	void			(*put)(struct klist_node *);
 };
 
 
-extern void klist_init(struct klist * k);
+extern void klist_init_embedded(struct klist * k,
+				void (*get)(struct klist_node *),
+				void (*put)(struct klist_node *));
+
+static inline void klist_init(struct klist * k)
+{
+	klist_init_embedded(k, NULL, NULL);
+}
+
 
 
 struct klist_node {
diff --git a/lib/klist.c b/lib/klist.c
--- a/lib/klist.c
+++ b/lib/klist.c
@@ -42,15 +42,26 @@
 /**
  *	klist_init - Initialize a klist structure. 
  *	@k:	The klist we're initializing.
+ *	@get:	The get function for the embedding object (NULL if none)
+ *	@put:	The put function for the embedding object (NULL if none)
+ *
+ * Initialises the klist structure.  If the klist_node structures are
+ * going to be embedded in refcounted objects (necessary for safe
+ * deletion) then the get/put arguments are used to initialise
+ * functions that take and release references on the embedding
+ * objects.
  */
 
-void klist_init(struct klist * k)
+void klist_init_embedded(struct klist * k, void (*get)(struct klist_node *),
+			 void (*put)(struct klist_node *))
 {
 	INIT_LIST_HEAD(&k->k_list);
 	spin_lock_init(&k->k_lock);
+	k->get = get;
+	k->put = put;
 }
 
-EXPORT_SYMBOL_GPL(klist_init);
+EXPORT_SYMBOL_GPL(klist_init_embedded);
 
 
 static void add_head(struct klist * k, struct klist_node * n)
@@ -74,6 +85,8 @@ static void klist_node_init(struct klist
 	init_completion(&n->n_removed);
 	kref_init(&n->n_ref);
 	n->n_klist = k;
+	if (k->get)
+		k->get(n);
 }
 
 
@@ -110,9 +123,12 @@ EXPORT_SYMBOL_GPL(klist_add_tail);
 static void klist_release(struct kref * kref)
 {
 	struct klist_node * n = container_of(kref, struct klist_node, n_ref);
+	void (*put)(struct klist_node *) = n->n_klist->put;
 	list_del(&n->n_node);
 	complete(&n->n_removed);
 	n->n_klist = NULL;
+	if (put)
+		put(n);
 }
 
 static int klist_dec_and_del(struct klist_node * n)

Re: [PATCH] correct attribute_container list usage

[Patrick Mansfield]

On Mon, Aug 22, 2005 at 06:26:25PM -0500, James Bottomley wrote:
> On Mon, 2005-08-22 at 17:47 -0500, James Bottomley wrote:
> > One apparent, but rather nasty, solution would be to embed object get
> > and put into the klist head as functions that take the node, so
> > klist_next would take the object reference as well as the list kref,
> > then drop it on klist_release.
> 
> Well, I'm not enormously fond of this, but it's not as downright nasty
> as I thought.  Patrick, could you try this (assuming you have a fast
> machine ... I'll be done with the complete kernel rebuild that touching
> klist.h requires eventually) you'll have to encode klist to device get
> and put functions and feed them to klist_init_embedded().

But, we have to pass in a struct kref, to affect put/get_device, correct?

While klist (and your patch) need klist_node. These pieces:

> +	void			(*get)(struct klist_node *);
> +	void			(*put)(struct klist_node *);

> +void klist_init_embedded(struct klist * k, void (*get)(struct klist_node *),
> +			 void (*put)(struct klist_node *))
>  {
>  	INIT_LIST_HEAD(&k->k_list);
>  	spin_lock_init(&k->k_lock);
> +	k->get = get;
> +	k->put = put;
>  }

In include/linux/device.h we have:

struct device {

	...
	struct kobject kobj;
	...
}

And get/put_device are:

struct kobject * kobject_get(struct kobject * kobj)
{
        if (kobj)
                kref_get(&kobj->kref);
        return kobj;
}

And then:

void kref_get(struct kref *kref)
{
        WARN_ON(!atomic_read(&kref->refcount));
        atomic_inc(&kref->refcount);
}

-- Patrick Mansfield

Re: [PATCH] correct attribute_container list usage

[James Bottomley]

On Mon, 2005-08-22 at 17:39 -0700, Patrick Mansfield wrote:
> But, we have to pass in a struct kref, to affect put/get_device, correct?

Correct.  Let me post the code mods to drivers/base/core.c so you can
see how it works.

Incidentally, it now passes your dev_child test without causing a slab
corruption.

James

diff --git a/drivers/base/core.c b/drivers/base/core.c
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -191,6 +191,20 @@ void device_remove_file(struct device * 
 	}
 }
 
+static void klist_children_get(struct klist_node *n)
+{
+	struct device *dev = container_of(n, struct device, knode_parent);
+
+	get_device(dev);
+}
+
+static void klist_children_put(struct klist_node *n)
+{
+	struct device *dev = container_of(n, struct device, knode_parent);
+
+	put_device(dev);
+}
+
 
 /**
  *	device_initialize - init device structure.
@@ -207,7 +221,8 @@ void device_initialize(struct device *de
 {
 	kobj_set_kset_s(dev, devices_subsys);
 	kobject_init(&dev->kobj);
-	klist_init(&dev->klist_children);
+	klist_init_embedded(&dev->klist_children, klist_children_get,
+			    klist_children_put);
 	INIT_LIST_HEAD(&dev->dma_pools);
 	init_MUTEX(&dev->sem);
 }

Re: [PATCH] correct attribute_container list usage

[Patrick Mansfield]

On Mon, Aug 22, 2005 at 09:03:03PM -0500, James Bottomley wrote:
> On Mon, 2005-08-22 at 17:39 -0700, Patrick Mansfield wrote:
> > But, we have to pass in a struct kref, to affect put/get_device, correct?
> 
> Correct.  Let me post the code mods to drivers/base/core.c so you can
> see how it works.

OK ... I thought that was what you intended but could not quite see it.

> Incidentally, it now passes your dev_child test without causing a slab
> corruption.

Great, tested with your two patches applied on recent 2.6.13-rc6-git tree,
and modprobe -r qla2300 now works fine.

-- Patrick Mansfield