Skip to contentSkip to conversation

A series of cleanup patches

PATCH In review Jeremy Fitzhardinge 6 participants 17 messages May 8, 2007 – May 9, 2007

Subsystems (10)
Networking Memory management ACPI Library code +6 more subsystems
lore
Share & tools
lore

Jeremy Fitzhardinge

author
#0 lore raw

cover A series of cleanup patches

Hi Andi, Andrew,

Here's a series of cleanup patches:

1. add kstrndup, which is needed by
2. add argv_split, a helper for splitting a string into an argv array,
3. a restructuring of the call_usermodehelper set of functions, to make them more flexible, needed by
4. a common orderly_poweroff() function, which replaces various ad-hoc ways of powering off the machine used around the kernel

Thanks,
J

author
#1 lore raw

PATCH 1/4 add kstrndup

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 <[email protected]>
Cc Chris Wright <[email protected]>
Cc Andrew Morton <[email protected]>
Cc YOSHIFUJI Hideaki <[email protected]>
Cc Akinobu Mita <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Al Viro <[email protected]>
Cc Panagiotis Issaris <[email protected]>
---
 include/linux/string.h  |    1 +
 mm/util.c               |   29 ++++++++++++++++++++++++++++-
 net/irda/irias_object.c |   43 +++++--------------------------------------
 3 files changed, 34 insertions(+), 39 deletions(-)

===================================================================

include/linux/string.h +1 -0
@@ -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

===================================================================

mm/util.c +28 -1
@@ -18,7 +18,7 @@ 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

@@ -39,6 +39,33 @@ 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

===================================================================

net/irda/irias_object.c +5 -38

79 lines not on this page · raw message

#2 lore raw

Re: [patch 1/4] add kstrndup

mm/util.c void *__kzalloc(size_t size, gfp_t flags · line 22

Quoted 9 lines · On Tue, 08 May 2007 13:51:30 -0700 Jeremy Fitzhardinge wrote:
mm/util.c +1 -1
@@ -18,7 +18,7 @@ void *__kzalloc(size_t size, gfp_t flags
 }
 EXPORT_SYMBOL(__kzalloc);
 
-/*
+/**
  * kstrdup - allocate space for and copy an existing string

No blank ("*") line between function name and its parameters. (occurs in multiple places in multiple patches in this series; I'm not going to point out each one of them separately)

Quoted 21 lines
mm/util.c +12 -0
  *
  * @s: the string to duplicate

@@ -39,6 +39,33 @@ 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)
+{
+}
+EXPORT_SYMBOL(kstrndup);
 
 /**
  * kmemdup - duplicate region of memory
Quoted 1 line

===================================================================

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

author
#3 lore raw

PATCH 2/4 add argv_split()

In reply to Jeremy Fitzhardinge

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 <[email protected]>
Cc Chris Wright <[email protected]>
Cc Andrew Morton <[email protected]>
---
 include/linux/string.h |    3 
 lib/Makefile           |    2 
 lib/argv_split.c       |  160 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 164 insertions(+), 1 deletion(-)

===================================================================

include/linux/string.h +3 -0
@@ -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

===================================================================

lib/Makefile +1 -1
@@ -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

===================================================================

lib/argv_split.c new file +160 -0

160 lines not on this page · raw message

#4 lore raw

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

lib/argv_split.c line 4

Quoted 7 lines · On Tue, 08 May 2007 13:51:31 -0700 Jeremy Fitzhardinge wrote:
lib/argv_split.c new file +4 -0
@@ -0,0 +1,160 @@
+
+static const char *skip_sep(const char *cp)
+{
+	while(*cp && isspace(*cp))

Kernel style is space after "while", "for", "if"...

Quoted 21 lines
lib/argv_split.c new file +21 -0
+		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++;

lib/argv_split.c line 44

Quoted 10 lines
lib/argv_split.c new file +10 -0
+			str = skip_arg(str);
+		}
+	}
+
+	return count;
+}
+
+/**
+ * argv_free - free an argv
+ *

no blank "*" line.

lib/argv_split.c line 45

Quoted 1 line
lib/argv_split.c new file +1 -0
+ * @argv - the argument vector to be freed

* @argv: the argument vector to be freed

lib/argv_split.c line 57

Quoted 12 lines
lib/argv_split.c new file +12 -0
+ *
+ * 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 ***

#5 lore raw

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

In reply to Jeremy Fitzhardinge

Review

Quoted 2 lines · Jeremy Fitzhardinge
(quoted diff) +2 -0
+
+static const char *skip_sep(const char *cp)

These are so simple they could use an 'inline' tag.

lib/argv_split.c

Quoted 9 lines
lib/argv_split.c new file +8 -0
+{
+	while(*cp && isspace(*cp))
+		cp++;
+
+	return cp;
+}
+
 
+#ifdef TEST

But not in the end result.

lib/argv_split.c line 127

Quoted 2 lines
lib/argv_split.c new file +2 -0
+int main() {
+	const char *testvec[] = {

Suggesting static const char *const testvec[].

Jan

author
#6 lore raw

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

Quoted 8 lines · Jan Engelhardt

Review

Quoted 3 lines · Jeremy Fitzhardinge
(quoted diff) +2 -0
+
+static const char *skip_sep(const char *cp)

These are so simple they could use an 'inline' tag.

I'm sure the compiler can work that out for itself.

Quoted 5 lines
Quoted 2 lines

+#ifdef TEST

But not in the end result.

?

Quoted 6 lines

lib/argv_split.c line 127

Quoted 3 lines
lib/argv_split.c new file +2 -0
+int main() {
+	const char *testvec[] = {

Suggesting static const char *const testvec[].

I guess, but it doesn't matter much.

J

author
#7 lore raw

PATCH 3/4 split usermodehelper setup from execution

In reply to Jeremy Fitzhardinge

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 <[email protected]>
Cc Andi Kleen <[email protected]>
Cc Rusty Russell <[email protected]>
Cc David Howells <[email protected]>
Cc Bj?rn Steinbrink <[email protected]>
---
 include/linux/kmod.h |   44 +++++++++-
 kernel/kmod.c        |  207 ++++++++++++++++++++++++++++++++++----------------
 2 files changed, 184 insertions(+), 67 deletions(-)

===================================================================

include/linux/kmod.h +41 -3
@@ -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);

===================================================================

kernel/kmod.c +143 -64

253 lines not on this page · raw message

#8 lore raw

Re: [patch 3/4] split usermodehelper setup from execution

Quoted 5 lines · On Tue, 08 May 2007 13:51:32 -0700 Jeremy Fitzhardinge wrote:
kernel/kmod.c +0 -0
@@ -253,11 +262,94 @@ static void __call_usermodehelper(struct
 }

kernel/kmod.c static void __call_usermodehelper(struct · line 268

Quoted 10 lines
kernel/kmod.c +4 -5
 /**
- * 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

s / - /: / above please.

Quoted 5 lines
kernel/kmod.c +5 -0
+ *
+ * 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.
+ */

kernel/kmod.c static void __call_usermodehelper(struct · line 295

Quoted 10 lines
kernel/kmod.c +10 -0
+struct subprocess_info *call_usermodehelper_setup(char *path,
+						  char **argv, char **envp)
+{
+}
+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

and here.

kernel/kmod.c static void __call_usermodehelper(struct · line 307

Quoted 6 lines
kernel/kmod.c +6 -0
+ */
+
+/**
+ * call_usermodehelper_setcleanup - set a cleanup function
+ * @info  - a subprocess_info returned by call_usermodehelper_setup
+ * @cleanup - a cleanup function

again.

Quoted 7 lines
kernel/kmod.c +7 -0
+ *
+ * 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,

kernel/kmod.c static void __call_usermodehelper(struct · line 324

Quoted 10 lines
kernel/kmod.c +10 -0
+				    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

again.

Quoted 9 lines
kernel/kmod.c +9 -0
+ *
+ * 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)
+{
+}
+EXPORT_SYMBOL(call_usermodehelper_stdinpipe);

...

Quoted 6 lines

+/**
+ * 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

and here.

Quoted 8 lines

+ *
+ * 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)
{

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

author
#9 lore raw

Re: [patch 3/4] split usermodehelper setup from execution

Quoted 8 lines · Randy Dunlap
Quoted 5 lines · On Tue, 08 May 2007 13:51:32 -0700 Jeremy Fitzhardinge wrote:
kernel/kmod.c +0 -0
@@ -253,11 +262,94 @@ static void __call_usermodehelper(struct
 }
Quoted 14 lines

kernel/kmod.c static void __call_usermodehelper(struct

Quoted 11 lines
kernel/kmod.c +4 -5
 /**
- * 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

s / - /: / above please.

The power of cut'n'paste makes the errors consistent, at least... Will fix up.

J

#10 lore raw

Re: [patch 3/4] split usermodehelper setup from execution

Quoted 24 lines · On Tue, 08 May 2007 15:00:44 -0700 Jeremy Fitzhardinge wrote:
Quoted 8 lines · Randy Dunlap
Quoted 5 lines · On Tue, 08 May 2007 13:51:32 -0700 Jeremy Fitzhardinge wrote:
kernel/kmod.c +0 -0
@@ -253,11 +262,94 @@ static void __call_usermodehelper(struct
 }
Quoted 14 lines

kernel/kmod.c static void __call_usermodehelper(struct

Quoted 11 lines
kernel/kmod.c +4 -5
 /**
- * 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

s / - /: / above please.

Quoted 2 lines

The power of cut'n'paste makes the errors consistent, at least... Will fix up.

Thanks. If you point me to such foobars (in general), I'll fix them up.

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

#11 lore raw

Re: [patch 3/4] split usermodehelper setup from execution

In reply to Jeremy Fitzhardinge
Quoted 4 lines · Jeremy Fitzhardinge

plain text document attachment (usermodehelper-split-init.patch) 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.

Erk, I hadn't realized how extended call_usermodehelper had become.

call_usermodehelper_pipe()? Erk. Grepping... for core dumping via a process? Without modifying the ELF core dumper (at least) to handle short writes? Why not dump it in an agreed location and exec the process with that as an arg?

When did this go in? 2.6.19... hmm, too late to rip it out I guess 8(

Anyway, I'm not sure exposing an open-ended interface wins: refactoring internally definitely makes sense though.

Rusty.

author
#12 lore raw

Re: [patch 3/4] split usermodehelper setup from execution

Quoted 5 lines · Rusty Russell

call_usermodehelper_pipe()? Erk. Grepping... for core dumping via a process? Without modifying the ELF core dumper (at least) to handle short writes? Why not dump it in an agreed location and exec the process with that as an arg?

I think the idea is that you might want to send it over the net on a diskless system, or filter/compress the core before it hits disk because you don't have enough space.

Quoted 4 lines

When did this go in? 2.6.19... hmm, too late to rip it out I guess 8(

Anyway, I'm not sure exposing an open-ended interface wins: refactoring internally definitely makes sense though.

Well, I added this specifically so I could add a cleanup callback, which can be used to free a dynamically allocated argv array (see the following patch). And if you're going to have all this stuff, you may as well make it work properly together. Why have "you can set the session keyring *or* use a pipe for stdin"?

As distended as it is, at least it actually fits together somewhat coherently now. I'd be up for dropping the single-use wrapper functions.

J

author
#13 lore raw

PATCH 4/4 Add common orderly_poweroff()

In reply to Jeremy Fitzhardinge

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 <[email protected]>
Cc Chris Wright <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Andi Kleen <[email protected]>
Cc Len Brown <[email protected]>
Cc Al Viro <[email protected]>
Cc Arnd Bergmann <[email protected]>
Cc David S. Miller <[email protected]>
---
 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                    |   50 +++++++++++++++++++++++++++++++++++++++
 kernel/sysctl.c                 |   10 +++++++
 8 files changed, 74 insertions(+), 68 deletions(-)
arch/sparc64/kernel/power.c +2 -38
diff -r 9eea40ea89b7 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");
drivers/acpi/thermal.c +2 -22
diff -r 9eea40ea89b7 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;
 }
drivers/sbus/char/bbc_envctrl.c +2 -3
diff -r 9eea40ea89b7 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");
 }
drivers/sbus/char/envctrl.c +2 -5
diff -r 9eea40ea89b7 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 */
include/linux/reboot.h +5 -0
diff -r 9eea40ea89b7 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.
  */
include/linux/sysctl.h +1 -0
diff -r 9eea40ea89b7 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 */
 };
kernel/sys.c +55 -0

58 lines not on this page · raw message

kernel/sysctl.c +10 -0
diff -r 9eea40ea89b7 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 }
 };
reviewer Acked-by on v1 4/4
#14 lore raw

Re: [patch 4/4] Add common orderly_poweroff()

Quoted 12 lines · Jeremy Fitzhardinge

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.

Quoted 5 lines
Signed-off-by Jeremy Fitzhardinge <[email protected]>
Cc Chris Wright <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Andi Kleen <[email protected]>
Cc Len Brown <[email protected]>

Re: the ACPI parts

Acked-by Len Brown <[email protected]>

thanks,
-Len

Quoted 14 lines
Cc Al Viro <[email protected]>
Cc Arnd Bergmann <[email protected]>
Cc David S. Miller <[email protected]>
---
 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                    |   50 +++++++++++++++++++++++++++++++++++++++
 kernel/sysctl.c                 |   10 +++++++
 8 files changed, 74 insertions(+), 68 deletions(-)
#15 lore raw

Re: [patch 4/4] Add common orderly_poweroff()

In reply to Jeremy Fitzhardinge
Quoted 6 lines · On Tue, 08 May 2007 13:51:33 -0700 Jeremy Fitzhardinge wrote:

diff -r 9eea40ea89b7 kernel/sys.c

kernel/sys.c +0 -0
@@ -2208,3 +2208,58 @@ asmlinkage long sys_getcpu(unsigned __us
 	}
 	return err ? -EFAULT : 0;

kernel/sys.c asmlinkage long sys_getcpu(unsigned __us · line 2219

Quoted 10 lines
kernel/sys.c +9 -0
 }
+
+char poweroff_cmd[POWEROFF_CMD_PATH_LEN] = "/sbin/poweroff";
+
+static void argv_cleanup(char **argv, char **envp)
+{
+	argv_free(argv);
+}
+
+/**

/*
would be OK since this isn't kernel-doc.... (or make it kernel-doc)

kernel/sys.c asmlinkage long sys_getcpu(unsigned __us · line 2226

Quoted 7 lines
kernel/sys.c +7 -0
+ * Trigger an orderly system poweroff
+ *
+ * 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)
+{

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

#16 lore raw

Re: [patch 4/4] Add common orderly_poweroff()

In reply to Jeremy Fitzhardinge
Quoted 3 lines · Jeremy Fitzhardinge

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

Fine for me from the x86 side.

-Andi