[patch 0/7] A series of cleanup patches

[patch 0/7] A series of cleanup patches

[Jeremy Fitzhardinge]

Andi, Andrew:

Here's a little set of cleanup patches.  They're general cleanups,
but also the final prerequisites for the Xen series.

They are:
 - move pieces of i386 SMP into a common file, so they can be used by
   other sub-arches and Xen
 - Make vsyscall (and vdso) use the common linux/elfnote.h macros
 - Add kstrndup
 - Add argv_split
 - Cleanup and generalize the call_usermodehelper APIs
 - Add a common orderly poweroff function
 - And some more usermodehelper cleanups, to change the "wait" arg from
   a semi-cryptic int into a less cryptic enum.

Thanks,
	J

-- 

[patch 1/7] i386: move common parts of smp into their own file

[Jeremy Fitzhardinge]

[-- Attachment #1: i386-common-smp.patch --]
[-- Type: text/plain, Size: 8779 bytes --]

Several parts of kernel/smp.c and smpboot.c are generally useful for
other subarchitectures and paravirt_ops implementations, so make them
available for reuse.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Acked-by: Chris Wright <chrisw@sous-sol.org>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>

---
 arch/i386/kernel/Makefile    |    1 
 arch/i386/kernel/smp.c       |   65 +++-------------------------------
 arch/i386/kernel/smpboot.c   |   22 -----------
 arch/i386/kernel/smpcommon.c |   79 ++++++++++++++++++++++++++++++++++++++++++
 include/asm-i386/processor.h |    4 ++
 5 files changed, 90 insertions(+), 81 deletions(-)

===================================================================
--- a/arch/i386/kernel/Makefile
+++ b/arch/i386/kernel/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_MICROCODE)		+= microcode.o
 obj-$(CONFIG_MICROCODE)		+= microcode.o
 obj-$(CONFIG_APM)		+= apm.o
 obj-$(CONFIG_X86_SMP)		+= smp.o smpboot.o tsc_sync.o
+obj-$(CONFIG_SMP)		+= smpcommon.o
 obj-$(CONFIG_X86_TRAMPOLINE)	+= trampoline.o
 obj-$(CONFIG_X86_MPPARSE)	+= mpparse.o
 obj-$(CONFIG_X86_LOCAL_APIC)	+= apic.o nmi.o
===================================================================
--- a/arch/i386/kernel/smp.c
+++ b/arch/i386/kernel/smp.c
@@ -468,7 +468,7 @@ void flush_tlb_all(void)
  * it goes straight through and wastes no time serializing
  * anything. Worst case is that we lose a reschedule ...
  */
-void native_smp_send_reschedule(int cpu)
+static void native_smp_send_reschedule(int cpu)
 {
 	WARN_ON(cpu_is_offline(cpu));
 	send_IPI_mask(cpumask_of_cpu(cpu), RESCHEDULE_VECTOR);
@@ -547,9 +547,10 @@ static void __smp_call_function(void (*f
  * You must not call this function with disabled interrupts or from a
  * hardware interrupt handler or from a bottom half handler.
  */
-int native_smp_call_function_mask(cpumask_t mask,
-				  void (*func)(void *), void *info,
-				  int wait)
+static int
+native_smp_call_function_mask(cpumask_t mask,
+			      void (*func)(void *), void *info,
+			      int wait)
 {
 	struct call_data_struct data;
 	cpumask_t allbutself;
@@ -600,60 +601,6 @@ int native_smp_call_function_mask(cpumas
 	return 0;
 }
 
-/**
- * smp_call_function(): Run a function on all other CPUs.
- * @func: The function to run. This must be fast and non-blocking.
- * @info: An arbitrary pointer to pass to the function.
- * @nonatomic: Unused.
- * @wait: If true, wait (atomically) until function has completed on other CPUs.
- *
- * Returns 0 on success, else a negative status code.
- *
- * If @wait is true, then returns once @func has returned; otherwise
- * it returns just before the target cpu calls @func.
- *
- * You must not call this function with disabled interrupts or from a
- * hardware interrupt handler or from a bottom half handler.
- */
-int smp_call_function(void (*func) (void *info), void *info, int nonatomic,
-		      int wait)
-{
-	return smp_call_function_mask(cpu_online_map, func, info, wait);
-}
-EXPORT_SYMBOL(smp_call_function);
-
-/**
- * smp_call_function_single - Run a function on another CPU
- * @cpu: The target CPU.  Cannot be the calling CPU.
- * @func: The function to run. This must be fast and non-blocking.
- * @info: An arbitrary pointer to pass to the function.
- * @nonatomic: Unused.
- * @wait: If true, wait until function has completed on other CPUs.
- *
- * Returns 0 on success, else a negative status code.
- *
- * If @wait is true, then returns once @func has returned; otherwise
- * it returns just before the target cpu calls @func.
- */
-int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
-			     int nonatomic, int wait)
-{
-	/* prevent preemption and reschedule on another processor */
-	int ret;
-	int me = get_cpu();
-	if (cpu == me) {
-		WARN_ON(1);
-		put_cpu();
-		return -EBUSY;
-	}
-
-	ret = smp_call_function_mask(cpumask_of_cpu(cpu), func, info, wait);
-
-	put_cpu();
-	return ret;
-}
-EXPORT_SYMBOL(smp_call_function_single);
-
 static void stop_this_cpu (void * dummy)
 {
 	local_irq_disable();
@@ -671,7 +618,7 @@ static void stop_this_cpu (void * dummy)
  * this function calls the 'stop' function on all other CPUs in the system.
  */
 
-void native_smp_send_stop(void)
+static void native_smp_send_stop(void)
 {
 	/* Don't deadlock on the call lock in panic */
 	int nolock = !spin_trylock(&call_lock);
===================================================================
--- a/arch/i386/kernel/smpboot.c
+++ b/arch/i386/kernel/smpboot.c
@@ -99,9 +99,6 @@ EXPORT_SYMBOL(x86_cpu_to_apicid);
 
 u8 apicid_2_node[MAX_APICID];
 
-DEFINE_PER_CPU(unsigned long, this_cpu_off);
-EXPORT_PER_CPU_SYMBOL(this_cpu_off);
-
 /*
  * Trampoline 80x86 program as an array.
  */
@@ -766,25 +763,6 @@ static inline struct task_struct * alloc
 #define alloc_idle_task(cpu) fork_idle(cpu)
 #endif
 
-/* Initialize the CPU's GDT.  This is either the boot CPU doing itself
-   (still using the master per-cpu area), or a CPU doing it for a
-   secondary which will soon come up. */
-static __cpuinit void init_gdt(int cpu)
-{
-	struct desc_struct *gdt = get_cpu_gdt_table(cpu);
-
-	pack_descriptor((u32 *)&gdt[GDT_ENTRY_PERCPU].a,
-			(u32 *)&gdt[GDT_ENTRY_PERCPU].b,
-			__per_cpu_offset[cpu], 0xFFFFF,
-			0x80 | DESCTYPE_S | 0x2, 0x8);
-
-	per_cpu(this_cpu_off, cpu) = __per_cpu_offset[cpu];
-	per_cpu(cpu_number, cpu) = cpu;
-}
-
-/* Defined in head.S */
-extern struct Xgt_desc_struct early_gdt_descr;
-
 static int __cpuinit do_boot_cpu(int apicid, int cpu)
 /*
  * NOTE - on most systems this is a PHYSICAL apic ID, but on multiquad
===================================================================
--- /dev/null
+++ b/arch/i386/kernel/smpcommon.c
@@ -0,0 +1,79 @@
+/*
+ * SMP stuff which is common to all sub-architectures.
+ */
+#include <linux/module.h>
+#include <asm/smp.h>
+
+DEFINE_PER_CPU(unsigned long, this_cpu_off);
+EXPORT_PER_CPU_SYMBOL(this_cpu_off);
+
+/* Initialize the CPU's GDT.  This is either the boot CPU doing itself
+   (still using the master per-cpu area), or a CPU doing it for a
+   secondary which will soon come up. */
+__cpuinit void init_gdt(int cpu)
+{
+	struct desc_struct *gdt = get_cpu_gdt_table(cpu);
+
+	pack_descriptor((u32 *)&gdt[GDT_ENTRY_PERCPU].a,
+			(u32 *)&gdt[GDT_ENTRY_PERCPU].b,
+			__per_cpu_offset[cpu], 0xFFFFF,
+			0x80 | DESCTYPE_S | 0x2, 0x8);
+
+	per_cpu(this_cpu_off, cpu) = __per_cpu_offset[cpu];
+	per_cpu(cpu_number, cpu) = cpu;
+}
+
+
+/**
+ * smp_call_function(): Run a function on all other CPUs.
+ * @func: The function to run. This must be fast and non-blocking.
+ * @info: An arbitrary pointer to pass to the function.
+ * @nonatomic: Unused.
+ * @wait: If true, wait (atomically) until function has completed on other CPUs.
+ *
+ * Returns 0 on success, else a negative status code.
+ *
+ * If @wait is true, then returns once @func has returned; otherwise
+ * it returns just before the target cpu calls @func.
+ *
+ * You must not call this function with disabled interrupts or from a
+ * hardware interrupt handler or from a bottom half handler.
+ */
+int smp_call_function(void (*func) (void *info), void *info, int nonatomic,
+		      int wait)
+{
+	return smp_call_function_mask(cpu_online_map, func, info, wait);
+}
+EXPORT_SYMBOL(smp_call_function);
+
+/**
+ * smp_call_function_single - Run a function on another CPU
+ * @cpu: The target CPU.  Cannot be the calling CPU.
+ * @func: The function to run. This must be fast and non-blocking.
+ * @info: An arbitrary pointer to pass to the function.
+ * @nonatomic: Unused.
+ * @wait: If true, wait until function has completed on other CPUs.
+ *
+ * Returns 0 on success, else a negative status code.
+ *
+ * If @wait is true, then returns once @func has returned; otherwise
+ * it returns just before the target cpu calls @func.
+ */
+int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
+			     int nonatomic, int wait)
+{
+	/* prevent preemption and reschedule on another processor */
+	int ret;
+	int me = get_cpu();
+	if (cpu == me) {
+		WARN_ON(1);
+		put_cpu();
+		return -EBUSY;
+	}
+
+	ret = smp_call_function_mask(cpumask_of_cpu(cpu), func, info, wait);
+
+	put_cpu();
+	return ret;
+}
+EXPORT_SYMBOL(smp_call_function_single);
===================================================================
--- a/include/asm-i386/processor.h
+++ b/include/asm-i386/processor.h
@@ -749,9 +749,13 @@ extern void enable_sep_cpu(void);
 extern void enable_sep_cpu(void);
 extern int sysenter_setup(void);
 
+/* Defined in head.S */
+extern struct Xgt_desc_struct early_gdt_descr;
+
 extern void cpu_set_gdt(int);
 extern void switch_to_new_gdt(void);
 extern void cpu_init(void);
+extern void init_gdt(int cpu);
 
 extern int force_mwait;
 

-- 

[patch 2/7] use elfnote.h to generate vsyscall notes.

[Jeremy Fitzhardinge]

[-- Attachment #1: vsyscall-note-use-elfnote_h.patch --]
[-- Type: text/plain, Size: 2978 bytes --]

Use existing elfnote.h to generate vsyscall notes, rather than doing
it locally.  Changes elfnote.h a bit to suite, since this is the first
asm user, and it wasn't quite right.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Roland McGrath <roland@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.com>

---
 arch/i386/kernel/vsyscall-note.S |   23 ++++++-----------------
 arch/x86_64/vdso/vdso-note.S     |    4 ++--
 include/linux/elfnote.h          |   18 +++++++++++++-----
 3 files changed, 21 insertions(+), 24 deletions(-)

===================================================================
--- a/arch/i386/kernel/vsyscall-note.S
+++ b/arch/i386/kernel/vsyscall-note.S
@@ -3,23 +3,12 @@
  * Here we can supply some information useful to userland.
  */
 
-#include <linux/uts.h>
 #include <linux/version.h>
+#include <linux/elfnote.h>
 
-#define ASM_ELF_NOTE_BEGIN(name, flags, vendor, type)			      \
-	.section name, flags;						      \
-	.balign 4;							      \
-	.long 1f - 0f;		/* name length */			      \
-	.long 3f - 2f;		/* data length */			      \
-	.long type;		/* note type */				      \
-0:	.asciz vendor;		/* vendor name */			      \
-1:	.balign 4;							      \
-2:
-
-#define ASM_ELF_NOTE_END						      \
-3:	.balign 4;		/* pad out section */			      \
-	.previous
-
-	ASM_ELF_NOTE_BEGIN(".note.kernel-version", "a", UTS_SYSNAME, 0)
+/* Ideally this would use UTS_NAME, but using a quoted string here
+   doesn't work. Remember to change this when changing the
+   kernel's name. */
+ELFNOTE_START(Linux, 0, "a")
 	.long LINUX_VERSION_CODE
-	ASM_ELF_NOTE_END
+ELFNOTE_END
===================================================================
--- a/arch/x86_64/vdso/vdso-note.S
+++ b/arch/x86_64/vdso/vdso-note.S
@@ -8,5 +8,5 @@
 #include <linux/elfnote.h>
 
 	/* Supply version information to coredumps */
-	ELFNOTE(kernelversion, 0, .long, LINUX_VERSION_CODE)
-	ELFNOTE(os, 0, .asciz, UTS_SYSNAME)
+	ELFNOTE(kernelversion, 0, .long LINUX_VERSION_CODE)
+	ELFNOTE(os, 0, .asciz UTS_SYSNAME)
===================================================================
--- a/include/linux/elfnote.h
+++ b/include/linux/elfnote.h
@@ -38,17 +38,25 @@
  * e.g. ELFNOTE(XYZCo, 42, .asciz, "forty-two")
  *      ELFNOTE(XYZCo, 12, .long, 0xdeadbeef)
  */
-#define ELFNOTE(name, type, desctype, descdata)	\
-.pushsection .note.name, "",@note	;	\
+#define ELFNOTE_START(name, type, flags)	\
+.pushsection .note.name, flags,@note	;	\
   .align 4				;	\
   .long 2f - 1f		/* namesz */	;	\
-  .long 4f - 3f		/* descsz */	;	\
+  .long 4484f - 3f	/* descsz */	;	\
   .long type				;	\
 1:.asciz #name				;	\
 2:.align 4				;	\
-3:desctype descdata			;	\
-4:.align 4				;	\
+3:
+
+#define ELFNOTE_END				\
+4484:.align 4				;	\
 .popsection				;
+
+#define ELFNOTE(name, type, desc)		\
+	ELFNOTE_START(name, type, "")		\
+		desc			;	\
+	ELFNOTE_END
+
 #else	/* !__ASSEMBLER__ */
 #include <linux/elf.h>
 /*

-- 

Re: [patch 2/7] use elfnote.h to generate vsyscall notes.

[Roland McGrath]

I used .balign because .align has inconsistent meaning on some platforms
(matches .p2align instead of .balign).  Using .align in a
machine-independent file seems questionable.


Thanks,
Roland

Re: [patch 2/7] use elfnote.h to generate vsyscall notes.

[Jeremy Fitzhardinge]

Roland McGrath wrote:
> I used .balign because .align has inconsistent meaning on some platforms
> (matches .p2align instead of .balign).  Using .align in a
> machine-independent file seems questionable.
>   

OK, good point.

    J

Re: [patch 2/7] use elfnote.h to generate vsyscall notes.

[Jeremy Fitzhardinge]

Roland McGrath wrote:
> I used .balign because .align has inconsistent meaning on some platforms
> (matches .p2align instead of .balign).  Using .align in a
> machine-independent file seems questionable.
>
>
>   

Subject: use balign in elfnote.h

Roland McGrath <roland@redhat.com>:
> I used .balign because .align has inconsistent meaning on some platforms
> (matches .p2align instead of .balign).  Using .align in a
> machine-independent file seems questionable.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: Roland McGrath <roland@redhat.com>

---
 include/linux/elfnote.h |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

===================================================================
--- a/include/linux/elfnote.h
+++ b/include/linux/elfnote.h
@@ -40,16 +40,16 @@
  */
 #define ELFNOTE_START(name, type, flags)	\
 .pushsection .note.name, flags,@note	;	\
-  .align 4				;	\
+  .balign 4				;	\
   .long 2f - 1f		/* namesz */	;	\
   .long 4484f - 3f	/* descsz */	;	\
   .long type				;	\
 1:.asciz #name				;	\
-2:.align 4				;	\
+2:.balign 4				;	\
 3:
 
 #define ELFNOTE_END				\
-4484:.align 4				;	\
+4484:.balign 4				;	\
 .popsection				;
 
 #define ELFNOTE(name, type, desc)		\

[patch 3/7] add kstrndup

[Jeremy Fitzhardinge]

[-- Attachment #1: add-kstrndup.patch --]
[-- Type: text/plain, Size: 5029 bytes --]

Add a kstrndup function, modelled on strndup.  Like strndup this
returns a string copied into its own allocated memory, but it copies
no more than the specified number of bytes from the source.

Remove private strndup() from irda code.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: Chris Wright <chrisw@sous-sol.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Randy Dunlap <randy.dunlap@oracle.com>
Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Cc: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Arnaldo Carvalho de Melo <acme@mandriva.com>
Cc: Al Viro <viro@ftp.linux.org.uk>
Cc: Panagiotis Issaris <takis@issaris.org>

---
 include/linux/string.h  |    1 +
 mm/util.c               |   30 +++++++++++++++++++++++++++---
 net/irda/irias_object.c |   43 +++++--------------------------------------
 3 files changed, 33 insertions(+), 41 deletions(-)

===================================================================
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -105,6 +105,7 @@ extern void * memchr(const void *,int,__
 #endif
 
 extern char *kstrdup(const char *s, gfp_t gfp);
+extern char *kstrndup(const char *s, size_t len, gfp_t gfp);
 extern void *kmemdup(const void *src, size_t len, gfp_t gfp);
 
 #ifdef __cplusplus
===================================================================
--- a/mm/util.c
+++ b/mm/util.c
@@ -18,9 +18,8 @@ void *__kzalloc(size_t size, gfp_t flags
 }
 EXPORT_SYMBOL(__kzalloc);
 
-/*
+/**
  * kstrdup - allocate space for and copy an existing string
- *
  * @s: the string to duplicate
  * @gfp: the GFP mask used in the kmalloc() call when allocating memory
  */
@@ -39,6 +38,32 @@ char *kstrdup(const char *s, gfp_t gfp)
 	return buf;
 }
 EXPORT_SYMBOL(kstrdup);
+
+/**
+ * kstrndup - allocate space for and copy an existing string
+ * @s: the string to duplicate
+ * @max: read at most @max chars from @s
+ * @gfp: the GFP mask used in the kmalloc() call when allocating memory
+ */
+char *kstrndup(const char *s, size_t max, gfp_t gfp)
+{
+	size_t len;
+	char *buf;
+
+	if (!s)
+		return NULL;
+
+	len = strlen(s);
+	if (len > max)
+		len = max;
+	buf = kmalloc_track_caller(len+1, gfp);
+ 	if (buf) {
+		memcpy(buf, s, len);
+		buf[len] = '\0';
+	}
+	return buf;
+}
+EXPORT_SYMBOL(kstrndup);
 
 /**
  * kmemdup - duplicate region of memory
@@ -60,7 +85,6 @@ EXPORT_SYMBOL(kmemdup);
 
 /*
  * strndup_user - duplicate an existing string from user space
- *
  * @s: The string to duplicate
  * @n: Maximum number of bytes to copy, including the trailing NUL.
  */
===================================================================
--- a/net/irda/irias_object.c
+++ b/net/irda/irias_object.c
@@ -36,39 +36,6 @@ hashbin_t *irias_objects;
  */
 struct ias_value irias_missing = { IAS_MISSING, 0, 0, 0, {0}};
 
-/*
- * Function strndup (str, max)
- *
- *    My own kernel version of strndup!
- *
- * Faster, check boundary... Jean II
- */
-static char *strndup(char *str, size_t max)
-{
-	char *new_str;
-	int len;
-
-	/* Check string */
-	if (str == NULL)
-		return NULL;
-	/* Check length, truncate */
-	len = strlen(str);
-	if(len > max)
-		len = max;
-
-	/* Allocate new string */
-	new_str = kmalloc(len + 1, GFP_ATOMIC);
-	if (new_str == NULL) {
-		IRDA_WARNING("%s: Unable to kmalloc!\n", __FUNCTION__);
-		return NULL;
-	}
-
-	/* Copy and truncate */
-	memcpy(new_str, str, len);
-	new_str[len] = '\0';
-
-	return new_str;
-}
 
 /*
  * Function ias_new_object (name, id)
@@ -90,7 +57,7 @@ struct ias_object *irias_new_object( cha
 	}
 
 	obj->magic = IAS_OBJECT_MAGIC;
-	obj->name = strndup(name, IAS_MAX_CLASSNAME);
+	obj->name = kstrndup(name, IAS_MAX_CLASSNAME, GFP_ATOMIC);
 	if (!obj->name) {
 		IRDA_WARNING("%s(), Unable to allocate name!\n",
 			     __FUNCTION__);
@@ -360,7 +327,7 @@ void irias_add_integer_attrib(struct ias
 	}
 
 	attrib->magic = IAS_ATTRIB_MAGIC;
-	attrib->name = strndup(name, IAS_MAX_ATTRIBNAME);
+	attrib->name = kstrndup(name, IAS_MAX_ATTRIBNAME, GFP_ATOMIC);
 
 	/* Insert value */
 	attrib->value = irias_new_integer_value(value);
@@ -404,7 +371,7 @@ void irias_add_octseq_attrib(struct ias_
 	}
 
 	attrib->magic = IAS_ATTRIB_MAGIC;
-	attrib->name = strndup(name, IAS_MAX_ATTRIBNAME);
+	attrib->name = kstrndup(name, IAS_MAX_ATTRIBNAME, GFP_ATOMIC);
 
 	attrib->value = irias_new_octseq_value( octets, len);
 	if (!attrib->name || !attrib->value) {
@@ -446,7 +413,7 @@ void irias_add_string_attrib(struct ias_
 	}
 
 	attrib->magic = IAS_ATTRIB_MAGIC;
-	attrib->name = strndup(name, IAS_MAX_ATTRIBNAME);
+	attrib->name = kstrndup(name, IAS_MAX_ATTRIBNAME, GFP_ATOMIC);
 
 	attrib->value = irias_new_string_value(value);
 	if (!attrib->name || !attrib->value) {
@@ -506,7 +473,7 @@ struct ias_value *irias_new_string_value
 
 	value->type = IAS_STRING;
 	value->charset = CS_ASCII;
-	value->t.string = strndup(string, IAS_MAX_STRING);
+	value->t.string = kstrndup(string, IAS_MAX_STRING, GFP_ATOMIC);
 	if (!value->t.string) {
 		IRDA_WARNING("%s: Unable to kmalloc!\n", __FUNCTION__);
 		kfree(value);

-- 

Re: [patch 3/7] add kstrndup

[René Scharfe]

> +	len = strlen(s);
> +	if (len > max)
> +		len = max;

You can avoid to run beyond max and save two lines by using strnlen:

	len = strnlen(s, max);

René

Re: [patch 3/7] add kstrndup

[Jeremy Fitzhardinge]

René Scharfe wrote:
>> +	len = strlen(s);
>> +	if (len > max)
>> +		len = max;
>>     
>
> You can avoid to run beyond max and save two lines by using strnlen:
>
> 	len = strnlen(s, max);
>   

OK.  Didn't know about strnlen.

Subject: Use strnlen in kstrndup

strnlen prevents over-reading the input.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: René Scharfe <rene.scharfe@lsrfire.ath.cx>

---
 mm/util.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

===================================================================
--- a/mm/util.c
+++ b/mm/util.c
@@ -53,9 +53,7 @@ char *kstrndup(const char *s, size_t max
 	if (!s)
 		return NULL;
 
-	len = strlen(s);
-	if (len > max)
-		len = max;
+	len = strnlen(s, max);
 	buf = kmalloc_track_caller(len+1, gfp);
  	if (buf) {
 		memcpy(buf, s, len);

[patch 4/7] add argv_split()

[Jeremy Fitzhardinge]

[-- Attachment #1: split-argv.patch --]
[-- Type: text/plain, Size: 4630 bytes --]

argv_split() is a helper function which takes a string, splits it at
whitespace, and returns a NULL-terminated argv vector.  This is
deliberately simple - it does no quote processing of any kind.

[ Seems to me that this is something which is already being done in
  the kernel, but I couldn't find any other implementations, either to
  steal or replace.  Keep an eye out. ]

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: Chris Wright <chrisw@sous-sol.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Randy Dunlap <randy.dunlap@oracle.com>

---
 include/linux/string.h |    3 
 lib/Makefile           |    2 
 lib/argv_split.c       |  159 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 163 insertions(+), 1 deletion(-)

===================================================================
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -108,6 +108,9 @@ extern char *kstrndup(const char *s, siz
 extern char *kstrndup(const char *s, size_t len, gfp_t gfp);
 extern void *kmemdup(const void *src, size_t len, gfp_t gfp);
 
+extern char **argv_split(gfp_t gfp, const char *str, int *argcp);
+extern void argv_free(char **argv);
+
 #ifdef __cplusplus
 }
 #endif
===================================================================
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -5,7 +5,7 @@ lib-y := ctype.o string.o vsprintf.o cmd
 lib-y := ctype.o string.o vsprintf.o cmdline.o \
 	 rbtree.o radix-tree.o dump_stack.o \
 	 idr.o int_sqrt.o bitmap.o extable.o prio_tree.o \
-	 sha1.o irq_regs.o reciprocal_div.o
+	 sha1.o irq_regs.o reciprocal_div.o argv_split.o
 
 lib-$(CONFIG_MMU) += ioremap.o
 lib-$(CONFIG_SMP) += cpumask.o
===================================================================
--- /dev/null
+++ b/lib/argv_split.c
@@ -0,0 +1,159 @@
+/*
+ * Helper function for splitting a string into an argv-like array.
+ */
+
+#ifndef TEST
+#include <linux/kernel.h>
+#include <linux/ctype.h>
+#include <linux/bug.h>
+#endif
+
+static const char *skip_sep(const char *cp)
+{
+	while (*cp && isspace(*cp))
+		cp++;
+
+	return cp;
+}
+
+static const char *skip_arg(const char *cp)
+{
+	while (*cp && !isspace(*cp))
+		cp++;
+
+	return cp;
+}
+
+static int count_argc(const char *str)
+{
+	int count = 0;
+
+	while (*str) {
+		str = skip_sep(str);
+		if (*str) {
+			count++;
+			str = skip_arg(str);
+		}
+	}
+
+	return count;
+}
+
+/**
+ * argv_free - free an argv
+ *
+ * @argv - the argument vector to be freed
+ *
+ * Frees an argv and the strings it points to.
+ */
+void argv_free(char **argv)
+{
+	char **p;
+	for (p = argv; *p; p++)
+		kfree(*p);
+
+	kfree(argv);
+}
+EXPORT_SYMBOL(argv_free);
+
+/**
+ * argv_split - split a string at whitespace, returning an argv
+ * @gfp: the GFP mask used to allocate memory
+ * @str: the string to be split
+ * @argcp: returned argument count
+ *
+ * Returns an array of pointers to strings which are split out from
+ * @str.  This is performed by strictly splitting on white-space; no
+ * quote processing is performed.  Multiple whitespace characters are
+ * considered to be a single argument separator.  The returned array
+ * is always NULL-terminated.  Returns NULL on memory allocation
+ * failure.
+ */
+char **argv_split(gfp_t gfp, const char *str, int *argcp)
+{
+	int argc = count_argc(str);
+	char **argv = kzalloc(sizeof(*argv) * (argc+1), gfp);
+	char **argvp;
+
+	if (argv == NULL)
+		goto out;
+
+	*argcp = argc;
+	argvp = argv;
+
+	while (*str) {
+		str = skip_sep(str);
+
+		if (*str) {
+			const char *p = str;
+			char *t;
+
+			str = skip_arg(str);
+
+			t = kstrndup(p, str-p, gfp);
+			if (t == NULL)
+				goto fail;
+			*argvp++ = t;
+		}
+	}
+	*argvp = NULL;
+
+  out:
+	return argv;
+
+  fail:
+	argv_free(argv);
+	return NULL;
+}
+EXPORT_SYMBOL(argv_split);
+
+#ifdef TEST
+#define _GNU_SOURCE
+#include <ctype.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+
+typedef enum {
+	GFP_KERNEL,
+} gfp_t;
+#define kzalloc(size, x)	malloc(size)
+#define kfree(x)		free(x)
+#define kstrndup(s, n, gfp)	strndup(s, n)
+#define BUG()	abort()
+
+int main() {
+	const char *testvec[] = {
+		"",
+		"x",
+		"\"",
+		"\\\0",
+		"\"",
+		"test one two three",
+		"arg\"foo\"bar biff",
+		"one two\\ three four",
+		"one \"two three\" four",
+		NULL,
+	};
+	const char **t;
+
+	for (t = testvec; *t; t++) {
+		char **argv;
+		int argc;
+		char **a;
+
+		printf("%d: test [%s]\n", t-testvec, *t);
+
+		argv = argv_split(GFP_KERNEL, *t, &argc);
+
+		printf("argc=%d vec=", argc);
+		for (a = argv; *a; a++)
+			printf("[%s] ", *a);
+		printf("\n");
+
+		argv_free(argv);
+	}
+
+	return 0;
+}
+#endif

-- 

Re: [patch 4/7] add argv_split()

[Randy Dunlap]

On Thu, 10 May 2007 16:57:12 -0700 Jeremy Fitzhardinge wrote:

> --- /dev/null
> +++ b/lib/argv_split.c
> @@ -0,0 +1,159 @@
> +
> +/**
> + * argv_free - free an argv
> + *

extra "blank" line.

> + * @argv - the argument vector to be freed
> + *
> + * Frees an argv and the strings it points to.
> + */
> +void argv_free(char **argv)
> +{
> +	char **p;
> +	for (p = argv; *p; p++)
> +		kfree(*p);
> +
> +	kfree(argv);
> +}
> +EXPORT_SYMBOL(argv_free);

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

[patch 5/7] split usermodehelper setup from execution

[Jeremy Fitzhardinge]

[-- Attachment #1: usermodehelper-split-init.patch --]
[-- Type: text/plain, Size: 10234 bytes --]

Rather than having hundreds of variations of call_usermodehelper for
various pieces of usermode state which could be set up, split the
info allocation and initialization from the actual process execution.

This means the general pattern becomes:
 info = call_usermodehelper_setup(path, argv, envp); /* basic state */
 call_usermodehelper_<SET EXTRA STATE>(info, stuff...);	/* extra state */
 call_usermodehelper_exec(info, wait);	/* run process and free info */

This patch introduces wrappers for all the existing calling styles for
call_usermodehelper_*, but folds their implementations into one.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: Andi Kleen <ak@suse.de>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: David Howells <dhowells@redhat.com>
Cc: Bj?rn Steinbrink <B.Steinbrink@gmx.de>
Cc: Randy Dunlap <randy.dunlap@oracle.com>

---
 include/linux/kmod.h |   44 +++++++++-
 kernel/kmod.c        |  207 ++++++++++++++++++++++++++++++++++----------------
 2 files changed, 184 insertions(+), 67 deletions(-)

===================================================================
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -36,13 +36,51 @@ static inline int request_module(const c
 #define try_then_request_module(x, mod...) ((x) ?: (request_module(mod), (x)))
 
 struct key;
-extern int call_usermodehelper_keys(char *path, char *argv[], char *envp[],
-				    struct key *session_keyring, int wait);
+struct file;
+struct subprocess_info;
+
+/* Allocate a subprocess_info structure */
+struct subprocess_info *call_usermodehelper_setup(char *path,
+						  char **argv, char **envp);
+
+/* Set various pieces of state into the subprocess_info structure */
+void call_usermodehelper_setkeys(struct subprocess_info *info,
+				 struct key *session_keyring);
+int call_usermodehelper_stdinpipe(struct subprocess_info *sub_info,
+				  struct file **filp);
+void call_usermodehelper_setcleanup(struct subprocess_info *info,
+				    void (*cleanup)(char **argv, char **envp));
+
+/* Actually execute the sub-process */
+int call_usermodehelper_exec(struct subprocess_info *info, int wait);
+
+/* Free the subprocess_info. This is only needed if you're not going
+   to call call_usermodehelper_exec */
+void call_usermodehelper_freeinfo(struct subprocess_info *info);
 
 static inline int
 call_usermodehelper(char *path, char **argv, char **envp, int wait)
 {
-	return call_usermodehelper_keys(path, argv, envp, NULL, wait);
+	struct subprocess_info *info;
+
+	info = call_usermodehelper_setup(path, argv, envp);
+	if (info == NULL)
+		return -ENOMEM;
+	return call_usermodehelper_exec(info, wait);
+}
+
+static inline int
+call_usermodehelper_keys(char *path, char **argv, char **envp,
+			 struct key *session_keyring, int wait)
+{
+	struct subprocess_info *info;
+
+	info = call_usermodehelper_setup(path, argv, envp);
+	if (info == NULL)
+		return -ENOMEM;
+
+	call_usermodehelper_setkeys(info, session_keyring);
+	return call_usermodehelper_exec(info, wait);
 }
 
 extern void usermodehelper_init(void);
===================================================================
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -123,6 +123,7 @@ struct subprocess_info {
 	int wait;
 	int retval;
 	struct file *stdin;
+	void (*cleanup)(char **argv, char **envp);
 };
 
 /*
@@ -176,6 +177,14 @@ static int ____call_usermodehelper(void 
 	do_exit(0);
 }
 
+void call_usermodehelper_freeinfo(struct subprocess_info *info)
+{
+	if (info->cleanup)
+		(*info->cleanup)(info->argv, info->envp);
+	kfree(info);
+}
+EXPORT_SYMBOL(call_usermodehelper_freeinfo);
+
 /* Keventd can't block, but this (a child) can. */
 static int wait_for_helper(void *data)
 {
@@ -218,7 +227,7 @@ static int wait_for_helper(void *data)
 	}
 
 	if (sub_info->wait < 0)
-		kfree(sub_info);
+		call_usermodehelper_freeinfo(sub_info);
 	else
 		complete(sub_info->complete);
 	return 0;
@@ -253,11 +262,94 @@ static void __call_usermodehelper(struct
 }
 
 /**
- * call_usermodehelper_keys - start a usermode application
- * @path: pathname for the application
- * @argv: null-terminated argument list
- * @envp: null-terminated environment list
- * @session_keyring: session keyring for process (NULL for an empty keyring)
+ * call_usermodehelper_setup - prepare to call a usermode helper
+ * @path - path to usermode executable
+ * @argv - arg vector for process
+ * @envp - environment for process
+ *
+ * Returns either NULL on allocation failure, or a subprocess_info
+ * structure.  This should be passed to call_usermodehelper_exec to
+ * exec the process and free the structure.
+ */
+struct subprocess_info *call_usermodehelper_setup(char *path,
+						  char **argv, char **envp)
+{
+	struct subprocess_info *sub_info;
+	sub_info = kzalloc(sizeof(struct subprocess_info),  GFP_ATOMIC);
+	if (!sub_info)
+		goto out;
+
+	INIT_WORK(&sub_info->work, __call_usermodehelper);
+	sub_info->path = path;
+	sub_info->argv = argv;
+	sub_info->envp = envp;
+
+  out:
+	return sub_info;
+}
+EXPORT_SYMBOL(call_usermodehelper_setup);
+
+/**
+ * call_usermodehelper_setkeys - set the session keys for usermode helper
+ * @info: a subprocess_info returned by call_usermodehelper_setup
+ * @session_keyring: the session keyring for the process
+ */
+void call_usermodehelper_setkeys(struct subprocess_info *info,
+				 struct key *session_keyring)
+{
+	info->ring = session_keyring;
+}
+EXPORT_SYMBOL(call_usermodehelper_setkeys);
+
+/**
+ * call_usermodehelper_setcleanup - set a cleanup function
+ * @info: a subprocess_info returned by call_usermodehelper_setup
+ * @cleanup: a cleanup function
+ *
+ * The cleanup function is just befor ethe subprocess_info is about to
+ * be freed.  This can be used for freeing the argv and envp.  The
+ * Function must be runnable in either a process context or the
+ * context in which call_usermodehelper_exec is called.
+ */
+void call_usermodehelper_setcleanup(struct subprocess_info *info,
+				    void (*cleanup)(char **argv, char **envp))
+{
+	info->cleanup = cleanup;
+}
+EXPORT_SYMBOL(call_usermodehelper_setcleanup);
+
+/**
+ * call_usermodehelper_stdinpipe - set up a pipe to be used for stdin
+ * @sub_info: a subprocess_info returned by call_usermodehelper_setup
+ * @filp: set to the write-end of a pipe
+ *
+ * This constructs a pipe, and sets the read end to be the stdin of the
+ * subprocess, and returns the write-end in *@filp.
+ */
+int call_usermodehelper_stdinpipe(struct subprocess_info *sub_info,
+				  struct file **filp)
+{
+	struct file *f;
+
+	f = create_write_pipe();
+	if (IS_ERR(f))
+		return PTR_ERR(f);
+	*filp = f;
+
+	f = create_read_pipe(f);
+	if (IS_ERR(f)) {
+		free_write_pipe(*filp);
+		return PTR_ERR(f);
+	}
+	sub_info->stdin = f;
+
+	return 0;
+}
+EXPORT_SYMBOL(call_usermodehelper_stdinpipe);
+
+/**
+ * call_usermodehelper_exec - start a usermode application
+ * @sub_info: information about the subprocessa
  * @wait: wait for the application to finish and return status.
  *        when -1 don't wait at all, but you get no useful error back when
  *        the program couldn't be exec'ed. This makes it safe to call
@@ -266,33 +358,24 @@ static void __call_usermodehelper(struct
  * Runs a user-space application.  The application is started
  * asynchronously if wait is not set, and runs as a child of keventd.
  * (ie. it runs with full root capabilities).
- *
- * Must be called from process context.  Returns a negative error code
- * if program was not execed successfully, or 0.
- */
-int call_usermodehelper_keys(char *path, char **argv, char **envp,
-			     struct key *session_keyring, int wait)
+ */
+int call_usermodehelper_exec(struct subprocess_info *sub_info,
+			     int wait)
 {
 	DECLARE_COMPLETION_ONSTACK(done);
-	struct subprocess_info *sub_info;
 	int retval;
 
-	if (!khelper_wq)
-		return -EBUSY;
-
-	if (path[0] == '\0')
-		return 0;
-
-	sub_info = kzalloc(sizeof(struct subprocess_info),  GFP_ATOMIC);
-	if (!sub_info)
-		return -ENOMEM;
-
-	INIT_WORK(&sub_info->work, __call_usermodehelper);
+	if (sub_info->path[0] == '\0') {
+		retval = 0;
+		goto out;
+	}
+
+	if (!khelper_wq) {
+		retval = -EBUSY;
+		goto out;
+	}
+
 	sub_info->complete = &done;
-	sub_info->path = path;
-	sub_info->argv = argv;
-	sub_info->envp = envp;
-	sub_info->ring = session_keyring;
 	sub_info->wait = wait;
 
 	queue_work(khelper_wq, &sub_info->work);
@@ -300,47 +383,43 @@ int call_usermodehelper_keys(char *path,
 		return 0;
 	wait_for_completion(&done);
 	retval = sub_info->retval;
-	kfree(sub_info);
+
+  out:
+	call_usermodehelper_freeinfo(sub_info);
 	return retval;
 }
-EXPORT_SYMBOL(call_usermodehelper_keys);
-
+EXPORT_SYMBOL(call_usermodehelper_exec);
+
+/**
+ * call_usermodehelper_pipe - call a usermode helper process with a pipe stdin
+ * @path: path to usermode executable
+ * @argv: arg vector for process
+ * @envp: environment for process
+ * @filp: set to the write-end of a pipe
+ *
+ * This is a simple wrapper which executes a usermode-helper function
+ * with a pipe as stdin.  It is implemented entirely in terms of
+ * lower-level call_usermodehelper_* functions.
+ */
 int call_usermodehelper_pipe(char *path, char **argv, char **envp,
 			     struct file **filp)
 {
-	DECLARE_COMPLETION(done);
-	struct subprocess_info sub_info = {
-		.work		= __WORK_INITIALIZER(sub_info.work,
-						     __call_usermodehelper),
-		.complete	= &done,
-		.path		= path,
-		.argv		= argv,
-		.envp		= envp,
-		.retval		= 0,
-	};
-	struct file *f;
-
-	if (!khelper_wq)
-		return -EBUSY;
-
-	if (path[0] == '\0')
-		return 0;
-
-	f = create_write_pipe();
-	if (IS_ERR(f))
-		return PTR_ERR(f);
-	*filp = f;
-
-	f = create_read_pipe(f);
-	if (IS_ERR(f)) {
-		free_write_pipe(*filp);
-		return PTR_ERR(f);
-	}
-	sub_info.stdin = f;
-
-	queue_work(khelper_wq, &sub_info.work);
-	wait_for_completion(&done);
-	return sub_info.retval;
+	struct subprocess_info *sub_info;
+	int ret;
+
+	sub_info = call_usermodehelper_setup(path, argv, envp);
+	if (sub_info == NULL)
+		return -ENOMEM;
+
+	ret = call_usermodehelper_stdinpipe(sub_info, filp);
+	if (ret < 0)
+		goto out;
+
+	return call_usermodehelper_exec(sub_info, 1);
+
+  out:
+	call_usermodehelper_freeinfo(sub_info);
+	return ret;
 }
 EXPORT_SYMBOL(call_usermodehelper_pipe);
 

-- 

[patch 6/7] Add common orderly_poweroff()

[Jeremy Fitzhardinge]

[-- Attachment #1: orderly-poweroff.patch --]
[-- Type: text/plain, Size: 9934 bytes --]

Various pieces of code around the kernel want to be able to trigger an
orderly poweroff.  This pulls them together into a single
implementation.

By default the poweroff command is /sbin/poweroff, but it can be set
via sysctl: kernel/poweroff_cmd.  This is split at whitespace, so it
can include command-line arguments.

This patch replaces four other instances of invoking either "poweroff"
or "shutdown -h now": one sparc64, two sbus drivers, and acpi thermal
management.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Acked-by: Len Brown <lenb@kernel.org>
Cc: Chris Wright <chrisw@sous-sol.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Randy Dunlap <randy.dunlap@oracle.com>
Cc: Andi Kleen <ak@suse.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: David S. Miller <davem@davemloft.net>

---
 arch/sparc64/kernel/power.c     |   40 +-------------------------
 drivers/acpi/thermal.c          |   24 +---------------
 drivers/sbus/char/bbc_envctrl.c |    5 +--
 drivers/sbus/char/envctrl.c     |    7 +---
 include/linux/reboot.h          |    5 +++
 include/linux/sysctl.h          |    1 
 kernel/sys.c                    |   58 +++++++++++++++++++++++++++++++++++++++
 kernel/sysctl.c                 |   10 ++++++
 8 files changed, 82 insertions(+), 68 deletions(-)

===================================================================
--- a/arch/sparc64/kernel/power.c
+++ b/arch/sparc64/kernel/power.c
@@ -13,6 +13,7 @@
 #include <linux/interrupt.h>
 #include <linux/pm.h>
 #include <linux/syscalls.h>
+#include <linux/reboot.h>
 
 #include <asm/system.h>
 #include <asm/auxio.h>
@@ -32,14 +33,13 @@ int scons_pwroff = 1;
 #include <linux/pci.h>
 static void __iomem *power_reg;
 
-static DECLARE_WAIT_QUEUE_HEAD(powerd_wait);
 static int button_pressed;
 
 static irqreturn_t power_handler(int irq, void *dev_id)
 {
 	if (button_pressed == 0) {
 		button_pressed = 1;
-		wake_up(&powerd_wait);
+		orderly_poweroff(true);
 	}
 
 	/* FIXME: Check registers for status... */
@@ -75,36 +75,6 @@ EXPORT_SYMBOL(pm_power_off);
 EXPORT_SYMBOL(pm_power_off);
 
 #ifdef CONFIG_PCI
-static int powerd(void *__unused)
-{
-	static char *envp[] = { "HOME=/", "TERM=linux", "PATH=/sbin:/usr/sbin:/bin:/usr/bin", NULL };
-	char *argv[] = { "/sbin/shutdown", "-h", "now", NULL };
-	DECLARE_WAITQUEUE(wait, current);
-
-	daemonize("powerd");
-
-	add_wait_queue(&powerd_wait, &wait);
-again:
-	for (;;) {
-		set_task_state(current, TASK_INTERRUPTIBLE);
-		if (button_pressed)
-			break;
-		flush_signals(current);
-		schedule();
-	}
-	__set_current_state(TASK_RUNNING);
-	remove_wait_queue(&powerd_wait, &wait);
-
-	/* Ok, down we go... */
-	button_pressed = 0;
-	if (kernel_execve("/sbin/shutdown", argv, envp) < 0) {
-		printk("powerd: shutdown execution failed\n");
-		add_wait_queue(&powerd_wait, &wait);
-		goto again;
-	}
-	return 0;
-}
-
 static int __init has_button_interrupt(unsigned int irq, struct device_node *dp)
 {
 	if (irq == PCI_IRQ_NONE)
@@ -128,12 +98,6 @@ static int __devinit power_probe(struct 
 	poweroff_method = machine_halt;  /* able to use the standard halt */
 
 	if (has_button_interrupt(irq, op->node)) {
-		if (kernel_thread(powerd, NULL, CLONE_FS) < 0) {
-			printk("Failed to start power daemon.\n");
-			return 0;
-		}
-		printk("powerd running.\n");
-
 		if (request_irq(irq,
 				power_handler, 0, "power", NULL) < 0)
 			printk("power: Error, cannot register IRQ handler.\n");
===================================================================
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -40,6 +40,7 @@
 #include <linux/jiffies.h>
 #include <linux/kmod.h>
 #include <linux/seq_file.h>
+#include <linux/reboot.h>
 #include <asm/uaccess.h>
 
 #include <acpi/acpi_bus.h>
@@ -61,7 +62,6 @@
 #define ACPI_THERMAL_MODE_ACTIVE	0x00
 #define ACPI_THERMAL_MODE_PASSIVE	0x01
 #define ACPI_THERMAL_MODE_CRITICAL   	0xff
-#define ACPI_THERMAL_PATH_POWEROFF	"/sbin/poweroff"
 
 #define ACPI_THERMAL_MAX_ACTIVE	10
 #define ACPI_THERMAL_MAX_LIMIT_STR_LEN 65
@@ -431,26 +431,6 @@ static int acpi_thermal_get_devices(stru
 	return 0;
 }
 
-static int acpi_thermal_call_usermode(char *path)
-{
-	char *argv[2] = { NULL, NULL };
-	char *envp[3] = { NULL, NULL, NULL };
-
-
-	if (!path)
-		return -EINVAL;
-
-	argv[0] = path;
-
-	/* minimal command environment */
-	envp[0] = "HOME=/";
-	envp[1] = "PATH=/sbin:/bin:/usr/sbin:/usr/bin";
-
-	call_usermodehelper(argv[0], argv, envp, 0);
-
-	return 0;
-}
-
 static int acpi_thermal_critical(struct acpi_thermal *tz)
 {
 	if (!tz || !tz->trips.critical.flags.valid)
@@ -468,7 +448,7 @@ static int acpi_thermal_critical(struct 
 	acpi_bus_generate_event(tz->device, ACPI_THERMAL_NOTIFY_CRITICAL,
 				tz->trips.critical.flags.enabled);
 
-	acpi_thermal_call_usermode(ACPI_THERMAL_PATH_POWEROFF);
+	orderly_poweroff(true);
 
 	return 0;
 }
===================================================================
--- a/drivers/sbus/char/bbc_envctrl.c
+++ b/drivers/sbus/char/bbc_envctrl.c
@@ -7,6 +7,7 @@
 #include <linux/kthread.h>
 #include <linux/delay.h>
 #include <linux/kmod.h>
+#include <linux/reboot.h>
 #include <asm/oplib.h>
 #include <asm/ebus.h>
 
@@ -170,8 +171,6 @@ static void do_envctrl_shutdown(struct b
 static void do_envctrl_shutdown(struct bbc_cpu_temperature *tp)
 {
 	static int shutting_down = 0;
-	static char *envp[] = { "HOME=/", "TERM=linux", "PATH=/sbin:/usr/sbin:/bin:/usr/bin", NULL };
-	char *argv[] = { "/sbin/shutdown", "-h", "now", NULL };
 	char *type = "???";
 	s8 val = -1;
 
@@ -195,7 +194,7 @@ static void do_envctrl_shutdown(struct b
 	printk(KERN_CRIT "kenvctrld: Shutting down the system now.\n");
 
 	shutting_down = 1;
-	if (call_usermodehelper("/sbin/shutdown", argv, envp, 0) < 0)
+	if (orderly_poweroff(true) < 0)
 		printk(KERN_CRIT "envctrl: shutdown execution failed\n");
 }
 
===================================================================
--- a/drivers/sbus/char/envctrl.c
+++ b/drivers/sbus/char/envctrl.c
@@ -26,6 +26,7 @@
 #include <linux/ioport.h>
 #include <linux/miscdevice.h>
 #include <linux/kmod.h>
+#include <linux/reboot.h>
 
 #include <asm/ebus.h>
 #include <asm/uaccess.h>
@@ -965,10 +966,6 @@ static void envctrl_do_shutdown(void)
 static void envctrl_do_shutdown(void)
 {
 	static int inprog = 0;
-	static char *envp[] = {	
-		"HOME=/", "TERM=linux", "PATH=/sbin:/usr/sbin:/bin:/usr/bin", NULL };
-	char *argv[] = { 
-		"/sbin/shutdown", "-h", "now", NULL };	
 	int ret;
 
 	if (inprog != 0)
@@ -976,7 +973,7 @@ static void envctrl_do_shutdown(void)
 
 	inprog = 1;
 	printk(KERN_CRIT "kenvctrld: WARNING: Shutting down the system now.\n");
-	ret = call_usermodehelper("/sbin/shutdown", argv, envp, 0);
+	ret = orderly_poweroff(true);
 	if (ret < 0) {
 		printk(KERN_CRIT "kenvctrld: WARNING: system shutdown failed!\n"); 
 		inprog = 0;  /* unlikely to succeed, but we could try again */
===================================================================
--- a/include/linux/reboot.h
+++ b/include/linux/reboot.h
@@ -67,6 +67,11 @@ extern void kernel_power_off(void);
 
 void ctrl_alt_del(void);
 
+#define POWEROFF_CMD_PATH_LEN	256
+extern char poweroff_cmd[POWEROFF_CMD_PATH_LEN];
+
+extern int orderly_poweroff(bool force);
+
 /*
  * Emergency restart, callable from an interrupt handler.
  */
===================================================================
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -165,6 +165,7 @@ enum
 	KERN_MAX_LOCK_DEPTH=74,
 	KERN_NMI_WATCHDOG=75, /* int: enable/disable nmi watchdog */
 	KERN_PANIC_ON_NMI=76, /* int: whether we will panic on an unrecovered */
+	KERN_POWEROFF_CMD=77,	/* string: poweroff command line */
 };
 
 
===================================================================
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2208,3 +2208,61 @@ asmlinkage long sys_getcpu(unsigned __us
 	}
 	return err ? -EFAULT : 0;
 }
+
+char poweroff_cmd[POWEROFF_CMD_PATH_LEN] = "/sbin/poweroff";
+
+static void argv_cleanup(char **argv, char **envp)
+{
+	argv_free(argv);
+}
+
+/**
+ * Trigger an orderly system poweroff
+ * @force: force poweroff if command execution fails
+ *
+ * This may be called from any context to trigger a system shutdown.
+ * If the orderly shutdown fails, it will force an immediate shutdown.
+ */
+int orderly_poweroff(bool force)
+{
+	int argc;
+	char **argv = argv_split(GFP_ATOMIC, poweroff_cmd, &argc);
+	static char *envp[] = {
+		"HOME=/",
+		"PATH=/sbin:/bin:/usr/sbin:/usr/bin",
+		NULL
+	};
+	int ret = -ENOMEM;
+	struct subprocess_info *info;
+
+	if (argv == NULL) {
+		printk(KERN_WARNING "%s failed to allocate memory for \"%s\"\n",
+		       __func__, poweroff_cmd);
+		goto out;
+	}
+
+	info = call_usermodehelper_setup(argv[0], argv, envp);
+	if (info == NULL) {
+		argv_free(argv);
+		goto out;
+	}
+
+	call_usermodehelper_setcleanup(info, argv_cleanup);
+
+	ret = call_usermodehelper_exec(info, -1);
+
+  out:
+	if (ret && force) {
+		printk(KERN_WARNING "Failed to start orderly shutdown: "
+		       "forcing the issue\n");
+
+		/* I guess this should try to kick off some daemon to
+		   sync and poweroff asap.  Or not even bother syncing
+		   if we're doing an emergency shutdown? */
+		emergency_sync();
+		kernel_power_off();
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(orderly_poweroff);
===================================================================
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -45,6 +45,7 @@
 #include <linux/syscalls.h>
 #include <linux/nfs_fs.h>
 #include <linux/acpi.h>
+#include <linux/reboot.h>
 
 #include <asm/uaccess.h>
 #include <asm/processor.h>
@@ -603,6 +604,15 @@ static ctl_table kern_table[] = {
 		.proc_handler	= &proc_dointvec,
 	},
 #endif
+	{
+		.ctl_name	= KERN_POWEROFF_CMD,
+		.procname	= "poweroff_cmd",
+		.data		= &poweroff_cmd,
+		.maxlen		= POWEROFF_CMD_PATH_LEN,
+		.mode		= 0644,
+		.proc_handler	= &proc_dostring,
+		.strategy	= &sysctl_string,
+	},
 
 	{ .ctl_name = 0 }
 };

-- 

Re: [patch 6/7] Add common orderly_poweroff()

[Randy Dunlap]

On Thu, 10 May 2007 16:57:14 -0700 Jeremy Fitzhardinge wrote:

> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -2208,3 +2208,61 @@ asmlinkage long sys_getcpu(unsigned __us
> +
> +/**
> + * Trigger an orderly system poweroff

    * orderly_poweroff - Trigger an orderly system poweroff

> + * @force: force poweroff if command execution fails
> + *
> + * This may be called from any context to trigger a system shutdown.
> + * If the orderly shutdown fails, it will force an immediate shutdown.
> + */
> +int orderly_poweroff(bool force)
> +{
> +	int argc;
> +	char **argv = argv_split(GFP_ATOMIC, poweroff_cmd, &argc);
> +	static char *envp[] = {
> +		"HOME=/",
> +		"PATH=/sbin:/bin:/usr/sbin:/usr/bin",
> +		NULL
> +	};
> +	int ret = -ENOMEM;
> +	struct subprocess_info *info;
> +
> +	if (argv == NULL) {
> +		printk(KERN_WARNING "%s failed to allocate memory for \"%s\"\n",
> +		       __func__, poweroff_cmd);
> +		goto out;
> +	}
> +
> +	info = call_usermodehelper_setup(argv[0], argv, envp);
> +	if (info == NULL) {
> +		argv_free(argv);
> +		goto out;
> +	}
> +
> +	call_usermodehelper_setcleanup(info, argv_cleanup);
> +
> +	ret = call_usermodehelper_exec(info, -1);
> +
> +  out:
> +	if (ret && force) {
> +		printk(KERN_WARNING "Failed to start orderly shutdown: "
> +		       "forcing the issue\n");
> +
> +		/* I guess this should try to kick off some daemon to
> +		   sync and poweroff asap.  Or not even bother syncing
> +		   if we're doing an emergency shutdown? */
> +		emergency_sync();
> +		kernel_power_off();
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(orderly_poweroff);


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [patch 6/7] Add common orderly_poweroff()

[Andrew Morton]

On Thu, 10 May 2007 16:57:14 -0700
Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> Various pieces of code around the kernel want to be able to trigger an
> orderly poweroff.  This pulls them together into a single
> implementation.
> 
> By default the poweroff command is /sbin/poweroff, but it can be set
> via sysctl: kernel/poweroff_cmd.  This is split at whitespace, so it
> can include command-line arguments.
> 
> This patch replaces four other instances of invoking either "poweroff"
> or "shutdown -h now": one sparc64, two sbus drivers, and acpi thermal
> management.
> 
> Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
> Acked-by: Len Brown <lenb@kernel.org>
> Cc: Chris Wright <chrisw@sous-sol.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Randy Dunlap <randy.dunlap@oracle.com>
> Cc: Andi Kleen <ak@suse.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: David S. Miller <davem@davemloft.net>
> 
> ---
>  arch/sparc64/kernel/power.c     |   40 +-------------------------
>  drivers/acpi/thermal.c          |   24 +---------------
>  drivers/sbus/char/bbc_envctrl.c |    5 +--
>  drivers/sbus/char/envctrl.c     |    7 +---
>  include/linux/reboot.h          |    5 +++
>  include/linux/sysctl.h          |    1 
>  kernel/sys.c                    |   58 +++++++++++++++++++++++++++++++++++++++
>  kernel/sysctl.c                 |   10 ++++++

I'm not sure how this ended up in the x86 tree
(ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt/patches/add-common-orderly_poweroff),
but recent changes in arch/sparc64/kernel/power.c have broken it.

Dave, those changes hit mainline without ever having appeared in the
sparc-2.6.git tree, afaict.  If they _had_ appeared there I'd have been
able to tell you that there wasn't any point in changing code which we're
planning on deleting.  Otoh,

    [SPARC64]: Export powerd facilities for external entities.
    
    Besides the existing usage for power-button interrupts, we'll
    want to make use of this code for domain-services where the
    LDOM manager can send reboot requests to the guest node.

make me think that this code isn't deleteable any more.

Ho hum, I will delete the sparc64 hunk and shall hope for the best.  Mainly
to avoid downstream rejects - I suspect this patch needs revisting now.

Andi, your tree is looking pretty stale - I hope it will be updated soon?

Re: [patch 6/7] Add common orderly_poweroff()

[Jeremy Fitzhardinge]

Andrew Morton wrote:
> On Thu, 10 May 2007 16:57:14 -0700
> Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>
>   
>> Various pieces of code around the kernel want to be able to trigger an
>> orderly poweroff.  This pulls them together into a single
>> implementation.
>>
>> By default the poweroff command is /sbin/poweroff, but it can be set
>> via sysctl: kernel/poweroff_cmd.  This is split at whitespace, so it
>> can include command-line arguments.
>>
>> This patch replaces four other instances of invoking either "poweroff"
>> or "shutdown -h now": one sparc64, two sbus drivers, and acpi thermal
>> management.
>>
>> Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
>> Acked-by: Len Brown <lenb@kernel.org>
>> Cc: Chris Wright <chrisw@sous-sol.org>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Randy Dunlap <randy.dunlap@oracle.com>
>> Cc: Andi Kleen <ak@suse.de>
>> Cc: Al Viro <viro@zeniv.linux.org.uk>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: David S. Miller <davem@davemloft.net>
>>
>> ---
>>  arch/sparc64/kernel/power.c     |   40 +-------------------------
>>  drivers/acpi/thermal.c          |   24 +---------------
>>  drivers/sbus/char/bbc_envctrl.c |    5 +--
>>  drivers/sbus/char/envctrl.c     |    7 +---
>>  include/linux/reboot.h          |    5 +++
>>  include/linux/sysctl.h          |    1 
>>  kernel/sys.c                    |   58 +++++++++++++++++++++++++++++++++++++++
>>  kernel/sysctl.c                 |   10 ++++++
>>     
>
> I'm not sure how this ended up in the x86 tree
> (ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt/patches/add-common-orderly_poweroff),
> but recent changes in arch/sparc64/kernel/power.c have broken it.
>   

I just sent the same mail to Dave - sorry, forgot to cc: you.  It's 
there because I added it as a prereq for Xen.  I'm planning on 
submitting it later today, ideally once we've sorted this out.

For reference, the patches I'm planning on submitting are:

Subject: add kstrndup
Subject: add argv_split()
Subject: usermodehelper: split setup from execution
Subject: Add common orderly_poweroff()
Subject: usermodehelper: Tidy up waiting
Subject: use elfnote.h to generate vsyscall notes.
Subject: paravirt: add an "mm" argument to alloc_pt
Subject: paravirt: add a hook for once the allocator is ready
Subject: paravirt: increase IRQ limit
Subject: paravirt: unstatic leave_mm
Subject: paravirt: unstatic smp_store_cpu_info
Subject: paravirt: make siblingmap functions visible
Subject: paravirt: export __supported_pte_mask
Subject: Allocate and free vmalloc areas
Subject: paravirt: helper to disable all IO space
Subject: Add a sched_clock paravirt_op
Subject: Add nosegneg capability to the vsyscall page notes
Subject: xen: Add Xen interface header files
Subject: xen: Core Xen implementation
Subject: xen: virtual mmu
Subject: xen: event channels
Subject: xen: time implementation
Subject: xen: configuration
Subject: xen: add pinned page flag
Subject: xen: Complete pagetable pinning
Subject: xen: ignore RW mapping of RO pages in pagetable_init
Subject: xen: Account for stolen time
Subject: xen: Implement sched_clock
Subject: xen: SMP guest support
Subject: xen: Add support for preemption
Subject: xen: lazy-mmu operations
Subject: xen: hack to prevent bad segment register reload
Subject: xen: use the hvc console infrastructure for Xen console
Subject: xen: Add 	grant table support
Subject: xen: add the Xenbus sysfs and virtual device hotplug driver
Subject: xen: add virtual block device driver.
Subject: xen: add virtual network device driver
Subject: xen: machine operations
Subject: xen: handle external requests for shutdown, reboot and sysrq
Subject: xen: Place vcpu_info structure into per-cpu memory
Subject: xen: Attempt to patch inline versions of common operations
Subject: xen: suppress abs symbol warnings for unused reloc pointers
Subject: xen: use iret directly when possible
Subject: xen: disable all non-virtual drivers

	J

Re: [patch 6/7] Add common orderly_poweroff()

[David Miller]

From: Andrew Morton <akpm@linux-foundation.org>
Date: Mon, 16 Jul 2007 12:52:23 -0700

> Dave, those changes hit mainline without ever having appeared in the
> sparc-2.6.git tree, afaict.

It's been in there for more than a week.

Re: [patch 6/7] Add common orderly_poweroff()

[Andrew Morton]

On Mon, 16 Jul 2007 14:31:06 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: Andrew Morton <akpm@linux-foundation.org>
> Date: Mon, 16 Jul 2007 12:52:23 -0700
> 
> > Dave, those changes hit mainline without ever having appeared in the
> > sparc-2.6.git tree, afaict.
> 
> It's been in there for more than a week.

Nope, I pulled all the git trees on July 1,2,5,6,8,10 and 13 (at least) but
git-sparc64.patch did not change between July 1 and July 16.

At all times I've been pulling from
git+ssh://master.kernel.org/pub/scm/linux/kernel/git/davem/sparc-2.6.git. 
Perhaps that was the wrong branch?  

[patch 7/7] tidy up usermode helper waiting a bit

[Jeremy Fitzhardinge]

[-- Attachment #1: usermodehelper-tidy.patch --]
[-- Type: text/plain, Size: 10366 bytes --]

Rather than using a tri-state integer for the wait flag in
call_usermodehelper_exec, define a proper enum, and use that.  I've
preserved the integer values so that any callers I've missed should
still work OK.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Randy Dunlap <randy.dunlap@oracle.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Andi Kleen <ak@suse.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Bjorn Helgaas <bjorn.helgaas@hp.com>
Cc: Joel Becker <joel.becker@oracle.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Kay Sievers <kay.sievers@vrfy.org>
Cc: Srivatsa Vaddagiri <vatsa@in.ibm.com>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
Cc: David Howells <dhowells@redhat.com>
---
 arch/i386/mach-voyager/voyager_thread.c |    2 +-
 arch/x86_64/kernel/mce.c                |    2 +-
 drivers/macintosh/therm_pm72.c          |    3 ++-
 drivers/macintosh/windfarm_core.c       |    3 ++-
 drivers/net/hamradio/baycom_epp.c       |    2 +-
 drivers/pnp/pnpbios/core.c              |    2 +-
 fs/ocfs2/heartbeat.c                    |    2 +-
 include/linux/kmod.h                    |   12 +++++++++---
 kernel/cpuset.c                         |    2 +-
 kernel/kmod.c                           |   29 +++++++++++++++++------------
 kernel/sys.c                            |    2 +-
 lib/kobject_uevent.c                    |    2 +-
 net/bridge/br_stp_if.c                  |    2 +-
 security/keys/request_key.c             |    3 ++-
 14 files changed, 41 insertions(+), 27 deletions(-)

===================================================================
--- a/arch/i386/mach-voyager/voyager_thread.c
+++ b/arch/i386/mach-voyager/voyager_thread.c
@@ -52,7 +52,7 @@ execute(const char *string)
 		NULL,
 	};
 
-	if ((ret = call_usermodehelper(argv[0], argv, envp, 1)) != 0) {
+	if ((ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC)) != 0) {
 		printk(KERN_ERR "Voyager failed to run \"%s\": %i\n",
 		       string, ret);
 	}
===================================================================
--- a/arch/x86_64/kernel/mce.c
+++ b/arch/x86_64/kernel/mce.c
@@ -174,7 +174,7 @@ static void do_mce_trigger(void)
 	if (events != atomic_read(&mce_logged) && trigger[0]) {
 		/* Small race window, but should be harmless.  */
 		atomic_set(&mce_logged, events);
-		call_usermodehelper(trigger, trigger_argv, NULL, -1);
+		call_usermodehelper(trigger, trigger_argv, NULL, UMH_NO_WAIT);
 	}
 }
 
===================================================================
--- a/drivers/macintosh/therm_pm72.c
+++ b/drivers/macintosh/therm_pm72.c
@@ -1770,7 +1770,8 @@ static int call_critical_overtemp(void)
 				"PATH=/sbin:/usr/sbin:/bin:/usr/bin",
 				NULL };
 
-	return call_usermodehelper(critical_overtemp_path, argv, envp, 0);
+	return call_usermodehelper(critical_overtemp_path,
+				   argv, envp, UMH_WAIT_EXEC);
 }
 
 
===================================================================
--- a/drivers/macintosh/windfarm_core.c
+++ b/drivers/macintosh/windfarm_core.c
@@ -80,7 +80,8 @@ int wf_critical_overtemp(void)
 				"PATH=/sbin:/usr/sbin:/bin:/usr/bin",
 				NULL };
 
-	return call_usermodehelper(critical_overtemp_path, argv, envp, 0);
+	return call_usermodehelper(critical_overtemp_path,
+				   argv, envp, UMH_WAIT_EXEC);
 }
 EXPORT_SYMBOL_GPL(wf_critical_overtemp);
 
===================================================================
--- a/drivers/net/hamradio/baycom_epp.c
+++ b/drivers/net/hamradio/baycom_epp.c
@@ -320,7 +320,7 @@ static int eppconfig(struct baycom_state
 	sprintf(portarg, "%ld", bc->pdev->port->base);
 	printk(KERN_DEBUG "%s: %s -s -p %s -m %s\n", bc_drvname, eppconfig_path, portarg, modearg);
 
-	return call_usermodehelper(eppconfig_path, argv, envp, 1);
+	return call_usermodehelper(eppconfig_path, argv, envp, UMH_WAIT_PROC);
 }
 
 /* ---------------------------------------------------------------------- */
===================================================================
--- a/drivers/pnp/pnpbios/core.c
+++ b/drivers/pnp/pnpbios/core.c
@@ -147,7 +147,7 @@ static int pnp_dock_event(int dock, stru
 		info->location_id, info->serial, info->capabilities);
 	envp[i] = NULL;
 	
-	value = call_usermodehelper (argv [0], argv, envp, 0);
+	value = call_usermodehelper (argv [0], argv, envp, UMH_WAIT_EXEC);
 	kfree (buf);
 	kfree (envp);
 	return 0;
===================================================================
--- a/fs/ocfs2/heartbeat.c
+++ b/fs/ocfs2/heartbeat.c
@@ -209,7 +209,7 @@ void ocfs2_stop_heartbeat(struct ocfs2_s
 	envp[1] = "PATH=/sbin:/bin:/usr/sbin:/usr/bin";
 	envp[2] = NULL;
 
-	ret = call_usermodehelper(argv[0], argv, envp, 1);
+	ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC);
 	if (ret < 0)
 		mlog_errno(ret);
 }
===================================================================
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -51,15 +51,21 @@ void call_usermodehelper_setcleanup(stru
 void call_usermodehelper_setcleanup(struct subprocess_info *info,
 				    void (*cleanup)(char **argv, char **envp));
 
+enum umh_wait {
+	UMH_NO_WAIT = -1,	/* don't wait at all */
+	UMH_WAIT_EXEC = 0,	/* wait for the exec, but not the process */
+	UMH_WAIT_PROC = 1,	/* wait for the process to complete */
+};
+
 /* Actually execute the sub-process */
-int call_usermodehelper_exec(struct subprocess_info *info, int wait);
+int call_usermodehelper_exec(struct subprocess_info *info, enum umh_wait wait);
 
 /* Free the subprocess_info. This is only needed if you're not going
    to call call_usermodehelper_exec */
 void call_usermodehelper_freeinfo(struct subprocess_info *info);
 
 static inline int
-call_usermodehelper(char *path, char **argv, char **envp, int wait)
+call_usermodehelper(char *path, char **argv, char **envp, enum umh_wait wait)
 {
 	struct subprocess_info *info;
 
@@ -71,7 +77,7 @@ call_usermodehelper(char *path, char **a
 
 static inline int
 call_usermodehelper_keys(char *path, char **argv, char **envp,
-			 struct key *session_keyring, int wait)
+			 struct key *session_keyring, enum umh_wait wait)
 {
 	struct subprocess_info *info;
 
===================================================================
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -516,7 +516,7 @@ static void cpuset_release_agent(const c
 	envp[i++] = "PATH=/sbin:/bin:/usr/sbin:/usr/bin";
 	envp[i] = NULL;
 
-	call_usermodehelper(argv[0], argv, envp, 0);
+	call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC);
 	kfree(pathbuf);
 }
 
===================================================================
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -119,7 +119,7 @@ struct subprocess_info {
 	char **argv;
 	char **envp;
 	struct key *ring;
-	int wait;
+	enum umh_wait wait;
 	int retval;
 	struct file *stdin;
 	void (*cleanup)(char **argv, char **envp);
@@ -225,7 +225,7 @@ static int wait_for_helper(void *data)
 			sub_info->retval = ret;
 	}
 
-	if (sub_info->wait < 0)
+	if (sub_info->wait == UMH_NO_WAIT)
 		call_usermodehelper_freeinfo(sub_info);
 	else
 		complete(sub_info->complete);
@@ -238,26 +238,31 @@ static void __call_usermodehelper(struct
 	struct subprocess_info *sub_info =
 		container_of(work, struct subprocess_info, work);
 	pid_t pid;
-	int wait = sub_info->wait;
+	enum umh_wait wait = sub_info->wait;
 
 	/* CLONE_VFORK: wait until the usermode helper has execve'd
 	 * successfully We need the data structures to stay around
 	 * until that is done.  */
-	if (wait)
+	if (wait == UMH_WAIT_PROC)
 		pid = kernel_thread(wait_for_helper, sub_info,
 				    CLONE_FS | CLONE_FILES | SIGCHLD);
 	else
 		pid = kernel_thread(____call_usermodehelper, sub_info,
 				    CLONE_VFORK | SIGCHLD);
 
-	if (wait < 0)
-		return;
-
-	if (pid < 0) {
+	switch(wait) {
+	case UMH_NO_WAIT:
+		break;
+
+	case UMH_WAIT_PROC:
+		if (pid > 0)
+			break;
 		sub_info->retval = pid;
+		/* FALLTHROUGH */
+
+	case UMH_WAIT_EXEC:
 		complete(sub_info->complete);
-	} else if (!wait)
-		complete(sub_info->complete);
+	}
 }
 
 /**
@@ -359,7 +364,7 @@ EXPORT_SYMBOL(call_usermodehelper_stdinp
  * (ie. it runs with full root capabilities).
  */
 int call_usermodehelper_exec(struct subprocess_info *sub_info,
-			     int wait)
+			     enum umh_wait wait)
 {
 	DECLARE_COMPLETION_ONSTACK(done);
 	int retval;
@@ -378,7 +383,7 @@ int call_usermodehelper_exec(struct subp
 	sub_info->wait = wait;
 
 	queue_work(khelper_wq, &sub_info->work);
-	if (wait < 0) /* task has freed sub_info */
+	if (wait == UMH_NO_WAIT) /* task has freed sub_info */
 		return 0;
 	wait_for_completion(&done);
 	retval = sub_info->retval;
===================================================================
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2310,7 +2310,7 @@ int orderly_poweroff(bool force)
 
 	call_usermodehelper_setcleanup(info, argv_cleanup);
 
-	ret = call_usermodehelper_exec(info, -1);
+	ret = call_usermodehelper_exec(info, UMH_NO_WAIT);
 
   out:
 	if (ret && force) {
===================================================================
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -208,7 +208,7 @@ int kobject_uevent_env(struct kobject *k
 		argv [0] = uevent_helper;
 		argv [1] = (char *)subsystem;
 		argv [2] = NULL;
-		call_usermodehelper (argv[0], argv, envp, 0);
+		call_usermodehelper (argv[0], argv, envp, UMH_WAIT_EXEC);
 	}
 
 exit:
===================================================================
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -125,7 +125,7 @@ static void br_stp_start(struct net_brid
 	char *argv[] = { BR_STP_PROG, br->dev->name, "start", NULL };
 	char *envp[] = { NULL };
 
-	r = call_usermodehelper(BR_STP_PROG, argv, envp, 1);
+	r = call_usermodehelper(BR_STP_PROG, argv, envp, UMH_WAIT_PROC);
 	if (r == 0) {
 		br->stp_enabled = BR_USER_STP;
 		printk(KERN_INFO "%s: userspace STP started\n", br->dev->name);
===================================================================
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -108,7 +108,8 @@ static int call_sbin_request_key(struct 
 	argv[i] = NULL;
 
 	/* do it */
-	ret = call_usermodehelper_keys(argv[0], argv, envp, keyring, 1);
+	ret = call_usermodehelper_keys(argv[0], argv, envp, keyring,
+				       UMH_WAIT_PROC);
 
 error_link:
 	key_put(keyring);

-- 

Re: [patch 7/7] tidy up usermode helper waiting a bit

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 611 bytes --]

[/me wonders why he was CC'ed]

>  	/* CLONE_VFORK: wait until the usermode helper has execve'd
>  	 * successfully We need the data structures to stay around
>  	 * until that is done.  */
> -	if (wait)
> +	if (wait == UMH_WAIT_PROC)
>  		pid = kernel_thread(wait_for_helper, sub_info,
>  				    CLONE_FS | CLONE_FILES | SIGCHLD);
>  	else
>  		pid = kernel_thread(____call_usermodehelper, sub_info,
>  				    CLONE_VFORK | SIGCHLD);

Isn't that a change in behaviour? Previously it said
	if (wait)
 <=>	if (wait != 0)
 <=>	if (wait != UMH_WAIT_EXEC)
or am I missing something?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: [patch 7/7] tidy up usermode helper waiting a bit

[Jeremy Fitzhardinge]

Johannes Berg wrote:
> [/me wonders why he was CC'ed]
>   

Because I knew you'd ask a good question ;)  No, I looked through the
recent changes on the files I changes and guessed at likely-looking
names.  But come to think of it, cc:ing completely random people is not
a bad way of getting some extra review cycles you mightn't otherwise get...


>>  	/* CLONE_VFORK: wait until the usermode helper has execve'd
>>  	 * successfully We need the data structures to stay around
>>  	 * until that is done.  */
>> -	if (wait)
>> +	if (wait == UMH_WAIT_PROC)
>>  		pid = kernel_thread(wait_for_helper, sub_info,
>>  				    CLONE_FS | CLONE_FILES | SIGCHLD);
>>  	else
>>  		pid = kernel_thread(____call_usermodehelper, sub_info,
>>  				    CLONE_VFORK | SIGCHLD);
>>     
>
> Isn't that a change in behaviour? Previously it said
> 	if (wait)
>  <=>	if (wait != 0)
>  <=>	if (wait != UMH_WAIT_EXEC)
> or am I missing something?
>   

Hm, you're right, it is a change in behaviour.  Yep, and a bug, since
its expecting wait_for_helper() to free the info in the UMH_NO_WAIT case.

---
 kernel/kmod.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

===================================================================
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -243,7 +243,7 @@ static void __call_usermodehelper(struct
 	/* CLONE_VFORK: wait until the usermode helper has execve'd
 	 * successfully We need the data structures to stay around
 	 * until that is done.  */
-	if (wait == UMH_WAIT_PROC)
+	if (wait == UMH_WAIT_PROC || wait == UMH_NO_WAIT)
 		pid = kernel_thread(wait_for_helper, sub_info,
 				    CLONE_FS | CLONE_FILES | SIGCHLD);
 	else

Thanks,
    J