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
ChangeSet 1.2023, 2004/10/22 15:43:17-07:00, [email protected]
hotplug: prevent skips in sequence number from happening
Signed-off-by: Greg Kroah-Hartman <[email protected]>
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);
ChangeSet 1.2021, 2004/10/22 14:29:57-07:00, [email protected]
[PATCH] remove cpu_run_sbin_hotplug()
From: Keshavamurthy Anil S <[email protected]>
Remove cpu_run_sbin_hotplug() - use kobject_hotplug() instead.
Signed-off-by: Anil S Keshavamurthy <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
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;
}
ChangeSet 1.2022, 2004/10/22 14:30:22-07:00, [email protected]
[PATCH] kobject_uevent warning fix
lib/kobject_uevent.c:39: warning: `action_to_string' defined but not used
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
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;
ChangeSet 1.2000.6.1, 2004/10/20 15:38:19-07:00, [email protected]
kobject: add CONFIG_DEBUG_KOBJECT
Signed-off-by: Greg Kroah-Hartman <[email protected]>
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);
}
ChangeSet 1.2020, 2004/10/22 14:29:19-07:00, [email protected]
[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 <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
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) {
ChangeSet 1.2018, 2004/10/22 13:55:29-07:00, [email protected]
[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 <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
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);
ChangeSet 1.2019, 2004/10/22 14:01:16-07:00, [email protected]
[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 <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
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);
ChangeSet 1.2450, 2004/11/01 13:06:02-08:00, [email protected]
[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 <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
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;
ChangeSet 1.2441, 2004/11/01 13:03:13-08:00, [email protected]
[PATCH] sysfs backing store - add sysfs_direct structure
From: Maneesh Soni <[email protected]>
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 <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
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
ChangeSet 1.2448, 2004/11/01 13:05:24-08:00, [email protected]
[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 <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
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);
ChangeSet 1.2447, 2004/11/01 13:05:06-08:00, [email protected]
[PATCH] Fix deadlocks on dpm_sem
From: Paul Mackerras <[email protected]>
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 <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
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);
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
ChangeSet 1.2449, 2004/11/01 13:05:43-08:00, [email protected]
[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 <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
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);
ChangeSet 1.2451, 2004/11/01 13:06:31-08:00, [email protected]
[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 <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
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
};
ChangeSet 1.2442, 2004/11/01 13:03:33-08:00, [email protected]
[PATCH] sysfs backing store: use sysfs_dirent based tree in file removal
From: Maneesh Soni <[email protected]>
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 <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
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);
}
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
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
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
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.
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
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.
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
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
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