public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2.6][resend] Add DEVPATH env variable to hotplug helper call
@ 2004-10-03 10:08 Josef 'Jeff' Sipek
  2004-10-03 23:20 ` Andrew Morton
  0 siblings, 1 reply; 12+ messages in thread
From: Josef 'Jeff' Sipek @ 2004-10-03 10:08 UTC (permalink / raw)
  To: linux-kernel; +Cc: akpm, torvalds, trivial, rusty

Add $DEVPATH to the environmental variables during /sbin/hotplug call.

Josef 'Jeff' Sipek.

Signed-off-by: Josef 'Jeff' Sipek <jeffpc@optonline.net>


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);


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6][resend] Add DEVPATH env variable to hotplug helper call
  2004-10-03 10:08 [PATCH 2.6][resend] Add DEVPATH env variable to hotplug helper call Josef 'Jeff' Sipek
@ 2004-10-03 23:20 ` Andrew Morton
  2004-10-04 17:22   ` Keshavamurthy Anil S
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2004-10-03 23:20 UTC (permalink / raw)
  To: Josef 'Jeff' Sipek; +Cc: linux-kernel, torvalds, trivial, rusty

"Josef 'Jeff' Sipek" <jeffpc@optonline.net> wrote:
>
> Add $DEVPATH to the environmental variables during /sbin/hotplug call.
> 
>  Josef 'Jeff' Sipek.
> 
>  Signed-off-by: Josef 'Jeff' Sipek <jeffpc@optonline.net>
> 
> 
>  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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6][resend] Add DEVPATH env variable to hotplug helper call
  2004-10-03 23:20 ` Andrew Morton
@ 2004-10-04 17:22   ` Keshavamurthy Anil S
  2004-10-04 19:37     ` Andrew Morton
  0 siblings, 1 reply; 12+ messages in thread
From: Keshavamurthy Anil S @ 2004-10-04 17:22 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Josef 'Jeff' Sipek, linux-kernel, torvalds, trivial,
	rusty

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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6][resend] Add DEVPATH env variable to hotplug helper call
  2004-10-04 17:22   ` Keshavamurthy Anil S
@ 2004-10-04 19:37     ` Andrew Morton
  2004-10-04 19:43       ` Keshavamurthy Anil S
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2004-10-04 19:37 UTC (permalink / raw)
  To: Keshavamurthy Anil S; +Cc: jeffpc, linux-kernel, torvalds, trivial, rusty

Keshavamurthy Anil S <anil.s.keshavamurthy@intel.com> 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().

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6][resend] Add DEVPATH env variable to hotplug helper call
  2004-10-04 19:37     ` Andrew Morton
@ 2004-10-04 19:43       ` Keshavamurthy Anil S
  2004-10-05  8:25         ` Keshavamurthy Anil S
  0 siblings, 1 reply; 12+ messages in thread
From: Keshavamurthy Anil S @ 2004-10-04 19:43 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Keshavamurthy Anil S, jeffpc, linux-kernel, torvalds, trivial,
	rusty

On Mon, Oct 04, 2004 at 12:37:25PM -0700, Andrew Morton wrote:
> Keshavamurthy Anil S <anil.s.keshavamurthy@intel.com> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6][resend] Add DEVPATH env variable to hotplug helper call
  2004-10-04 19:43       ` Keshavamurthy Anil S
@ 2004-10-05  8:25         ` Keshavamurthy Anil S
  2004-10-05 17:18           ` Andrew Morton
  0 siblings, 1 reply; 12+ messages in thread
From: Keshavamurthy Anil S @ 2004-10-05  8:25 UTC (permalink / raw)
  To: Keshavamurthy Anil S, akpm
  Cc: Andrew Morton, jeffpc, linux-kernel, torvalds, trivial, rusty,
	Greg KH

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 <anil.s.keshavamurthy@intel.com> 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 <anil.s.keshavamurthy@intel.com>

---

 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)
_

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6][resend] Add DEVPATH env variable to hotplug helper call
  2004-10-05  8:25         ` Keshavamurthy Anil S
@ 2004-10-05 17:18           ` Andrew Morton
  2004-10-05 17:27             ` Keshavamurthy Anil S
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2004-10-05 17:18 UTC (permalink / raw)
  To: Keshavamurthy Anil S; +Cc: jeffpc, linux-kernel, torvalds, trivial, rusty, greg

Keshavamurthy Anil S <anil.s.keshavamurthy@intel.com> 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 <akpm@osdl.org>
---

 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) {
_


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6][resend] Add DEVPATH env variable to hotplug helper call
  2004-10-05 17:18           ` Andrew Morton
@ 2004-10-05 17:27             ` Keshavamurthy Anil S
  2004-10-05 17:47               ` Andrew Morton
  0 siblings, 1 reply; 12+ messages in thread
From: Keshavamurthy Anil S @ 2004-10-05 17:27 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Keshavamurthy Anil S, jeffpc, linux-kernel, torvalds, trivial,
	rusty, greg

On Tue, Oct 05, 2004 at 10:18:23AM -0700, Andrew Morton wrote:
> Keshavamurthy Anil S <anil.s.keshavamurthy@intel.com> 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?


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6][resend] Add DEVPATH env variable to hotplug helper call
  2004-10-05 17:27             ` Keshavamurthy Anil S
@ 2004-10-05 17:47               ` Andrew Morton
  2004-10-05 18:01                 ` Keshavamurthy Anil S
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2004-10-05 17:47 UTC (permalink / raw)
  To: Keshavamurthy Anil S; +Cc: jeffpc, linux-kernel, torvalds, trivial, rusty, greg

Keshavamurthy Anil S <anil.s.keshavamurthy@intel.com> wrote:
>
> On Tue, Oct 05, 2004 at 10:18:23AM -0700, Andrew Morton wrote:
>  > Keshavamurthy Anil S <anil.s.keshavamurthy@intel.com> 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.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6][resend] Add DEVPATH env variable to hotplug helper call
  2004-10-05 17:47               ` Andrew Morton
@ 2004-10-05 18:01                 ` Keshavamurthy Anil S
  2004-10-05 18:23                   ` Andrew Morton
  0 siblings, 1 reply; 12+ messages in thread
From: Keshavamurthy Anil S @ 2004-10-05 18:01 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Keshavamurthy Anil S, jeffpc, linux-kernel, torvalds, trivial,
	rusty, greg

On Tue, Oct 05, 2004 at 10:47:44AM -0700, Andrew Morton wrote:
> Keshavamurthy Anil S <anil.s.keshavamurthy@intel.com> wrote:
> >
> > On Tue, Oct 05, 2004 at 10:18:23AM -0700, Andrew Morton wrote:
> >  > Keshavamurthy Anil S <anil.s.keshavamurthy@intel.com> 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);
 
_


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6][resend] Add DEVPATH env variable to hotplug helper call
  2004-10-05 18:01                 ` Keshavamurthy Anil S
@ 2004-10-05 18:23                   ` Andrew Morton
  2004-10-05 19:02                     ` Keshavamurthy Anil S
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2004-10-05 18:23 UTC (permalink / raw)
  To: Keshavamurthy Anil S; +Cc: jeffpc, linux-kernel, torvalds, trivial, rusty, greg

Keshavamurthy Anil S <anil.s.keshavamurthy@intel.com> 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 <akpm@osdl.org>
---

 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) {
_


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2.6][resend] Add DEVPATH env variable to hotplug helper call
  2004-10-05 18:23                   ` Andrew Morton
@ 2004-10-05 19:02                     ` Keshavamurthy Anil S
  0 siblings, 0 replies; 12+ messages in thread
From: Keshavamurthy Anil S @ 2004-10-05 19:02 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Keshavamurthy Anil S, jeffpc, linux-kernel, torvalds, trivial,
	rusty, greg

On Tue, Oct 05, 2004 at 11:23:09AM -0700, Andrew Morton wrote:
> Keshavamurthy Anil S <anil.s.keshavamurthy@intel.com> 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


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2004-10-05 19:03 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-10-03 10:08 [PATCH 2.6][resend] Add DEVPATH env variable to hotplug helper call Josef 'Jeff' Sipek
2004-10-03 23:20 ` Andrew Morton
2004-10-04 17:22   ` Keshavamurthy Anil S
2004-10-04 19:37     ` Andrew Morton
2004-10-04 19:43       ` Keshavamurthy Anil S
2004-10-05  8:25         ` Keshavamurthy Anil S
2004-10-05 17:18           ` Andrew Morton
2004-10-05 17:27             ` Keshavamurthy Anil S
2004-10-05 17:47               ` Andrew Morton
2004-10-05 18:01                 ` Keshavamurthy Anil S
2004-10-05 18:23                   ` Andrew Morton
2004-10-05 19:02                     ` Keshavamurthy Anil S

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox