Add $DEVPATH to the environmental variables during /sbin/hotplug call.
Josef 'Jeff' Sipek.
Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
diff -Nru a/kernel/cpu.c b/kernel/cpu.c
--- a/kernel/cpu.c 2004-09-24 13:08:57 -04:00
+++ b/kernel/cpu.c 2004-09-24 13:08:57 -04:00
@@ -61,13 +61,13 @@
* cpu' with certain environment variables set. */
static int cpu_run_sbin_hotplug(unsigned int cpu, const char *action)
{
- char *argv[3], *envp[5], cpu_str[12], action_str[32];
+ char *argv[3], *envp[6], cpu_str[12], action_str[32], devpath_str[38];
int i;
sprintf(cpu_str, "CPU=%d", cpu);
sprintf(action_str, "ACTION=%s", action);
- /* FIXME: Add DEVPATH. --RR */
-
+ sprintf(devpath_str, "DEVPATH=devices/system/cpu/cpu%d", cpu);
+
i = 0;
argv[i++] = hotplug_path;
argv[i++] = "cpu";
@@ -79,6 +79,7 @@
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);
"Josef 'Jeff' Sipek" <[email protected]> wrote:
>
> Add $DEVPATH to the environmental variables during /sbin/hotplug call.
>
> Josef 'Jeff' Sipek.
>
> Signed-off-by: Josef 'Jeff' Sipek <[email protected]>
>
>
> diff -Nru a/kernel/cpu.c b/kernel/cpu.c
> --- a/kernel/cpu.c 2004-09-24 13:08:57 -04:00
> +++ b/kernel/cpu.c 2004-09-24 13:08:57 -04:00
> @@ -61,13 +61,13 @@
> * cpu' with certain environment variables set. */
> static int cpu_run_sbin_hotplug(unsigned int cpu, const char *action)
I don't think this function should exist at all. We already have kobjects
which represent the CPUs so it should be possible to find that kobject and
use kobject_hotplug() on it. That gives you your $DEVPATH.
Does CPU hotplug behave correctly wrt /sys/devices/system/cpu? Given that
register_cpu() is still marked __init, I assume not.
On Sun, Oct 03, 2004 at 04:20:12PM -0700, Andrew Morton wrote:
> Does CPU hotplug behave correctly wrt /sys/devices/system/cpu? Given that
> register_cpu() is still marked __init, I assume not.
Currently what we have in the kernel is logical cpu hotplug, i.e once the
cpu is registered via register_cpu() that cpu can only go offline and still
the entry for that cpu will be present in the /sys/devices/system/cpu/cpuX/online.
So __init register_cpu() is fine untill we support unregister_cpu()
which is required for physical cpu hotplug case.
I have submitted ACPI based physical cpu hotplug patches and waiting to here from
ACPI mainitainer Len Brown, there I have taken care to support unregister_cpu()
and register_cpu() is marked as __devinit in those patches.
> -
> 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/
Keshavamurthy Anil S <[email protected]> wrote:
>
> On Sun, Oct 03, 2004 at 04:20:12PM -0700, Andrew Morton wrote:
> > Does CPU hotplug behave correctly wrt /sys/devices/system/cpu? Given that
> > register_cpu() is still marked __init, I assume not.
>
> Currently what we have in the kernel is logical cpu hotplug, i.e once the
> cpu is registered via register_cpu() that cpu can only go offline and still
> the entry for that cpu will be present in the /sys/devices/system/cpu/cpuX/online.
>
> So __init register_cpu() is fine untill we support unregister_cpu()
> which is required for physical cpu hotplug case.
>
> I have submitted ACPI based physical cpu hotplug patches and waiting to here from
> ACPI mainitainer Len Brown, there I have taken care to support unregister_cpu()
> and register_cpu() is marked as __devinit in those patches.
OK...
But still, cpu_run_sbin_hotplug() should not exist. It is duplicating
(indeed, emulating) kobject_hotplug() behaviour. To the extent that it now
has a hardwired sysfs path embedded in it:
sprintf(devpath_str, "DEVPATH=devices/system/cpu/cpu%d", cpu);
which should have been obtained from kobject_get_path().
On Mon, Oct 04, 2004 at 12:37:25PM -0700, Andrew Morton wrote:
> Keshavamurthy Anil S <[email protected]> wrote:
> >
> > On Sun, Oct 03, 2004 at 04:20:12PM -0700, Andrew Morton wrote:
> > > Does CPU hotplug behave correctly wrt /sys/devices/system/cpu? Given that
> > > register_cpu() is still marked __init, I assume not.
> >
> > Currently what we have in the kernel is logical cpu hotplug, i.e once the
> > cpu is registered via register_cpu() that cpu can only go offline and still
> > the entry for that cpu will be present in the /sys/devices/system/cpu/cpuX/online.
> >
> > So __init register_cpu() is fine untill we support unregister_cpu()
> > which is required for physical cpu hotplug case.
> >
> > I have submitted ACPI based physical cpu hotplug patches and waiting to here from
> > ACPI mainitainer Len Brown, there I have taken care to support unregister_cpu()
> > and register_cpu() is marked as __devinit in those patches.
>
> OK...
>
> But still, cpu_run_sbin_hotplug() should not exist. It is duplicating
> (indeed, emulating) kobject_hotplug() behaviour. To the extent that it now
> has a hardwired sysfs path embedded in it:
>
> sprintf(devpath_str, "DEVPATH=devices/system/cpu/cpu%d", cpu);
>
> which should have been obtained from kobject_get_path().
Yes, I agree to your point that cpu_run_sbin_hotplug() is duplication kobject_hotplug()
behaviour. I will send you a patch to fix this ASAP(hopefully before end of today).
thanks,
Anil
On Mon, Oct 04, 2004 at 12:43:55PM -0700, Keshavamurthy Anil S wrote:
> On Mon, Oct 04, 2004 at 12:37:25PM -0700, Andrew Morton wrote:
> > Keshavamurthy Anil S <[email protected]> wrote:
> > >
> >
> > But still, cpu_run_sbin_hotplug() should not exist. It is duplicating
> > (indeed, emulating) kobject_hotplug() behaviour. To the extent that it now
> > has a hardwired sysfs path embedded in it:
> >
> > sprintf(devpath_str, "DEVPATH=devices/system/cpu/cpu%d", cpu);
> >
> > which should have been obtained from kobject_get_path().
> Yes, I agree to your point that cpu_run_sbin_hotplug() is duplication kobject_hotplug()
> behaviour. I will send you a patch to fix this ASAP(hopefully before end of today).
Hi Andrew,
Here is what I have come up with(please take a look at this patch).
I was successfully able to get rid of cpu_run_sbin_hotplug() function, but
when I call kobject_hotplug() function, it is finding
top_kobj->kset->hotplug_ops set to NULL and hence returns without calling
call_usermodehelper(). Not sure if this is a bug in kobject_hotplug(),
I feel kobject_hotplug() function should continue even if
top_kobj->kset-hotplug_ops is NULL.
CC'ing grek-KH on this mailing list.
Attached is the patch to remove cpu_run_sbin_hotplug.
Signed-off-by: Anil S Keshavamurthy <[email protected]>
---
linux-2.6.9-rc3-test-askeshav/drivers/base/cpu.c | 2
linux-2.6.9-rc3-test-askeshav/include/linux/kobject_uevent.h | 1
linux-2.6.9-rc3-test-askeshav/kernel/cpu.c | 34 -----------
linux-2.6.9-rc3-test-askeshav/lib/kobject_uevent.c | 1
4 files changed, 4 insertions(+), 34 deletions(-)
diff -puN kernel/cpu.c~cpu_run_sbin_hotplug_v2 kernel/cpu.c
--- linux-2.6.9-rc3-test/kernel/cpu.c~cpu_run_sbin_hotplug_v2 2004-10-04 22:51:15.897914273 -0700
+++ linux-2.6.9-rc3-test-askeshav/kernel/cpu.c 2004-10-04 22:51:16.002406459 -0700
@@ -57,33 +57,6 @@ static inline void check_for_tasks(int c
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[5], cpu_str[12], action_str[32];
- int i;
-
- sprintf(cpu_str, "CPU=%d", cpu);
- sprintf(action_str, "ACTION=%s", action);
- /* FIXME: Add DEVPATH. --RR */
-
- 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] = NULL;
-
- return call_usermodehelper(argv[0], argv, envp, 0);
-}
-
/* Take this CPU down. */
static int take_cpu_down(void *unused)
{
@@ -155,8 +128,6 @@ int cpu_down(unsigned int cpu)
check_for_tasks(cpu);
- cpu_run_sbin_hotplug(cpu, "offline");
-
out_thread:
err = kthread_stop(p);
out_allowed:
@@ -165,11 +136,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*/
int __devinit cpu_up(unsigned int cpu)
diff -puN drivers/base/cpu.c~cpu_run_sbin_hotplug_v2 drivers/base/cpu.c
--- linux-2.6.9-rc3-test/drivers/base/cpu.c~cpu_run_sbin_hotplug_v2 2004-10-04 22:51:15.902797086 -0700
+++ linux-2.6.9-rc3-test-askeshav/drivers/base/cpu.c 2004-10-04 22:52:17.902796326 -0700
@@ -32,6 +32,8 @@ static ssize_t store_online(struct sys_d
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 -puN include/linux/kobject_uevent.h~cpu_run_sbin_hotplug_v2 include/linux/kobject_uevent.h
--- linux-2.6.9-rc3-test/include/linux/kobject_uevent.h~cpu_run_sbin_hotplug_v2 2004-10-04 22:51:15.908656461 -0700
+++ linux-2.6.9-rc3-test-askeshav/include/linux/kobject_uevent.h 2004-10-04 22:51:16.004359584 -0700
@@ -22,6 +22,7 @@ enum kobject_action {
KOBJ_CHANGE = 0x02, /* a sysfs attribute file has changed */
KOBJ_MOUNT = 0x03, /* mount event for block devices */
KOBJ_UMOUNT = 0x04, /* umount event for block devices */
+ KOBJ_OFFLINE = 0x05, /* offline event for hotplug devices */
KOBJ_MAX_ACTION, /* must be last action listed */
};
diff -puN lib/kobject_uevent.c~cpu_run_sbin_hotplug_v2 lib/kobject_uevent.c
--- linux-2.6.9-rc3-test/lib/kobject_uevent.c~cpu_run_sbin_hotplug_v2 2004-10-04 22:52:35.953577355 -0700
+++ linux-2.6.9-rc3-test-askeshav/lib/kobject_uevent.c 2004-10-04 22:53:39.304162516 -0700
@@ -33,6 +33,7 @@ static char *actions[] = {
"change", /* 0x02 */
"mount", /* 0x03 */
"umount", /* 0x04 */
+ "offline", /* 0x05 */
};
static char *action_to_string(enum kobject_action action)
_
Keshavamurthy Anil S <[email protected]> wrote:
>
> Here is what I have come up with(please take a look at this patch).
> I was successfully able to get rid of cpu_run_sbin_hotplug() function, but
> when I call kobject_hotplug() function, it is finding
> top_kobj->kset->hotplug_ops set to NULL and hence returns without calling
> call_usermodehelper(). Not sure if this is a bug in kobject_hotplug(),
> I feel kobject_hotplug() function should continue even if
> top_kobj->kset-hotplug_ops is NULL.
Yes, it doesn't seem necessary. We could give cpu_sysdev_class a
valid-but-empty hotplug_ops but it seems simpler and more general to do it
in kobject_hotplug().
Make kobject_hotplug() work even if the kobject's kset doesn't implement any
hotplug_ops.
Signed-off-by: Andrew Morton <[email protected]>
---
25-akpm/lib/kobject_uevent.c | 19 ++++++++++++-------
1 files changed, 12 insertions(+), 7 deletions(-)
diff -puN lib/kobject_uevent.c~kobject_hotplug-no-hotplug_ops lib/kobject_uevent.c
--- 25/lib/kobject_uevent.c~kobject_hotplug-no-hotplug_ops 2004-10-05 10:11:59.404291240 -0700
+++ 25-akpm/lib/kobject_uevent.c 2004-10-05 10:13:59.132089848 -0700
@@ -191,6 +191,8 @@ void kobject_hotplug(struct kobject *kob
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 {
@@ -198,15 +200,18 @@ void kobject_hotplug(struct kobject *kob
} 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;
}
@@ -225,8 +230,8 @@ void kobject_hotplug(struct kobject *kob
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;
@@ -260,9 +265,9 @@ void kobject_hotplug(struct kobject *kob
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) {
_
On Tue, Oct 05, 2004 at 10:18:23AM -0700, Andrew Morton wrote:
> Keshavamurthy Anil S <[email protected]> wrote:
> >
> > Here is what I have come up with(please take a look at this patch).
> > I was successfully able to get rid of cpu_run_sbin_hotplug() function, but
> > when I call kobject_hotplug() function, it is finding
> > top_kobj->kset->hotplug_ops set to NULL and hence returns without calling
> > call_usermodehelper(). Not sure if this is a bug in kobject_hotplug(),
> > I feel kobject_hotplug() function should continue even if
> > top_kobj->kset-hotplug_ops is NULL.
>
> Yes, it doesn't seem necessary. We could give cpu_sysdev_class a
> valid-but-empty hotplug_ops but it seems simpler and more general to do it
> in kobject_hotplug().
I tried that, but I found that parent "cpu" directory i.e
/sys/devices/system/cpu itself was not getting created. Any clues?
Keshavamurthy Anil S <[email protected]> wrote:
>
> On Tue, Oct 05, 2004 at 10:18:23AM -0700, Andrew Morton wrote:
> > Keshavamurthy Anil S <[email protected]> wrote:
> > >
> > > Here is what I have come up with(please take a look at this patch).
> > > I was successfully able to get rid of cpu_run_sbin_hotplug() function, but
> > > when I call kobject_hotplug() function, it is finding
> > > top_kobj->kset->hotplug_ops set to NULL and hence returns without calling
> > > call_usermodehelper(). Not sure if this is a bug in kobject_hotplug(),
> > > I feel kobject_hotplug() function should continue even if
> > > top_kobj->kset-hotplug_ops is NULL.
> >
> > Yes, it doesn't seem necessary. We could give cpu_sysdev_class a
> > valid-but-empty hotplug_ops but it seems simpler and more general to do it
> > in kobject_hotplug().
>
> I tried that, but I found that parent "cpu" directory i.e
> /sys/devices/system/cpu itself was not getting created. Any clues?
I don't see why the change to kobject_hotplug() would cause that directory
to not be created.
With your patch and mine applied, /sys/devices/system/cpu is present and
populated on my test box.
On Tue, Oct 05, 2004 at 10:47:44AM -0700, Andrew Morton wrote:
> Keshavamurthy Anil S <[email protected]> wrote:
> >
> > On Tue, Oct 05, 2004 at 10:18:23AM -0700, Andrew Morton wrote:
> > > Keshavamurthy Anil S <[email protected]> wrote:
> > > >
> > > > Here is what I have come up with(please take a look at this patch).
> > > > I was successfully able to get rid of cpu_run_sbin_hotplug() function, but
> > > > when I call kobject_hotplug() function, it is finding
> > > > top_kobj->kset->hotplug_ops set to NULL and hence returns without calling
> > > > call_usermodehelper(). Not sure if this is a bug in kobject_hotplug(),
> > > > I feel kobject_hotplug() function should continue even if
> > > > top_kobj->kset-hotplug_ops is NULL.
> > >
> > > Yes, it doesn't seem necessary. We could give cpu_sysdev_class a
> > > valid-but-empty hotplug_ops but it seems simpler and more general to do it
> > > in kobject_hotplug().
> >
> > I tried that, but I found that parent "cpu" directory i.e
> > /sys/devices/system/cpu itself was not getting created. Any clues?
>
> I don't see why the change to kobject_hotplug() would cause that directory
> to not be created.
>
> With your patch and mine applied, /sys/devices/system/cpu is present and
> populated on my test box.
Hi Andrew,
I am attaching the second one, just to make sure you and I have the same one.
If this is different than what you are having let me know.
By the way I am testing on IA64 box, with 2.6.9-rc3 + just bk-driver-core.patch from
your 2.6.9-rc3-mm2-broken-out.tar.
I had to go for just above as I was seeing some out of memory messages on my IA64 box
with complete rc3-mm2 patch.
thanks,
Anil
---
linux-2.6.9-rc3-test-askeshav/drivers/base/cpu.c | 2 ++
1 files changed, 2 insertions(+)
diff -puN drivers/base/cpu.c~test_akpm drivers/base/cpu.c
--- linux-2.6.9-rc3-test/drivers/base/cpu.c~test_akpm 2004-10-04 23:45:45.304124223 -0700
+++ linux-2.6.9-rc3-test-askeshav/drivers/base/cpu.c 2004-10-05 10:54:08.725507956 -0700
@@ -9,9 +9,11 @@
#include <linux/topology.h>
#include <linux/device.h>
+struct kset_hotplug_ops cpu_kset_hotplug_ops;
struct sysdev_class cpu_sysdev_class = {
set_kset_name("cpu"),
+ .kset = { .hotplug_ops = &cpu_kset_hotplug_ops},
};
EXPORT_SYMBOL(cpu_sysdev_class);
_
Keshavamurthy Anil S <[email protected]> wrote:
>
> I am attaching the second one, just to make sure you and I have the same one.
> If this is different than what you are having let me know.
yes, it's different from the one I'm using. I'm using the below, which I
sent you an hour ago.
> By the way I am testing on IA64 box, with 2.6.9-rc3 + just bk-driver-core.patch from
> your 2.6.9-rc3-mm2-broken-out.tar.
> I had to go for just above as I was seeing some out of memory messages on my IA64 box
> with complete rc3-mm2 patch.
yes, my ia64 box goes oom during boot too. Something's broken in Tony's
ia64 tree.
Make kobject_hotplug() work even if the kobject's kset doesn't implement any
hotplug_ops.
Signed-off-by: Andrew Morton <[email protected]>
---
25-akpm/lib/kobject_uevent.c | 19 ++++++++++++-------
1 files changed, 12 insertions(+), 7 deletions(-)
diff -puN lib/kobject_uevent.c~kobject_hotplug-no-hotplug_ops lib/kobject_uevent.c
--- 25/lib/kobject_uevent.c~kobject_hotplug-no-hotplug_ops 2004-10-05 10:11:59.404291240 -0700
+++ 25-akpm/lib/kobject_uevent.c 2004-10-05 10:13:59.132089848 -0700
@@ -191,6 +191,8 @@ void kobject_hotplug(struct kobject *kob
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 {
@@ -198,15 +200,18 @@ void kobject_hotplug(struct kobject *kob
} 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;
}
@@ -225,8 +230,8 @@ void kobject_hotplug(struct kobject *kob
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;
@@ -260,9 +265,9 @@ void kobject_hotplug(struct kobject *kob
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) {
_
On Tue, Oct 05, 2004 at 11:23:09AM -0700, Andrew Morton wrote:
> Keshavamurthy Anil S <[email protected]> wrote:
Andrew,
My patch which removes cpu_run_sbin_hotplug() to use kobject_hotplug() works
fine with your patch which modifies kobject_hotplug().
I am able to get the same cpu "offline" notifications to user land.
You can safely apply both of our patches.
Thanks and regards,
Anil