A series of cleanup patches
Subsystems (18)
cover A series of cleanup patches
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
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.
--- 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(-)
===================================================================
arch/i386/kernel/Makefile +1 -0
@@ -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
===================================================================
arch/i386/kernel/smp.c +6 -59
89 lines not on this page · raw message
===================================================================
arch/i386/kernel/smpboot.c +0 -22
@@ -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
===================================================================
arch/i386/kernel/smpcommon.c new file +79 -0
79 lines not on this page · raw message
===================================================================
include/asm-i386/processor.h +4 -0
@@ -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.
In reply to Jeremy FitzhardingeUse 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.
--- 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(-)
===================================================================
arch/i386/kernel/vsyscall-note.S +6 -17
@@ -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
===================================================================
arch/x86_64/vdso/vdso-note.S +2 -2
@@ -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)
===================================================================
include/linux/elfnote.h +13 -5
@@ -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.
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.
Quoted 4 lines · 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.
OK, good point.
J
Re: [patch 2/7] use elfnote.h to generate vsyscall notes.
In reply to Roland McGrathQuoted 6 lines · 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.
Subject: use balign in elfnote.h
Roland McGrath <[email protected]>:
Quoted 3 lines
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.
--- include/linux/elfnote.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
===================================================================
include/linux/elfnote.h +3 -3
@@ -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
In reply to Jeremy FitzhardingeAdd 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.
--- include/linux/string.h | 1 + mm/util.c | 30 +++++++++++++++++++++++++++--- net/irda/irias_object.c | 43 +++++-------------------------------------- 3 files changed, 33 insertions(+), 41 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 +27 -3
@@ -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. */
===================================================================
net/irda/irias_object.c +5 -38
79 lines not on this page · raw message
Re: [patch 3/7] add kstrndup
mm/util.c char *kstrdup(const char *s, gfp_t gfp) · line 58
Quoted 3 lines
mm/util.c +3
-0+ 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
Quoted 9 lines · René Scharfe
Review
Quoted 4 lines
(quoted diff) +3
-0+ 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.
--- mm/util.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
===================================================================
mm/util.c +1 -3
@@ -53,9 +53,7 @@ char *kstrndup(const char *s, size_t maxif (!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()
In reply to Jeremy Fitzhardingeargv_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. ]
--- include/linux/string.h | 3 lib/Makefile | 2 lib/argv_split.c | 159 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 163 insertions(+), 1 deletion(-)
===================================================================
include/linux/string.h +3 -0
@@ -108,6 +108,9 @@ extern char *kstrndup(const char *s, sizextern 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 +159 -0
159 lines not on this page · raw message
Re: [patch 4/7] add argv_split()
lib/argv_split.c line 4
Quoted 7 lines · On Thu, 10 May 2007 16:57:12 -0700 Jeremy Fitzhardinge wrote:
lib/argv_split.c new file +4
-0@@ -0,0 +1,159 @@ + +/** +* argv_free - free an argv +*
extra "blank" line.
lib/argv_split.c line 57
Quoted 13 lines
lib/argv_split.c new file +13
-0+* @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
In reply to Jeremy FitzhardingeRather 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.
--- 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
PATCH 6/7 Add common orderly_poweroff()
In reply to Jeremy FitzhardingeVarious 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.
--- 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(-)
===================================================================
arch/sparc64/kernel/power.c +2 -38
@@ -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
@@ -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(strureturn 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(structacpi_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
@@ -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 bstatic 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 bprintk (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
@@ -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
@@ -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
@@ -165,6 +165,7 @@ enumKERN_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 +58 -0
61 lines not on this page · raw message
===================================================================
kernel/sysctl.c +10 -0
@@ -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()
kernel/sys.c asmlinkage long sys_getcpu(unsigned __us · line 2210
Quoted 6 lines · On Thu, 10 May 2007 16:57:14 -0700 Jeremy Fitzhardinge wrote:
kernel/sys.c +3
-0@@ -2208,3 +2208,61 @@ asmlinkage long sys_getcpu(unsigned __us + +/** +* Trigger an orderly system poweroff
* orderly_poweroff - Trigger an orderly system poweroff
Quoted 38 lines
kernel/sys.c +38
-0+* @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" ); +
kernel/sys.c asmlinkage long sys_getcpu(unsigned __us · line 2268
Quoted 10 lines
kernel/sys.c +10
-0+/* 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 ***
Andrew Morton
maintainerRe: [patch 6/7] Add common orderly_poweroff()
In reply to Jeremy FitzhardingeQuoted 22 lines · On Thu, 10 May 2007 16:57:14 -0700 Jeremy Fitzhardinge <[email protected]> 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 <[email protected]>Acked-by Len Brown <[email protected]>Cc Chris Wright <[email protected]>Cc Andrew Morton <[email protected]>Cc Randy Dunlap <[email protected]>Cc Andi Kleen <[email protected]>Cc Al Viro <[email protected]>Cc Arnd Bergmann <[email protected]>Cc David S. Miller <[email protected]>
Quoted 9 lines
--- 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()
Quoted 37 lines · Andrew Morton
Quoted 22 lines · On Thu, 10 May 2007 16:57:14 -0700 Jeremy Fitzhardinge <[email protected]> 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 <[email protected]>Acked-by Len Brown <[email protected]>Cc Chris Wright <[email protected]>Cc Andrew Morton <[email protected]>Cc Randy Dunlap <[email protected]>Cc Andi Kleen <[email protected]>Cc Al Viro <[email protected]>Cc Arnd Bergmann <[email protected]>Cc David S. Miller <[email protected]>Quoted 10 lines
--- 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 ++++++
Quoted 4 lines
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.chave 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
David Miller
maintainerRe: [patch 6/7] Add common orderly_poweroff()
In reply to Andrew MortonFrom: Andrew Morton <[email protected]>
Date: Mon, 16 Jul 2007 12:52:23 -0700
Quoted 2 lines
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.
Andrew Morton
maintainerRe: [patch 6/7] Add common orderly_poweroff()
Quoted 7 lines · On Mon, 16 Jul 2007 14:31:06 -0700 (PDT) David Miller <[email protected]>…
From: Andrew Morton <[email protected]>
Date: Mon, 16 Jul 2007 12:52:23 -0700Quoted 2 lines
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
In reply to Jeremy FitzhardingeRather 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.
--- 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(-)
===================================================================
arch/i386/mach-voyager/voyager_thread.c +1 -1
@@ -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); }
===================================================================
arch/x86_64/kernel/mce.c +1 -1
@@ -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 ); } }
===================================================================
drivers/macintosh/therm_pm72.c +2 -1
@@ -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 ); }
===================================================================
drivers/macintosh/windfarm_core.c +2 -1
@@ -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);
===================================================================
drivers/net/hamradio/baycom_epp.c +1 -1
@@ -320,7 +320,7 @@ static int eppconfig(struct baycom_statesprintf (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 ); }/* ---------------------------------------------------------------------- */
===================================================================
drivers/pnp/pnpbios/core.c +1 -1
@@ -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 ;
===================================================================
fs/ocfs2/heartbeat.c +1 -1
@@ -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); }
===================================================================
include/linux/kmod.h +9 -3
@@ -51,15 +51,21 @@ void call_usermodehelper_setcleanup(struvoid 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 **astatic 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;
===================================================================
kernel/cpuset.c +1 -1
@@ -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); }
===================================================================
kernel/kmod.c +17 -12
@@ -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(structstruct 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;
===================================================================
kernel/sys.c +1 -1
@@ -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) {
===================================================================
lib/kobject_uevent.c +1 -1
@@ -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:
===================================================================
net/bridge/br_stp_if.c +1 -1
@@ -125,7 +125,7 @@ static void br_stp_start(struct net_bridchar *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);
===================================================================
security/keys/request_key.c +2 -1
@@ -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
[/me wonders why he was CC'ed]
kernel/kmod.c static void __call_usermodehelper(struct · line 251
Quoted 10 lines
kernel/kmod.c +1
-1/* 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
Re: [patch 7/7] tidy up usermode helper waiting a bit
Quoted 2 lines · Johannes Berg
[/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...
Quoted 9 lines
Quoted 9 lines
(quoted diff) +1
-1/* 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,
Quoted 9 lines
Quoted 2 lines
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(-)
===================================================================
kernel/kmod.c +1 -1
@@ -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