drivers/base/class:
- class_device_rename(): function basically shat itself if anything
failed, leaving things in an indeterminant state. If kmalloc() ever
failed, it would dereference ERR_PTR(-ENOMEM). Fix a bunch of bugs,
over and above sysfs_create_link() error handling.
- class_device_add(): add missing sysfs_remove_link() [fix leak] to error path
- class_device_add(): handle sysfs_create_link() failure
drivers/base/dmapool:
- kmalloc() takes a GFP_xxx argument
- handle device_create_file() failure
drivers/base/platform:
- properly handle errors (fix leak-on-err) in platform_bus_init()
drivers/base/topology:
- return sysfs error via NOTIFY_BAD
Signed-off-by: Jeff Garzik <[email protected]>
---
drivers/base/class.c | 56 ++++++++++++++++++++++++++++++++++++++++++------
drivers/base/dmapool.c | 8 +++++-
drivers/base/platform.c | 10 ++++++--
drivers/base/topology.c | 11 ++++-----
4 files changed, 69 insertions(+), 16 deletions(-)
diff --git a/drivers/base/class.c b/drivers/base/class.c
index b32b77f..673db14 100644
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -562,7 +562,9 @@ int class_device_add(struct class_device
goto out2;
/* add the needed attributes to this device */
- sysfs_create_link(&class_dev->kobj, &parent_class->subsys.kset.kobj, "subsystem");
+ error = sysfs_create_link(&class_dev->kobj, &parent_class->subsys.kset.kobj, "subsystem");
+ if (error)
+ goto out2_5;
class_dev->uevent_attr.attr.name = "uevent";
class_dev->uevent_attr.attr.mode = S_IWUSR;
class_dev->uevent_attr.attr.owner = parent_class->owner;
@@ -639,6 +641,8 @@ int class_device_add(struct class_device
out4:
class_device_remove_file(class_dev, &class_dev->uevent_attr);
out3:
+ sysfs_remove_link(&class_dev->kobj, "subsystem");
+ out2_5:
kobject_del(&class_dev->kobj);
out2:
if(parent_class_dev)
@@ -791,36 +795,76 @@ void class_device_destroy(struct class *
int class_device_rename(struct class_device *class_dev, char *new_name)
{
- int error = 0;
+ int error = 0, err2;
char *old_class_name = NULL, *new_class_name = NULL;
+ char *old_name;
class_dev = class_device_get(class_dev);
if (!class_dev)
return -EINVAL;
- pr_debug("CLASS: renaming '%s' to '%s'\n", class_dev->class_id,
+ old_name = kstrdup(class_dev->class_id, GFP_KERNEL);
+ if (!old_name) {
+ error = -ENOMEM;
+ goto err_out;
+ }
+
+ pr_debug("CLASS: renaming '%s' to '%s'\n", old_name,
new_name);
- if (class_dev->dev)
+ if (class_dev->dev) {
old_class_name = make_class_name(class_dev->class->name,
&class_dev->kobj);
+ if (IS_ERR(old_class_name)) {
+ error = PTR_ERR(old_class_name);
+ goto err_out_oname;
+ }
+ }
strlcpy(class_dev->class_id, new_name, KOBJ_NAME_LEN);
error = kobject_rename(&class_dev->kobj, new_name);
+ if (error)
+ goto err_out_nameset;
if (class_dev->dev) {
new_class_name = make_class_name(class_dev->class->name,
&class_dev->kobj);
- sysfs_create_link(&class_dev->dev->kobj, &class_dev->kobj,
- new_class_name);
+ if (IS_ERR(new_class_name)) {
+ error = PTR_ERR(new_class_name);
+ goto err_out_rename;
+ }
+
+ error = sysfs_create_link(&class_dev->dev->kobj,
+ &class_dev->kobj, new_class_name);
+ if (error)
+ goto err_out_new_class;
+
sysfs_remove_link(&class_dev->dev->kobj, old_class_name);
}
+
class_device_put(class_dev);
+ kfree(old_name);
kfree(old_class_name);
kfree(new_class_name);
+ return 0;
+
+err_out_new_class:
+ kfree(new_class_name);
+err_out_rename:
+ err2 = kobject_rename(&class_dev->kobj, old_name);
+ if (err2)
+ printk(KERN_ERR "Error %d while undoing kobject_rename('%s') failure... ruh roh, raggy\n",
+ err2, new_name);
+err_out_nameset:
+ strlcpy(class_dev->class_id, old_name, KOBJ_NAME_LEN);
+ kfree(old_class_name);
+err_out_oname:
+ kfree(old_name);
+err_out:
+ class_device_put(class_dev);
return error;
}
diff --git a/drivers/base/dmapool.c b/drivers/base/dmapool.c
index 33c5cce..b418b6b 100644
--- a/drivers/base/dmapool.c
+++ b/drivers/base/dmapool.c
@@ -126,7 +126,7 @@ dma_pool_create (const char *name, struc
} else if (allocation < size)
return NULL;
- if (!(retval = kmalloc (sizeof *retval, SLAB_KERNEL)))
+ if (!(retval = kmalloc (sizeof *retval, GFP_KERNEL)))
return retval;
strlcpy (retval->name, name, sizeof retval->name);
@@ -143,7 +143,11 @@ dma_pool_create (const char *name, struc
if (dev) {
down (&pools_lock);
if (list_empty (&dev->dma_pools))
- device_create_file (dev, &dev_attr_pools);
+ if (device_create_file (dev, &dev_attr_pools)) {
+ up (&pools_lock);
+ kfree(retval);
+ return NULL;
+ }
/* note: not currently insisting "name" be unique */
list_add (&retval->pools, &dev->dma_pools);
up (&pools_lock);
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 940ce41..3b6b758 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -563,8 +563,14 @@ EXPORT_SYMBOL_GPL(platform_bus_type);
int __init platform_bus_init(void)
{
- device_register(&platform_bus);
- return bus_register(&platform_bus_type);
+ int err = device_register(&platform_bus);
+ if (err)
+ return err;
+
+ err = bus_register(&platform_bus_type);
+ if (err)
+ device_unregister(&platform_bus);
+ return err;
}
#ifndef ARCH_HAS_DMA_GET_REQUIRED_MASK
diff --git a/drivers/base/topology.c b/drivers/base/topology.c
index 3ef9d51..2d0c965 100644
--- a/drivers/base/topology.c
+++ b/drivers/base/topology.c
@@ -97,14 +97,12 @@ static struct attribute_group topology_a
/* Add/Remove cpu_topology interface for CPU device */
static int __cpuinit topology_add_dev(struct sys_device * sys_dev)
{
- sysfs_create_group(&sys_dev->kobj, &topology_attr_group);
- return 0;
+ return sysfs_create_group(&sys_dev->kobj, &topology_attr_group);
}
-static int __cpuinit topology_remove_dev(struct sys_device * sys_dev)
+static void __cpuinit topology_remove_dev(struct sys_device * sys_dev)
{
sysfs_remove_group(&sys_dev->kobj, &topology_attr_group);
- return 0;
}
static int __cpuinit topology_cpu_callback(struct notifier_block *nfb,
@@ -112,17 +110,18 @@ static int __cpuinit topology_cpu_callba
{
unsigned int cpu = (unsigned long)hcpu;
struct sys_device *sys_dev;
+ int rc = 0;
sys_dev = get_cpu_sysdev(cpu);
switch (action) {
case CPU_ONLINE:
- topology_add_dev(sys_dev);
+ rc = topology_add_dev(sys_dev);
break;
case CPU_DEAD:
topology_remove_dev(sys_dev);
break;
}
- return NOTIFY_OK;
+ return rc ? NOTIFY_BAD : NOTIFY_OK;
}
static struct notifier_block __cpuinitdata topology_cpu_notifier =
On Wed, 4 Oct 2006 09:05:54 -0400,
Jeff Garzik <[email protected]> wrote:
> drivers/base/class:
> - class_device_rename(): function basically shat itself if anything
> failed, leaving things in an indeterminant state. If kmalloc() ever
> failed, it would dereference ERR_PTR(-ENOMEM). Fix a bunch of bugs,
> over and above sysfs_create_link() error handling.
>
> - class_device_add(): add missing sysfs_remove_link() [fix leak] to error path
> - class_device_add(): handle sysfs_create_link() failure
>
> drivers/base/dmapool:
> - kmalloc() takes a GFP_xxx argument
> - handle device_create_file() failure
>
> drivers/base/platform:
> - properly handle errors (fix leak-on-err) in platform_bus_init()
>
> drivers/base/topology:
> - return sysfs error via NOTIFY_BAD
Hm, I already did fixes for some of these which are in -mm / in Greg's
tree. It would perhaps make sense if you rediffed against one of those
trees.
Cornelia
On Wed, 4 Oct 2006 09:05:54 -0400,
Jeff Garzik <[email protected]> wrote:
> static int __cpuinit topology_cpu_callback(struct notifier_block *nfb,
> @@ -112,17 +110,18 @@ static int __cpuinit topology_cpu_callba
> {
> unsigned int cpu = (unsigned long)hcpu;
> struct sys_device *sys_dev;
> + int rc = 0;
>
> sys_dev = get_cpu_sysdev(cpu);
> switch (action) {
> case CPU_ONLINE:
> - topology_add_dev(sys_dev);
> + rc = topology_add_dev(sys_dev);
> break;
> case CPU_DEAD:
> topology_remove_dev(sys_dev);
> break;
> }
> - return NOTIFY_OK;
> + return rc ? NOTIFY_BAD : NOTIFY_OK;
> }
Wouldn't that also require that _cpu_up checked the return code when
doing CPU_ONLINE notification (and clean up on error)?
On Wed, Oct 04, 2006 at 05:24:34PM +0200, Cornelia Huck wrote:
> On Wed, 4 Oct 2006 09:05:54 -0400,
> Jeff Garzik <[email protected]> wrote:
>
> > static int __cpuinit topology_cpu_callback(struct notifier_block *nfb,
> > @@ -112,17 +110,18 @@ static int __cpuinit topology_cpu_callba
> > {
> > unsigned int cpu = (unsigned long)hcpu;
> > struct sys_device *sys_dev;
> > + int rc = 0;
> >
> > sys_dev = get_cpu_sysdev(cpu);
> > switch (action) {
> > case CPU_ONLINE:
> > - topology_add_dev(sys_dev);
> > + rc = topology_add_dev(sys_dev);
> > break;
> > case CPU_DEAD:
> > topology_remove_dev(sys_dev);
> > break;
> > }
> > - return NOTIFY_OK;
> > + return rc ? NOTIFY_BAD : NOTIFY_OK;
> > }
>
> Wouldn't that also require that _cpu_up checked the return code when
> doing CPU_ONLINE notification (and clean up on error)?
After all code that gets a CPU_ONLINE notification is not supposed to fail.
For allocating resources while bringing up a cpu CPU_UP_PREPARE is supposed
to be used. That one is allowed to fail.
Heiko Carstens wrote:
> On Wed, Oct 04, 2006 at 05:24:34PM +0200, Cornelia Huck wrote:
>> On Wed, 4 Oct 2006 09:05:54 -0400,
>> Jeff Garzik <[email protected]> wrote:
>>
>>> static int __cpuinit topology_cpu_callback(struct notifier_block *nfb,
>>> @@ -112,17 +110,18 @@ static int __cpuinit topology_cpu_callba
>>> {
>>> unsigned int cpu = (unsigned long)hcpu;
>>> struct sys_device *sys_dev;
>>> + int rc = 0;
>>>
>>> sys_dev = get_cpu_sysdev(cpu);
>>> switch (action) {
>>> case CPU_ONLINE:
>>> - topology_add_dev(sys_dev);
>>> + rc = topology_add_dev(sys_dev);
>>> break;
>>> case CPU_DEAD:
>>> topology_remove_dev(sys_dev);
>>> break;
>>> }
>>> - return NOTIFY_OK;
>>> + return rc ? NOTIFY_BAD : NOTIFY_OK;
>>> }
>> Wouldn't that also require that _cpu_up checked the return code when
>> doing CPU_ONLINE notification (and clean up on error)?
>
> After all code that gets a CPU_ONLINE notification is not supposed to fail.
> For allocating resources while bringing up a cpu CPU_UP_PREPARE is supposed
> to be used. That one is allowed to fail.
It's a bug no matter how you look at it... I just lessen the impact. :)
If someone wants to provide a better fix, let's see the patch...
Jeff
> >>> static int __cpuinit topology_cpu_callback(struct notifier_block *nfb,
> >>>@@ -112,17 +110,18 @@ static int __cpuinit topology_cpu_callba
> >>> {
> >>> unsigned int cpu = (unsigned long)hcpu;
> >>> struct sys_device *sys_dev;
> >>>+ int rc = 0;
> >>> sys_dev = get_cpu_sysdev(cpu);
> >>> switch (action) {
> >>> case CPU_ONLINE:
> >>>- topology_add_dev(sys_dev);
> >>>+ rc = topology_add_dev(sys_dev);
> >>> break;
> >>> case CPU_DEAD:
> >>> topology_remove_dev(sys_dev);
> >>> break;
> >>> }
> >>>- return NOTIFY_OK;
> >>>+ return rc ? NOTIFY_BAD : NOTIFY_OK;
> >>> }
> >>Wouldn't that also require that _cpu_up checked the return code when
> >>doing CPU_ONLINE notification (and clean up on error)?
> >After all code that gets a CPU_ONLINE notification is not supposed to fail.
> >For allocating resources while bringing up a cpu CPU_UP_PREPARE is supposed
> >to be used. That one is allowed to fail.
>
> It's a bug no matter how you look at it... I just lessen the impact. :)
>
> If someone wants to provide a better fix, let's see the patch...
If sysfs_remove_group() would also work for non-created (-existent) groups
then the patch below would work. Unfortunately that is not the case. So one
would have to remember if sysfs_create_group() was done and succeeded before
calling sysfs_remove_group()...
There must be an easier way.
diff --git a/drivers/base/topology.c b/drivers/base/topology.c
index 3ef9d51..d0056c3 100644
--- a/drivers/base/topology.c
+++ b/drivers/base/topology.c
@@ -97,8 +97,7 @@ static struct attribute_group topology_a
/* Add/Remove cpu_topology interface for CPU device */
static int __cpuinit topology_add_dev(struct sys_device * sys_dev)
{
- sysfs_create_group(&sys_dev->kobj, &topology_attr_group);
- return 0;
+ return sysfs_create_group(&sys_dev->kobj, &topology_attr_group);
}
static int __cpuinit topology_remove_dev(struct sys_device * sys_dev)
@@ -112,12 +111,16 @@ static int __cpuinit topology_cpu_callba
{
unsigned int cpu = (unsigned long)hcpu;
struct sys_device *sys_dev;
+ int rc;
sys_dev = get_cpu_sysdev(cpu);
switch (action) {
- case CPU_ONLINE:
- topology_add_dev(sys_dev);
+ case CPU_UP_PREPARE:
+ rc = topology_add_dev(sys_dev);
+ if (rc)
+ return NOTIFY_BAD;
break;
+ case CPU_UP_CANCELED:
case CPU_DEAD:
topology_remove_dev(sys_dev);
break;
@@ -132,11 +135,15 @@ static struct notifier_block __cpuinitda
static int __cpuinit topology_sysfs_init(void)
{
- int i;
-
- for_each_online_cpu(i) {
- topology_cpu_callback(&topology_cpu_notifier, CPU_ONLINE,
- (void *)(long)i);
+ struct sys_device *sys_dev;
+ int cpu;
+ int rc;
+
+ for_each_online_cpu(cpu) {
+ sys_dev = get_cpu_sysdev(cpu);
+ rc = topology_add_dev(sys_dev);
+ if (rc)
+ return rc;
}
register_hotcpu_notifier(&topology_cpu_notifier);
Heiko Carstens wrote:
>>>>> static int __cpuinit topology_cpu_callback(struct notifier_block *nfb,
>>>>> @@ -112,17 +110,18 @@ static int __cpuinit topology_cpu_callba
>>>>> {
>>>>> unsigned int cpu = (unsigned long)hcpu;
>>>>> struct sys_device *sys_dev;
>>>>> + int rc = 0;
>>>>> sys_dev = get_cpu_sysdev(cpu);
>>>>> switch (action) {
>>>>> case CPU_ONLINE:
>>>>> - topology_add_dev(sys_dev);
>>>>> + rc = topology_add_dev(sys_dev);
>>>>> break;
>>>>> case CPU_DEAD:
>>>>> topology_remove_dev(sys_dev);
>>>>> break;
>>>>> }
>>>>> - return NOTIFY_OK;
>>>>> + return rc ? NOTIFY_BAD : NOTIFY_OK;
>>>>> }
>>>> Wouldn't that also require that _cpu_up checked the return code when
>>>> doing CPU_ONLINE notification (and clean up on error)?
>>> After all code that gets a CPU_ONLINE notification is not supposed to fail.
>>> For allocating resources while bringing up a cpu CPU_UP_PREPARE is supposed
>>> to be used. That one is allowed to fail.
>> It's a bug no matter how you look at it... I just lessen the impact. :)
>>
>> If someone wants to provide a better fix, let's see the patch...
>
> If sysfs_remove_group() would also work for non-created (-existent) groups
> then the patch below would work. Unfortunately that is not the case. So one
> would have to remember if sysfs_create_group() was done and succeeded before
> calling sysfs_remove_group()...
> There must be an easier way.
>
> diff --git a/drivers/base/topology.c b/drivers/base/topology.c
> index 3ef9d51..d0056c3 100644
> --- a/drivers/base/topology.c
> +++ b/drivers/base/topology.c
ACK
On Thu, 5 Oct 2006 14:48:48 +0200,
Heiko Carstens <[email protected]> wrote:
> If sysfs_remove_group() would also work for non-created (-existent) groups
> then the patch below would work. Unfortunately that is not the case. So one
> would have to remember if sysfs_create_group() was done and succeeded before
> calling sysfs_remove_group()...
> There must be an easier way.
<snip>
> @@ -132,11 +135,15 @@ static struct notifier_block __cpuinitda
>
> static int __cpuinit topology_sysfs_init(void)
> {
> - int i;
> -
> - for_each_online_cpu(i) {
> - topology_cpu_callback(&topology_cpu_notifier, CPU_ONLINE,
> - (void *)(long)i);
> + struct sys_device *sys_dev;
> + int cpu;
> + int rc;
> +
> + for_each_online_cpu(cpu) {
> + sys_dev = get_cpu_sysdev(cpu);
> + rc = topology_add_dev(sys_dev);
> + if (rc)
> + return rc;
> }
>
> register_hotcpu_notifier(&topology_cpu_notifier);
Shouldn't the added attribute groups be removed again in the failure
case?
Also, it might be a bit overkill to fail the whole initialization
because of one "bad" cpu. (And the "bad" cpu wouldn't matter if we
could safely remove non-existent groups :)
> >>If someone wants to provide a better fix, let's see the patch...
> >If sysfs_remove_group() would also work for non-created (-existent) groups
> >then the patch below would work. Unfortunately that is not the case. So one
> >would have to remember if sysfs_create_group() was done and succeeded before
> >calling sysfs_remove_group()...
> >There must be an easier way.
> >diff --git a/drivers/base/topology.c b/drivers/base/topology.c
> >index 3ef9d51..d0056c3 100644
> >--- a/drivers/base/topology.c
> >+++ b/drivers/base/topology.c
>
> ACK
Ah, you probably got me wrong. The patch doesn't work, because of the
sysfs_remove_group() stuff. That's why there is no Signed-off-by:...
What _could_ happen with this patch applied: some code fails on
CPU_UP_PREPARE and then all notifiers get called with CPU_UP_CANCELLED.
That would cause the call of topology_remove_dev(sys_dev) and therefore
sysfs_remove_group(&sys_dev->kobj, &topology_attr_group) which will crash,
because sysfs_create_group() wasn't even called.
To fix this one has to remember if sysfs_create_group() succeeded or was
even called. That would be a per-cpu array. Now, I think it's just
overkill to introduce a per-cpu array for error-checking.
IMHO it would make sense to change sysfs_remove_group() so it will survive
if being asked to remove groups that don't exist.
> > + for_each_online_cpu(cpu) {
> > + sys_dev = get_cpu_sysdev(cpu);
> > + rc = topology_add_dev(sys_dev);
> > + if (rc)
> > + return rc;
> > }
> >
> > register_hotcpu_notifier(&topology_cpu_notifier);
>
> Shouldn't the added attribute groups be removed again in the failure
> case?
>
> Also, it might be a bit overkill to fail the whole initialization
> because of one "bad" cpu. (And the "bad" cpu wouldn't matter if we
> could safely remove non-existent groups :)
This is initcall stuff. The only sane reason why this would fail, would
be an out of memory situation . If we are that early short on memory, we
are in serious trouble anyway. So I doubt it's worth the extra code.
On Thu, 5 Oct 2006 15:20:06 +0200,
Heiko Carstens <[email protected]> wrote:
> This is initcall stuff. The only sane reason why this would fail, would
> be an out of memory situation . If we are that early short on memory, we
> are in serious trouble anyway. So I doubt it's worth the extra code.
OK, at this stage -ENOMEM sounds like the only possiblilty. Agreed.
From: Heiko Carstens <[email protected]>
This patch makes it safe to call sysfs_remove_group() with a name group
that doesn't exist. Needed to make fix cpu hotplug stuff in topology code.
Signed-off-by: Heiko Carstens <[email protected]>
---
fs/sysfs/group.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletion(-)
Index: linux-2.6/fs/sysfs/group.c
===================================================================
--- linux-2.6.orig/fs/sysfs/group.c 2006-10-09 09:15:25.000000000 +0200
+++ linux-2.6/fs/sysfs/group.c 2006-10-09 09:25:23.000000000 +0200
@@ -68,9 +68,12 @@
{
struct dentry * dir;
- if (grp->name)
+ if (grp->name) {
+ if (!sysfs_dirent_exist(kobj->dentry->d_fsdata, grp->name))
+ return;
dir = lookup_one_len(grp->name, kobj->dentry,
strlen(grp->name));
+ }
else
dir = dget(kobj->dentry);
From: Heiko Carstens <[email protected]>
- take return valie sysfs_create_group into account.
- don't call code that might fail on CPU_ONLINE notification. Instead call
it on CPU_UP_PREPARE and check it's return value.
- since CPU_UP_PREPARE might fail add a CPU_UP_CANCELED handling as well.
- use hotcpu_notifier instead of register_hotcpu_notifier
- #ifdef code that isn't needed in the !CONFIG_HOTPLUG_CPU case.
Signed-off-by: Heiko Carstens <[email protected]>
---
Could be split up into a bunch of several patches if really needed.
drivers/base/topology.c | 33 +++++++++++++++++----------------
1 files changed, 17 insertions(+), 16 deletions(-)
Index: linux-2.6/drivers/base/topology.c
===================================================================
--- linux-2.6.orig/drivers/base/topology.c 2006-10-09 09:15:27.000000000 +0200
+++ linux-2.6/drivers/base/topology.c 2006-10-09 09:25:05.000000000 +0200
@@ -97,10 +97,10 @@
/* Add/Remove cpu_topology interface for CPU device */
static int __cpuinit topology_add_dev(struct sys_device * sys_dev)
{
- sysfs_create_group(&sys_dev->kobj, &topology_attr_group);
- return 0;
+ return sysfs_create_group(&sys_dev->kobj, &topology_attr_group);
}
+#ifdef CONFIG_HOTPLUG_CPU
static int __cpuinit topology_remove_dev(struct sys_device * sys_dev)
{
sysfs_remove_group(&sys_dev->kobj, &topology_attr_group);
@@ -112,34 +112,35 @@
{
unsigned int cpu = (unsigned long)hcpu;
struct sys_device *sys_dev;
+ int rc = 0;
sys_dev = get_cpu_sysdev(cpu);
switch (action) {
- case CPU_ONLINE:
- topology_add_dev(sys_dev);
+ case CPU_UP_PREPARE:
+ rc = topology_add_dev(sys_dev);
break;
+ case CPU_UP_CANCELED:
case CPU_DEAD:
topology_remove_dev(sys_dev);
break;
}
- return NOTIFY_OK;
+ return rc ? NOTIFY_BAD : NOTIFY_OK;
}
-
-static struct notifier_block __cpuinitdata topology_cpu_notifier =
-{
- .notifier_call = topology_cpu_callback,
-};
+#endif
static int __cpuinit topology_sysfs_init(void)
{
- int i;
+ struct sys_device *sys_dev;
+ int cpu;
+ int rc;
- for_each_online_cpu(i) {
- topology_cpu_callback(&topology_cpu_notifier, CPU_ONLINE,
- (void *)(long)i);
+ for_each_online_cpu(cpu) {
+ sys_dev = get_cpu_sysdev(cpu);
+ rc = topology_add_dev(sys_dev);
+ if (rc)
+ return rc;
}
-
- register_hotcpu_notifier(&topology_cpu_notifier);
+ hotcpu_notifier(topology_cpu_callback, 0);
return 0;
}
On Mon, 9 Oct 2006 09:29:20 +0200
Heiko Carstens <[email protected]> wrote:
> From: Heiko Carstens <[email protected]>
>
> This patch makes it safe to call sysfs_remove_group() with a name group
> that doesn't exist. Needed to make fix cpu hotplug stuff in topology code.
>
Surely an attempt to remove a non-existent entry is a bug, and this
(racy-looking) patch just covers that up?
> ---
> fs/sysfs/group.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletion(-)
>
> Index: linux-2.6/fs/sysfs/group.c
> ===================================================================
> --- linux-2.6.orig/fs/sysfs/group.c 2006-10-09 09:15:25.000000000 +0200
> +++ linux-2.6/fs/sysfs/group.c 2006-10-09 09:25:23.000000000 +0200
> @@ -68,9 +68,12 @@
> {
> struct dentry * dir;
>
> - if (grp->name)
> + if (grp->name) {
> + if (!sysfs_dirent_exist(kobj->dentry->d_fsdata, grp->name))
> + return;
> dir = lookup_one_len(grp->name, kobj->dentry,
> strlen(grp->name));
> + }
> else
> dir = dget(kobj->dentry);
>
On Mon, Oct 09, 2006 at 12:40:14AM -0700, Andrew Morton wrote:
> On Mon, 9 Oct 2006 09:29:20 +0200
> Heiko Carstens <[email protected]> wrote:
>
> > From: Heiko Carstens <[email protected]>
> >
> > This patch makes it safe to call sysfs_remove_group() with a name group
> > that doesn't exist. Needed to make fix cpu hotplug stuff in topology code.
> >
>
> Surely an attempt to remove a non-existent entry is a bug, and this
> (racy-looking) patch just covers that up?
It just tries to keep cpu hotplug code in drivers/base/topology.c simple. Since
otherwise one would have to remember if sysfs_create_group() succeeded or not.
Hmm.. thinking again, this patch looks indeed racy.
> > Index: linux-2.6/fs/sysfs/group.c
> > ===================================================================
> > --- linux-2.6.orig/fs/sysfs/group.c 2006-10-09 09:15:25.000000000 +0200
> > +++ linux-2.6/fs/sysfs/group.c 2006-10-09 09:25:23.000000000 +0200
> > @@ -68,9 +68,12 @@
> > {
> > struct dentry * dir;
> >
> > - if (grp->name)
> > + if (grp->name) {
> > + if (!sysfs_dirent_exist(kobj->dentry->d_fsdata, grp->name))
> > + return;
> > dir = lookup_one_len(grp->name, kobj->dentry,
> > strlen(grp->name));
> > + }
> > else
> > dir = dget(kobj->dentry);
> >
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/