[BK PATCH] Driver Core patches for 2.6.10-rc1

[BK PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

Hi,

Here are a few minot driver core changes for 2.6.10-rc1 that were laying
around and didn't make it in the last big batch of merges.  Most of
these have been in the -mm tree for a while now.

No, they don't include the backing store sysfs patches, I'll get to
those next week.  I'll be in the same building as Maneesh, so I know
he'll be hounding me about them... :)

Oh, and these patches will cause the wait_for_sysfs program in udev to
start spiting out a lot more warnings.  It's not a bug in the kernel,
it's a userspace issue.  I'll get that fixed up in the next udev
release.

Please pull from:
	bk://kernel.bkbits.net/gregkh/linux/driver-2.6

thanks,

greg k-h

p.s. I'll send these as patches in response to this email to lkml for
those who want to see them.


 drivers/base/bus.c                     |    4 +--
 drivers/base/class.c                   |    4 +--
 drivers/base/core.c                    |    2 -
 drivers/base/cpu.c                     |    2 +
 drivers/firmware/efivars.c             |    2 -
 drivers/pci/hotplug/pci_hotplug_core.c |    2 -
 fs/char_dev.c                          |    4 +--
 fs/sysfs/dir.c                         |    2 -
 include/linux/kobject_uevent.h         |    1 
 kernel/cpu.c                           |   35 ----------------------------
 kernel/module.c                        |    2 -
 lib/Kconfig.debug                      |    7 +++++
 lib/Makefile                           |    7 ++++-
 lib/kobject.c                          |    4 ---
 lib/kobject_uevent.c                   |   40 ++++++++++++++++++++-------------
 15 files changed, 53 insertions(+), 65 deletions(-)
-----

Andrew Morton:
  o kobject_uevent warning fix
  o kobject_hotplug: permit no hotplug_ops

Anil Keshavamurthy:
  o remove cpu_run_sbin_hotplug()

Greg Kroah-Hartman:
  o hotplug: prevent skips in sequence number from happening
  o kobject: add CONFIG_DEBUG_KOBJECT

Stephen Hemminger:
  o avoid problems with kobject_set_name and name with %
  o cdev: protect against buggy drivers

[PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2000.6.1, 2004/10/20 15:38:19-07:00, greg@kroah.com

kobject: add CONFIG_DEBUG_KOBJECT

Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 lib/Kconfig.debug |    7 +++++++
 lib/Makefile      |    6 +++++-
 lib/kobject.c     |    4 +---
 3 files changed, 13 insertions(+), 4 deletions(-)


diff -Nru a/lib/Kconfig.debug b/lib/Kconfig.debug
--- a/lib/Kconfig.debug	2004-10-22 16:00:44 -07:00
+++ b/lib/Kconfig.debug	2004-10-22 16:00:44 -07:00
@@ -64,6 +64,13 @@
 	  If you say Y here, various routines which may sleep will become very
 	  noisy if they are called with a spinlock held.
 
+config DEBUG_KOBJECT
+	bool "kobject debugging"
+	depends on DEBUG_KERNEL
+	help
+	  If you say Y here, some extra kobject debugging messages will be sent
+	  to the syslog. 
+
 config DEBUG_HIGHMEM
 	bool "Highmem debugging"
 	depends on DEBUG_KERNEL && HIGHMEM && (X86 || PPC32 || MIPS || SPARC32)
diff -Nru a/lib/Makefile b/lib/Makefile
--- a/lib/Makefile	2004-10-22 16:00:44 -07:00
+++ b/lib/Makefile	2004-10-22 16:00:44 -07:00
@@ -2,11 +2,15 @@
 # Makefile for some libs needed in the kernel.
 #
 
-
 lib-y := errno.o ctype.o string.o vsprintf.o cmdline.o \
 	 bust_spinlocks.o rbtree.o radix-tree.o dump_stack.o \
 	 kobject.o kref.o idr.o div64.o parser.o int_sqrt.o \
 	 bitmap.o extable.o kobject_uevent.o
+
+ifeq ($(CONFIG_DEBUG_KOBJECT),y)
+CFLAGS_kobject.o += -DDEBUG
+CFLAGS_kobject_uevent.o += -DDEBUG
+endif
 
 lib-$(CONFIG_RWSEM_GENERIC_SPINLOCK) += rwsem-spinlock.o
 lib-$(CONFIG_RWSEM_XCHGADD_ALGORITHM) += rwsem.o
diff -Nru a/lib/kobject.c b/lib/kobject.c
--- a/lib/kobject.c	2004-10-22 16:00:44 -07:00
+++ b/lib/kobject.c	2004-10-22 16:00:44 -07:00
@@ -10,8 +10,6 @@
  * about using the kobject interface.
  */
 
-#undef DEBUG
-
 #include <linux/kobject.h>
 #include <linux/string.h>
 #include <linux/module.h>
@@ -123,7 +121,7 @@
  */
 void kobject_init(struct kobject * kobj)
 {
- 	kref_init(&kobj->kref);
+	kref_init(&kobj->kref);
 	INIT_LIST_HEAD(&kobj->entry);
 	kobj->kset = kset_get(kobj->kset);
 }

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2020, 2004/10/22 14:29:19-07:00, akpm@osdl.org

[PATCH] kobject_hotplug: permit no hotplug_ops

Make kobject_hotplug() work even if the kobject's kset doesn't implement any
hotplug_ops.

Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 lib/kobject_uevent.c |   19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)


diff -Nru a/lib/kobject_uevent.c b/lib/kobject_uevent.c
--- a/lib/kobject_uevent.c	2004-10-22 16:00:04 -07:00
+++ b/lib/kobject_uevent.c	2004-10-22 16:00:04 -07:00
@@ -187,6 +187,8 @@
 	u64 seq;
 	struct kobject *top_kobj = kobj;
 	struct kset *kset;
+	static struct kset_hotplug_ops null_hotplug_ops;
+	struct kset_hotplug_ops *hotplug_ops = &null_hotplug_ops;
 
 	if (!top_kobj->kset && top_kobj->parent) {
 		do {
@@ -194,15 +196,18 @@
 		} while (!top_kobj->kset && top_kobj->parent);
 	}
 
-	if (top_kobj->kset && top_kobj->kset->hotplug_ops)
+	if (top_kobj->kset)
 		kset = top_kobj->kset;
 	else
 		return;
 
+	if (kset->hotplug_ops)
+		hotplug_ops = kset->hotplug_ops;
+
 	/* If the kset has a filter operation, call it.
 	   Skip the event, if the filter returns zero. */
-	if (kset->hotplug_ops->filter) {
-		if (!kset->hotplug_ops->filter(kset, kobj))
+	if (hotplug_ops->filter) {
+		if (!hotplug_ops->filter(kset, kobj))
 			return;
 	}
 
@@ -221,8 +226,8 @@
 	if (!buffer)
 		goto exit;
 
-	if (kset->hotplug_ops->name)
-		name = kset->hotplug_ops->name(kset, kobj);
+	if (hotplug_ops->name)
+		name = hotplug_ops->name(kset, kobj);
 	if (name == NULL)
 		name = kset->kobj.name;
 
@@ -256,9 +261,9 @@
 	envp [i++] = scratch;
 	scratch += sprintf(scratch, "SUBSYSTEM=%s", name) + 1;
 
-	if (kset->hotplug_ops->hotplug) {
+	if (hotplug_ops->hotplug) {
 		/* have the kset specific function add its stuff */
-		retval = kset->hotplug_ops->hotplug (kset, kobj,
+		retval = hotplug_ops->hotplug (kset, kobj,
 				  &envp[i], NUM_ENVP - i, scratch,
 				  BUFFER_SIZE - (scratch - buffer));
 		if (retval) {

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2018, 2004/10/22 13:55:29-07:00, shemminger@osdl.org

[PATCH] cdev: protect against buggy drivers

Here is a better fix (thanks Greg) that allows long names for character
device objects.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 fs/char_dev.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)


diff -Nru a/fs/char_dev.c b/fs/char_dev.c
--- a/fs/char_dev.c	2004-10-22 16:00:27 -07:00
+++ b/fs/char_dev.c	2004-10-22 16:00:27 -07:00
@@ -207,8 +207,8 @@
 
 	cdev->owner = fops->owner;
 	cdev->ops = fops;
-	strcpy(cdev->kobj.name, name);
-	for (s = strchr(cdev->kobj.name, '/'); s; s = strchr(s, '/'))
+	kobject_set_name(&cdev->kobj, "%s", name);
+	for (s = strchr(kobject_name(&cdev->kobj),'/'); s; s = strchr(s, '/'))
 		*s = '!';
 		
 	err = cdev_add(cdev, MKDEV(cd->major, 0), 256);

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2019, 2004/10/22 14:01:16-07:00, shemminger@osdl.org

[PATCH] avoid problems with kobject_set_name and name with %

kobject_set_name takes a printf style argument list. There are many
callers that pass only one string, if this string contained a '%' character
than bad things would happen.  The fix is simple.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 drivers/base/bus.c                     |    4 ++--
 drivers/base/class.c                   |    4 ++--
 drivers/base/core.c                    |    2 +-
 drivers/firmware/efivars.c             |    2 +-
 drivers/pci/hotplug/pci_hotplug_core.c |    2 +-
 fs/sysfs/dir.c                         |    2 +-
 kernel/module.c                        |    2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)


diff -Nru a/drivers/base/bus.c b/drivers/base/bus.c
--- a/drivers/base/bus.c	2004-10-22 16:00:16 -07:00
+++ b/drivers/base/bus.c	2004-10-22 16:00:16 -07:00
@@ -515,7 +515,7 @@
 
 	if (bus) {
 		pr_debug("bus %s: add driver %s\n", bus->name, drv->name);
-		error = kobject_set_name(&drv->kobj, drv->name);
+		error = kobject_set_name(&drv->kobj, "%s", drv->name);
 		if (error) {
 			put_bus(bus);
 			return error;
@@ -666,7 +666,7 @@
 {
 	int retval;
 
-	retval = kobject_set_name(&bus->subsys.kset.kobj, bus->name);
+	retval = kobject_set_name(&bus->subsys.kset.kobj, "%s", bus->name);
 	if (retval)
 		goto out;
 
diff -Nru a/drivers/base/class.c b/drivers/base/class.c
--- a/drivers/base/class.c	2004-10-22 16:00:16 -07:00
+++ b/drivers/base/class.c	2004-10-22 16:00:16 -07:00
@@ -139,7 +139,7 @@
 
 	INIT_LIST_HEAD(&cls->children);
 	INIT_LIST_HEAD(&cls->interfaces);
-	error = kobject_set_name(&cls->subsys.kset.kobj, cls->name);
+	error = kobject_set_name(&cls->subsys.kset.kobj, "%s", cls->name);
 	if (error)
 		return error;
 
@@ -368,7 +368,7 @@
 		 class_dev->class_id);
 
 	/* first, register with generic layer. */
-	kobject_set_name(&class_dev->kobj, class_dev->class_id);
+	kobject_set_name(&class_dev->kobj, "%s", class_dev->class_id);
 	if (parent)
 		class_dev->kobj.parent = &parent->subsys.kset.kobj;
 
diff -Nru a/drivers/base/core.c b/drivers/base/core.c
--- a/drivers/base/core.c	2004-10-22 16:00:16 -07:00
+++ b/drivers/base/core.c	2004-10-22 16:00:16 -07:00
@@ -221,7 +221,7 @@
 	pr_debug("DEV: registering device: ID = '%s'\n", dev->bus_id);
 
 	/* first, register with generic layer. */
-	kobject_set_name(&dev->kobj, dev->bus_id);
+	kobject_set_name(&dev->kobj, "%s", dev->bus_id);
 	if (parent)
 		dev->kobj.parent = &parent->kobj;
 
diff -Nru a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
--- a/drivers/firmware/efivars.c	2004-10-22 16:00:16 -07:00
+++ b/drivers/firmware/efivars.c	2004-10-22 16:00:16 -07:00
@@ -640,7 +640,7 @@
 	*(short_name + strlen(short_name)) = '-';
 	efi_guid_unparse(vendor_guid, short_name + strlen(short_name));
 
-	kobject_set_name(&new_efivar->kobj, short_name);
+	kobject_set_name(&new_efivar->kobj, "%s", short_name);
 	kobj_set_kset_s(new_efivar, vars_subsys);
 	kobject_register(&new_efivar->kobj);
 
diff -Nru a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
--- a/drivers/pci/hotplug/pci_hotplug_core.c	2004-10-22 16:00:16 -07:00
+++ b/drivers/pci/hotplug/pci_hotplug_core.c	2004-10-22 16:00:16 -07:00
@@ -568,7 +568,7 @@
 	if ((slot->info == NULL) || (slot->ops == NULL))
 		return -EINVAL;
 
-	kobject_set_name(&slot->kobj, slot->name);
+	kobject_set_name(&slot->kobj, "%s", slot->name);
 	kobj_set_kset_s(slot, pci_hotplug_slots_subsys);
 
 	/* this can fail if we have already registered a slot with the same name */
diff -Nru a/fs/sysfs/dir.c b/fs/sysfs/dir.c
--- a/fs/sysfs/dir.c	2004-10-22 16:00:16 -07:00
+++ b/fs/sysfs/dir.c	2004-10-22 16:00:16 -07:00
@@ -181,7 +181,7 @@
 	new_dentry = sysfs_get_dentry(parent, new_name);
 	if (!IS_ERR(new_dentry)) {
   		if (!new_dentry->d_inode) {
-			error = kobject_set_name(kobj,new_name);
+			error = kobject_set_name(kobj, "%s", new_name);
 			if (!error)
 				d_move(kobj->dentry, new_dentry);
 		}
diff -Nru a/kernel/module.c b/kernel/module.c
--- a/kernel/module.c	2004-10-22 16:00:16 -07:00
+++ b/kernel/module.c	2004-10-22 16:00:16 -07:00
@@ -1120,7 +1120,7 @@
 		return -ENOMEM;
 
 	memset(&mod->mkobj->kobj, 0, sizeof(mod->mkobj->kobj));
-	err = kobject_set_name(&mod->mkobj->kobj, mod->name);
+	err = kobject_set_name(&mod->mkobj->kobj, "%s", mod->name);
 	if (err)
 		goto out;
 	kobj_set_kset_s(mod->mkobj, module_subsys);

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2023, 2004/10/22 15:43:17-07:00, greg@kroah.com

hotplug: prevent skips in sequence number from happening

Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 lib/kobject_uevent.c |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)


diff -Nru a/lib/kobject_uevent.c b/lib/kobject_uevent.c
--- a/lib/kobject_uevent.c	2004-10-22 15:59:34 -07:00
+++ b/lib/kobject_uevent.c	2004-10-22 15:59:34 -07:00
@@ -255,13 +255,6 @@
 	envp [i++] = scratch;
 	scratch += sprintf (scratch, "DEVPATH=%s", kobj_path) + 1;
 
-	spin_lock(&sequence_lock);
-	seq = ++hotplug_seqnum;
-	spin_unlock(&sequence_lock);
-
-	envp [i++] = scratch;
-	scratch += sprintf(scratch, "SEQNUM=%lld", (long long)seq) + 1;
-
 	envp [i++] = scratch;
 	scratch += sprintf(scratch, "SUBSYSTEM=%s", name) + 1;
 
@@ -277,7 +270,15 @@
 		}
 	}
 
-	pr_debug ("%s: %s %s %s %s %s %s %s\n", __FUNCTION__, argv[0], argv[1],
+	spin_lock(&sequence_lock);
+	seq = ++hotplug_seqnum;
+	spin_unlock(&sequence_lock);
+
+	envp [i++] = scratch;
+	scratch += sprintf(scratch, "SEQNUM=%lld", (long long)seq) + 1;
+
+	pr_debug ("%s: %s %s seq=%lld %s %s %s %s %s\n",
+		  __FUNCTION__, argv[0], argv[1], (long long)seq,
 		  envp[0], envp[1], envp[2], envp[3], envp[4]);
 
 	send_uevent(action_string, kobj_path, buffer, scratch - buffer, GFP_KERNEL);

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2021, 2004/10/22 14:29:57-07:00, akpm@osdl.org

[PATCH] remove cpu_run_sbin_hotplug()

From: Keshavamurthy Anil S <anil.s.keshavamurthy@intel.com>

Remove cpu_run_sbin_hotplug() - use kobject_hotplug() instead.

Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 drivers/base/cpu.c             |    2 ++
 include/linux/kobject_uevent.h |    1 +
 kernel/cpu.c                   |   35 -----------------------------------
 lib/kobject_uevent.c           |    2 ++
 4 files changed, 5 insertions(+), 35 deletions(-)


diff -Nru a/drivers/base/cpu.c b/drivers/base/cpu.c
--- a/drivers/base/cpu.c	2004-10-22 15:59:52 -07:00
+++ b/drivers/base/cpu.c	2004-10-22 15:59:52 -07:00
@@ -32,6 +32,8 @@
 	switch (buf[0]) {
 	case '0':
 		ret = cpu_down(cpu->sysdev.id);
+		if (!ret)
+			kobject_hotplug(&dev->kobj, KOBJ_OFFLINE);
 		break;
 	case '1':
 		ret = cpu_up(cpu->sysdev.id);
diff -Nru a/include/linux/kobject_uevent.h b/include/linux/kobject_uevent.h
--- a/include/linux/kobject_uevent.h	2004-10-22 15:59:52 -07:00
+++ b/include/linux/kobject_uevent.h	2004-10-22 15:59:52 -07:00
@@ -22,6 +22,7 @@
 	KOBJ_CHANGE	= (__force kobject_action_t) 0x03,	/* a sysfs attribute file has changed */
 	KOBJ_MOUNT	= (__force kobject_action_t) 0x04,	/* mount event for block devices */
 	KOBJ_UMOUNT	= (__force kobject_action_t) 0x05,	/* umount event for block devices */
+	KOBJ_OFFLINE	= (__force kobject_action_t) 0x06,	/* offline event for hotplug devices */
 };
 
 
diff -Nru a/kernel/cpu.c b/kernel/cpu.c
--- a/kernel/cpu.c	2004-10-22 15:59:52 -07:00
+++ b/kernel/cpu.c	2004-10-22 15:59:52 -07:00
@@ -57,34 +57,6 @@
 	write_unlock_irq(&tasklist_lock);
 }
 
-/* Notify userspace when a cpu event occurs, by running '/sbin/hotplug
- * cpu' with certain environment variables set.  */
-static int cpu_run_sbin_hotplug(unsigned int cpu, const char *action)
-{
-	char *argv[3], *envp[6], cpu_str[12], action_str[32], devpath_str[40];
-	int i;
-
-	sprintf(cpu_str, "CPU=%d", cpu);
-	sprintf(action_str, "ACTION=%s", action);
-	sprintf(devpath_str, "DEVPATH=devices/system/cpu/cpu%d", cpu);
-	
-	i = 0;
-	argv[i++] = hotplug_path;
-	argv[i++] = "cpu";
-	argv[i] = NULL;
-
-	i = 0;
-	/* minimal command environment */
-	envp[i++] = "HOME=/";
-	envp[i++] = "PATH=/sbin:/bin:/usr/sbin:/usr/bin";
-	envp[i++] = cpu_str;
-	envp[i++] = action_str;
-	envp[i++] = devpath_str;
-	envp[i] = NULL;
-
-	return call_usermodehelper(argv[0], argv, envp, 0);
-}
-
 /* Take this CPU down. */
 static int take_cpu_down(void *unused)
 {
@@ -170,8 +142,6 @@
 
 	check_for_tasks(cpu);
 
-	cpu_run_sbin_hotplug(cpu, "offline");
-
 out_thread:
 	err = kthread_stop(p);
 out_allowed:
@@ -179,11 +149,6 @@
 out:
 	unlock_cpu_hotplug();
 	return err;
-}
-#else
-static inline int cpu_run_sbin_hotplug(unsigned int cpu, const char *action)
-{
-	return 0;
 }
 #endif /*CONFIG_HOTPLUG_CPU*/
 
diff -Nru a/lib/kobject_uevent.c b/lib/kobject_uevent.c
--- a/lib/kobject_uevent.c	2004-10-22 15:59:52 -07:00
+++ b/lib/kobject_uevent.c	2004-10-22 15:59:52 -07:00
@@ -36,6 +36,8 @@
 		return "mount";
 	case KOBJ_UMOUNT:
 		return "umount";
+	case KOBJ_OFFLINE:
+		return "offline";
 	default:
 		return NULL;
 	}

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2022, 2004/10/22 14:30:22-07:00, akpm@osdl.org

[PATCH] kobject_uevent warning fix

lib/kobject_uevent.c:39: warning: `action_to_string' defined but not used

Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 lib/kobject_uevent.c |    2 ++
 1 files changed, 2 insertions(+)


diff -Nru a/lib/kobject_uevent.c b/lib/kobject_uevent.c
--- a/lib/kobject_uevent.c	2004-10-22 15:59:42 -07:00
+++ b/lib/kobject_uevent.c	2004-10-22 15:59:42 -07:00
@@ -23,6 +23,7 @@
 #include <linux/kobject.h>
 #include <net/sock.h>
 
+#if defined(CONFIG_KOBJECT_UEVENT) || defined(CONFIG_HOTPLUG)
 static char *action_to_string(enum kobject_action action)
 {
 	switch (action) {
@@ -42,6 +43,7 @@
 		return NULL;
 	}
 }
+#endif
 
 #ifdef CONFIG_KOBJECT_UEVENT
 static struct sock *uevent_sock;

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

On Thu, Nov 11, 2004 at 08:36:07AM +1100, Paul Mackerras wrote:
> Russell King writes:
> 
> > On Wed, Nov 10, 2004 at 01:57:17PM +1100, Paul Mackerras wrote:
> > > So we can get a driver's probe method called concurrently with its
> > > bus's suspend or resume method.
> > 
> > If correct, we probably have rather a lot of buggy drivers, because
> > I certainly was not aware that this could happen.  I suspect the
> > average driver writer also would not be aware of that.
> 
> No doubt.  I'd still like to hear from Greg or Pat about what the
> concurrency rules are supposed to be, or were intended to be.

They are still under flux as to what they should be :)

I'm currently working on reworking all of the locking in the driver
core, to document, and fix the number of issues that suspend, and
manual disconnect, and manual attach are causing.  It should also fix
the issue that I know you want Russell, namely adding a new device from
a probe() callback.

See the other threads about manual disconnect in the driver model on
lkml for some other details.  I'll post more here when I have some
working code.

thanks,

greg k-h

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Russell King]

On Thu, Nov 11, 2004 at 08:36:07AM +1100, Paul Mackerras wrote:
> I think that what has saved us to some extent is that we only do
> suspend/resume on UP machines so far.

and probably without kernel preemption enabled.  Kernel preemption
should in theory at least open the same can of worms in this area
as SMP does.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Nigel Cunningham]

Hi.

On Thu, 2004-11-11 at 08:36, Paul Mackerras wrote:
> Russell King writes:
> However, pci_dev->driver only gets cleared *after* the driver's remove
> method returns.  Thus it would be quite possible for a PCI device to
> have its suspend/resume methods called while another CPU is in its
> remove method.
> 
> I think that what has saved us to some extent is that we only do
> suspend/resume on UP machines so far.

I'm suspending and resuming all the time using HT. FWIW, I think it's
more likely that you're not seeing the issue for two reasons.

1) User space has been frozen when the suspend/resume methods are called
(I'm right in assuming that only swsusp/pmdisk, suspend-to-ram and
suspend2 would be doing suspends, aren't I?).
2) People tend to wait until the machine has powered down before
unplugging things, and plug things in before powering on.

Please excuse me if I'm speaking nonsense :>

Regards,

Nigel
-- 
Nigel Cunningham
Pastoral Worker
Christian Reformed Church of Tuggeranong
PO Box 1004, Tuggeranong, ACT 2901

You see, at just the right time, when we were still powerless, Christ
died for the ungodly.		-- Romans 5:6

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Paul Mackerras]

Russell King writes:

> On Wed, Nov 10, 2004 at 01:57:17PM +1100, Paul Mackerras wrote:
> > So we can get a driver's probe method called concurrently with its
> > bus's suspend or resume method.
> 
> If correct, we probably have rather a lot of buggy drivers, because
> I certainly was not aware that this could happen.  I suspect the
> average driver writer also would not be aware of that.

No doubt.  I'd still like to hear from Greg or Pat about what the
concurrency rules are supposed to be, or were intended to be.

Note also that I said the *bus's* suspend/resume method.  For PCI, we
have some protection since the PCI suspend/resume methods won't call
the individual pci driver's suspend/resume until pci_dev->driver is
set, which happens after the pci driver's probe routine has returned
(but of course the store to pci_dev->driver can get reordered on
some architectures since there is no barrier).

However, pci_dev->driver only gets cleared *after* the driver's remove
method returns.  Thus it would be quite possible for a PCI device to
have its suspend/resume methods called while another CPU is in its
remove method.

I think that what has saved us to some extent is that we only do
suspend/resume on UP machines so far.

Regards,
Paul.

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Russell King]

On Wed, Nov 10, 2004 at 01:57:17PM +1100, Paul Mackerras wrote:
> So we can get a driver's probe method called concurrently with its
> bus's suspend or resume method.

If correct, we probably have rather a lot of buggy drivers, because
I certainly was not aware that this could happen.  I suspect the
average driver writer also would not be aware of that.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Paul Mackerras]

Greg KH writes:

> On Sun, Nov 07, 2004 at 03:28:05PM +0000, Russell King wrote:
> > On Tue, Nov 02, 2004 at 10:32:29PM +0000, Russell King wrote:
> > > 
> > > Does this mean that a device driver can have its suspend or resume
> > > methods called in the middle of a probe or remove on a different CPU ?
> > > (note: x86 APM does not freeze all processes last time I checked...)
> > > 
> > > If yes, has anyone audited the drivers to ensure that they're correct
> > > in respect of this?
> > 
> > I'll repost the above question since it's of fundamental importance.

I didn't change anything that would affect that.

Unless I am misunderstanding the question, it has always been the case
that a bus's suspend or resume method could be called concurrently
with a driver probe call.  device_add() calls device_pm_add() followed
by bus_add_device().  device_pm_add() puts the device on the
dpm_active list, and once it is there, device_suspend will see it, and
will call the bus's suspend function.  bus_add_device() calls
device_attach() which either calls device_bind_driver() or
driver_probe_device(), and driver_probe_device() calls the driver's
probe function.

So we can get a driver's probe method called concurrently with its
bus's suspend or resume method.  Whether the driver's suspend/resume
method gets called depends on the bus-specific code.  In the case of
PCI, it looks like pci_device_suspend won't call the driver's suspend
function unless pci_dev->driver is set.  That gets set in
pci_device_probe_{dynamic,static} after the drv->probe() call, so that
probably mostly excludes concurrency with the suspend/resume calls
(except that there is no memory barrier in there).  I don't know
whether driver_probe_device() can ever get called for PCI devices.  If
it can I then don't know how/where pci_dev->driver would get set (as
opposed to dev->driver).

I would stress that none of this analysis is any different before or
after my patch though.  The question (of concurrency between
probe/remove and suspend/resume) is an interesting one, but it's not
something that's affected one way or the other by my patch.

Greg, do you have an analysis (or set of rules) for concurrency
between the various driver functions?

Paul.

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

On Sun, Nov 07, 2004 at 03:28:05PM +0000, Russell King wrote:
> On Tue, Nov 02, 2004 at 10:32:29PM +0000, Russell King wrote:
> > On Mon, Nov 01, 2004 at 01:57:57PM -0800, Greg KH wrote:
> > > This patch fixes the problem by using a separate semaphore, called
> > > dpm_list_sem, to cover the places where we need the device pm lists to be
> > > stable, and by being careful about how we traverse the lists on suspend and
> > > resume.  I have analysed the various cases that can occur and I am
> > > confident that I have handled them all correctly.  I posted this patch
> > > together with a detailed analysis 10 days ago.
> > 
> > Does this mean that a device driver can have its suspend or resume
> > methods called in the middle of a probe or remove on a different CPU ?
> > (note: x86 APM does not freeze all processes last time I checked...)
> > 
> > If yes, has anyone audited the drivers to ensure that they're correct
> > in respect of this?
> 
> I'll repost the above question since it's of fundamental importance.

Paul sent in this change, so I'll let him address this.

thanks,

greg k-h

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Russell King]

On Tue, Nov 02, 2004 at 10:32:29PM +0000, Russell King wrote:
> On Mon, Nov 01, 2004 at 01:57:57PM -0800, Greg KH wrote:
> > This patch fixes the problem by using a separate semaphore, called
> > dpm_list_sem, to cover the places where we need the device pm lists to be
> > stable, and by being careful about how we traverse the lists on suspend and
> > resume.  I have analysed the various cases that can occur and I am
> > confident that I have handled them all correctly.  I posted this patch
> > together with a detailed analysis 10 days ago.
> 
> Does this mean that a device driver can have its suspend or resume
> methods called in the middle of a probe or remove on a different CPU ?
> (note: x86 APM does not freeze all processes last time I checked...)
> 
> If yes, has anyone audited the drivers to ensure that they're correct
> in respect of this?

I'll repost the above question since it's of fundamental importance.

Thanks.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Russell King]

On Mon, Nov 01, 2004 at 01:57:57PM -0800, Greg KH wrote:
> This patch fixes the problem by using a separate semaphore, called
> dpm_list_sem, to cover the places where we need the device pm lists to be
> stable, and by being careful about how we traverse the lists on suspend and
> resume.  I have analysed the various cases that can occur and I am
> confident that I have handled them all correctly.  I posted this patch
> together with a detailed analysis 10 days ago.

Does this mean that a device driver can have its suspend or resume
methods called in the middle of a probe or remove on a different CPU ?
(note: x86 APM does not freeze all processes last time I checked...)

If yes, has anyone audited the drivers to ensure that they're correct
in respect of this?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Pozsar Balazs]

On Mon, Nov 01, 2004 at 01:57:56PM -0800, Greg KH wrote:
> diff -Nru a/fs/sysfs/bin.c b/fs/sysfs/bin.c
> --- a/fs/sysfs/bin.c	2004-11-01 13:37:18 -08:00
> +++ b/fs/sysfs/bin.c	2004-11-01 13:37:18 -08:00
> @@ -160,24 +160,26 @@
>  {
>  	struct dentry * dentry;
>  	struct dentry * parent;
> +	umode_t mode = (attr->attr.mode & S_IALLUGO) | S_IFREG;
                        ^^^^^^^^^^^^^^^
>  	int error = 0;
>  
> -	if (!kobj || !attr)
> -		return -EINVAL;
> +	BUG_ON(!kobj || !kobj->dentry || !attr);
                                         ^^^^^

attr is already dereferenced, maybe this check should be moved before 
it.



-- 
pozsy

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2450, 2004/11/01 13:06:02-08:00, kay.sievers@vrfy.org

[PATCH] kobject: fix hotplug bug with seqnum

On Sat, Oct 30, 2004 at 04:54:29AM +0200, Kay Sievers wrote:
> On Sat, Oct 30, 2004 at 02:25:23AM +0200, Kay Sievers wrote:
> > On Sat, Oct 30, 2004 at 02:00:45AM +0200, Kay Sievers wrote:
> > > On Fri, Oct 29, 2004 at 06:13:19PM -0500, Greg KH wrote:
> > > > On Fri, Oct 29, 2004 at 11:28:56PM +0200, Kay Sievers wrote:
> > > > > > But there might still be a problem.  With this change, the sequence
> > > > > > number is not sent out the kevent message.  Kay, do you think this is an
> > > > > > issue?  I don't think we can get netlink messages out of order, right?
> > > > >
> > > > > Right, especially not the events with the same DEVPATH, like "remove"
> > > > > beating an "add". But I'm not sure if the number isn't useful. Whatever
> > > > > we may do with the hotplug over netlink in the future, we will only have
> > > > > /sbin/hotplug for the early boot and it may be nice to know, what events
> > > > > we have already handled...
> > > > >
> > > > > > I'll hold off on applying this patch until we figure this out...
> > > > >
> > > > > How about just reserving 20 bytes for the number (u64 will never be
> > > > > more than that), save the pointer to that field, and fill the number in
> > > > > later?
> > > >
> > > > Ah, something like this instead?  I like it, it's even smaller than the
> > > > previous patch.  Compile tested only...
> > >
> > > I like that. How about the following. It will keep the buffer clean from
> > > random chars, cause the kevent does not have the vector and relies on
> > > the '\0' to separate the strings from each other.
> > > I've tested it. The netlink-hotplug message looks like this:
> > >
> > > recv(3, "remove@/class/input/mouse2\0ACTION=remove\0DEVPATH=/class/input/mouse2\0SUBSYSTEM=input\0SEQNUM=961                 \0", 1024, 0) = 113
> >
> > Hmm, these trailing spaces are just bad, sorry. I'll better pass the
> > envp array over to send_uevent() and clean up the keys while copying
> > the env values into the skb buffer. This will make the event payload
> > more safe too. So your first version looks better.
>
> How about this? We copy over key by key into the skb buffer and the
> netlink message can get the envp array without depending on a single
> continuous buffer.
>
> The netlink message looks nice like this now:
>
> recv(3, "
>   add@/devices/pci0000:00/0000:00:1d.1/usb3/3-2/3-2:1.0\0
>   HOME=/\0
>   PATH=/sbin:/bin:/usr/sbin:/usr/bin\0
>   ACTION=add\0
>   DEVPATH=/devices/pci0000:00/0000:00:1d.1/usb3/3-2/3-2:1.0\0
>   SUBSYSTEM=usb\0
>   SEQNUM=991\0
>   DEVICE=/proc/bus/usb/003/008\0
>   PRODUCT=46d/c03e/2000\0
>   TYPE=0/0/0\0
>   INTERFACE=3/1/2\0
> ", 1024, 0) = 268

Here is an improved version that uses skb_put() to fill the skb buffer,
instead of trimming the buffer to the final size after we've copied over
all keys.


Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 lib/kobject_uevent.c |   47 ++++++++++++++++++++++++++++++-----------------
 1 files changed, 30 insertions(+), 17 deletions(-)


diff -Nru a/lib/kobject_uevent.c b/lib/kobject_uevent.c
--- a/lib/kobject_uevent.c	2004-11-01 13:36:12 -08:00
+++ b/lib/kobject_uevent.c	2004-11-01 13:36:12 -08:00
@@ -23,6 +23,9 @@
 #include <linux/kobject.h>
 #include <net/sock.h>
 
+#define BUFFER_SIZE	1024	/* buffer for the hotplug env */
+#define NUM_ENVP	32	/* number of env pointers */
+
 #if defined(CONFIG_KOBJECT_UEVENT) || defined(CONFIG_HOTPLUG)
 static char *action_to_string(enum kobject_action action)
 {
@@ -53,12 +56,11 @@
  *
  * @signal: signal name
  * @obj: object path (kobject)
- * @buf: buffer used to pass auxiliary data like the hotplug environment
- * @buflen:
- * gfp_mask:
+ * @envp: possible hotplug environment to pass with the message
+ * @gfp_mask:
  */
-static int send_uevent(const char *signal, const char *obj, const void *buf,
-			int buflen, int gfp_mask)
+static int send_uevent(const char *signal, const char *obj,
+		       char **envp, int gfp_mask)
 {
 	struct sk_buff *skb;
 	char *pos;
@@ -69,16 +71,25 @@
 
 	len = strlen(signal) + 1;
 	len += strlen(obj) + 1;
-	len += buflen;
 
-	skb = alloc_skb(len, gfp_mask);
+	/* allocate buffer with the maximum possible message size */
+	skb = alloc_skb(len + BUFFER_SIZE, gfp_mask);
 	if (!skb)
 		return -ENOMEM;
 
 	pos = skb_put(skb, len);
+	sprintf(pos, "%s@%s", signal, obj);
 
-	pos += sprintf(pos, "%s@%s", signal, obj) + 1;
-	memcpy(pos, buf, buflen);
+	/* copy the environment key by key to our continuous buffer */
+	if (envp) {
+		int i;
+
+		for (i = 2; envp[i]; i++) {
+			len = strlen(envp[i]) + 1;
+			pos = skb_put(skb, len);
+			strcpy(pos, envp[i]);
+		}
+	}
 
 	return netlink_broadcast(uevent_sock, skb, 0, 1, gfp_mask);
 }
@@ -107,10 +118,10 @@
 		if (!attrpath)
 			goto exit;
 		sprintf(attrpath, "%s/%s", path, attr->name);
-		rc = send_uevent(signal, attrpath, NULL, 0, gfp_mask);
+		rc = send_uevent(signal, attrpath, NULL, gfp_mask);
 		kfree(attrpath);
 	} else {
-		rc = send_uevent(signal, path, NULL, 0, gfp_mask);
+		rc = send_uevent(signal, path, NULL, gfp_mask);
 	}
 
 exit:
@@ -169,8 +180,6 @@
 u64 hotplug_seqnum;
 static spinlock_t sequence_lock = SPIN_LOCK_UNLOCKED;
 
-#define BUFFER_SIZE	1024	/* should be enough memory for the env */
-#define NUM_ENVP	32	/* number of env pointers */
 /**
  * kobject_hotplug - notify userspace by executing /sbin/hotplug
  *
@@ -182,6 +191,7 @@
 	char *argv [3];
 	char **envp = NULL;
 	char *buffer = NULL;
+	char *seq_buff;
 	char *scratch;
 	int i = 0;
 	int retval;
@@ -258,6 +268,11 @@
 	envp [i++] = scratch;
 	scratch += sprintf(scratch, "SUBSYSTEM=%s", name) + 1;
 
+	/* reserve space for the sequence,
+	 * put the real one in after the hotplug call */
+	envp[i++] = seq_buff = scratch;
+	scratch += strlen("SEQNUM=18446744073709551616") + 1;
+
 	if (hotplug_ops->hotplug) {
 		/* have the kset specific function add its stuff */
 		retval = hotplug_ops->hotplug (kset, kobj,
@@ -273,15 +288,13 @@
 	spin_lock(&sequence_lock);
 	seq = ++hotplug_seqnum;
 	spin_unlock(&sequence_lock);
-
-	envp [i++] = scratch;
-	scratch += sprintf(scratch, "SEQNUM=%lld", (long long)seq) + 1;
+	sprintf(seq_buff, "SEQNUM=%lld", (long long)seq);
 
 	pr_debug ("%s: %s %s seq=%lld %s %s %s %s %s\n",
 		  __FUNCTION__, argv[0], argv[1], (long long)seq,
 		  envp[0], envp[1], envp[2], envp[3], envp[4]);
 
-	send_uevent(action_string, kobj_path, buffer, scratch - buffer, GFP_KERNEL);
+	send_uevent(action_string, kobj_path, envp, GFP_KERNEL);
 
 	if (!hotplug_path[0])
 		goto exit;

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2448, 2004/11/01 13:05:24-08:00, dtor_core@ameritech.net

[PATCH] Driver core: export device_attach

Driver core: make device_attach() global and export it and
             driver_attach() so subsystems can have finer
             control over binding process.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 drivers/base/bus.c     |    4 +++-
 include/linux/device.h |    1 +
 2 files changed, 4 insertions(+), 1 deletion(-)


diff -Nru a/drivers/base/bus.c b/drivers/base/bus.c
--- a/drivers/base/bus.c	2004-11-01 13:36:27 -08:00
+++ b/drivers/base/bus.c	2004-11-01 13:36:27 -08:00
@@ -288,7 +288,7 @@
  *	Walk the list of drivers that the bus has and call bus_match()
  *	for each pair. If a compatible pair is found, break out and return.
  */
-static int device_attach(struct device * dev)
+int device_attach(struct device * dev)
 {
  	struct bus_type * bus = dev->bus;
 	struct list_head * entry;
@@ -728,6 +728,8 @@
 
 EXPORT_SYMBOL_GPL(device_bind_driver);
 EXPORT_SYMBOL_GPL(device_release_driver);
+EXPORT_SYMBOL_GPL(device_attach);
+EXPORT_SYMBOL_GPL(driver_attach);
 
 EXPORT_SYMBOL_GPL(bus_add_device);
 EXPORT_SYMBOL_GPL(bus_remove_device);
diff -Nru a/include/linux/device.h b/include/linux/device.h
--- a/include/linux/device.h	2004-11-01 13:36:27 -08:00
+++ b/include/linux/device.h	2004-11-01 13:36:27 -08:00
@@ -327,6 +327,7 @@
  */
 extern void device_bind_driver(struct device * dev);
 extern void device_release_driver(struct device * dev);
+extern int  device_attach(struct device * dev);
 extern void driver_attach(struct device_driver * drv);
 
 

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2447, 2004/11/01 13:05:06-08:00, akpm@osdl.org

[PATCH] Fix deadlocks on dpm_sem

From: Paul Mackerras <paulus@samba.org>

Currently the device_pm_foo() functions are rather prone to deadlocks
during suspend/resume.  This is because the dpm_sem is held for the
duration of device_suspend() and device_resume() as well as device_pm_add()
and device_pm_remove().  If for any reason you get a device addition or
removal triggered by a device's suspend or resume code, you get a deadlock.
 (The classic example is a USB host adaptor resuming and discovering that
the mouse you used to have plugged in has gone away.)

This patch fixes the problem by using a separate semaphore, called
dpm_list_sem, to cover the places where we need the device pm lists to be
stable, and by being careful about how we traverse the lists on suspend and
resume.  I have analysed the various cases that can occur and I am
confident that I have handled them all correctly.  I posted this patch
together with a detailed analysis 10 days ago.

Signed-off-by Paul Mackerras <paulus@samba.org>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 drivers/base/power/main.c    |   11 +++++-----
 drivers/base/power/power.h   |    5 ++++
 drivers/base/power/resume.c  |   16 ++++++++++++---
 drivers/base/power/suspend.c |   44 ++++++++++++++++++++++++-------------------
 4 files changed, 49 insertions(+), 27 deletions(-)


diff -Nru a/drivers/base/power/main.c b/drivers/base/power/main.c
--- a/drivers/base/power/main.c	2004-11-01 13:36:34 -08:00
+++ b/drivers/base/power/main.c	2004-11-01 13:36:34 -08:00
@@ -28,6 +28,7 @@
 LIST_HEAD(dpm_off_irq);
 
 DECLARE_MUTEX(dpm_sem);
+DECLARE_MUTEX(dpm_list_sem);
 
 /*
  * PM Reference Counting.
@@ -75,12 +76,12 @@
 	pr_debug("PM: Adding info for %s:%s\n",
 		 dev->bus ? dev->bus->name : "No Bus", dev->kobj.name);
 	atomic_set(&dev->power.pm_users, 0);
-	down(&dpm_sem);
+	down(&dpm_list_sem);
 	list_add_tail(&dev->power.entry, &dpm_active);
 	device_pm_set_parent(dev, dev->parent);
 	if ((error = dpm_sysfs_add(dev)))
 		list_del(&dev->power.entry);
-	up(&dpm_sem);
+	up(&dpm_list_sem);
 	return error;
 }
 
@@ -88,11 +89,11 @@
 {
 	pr_debug("PM: Removing info for %s:%s\n",
 		 dev->bus ? dev->bus->name : "No Bus", dev->kobj.name);
-	down(&dpm_sem);
+	down(&dpm_list_sem);
 	dpm_sysfs_remove(dev);
 	device_pm_release(dev->power.pm_parent);
-	list_del(&dev->power.entry);
-	up(&dpm_sem);
+	list_del_init(&dev->power.entry);
+	up(&dpm_list_sem);
 }
 
 
diff -Nru a/drivers/base/power/power.h b/drivers/base/power/power.h
--- a/drivers/base/power/power.h	2004-11-01 13:36:34 -08:00
+++ b/drivers/base/power/power.h	2004-11-01 13:36:34 -08:00
@@ -28,6 +28,11 @@
 extern struct semaphore dpm_sem;
 
 /*
+ * Used to serialize changes to the dpm_* lists.
+ */
+extern struct semaphore dpm_list_sem;
+
+/*
  * The PM lists.
  */
 extern struct list_head dpm_active;
diff -Nru a/drivers/base/power/resume.c b/drivers/base/power/resume.c
--- a/drivers/base/power/resume.c	2004-11-01 13:36:34 -08:00
+++ b/drivers/base/power/resume.c	2004-11-01 13:36:34 -08:00
@@ -31,16 +31,22 @@
 
 void dpm_resume(void)
 {
+	down(&dpm_list_sem);
 	while(!list_empty(&dpm_off)) {
 		struct list_head * entry = dpm_off.next;
 		struct device * dev = to_device(entry);
+
+		get_device(dev);
 		list_del_init(entry);
+		list_add_tail(entry, &dpm_active);
 
+		up(&dpm_list_sem);
 		if (!dev->power.prev_state)
 			resume_device(dev);
-
-		list_add_tail(entry, &dpm_active);
+		down(&dpm_list_sem);
+		put_device(dev);
 	}
+	up(&dpm_list_sem);
 }
 
 
@@ -76,9 +82,13 @@
 {
 	while(!list_empty(&dpm_off_irq)) {
 		struct list_head * entry = dpm_off_irq.next;
+		struct device * dev = to_device(entry);
+
+		get_device(dev);
 		list_del_init(entry);
-		resume_device(to_device(entry));
 		list_add_tail(entry, &dpm_active);
+		resume_device(dev);
+		put_device(dev);
 	}
 }
 
diff -Nru a/drivers/base/power/suspend.c b/drivers/base/power/suspend.c
--- a/drivers/base/power/suspend.c	2004-11-01 13:36:34 -08:00
+++ b/drivers/base/power/suspend.c	2004-11-01 13:36:34 -08:00
@@ -63,11 +63,6 @@
  *	If we hit a failure with any of the devices, call device_resume()
  *	above to bring the suspended devices back to life.
  *
- *	Note this function leaves dpm_sem held to
- *	a) block other devices from registering.
- *	b) prevent other PM operations from happening after we've begun.
- *	c) make sure we're exclusive when we disable interrupts.
- *
  */
 
 int device_suspend(u32 state)
@@ -75,29 +70,40 @@
 	int error = 0;
 
 	down(&dpm_sem);
-	while(!list_empty(&dpm_active)) {
+	down(&dpm_list_sem);
+	while (!list_empty(&dpm_active) && error == 0) {
 		struct list_head * entry = dpm_active.prev;
 		struct device * dev = to_device(entry);
+
+		get_device(dev);
+		up(&dpm_list_sem);
+
 		error = suspend_device(dev, state);
 
-		if (!error) {
-			list_del(&dev->power.entry);
-			list_add(&dev->power.entry, &dpm_off);
-		} else if (error == -EAGAIN) {
-			list_del(&dev->power.entry);
-			list_add(&dev->power.entry, &dpm_off_irq);
-		} else {
+		down(&dpm_list_sem);
+
+		/* Check if the device got removed */
+		if (!list_empty(&dev->power.entry)) {
+			/* Move it to the dpm_off or dpm_off_irq list */
+			if (!error) {
+				list_del(&dev->power.entry);
+				list_add(&dev->power.entry, &dpm_off);
+			} else if (error == -EAGAIN) {
+				list_del(&dev->power.entry);
+				list_add(&dev->power.entry, &dpm_off_irq);
+				error = 0;
+			}
+		}
+		if (error)
 			printk(KERN_ERR "Could not suspend device %s: "
 				"error %d\n", kobject_name(&dev->kobj), error);
-			goto Error;
-		}
+		put_device(dev);
 	}
- Done:
+	up(&dpm_list_sem);
+	if (error)
+		dpm_resume();
 	up(&dpm_sem);
 	return error;
- Error:
-	dpm_resume();
-	goto Done;
 }
 
 EXPORT_SYMBOL_GPL(device_suspend);

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2452, 2004/11/01 13:06:49-08:00, kay.sievers@vrfy.org

[PATCH] take me home, hotplug_path[]

Move hotplug_path[] out of kmod.[ch] to kobject_uevent.[ch] where
it belongs now. At some time in the future we should fix the remaining bad
hotplug calls (no SEQNUM, no netlink uevent):

  ./drivers/input/input.c (no DEVPATH on some hotplug events!)
  ./drivers/pnp/pnpbios/core.c
  ./drivers/s390/crypto/z90main.c

Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 drivers/input/input.c          |    2 +-
 drivers/pnp/pnpbios/core.c     |    2 +-
 drivers/s390/crypto/z90main.c  |    1 +
 include/linux/kmod.h           |    4 ----
 include/linux/kobject_uevent.h |    5 +++++
 kernel/cpu.c                   |    1 -
 kernel/kmod.c                  |   23 -----------------------
 kernel/sysctl.c                |    2 +-
 lib/kobject_uevent.c           |    1 +
 9 files changed, 10 insertions(+), 31 deletions(-)


diff -Nru a/drivers/input/input.c b/drivers/input/input.c
--- a/drivers/input/input.c	2004-11-01 13:35:57 -08:00
+++ b/drivers/input/input.c	2004-11-01 13:35:57 -08:00
@@ -19,7 +19,7 @@
 #include <linux/major.h>
 #include <linux/pm.h>
 #include <linux/proc_fs.h>
-#include <linux/kmod.h>
+#include <linux/kobject_uevent.h>
 #include <linux/interrupt.h>
 #include <linux/poll.h>
 #include <linux/device.h>
diff -Nru a/drivers/pnp/pnpbios/core.c b/drivers/pnp/pnpbios/core.c
--- a/drivers/pnp/pnpbios/core.c	2004-11-01 13:35:57 -08:00
+++ b/drivers/pnp/pnpbios/core.c	2004-11-01 13:35:57 -08:00
@@ -56,7 +56,7 @@
 #include <linux/mm.h>
 #include <linux/smp.h>
 #include <linux/slab.h>
-#include <linux/kmod.h>
+#include <linux/kobject_uevent.h>
 #include <linux/completion.h>
 #include <linux/spinlock.h>
 #include <linux/dmi.h>
diff -Nru a/drivers/s390/crypto/z90main.c b/drivers/s390/crypto/z90main.c
--- a/drivers/s390/crypto/z90main.c	2004-11-01 13:35:57 -08:00
+++ b/drivers/s390/crypto/z90main.c	2004-11-01 13:35:57 -08:00
@@ -33,6 +33,7 @@
 #include <linux/ioctl32.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/kobject_uevent.h>
 #include <linux/proc_fs.h>
 #include <linux/syscalls.h>
 #include <linux/version.h>
diff -Nru a/include/linux/kmod.h b/include/linux/kmod.h
--- a/include/linux/kmod.h	2004-11-01 13:35:57 -08:00
+++ b/include/linux/kmod.h	2004-11-01 13:35:57 -08:00
@@ -37,8 +37,4 @@
 extern int call_usermodehelper(char *path, char *argv[], char *envp[], int wait);
 extern void usermodehelper_init(void);
 
-#ifdef CONFIG_HOTPLUG
-extern char hotplug_path [];
-#endif
-
 #endif /* __LINUX_KMOD_H__ */
diff -Nru a/include/linux/kobject_uevent.h b/include/linux/kobject_uevent.h
--- a/include/linux/kobject_uevent.h	2004-11-01 13:35:57 -08:00
+++ b/include/linux/kobject_uevent.h	2004-11-01 13:35:57 -08:00
@@ -11,6 +11,11 @@
 #ifndef _KOBJECT_EVENT_H_
 #define _KOBJECT_EVENT_H_
 
+#define HOTPLUG_PATH_LEN	256
+
+/* path to the hotplug userspace helper executed on an event */
+extern char hotplug_path[];
+
 /*
  * If you add an action here, you must also add the proper string to the
  * lib/kobject_uevent.c file.
diff -Nru a/kernel/cpu.c b/kernel/cpu.c
--- a/kernel/cpu.c	2004-11-01 13:35:57 -08:00
+++ b/kernel/cpu.c	2004-11-01 13:35:57 -08:00
@@ -11,7 +11,6 @@
 #include <linux/unistd.h>
 #include <linux/cpu.h>
 #include <linux/module.h>
-#include <linux/kmod.h>		/* for hotplug_path */
 #include <linux/kthread.h>
 #include <linux/stop_machine.h>
 #include <asm/semaphore.h>
diff -Nru a/kernel/kmod.c b/kernel/kmod.c
--- a/kernel/kmod.c	2004-11-01 13:35:57 -08:00
+++ b/kernel/kmod.c	2004-11-01 13:35:57 -08:00
@@ -115,29 +115,6 @@
 EXPORT_SYMBOL(request_module);
 #endif /* CONFIG_KMOD */
 
-#ifdef CONFIG_HOTPLUG
-/*
-	hotplug path is set via /proc/sys
-	invoked by hotplug-aware bus drivers,
-	with call_usermodehelper
-
-	argv [0] = hotplug_path;
-	argv [1] = "usb", "scsi", "pci", "network", etc;
-	... plus optional type-specific parameters
-	argv [n] = 0;
-
-	envp [*] = HOME, PATH; optional type-specific parameters
-
-	a hotplug bus should invoke this for device add/remove
-	events.  the command is expected to load drivers when
-	necessary, and may perform additional system setup.
-*/
-char hotplug_path[KMOD_PATH_LEN] = "/sbin/hotplug";
-
-EXPORT_SYMBOL(hotplug_path);
-
-#endif /* CONFIG_HOTPLUG */
-
 struct subprocess_info {
 	struct completion *complete;
 	char *path;
diff -Nru a/kernel/sysctl.c b/kernel/sysctl.c
--- a/kernel/sysctl.c	2004-11-01 13:35:57 -08:00
+++ b/kernel/sysctl.c	2004-11-01 13:35:57 -08:00
@@ -394,7 +394,7 @@
 		.ctl_name	= KERN_HOTPLUG,
 		.procname	= "hotplug",
 		.data		= &hotplug_path,
-		.maxlen		= KMOD_PATH_LEN,
+		.maxlen		= HOTPLUG_PATH_LEN,
 		.mode		= 0644,
 		.proc_handler	= &proc_dostring,
 		.strategy	= &sysctl_string,
diff -Nru a/lib/kobject_uevent.c b/lib/kobject_uevent.c
--- a/lib/kobject_uevent.c	2004-11-01 13:35:57 -08:00
+++ b/lib/kobject_uevent.c	2004-11-01 13:35:57 -08:00
@@ -177,6 +177,7 @@
 
 
 #ifdef CONFIG_HOTPLUG
+char hotplug_path[HOTPLUG_PATH_LEN] = "/sbin/hotplug";
 u64 hotplug_seqnum;
 static spinlock_t sequence_lock = SPIN_LOCK_UNLOCKED;
 

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2449, 2004/11/01 13:05:43-08:00, dtor_core@ameritech.net

[PATCH] Driver core: add driver_probe_device

Driver core: rename bus_match into driver_probe_device and export
             it so subsystems can bind an individual device to a
             specific driver without getting involved with driver
             core internals.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 drivers/base/bus.c     |   55 +++++++++++++++++++++++++------------------------
 include/linux/device.h |    3 +-
 2 files changed, 31 insertions(+), 27 deletions(-)


diff -Nru a/drivers/base/bus.c b/drivers/base/bus.c
--- a/drivers/base/bus.c	2004-11-01 13:36:20 -08:00
+++ b/drivers/base/bus.c	2004-11-01 13:36:20 -08:00
@@ -250,34 +250,35 @@
 
 
 /**
- *	bus_match - check compatibility between device & driver.
- *	@dev:	device.
+ *	driver_probe_device - attempt to bind device & driver.
  *	@drv:	driver.
+ *	@dev:	device.
  *
- *	First, we call the bus's match function, which should compare
- *	the device IDs the driver supports with the device IDs of the
- *	device. Note we don't do this ourselves because we don't know
- *	the format of the ID structures, nor what is to be considered
- *	a match and what is not.
+ *	First, we call the bus's match function, if one present, which
+ *	should compare the device IDs the driver supports with the
+ *	device IDs of the device. Note we don't do this ourselves
+ *	because we don't know the format of the ID structures, nor what
+ *	is to be considered a match and what is not.
  *
  *	If we find a match, we call @drv->probe(@dev) if it exists, and
- *	call attach() above.
+ *	call device_bind_driver() above.
  */
-static int bus_match(struct device * dev, struct device_driver * drv)
+int driver_probe_device(struct device_driver * drv, struct device * dev)
 {
-	int error = -ENODEV;
-	if (dev->bus->match(dev, drv)) {
-		dev->driver = drv;
-		if (drv->probe) {
-			if ((error = drv->probe(dev))) {
-				dev->driver = NULL;
-				return error;
-			}
+	if (drv->bus->match && !drv->bus->match(dev, drv))
+		return -ENODEV;
+
+	dev->driver = drv;
+	if (drv->probe) {
+		int error = drv->probe(dev);
+		if (error) {
+			dev->driver = NULL;
+			return error;
 		}
-		device_bind_driver(dev);
-		error = 0;
 	}
-	return error;
+
+	device_bind_driver(dev);
+	return 0;
 }
 
 
@@ -285,8 +286,9 @@
  *	device_attach - try to attach device to a driver.
  *	@dev:	device.
  *
- *	Walk the list of drivers that the bus has and call bus_match()
- *	for each pair. If a compatible pair is found, break out and return.
+ *	Walk the list of drivers that the bus has and call
+ *	driver_probe_device() for each pair. If a compatible
+ *	pair is found, break out and return.
  */
 int device_attach(struct device * dev)
 {
@@ -302,7 +304,7 @@
 	if (bus->match) {
 		list_for_each(entry, &bus->drivers.list) {
 			struct device_driver * drv = to_drv(entry);
-			error = bus_match(dev, drv);
+			error = driver_probe_device(drv, dev);
 			if (!error)
 				/* success, driver matched */
 				return 1;
@@ -327,8 +329,8 @@
  *	If bus_match() returns 0 and the @dev->driver is set, we've found
  *	a compatible pair.
  *
- *	Note that we ignore the -ENODEV error from bus_match(), since it's
- *	perfectly valid for a driver not to bind to any devices.
+ *	Note that we ignore the -ENODEV error from driver_probe_device(),
+ *	since it's perfectly valid for a driver not to bind to any devices.
  */
 void driver_attach(struct device_driver * drv)
 {
@@ -342,7 +344,7 @@
 	list_for_each(entry, &bus->devices.list) {
 		struct device * dev = container_of(entry, struct device, bus_list);
 		if (!dev->driver) {
-			error = bus_match(dev, drv);
+			error = driver_probe_device(drv, dev);
 			if (error && (error != -ENODEV))
 				/* driver matched but the probe failed */
 				printk(KERN_WARNING
@@ -726,6 +728,7 @@
 EXPORT_SYMBOL_GPL(bus_for_each_dev);
 EXPORT_SYMBOL_GPL(bus_for_each_drv);
 
+EXPORT_SYMBOL_GPL(driver_probe_device);
 EXPORT_SYMBOL_GPL(device_bind_driver);
 EXPORT_SYMBOL_GPL(device_release_driver);
 EXPORT_SYMBOL_GPL(device_attach);
diff -Nru a/include/linux/device.h b/include/linux/device.h
--- a/include/linux/device.h	2004-11-01 13:36:20 -08:00
+++ b/include/linux/device.h	2004-11-01 13:36:20 -08:00
@@ -322,9 +322,10 @@
 		     int (*fn)(struct device *, void *));
 
 /*
- * Manual binding of a device to driver. See drivers/base/bus.c 
+ * Manual binding of a device to driver. See drivers/base/bus.c
  * for information on use.
  */
+extern int  driver_probe_device(struct device_driver * drv, struct device * dev);
 extern void device_bind_driver(struct device * dev);
 extern void device_release_driver(struct device * dev);
 extern int  device_attach(struct device * dev);

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2451, 2004/11/01 13:06:31-08:00, dtor_core@ameritech.net

[PATCH] Driver core: add driver symlink to device

Driver core: when binding device to a driver create "driver"
             symlink in device's directory. Rename serio's
             "driver" attribute to "drvctl" (write-only)

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 drivers/base/bus.c          |    2 ++
 drivers/input/serio/serio.c |    7 +------
 2 files changed, 3 insertions(+), 6 deletions(-)


diff -Nru a/drivers/base/bus.c b/drivers/base/bus.c
--- a/drivers/base/bus.c	2004-11-01 13:36:05 -08:00
+++ b/drivers/base/bus.c	2004-11-01 13:36:05 -08:00
@@ -246,6 +246,7 @@
 	list_add_tail(&dev->driver_list, &dev->driver->devices);
 	sysfs_create_link(&dev->driver->kobj, &dev->kobj,
 			  kobject_name(&dev->kobj));
+	sysfs_create_link(&dev->kobj, &dev->driver->kobj, "driver");
 }
 
 
@@ -370,6 +371,7 @@
 	struct device_driver * drv = dev->driver;
 	if (drv) {
 		sysfs_remove_link(&drv->kobj, kobject_name(&dev->kobj));
+		sysfs_remove_link(&dev->kobj, "driver");
 		list_del_init(&dev->driver_list);
 		device_detach_shutdown(dev);
 		if (drv->remove)
diff -Nru a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
--- a/drivers/input/serio/serio.c	2004-11-01 13:36:05 -08:00
+++ b/drivers/input/serio/serio.c	2004-11-01 13:36:05 -08:00
@@ -246,11 +246,6 @@
 	return sprintf(buf, "%s\n", serio->name);
 }
 
-static ssize_t serio_show_driver(struct device *dev, char *buf)
-{
-	return sprintf(buf, "%s\n", dev->driver ? dev->driver->name : "(none)");
-}
-
 static ssize_t serio_rebind_driver(struct device *dev, const char *buf, size_t count)
 {
 	struct serio *serio = to_serio_port(dev);
@@ -307,7 +302,7 @@
 
 static struct device_attribute serio_device_attrs[] = {
 	__ATTR(description, S_IRUGO, serio_show_description, NULL),
-	__ATTR(driver, S_IWUSR | S_IRUGO, serio_show_driver, serio_rebind_driver),
+	__ATTR(drvctl, S_IWUSR, NULL, serio_rebind_driver),
 	__ATTR(bind_mode, S_IWUSR | S_IRUGO, serio_show_bind_mode, serio_set_bind_mode),
 	__ATTR_NULL
 };

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2441, 2004/11/01 13:03:13-08:00, akpm@osdl.org

[PATCH] sysfs backing store - add sysfs_direct structure

From: Maneesh Soni <maneesh@in.ibm.com>

o This patch introduces the new sysfs_dirent data structure. The sysfs_dirent
  is added to the dentry corresponding to each of the element which can be
  represented in sysfs like, kobject (directory), text or binary attributes
  (files), attribute groups (directory) and symlinks.

o It uses dentry's d_fsdata field to attach the corresponding sysfs_dirent.

o The sysfs_dirents are maintained in a tree of all the sysfs entries using the
  s_children and s_sibling list pointers.

o This patch also changes how we access attributes and kobjects in
  file_operations from a given dentry, basically introducing one more level of
  indirection.

o The sysfs_dirents are freed and the sysfs_dirent tree is updated accordingly
  upon the deletion of corresponding dentry. The sysfs dirents are kept alive
  as long as there is corresponding dentry around. The are freed when the
  dentry is finally out of dcache using the ->d_iput() method.

o This also fixes the dentry leaks in case of error paths after sysfs has
  got a newly alocated (and hashed) dentry from sysfs_get_dentry() by
  d_drop()'ing the dentry.

Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 fs/sysfs/bin.c        |   14 ++++----
 fs/sysfs/dir.c        |   84 ++++++++++++++++++++++++++++++++++++++++++++------
 fs/sysfs/file.c       |   25 ++++++++------
 fs/sysfs/group.c      |    4 +-
 fs/sysfs/inode.c      |   10 ++++-
 fs/sysfs/mount.c      |    8 ++++
 fs/sysfs/symlink.c    |   39 ++++++++++++++++++++---
 fs/sysfs/sysfs.h      |   39 +++++++++++++++++++----
 include/linux/sysfs.h |   19 +++++++++++
 9 files changed, 204 insertions(+), 38 deletions(-)


diff -Nru a/fs/sysfs/bin.c b/fs/sysfs/bin.c
--- a/fs/sysfs/bin.c	2004-11-01 13:37:18 -08:00
+++ b/fs/sysfs/bin.c	2004-11-01 13:37:18 -08:00
@@ -160,24 +160,26 @@
 {
 	struct dentry * dentry;
 	struct dentry * parent;
+	umode_t mode = (attr->attr.mode & S_IALLUGO) | S_IFREG;
 	int error = 0;
 
-	if (!kobj || !attr)
-		return -EINVAL;
+	BUG_ON(!kobj || !kobj->dentry || !attr);
 
 	parent = kobj->dentry;
 
 	down(&parent->d_inode->i_sem);
 	dentry = sysfs_get_dentry(parent,attr->attr.name);
 	if (!IS_ERR(dentry)) {
-		dentry->d_fsdata = (void *)attr;
-		error = sysfs_create(dentry,
-				     (attr->attr.mode & S_IALLUGO) | S_IFREG,
-				     NULL);
+		error = sysfs_create(dentry, mode, NULL);
 		if (!error) {
 			dentry->d_inode->i_size = attr->size;
 			dentry->d_inode->i_fop = &bin_fops;
+			error = sysfs_make_dirent(parent->d_fsdata, dentry,
+						  (void *) attr, mode,
+						  SYSFS_KOBJ_BIN_ATTR);
 		}
+		if (error)
+			d_drop(dentry);
 		dput(dentry);
 	} else
 		error = PTR_ERR(dentry);
diff -Nru a/fs/sysfs/dir.c b/fs/sysfs/dir.c
--- a/fs/sysfs/dir.c	2004-11-01 13:37:18 -08:00
+++ b/fs/sysfs/dir.c	2004-11-01 13:37:18 -08:00
@@ -12,6 +12,61 @@
 
 DECLARE_RWSEM(sysfs_rename_sem);
 
+static void sysfs_d_iput(struct dentry * dentry, struct inode * inode)
+{
+	struct sysfs_dirent * sd = dentry->d_fsdata;
+
+	if (sd) {
+		BUG_ON(sd->s_dentry != dentry);
+		sd->s_dentry = NULL;
+		release_sysfs_dirent(sd);
+	}
+	iput(inode);
+}
+
+static struct dentry_operations sysfs_dentry_ops = {
+	.d_iput		= sysfs_d_iput,
+};
+
+/*
+ * Allocates a new sysfs_dirent and links it to the parent sysfs_dirent
+ */
+static struct sysfs_dirent * sysfs_new_dirent(struct sysfs_dirent * parent_sd,
+						void * element)
+{
+	struct sysfs_dirent * sd;
+
+	sd = kmalloc(sizeof(*sd), GFP_KERNEL);
+	if (!sd)
+		return ERR_PTR(-ENOMEM);
+
+	memset(sd, 0, sizeof(*sd));
+	atomic_set(&sd->s_count, 1);
+	INIT_LIST_HEAD(&sd->s_children);
+	list_add(&sd->s_sibling, &parent_sd->s_children);
+	sd->s_element = element;
+
+	return sd;
+}
+
+int sysfs_make_dirent(struct sysfs_dirent * parent_sd, struct dentry * dentry,
+			void * element, umode_t mode, int type)
+{
+	struct sysfs_dirent * sd;
+
+	sd = sysfs_new_dirent(parent_sd, element);
+	if (!sd)
+		return -ENOMEM;
+
+	sd->s_mode = mode;
+	sd->s_type = type;
+	sd->s_dentry = dentry;
+	dentry->d_fsdata = sd;
+	dentry->d_op = &sysfs_dentry_ops;
+
+	return 0;
+}
+
 static int init_dir(struct inode * inode)
 {
 	inode->i_op = &simple_dir_inode_operations;
@@ -27,17 +82,20 @@
 		      const char * n, struct dentry ** d)
 {
 	int error;
+	umode_t mode = S_IFDIR| S_IRWXU | S_IRUGO | S_IXUGO;
 
 	down(&p->d_inode->i_sem);
 	*d = sysfs_get_dentry(p,n);
 	if (!IS_ERR(*d)) {
-		error = sysfs_create(*d,
-					 S_IFDIR| S_IRWXU | S_IRUGO | S_IXUGO,
-					 init_dir);
+		error = sysfs_create(*d, mode, init_dir);
 		if (!error) {
-			(*d)->d_fsdata = k;
-			p->d_inode->i_nlink++;
+			error = sysfs_make_dirent(p->d_fsdata, *d, k, mode,
+						SYSFS_DIR);
+			if (!error)
+				p->d_inode->i_nlink++;
 		}
+		if (error)
+			d_drop(*d);
 		dput(*d);
 	} else
 		error = PTR_ERR(*d);
@@ -63,8 +121,7 @@
 	struct dentry * parent;
 	int error = 0;
 
-	if (!kobj)
-		return -EINVAL;
+	BUG_ON(!kobj);
 
 	if (kobj->parent)
 		parent = kobj->parent->dentry;
@@ -83,8 +140,12 @@
 static void remove_dir(struct dentry * d)
 {
 	struct dentry * parent = dget(d->d_parent);
+	struct sysfs_dirent * sd;
+
 	down(&parent->d_inode->i_sem);
 	d_delete(d);
+	sd = d->d_fsdata;
+ 	list_del_init(&sd->s_sibling);
 	if (d->d_inode)
 		simple_rmdir(parent->d_inode,d);
 
@@ -130,6 +191,7 @@
 		node = node->next;
 		pr_debug(" o %s (%d): ",d->d_name.name,atomic_read(&d->d_count));
 		if (!d_unhashed(d) && (d->d_inode)) {
+			struct sysfs_dirent * sd = d->d_fsdata;
 			d = dget_locked(d);
 			pr_debug("removing");
 
@@ -142,8 +204,9 @@
 			 * a symlink
 			 */
 			if (S_ISLNK(d->d_inode->i_mode))
-				kobject_put(d->d_fsdata);
+				kobject_put(sd->s_element);
 			
+			list_del_init(&sd->s_sibling);
 			simple_unlink(dentry->d_inode,d);
 			dput(d);
 			pr_debug(" done\n");
@@ -184,7 +247,10 @@
 			error = kobject_set_name(kobj, "%s", new_name);
 			if (!error)
 				d_move(kobj->dentry, new_dentry);
-		}
+			else
+				d_drop(new_dentry);
+		} else
+			error = -EEXIST;
 		dput(new_dentry);
 	}
 	up(&parent->d_inode->i_sem);	
diff -Nru a/fs/sysfs/file.c b/fs/sysfs/file.c
--- a/fs/sysfs/file.c	2004-11-01 13:37:18 -08:00
+++ b/fs/sysfs/file.c	2004-11-01 13:37:18 -08:00
@@ -346,19 +346,22 @@
 };
 
 
-int sysfs_add_file(struct dentry * dir, const struct attribute * attr)
+int sysfs_add_file(struct dentry * dir, const struct attribute * attr, int type)
 {
 	struct dentry * dentry;
-	int error;
+	struct sysfs_dirent * parent_sd = dir->d_fsdata;
+	umode_t mode = (attr->mode & S_IALLUGO) | S_IFREG;
+	int error = 0;
 
 	down(&dir->d_inode->i_sem);
 	dentry = sysfs_get_dentry(dir,attr->name);
 	if (!IS_ERR(dentry)) {
-		error = sysfs_create(dentry,
-				     (attr->mode & S_IALLUGO) | S_IFREG,
-				     init_file);
+		error = sysfs_create(dentry, mode, init_file);
 		if (!error)
-			dentry->d_fsdata = (void *)attr;
+			error = sysfs_make_dirent(parent_sd, dentry,
+						(void *) attr, mode, type);
+		if (error)
+			d_drop(dentry);
 		dput(dentry);
 	} else
 		error = PTR_ERR(dentry);
@@ -375,9 +378,10 @@
 
 int sysfs_create_file(struct kobject * kobj, const struct attribute * attr)
 {
-	if (kobj && attr)
-		return sysfs_add_file(kobj->dentry,attr);
-	return -EINVAL;
+	BUG_ON(!kobj || !kobj->dentry || !attr);
+
+	return sysfs_add_file(kobj->dentry, attr, SYSFS_KOBJ_ATTR);
+
 }
 
 
@@ -409,7 +413,8 @@
 			 */
 			dput(victim);
 			res = 0;
-		}
+		} else
+			d_drop(victim);
 		
 		/**
 		 * Drop the reference acquired from sysfs_get_dentry() above.
diff -Nru a/fs/sysfs/group.c b/fs/sysfs/group.c
--- a/fs/sysfs/group.c	2004-11-01 13:37:18 -08:00
+++ b/fs/sysfs/group.c	2004-11-01 13:37:18 -08:00
@@ -31,7 +31,7 @@
 	int error = 0;
 
 	for (attr = grp->attrs; *attr && !error; attr++) {
-		error = sysfs_add_file(dir,*attr);
+		error = sysfs_add_file(dir, *attr, SYSFS_KOBJ_ATTR);
 	}
 	if (error)
 		remove_files(dir,grp);
@@ -44,6 +44,8 @@
 {
 	struct dentry * dir;
 	int error;
+
+	BUG_ON(!kobj || !kobj->dentry);
 
 	if (grp->name) {
 		error = sysfs_create_subdir(kobj,grp->name,&dir);
diff -Nru a/fs/sysfs/inode.c b/fs/sysfs/inode.c
--- a/fs/sysfs/inode.c	2004-11-01 13:37:18 -08:00
+++ b/fs/sysfs/inode.c	2004-11-01 13:37:18 -08:00
@@ -11,6 +11,8 @@
 #include <linux/pagemap.h>
 #include <linux/namei.h>
 #include <linux/backing-dev.h>
+#include "sysfs.h"
+
 extern struct super_block * sysfs_sb;
 
 static struct address_space_operations sysfs_aops = {
@@ -91,6 +93,7 @@
 void sysfs_hash_and_remove(struct dentry * dir, const char * name)
 {
 	struct dentry * victim;
+	struct sysfs_dirent * sd;
 
 	down(&dir->d_inode->i_sem);
 	victim = sysfs_get_dentry(dir,name);
@@ -101,14 +104,17 @@
 			pr_debug("sysfs: Removing %s (%d)\n", victim->d_name.name,
 				 atomic_read(&victim->d_count));
 
+			sd = victim->d_fsdata;
 			d_drop(victim);
 			/* release the target kobject in case of 
 			 * a symlink
 			 */
 			if (S_ISLNK(victim->d_inode->i_mode))
-				kobject_put(victim->d_fsdata);
+				kobject_put(sd->s_element);
+			list_del_init(&sd->s_sibling);
 			simple_unlink(dir->d_inode,victim);
-		}
+		} else
+			d_drop(victim);
 		/*
 		 * Drop reference from sysfs_get_dentry() above.
 		 */
diff -Nru a/fs/sysfs/mount.c b/fs/sysfs/mount.c
--- a/fs/sysfs/mount.c	2004-11-01 13:37:18 -08:00
+++ b/fs/sysfs/mount.c	2004-11-01 13:37:18 -08:00
@@ -22,6 +22,13 @@
 	.drop_inode	= generic_delete_inode,
 };
 
+struct sysfs_dirent sysfs_root = {
+	.s_sibling	= LIST_HEAD_INIT(sysfs_root.s_sibling),
+	.s_children	= LIST_HEAD_INIT(sysfs_root.s_children),
+	.s_element	= NULL,
+	.s_type		= SYSFS_ROOT,
+};
+
 static int sysfs_fill_super(struct super_block *sb, void *data, int silent)
 {
 	struct inode *inode;
@@ -50,6 +57,7 @@
 		iput(inode);
 		return -ENOMEM;
 	}
+	root->d_fsdata = &sysfs_root;
 	sb->s_root = root;
 	return 0;
 }
diff -Nru a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
--- a/fs/sysfs/symlink.c	2004-11-01 13:37:18 -08:00
+++ b/fs/sysfs/symlink.c	2004-11-01 13:37:18 -08:00
@@ -55,6 +55,36 @@
 	}
 }
 
+static int sysfs_add_link(struct dentry * dentry, char * name, struct kobject * target)
+{
+	struct sysfs_dirent * parent_sd = dentry->d_parent->d_fsdata;
+	struct sysfs_symlink * sl;
+	int error = 0;
+
+	error = -ENOMEM;
+	sl = kmalloc(sizeof(*sl), GFP_KERNEL);
+	if (!sl)
+		goto exit1;
+
+	sl->link_name = kmalloc(strlen(name) + 1, GFP_KERNEL);
+	if (!sl->link_name)
+		goto exit2;
+
+	strcpy(sl->link_name, name);
+	sl->target_kobj = kobject_get(target);
+
+	error = sysfs_make_dirent(parent_sd, dentry, sl, S_IFLNK|S_IRWXUGO,
+				SYSFS_KOBJ_LINK);
+	if (!error)
+		return 0;
+
+	kfree(sl->link_name);
+exit2:
+	kfree(sl);
+exit1:
+	return error;
+}
+
 /**
  *	sysfs_create_link - create symlink between two objects.
  *	@kobj:	object whose directory we're creating the link in.
@@ -67,15 +97,16 @@
 	struct dentry * d;
 	int error = 0;
 
+	BUG_ON(!kobj || !kobj->dentry || !name);
+
 	down(&dentry->d_inode->i_sem);
 	d = sysfs_get_dentry(dentry,name);
 	if (!IS_ERR(d)) {
 		error = sysfs_create(d, S_IFLNK|S_IRWXUGO, init_symlink);
 		if (!error)
-			/* 
-			 * associate the link dentry with the target kobject 
-			 */
-			d->d_fsdata = kobject_get(target);
+			error = sysfs_add_link(d, name, target);
+		if (error)
+			d_drop(d);
 		dput(d);
 	} else 
 		error = PTR_ERR(d);
diff -Nru a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
--- a/fs/sysfs/sysfs.h	2004-11-01 13:37:18 -08:00
+++ b/fs/sysfs/sysfs.h	2004-11-01 13:37:18 -08:00
@@ -4,9 +4,11 @@
 extern struct inode * sysfs_new_inode(mode_t mode);
 extern int sysfs_create(struct dentry *, int mode, int (*init)(struct inode *));
 
+extern int sysfs_make_dirent(struct sysfs_dirent *, struct dentry *, void *,
+				umode_t, int);
 extern struct dentry * sysfs_get_dentry(struct dentry *, const char *);
 
-extern int sysfs_add_file(struct dentry * dir, const struct attribute * attr);
+extern int sysfs_add_file(struct dentry *, const struct attribute *, int);
 extern void sysfs_hash_and_remove(struct dentry * dir, const char * name);
 
 extern int sysfs_create_subdir(struct kobject *, const char *, struct dentry **);
@@ -16,19 +18,27 @@
 extern void sysfs_put_link(struct dentry *, struct nameidata *);
 extern struct rw_semaphore sysfs_rename_sem;
 
+struct sysfs_symlink {
+	char * link_name;
+	struct kobject * target_kobj;
+};
+
 static inline struct kobject * to_kobj(struct dentry * dentry)
 {
-	return ((struct kobject *) dentry->d_fsdata);
+	struct sysfs_dirent * sd = dentry->d_fsdata;
+	return ((struct kobject *) sd->s_element);
 }
 
 static inline struct attribute * to_attr(struct dentry * dentry)
 {
-	return ((struct attribute *) dentry->d_fsdata);
+	struct sysfs_dirent * sd = dentry->d_fsdata;
+	return ((struct attribute *) sd->s_element);
 }
 
 static inline struct bin_attribute * to_bin_attr(struct dentry * dentry)
 {
-	return ((struct bin_attribute *) dentry->d_fsdata);
+	struct sysfs_dirent * sd = dentry->d_fsdata;
+	return ((struct bin_attribute *) sd->s_element);
 }
 
 static inline struct kobject *sysfs_get_kobject(struct dentry *dentry)
@@ -36,10 +46,27 @@
 	struct kobject * kobj = NULL;
 
 	spin_lock(&dcache_lock);
-	if (!d_unhashed(dentry))
-		kobj = kobject_get(to_kobj(dentry));
+	if (!d_unhashed(dentry)) {
+		struct sysfs_dirent * sd = dentry->d_fsdata;
+		if (sd->s_type & SYSFS_KOBJ_LINK) {
+			struct sysfs_symlink * sl = sd->s_element;
+			kobj = kobject_get(sl->target_kobj);
+		} else
+			kobj = kobject_get(sd->s_element);
+	}
 	spin_unlock(&dcache_lock);
 
 	return kobj;
+}
+
+static inline void release_sysfs_dirent(struct sysfs_dirent * sd)
+{
+	if (sd->s_type & SYSFS_KOBJ_LINK) {
+		struct sysfs_symlink * sl = sd->s_element;
+		kfree(sl->link_name);
+		kobject_put(sl->target_kobj);
+		kfree(sl);
+	}
+	kfree(sd);
 }
 
diff -Nru a/include/linux/sysfs.h b/include/linux/sysfs.h
--- a/include/linux/sysfs.h	2004-11-01 13:37:18 -08:00
+++ b/include/linux/sysfs.h	2004-11-01 13:37:18 -08:00
@@ -9,6 +9,8 @@
 #ifndef _SYSFS_H_
 #define _SYSFS_H_
 
+#include <asm/atomic.h>
+
 struct kobject;
 struct module;
 
@@ -56,6 +58,23 @@
 	ssize_t	(*show)(struct kobject *, struct attribute *,char *);
 	ssize_t	(*store)(struct kobject *,struct attribute *,const char *, size_t);
 };
+
+struct sysfs_dirent {
+	atomic_t		s_count;
+	struct list_head	s_sibling;
+	struct list_head	s_children;
+	void 			* s_element;
+	int			s_type;
+	umode_t			s_mode;
+	struct dentry		* s_dentry;
+};
+
+#define SYSFS_ROOT		0x0001
+#define SYSFS_DIR		0x0002
+#define SYSFS_KOBJ_ATTR 	0x0004
+#define SYSFS_KOBJ_BIN_ATTR	0x0008
+#define SYSFS_KOBJ_LINK 	0x0020
+#define SYSFS_NOT_PINNED	(SYSFS_KOBJ_ATTR | SYSFS_KOBJ_BIN_ATTR | SYSFS_KOBJ_LINK)
 
 #ifdef CONFIG_SYSFS
 

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2444, 2004/11/01 13:04:09-08:00, akpm@osdl.org

[PATCH] sysfs backing store: stop pinning dentries/inodes for leaf entries

From: Maneesh Soni <maneesh@in.ibm.com>

o This patch stops the pinning of non-directory or leaf dentries and inodes.
  The leaf dentries and inodes are created during lookup based on the
  entries on sysfs_dirent tree. These leaves are removed from the dcache
  through the VFS dentry ageing process during shrink dcache operations. Thus
  reducing about 80% of sysfs lowmem needs.

o This implments the ->lookup() for sysfs directory inodes and allocates
  dentry and inode if the lookup is successful and avoids the need of
  allocating and pinning of dentry and inodes during the creation of
  corresponding sysfs leaf entry. As of now the implementation has not
  required negative dentry creation on failed lookup. As sysfs is still a
  RAM based filesystem, negative dentries are not of any use IMO.

o The leaf dentry allocated after successful lookup is connected to the
  existing corresponding sysfs_dirent through the d_fsdata field.

Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 fs/sysfs/bin.c     |   28 +-------------
 fs/sysfs/dir.c     |  106 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 fs/sysfs/file.c    |   25 +-----------
 fs/sysfs/mount.c   |    2 -
 fs/sysfs/symlink.c |   26 ++-----------
 fs/sysfs/sysfs.h   |    4 ++
 6 files changed, 115 insertions(+), 76 deletions(-)


diff -Nru a/fs/sysfs/bin.c b/fs/sysfs/bin.c
--- a/fs/sysfs/bin.c	2004-11-01 13:36:56 -08:00
+++ b/fs/sysfs/bin.c	2004-11-01 13:36:56 -08:00
@@ -141,7 +141,7 @@
 	return 0;
 }
 
-static struct file_operations bin_fops = {
+struct file_operations bin_fops = {
 	.read		= read,
 	.write		= write,
 	.llseek		= generic_file_llseek,
@@ -158,33 +158,9 @@
 
 int sysfs_create_bin_file(struct kobject * kobj, struct bin_attribute * attr)
 {
-	struct dentry * dentry;
-	struct dentry * parent;
-	umode_t mode = (attr->attr.mode & S_IALLUGO) | S_IFREG;
-	int error = 0;
-
 	BUG_ON(!kobj || !kobj->dentry || !attr);
 
-	parent = kobj->dentry;
-
-	down(&parent->d_inode->i_sem);
-	dentry = sysfs_get_dentry(parent,attr->attr.name);
-	if (!IS_ERR(dentry)) {
-		error = sysfs_create(dentry, mode, NULL);
-		if (!error) {
-			dentry->d_inode->i_size = attr->size;
-			dentry->d_inode->i_fop = &bin_fops;
-			error = sysfs_make_dirent(parent->d_fsdata, dentry,
-						  (void *) attr, mode,
-						  SYSFS_KOBJ_BIN_ATTR);
-		}
-		if (error)
-			d_drop(dentry);
-		dput(dentry);
-	} else
-		error = PTR_ERR(dentry);
-	up(&parent->d_inode->i_sem);
-	return error;
+	return sysfs_add_file(kobj->dentry, &attr->attr, SYSFS_KOBJ_BIN_ATTR);
 }
 
 
diff -Nru a/fs/sysfs/dir.c b/fs/sysfs/dir.c
--- a/fs/sysfs/dir.c	2004-11-01 13:36:56 -08:00
+++ b/fs/sysfs/dir.c	2004-11-01 13:36:56 -08:00
@@ -61,15 +61,17 @@
 	sd->s_mode = mode;
 	sd->s_type = type;
 	sd->s_dentry = dentry;
-	dentry->d_fsdata = sysfs_get(sd);
-	dentry->d_op = &sysfs_dentry_ops;
+	if (dentry) {
+		dentry->d_fsdata = sysfs_get(sd);
+		dentry->d_op = &sysfs_dentry_ops;
+	}
 
 	return 0;
 }
 
 static int init_dir(struct inode * inode)
 {
-	inode->i_op = &simple_dir_inode_operations;
+	inode->i_op = &sysfs_dir_inode_operations;
 	inode->i_fop = &sysfs_dir_operations;
 
 	/* directory inodes start off with i_nlink == 2 (for "." entry) */
@@ -77,6 +79,18 @@
 	return 0;
 }
 
+static int init_file(struct inode * inode)
+{
+	inode->i_size = PAGE_SIZE;
+	inode->i_fop = &sysfs_file_operations;
+	return 0;
+}
+
+static int init_symlink(struct inode * inode)
+{
+	inode->i_op = &sysfs_symlink_inode_operations;
+	return 0;
+}
 
 static int create_dir(struct kobject * k, struct dentry * p,
 		      const char * n, struct dentry ** d)
@@ -91,8 +105,11 @@
 		if (!error) {
 			error = sysfs_make_dirent(p->d_fsdata, *d, k, mode,
 						SYSFS_DIR);
-			if (!error)
+			if (!error) {
 				p->d_inode->i_nlink++;
+				(*d)->d_op = &sysfs_dentry_ops;
+				d_rehash(*d);
+			}
 		}
 		if (error)
 			d_drop(*d);
@@ -136,6 +153,82 @@
 	return error;
 }
 
+/* attaches attribute's sysfs_dirent to the dentry corresponding to the
+ * attribute file
+ */
+static int sysfs_attach_attr(struct sysfs_dirent * sd, struct dentry * dentry)
+{
+	struct attribute * attr = NULL;
+	struct bin_attribute * bin_attr = NULL;
+	int (* init) (struct inode *) = NULL;
+	int error = 0;
+
+        if (sd->s_type & SYSFS_KOBJ_BIN_ATTR) {
+                bin_attr = sd->s_element;
+                attr = &bin_attr->attr;
+        } else {
+                attr = sd->s_element;
+                init = init_file;
+        }
+
+	error = sysfs_create(dentry, (attr->mode & S_IALLUGO) | S_IFREG, init);
+	if (error)
+		return error;
+
+        if (bin_attr) {
+		dentry->d_inode->i_size = bin_attr->size;
+		dentry->d_inode->i_fop = &bin_fops;
+	}
+	dentry->d_op = &sysfs_dentry_ops;
+	dentry->d_fsdata = sysfs_get(sd);
+	sd->s_dentry = dentry;
+	d_rehash(dentry);
+
+	return 0;
+}
+
+static int sysfs_attach_link(struct sysfs_dirent * sd, struct dentry * dentry)
+{
+	int err = 0;
+
+	err = sysfs_create(dentry, S_IFLNK|S_IRWXUGO, init_symlink);
+	if (!err) {
+		dentry->d_op = &sysfs_dentry_ops;
+		dentry->d_fsdata = sysfs_get(sd);
+		sd->s_dentry = dentry;
+		d_rehash(dentry);
+	}
+	return err;
+}
+
+struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry,
+				struct nameidata *nd)
+{
+	struct sysfs_dirent * parent_sd = dentry->d_parent->d_fsdata;
+	struct sysfs_dirent * sd;
+	int err = 0;
+
+	list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
+		if (sd->s_type & SYSFS_NOT_PINNED) {
+			const unsigned char * name = sysfs_get_name(sd);
+
+			if (strcmp(name, dentry->d_name.name))
+				continue;
+
+			if (sd->s_type & SYSFS_KOBJ_LINK)
+				err = sysfs_attach_link(sd, dentry);
+			else
+				err = sysfs_attach_attr(sd, dentry);
+			break;
+		}
+	}
+
+	return ERR_PTR(err);
+}
+
+struct inode_operations sysfs_dir_inode_operations = {
+	.lookup		= sysfs_lookup,
+};
 
 static void remove_dir(struct dentry * d)
 {
@@ -219,8 +312,10 @@
 	if (!IS_ERR(new_dentry)) {
   		if (!new_dentry->d_inode) {
 			error = kobject_set_name(kobj, "%s", new_name);
-			if (!error)
+			if (!error) {
+				d_add(new_dentry, NULL);
 				d_move(kobj->dentry, new_dentry);
+			}
 			else
 				d_drop(new_dentry);
 		} else
@@ -254,7 +349,6 @@
 	down(&dentry->d_inode->i_sem);
 	list_del_init(&cursor->s_sibling);
 	up(&dentry->d_inode->i_sem);
-	sysfs_put(file->private_data);
 
 	return 0;
 }
diff -Nru a/fs/sysfs/file.c b/fs/sysfs/file.c
--- a/fs/sysfs/file.c	2004-11-01 13:36:56 -08:00
+++ b/fs/sysfs/file.c	2004-11-01 13:36:56 -08:00
@@ -9,15 +9,6 @@
 
 #include "sysfs.h"
 
-static struct file_operations sysfs_file_operations;
-
-static int init_file(struct inode * inode)
-{
-	inode->i_size = PAGE_SIZE;
-	inode->i_fop = &sysfs_file_operations;
-	return 0;
-}
-
 #define to_subsys(k) container_of(k,struct subsystem,kset.kobj)
 #define to_sattr(a) container_of(a,struct subsys_attribute,attr)
 
@@ -337,7 +328,7 @@
 	return 0;
 }
 
-static struct file_operations sysfs_file_operations = {
+struct file_operations sysfs_file_operations = {
 	.read		= sysfs_read_file,
 	.write		= sysfs_write_file,
 	.llseek		= generic_file_llseek,
@@ -348,24 +339,14 @@
 
 int sysfs_add_file(struct dentry * dir, const struct attribute * attr, int type)
 {
-	struct dentry * dentry;
 	struct sysfs_dirent * parent_sd = dir->d_fsdata;
 	umode_t mode = (attr->mode & S_IALLUGO) | S_IFREG;
 	int error = 0;
 
 	down(&dir->d_inode->i_sem);
-	dentry = sysfs_get_dentry(dir,attr->name);
-	if (!IS_ERR(dentry)) {
-		error = sysfs_create(dentry, mode, init_file);
-		if (!error)
-			error = sysfs_make_dirent(parent_sd, dentry,
-						(void *) attr, mode, type);
-		if (error)
-			d_drop(dentry);
-		dput(dentry);
-	} else
-		error = PTR_ERR(dentry);
+	error = sysfs_make_dirent(parent_sd, NULL, (void *) attr, mode, type);
 	up(&dir->d_inode->i_sem);
+
 	return error;
 }
 
diff -Nru a/fs/sysfs/mount.c b/fs/sysfs/mount.c
--- a/fs/sysfs/mount.c	2004-11-01 13:36:56 -08:00
+++ b/fs/sysfs/mount.c	2004-11-01 13:36:56 -08:00
@@ -42,7 +42,7 @@
 
 	inode = sysfs_new_inode(S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO);
 	if (inode) {
-		inode->i_op = &simple_dir_inode_operations;
+		inode->i_op = &sysfs_dir_inode_operations;
 		inode->i_fop = &sysfs_dir_operations;
 		/* directory inodes start off with i_nlink == 2 (for "." entry) */
 		inode->i_nlink++;	
diff -Nru a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
--- a/fs/sysfs/symlink.c	2004-11-01 13:36:56 -08:00
+++ b/fs/sysfs/symlink.c	2004-11-01 13:36:56 -08:00
@@ -9,18 +9,12 @@
 
 #include "sysfs.h"
 
-static struct inode_operations sysfs_symlink_inode_operations = {
+struct inode_operations sysfs_symlink_inode_operations = {
 	.readlink = generic_readlink,
 	.follow_link = sysfs_follow_link,
 	.put_link = sysfs_put_link,
 };
 
-static int init_symlink(struct inode * inode)
-{
-	inode->i_op = &sysfs_symlink_inode_operations;
-	return 0;
-}
-
 static int object_depth(struct kobject * kobj)
 {
 	struct kobject * p = kobj;
@@ -55,9 +49,9 @@
 	}
 }
 
-static int sysfs_add_link(struct dentry * dentry, char * name, struct kobject * target)
+static int sysfs_add_link(struct dentry * parent, char * name, struct kobject * target)
 {
-	struct sysfs_dirent * parent_sd = dentry->d_parent->d_fsdata;
+	struct sysfs_dirent * parent_sd = parent->d_fsdata;
 	struct sysfs_symlink * sl;
 	int error = 0;
 
@@ -73,7 +67,7 @@
 	strcpy(sl->link_name, name);
 	sl->target_kobj = kobject_get(target);
 
-	error = sysfs_make_dirent(parent_sd, dentry, sl, S_IFLNK|S_IRWXUGO,
+	error = sysfs_make_dirent(parent_sd, NULL, sl, S_IFLNK|S_IRWXUGO,
 				SYSFS_KOBJ_LINK);
 	if (!error)
 		return 0;
@@ -94,22 +88,12 @@
 int sysfs_create_link(struct kobject * kobj, struct kobject * target, char * name)
 {
 	struct dentry * dentry = kobj->dentry;
-	struct dentry * d;
 	int error = 0;
 
 	BUG_ON(!kobj || !kobj->dentry || !name);
 
 	down(&dentry->d_inode->i_sem);
-	d = sysfs_get_dentry(dentry,name);
-	if (!IS_ERR(d)) {
-		error = sysfs_create(d, S_IFLNK|S_IRWXUGO, init_symlink);
-		if (!error)
-			error = sysfs_add_link(d, name, target);
-		if (error)
-			d_drop(d);
-		dput(d);
-	} else 
-		error = PTR_ERR(d);
+	error = sysfs_add_link(dentry, name, target);
 	up(&dentry->d_inode->i_sem);
 	return error;
 }
diff -Nru a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
--- a/fs/sysfs/sysfs.h	2004-11-01 13:36:56 -08:00
+++ b/fs/sysfs/sysfs.h	2004-11-01 13:36:56 -08:00
@@ -22,6 +22,10 @@
 extern struct rw_semaphore sysfs_rename_sem;
 extern struct super_block * sysfs_sb;
 extern struct file_operations sysfs_dir_operations;
+extern struct file_operations sysfs_file_operations;
+extern struct file_operations bin_fops;
+extern struct inode_operations sysfs_dir_inode_operations;
+extern struct inode_operations sysfs_symlink_inode_operations;
 
 struct sysfs_symlink {
 	char * link_name;

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2446, 2004/11/01 13:04:46-08:00, akpm@osdl.org

[PATCH] Possible race in sysfs_read_file() and sysfs_write_file()

From: Simon Derr <Simon.Derr@bull.net>

Add a `needs_read_fill' field in sysfs_buffer so that reading after a write in
a sysfs file returns valid data.

(instead of the data that have been written, that may be invalid or at the
wrong offset)

Signed-off-by: Simon Derr <simon.derr@bull.net>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 fs/sysfs/file.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletion(-)


diff -Nru a/fs/sysfs/file.c b/fs/sysfs/file.c
--- a/fs/sysfs/file.c	2004-11-01 13:36:42 -08:00
+++ b/fs/sysfs/file.c	2004-11-01 13:36:42 -08:00
@@ -55,6 +55,7 @@
 	char			* page;
 	struct sysfs_ops	* ops;
 	struct semaphore	sem;
+	int			needs_read_fill;
 };
 
 
@@ -82,6 +83,7 @@
 		return -ENOMEM;
 
 	count = ops->show(kobj,attr,buffer->page);
+	buffer->needs_read_fill = 0;
 	BUG_ON(count > (ssize_t)PAGE_SIZE);
 	if (count >= 0)
 		buffer->count = count;
@@ -146,7 +148,7 @@
 	ssize_t retval = 0;
 
 	down(&buffer->sem);
-	if ((!*ppos) || (!buffer->page)) {
+	if (buffer->needs_read_fill) {
 		if ((retval = fill_read_buffer(file->f_dentry,buffer)))
 			goto out;
 	}
@@ -182,6 +184,7 @@
 	if (count >= PAGE_SIZE)
 		count = PAGE_SIZE - 1;
 	error = copy_from_user(buffer->page,buf,count);
+	buffer->needs_read_fill = 1;
 	return error ? -EFAULT : count;
 }
 
@@ -299,6 +302,7 @@
 	if (buffer) {
 		memset(buffer,0,sizeof(struct sysfs_buffer));
 		init_MUTEX(&buffer->sem);
+		buffer->needs_read_fill = 1;
 		buffer->ops = ops;
 		file->private_data = buffer;
 	} else

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2443, 2004/11/01 13:03:53-08:00, akpm@osdl.org

[PATCH] sysfs backing store: use sysfs_dirent based tree in dir file operations

From: Maneesh Soni <maneesh@in.ibm.com>

o This patch implements the sysfs_dir_operations file_operations strucutre for
  sysfs directories. It uses the sysfs_dirent based tree for ->readdir() and
  ->lseek() methods instead of simple_dir_operations which use dentry based
  tree.

Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 fs/sysfs/dir.c   |  141 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 fs/sysfs/mount.c |    2 
 fs/sysfs/sysfs.h |    2 
 3 files changed, 143 insertions(+), 2 deletions(-)


diff -Nru a/fs/sysfs/dir.c b/fs/sysfs/dir.c
--- a/fs/sysfs/dir.c	2004-11-01 13:37:04 -08:00
+++ b/fs/sysfs/dir.c	2004-11-01 13:37:04 -08:00
@@ -70,7 +70,7 @@
 static int init_dir(struct inode * inode)
 {
 	inode->i_op = &simple_dir_inode_operations;
-	inode->i_fop = &simple_dir_operations;
+	inode->i_fop = &sysfs_dir_operations;
 
 	/* directory inodes start off with i_nlink == 2 (for "." entry) */
 	inode->i_nlink++;
@@ -232,6 +232,145 @@
 
 	return error;
 }
+
+static int sysfs_dir_open(struct inode *inode, struct file *file)
+{
+	struct dentry * dentry = file->f_dentry;
+	struct sysfs_dirent * parent_sd = dentry->d_fsdata;
+
+	down(&dentry->d_inode->i_sem);
+	file->private_data = sysfs_new_dirent(parent_sd, NULL);
+	up(&dentry->d_inode->i_sem);
+
+	return file->private_data ? 0 : -ENOMEM;
+
+}
+
+static int sysfs_dir_close(struct inode *inode, struct file *file)
+{
+	struct dentry * dentry = file->f_dentry;
+	struct sysfs_dirent * cursor = file->private_data;
+
+	down(&dentry->d_inode->i_sem);
+	list_del_init(&cursor->s_sibling);
+	up(&dentry->d_inode->i_sem);
+	sysfs_put(file->private_data);
+
+	return 0;
+}
+
+/* Relationship between s_mode and the DT_xxx types */
+static inline unsigned char dt_type(struct sysfs_dirent *sd)
+{
+	return (sd->s_mode >> 12) & 15;
+}
+
+static int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir)
+{
+	struct dentry *dentry = filp->f_dentry;
+	struct sysfs_dirent * parent_sd = dentry->d_fsdata;
+	struct sysfs_dirent *cursor = filp->private_data;
+	struct list_head *p, *q = &cursor->s_sibling;
+	ino_t ino;
+	int i = filp->f_pos;
+
+	switch (i) {
+		case 0:
+			ino = dentry->d_inode->i_ino;
+			if (filldir(dirent, ".", 1, i, ino, DT_DIR) < 0)
+				break;
+			filp->f_pos++;
+			i++;
+			/* fallthrough */
+		case 1:
+			ino = parent_ino(dentry);
+			if (filldir(dirent, "..", 2, i, ino, DT_DIR) < 0)
+				break;
+			filp->f_pos++;
+			i++;
+			/* fallthrough */
+		default:
+			if (filp->f_pos == 2) {
+				list_del(q);
+				list_add(q, &parent_sd->s_children);
+			}
+			for (p=q->next; p!= &parent_sd->s_children; p=p->next) {
+				struct sysfs_dirent *next;
+				const char * name;
+				int len;
+
+				next = list_entry(p, struct sysfs_dirent,
+						   s_sibling);
+				if (!next->s_element)
+					continue;
+
+				name = sysfs_get_name(next);
+				len = strlen(name);
+				if (next->s_dentry)
+					ino = next->s_dentry->d_inode->i_ino;
+				else
+					ino = iunique(sysfs_sb, 2);
+
+				if (filldir(dirent, name, len, filp->f_pos, ino,
+						 dt_type(next)) < 0)
+					return 0;
+
+				list_del(q);
+				list_add(q, p);
+				p = q;
+				filp->f_pos++;
+			}
+	}
+	return 0;
+}
+
+static loff_t sysfs_dir_lseek(struct file * file, loff_t offset, int origin)
+{
+	struct dentry * dentry = file->f_dentry;
+
+	down(&dentry->d_inode->i_sem);
+	switch (origin) {
+		case 1:
+			offset += file->f_pos;
+		case 0:
+			if (offset >= 0)
+				break;
+		default:
+			up(&file->f_dentry->d_inode->i_sem);
+			return -EINVAL;
+	}
+	if (offset != file->f_pos) {
+		file->f_pos = offset;
+		if (file->f_pos >= 2) {
+			struct sysfs_dirent *sd = dentry->d_fsdata;
+			struct sysfs_dirent *cursor = file->private_data;
+			struct list_head *p;
+			loff_t n = file->f_pos - 2;
+
+			list_del(&cursor->s_sibling);
+			p = sd->s_children.next;
+			while (n && p != &sd->s_children) {
+				struct sysfs_dirent *next;
+				next = list_entry(p, struct sysfs_dirent,
+						   s_sibling);
+				if (next->s_element)
+					n--;
+				p = p->next;
+			}
+			list_add_tail(&cursor->s_sibling, p);
+		}
+	}
+	up(&dentry->d_inode->i_sem);
+	return offset;
+}
+
+struct file_operations sysfs_dir_operations = {
+	.open		= sysfs_dir_open,
+	.release	= sysfs_dir_close,
+	.llseek		= sysfs_dir_lseek,
+	.read		= generic_read_dir,
+	.readdir	= sysfs_readdir,
+};
 
 EXPORT_SYMBOL_GPL(sysfs_create_dir);
 EXPORT_SYMBOL_GPL(sysfs_remove_dir);
diff -Nru a/fs/sysfs/mount.c b/fs/sysfs/mount.c
--- a/fs/sysfs/mount.c	2004-11-01 13:37:04 -08:00
+++ b/fs/sysfs/mount.c	2004-11-01 13:37:04 -08:00
@@ -43,7 +43,7 @@
 	inode = sysfs_new_inode(S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO);
 	if (inode) {
 		inode->i_op = &simple_dir_inode_operations;
-		inode->i_fop = &simple_dir_operations;
+		inode->i_fop = &sysfs_dir_operations;
 		/* directory inodes start off with i_nlink == 2 (for "." entry) */
 		inode->i_nlink++;	
 	} else {
diff -Nru a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
--- a/fs/sysfs/sysfs.h	2004-11-01 13:37:04 -08:00
+++ b/fs/sysfs/sysfs.h	2004-11-01 13:37:04 -08:00
@@ -20,6 +20,8 @@
 extern int sysfs_follow_link(struct dentry *, struct nameidata *);
 extern void sysfs_put_link(struct dentry *, struct nameidata *);
 extern struct rw_semaphore sysfs_rename_sem;
+extern struct super_block * sysfs_sb;
+extern struct file_operations sysfs_dir_operations;
 
 struct sysfs_symlink {
 	char * link_name;

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2445, 2004/11/01 13:04:30-08:00, akpm@osdl.org

[PATCH] Fix race in sysfs_read_file() and sysfs_write_file()

From: Simon Derr <Simon.Derr@bull.net>

- fixes the race between threads by adding a semaphore in sysfs_buffer

- allocates the buffer upon call to pread().  We still call again
  fill_read_buffer() if the file is "rewinded" to offset zero.

- fixes the comparison in flush_read_buffer().

Signed-off-by: Simon Derr <simon.derr@bull.net>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 fs/sysfs/file.c |   18 +++++++++++++++---
 1 files changed, 15 insertions(+), 3 deletions(-)


diff -Nru a/fs/sysfs/file.c b/fs/sysfs/file.c
--- a/fs/sysfs/file.c	2004-11-01 13:36:49 -08:00
+++ b/fs/sysfs/file.c	2004-11-01 13:36:49 -08:00
@@ -6,6 +6,7 @@
 #include <linux/dnotify.h>
 #include <linux/kobject.h>
 #include <asm/uaccess.h>
+#include <asm/semaphore.h>
 
 #include "sysfs.h"
 
@@ -53,6 +54,7 @@
 	loff_t			pos;
 	char			* page;
 	struct sysfs_ops	* ops;
+	struct semaphore	sem;
 };
 
 
@@ -106,6 +108,9 @@
 {
 	int error;
 
+	if (*ppos > buffer->count)
+		return 0;
+
 	if (count > (buffer->count - *ppos))
 		count = buffer->count - *ppos;
 
@@ -140,13 +145,17 @@
 	struct sysfs_buffer * buffer = file->private_data;
 	ssize_t retval = 0;
 
-	if (!*ppos) {
+	down(&buffer->sem);
+	if ((!*ppos) || (!buffer->page)) {
 		if ((retval = fill_read_buffer(file->f_dentry,buffer)))
-			return retval;
+			goto out;
 	}
 	pr_debug("%s: count = %d, ppos = %lld, buf = %s\n",
 		 __FUNCTION__,count,*ppos,buffer->page);
-	return flush_read_buffer(buffer,buf,count,ppos);
+	retval = flush_read_buffer(buffer,buf,count,ppos);
+out:
+	up(&buffer->sem);
+	return retval;
 }
 
 
@@ -220,11 +229,13 @@
 {
 	struct sysfs_buffer * buffer = file->private_data;
 
+	down(&buffer->sem);
 	count = fill_write_buffer(buffer,buf,count);
 	if (count > 0)
 		count = flush_write_buffer(file->f_dentry,buffer,count);
 	if (count > 0)
 		*ppos += count;
+	up(&buffer->sem);
 	return count;
 }
 
@@ -287,6 +298,7 @@
 	buffer = kmalloc(sizeof(struct sysfs_buffer),GFP_KERNEL);
 	if (buffer) {
 		memset(buffer,0,sizeof(struct sysfs_buffer));
+		init_MUTEX(&buffer->sem);
 		buffer->ops = ops;
 		file->private_data = buffer;
 	} else

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2442, 2004/11/01 13:03:33-08:00, akpm@osdl.org

[PATCH] sysfs backing store: use sysfs_dirent based tree in file removal

From: Maneesh Soni <maneesh@in.ibm.com>

o This patch uses the sysfs_dirent based tree while removing sysfs files
  and directories. This avoids holding dcache_lock by not using dentry
  based vfs tree. Thus simplyfying the removal logic in sysfs.

o It uses two helper routines sysfs_get_name(), to get the name for
  sysfs element and sysfs_drop_dentry() to delete the dentry given a
  sysfs_dirent.

Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 fs/sysfs/dir.c   |   48 ++++++----------------------
 fs/sysfs/inode.c |   92 +++++++++++++++++++++++++++++++++++++++----------------
 fs/sysfs/sysfs.h |   18 ++++++++++
 3 files changed, 95 insertions(+), 63 deletions(-)


diff -Nru a/fs/sysfs/dir.c b/fs/sysfs/dir.c
--- a/fs/sysfs/dir.c	2004-11-01 13:37:11 -08:00
+++ b/fs/sysfs/dir.c	2004-11-01 13:37:11 -08:00
@@ -19,7 +19,7 @@
 	if (sd) {
 		BUG_ON(sd->s_dentry != dentry);
 		sd->s_dentry = NULL;
-		release_sysfs_dirent(sd);
+		sysfs_put(sd);
 	}
 	iput(inode);
 }
@@ -61,7 +61,7 @@
 	sd->s_mode = mode;
 	sd->s_type = type;
 	sd->s_dentry = dentry;
-	dentry->d_fsdata = sd;
+	dentry->d_fsdata = sysfs_get(sd);
 	dentry->d_op = &sysfs_dentry_ops;
 
 	return 0;
@@ -146,6 +146,7 @@
 	d_delete(d);
 	sd = d->d_fsdata;
  	list_del_init(&sd->s_sibling);
+	sysfs_put(sd);
 	if (d->d_inode)
 		simple_rmdir(parent->d_inode,d);
 
@@ -173,49 +174,22 @@
 
 void sysfs_remove_dir(struct kobject * kobj)
 {
-	struct list_head * node;
 	struct dentry * dentry = dget(kobj->dentry);
+	struct sysfs_dirent * parent_sd = dentry->d_fsdata;
+	struct sysfs_dirent * sd, * tmp;
 
 	if (!dentry)
 		return;
 
 	pr_debug("sysfs %s: removing dir\n",dentry->d_name.name);
 	down(&dentry->d_inode->i_sem);
-
-	spin_lock(&dcache_lock);
-restart:
-	node = dentry->d_subdirs.next;
-	while (node != &dentry->d_subdirs) {
-		struct dentry * d = list_entry(node,struct dentry,d_child);
-
-		node = node->next;
-		pr_debug(" o %s (%d): ",d->d_name.name,atomic_read(&d->d_count));
-		if (!d_unhashed(d) && (d->d_inode)) {
-			struct sysfs_dirent * sd = d->d_fsdata;
-			d = dget_locked(d);
-			pr_debug("removing");
-
-			/**
-			 * Unlink and unhash.
-			 */
-			__d_drop(d);
-			spin_unlock(&dcache_lock);
-			/* release the target kobject in case of 
-			 * a symlink
-			 */
-			if (S_ISLNK(d->d_inode->i_mode))
-				kobject_put(sd->s_element);
-			
-			list_del_init(&sd->s_sibling);
-			simple_unlink(dentry->d_inode,d);
-			dput(d);
-			pr_debug(" done\n");
-			spin_lock(&dcache_lock);
-			/* re-acquired dcache_lock, need to restart */
-			goto restart;
-		}
+	list_for_each_entry_safe(sd, tmp, &parent_sd->s_children, s_sibling) {
+		if (!sd->s_element)
+			continue;
+		list_del_init(&sd->s_sibling);
+		sysfs_drop_dentry(sd, dentry);
+		sysfs_put(sd);
 	}
-	spin_unlock(&dcache_lock);
 	up(&dentry->d_inode->i_sem);
 
 	remove_dir(dentry);
diff -Nru a/fs/sysfs/inode.c b/fs/sysfs/inode.c
--- a/fs/sysfs/inode.c	2004-11-01 13:37:11 -08:00
+++ b/fs/sysfs/inode.c	2004-11-01 13:37:11 -08:00
@@ -31,8 +31,8 @@
 	struct inode * inode = new_inode(sysfs_sb);
 	if (inode) {
 		inode->i_mode = mode;
-		inode->i_uid = current->fsuid;
-		inode->i_gid = current->fsgid;
+		inode->i_uid = 0;
+		inode->i_gid = 0;
 		inode->i_blksize = PAGE_CACHE_SIZE;
 		inode->i_blocks = 0;
 		inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
@@ -68,7 +68,8 @@
 		error = init(inode);
 	if (!error) {
 		d_instantiate(dentry, inode);
-		dget(dentry); /* Extra count - pin the dentry in core */
+		if (S_ISDIR(mode))
+			dget(dentry);  /* pin only directory dentry in core */
 	} else
 		iput(inode);
  Done:
@@ -90,35 +91,74 @@
 	return lookup_hash(&qstr,parent);
 }
 
+/*
+ * Get the name for corresponding element represented by the given sysfs_dirent
+ */
+const unsigned char * sysfs_get_name(struct sysfs_dirent *sd)
+{
+	struct attribute * attr;
+	struct bin_attribute * bin_attr;
+	struct sysfs_symlink  * sl;
+
+	if (!sd || !sd->s_element)
+		BUG();
+
+	switch (sd->s_type) {
+		case SYSFS_DIR:
+			/* Always have a dentry so use that */
+			return sd->s_dentry->d_name.name;
+
+		case SYSFS_KOBJ_ATTR:
+			attr = sd->s_element;
+			return attr->name;
+
+		case SYSFS_KOBJ_BIN_ATTR:
+			bin_attr = sd->s_element;
+			return bin_attr->attr.name;
+
+		case SYSFS_KOBJ_LINK:
+			sl = sd->s_element;
+			return sl->link_name;
+	}
+	return NULL;
+}
+
+
+/*
+ * Unhashes the dentry corresponding to given sysfs_dirent
+ * Called with parent inode's i_sem held.
+ */
+void sysfs_drop_dentry(struct sysfs_dirent * sd, struct dentry * parent)
+{
+	struct dentry * dentry = sd->s_dentry;
+
+	if (dentry) {
+		spin_lock(&dcache_lock);
+		if (!(d_unhashed(dentry) && dentry->d_inode)) {
+			dget_locked(dentry);
+			__d_drop(dentry);
+			spin_unlock(&dcache_lock);
+			simple_unlink(parent->d_inode, dentry);
+		} else
+			spin_unlock(&dcache_lock);
+	}
+}
+
 void sysfs_hash_and_remove(struct dentry * dir, const char * name)
 {
-	struct dentry * victim;
 	struct sysfs_dirent * sd;
+	struct sysfs_dirent * parent_sd = dir->d_fsdata;
 
 	down(&dir->d_inode->i_sem);
-	victim = sysfs_get_dentry(dir,name);
-	if (!IS_ERR(victim)) {
-		/* make sure dentry is really there */
-		if (victim->d_inode && 
-		    (victim->d_parent->d_inode == dir->d_inode)) {
-			pr_debug("sysfs: Removing %s (%d)\n", victim->d_name.name,
-				 atomic_read(&victim->d_count));
-
-			sd = victim->d_fsdata;
-			d_drop(victim);
-			/* release the target kobject in case of 
-			 * a symlink
-			 */
-			if (S_ISLNK(victim->d_inode->i_mode))
-				kobject_put(sd->s_element);
+	list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
+		if (!sd->s_element)
+			continue;
+		if (!strcmp(sysfs_get_name(sd), name)) {
 			list_del_init(&sd->s_sibling);
-			simple_unlink(dir->d_inode,victim);
-		} else
-			d_drop(victim);
-		/*
-		 * Drop reference from sysfs_get_dentry() above.
-		 */
-		dput(victim);
+			sysfs_drop_dentry(sd, dir);
+			sysfs_put(sd);
+			break;
+		}
 	}
 	up(&dir->d_inode->i_sem);
 }
diff -Nru a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
--- a/fs/sysfs/sysfs.h	2004-11-01 13:37:11 -08:00
+++ b/fs/sysfs/sysfs.h	2004-11-01 13:37:11 -08:00
@@ -14,6 +14,9 @@
 extern int sysfs_create_subdir(struct kobject *, const char *, struct dentry **);
 extern void sysfs_remove_subdir(struct dentry *);
 
+extern const unsigned char * sysfs_get_name(struct sysfs_dirent *sd);
+extern void sysfs_drop_dentry(struct sysfs_dirent *sd, struct dentry *parent);
+
 extern int sysfs_follow_link(struct dentry *, struct nameidata *);
 extern void sysfs_put_link(struct dentry *, struct nameidata *);
 extern struct rw_semaphore sysfs_rename_sem;
@@ -68,5 +71,20 @@
 		kfree(sl);
 	}
 	kfree(sd);
+}
+
+static inline struct sysfs_dirent * sysfs_get(struct sysfs_dirent * sd)
+{
+	if (sd) {
+		WARN_ON(!atomic_read(&sd->s_count));
+		atomic_inc(&sd->s_count);
+	}
+	return sd;
+}
+
+static inline void sysfs_put(struct sysfs_dirent * sd)
+{
+	if (atomic_dec_and_test(&sd->s_count))
+		release_sysfs_dirent(sd);
 }
 

[PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2439, 2004/11/01 13:02:34-08:00, akpm@osdl.org

[PATCH] sysfs backing store - prepare sysfs_file_operations helpers

From: Maneesh Soni <maneesh@in.ibm.com>

o The following patch provides dumb helpers to access the corresponding
  kobject, attribute or binary attribute given a dentry and prepare the
  sysfs_file_operation methods for using sysfs_dirents.

Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 fs/sysfs/bin.c   |   14 +++++++-------
 fs/sysfs/file.c  |   24 ++++++++++++------------
 fs/sysfs/sysfs.h |   18 +++++++++++++++++-
 3 files changed, 36 insertions(+), 20 deletions(-)


diff -Nru a/fs/sysfs/bin.c b/fs/sysfs/bin.c
--- a/fs/sysfs/bin.c	2004-11-01 13:37:33 -08:00
+++ b/fs/sysfs/bin.c	2004-11-01 13:37:33 -08:00
@@ -17,8 +17,8 @@
 static int
 fill_read(struct dentry *dentry, char *buffer, loff_t off, size_t count)
 {
-	struct bin_attribute * attr = dentry->d_fsdata;
-	struct kobject * kobj = dentry->d_parent->d_fsdata;
+	struct bin_attribute * attr = to_bin_attr(dentry);
+	struct kobject * kobj = to_kobj(dentry->d_parent);
 
 	return attr->read(kobj, buffer, off, count);
 }
@@ -60,8 +60,8 @@
 static int
 flush_write(struct dentry *dentry, char *buffer, loff_t offset, size_t count)
 {
-	struct bin_attribute *attr = dentry->d_fsdata;
-	struct kobject *kobj = dentry->d_parent->d_fsdata;
+	struct bin_attribute *attr = to_bin_attr(dentry->d_parent);
+	struct kobject *kobj = to_kobj(dentry);
 
 	return attr->write(kobj, buffer, offset, count);
 }
@@ -95,7 +95,7 @@
 static int open(struct inode * inode, struct file * file)
 {
 	struct kobject *kobj = sysfs_get_kobject(file->f_dentry->d_parent);
-	struct bin_attribute * attr = file->f_dentry->d_fsdata;
+	struct bin_attribute * attr = to_bin_attr(file->f_dentry);
 	int error = -EINVAL;
 
 	if (!kobj || !attr)
@@ -130,8 +130,8 @@
 
 static int release(struct inode * inode, struct file * file)
 {
-	struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
-	struct bin_attribute * attr = file->f_dentry->d_fsdata;
+	struct kobject * kobj = to_kobj(file->f_dentry->d_parent);
+	struct bin_attribute * attr = to_bin_attr(file->f_dentry);
 	u8 * buffer = file->private_data;
 
 	if (kobj) 
diff -Nru a/fs/sysfs/file.c b/fs/sysfs/file.c
--- a/fs/sysfs/file.c	2004-11-01 13:37:33 -08:00
+++ b/fs/sysfs/file.c	2004-11-01 13:37:33 -08:00
@@ -67,7 +67,7 @@
 
 /**
  *	fill_read_buffer - allocate and fill buffer from object.
- *	@file:		file pointer.
+ *	@dentry:	dentry pointer.
  *	@buffer:	data buffer for file.
  *
  *	Allocate @buffer->page, if it hasn't been already, then call the
@@ -75,10 +75,10 @@
  *	data. 
  *	This is called only once, on the file's first read. 
  */
-static int fill_read_buffer(struct file * file, struct sysfs_buffer * buffer)
+static int fill_read_buffer(struct dentry * dentry, struct sysfs_buffer * buffer)
 {
-	struct attribute * attr = file->f_dentry->d_fsdata;
-	struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
+	struct attribute * attr = to_attr(dentry);
+	struct kobject * kobj = to_kobj(dentry->d_parent);
 	struct sysfs_ops * ops = buffer->ops;
 	int ret = 0;
 	ssize_t count;
@@ -150,7 +150,7 @@
 	ssize_t retval = 0;
 
 	if (!*ppos) {
-		if ((retval = fill_read_buffer(file,buffer)))
+		if ((retval = fill_read_buffer(file->f_dentry,buffer)))
 			return retval;
 	}
 	pr_debug("%s: count = %d, ppos = %lld, buf = %s\n",
@@ -197,10 +197,10 @@
  */
 
 static int 
-flush_write_buffer(struct file * file, struct sysfs_buffer * buffer, size_t count)
+flush_write_buffer(struct dentry * dentry, struct sysfs_buffer * buffer, size_t count)
 {
-	struct attribute * attr = file->f_dentry->d_fsdata;
-	struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
+	struct attribute * attr = to_attr(dentry);
+	struct kobject * kobj = to_kobj(dentry->d_parent);
 	struct sysfs_ops * ops = buffer->ops;
 
 	return ops->store(kobj,attr,buffer->page,count);
@@ -231,7 +231,7 @@
 
 	count = fill_write_buffer(buffer,buf,count);
 	if (count > 0)
-		count = flush_write_buffer(file,buffer,count);
+		count = flush_write_buffer(file->f_dentry,buffer,count);
 	if (count > 0)
 		*ppos += count;
 	return count;
@@ -240,7 +240,7 @@
 static int check_perm(struct inode * inode, struct file * file)
 {
 	struct kobject *kobj = sysfs_get_kobject(file->f_dentry->d_parent);
-	struct attribute * attr = file->f_dentry->d_fsdata;
+	struct attribute * attr = to_attr(file->f_dentry);
 	struct sysfs_buffer * buffer;
 	struct sysfs_ops * ops = NULL;
 	int error = 0;
@@ -321,8 +321,8 @@
 
 static int sysfs_release(struct inode * inode, struct file * filp)
 {
-	struct kobject * kobj = filp->f_dentry->d_parent->d_fsdata;
-	struct attribute * attr = filp->f_dentry->d_fsdata;
+	struct kobject * kobj = to_kobj(filp->f_dentry->d_parent);
+	struct attribute * attr = to_attr(filp->f_dentry);
 	struct sysfs_buffer * buffer = filp->private_data;
 
 	if (kobj) 
diff -Nru a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
--- a/fs/sysfs/sysfs.h	2004-11-01 13:37:33 -08:00
+++ b/fs/sysfs/sysfs.h	2004-11-01 13:37:33 -08:00
@@ -16,14 +16,30 @@
 extern void sysfs_put_link(struct dentry *, struct nameidata *);
 extern struct rw_semaphore sysfs_rename_sem;
 
+static inline struct kobject * to_kobj(struct dentry * dentry)
+{
+	return ((struct kobject *) dentry->d_fsdata);
+}
+
+static inline struct attribute * to_attr(struct dentry * dentry)
+{
+	return ((struct attribute *) dentry->d_fsdata);
+}
+
+static inline struct bin_attribute * to_bin_attr(struct dentry * dentry)
+{
+	return ((struct bin_attribute *) dentry->d_fsdata);
+}
+
 static inline struct kobject *sysfs_get_kobject(struct dentry *dentry)
 {
 	struct kobject * kobj = NULL;
 
 	spin_lock(&dcache_lock);
 	if (!d_unhashed(dentry))
-		kobj = kobject_get(dentry->d_fsdata);
+		kobj = kobject_get(to_kobj(dentry));
 	spin_unlock(&dcache_lock);
 
 	return kobj;
 }
+

Re: [PATCH] Driver Core patches for 2.6.10-rc1

[Greg KH]

ChangeSet 1.2440, 2004/11/01 13:02:55-08:00, akpm@osdl.org

[PATCH] fix oops with firmware loading

From: Maneesh Soni <maneesh@in.ibm.com>

My fault, a bad typo in fs/sysfs/bin.c.

Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 fs/sysfs/bin.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)


diff -Nru a/fs/sysfs/bin.c b/fs/sysfs/bin.c
--- a/fs/sysfs/bin.c	2004-11-01 13:37:25 -08:00
+++ b/fs/sysfs/bin.c	2004-11-01 13:37:25 -08:00
@@ -60,8 +60,8 @@
 static int
 flush_write(struct dentry *dentry, char *buffer, loff_t offset, size_t count)
 {
-	struct bin_attribute *attr = to_bin_attr(dentry->d_parent);
-	struct kobject *kobj = to_kobj(dentry);
+	struct bin_attribute *attr = to_bin_attr(dentry);
+	struct kobject *kobj = to_kobj(dentry->d_parent);
 
 	return attr->write(kobj, buffer, offset, count);
 }