[PATCH 0/23] reboot-fixes

[PATCH 0/23] reboot-fixes

[Eric W. Biederman]

The reboot code paths seems to be suffering from 15 years of people
only looking at the code when it breaks.  The result is there
are several code paths in which different callers expect different
semantics from the same functions, and a fair amount of imperfect
inline replication of code.

For a year or more every time I fix one bug in the bug fix reveals yet
another bug.  In an attempt to end the cycle of bug fixes revealing
yet more bugs I have generated a series of patches to clean up
the semantics along the reboot path.

With the callers all agreeing on what to expect from the functions
they call it should at least be possible to kill bugs without
more showing up because of the bug fix.

My primary approach is to factor sys_reboot into several smaller
functions and provide those functions for the general kernel
consumers instead of the architecture dependent restart and
halt hooks.

I don't expect this to noticeably fix any bugs along the
main code paths but magic sysrq and several of the more obscure
code paths should work much more reliably.

Eric

[PATCH 1/23] Add missing device_suspsend(PMSG_FREEZE) calls.

[Eric W. Biederman]

In the recent addition of device_suspend calls into
sys_reboot two code paths were missed.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 kernel/sys.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

5f0fb00783b94248b5a76c161f1c30a033fce4d3
diff --git a/kernel/sys.c b/kernel/sys.c
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -391,6 +391,7 @@ asmlinkage long sys_reboot(int magic1, i
 	case LINUX_REBOOT_CMD_RESTART:
 		notifier_call_chain(&reboot_notifier_list, SYS_RESTART, NULL);
 		system_state = SYSTEM_RESTART;
+		device_suspend(PMSG_FREEZE);
 		device_shutdown();
 		printk(KERN_EMERG "Restarting system.\n");
 		machine_restart(NULL);
@@ -452,6 +453,7 @@ asmlinkage long sys_reboot(int magic1, i
 		}
 		notifier_call_chain(&reboot_notifier_list, SYS_RESTART, NULL);
 		system_state = SYSTEM_RESTART;
+		device_suspend(PMSG_FREEZE);
 		device_shutdown();
 		printk(KERN_EMERG "Starting new kernel\n");
 		machine_shutdown();

[PATCH 2/23] Refactor sys_reboot into reusable parts

[Eric W. Biederman]

Because the factors of sys_reboot don't exist people calling
into the reboot path duplicate the code badly, leading to
inconsistent expectations of code in the reboot path.

This patch should is just code motion.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 include/linux/reboot.h |    9 ++++
 kernel/sys.c           |  106 +++++++++++++++++++++++++++++-------------------
 2 files changed, 73 insertions(+), 42 deletions(-)

bbc94d1109869fe5054f7cd697c978d494edeb62
diff --git a/include/linux/reboot.h b/include/linux/reboot.h
--- a/include/linux/reboot.h
+++ b/include/linux/reboot.h
@@ -55,6 +55,15 @@ extern void machine_shutdown(void);
 struct pt_regs;
 extern void machine_crash_shutdown(struct pt_regs *);
 
+/* 
+ * Architecture independent implemenations of sys_reboot commands.
+ */
+
+extern void kernel_restart(char *cmd);
+extern void kernel_halt(void);
+extern void kernel_power_off(void);
+extern void kernel_kexec(void);
+
 #endif
 
 #endif /* _LINUX_REBOOT_H */
diff --git a/kernel/sys.c b/kernel/sys.c
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -361,6 +361,62 @@ out_unlock:
 	return retval;
 }
 
+void kernel_restart(char *cmd)
+{
+	notifier_call_chain(&reboot_notifier_list, SYS_RESTART, cmd);
+	system_state = SYSTEM_RESTART;
+	device_suspend(PMSG_FREEZE);
+	device_shutdown();
+	if (!cmd) {
+		printk(KERN_EMERG "Restarting system.\n");
+	} else {
+		printk(KERN_EMERG "Restarting system with command '%s'.\n", cmd);
+	}
+	printk(".\n");
+	machine_restart(cmd);
+}
+EXPORT_SYMBOL_GPL(kernel_restart);
+
+void kernel_kexec(void)
+{
+#ifdef CONFIG_KEXEC
+	struct kimage *image;
+	image = xchg(&kexec_image, 0);
+	if (!image) {
+		return;
+	}
+	notifier_call_chain(&reboot_notifier_list, SYS_RESTART, NULL);
+	system_state = SYSTEM_RESTART;
+	device_suspend(PMSG_FREEZE);
+	device_shutdown();
+	printk(KERN_EMERG "Starting new kernel\n");
+	machine_shutdown();
+	machine_kexec(image);
+#endif
+}
+EXPORT_SYMBOL_GPL(kernel_kexec);
+
+void kernel_halt(void)
+{
+	notifier_call_chain(&reboot_notifier_list, SYS_HALT, NULL);
+	system_state = SYSTEM_HALT;
+	device_suspend(PMSG_SUSPEND);
+	device_shutdown();
+	printk(KERN_EMERG "System halted.\n");
+	machine_halt();
+}
+EXPORT_SYMBOL_GPL(kernel_halt);
+
+void kernel_power_off(void)
+{
+	notifier_call_chain(&reboot_notifier_list, SYS_POWER_OFF, NULL);
+	system_state = SYSTEM_POWER_OFF;
+	device_suspend(PMSG_SUSPEND);
+	device_shutdown();
+	printk(KERN_EMERG "Power down.\n");
+	machine_power_off();
+}
+EXPORT_SYMBOL_GPL(kernel_power_off);
 
 /*
  * Reboot system call: for obvious reasons only root may call it,
@@ -389,12 +445,7 @@ asmlinkage long sys_reboot(int magic1, i
 	lock_kernel();
 	switch (cmd) {
 	case LINUX_REBOOT_CMD_RESTART:
-		notifier_call_chain(&reboot_notifier_list, SYS_RESTART, NULL);
-		system_state = SYSTEM_RESTART;
-		device_suspend(PMSG_FREEZE);
-		device_shutdown();
-		printk(KERN_EMERG "Restarting system.\n");
-		machine_restart(NULL);
+		kernel_restart(NULL);
 		break;
 
 	case LINUX_REBOOT_CMD_CAD_ON:
@@ -406,23 +457,13 @@ asmlinkage long sys_reboot(int magic1, i
 		break;
 
 	case LINUX_REBOOT_CMD_HALT:
-		notifier_call_chain(&reboot_notifier_list, SYS_HALT, NULL);
-		system_state = SYSTEM_HALT;
-		device_suspend(PMSG_SUSPEND);
-		device_shutdown();
-		printk(KERN_EMERG "System halted.\n");
-		machine_halt();
+		kernel_halt();
 		unlock_kernel();
 		do_exit(0);
 		break;
 
 	case LINUX_REBOOT_CMD_POWER_OFF:
-		notifier_call_chain(&reboot_notifier_list, SYS_POWER_OFF, NULL);
-		system_state = SYSTEM_POWER_OFF;
-		device_suspend(PMSG_SUSPEND);
-		device_shutdown();
-		printk(KERN_EMERG "Power down.\n");
-		machine_power_off();
+		kernel_power_off();
 		unlock_kernel();
 		do_exit(0);
 		break;
@@ -434,33 +475,14 @@ asmlinkage long sys_reboot(int magic1, i
 		}
 		buffer[sizeof(buffer) - 1] = '\0';
 
-		notifier_call_chain(&reboot_notifier_list, SYS_RESTART, buffer);
-		system_state = SYSTEM_RESTART;
-		device_suspend(PMSG_FREEZE);
-		device_shutdown();
-		printk(KERN_EMERG "Restarting system with command '%s'.\n", buffer);
-		machine_restart(buffer);
+		kernel_restart(buffer);
 		break;
 
-#ifdef CONFIG_KEXEC
 	case LINUX_REBOOT_CMD_KEXEC:
-	{
-		struct kimage *image;
-		image = xchg(&kexec_image, 0);
-		if (!image) {
-			unlock_kernel();
-			return -EINVAL;
-		}
-		notifier_call_chain(&reboot_notifier_list, SYS_RESTART, NULL);
-		system_state = SYSTEM_RESTART;
-		device_suspend(PMSG_FREEZE);
-		device_shutdown();
-		printk(KERN_EMERG "Starting new kernel\n");
-		machine_shutdown();
-		machine_kexec(image);
-		break;
-	}
-#endif
+		kernel_kexec();
+		unlock_kernel();
+		return -EINVAL;
+
 #ifdef CONFIG_SOFTWARE_SUSPEND
 	case LINUX_REBOOT_CMD_SW_SUSPEND:
 		{

[PATCH 3/23] Make ctrl_alt_del call kernel_restart to get a proper reboot.

[Eric W. Biederman]

It is obvious we wanted to call kernel_restart here
but since we don't have it the code was expanded inline and hasn't
been correct since sometime in 2.4.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 kernel/sys.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

252e08b37dc29ce42a0aef357b75ec1882eb634c
diff --git a/kernel/sys.c b/kernel/sys.c
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -502,8 +502,7 @@ asmlinkage long sys_reboot(int magic1, i
 
 static void deferred_cad(void *dummy)
 {
-	notifier_call_chain(&reboot_notifier_list, SYS_RESTART, NULL);
-	machine_restart(NULL);
+	kernel_restart(NULL);
 }
 
 /*

[PATCH 4/23] Add emergency_restart()

[Eric W. Biederman]

When the kernel is working well and we want to restart cleanly
kernel_restart is the function to use.   But in many instances
the kernel wants to reboot when thing are expected to be working
very badly such as from panic or a software watchdog handler.

This patch adds the function emergency_restart() so that
callers can be clear what semantics they expect when calling
restart.  emergency_restart() is expected to be callable
from interrupt context and possibly reliable in even more
trying circumstances.

This is an initial generic implementation for all architectures.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 include/asm-alpha/emergency-restart.h     |    6 ++++++
 include/asm-arm/emergency-restart.h       |    6 ++++++
 include/asm-arm26/emergency-restart.h     |    6 ++++++
 include/asm-cris/emergency-restart.h      |    6 ++++++
 include/asm-frv/emergency-restart.h       |    6 ++++++
 include/asm-generic/emergency-restart.h   |    9 +++++++++
 include/asm-h8300/emergency-restart.h     |    6 ++++++
 include/asm-i386/emergency-restart.h      |    6 ++++++
 include/asm-ia64/emergency-restart.h      |    6 ++++++
 include/asm-m32r/emergency-restart.h      |    6 ++++++
 include/asm-m68k/emergency-restart.h      |    6 ++++++
 include/asm-m68knommu/emergency-restart.h |    6 ++++++
 include/asm-mips/emergency-restart.h      |    6 ++++++
 include/asm-parisc/emergency-restart.h    |    6 ++++++
 include/asm-ppc/emergency-restart.h       |    6 ++++++
 include/asm-ppc64/emergency-restart.h     |    6 ++++++
 include/asm-s390/emergency-restart.h      |    6 ++++++
 include/asm-sh/emergency-restart.h        |    6 ++++++
 include/asm-sh64/emergency-restart.h      |    6 ++++++
 include/asm-sparc/emergency-restart.h     |    6 ++++++
 include/asm-sparc64/emergency-restart.h   |    6 ++++++
 include/asm-um/emergency-restart.h        |    6 ++++++
 include/asm-v850/emergency-restart.h      |    6 ++++++
 include/asm-x86_64/emergency-restart.h    |    6 ++++++
 include/asm-xtensa/emergency-restart.h    |    6 ++++++
 include/linux/reboot.h                    |    7 +++++++
 kernel/sys.c                              |    6 ++++++
 27 files changed, 166 insertions(+), 0 deletions(-)
 create mode 100644 include/asm-alpha/emergency-restart.h
 create mode 100644 include/asm-arm/emergency-restart.h
 create mode 100644 include/asm-arm26/emergency-restart.h
 create mode 100644 include/asm-cris/emergency-restart.h
 create mode 100644 include/asm-frv/emergency-restart.h
 create mode 100644 include/asm-generic/emergency-restart.h
 create mode 100644 include/asm-h8300/emergency-restart.h
 create mode 100644 include/asm-i386/emergency-restart.h
 create mode 100644 include/asm-ia64/emergency-restart.h
 create mode 100644 include/asm-m32r/emergency-restart.h
 create mode 100644 include/asm-m68k/emergency-restart.h
 create mode 100644 include/asm-m68knommu/emergency-restart.h
 create mode 100644 include/asm-mips/emergency-restart.h
 create mode 100644 include/asm-parisc/emergency-restart.h
 create mode 100644 include/asm-ppc/emergency-restart.h
 create mode 100644 include/asm-ppc64/emergency-restart.h
 create mode 100644 include/asm-s390/emergency-restart.h
 create mode 100644 include/asm-sh/emergency-restart.h
 create mode 100644 include/asm-sh64/emergency-restart.h
 create mode 100644 include/asm-sparc/emergency-restart.h
 create mode 100644 include/asm-sparc64/emergency-restart.h
 create mode 100644 include/asm-um/emergency-restart.h
 create mode 100644 include/asm-v850/emergency-restart.h
 create mode 100644 include/asm-x86_64/emergency-restart.h
 create mode 100644 include/asm-xtensa/emergency-restart.h

3d8ea7bd8df5d92589d8e02f45b979073c855848
diff --git a/include/asm-alpha/emergency-restart.h b/include/asm-alpha/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-alpha/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-arm/emergency-restart.h b/include/asm-arm/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-arm/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-arm26/emergency-restart.h b/include/asm-arm26/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-arm26/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-cris/emergency-restart.h b/include/asm-cris/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-cris/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-frv/emergency-restart.h b/include/asm-frv/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-frv/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-generic/emergency-restart.h b/include/asm-generic/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-generic/emergency-restart.h
@@ -0,0 +1,9 @@
+#ifndef _ASM_GENERIC_EMERGENCY_RESTART_H
+#define _ASM_GENERIC_EMERGENCY_RESTART_H
+
+static inline void machine_emergency_restart(void)
+{
+	machine_restart(NULL);
+}
+
+#endif /* _ASM_GENERIC_EMERGENCY_RESTART_H */
diff --git a/include/asm-h8300/emergency-restart.h b/include/asm-h8300/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-h8300/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-i386/emergency-restart.h b/include/asm-i386/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-i386/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-ia64/emergency-restart.h b/include/asm-ia64/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-ia64/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-m32r/emergency-restart.h b/include/asm-m32r/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-m32r/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-m68k/emergency-restart.h b/include/asm-m68k/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-m68k/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-m68knommu/emergency-restart.h b/include/asm-m68knommu/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-m68knommu/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-mips/emergency-restart.h b/include/asm-mips/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-mips/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-parisc/emergency-restart.h b/include/asm-parisc/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-parisc/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-ppc/emergency-restart.h b/include/asm-ppc/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-ppc/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-ppc64/emergency-restart.h b/include/asm-ppc64/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-ppc64/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-s390/emergency-restart.h b/include/asm-s390/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-s390/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-sh/emergency-restart.h b/include/asm-sh/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-sh/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-sh64/emergency-restart.h b/include/asm-sh64/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-sh64/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-sparc/emergency-restart.h b/include/asm-sparc/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-sparc/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-sparc64/emergency-restart.h b/include/asm-sparc64/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-sparc64/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-um/emergency-restart.h b/include/asm-um/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-um/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-v850/emergency-restart.h b/include/asm-v850/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-v850/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-x86_64/emergency-restart.h b/include/asm-x86_64/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-x86_64/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/asm-xtensa/emergency-restart.h b/include/asm-xtensa/emergency-restart.h
new file mode 100644
--- /dev/null
+++ b/include/asm-xtensa/emergency-restart.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_EMERGENCY_RESTART_H
+#define _ASM_EMERGENCY_RESTART_H
+
+#include <asm-generic/emergency-restart.h>
+
+#endif /* _ASM_EMERGENCY_RESTART_H */
diff --git a/include/linux/reboot.h b/include/linux/reboot.h
--- a/include/linux/reboot.h
+++ b/include/linux/reboot.h
@@ -64,6 +64,13 @@ extern void kernel_halt(void);
 extern void kernel_power_off(void);
 extern void kernel_kexec(void);
 
+/*
+ * Emergency restart, callable from an interrupt handler.
+ */
+
+extern void emergency_restart(void);
+#include <asm/emergency-restart.h>
+
 #endif
 
 #endif /* _LINUX_REBOOT_H */
diff --git a/kernel/sys.c b/kernel/sys.c
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -361,6 +361,12 @@ out_unlock:
 	return retval;
 }
 
+void emergency_restart(void)
+{
+	machine_emergency_restart();
+}
+EXPORT_SYMBOL_GPL(emergency_restart);
+
 void kernel_restart(char *cmd)
 {
 	notifier_call_chain(&reboot_notifier_list, SYS_RESTART, cmd);

[PATCH 5/23] Fix the arguments to machine_restart on cris

[Eric W. Biederman]

It appears machine_restart has been working cris just
by luck.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 arch/cris/kernel/process.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

bb30d3f0b58c6ecde77e7446b8bab12610fb5f97
diff --git a/arch/cris/kernel/process.c b/arch/cris/kernel/process.c
--- a/arch/cris/kernel/process.c
+++ b/arch/cris/kernel/process.c
@@ -113,6 +113,7 @@
 #include <linux/user.h>
 #include <linux/elfcore.h>
 #include <linux/mqueue.h>
+#include <linux/reboot.h>
 
 //#define DEBUG
 
@@ -208,7 +209,7 @@ void cpu_idle (void)
 
 void hard_reset_now (void);
 
-void machine_restart(void)
+void machine_restart(char *cmd)
 {
 	hard_reset_now();
 }

[PATCH 6/23] Don't export machine_restart, machine_halt, or machine_power_off.

[Eric W. Biederman]

machine_restart, machine_halt and machine_power_off are machine
specific hooks deep into the reboot logic, that modules
have no business messing with.  Usually code should be calling
kernel_restart, kernel_halt, kernel_power_off, or
emergency_restart. So don't export machine_restart,
machine_halt, and machine_power_off so we can catch buggy users.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 arch/alpha/kernel/process.c            |    3 ---
 arch/arm/kernel/process.c              |    4 ----
 arch/arm26/kernel/process.c            |    5 -----
 arch/cris/kernel/process.c             |    6 ------
 arch/h8300/kernel/process.c            |    6 ------
 arch/i386/kernel/reboot.c              |    5 -----
 arch/i386/mach-visws/reboot.c          |    5 -----
 arch/i386/mach-voyager/voyager_basic.c |    5 -----
 arch/ia64/kernel/process.c             |    5 -----
 arch/m32r/kernel/process.c             |    6 ------
 arch/m68k/kernel/process.c             |    6 ------
 arch/m68knommu/kernel/process.c        |    6 ------
 arch/mips/kernel/reset.c               |    5 -----
 arch/parisc/kernel/process.c           |    6 ------
 arch/ppc/kernel/setup.c                |    6 ------
 arch/ppc64/kernel/setup.c              |    3 ---
 arch/s390/kernel/setup.c               |    6 ------
 arch/sh/kernel/process.c               |    6 ------
 arch/sparc/kernel/process.c            |    6 ------
 arch/sparc64/kernel/power.c            |    2 --
 arch/sparc64/kernel/process.c          |    4 ----
 arch/um/kernel/reboot.c                |    6 ------
 arch/v850/kernel/anna.c                |    6 ------
 arch/v850/kernel/as85ep1.c             |    6 ------
 arch/v850/kernel/fpga85e2c.c           |    6 ------
 arch/v850/kernel/rte_cb.c              |    6 ------
 arch/v850/kernel/sim.c                 |    6 ------
 arch/v850/kernel/sim85e2.c             |    5 -----
 arch/x86_64/kernel/reboot.c            |    5 -----
 29 files changed, 0 insertions(+), 152 deletions(-)

ddf8bd62b8e8ce33ff2b55fd047106bd38eed486
diff --git a/arch/alpha/kernel/process.c b/arch/alpha/kernel/process.c
--- a/arch/alpha/kernel/process.c
+++ b/arch/alpha/kernel/process.c
@@ -165,7 +165,6 @@ machine_restart(char *restart_cmd)
 	common_shutdown(LINUX_REBOOT_CMD_RESTART, restart_cmd);
 }
 
-EXPORT_SYMBOL(machine_restart);
 
 void
 machine_halt(void)
@@ -173,7 +172,6 @@ machine_halt(void)
 	common_shutdown(LINUX_REBOOT_CMD_HALT, NULL);
 }
 
-EXPORT_SYMBOL(machine_halt);
 
 void
 machine_power_off(void)
@@ -181,7 +179,6 @@ machine_power_off(void)
 	common_shutdown(LINUX_REBOOT_CMD_POWER_OFF, NULL);
 }
 
-EXPORT_SYMBOL(machine_power_off);
 
 /* Used by sysrq-p, among others.  I don't believe r9-r15 are ever
    saved in the context it's used.  */
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -131,7 +131,6 @@ void machine_halt(void)
 {
 }
 
-EXPORT_SYMBOL(machine_halt);
 
 void machine_power_off(void)
 {
@@ -139,7 +138,6 @@ void machine_power_off(void)
 		pm_power_off();
 }
 
-EXPORT_SYMBOL(machine_power_off);
 
 void machine_restart(char * __unused)
 {
@@ -169,8 +167,6 @@ void machine_restart(char * __unused)
 	while (1);
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void __show_regs(struct pt_regs *regs)
 {
 	unsigned long flags = condition_codes(regs);
diff --git a/arch/arm26/kernel/process.c b/arch/arm26/kernel/process.c
--- a/arch/arm26/kernel/process.c
+++ b/arch/arm26/kernel/process.c
@@ -103,9 +103,6 @@ void machine_power_off(void)
 {
 }
 
-EXPORT_SYMBOL(machine_halt);
-EXPORT_SYMBOL(machine_power_off);
-
 void machine_restart(char * __unused)
 {
 	/*
@@ -136,8 +133,6 @@ void machine_restart(char * __unused)
 	while (1);
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void show_regs(struct pt_regs * regs)
 {
 	unsigned long flags;
diff --git a/arch/cris/kernel/process.c b/arch/cris/kernel/process.c
--- a/arch/cris/kernel/process.c
+++ b/arch/cris/kernel/process.c
@@ -214,8 +214,6 @@ void machine_restart(char *cmd)
 	hard_reset_now();
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 /*
  * Similar to machine_power_off, but don't shut off power.  Add code
  * here to freeze the system for e.g. post-mortem debug purpose when
@@ -226,16 +224,12 @@ void machine_halt(void)
 {
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 /* If or when software power-off is implemented, add code here.  */
 
 void machine_power_off(void)
 {
 }
 
-EXPORT_SYMBOL(machine_power_off);
-
 /*
  * When a process does an "exec", machine state like FPU and debug
  * registers need to be reset.  This is a hook function for that.
diff --git a/arch/h8300/kernel/process.c b/arch/h8300/kernel/process.c
--- a/arch/h8300/kernel/process.c
+++ b/arch/h8300/kernel/process.c
@@ -90,8 +90,6 @@ void machine_restart(char * __unused)
 	__asm__("jmp @@0"); 
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_halt(void)
 {
 	local_irq_disable();
@@ -99,8 +97,6 @@ void machine_halt(void)
 	for (;;);
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void machine_power_off(void)
 {
 	local_irq_disable();
@@ -108,8 +104,6 @@ void machine_power_off(void)
 	for (;;);
 }
 
-EXPORT_SYMBOL(machine_power_off);
-
 void show_regs(struct pt_regs * regs)
 {
 	printk("\nPC: %08lx  Status: %02x",
diff --git a/arch/i386/kernel/reboot.c b/arch/i386/kernel/reboot.c
--- a/arch/i386/kernel/reboot.c
+++ b/arch/i386/kernel/reboot.c
@@ -337,14 +337,10 @@ void machine_restart(char * __unused)
 	machine_real_restart(jump_to_bios, sizeof(jump_to_bios));
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_halt(void)
 {
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void machine_power_off(void)
 {
 	lapic_shutdown();
@@ -355,5 +351,4 @@ void machine_power_off(void)
 		pm_power_off();
 }
 
-EXPORT_SYMBOL(machine_power_off);
 
diff --git a/arch/i386/mach-visws/reboot.c b/arch/i386/mach-visws/reboot.c
--- a/arch/i386/mach-visws/reboot.c
+++ b/arch/i386/mach-visws/reboot.c
@@ -22,8 +22,6 @@ void machine_restart(char * __unused)
 	outb(PIIX4_RESET_VAL, PIIX4_RESET_PORT);
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_power_off(void)
 {
 	unsigned short pm_status;
@@ -43,10 +41,7 @@ void machine_power_off(void)
 	outl(PIIX_SPECIAL_STOP, 0xCFC);
 }
 
-EXPORT_SYMBOL(machine_power_off);
-
 void machine_halt(void)
 {
 }
 
-EXPORT_SYMBOL(machine_halt);
diff --git a/arch/i386/mach-voyager/voyager_basic.c b/arch/i386/mach-voyager/voyager_basic.c
--- a/arch/i386/mach-voyager/voyager_basic.c
+++ b/arch/i386/mach-voyager/voyager_basic.c
@@ -278,8 +278,6 @@ machine_restart(char *cmd)
 	}
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void
 mca_nmi_hook(void)
 {
@@ -315,12 +313,9 @@ machine_halt(void)
 	machine_power_off();
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void machine_power_off(void)
 {
 	if (pm_power_off)
 		pm_power_off();
 }
 
-EXPORT_SYMBOL(machine_power_off);
diff --git a/arch/ia64/kernel/process.c b/arch/ia64/kernel/process.c
--- a/arch/ia64/kernel/process.c
+++ b/arch/ia64/kernel/process.c
@@ -807,16 +807,12 @@ machine_restart (char *restart_cmd)
 	(*efi.reset_system)(EFI_RESET_WARM, 0, 0, NULL);
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void
 machine_halt (void)
 {
 	cpu_halt();
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void
 machine_power_off (void)
 {
@@ -825,4 +821,3 @@ machine_power_off (void)
 	machine_halt();
 }
 
-EXPORT_SYMBOL(machine_power_off);
diff --git a/arch/m32r/kernel/process.c b/arch/m32r/kernel/process.c
--- a/arch/m32r/kernel/process.c
+++ b/arch/m32r/kernel/process.c
@@ -115,8 +115,6 @@ void machine_restart(char *__unused)
 		cpu_relax();
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_halt(void)
 {
 	printk("Please push reset button!\n");
@@ -124,15 +122,11 @@ void machine_halt(void)
 		cpu_relax();
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void machine_power_off(void)
 {
 	/* M32R_FIXME */
 }
 
-EXPORT_SYMBOL(machine_power_off);
-
 static int __init idle_setup (char *str)
 {
 	if (!strncmp(str, "poll", 4)) {
diff --git a/arch/m68k/kernel/process.c b/arch/m68k/kernel/process.c
--- a/arch/m68k/kernel/process.c
+++ b/arch/m68k/kernel/process.c
@@ -113,8 +113,6 @@ void machine_restart(char * __unused)
 	for (;;);
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_halt(void)
 {
 	if (mach_halt)
@@ -122,8 +120,6 @@ void machine_halt(void)
 	for (;;);
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void machine_power_off(void)
 {
 	if (mach_power_off)
@@ -131,8 +127,6 @@ void machine_power_off(void)
 	for (;;);
 }
 
-EXPORT_SYMBOL(machine_power_off);
-
 void show_regs(struct pt_regs * regs)
 {
 	printk("\n");
diff --git a/arch/m68knommu/kernel/process.c b/arch/m68knommu/kernel/process.c
--- a/arch/m68knommu/kernel/process.c
+++ b/arch/m68knommu/kernel/process.c
@@ -80,8 +80,6 @@ void machine_restart(char * __unused)
 	for (;;);
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_halt(void)
 {
 	if (mach_halt)
@@ -89,8 +87,6 @@ void machine_halt(void)
 	for (;;);
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void machine_power_off(void)
 {
 	if (mach_power_off)
@@ -98,8 +94,6 @@ void machine_power_off(void)
 	for (;;);
 }
 
-EXPORT_SYMBOL(machine_power_off);
-
 void show_regs(struct pt_regs * regs)
 {
 	printk(KERN_NOTICE "\n");
diff --git a/arch/mips/kernel/reset.c b/arch/mips/kernel/reset.c
--- a/arch/mips/kernel/reset.c
+++ b/arch/mips/kernel/reset.c
@@ -26,18 +26,13 @@ void machine_restart(char *command)
 	_machine_restart(command);
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_halt(void)
 {
 	_machine_halt();
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void machine_power_off(void)
 {
 	_machine_power_off();
 }
 
-EXPORT_SYMBOL(machine_power_off);
diff --git a/arch/parisc/kernel/process.c b/arch/parisc/kernel/process.c
--- a/arch/parisc/kernel/process.c
+++ b/arch/parisc/kernel/process.c
@@ -150,8 +150,6 @@ void machine_restart(char *cmd)
 
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_halt(void)
 {
 	/*
@@ -160,8 +158,6 @@ void machine_halt(void)
 	*/
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 
 /*
  * This routine is called from sys_reboot to actually turn off the
@@ -187,8 +183,6 @@ void machine_power_off(void)
 	       KERN_EMERG "Please power this system off now.");
 }
 
-EXPORT_SYMBOL(machine_power_off);
-
 
 /*
  * Create a kernel thread
diff --git a/arch/ppc/kernel/setup.c b/arch/ppc/kernel/setup.c
--- a/arch/ppc/kernel/setup.c
+++ b/arch/ppc/kernel/setup.c
@@ -121,8 +121,6 @@ void machine_restart(char *cmd)
 	ppc_md.restart(cmd);
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_power_off(void)
 {
 #ifdef CONFIG_NVRAM
@@ -131,8 +129,6 @@ void machine_power_off(void)
 	ppc_md.power_off();
 }
 
-EXPORT_SYMBOL(machine_power_off);
-
 void machine_halt(void)
 {
 #ifdef CONFIG_NVRAM
@@ -141,8 +137,6 @@ void machine_halt(void)
 	ppc_md.halt();
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void (*pm_power_off)(void) = machine_power_off;
 
 #ifdef CONFIG_TAU
diff --git a/arch/ppc64/kernel/setup.c b/arch/ppc64/kernel/setup.c
--- a/arch/ppc64/kernel/setup.c
+++ b/arch/ppc64/kernel/setup.c
@@ -694,7 +694,6 @@ void machine_restart(char *cmd)
 	local_irq_disable();
 	while (1) ;
 }
-EXPORT_SYMBOL(machine_restart);
 
 void machine_power_off(void)
 {
@@ -707,7 +706,6 @@ void machine_power_off(void)
 	local_irq_disable();
 	while (1) ;
 }
-EXPORT_SYMBOL(machine_power_off);
 
 void machine_halt(void)
 {
@@ -720,7 +718,6 @@ void machine_halt(void)
 	local_irq_disable();
 	while (1) ;
 }
-EXPORT_SYMBOL(machine_halt);
 
 static int ppc64_panic_event(struct notifier_block *this,
                              unsigned long event, void *ptr)
diff --git a/arch/s390/kernel/setup.c b/arch/s390/kernel/setup.c
--- a/arch/s390/kernel/setup.c
+++ b/arch/s390/kernel/setup.c
@@ -299,24 +299,18 @@ void machine_restart(char *command)
 	_machine_restart(command);
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_halt(void)
 {
 	console_unblank();
 	_machine_halt();
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void machine_power_off(void)
 {
 	console_unblank();
 	_machine_power_off();
 }
 
-EXPORT_SYMBOL(machine_power_off);
-
 static void __init
 add_memory_hole(unsigned long start, unsigned long end)
 {
diff --git a/arch/sh/kernel/process.c b/arch/sh/kernel/process.c
--- a/arch/sh/kernel/process.c
+++ b/arch/sh/kernel/process.c
@@ -80,8 +80,6 @@ void machine_restart(char * __unused)
 		     "mov.l @%1, %0" : : "r" (0x10000000), "r" (0x80000001));
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_halt(void)
 {
 #if defined(CONFIG_SH_HS7751RVOIP)
@@ -96,8 +94,6 @@ void machine_halt(void)
 		cpu_sleep();
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void machine_power_off(void)
 {
 #if defined(CONFIG_SH_HS7751RVOIP)
@@ -110,8 +106,6 @@ void machine_power_off(void)
 #endif
 }
 
-EXPORT_SYMBOL(machine_power_off);
-
 void show_regs(struct pt_regs * regs)
 {
 	printk("\n");
diff --git a/arch/sparc/kernel/process.c b/arch/sparc/kernel/process.c
--- a/arch/sparc/kernel/process.c
+++ b/arch/sparc/kernel/process.c
@@ -158,8 +158,6 @@ void machine_halt(void)
 	panic("Halt failed!");
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void machine_restart(char * cmd)
 {
 	char *p;
@@ -180,8 +178,6 @@ void machine_restart(char * cmd)
 	panic("Reboot failed!");
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_power_off(void)
 {
 #ifdef CONFIG_SUN_AUXIO
@@ -191,8 +187,6 @@ void machine_power_off(void)
 	machine_halt();
 }
 
-EXPORT_SYMBOL(machine_power_off);
-
 static DEFINE_SPINLOCK(sparc_backtrace_lock);
 
 void __show_backtrace(unsigned long fp)
diff --git a/arch/sparc64/kernel/power.c b/arch/sparc64/kernel/power.c
--- a/arch/sparc64/kernel/power.c
+++ b/arch/sparc64/kernel/power.c
@@ -69,8 +69,6 @@ void machine_power_off(void)
 	machine_halt();
 }
 
-EXPORT_SYMBOL(machine_power_off);
-
 #ifdef CONFIG_PCI
 static int powerd(void *__unused)
 {
diff --git a/arch/sparc64/kernel/process.c b/arch/sparc64/kernel/process.c
--- a/arch/sparc64/kernel/process.c
+++ b/arch/sparc64/kernel/process.c
@@ -124,8 +124,6 @@ void machine_halt(void)
 	panic("Halt failed!");
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void machine_alt_power_off(void)
 {
 	if (!serial_console && prom_palette)
@@ -154,8 +152,6 @@ void machine_restart(char * cmd)
 	panic("Reboot failed!");
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 static void show_regwindow32(struct pt_regs *regs)
 {
 	struct reg_window32 __user *rw;
diff --git a/arch/um/kernel/reboot.c b/arch/um/kernel/reboot.c
--- a/arch/um/kernel/reboot.c
+++ b/arch/um/kernel/reboot.c
@@ -49,23 +49,17 @@ void machine_restart(char * __unused)
 	CHOOSE_MODE(reboot_tt(), reboot_skas());
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_power_off(void)
 {
         uml_cleanup();
 	CHOOSE_MODE(halt_tt(), halt_skas());
 }
 
-EXPORT_SYMBOL(machine_power_off);
-
 void machine_halt(void)
 {
 	machine_power_off();
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 /*
  * Overrides for Emacs so that we follow Linus's tabbing style.
  * Emacs will notice this stuff at the end of the file and automatically
diff --git a/arch/v850/kernel/anna.c b/arch/v850/kernel/anna.c
--- a/arch/v850/kernel/anna.c
+++ b/arch/v850/kernel/anna.c
@@ -132,8 +132,6 @@ void machine_restart (char *__unused)
 	asm ("jmp r0"); /* Jump to the reset vector.  */
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_halt (void)
 {
 #ifdef CONFIG_RESET_GUARD
@@ -145,15 +143,11 @@ void machine_halt (void)
 		asm ("halt; nop; nop; nop; nop; nop");
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void machine_power_off (void)
 {
 	machine_halt ();
 }
 
-EXPORT_SYMBOL(machine_power_off);
-
 /* Called before configuring an on-chip UART.  */
 void anna_uart_pre_configure (unsigned chan, unsigned cflags, unsigned baud)
 {
diff --git a/arch/v850/kernel/as85ep1.c b/arch/v850/kernel/as85ep1.c
--- a/arch/v850/kernel/as85ep1.c
+++ b/arch/v850/kernel/as85ep1.c
@@ -160,8 +160,6 @@ void machine_restart (char *__unused)
 	asm ("jmp r0"); /* Jump to the reset vector.  */
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_halt (void)
 {
 #ifdef CONFIG_RESET_GUARD
@@ -173,15 +171,11 @@ void machine_halt (void)
 		asm ("halt; nop; nop; nop; nop; nop");
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void machine_power_off (void)
 {
 	machine_halt ();
 }
 
-EXPORT_SYMBOL(machine_power_off);
-
 /* Called before configuring an on-chip UART.  */
 void as85ep1_uart_pre_configure (unsigned chan, unsigned cflags, unsigned baud)
 {
diff --git a/arch/v850/kernel/fpga85e2c.c b/arch/v850/kernel/fpga85e2c.c
--- a/arch/v850/kernel/fpga85e2c.c
+++ b/arch/v850/kernel/fpga85e2c.c
@@ -121,22 +121,16 @@ void machine_halt (void)
 	}
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void machine_restart (char *__unused)
 {
 	machine_halt ();
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_power_off (void)
 {
 	machine_halt ();
 }
 
-EXPORT_SYMBOL(machine_power_off);
-
 \f
 /* Interrupts */
 
diff --git a/arch/v850/kernel/rte_cb.c b/arch/v850/kernel/rte_cb.c
--- a/arch/v850/kernel/rte_cb.c
+++ b/arch/v850/kernel/rte_cb.c
@@ -67,8 +67,6 @@ void machine_restart (char *__unused)
 	asm ("jmp r0"); /* Jump to the reset vector.  */
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 /* This says `HALt.' in LEDese.  */
 static unsigned char halt_leds_msg[] = { 0x76, 0x77, 0x38, 0xF8 };
 
@@ -89,15 +87,11 @@ void machine_halt (void)
 		asm ("halt; nop; nop; nop; nop; nop");
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void machine_power_off (void)
 {
 	machine_halt ();
 }
 
-EXPORT_SYMBOL(machine_power_off);
-
 \f
 /* Animated LED display for timer tick.  */
 
diff --git a/arch/v850/kernel/sim.c b/arch/v850/kernel/sim.c
--- a/arch/v850/kernel/sim.c
+++ b/arch/v850/kernel/sim.c
@@ -104,24 +104,18 @@ void machine_restart (char *__unused)
 	V850_SIM_SYSCALL (exit, 0);
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_halt (void)
 {
 	V850_SIM_SYSCALL (write, 1, "HALT\n", 5);
 	V850_SIM_SYSCALL (exit, 0);
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void machine_power_off (void)
 {
 	V850_SIM_SYSCALL (write, 1, "POWER OFF\n", 10);
 	V850_SIM_SYSCALL (exit, 0);
 }
 
-EXPORT_SYMBOL(machine_power_off);
-
 \f
 /* Load data from a file called NAME into ram.  The address and length
    of the data image are returned in ADDR and LEN.  */
diff --git a/arch/v850/kernel/sim85e2.c b/arch/v850/kernel/sim85e2.c
--- a/arch/v850/kernel/sim85e2.c
+++ b/arch/v850/kernel/sim85e2.c
@@ -184,18 +184,13 @@ void machine_halt (void)
 	for (;;) {}
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void machine_restart (char *__unused)
 {
 	machine_halt ();
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_power_off (void)
 {
 	machine_halt ();
 }
 
-EXPORT_SYMBOL(machine_power_off);
diff --git a/arch/x86_64/kernel/reboot.c b/arch/x86_64/kernel/reboot.c
--- a/arch/x86_64/kernel/reboot.c
+++ b/arch/x86_64/kernel/reboot.c
@@ -150,18 +150,13 @@ void machine_restart(char * __unused)
 	}      
 }
 
-EXPORT_SYMBOL(machine_restart);
-
 void machine_halt(void)
 {
 }
 
-EXPORT_SYMBOL(machine_halt);
-
 void machine_power_off(void)
 {
 	if (pm_power_off)
 		pm_power_off();
 }
 
-EXPORT_SYMBOL(machine_power_off);

[PATCH 7/23] i386: Implement machine_emergency_reboot

[Eric W. Biederman]

set_cpus_allowed is not safe in interrupt context
and disabling apics is complicated code so don't
call machine_shutdown on i386 from emergency_restart().

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 arch/i386/kernel/reboot.c            |   10 +++++++---
 include/asm-i386/emergency-restart.h |    2 +-
 2 files changed, 8 insertions(+), 4 deletions(-)

fcfe4ee919b83e5002dd696a94834fd3ccb31be8
diff --git a/arch/i386/kernel/reboot.c b/arch/i386/kernel/reboot.c
--- a/arch/i386/kernel/reboot.c
+++ b/arch/i386/kernel/reboot.c
@@ -311,10 +311,8 @@ void machine_shutdown(void)
 #endif
 }
 
-void machine_restart(char * __unused)
+void machine_emergency_restart(void)
 {
-	machine_shutdown();
-
 	if (!reboot_thru_bios) {
 		if (efi_enabled) {
 			efi.reset_system(EFI_RESET_COLD, EFI_SUCCESS, 0, NULL);
@@ -337,6 +335,12 @@ void machine_restart(char * __unused)
 	machine_real_restart(jump_to_bios, sizeof(jump_to_bios));
 }
 
+void machine_restart(char * __unused)
+{
+	machine_shutdown();
+	machine_emergency_restart();
+}
+
 void machine_halt(void)
 {
 }
diff --git a/include/asm-i386/emergency-restart.h b/include/asm-i386/emergency-restart.h
--- a/include/asm-i386/emergency-restart.h
+++ b/include/asm-i386/emergency-restart.h
@@ -1,6 +1,6 @@
 #ifndef _ASM_EMERGENCY_RESTART_H
 #define _ASM_EMERGENCY_RESTART_H
 
-#include <asm-generic/emergency-restart.h>
+extern void machine_emergency_restart(void);
 
 #endif /* _ASM_EMERGENCY_RESTART_H */

[PATCH 8/23] x86_64: Fix reboot_force

[Eric W. Biederman]

We only want to shutdown the apics if reboot_force
is not specified.  Be we are doing this both
in machine_shutdown which is called unconditionally
and if (!reboot_force).  So simply call machine_shutdown
if (!reboot_force).  It looks like something
went weird with merging some of the kexec patches for
x86_64, and caused this.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 arch/x86_64/kernel/reboot.c |    9 +--------
 1 files changed, 1 insertions(+), 8 deletions(-)

a044301f20f8e977206a79ade92c6b385f9e2703
diff --git a/arch/x86_64/kernel/reboot.c b/arch/x86_64/kernel/reboot.c
--- a/arch/x86_64/kernel/reboot.c
+++ b/arch/x86_64/kernel/reboot.c
@@ -115,15 +115,8 @@ void machine_restart(char * __unused)
 
 	printk("machine restart\n");
 
-	machine_shutdown();
-
 	if (!reboot_force) {
-		local_irq_disable();
-#ifndef CONFIG_SMP
-		disable_local_APIC();
-#endif
-		disable_IO_APIC();
-		local_irq_enable();
+		machine_shutdown();
 	}
 	
 	/* Tell the BIOS if we want cold or warm reboot */

[PATCH 9/23] x86_64: Implemenent machine_emergency_restart

[Eric W. Biederman]

It is not safe to call set_cpus_allowed() in interrupt
context and disabling the apics is complicated code.
So unconditionally skip machine_shutdown in machine_emergency_reboot
on x86_64.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 arch/x86_64/kernel/reboot.c            |   18 +++++++++++-------
 include/asm-x86_64/emergency-restart.h |    2 +-
 2 files changed, 12 insertions(+), 8 deletions(-)

c274d84d274bd40c63be01cd6a4ba26ae2be135a
diff --git a/arch/x86_64/kernel/reboot.c b/arch/x86_64/kernel/reboot.c
--- a/arch/x86_64/kernel/reboot.c
+++ b/arch/x86_64/kernel/reboot.c
@@ -109,16 +109,10 @@ void machine_shutdown(void)
 	local_irq_enable();
 }
 
-void machine_restart(char * __unused)
+void machine_emergency_restart(void)
 {
 	int i;
 
-	printk("machine restart\n");
-
-	if (!reboot_force) {
-		machine_shutdown();
-	}
-	
 	/* Tell the BIOS if we want cold or warm reboot */
 	*((unsigned short *)__va(0x472)) = reboot_mode;
        
@@ -143,6 +137,16 @@ void machine_restart(char * __unused)
 	}      
 }
 
+void machine_restart(char * __unused)
+{
+	printk("machine restart\n");
+
+	if (!reboot_force) {
+		machine_shutdown();
+	}
+	machine_emergency_restart();
+}
+
 void machine_halt(void)
 {
 }
diff --git a/include/asm-x86_64/emergency-restart.h b/include/asm-x86_64/emergency-restart.h
--- a/include/asm-x86_64/emergency-restart.h
+++ b/include/asm-x86_64/emergency-restart.h
@@ -1,6 +1,6 @@
 #ifndef _ASM_EMERGENCY_RESTART_H
 #define _ASM_EMERGENCY_RESTART_H
 
-#include <asm-generic/emergency-restart.h>
+extern void machine_emergency_restart(void);
 
 #endif /* _ASM_EMERGENCY_RESTART_H */

[PATCH 10/23] Use kernel_power_off in sysrq-o

[Eric W. Biederman]

We already do all of the gymnastics to run from process context
to call the power off code so call into the power off code cleanly.

This especially helps acpi as part of it's shutdown logic should
run acpi_shutdown called from device_shutdown which was not
being called from here.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 kernel/power/poweroff.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

acf31d44f4864bf13fa5a9fe49e0cc9c0ec93cee
diff --git a/kernel/power/poweroff.c b/kernel/power/poweroff.c
--- a/kernel/power/poweroff.c
+++ b/kernel/power/poweroff.c
@@ -9,6 +9,7 @@
 #include <linux/init.h>
 #include <linux/pm.h>
 #include <linux/workqueue.h>
+#include <linux/reboot.h>
 
 /*
  * When the user hits Sys-Rq o to power down the machine this is the
@@ -17,8 +18,7 @@
 
 static void do_poweroff(void *dummy)
 {
-	if (pm_power_off)
-		pm_power_off();
+	kernel_power_off();
 }
 
 static DECLARE_WORK(poweroff_work, do_poweroff, NULL);

[PATCH 11/23] Call emergency_reboot from panic

[Eric W. Biederman]

We know the system is in trouble so there is no question if this
is an emergecy :)

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 kernel/panic.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

fd248dd54c86f633bd2618d8c475c39465f8d552
diff --git a/kernel/panic.c b/kernel/panic.c
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -111,12 +111,11 @@ NORET_TYPE void panic(const char * fmt, 
 			mdelay(1);
 			i++;
 		}
-		/*
-		 *	Should we run the reboot notifier. For the moment Im
-		 *	choosing not too. It might crash, be corrupt or do
-		 *	more harm than good for other reasons.
+		/*	This will not be a clean reboot, with everything
+		 *	shutting down.  But if there is a chance of
+		 *	rebooting the system it will be rebooted.
 		 */
-		machine_restart(NULL);
+		emergency_restart();
 	}
 #ifdef __sparc__
 	{

[PATCH 12/23] Update sysrq-B to use emergency_restart()

[Eric W. Biederman]

sysrq calls into the reboot path from an interrupt handler
we can either push the code do into process context and
call kernel_restart and get a clean reboot or we can simply
reboot the machine, and increase our chances of actually
rebooting.  emergency_reboot() seems like the closest match
to what we have previously done, and what we want.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 drivers/char/sysrq.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

d7b573e957fb71b166223ee15ea97c93c14f5faa
diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
--- a/drivers/char/sysrq.c
+++ b/drivers/char/sysrq.c
@@ -115,7 +115,7 @@ static void sysrq_handle_reboot(int key,
 				struct tty_struct *tty) 
 {
 	local_irq_enable();
-	machine_restart(NULL);
+	emergency_restart();
 }
 
 static struct sysrq_key_op sysrq_reboot_op = {

[PATCH 13/23] Fix watchdog drivers to call emergency_reboot()

[Eric W. Biederman]

If a watchdog driver has decided it is time to reboot the system
we know something is wrong and we are in interrupt context
so emergency_reboot() is what we want.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 drivers/char/watchdog/eurotechwdt.c |    2 +-
 drivers/char/watchdog/softdog.c     |    2 +-
 drivers/char/watchdog/wdt.c         |    2 +-
 drivers/char/watchdog/wdt_pci.c     |    2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

7dda2f8e340a007a29cef304bee65437d09d4738
diff --git a/drivers/char/watchdog/eurotechwdt.c b/drivers/char/watchdog/eurotechwdt.c
--- a/drivers/char/watchdog/eurotechwdt.c
+++ b/drivers/char/watchdog/eurotechwdt.c
@@ -167,7 +167,7 @@ static irqreturn_t eurwdt_interrupt(int 
 	printk(KERN_CRIT "Would Reboot.\n");
 #else
 	printk(KERN_CRIT "Initiating system reboot.\n");
-	machine_restart(NULL);
+	emergency_restart(NULL);
 #endif
 	return IRQ_HANDLED;
 }
diff --git a/drivers/char/watchdog/softdog.c b/drivers/char/watchdog/softdog.c
--- a/drivers/char/watchdog/softdog.c
+++ b/drivers/char/watchdog/softdog.c
@@ -97,7 +97,7 @@ static void watchdog_fire(unsigned long 
 	else
 	{
 		printk(KERN_CRIT PFX "Initiating system reboot.\n");
-		machine_restart(NULL);
+		emergency_restart(NULL);
 		printk(KERN_CRIT PFX "Reboot didn't ?????\n");
 	}
 }
diff --git a/drivers/char/watchdog/wdt.c b/drivers/char/watchdog/wdt.c
--- a/drivers/char/watchdog/wdt.c
+++ b/drivers/char/watchdog/wdt.c
@@ -266,7 +266,7 @@ static irqreturn_t wdt_interrupt(int irq
 		printk(KERN_CRIT "Would Reboot.\n");
 #else
 		printk(KERN_CRIT "Initiating system reboot.\n");
-		machine_restart(NULL);
+		emergency_restart();
 #endif
 #else
 		printk(KERN_CRIT "Reset in 5ms.\n");
diff --git a/drivers/char/watchdog/wdt_pci.c b/drivers/char/watchdog/wdt_pci.c
--- a/drivers/char/watchdog/wdt_pci.c
+++ b/drivers/char/watchdog/wdt_pci.c
@@ -311,7 +311,7 @@ static irqreturn_t wdtpci_interrupt(int 
 		printk(KERN_CRIT PFX "Would Reboot.\n");
 #else
 		printk(KERN_CRIT PFX "Initiating system reboot.\n");
-		machine_restart(NULL);
+		emergency_restart(NULL);
 #endif
 #else
 		printk(KERN_CRIT PFX "Reset in 5ms.\n");

Re: [PATCH 1/23] Add missing device_suspsend(PMSG_FREEZE) calls.

[Nigel Cunningham]

Hi.

Could you please send PMSG_* related patches to linux-pm at
lists.osdl.org as well?

Thanks!

Nigel

On Wed, 2005-07-27 at 03:21, Eric W. Biederman wrote:
> In the recent addition of device_suspend calls into
> sys_reboot two code paths were missed.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> ---
> 
>  kernel/sys.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> 5f0fb00783b94248b5a76c161f1c30a033fce4d3
> diff --git a/kernel/sys.c b/kernel/sys.c
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -391,6 +391,7 @@ asmlinkage long sys_reboot(int magic1, i
>  	case LINUX_REBOOT_CMD_RESTART:
>  		notifier_call_chain(&reboot_notifier_list, SYS_RESTART, NULL);
>  		system_state = SYSTEM_RESTART;
> +		device_suspend(PMSG_FREEZE);
>  		device_shutdown();
>  		printk(KERN_EMERG "Restarting system.\n");
>  		machine_restart(NULL);
> @@ -452,6 +453,7 @@ asmlinkage long sys_reboot(int magic1, i
>  		}
>  		notifier_call_chain(&reboot_notifier_list, SYS_RESTART, NULL);
>  		system_state = SYSTEM_RESTART;
> +		device_suspend(PMSG_FREEZE);
>  		device_shutdown();
>  		printk(KERN_EMERG "Starting new kernel\n");
>  		machine_shutdown();
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
-- 
Evolution.
Enumerate the requirements.
Consider the interdependencies.
Calculate the probabilities.

[PATCH 14/23] In hangcheck-timer.c call emergency_restart()

[Eric W. Biederman]

If we've hung a clean reboot does not sound like a real
option.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 drivers/char/hangcheck-timer.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

8d6217cc6e8fa4832c84fb918848ba0eb2e0fb0e
diff --git a/drivers/char/hangcheck-timer.c b/drivers/char/hangcheck-timer.c
--- a/drivers/char/hangcheck-timer.c
+++ b/drivers/char/hangcheck-timer.c
@@ -173,7 +173,7 @@ static void hangcheck_fire(unsigned long
 		}
 		if (hangcheck_reboot) {
 			printk(KERN_CRIT "Hangcheck: hangcheck is restarting the machine.\n");
-			machine_restart(NULL);
+			emergency_restart();
 		} else {
 			printk(KERN_CRIT "Hangcheck: hangcheck value past margin!\n");
 		}

[PATCH 15/23] 68328serial: sysrq should use emergency_reboot

[Eric W. Biederman]

The 68328serial.c driver has a weird local reimplementation of
magic sysrq.  The code is architecture specific enough that calling
machine_restart() is probably ok.  But there is no reason not to call
emergency_restart() so do so.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 drivers/serial/68328serial.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

eaa1c799cd187691a28251a4e2db288cde518b13
diff --git a/drivers/serial/68328serial.c b/drivers/serial/68328serial.c
--- a/drivers/serial/68328serial.c
+++ b/drivers/serial/68328serial.c
@@ -316,7 +316,7 @@ static _INLINE_ void receive_chars(struc
 /*				show_net_buffers(); */
 				return;
 			} else if (ch == 0x12) { /* ^R */
-				machine_restart(NULL);
+				emergency_restart();
 				return;
 #endif /* CONFIG_MAGIC_SYSRQ */
 			}

[PATCH 16/23] swpsuspend: Have suspend to disk use factors of sys_reboot

[Eric W. Biederman]

The suspend to disk code was a poor copy of the code in
sys_reboot now that we have kernel_power_off, kernel_restart
and kernel_halt use them instead of poorly duplicating them inline.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 kernel/power/disk.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

78a2f83d732e327874fe73728d5667875dfeea46
diff --git a/kernel/power/disk.c b/kernel/power/disk.c
--- a/kernel/power/disk.c
+++ b/kernel/power/disk.c
@@ -59,16 +59,13 @@ static void power_down(suspend_disk_meth
 		error = pm_ops->enter(PM_SUSPEND_DISK);
 		break;
 	case PM_DISK_SHUTDOWN:
-		printk("Powering off system\n");
-		device_shutdown();
-		machine_power_off();
+		kernel_power_off();
 		break;
 	case PM_DISK_REBOOT:
-		device_shutdown();
-		machine_restart(NULL);
+		kernel_restart(NULL);
 		break;
 	}
-	machine_halt();
+	kernel_halt();
 	/* Valid image is on the disk, if we continue we risk serious data corruption
 	   after resume. */
 	printk(KERN_CRIT "Please power me down manually\n");

[PATCH 17/23] pcwd.c: Call kernel_power_off not machine_power_off

[Eric W. Biederman]

The call appears to come from process context so kernel_power_off
should be safe.  And acpi_power_off won't necessarily work if you just
call machine_power_off.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 drivers/char/watchdog/pcwd.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

b663accd4b051598a8c877aeb2e4ee7da94e3af3
diff --git a/drivers/char/watchdog/pcwd.c b/drivers/char/watchdog/pcwd.c
--- a/drivers/char/watchdog/pcwd.c
+++ b/drivers/char/watchdog/pcwd.c
@@ -344,7 +344,7 @@ static int pcwd_get_status(int *status)
 			*status |= WDIOF_OVERHEAT;
 			if (temp_panic) {
 				printk (KERN_INFO PFX "Temperature overheat trip!\n");
-				machine_power_off();
+				kernel_power_off();
 			}
 		}
 	} else {
@@ -355,7 +355,7 @@ static int pcwd_get_status(int *status)
 			*status |= WDIOF_OVERHEAT;
 			if (temp_panic) {
 				printk (KERN_INFO PFX "Temperature overheat trip!\n");
-				machine_power_off();
+				kernel_power_off();
 			}
 		}
 	}

[PATCH 18/23] machine_shutdown: Typo fix to actually allow specifying which cpu to reboot on

[Eric W. Biederman]

This appears to be a typo I introduced when cleaning
this code up earlier. Ooops.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 arch/i386/kernel/reboot.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

75a1cfd3c44dbea3af15e04704c7db2be70478c7
diff --git a/arch/i386/kernel/reboot.c b/arch/i386/kernel/reboot.c
--- a/arch/i386/kernel/reboot.c
+++ b/arch/i386/kernel/reboot.c
@@ -284,7 +284,7 @@ void machine_shutdown(void)
 	reboot_cpu_id = 0;
 
 	/* See if there has been given a command line override */
-	if ((reboot_cpu_id != -1) && (reboot_cpu < NR_CPUS) &&
+	if ((reboot_cpu != -1) && (reboot_cpu < NR_CPUS) &&
 		cpu_isset(reboot_cpu, cpu_online_map)) {
 		reboot_cpu_id = reboot_cpu;
 	}

[PATCH 19/23] i386 machine_power_off cleanup

[Eric W. Biederman]

Call machine_shutdown() to move to the boot cpu
and disable apics.  Both acpi_power_off and
apm_power_off want to move to the boot cpu.
and we are already disabling the local apics
so calling machine_shutdown simply reuses
code.

ia64 doesn't have a special path in power_off
for efi so there is no reason i386 should.  If
we really need to call the efi power off path
the efi driver can set pm_power_off like everyone
else.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 arch/i386/kernel/reboot.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

9f163caa28f9d3392f0d8d3e5f131ea658a2a887
diff --git a/arch/i386/kernel/reboot.c b/arch/i386/kernel/reboot.c
--- a/arch/i386/kernel/reboot.c
+++ b/arch/i386/kernel/reboot.c
@@ -347,10 +347,8 @@ void machine_halt(void)
 
 void machine_power_off(void)
 {
-	lapic_shutdown();
+	machine_shutdown();
 
-	if (efi_enabled)
-		efi.reset_system(EFI_RESET_SHUTDOWN, EFI_SUCCESS, 0, NULL);
 	if (pm_power_off)
 		pm_power_off();
 }

[PATCH 20/23] APM: Remove redundant call to set_cpus_allowed

[Eric W. Biederman]

machine_power_off now always switches to the boot cpu so there
is no reason for APM to also do that.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 arch/i386/kernel/apm.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

8e1e879e7ead62da7d2c2030eebbf8142547b619
diff --git a/arch/i386/kernel/apm.c b/arch/i386/kernel/apm.c
--- a/arch/i386/kernel/apm.c
+++ b/arch/i386/kernel/apm.c
@@ -911,14 +911,7 @@ static void apm_power_off(void)
 		0xcd, 0x15		/* int   $0x15       */
 	};
 
-	/*
-	 * This may be called on an SMP machine.
-	 */
-#ifdef CONFIG_SMP
 	/* Some bioses don't like being called from CPU != 0 */
-	set_cpus_allowed(current, cpumask_of_cpu(0));
-	BUG_ON(smp_processor_id() != 0);
-#endif
 	if (apm_info.realmode_power_off)
 	{
 		(void)apm_save_cpus();

[PATCH 21/23] x86_64 sync machine_power_off with i386

[Eric W. Biederman]

i386 machine_power_off was disabling the local apic
and all of it's users wanted to be on the boot cpu.
So call machine_shutdown which places us on the boot
cpu and disables the apics.  This keeps us in sync
and reduces the number of cases we need to worry about in
the power management code.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 arch/x86_64/kernel/reboot.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

dbc7607f71b4c6e38b141340f7afd33248a4205a
diff --git a/arch/x86_64/kernel/reboot.c b/arch/x86_64/kernel/reboot.c
--- a/arch/x86_64/kernel/reboot.c
+++ b/arch/x86_64/kernel/reboot.c
@@ -153,6 +153,9 @@ void machine_halt(void)
 
 void machine_power_off(void)
 {
+	if (!reboot_force) {
+		machine_shutdown();
+	}
 	if (pm_power_off)
 		pm_power_off();
 }

[PATCH 22/23] acpi_power_off: Don't switch to the boot cpu

[Eric W. Biederman]

machine_power_off on i386 and x86_64 now switch to the
boot cpu out of paranoia and because the MP Specification indicates it
is a good idea on reboot, so for those architectures it is a noop.
I can't see anything in the acpi spec that requires you to be on
the boot cpu to power off the system, so this should not be an issue
for ia64.  In addition ia64 has the altix a massive multi-node
system where switching to the boot cpu sounds insane as we may
hot removed the boot cpu.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 drivers/acpi/sleep/poweroff.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

c4ca5713b37cce7fcfdb8f212c789b552fc55e6f
diff --git a/drivers/acpi/sleep/poweroff.c b/drivers/acpi/sleep/poweroff.c
--- a/drivers/acpi/sleep/poweroff.c
+++ b/drivers/acpi/sleep/poweroff.c
@@ -54,7 +54,6 @@ void acpi_power_off(void)
 	acpi_sleep_prepare(ACPI_STATE_S5);
 	local_irq_disable();
 	/* Some SMP machines only can poweroff in boot CPU */
-	set_cpus_allowed(current, cpumask_of_cpu(0));
 	acpi_enter_sleep_state(ACPI_STATE_S5);
 }
 

[PATCH 23/23] acpi: Don't call acpi_sleep_prepare from acpi_power_off

[Eric W. Biederman]

Now that all of the code paths that call acpi_power_off
have been modified to call either call kernel_power_off
(which calls apci_sleep_prepare by way of acpi_shutdown)
or to call acpi_sleep_prepare directly it is redundant to call
acpi_sleep_prepare from acpi_power_off.

So simplify the code and simply don't call acpi_sleep_prepare.

In addition there is a little error handling done so if we
can't register the acpi class we don't hook pm_power_off.

I think I have done the right thing with the CONFIG_PM define
but I'm not certain.  Can this code even be compiled if
CONFIG_PM is false?

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 drivers/acpi/sleep/poweroff.c |   33 ++++++++++++---------------------
 1 files changed, 12 insertions(+), 21 deletions(-)

415ee58560128bb4f90f8252c189bdf489d1f0d3
diff --git a/drivers/acpi/sleep/poweroff.c b/drivers/acpi/sleep/poweroff.c
--- a/drivers/acpi/sleep/poweroff.c
+++ b/drivers/acpi/sleep/poweroff.c
@@ -19,8 +19,6 @@
 
 int acpi_sleep_prepare(u32 acpi_state)
 {
-	/* Flag to do not allow second time invocation for S5 state */
-	static int shutdown_prepared = 0;
 #ifdef CONFIG_ACPI_SLEEP
 	/* do we have a wakeup address for S2 and S3? */
 	/* Here, we support only S4BIOS, those we set the wakeup address */
@@ -38,27 +36,23 @@ int acpi_sleep_prepare(u32 acpi_state)
 	acpi_enable_wakeup_device_prep(acpi_state);
 #endif
 	if (acpi_state == ACPI_STATE_S5) {
-		/* Check if we were already called */
-		if (shutdown_prepared)
-			return 0;
 		acpi_wakeup_gpe_poweroff_prepare();
-		shutdown_prepared = 1;
 	}
 	acpi_enter_sleep_state_prep(acpi_state);
 	return 0;
 }
 
+#ifdef CONFIG_PM
+
 void acpi_power_off(void)
 {
+	/* acpi_sleep_prepare(ACPI_STATE_S5) should have already been called */
 	printk("%s called\n", __FUNCTION__);
-	acpi_sleep_prepare(ACPI_STATE_S5);
 	local_irq_disable();
 	/* Some SMP machines only can poweroff in boot CPU */
 	acpi_enter_sleep_state(ACPI_STATE_S5);
 }
 
-#ifdef CONFIG_PM
-
 static int acpi_shutdown(struct sys_device *x)
 {
 	return acpi_sleep_prepare(ACPI_STATE_S5);
@@ -74,8 +68,6 @@ static struct sys_device device_acpi = {
 	.cls = &acpi_sysclass,
 };
 
-#endif
-
 static int acpi_poweroff_init(void)
 {
 	if (!acpi_disabled) {
@@ -85,19 +77,18 @@ static int acpi_poweroff_init(void)
 		status =
 		    acpi_get_sleep_type_data(ACPI_STATE_S5, &type_a, &type_b);
 		if (ACPI_SUCCESS(status)) {
-			pm_power_off = acpi_power_off;
-#ifdef CONFIG_PM
-			{
-				int error;
-				error = sysdev_class_register(&acpi_sysclass);
-				if (!error)
-					error = sysdev_register(&device_acpi);
-				return error;
-			}
-#endif
+			int error;
+			error = sysdev_class_register(&acpi_sysclass);
+			if (!error)
+				error = sysdev_register(&device_acpi);
+			if (!error)
+				pm_power_off = acpi_power_off;
+			return error;
 		}
 	}
 	return 0;
 }
 
 late_initcall(acpi_poweroff_init);
+
+#endif /* CONFIG_PM */

Re: [PATCH 0/23] reboot-fixes

[Pavel Machek]

Hi!

> The reboot code paths seems to be suffering from 15 years of people
> only looking at the code when it breaks.  The result is there
> are several code paths in which different callers expect different
> semantics from the same functions, and a fair amount of imperfect
> inline replication of code.
> 
> For a year or more every time I fix one bug in the bug fix reveals yet
> another bug.  In an attempt to end the cycle of bug fixes revealing
> yet more bugs I have generated a series of patches to clean up
> the semantics along the reboot path.
> 
> With the callers all agreeing on what to expect from the functions
> they call it should at least be possible to kill bugs without
> more showing up because of the bug fix.
> 
> My primary approach is to factor sys_reboot into several smaller
> functions and provide those functions for the general kernel
> consumers instead of the architecture dependent restart and
> halt hooks.
> 
> I don't expect this to noticeably fix any bugs along the
> main code paths but magic sysrq and several of the more obscure
> code paths should work much more reliably.

It looks good to me. Good ammount of cruft really accumulated there...

								Pavel
-- 
teflon -- maybe it is a trademark, but it should not be.

Re: [PATCH 16/23] swpsuspend: Have suspend to disk use factors of sys_reboot

[Andrew Morton]

ebiederm@xmission.com (Eric W. Biederman) wrote:
>
> 
> The suspend to disk code was a poor copy of the code in
> sys_reboot now that we have kernel_power_off, kernel_restart
> and kernel_halt use them instead of poorly duplicating them inline.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> ---
> 
>  kernel/power/disk.c |    9 +++------
>  1 files changed, 3 insertions(+), 6 deletions(-)
> 
> 78a2f83d732e327874fe73728d5667875dfeea46
> diff --git a/kernel/power/disk.c b/kernel/power/disk.c
> --- a/kernel/power/disk.c
> +++ b/kernel/power/disk.c
> @@ -59,16 +59,13 @@ static void power_down(suspend_disk_meth
>  		error = pm_ops->enter(PM_SUSPEND_DISK);
>  		break;
>  	case PM_DISK_SHUTDOWN:
> -		printk("Powering off system\n");
> -		device_shutdown();
> -		machine_power_off();
> +		kernel_power_off();
>  		break;
>  	case PM_DISK_REBOOT:
> -		device_shutdown();
> -		machine_restart(NULL);
> +		kernel_restart(NULL);
>  		break;
>  	}
> -	machine_halt();
> +	kernel_halt();
>  	/* Valid image is on the disk, if we continue we risk serious data corruption
>  	   after resume. */
>  	printk(KERN_CRIT "Please power me down manually\n");

This one conflicts in both implementation and intent with the below, from Pavel.  I'll
drop Pavel's patch.


From: Pavel Machek <pavel@ucw.cz>

Do not call device_shutdown with interrupts disabled.  It is wrong and
produces ugly warnings.

Signed-off-by: Pavel Machek <pavel@suse.cz>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 kernel/power/disk.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff -puN kernel/power/disk.c~call-device_shutdown-with-interrupts-enabled kernel/power/disk.c
--- devel/kernel/power/disk.c~call-device_shutdown-with-interrupts-enabled	2005-07-08 23:11:23.000000000 -0700
+++ devel-akpm/kernel/power/disk.c	2005-07-08 23:11:23.000000000 -0700
@@ -52,19 +52,21 @@ static void power_down(suspend_disk_meth
 	unsigned long flags;
 	int error = 0;
 
-	local_irq_save(flags);
 	switch(mode) {
 	case PM_DISK_PLATFORM:
- 		device_shutdown();
+		device_shutdown();
+		local_irq_save(flags);
 		error = pm_ops->enter(PM_SUSPEND_DISK);
 		break;
 	case PM_DISK_SHUTDOWN:
 		printk("Powering off system\n");
 		device_shutdown();
+		local_irq_save(flags);
 		machine_power_off();
 		break;
 	case PM_DISK_REBOOT:
 		device_shutdown();
+		local_irq_save(flags);
 		machine_restart(NULL);
 		break;
 	}
_

Re: [PATCH 16/23] swpsuspend: Have suspend to disk use factors of sys_reboot

[Pavel Machek]

Hi!

> > The suspend to disk code was a poor copy of the code in
> > sys_reboot now that we have kernel_power_off, kernel_restart
> > and kernel_halt use them instead of poorly duplicating them inline.
> > 
> > Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
..
> This one conflicts in both implementation and intent with the below, from Pavel.  I'll
> drop Pavel's patch.
> 
> 
> From: Pavel Machek <pavel@ucw.cz>
> 
> Do not call device_shutdown with interrupts disabled.  It is wrong and
> produces ugly warnings.

Okay with me.
								Pavel
-- 
teflon -- maybe it is a trademark, but it should not be.

Re: [PATCH 6/23] Don't export machine_restart, machine_halt, or machine_power_off.

[Marc Ballarin]

On Tue, 26 Jul 2005 11:36:01 -0600
ebiederm@xmission.com (Eric W. Biederman) wrote:

> 
> machine_restart, machine_halt and machine_power_off are machine
> specific hooks deep into the reboot logic, that modules
> have no business messing with. Usually code should be calling
> kernel_restart, kernel_halt, kernel_power_off, or
> emergency_restart. So don't export machine_restart,
> machine_halt, and machine_power_off so we can catch buggy users.

The first is reiser4 in fs/reiser4/vfs_ops.c, line 1338.
(Are filesystems supposed to restart the machine at all?!)

Patch not tested properly, since this seems to be in error handling code,
but compiles und runs fine.

--- linux-2.6.13-rc3-mm1/fs/reiser4/vfs_ops.c.orig	2005-07-27 01:41:41.326382750 +0200
+++ linux-2.6.13-rc3-mm1/fs/reiser4/vfs_ops.c	2005-07-27 01:42:56.783098500 +0200
@@ -1335,7 +1335,7 @@ reiser4_internal void reiser4_handle_err
 		sb->s_flags |= MS_RDONLY;
 		break;
 	case 2:
-		machine_restart(NULL);
+		kernel_restart(NULL);
 	}
 }

Re: [PATCH 6/23] Don't export machine_restart, machine_halt, or machine_power_off.

[Eric W. Biederman]

Marc Ballarin <Ballarin.Marc@gmx.de> writes:

> On Tue, 26 Jul 2005 11:36:01 -0600
> ebiederm@xmission.com (Eric W. Biederman) wrote:
>
>> 
>> machine_restart, machine_halt and machine_power_off are machine
>> specific hooks deep into the reboot logic, that modules
>> have no business messing with. Usually code should be calling
>> kernel_restart, kernel_halt, kernel_power_off, or
>> emergency_restart. So don't export machine_restart,
>> machine_halt, and machine_power_off so we can catch buggy users.
>
> The first is reiser4 in fs/reiser4/vfs_ops.c, line 1338.
> (Are filesystems supposed to restart the machine at all?!)

I suspect a call to panic would be more appropriate there.

I actually missed this one as I generated the patches against
Linus's latest tree.

Are we in process context where we can afford to do a clean shutdown
of the machine?  I would have expected an error handling path to
not be able to do better than emergency_restart. 

Regardless a panic sounds much more appropriate and will let the action
taken depend on the users policy.

Eric

Re: [PATCH 6/23] Don't export machine_restart, machine_halt, or machine_power_off.

[Andrew Morton]

ebiederm@xmission.com (Eric W. Biederman) wrote:
>
> Marc Ballarin <Ballarin.Marc@gmx.de> writes:
> 
> > On Tue, 26 Jul 2005 11:36:01 -0600
> > ebiederm@xmission.com (Eric W. Biederman) wrote:
> >
> >> 
> >> machine_restart, machine_halt and machine_power_off are machine
> >> specific hooks deep into the reboot logic, that modules
> >> have no business messing with. Usually code should be calling
> >> kernel_restart, kernel_halt, kernel_power_off, or
> >> emergency_restart. So don't export machine_restart,
> >> machine_halt, and machine_power_off so we can catch buggy users.
> >
> > The first is reiser4 in fs/reiser4/vfs_ops.c, line 1338.
> > (Are filesystems supposed to restart the machine at all?!)
> 
> I suspect a call to panic would be more appropriate there.
> 

That's all stuff which the reiser4 team are supposed to be removing, so
I'll add this patch to -mm for now just to keep things happy, thanks.

Re: [PATCH 6/23] Don't export machine_restart, machine_halt, or machine_power_off.

[Linus Torvalds]

On Tue, 26 Jul 2005, Eric W. Biederman wrote:
> 
> I suspect a call to panic would be more appropriate there.

Absolutely. Then the sysadmin can say whether they want reboot-on-panic 
behaviour or not.

A filesystem should definitely _not_ decide to reboot the machine. Ever.

		Linus

Re: [PATCH 0/23] reboot-fixes

[Andrew Morton]

My fairly ordinary x86 test box gets stuck during reboot on the
wait_for_completion() in ide_do_drive_cmd():


(gdb) thread 73
[Switching to thread 73 (Thread 2906)]#0  ide_do_drive_cmd (drive=0xc072dd10, rq=0x0, 
    action=ide_wait) at drivers/ide/ide-io.c:1671
1671                    rq->waiting = NULL;
(gdb) bt
#0  ide_do_drive_cmd (drive=0xc072dd10, rq=0x0, action=ide_wait) at drivers/ide/ide-io.c:1671
#1  0xc02e64c0 in ide_diag_taskfile (drive=0xc072dd10, args=0xcc0c1e20, data_size=0, buf=0x0)
    at drivers/ide/ide-taskfile.c:503
#2  0xc02e64e6 in ide_raw_taskfile (drive=0x0, args=0x0, buf=0x0) at drivers/ide/ide-taskfile.c:508
#3  0xc02eab6d in do_idedisk_flushcache (drive=0x0) at drivers/ide/ide-disk.c:831
#4  0xc02eb232 in ide_cacheflush_p (drive=0xc072dd10) at drivers/ide/ide-disk.c:1027
#5  0xc02eb2e4 in ide_device_shutdown (dev=0xc072ddf4) at drivers/ide/ide-disk.c:1083
#6  0xc02af75c in device_shutdown () at drivers/base/power/shutdown.c:45
#7  0xc0128bfe in kernel_restart (cmd=0x0) at kernel/sys.c:375
#8  0xc0128dea in sys_reboot (magic1=-18751827, magic2=672274793, cmd=19088743, arg=0x0)
    at kernel/sys.c:484
#9  0xc0102ba3 in sysenter_past_esp () at arch/i386/kernel/semaphore.c:177

Re: [PATCH 0/23] reboot-fixes

[Eric W. Biederman]

Andrew Morton <akpm@osdl.org> writes:

> My fairly ordinary x86 test box gets stuck during reboot on the
> wait_for_completion() in ide_do_drive_cmd():

Hmm. The only thing I can think of is someone started adding calls
to device_suspend() before device_shutdown().  Not understanding
where it was a good idea I made certain the calls were in there
consistently.  

Andrew can you remove the call to device_suspend from kernel_restart
and see if this still happens?

I would suspect interrupts of being disabled but it looks like
kgdb is working and I think that requires an interrupt to notice
new characters.

Eric

Re: [PATCH 0/23] reboot-fixes

[Eric W. Biederman]

ebiederm@xmission.com (Eric W. Biederman) writes:

> Andrew Morton <akpm@osdl.org> writes:
>
>> My fairly ordinary x86 test box gets stuck during reboot on the
>> wait_for_completion() in ide_do_drive_cmd():
>
> Hmm. The only thing I can think of is someone started adding calls
> to device_suspend() before device_shutdown().  Not understanding
> where it was a good idea I made certain the calls were in there
> consistently.  
>
> Andrew can you remove the call to device_suspend from kernel_restart
> and see if this still happens?
>
> I would suspect interrupts of being disabled but it looks like
> kgdb is working and I think that requires an interrupt to notice
> new characters.

Looking at it the device_suspend calls should be safe but
in case we need to follow it up the device_suspend calls in sys_reboot
were initially introduced in:


commit 620b03276488c3cf103caf1e326bd21f00d3df84
Author: Pavel Machek <pavel@ucw.cz>
Date:   Sat Jun 25 14:55:11 2005 -0700

    [PATCH] properly stop devices before poweroff

    Without this patch, Linux provokes emergency disk shutdowns and
    similar nastiness. It was in SuSE kernels for some time, IIRC.

    Signed-off-by: Pavel Machek <pavel@suse.cz>
    Signed-off-by: Andrew Morton <akpm@osdl.org>
    Signed-off-by: Linus Torvalds <torvalds@osdl.org>

Eric

Re: [PATCH 0/23] reboot-fixes

[Andrew Morton]

ebiederm@xmission.com (Eric W. Biederman) wrote:
>
> Andrew Morton <akpm@osdl.org> writes:
> 
>  > My fairly ordinary x86 test box gets stuck during reboot on the
>  > wait_for_completion() in ide_do_drive_cmd():
> 
>  Hmm. The only thing I can think of is someone started adding calls
>  to device_suspend() before device_shutdown().  Not understanding
>  where it was a good idea I made certain the calls were in there
>  consistently.  
> 
>  Andrew can you remove the call to device_suspend from kernel_restart
>  and see if this still happens?

yup, that fixes it.

--- devel/kernel/sys.c~a	2005-07-27 10:36:06.000000000 -0700
+++ devel-akpm/kernel/sys.c	2005-07-27 10:36:26.000000000 -0700
@@ -371,7 +371,6 @@ void kernel_restart(char *cmd)
 {
 	notifier_call_chain(&reboot_notifier_list, SYS_RESTART, cmd);
 	system_state = SYSTEM_RESTART;
-	device_suspend(PMSG_FREEZE);
 	device_shutdown();
 	if (!cmd) {
 		printk(KERN_EMERG "Restarting system.\n");
_


Presumably it unfixes Pavel's patch?


From: Pavel Machek <pavel@ucw.cz>

Without this patch, Linux provokes emergency disk shutdowns and
similar nastiness. It was in SuSE kernels for some time, IIRC.

Signed-off-by: Pavel Machek <pavel@suse.cz>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 include/linux/pm.h |   33 +++++++++++++++++++++------------
 kernel/sys.c       |    3 +++
 2 files changed, 24 insertions(+), 12 deletions(-)

diff -puN kernel/sys.c~properly-stop-devices-before-poweroff kernel/sys.c
--- 25/kernel/sys.c~properly-stop-devices-before-poweroff	2005-06-25 14:47:00.000000000 -0700
+++ 25-akpm/kernel/sys.c	2005-06-25 14:50:53.000000000 -0700
@@ -405,6 +405,7 @@ asmlinkage long sys_reboot(int magic1, i
 	case LINUX_REBOOT_CMD_HALT:
 		notifier_call_chain(&reboot_notifier_list, SYS_HALT, NULL);
 		system_state = SYSTEM_HALT;
+		device_suspend(PMSG_SUSPEND);
 		device_shutdown();
 		printk(KERN_EMERG "System halted.\n");
 		machine_halt();
@@ -415,6 +416,7 @@ asmlinkage long sys_reboot(int magic1, i
 	case LINUX_REBOOT_CMD_POWER_OFF:
 		notifier_call_chain(&reboot_notifier_list, SYS_POWER_OFF, NULL);
 		system_state = SYSTEM_POWER_OFF;
+		device_suspend(PMSG_SUSPEND);
 		device_shutdown();
 		printk(KERN_EMERG "Power down.\n");
 		machine_power_off();
@@ -431,6 +433,7 @@ asmlinkage long sys_reboot(int magic1, i
 
 		notifier_call_chain(&reboot_notifier_list, SYS_RESTART, buffer);
 		system_state = SYSTEM_RESTART;
+		device_suspend(PMSG_FREEZE);
 		device_shutdown();
 		printk(KERN_EMERG "Restarting system with command '%s'.\n", buffer);
 		machine_restart(buffer);
diff -puN include/linux/pm.h~properly-stop-devices-before-poweroff include/linux/pm.h
--- 25/include/linux/pm.h~properly-stop-devices-before-poweroff	2005-06-25 14:47:00.000000000 -0700
+++ 25-akpm/include/linux/pm.h	2005-06-25 14:47:00.000000000 -0700
@@ -103,7 +103,8 @@ extern int pm_active;
 /*
  * Register a device with power management
  */
-struct pm_dev __deprecated *pm_register(pm_dev_t type, unsigned long id, pm_callback callback);
+struct pm_dev __deprecated *
+pm_register(pm_dev_t type, unsigned long id, pm_callback callback);
 
 /*
  * Unregister a device with power management
@@ -190,17 +191,18 @@ typedef u32 __bitwise pm_message_t;
 /*
  * There are 4 important states driver can be in:
  * ON     -- driver is working
- * FREEZE -- stop operations and apply whatever policy is applicable to a suspended driver
- *           of that class, freeze queues for block like IDE does, drop packets for
- *           ethernet, etc... stop DMA engine too etc... so a consistent image can be
- *           saved; but do not power any hardware down.
- * SUSPEND - like FREEZE, but hardware is doing as much powersaving as possible. Roughly
- *           pci D3.
+ * FREEZE -- stop operations and apply whatever policy is applicable to a
+ *           suspended driver of that class, freeze queues for block like IDE
+ *           does, drop packets for ethernet, etc... stop DMA engine too etc...
+ *           so a consistent image can be saved; but do not power any hardware
+ *           down.
+ * SUSPEND - like FREEZE, but hardware is doing as much powersaving as
+ *           possible. Roughly pci D3.
  *
- * Unfortunately, current drivers only recognize numeric values 0 (ON) and 3 (SUSPEND).
- * We'll need to fix the drivers. So yes, putting 3 to all diferent defines is intentional,
- * and will go away as soon as drivers are fixed. Also note that typedef is neccessary,
- * we'll probably want to switch to
+ * Unfortunately, current drivers only recognize numeric values 0 (ON) and 3
+ * (SUSPEND).  We'll need to fix the drivers. So yes, putting 3 to all different
+ * defines is intentional, and will go away as soon as drivers are fixed.  Also
+ * note that typedef is neccessary, we'll probably want to switch to
  *   typedef struct pm_message_t { int event; int flags; } pm_message_t
  * or something similar soon.
  */
@@ -222,11 +224,18 @@ struct dev_pm_info {
 
 extern void device_pm_set_parent(struct device * dev, struct device * parent);
 
-extern int device_suspend(pm_message_t state);
 extern int device_power_down(pm_message_t state);
 extern void device_power_up(void);
 extern void device_resume(void);
 
+#ifdef CONFIG_PM
+extern int device_suspend(pm_message_t state);
+#else
+static inline int device_suspend(pm_message_t state)
+{
+	return 0;
+}
+#endif
 
 #endif /* __KERNEL__ */
 
_

Re: [PATCH 0/23] reboot-fixes

[Eric W. Biederman]

Andrew Morton <akpm@osdl.org> writes:

> ebiederm@xmission.com (Eric W. Biederman) wrote:
>>
>> Andrew Morton <akpm@osdl.org> writes:
>> 
>>  > My fairly ordinary x86 test box gets stuck during reboot on the
>>  > wait_for_completion() in ide_do_drive_cmd():
>> 
>>  Hmm. The only thing I can think of is someone started adding calls
>>  to device_suspend() before device_shutdown().  Not understanding
>>  where it was a good idea I made certain the calls were in there
>>  consistently.  
>> 
>>  Andrew can you remove the call to device_suspend from kernel_restart
>>  and see if this still happens?
>
> yup, that fixes it.
>
> --- devel/kernel/sys.c~a	2005-07-27 10:36:06.000000000 -0700
> +++ devel-akpm/kernel/sys.c	2005-07-27 10:36:26.000000000 -0700
> @@ -371,7 +371,6 @@ void kernel_restart(char *cmd)
>  {
>  	notifier_call_chain(&reboot_notifier_list, SYS_RESTART, cmd);
>  	system_state = SYSTEM_RESTART;
> -	device_suspend(PMSG_FREEZE);
>  	device_shutdown();
>  	if (!cmd) {
>  		printk(KERN_EMERG "Restarting system.\n");
> _
>
>
> Presumably it unfixes Pavel's patch?

Good question.  I'm not certain if Pavel intended to add
device_suspend(PMSG_FREEZE) to the reboot path.  It was
there in only one instance.  Pavel comments talk only about
the suspend path.

My gut feel is the device_suspend calls are the right direction
as it allows us to remove code from the drivers and possible
kill device_shutdown completely. 

But this close to 2.6.13 I'm not certain what the correct solution
is.  With this we have had issues with both ide and the e1000.
But those are among the few drivers that do anything in either
device_shutdown() or the reboot_notifier.

The e1000 has been fixed. Can we fix the ide driver?

Looking at it more closely the code is confusing because
FREEZE and SUSPEND are actually the same message, and in
addition to what shutdown does they place the device in
a low power state.  That worries me because last I checked
the e1000 driver could not bring itself out of the low power
state.  

Are the semantic differences to great to make this interesting?

Is this additional suspend what is causing some of the oddball
power off bug reports?

My gut feel is that device_suspend(PMSG_FREEZE) should be
removed from kernel_restart until is a different message
from PMSG_SUSPEND at which point it should be equivalent
to device_shutdown and we can remove that case.

We really need to get a consistent set of requirements
for the device driver authors so they have a chance of
catching up.

Eric

Re: [PATCH 0/23] reboot-fixes

[Eric W. Biederman]

ebiederm@xmission.com (Eric W. Biederman) writes:

> We really need to get a consistent set of requirements
> for the device driver authors so they have a chance of
> catching up.

I meant to say.  We really need to get a simple and
stable set of requirement for the device driver authors so they 
have a chance of catching up and implementing it correctly.

Eric

Re: [PATCH 0/23] reboot-fixes

[Andrew Morton]

ebiederm@xmission.com (Eric W. Biederman) wrote:
>
> Andrew Morton <akpm@osdl.org> writes:
> 
> > ebiederm@xmission.com (Eric W. Biederman) wrote:
> >>
> >> Andrew Morton <akpm@osdl.org> writes:
> >> 
> >>  > My fairly ordinary x86 test box gets stuck during reboot on the
> >>  > wait_for_completion() in ide_do_drive_cmd():
> >> 
> >>  Hmm. The only thing I can think of is someone started adding calls
> >>  to device_suspend() before device_shutdown().  Not understanding
> >>  where it was a good idea I made certain the calls were in there
> >>  consistently.  
> >> 
> >>  Andrew can you remove the call to device_suspend from kernel_restart
> >>  and see if this still happens?
> >
> > yup, that fixes it.
> >
> > --- devel/kernel/sys.c~a	2005-07-27 10:36:06.000000000 -0700
> > +++ devel-akpm/kernel/sys.c	2005-07-27 10:36:26.000000000 -0700
> > @@ -371,7 +371,6 @@ void kernel_restart(char *cmd)
> >  {
> >  	notifier_call_chain(&reboot_notifier_list, SYS_RESTART, cmd);
> >  	system_state = SYSTEM_RESTART;
> > -	device_suspend(PMSG_FREEZE);
> >  	device_shutdown();
> >  	if (!cmd) {
> >  		printk(KERN_EMERG "Restarting system.\n");
> > _
> >
> >
> > Presumably it unfixes Pavel's patch?
> 
> Good question.  I'm not certain if Pavel intended to add
> device_suspend(PMSG_FREEZE) to the reboot path.  It was
> there in only one instance.  Pavel comments talk only about
> the suspend path.
> 
> My gut feel is the device_suspend calls are the right direction
> as it allows us to remove code from the drivers and possible
> kill device_shutdown completely. 
> 
> But this close to 2.6.13 I'm not certain what the correct solution
> is.  With this we have had issues with both ide and the e1000.
> But those are among the few drivers that do anything in either
> device_shutdown() or the reboot_notifier.
>
> The e1000 has been fixed.

By "fixed" do you mean Tony Luck's patch which I added to rc3-mm2?  If so,
do you think that's needed for 2.6.13?  Getting patches into e100[0] is a
bit of an exercise :(

Re: [PATCH 0/23] reboot-fixes

[Eric W. Biederman]

Andrew Morton <akpm@osdl.org> writes:

> ebiederm@xmission.com (Eric W. Biederman) wrote:
>
> By "fixed" do you mean Tony Luck's patch which I added to rc3-mm2?  If so,
> do you think that's needed for 2.6.13?  Getting patches into e100[0] is a
> bit of an exercise :(

That is what I was referring to.

Eric

Re: [PATCH 0/23] reboot-fixes

[Pavel Machek]

Hi!

> >>  > My fairly ordinary x86 test box gets stuck during reboot on the
> >>  > wait_for_completion() in ide_do_drive_cmd():
> >> 
> >>  Hmm. The only thing I can think of is someone started adding calls
> >>  to device_suspend() before device_shutdown().  Not understanding
> >>  where it was a good idea I made certain the calls were in there
> >>  consistently.  
> >> 
> >>  Andrew can you remove the call to device_suspend from kernel_restart
> >>  and see if this still happens?
> >
> > yup, that fixes it.
> >
> > --- devel/kernel/sys.c~a	2005-07-27 10:36:06.000000000 -0700
> > +++ devel-akpm/kernel/sys.c	2005-07-27 10:36:26.000000000 -0700
> > @@ -371,7 +371,6 @@ void kernel_restart(char *cmd)
> >  {
> >  	notifier_call_chain(&reboot_notifier_list, SYS_RESTART, cmd);
> >  	system_state = SYSTEM_RESTART;
> > -	device_suspend(PMSG_FREEZE);
> >  	device_shutdown();
> >  	if (!cmd) {
> >  		printk(KERN_EMERG "Restarting system.\n");
> > _
> >
> >
> > Presumably it unfixes Pavel's patch?
> 
> Good question.  I'm not certain if Pavel intended to add
> device_suspend(PMSG_FREEZE) to the reboot path.  It was
> there in only one instance.  Pavel comments talk only about
> the suspend path.

Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path.

> My gut feel is the device_suspend calls are the right direction
> as it allows us to remove code from the drivers and possible
> kill device_shutdown completely. 
> 
> But this close to 2.6.13 I'm not certain what the correct solution
> is.  With this we have had issues with both ide and the e1000.
> But those are among the few drivers that do anything in either
> device_shutdown() or the reboot_notifier.
..
> Looking at it more closely the code is confusing because
> FREEZE and SUSPEND are actually the same message, and in
> addition to what shutdown does they place the device in

Not in -mm; I was finally able to fix that one.

> My gut feel is that device_suspend(PMSG_FREEZE) should be
> removed from kernel_restart until is a different message
> from PMSG_SUSPEND at which point it should be equivalent
> to device_shutdown and we can remove that case.

PMSG_FREEZE != PMSG_SUSPEND in current -mm, but I'm not sure if we can
push that to 2.6.13...
							Pavel
-- 
teflon -- maybe it is a trademark, but it should not be.

Re: [PATCH 0/23] reboot-fixes

[Andrew Morton]

Pavel Machek <pavel@ucw.cz> wrote:
>
>  > Good question.  I'm not certain if Pavel intended to add
>  > device_suspend(PMSG_FREEZE) to the reboot path.  It was
>  > there in only one instance.  Pavel comments talk only about
>  > the suspend path.
> 
>  Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path.

Why?

Re: [PATCH 0/23] reboot-fixes

[Linus Torvalds]

On Thu, 28 Jul 2005, Pavel Machek wrote:
> 
> Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path.

Considering how many device drivers that are likely broken, I disagree. 
Especially since Andrew seems to have trivially found a machine where it 
doesn't work.

		Linus

Re: [PATCH 0/23] reboot-fixes

[Pavel Machek]

Hi!

> > Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path.
> 
> Considering how many device drivers that are likely broken, I disagree. 
> Especially since Andrew seems to have trivially found a machine where it 
> doesn't work.

I'm not sure if we want to do that for 2.6.13, but long term, we
should just tell drivers to FREEZE instead of inventing reboot
notifier lists and similar uglynesses...

							Pavel
-- 
teflon -- maybe it is a trademark, but it should not be.

Re: [PATCH 0/23] reboot-fixes

[Pavel Machek]

Hi!

> >  > Good question.  I'm not certain if Pavel intended to add
> >  > device_suspend(PMSG_FREEZE) to the reboot path.  It was
> >  > there in only one instance.  Pavel comments talk only about
> >  > the suspend path.
> > 
> >  Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path.
> 
> Why?

Many bioses are broken; if you leave hardware active during reboot,
they'll hang during reboot. It is so common problem that I think that
only sane solution is make hardware quiet before reboot.

								Pavel
-- 
teflon -- maybe it is a trademark, but it should not be.

Re: [PATCH 0/23] reboot-fixes

[Eric W. Biederman]

Pavel Machek <pavel@ucw.cz> writes:

> Hi!
>
> Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path.
>
>> My gut feel is the device_suspend calls are the right direction
>> as it allows us to remove code from the drivers and possible
>> kill device_shutdown completely. 
>> 
>> But this close to 2.6.13 I'm not certain what the correct solution
>> is.  With this we have had issues with both ide and the e1000.
>> But those are among the few drivers that do anything in either
>> device_shutdown() or the reboot_notifier.
> ..
>> Looking at it more closely the code is confusing because
>> FREEZE and SUSPEND are actually the same message, and in
>> addition to what shutdown does they place the device in
>
> Not in -mm; I was finally able to fix that one.

Cool.

>> My gut feel is that device_suspend(PMSG_FREEZE) should be
>> removed from kernel_restart until is a different message
>> from PMSG_SUSPEND at which point it should be equivalent
>> to device_shutdown and we can remove that case.
>
> PMSG_FREEZE != PMSG_SUSPEND in current -mm, but I'm not sure if we can
> push that to 2.6.13...

Currently device_suspend(PMSG_FREEZE) in the reboot path breaks
the e1000 and the ide driver.  Which is common enough hardware it
effectively breaks reboots in 2.6.13 despite the fact that nearly
everything thing else works.

To make device_suspend(PMSG_FREEZE) solid in the reboot path I think
it would take pushing and stabilizing all of PMSG_FREEZE != PMSG_SUSPEND.
It will certainly take a bunch of digging to make certain reboots keep
working.  Since the number of drivers that implement either a reboot
notifier or a shutdown method is small the it is conceivable we could
track down all of the serious issues before 2.6.13.  However I'm
not ready to take point on the bug hunt.

So unless you are really ambitious I'd like to take
device_suspend(PMSG_FREEZE) out of the reboot path for
2.6.13, put in -mm where people can bang on it for a bit
and see that it is coming and delay the merge with the stable
branch until the bugs with common hardware have been sorted out.

Eric

Re: [PATCH 0/23] reboot-fixes

[Eric W. Biederman]

Pavel Machek <pavel@suse.cz> writes:

> Hi!
>
>> > Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path.
>> 
>> Considering how many device drivers that are likely broken, I disagree. 
>> Especially since Andrew seems to have trivially found a machine where it 
>> doesn't work.
>
> I'm not sure if we want to do that for 2.6.13, but long term, we
> should just tell drivers to FREEZE instead of inventing reboot
> notifier lists and similar uglynesses...

Then as part of the patch device_shutdown should disappear.  It
is silly to have two functions that want to achieve the same
thing.  

Right now the device driver model is ugly and over complicated in
that case and it needs to be simplified so driver writers have
a chance of getting it correct. 

device_suspend(PMSG_FREEZE) feels more reusable than device_shutdown
so long term it feels right.

But a simple question is why can't you simply use the shutdown
method instead of extending the suspend method in the drivers.

Eric

Re: [PATCH 1/23] Add missing device_suspsend(PMSG_FREEZE) calls.

[Eric W. Biederman]

Nigel Cunningham <ncunningham@cyclades.com> writes:

> Hi.
>
> Could you please send PMSG_* related patches to linux-pm at
> lists.osdl.org as well?

I'll try.  My goal was not to add or change not functionality but to
make what the kernel was already doing be consistent.

It turns out the device_suspend(PMSG_FREEZE) is a major pain
sitting in the reboot path and I will be submitting a patch to
remove it from the reboot path in 2.6.13 completely.

At the very least the ide driver breaks, and the e1000 driver
is affected.

And there is of course the puzzle of why there exists simultaneously
driver shutdown() and suspend(PMSG_FREEZE) methods as I believed they
are defined to do exactly the same thing.

Eric

Re: [linux-pm] Re: [PATCH 1/23] Add missing device_suspsend(PMSG_FREEZE) calls.

[david-b]

> And there is of course the puzzle of why there exists simultaneously
> driver shutdown() and suspend(PMSG_FREEZE) methods as I believed they
> are defined to do exactly the same thing.

No puzzle; that's not how they're defined.  Both of them cause the
device to quiesce that device's activity.  But:

 - shutdown() is a "dirty shutdown OK" heads-up for some level
   of restart/reboot; hardware will be completely re-initialized
   later, normally with hardware level resets and OS rebooting.

 - freeze() is a "clean shutdown only" that normally sees hardware
   state preserved, and is followed by suspend() or resume().

Or so I had understood.  That does suggest why having FREEZE in the
reboot path could be trouble:  you could be rebooting because that
hardware won't do a clean shutdown!

- Dave

Re: [linux-pm] Re: [PATCH 1/23] Add missing device_suspsend(PMSG_FREEZE) calls.

[Shaohua Li]

On Wed, 2005-07-27 at 19:12 -0600, Eric W. Biederman wrote:
> Nigel Cunningham <ncunningham@cyclades.com> writes:
> 
> > Hi.
> >
> > Could you please send PMSG_* related patches to linux-pm at
> > lists.osdl.org as well?
> 
> I'll try.  My goal was not to add or change not functionality but to
> make what the kernel was already doing be consistent.
> 
> It turns out the device_suspend(PMSG_FREEZE) is a major pain
> sitting in the reboot path and I will be submitting a patch to
> remove it from the reboot path in 2.6.13 completely.
> 
> At the very least the ide driver breaks, and the e1000 driver
> is affected.
> 
> And there is of course the puzzle of why there exists simultaneously
> driver shutdown() and suspend(PMSG_FREEZE) methods as I believed they
> are defined to do exactly the same thing.
I would expect more driver breakage and for the shutdown either. In
current stage, suspend(PMSG_FREEZE) might put devices into D3 state. How
can a shutdown() be done again?

Thanks,
Shaohua

Re: [PATCH 0/23] reboot-fixes

[Pavel Machek]

Hi!

> So unless you are really ambitious I'd like to take
> device_suspend(PMSG_FREEZE) out of the reboot path for
> 2.6.13, put in -mm where people can bang on it for a bit
> and see that it is coming and delay the merge with the stable
> branch until the bugs with common hardware have been sorted out.

Works for me. I may feel ambitious, but I don't feel like debugging
every reboot problem or every strange architecture for 2.6.13 :-).

							Pavel
-- 
teflon -- maybe it is a trademark, but it should not be.

Re: [PATCH 0/23] reboot-fixes

[Pavel Machek]

Hi!

> >> > Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path.
> >> 
> >> Considering how many device drivers that are likely broken, I disagree. 
> >> Especially since Andrew seems to have trivially found a machine where it 
> >> doesn't work.
> >
> > I'm not sure if we want to do that for 2.6.13, but long term, we
> > should just tell drivers to FREEZE instead of inventing reboot
> > notifier lists and similar uglynesses...
> 
> Then as part of the patch device_shutdown should disappear.  It
> is silly to have two functions that want to achieve the same
> thing.  

I always thought that device_shutdown is different phase -- the one
with interrupts disabled...
								Pavel
-- 
teflon -- maybe it is a trademark, but it should not be.

Re: [PATCH 0/23] reboot-fixes

[Eric W. Biederman]

Pavel Machek <pavel@suse.cz> writes:

> I always thought that device_shutdown is different phase -- the one
> with interrupts disabled...

Nope.  device_shutdown runs before interrupts are disabled.

Eric

Re: [PATCH 0/23] reboot-fixes

[Eric W. Biederman]

Pavel Machek <pavel@suse.cz> writes:


> I always thought that device_shutdown is different phase -- the one
> with interrupts disabled...

At the end of device_shutdown interrupts are disabled because we
shutdown the interrupt controllers.  

I don't think we have a phase where the interrupts are disabled,
except for the code that runs after device_shutdown, and the bootup
code that happens before interrupts are enabled.

Eric

Re: [PATCH 0/23] reboot-fixes

[Eric W. Biederman]

Pavel Machek <pavel@suse.cz> writes:

> Hi!
>
>> So unless you are really ambitious I'd like to take
>> device_suspend(PMSG_FREEZE) out of the reboot path for
>> 2.6.13, put in -mm where people can bang on it for a bit
>> and see that it is coming and delay the merge with the stable
>> branch until the bugs with common hardware have been sorted out.
>
> Works for me. I may feel ambitious, but I don't feel like debugging
> every reboot problem or every strange architecture for 2.6.13 :-).

Curses.  Foiled again!

I have failed to pass the buck.

Eric

Re: [PATCH 0/23] reboot-fixes

[Nigel Cunningham]

Hi.

On Thu, 2005-07-28 at 08:54, Pavel Machek wrote:
> Hi!
> 
> > >  > Good question.  I'm not certain if Pavel intended to add
> > >  > device_suspend(PMSG_FREEZE) to the reboot path.  It was
> > >  > there in only one instance.  Pavel comments talk only about
> > >  > the suspend path.
> > > 
> > >  Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path.
> > 
> > Why?
> 
> Many bioses are broken; if you leave hardware active during reboot,
> they'll hang during reboot. It is so common problem that I think that
> only sane solution is make hardware quiet before reboot.

Sorry for my slow reply.

If I remember correctly PMSG_FREEZE was intended solely for stopping
activity when suspend to disk implementations are about to do their
atomic copies. I thought that ide reacts to this message by putting a
hold on queues, but doesn't otherwise do anything to prepare a drive for
a restart. If that's true, using FREEZE here isn't going to stop drives
from doing their emergency shutdown actions. Don't we need PMSG_SUSPEND
instead?

Regards,

Nigel
-- 
Evolution.
Enumerate the requirements.
Consider the interdependencies.
Calculate the probabilities.

Re: [PATCH 0/23] reboot-fixes

[Pavel Machek]

Hi!

> > > >  > Good question.  I'm not certain if Pavel intended to add
> > > >  > device_suspend(PMSG_FREEZE) to the reboot path.  It was
> > > >  > there in only one instance.  Pavel comments talk only about
> > > >  > the suspend path.
> > > > 
> > > >  Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path.
> > > 
> > > Why?
> > 
> > Many bioses are broken; if you leave hardware active during reboot,
> > they'll hang during reboot. It is so common problem that I think that
> > only sane solution is make hardware quiet before reboot.
> 
> Sorry for my slow reply.
> 
> If I remember correctly PMSG_FREEZE was intended solely for stopping
> activity when suspend to disk implementations are about to do their

Well, I think that PMSG_FREEZE can be handy when we want to stop
activity for other reasons, too...

> atomic copies. I thought that ide reacts to this message by putting a
> hold on queues, but doesn't otherwise do anything to prepare a drive for
> a restart. If that's true, using FREEZE here isn't going to stop drives
> from doing their emergency shutdown actions. Don't we need PMSG_SUSPEND
> instead?

Spinning disk down is not neccessary for reboot. Users will be angry
if we do it before reboot...
								Pavel
-- 
if you have sharp zaurus hardware you don't need... you know my address

Re: [PATCH 0/23] reboot-fixes

[Nigel Cunningham]

Hi.

On Fri, 2005-08-05 at 07:45, Pavel Machek wrote:
> Hi!
> 
> > > > >  > Good question.  I'm not certain if Pavel intended to add
> > > > >  > device_suspend(PMSG_FREEZE) to the reboot path.  It was
> > > > >  > there in only one instance.  Pavel comments talk only about
> > > > >  > the suspend path.
> > > > > 
> > > > >  Yes, I think we should do device_suspend(PMSG_FREEZE) in reboot path.
> > > > 
> > > > Why?
> > > 
> > > Many bioses are broken; if you leave hardware active during reboot,
> > > they'll hang during reboot. It is so common problem that I think that
> > > only sane solution is make hardware quiet before reboot.
> > 
> > Sorry for my slow reply.
> > 
> > If I remember correctly PMSG_FREEZE was intended solely for stopping
> > activity when suspend to disk implementations are about to do their
> 
> Well, I think that PMSG_FREEZE can be handy when we want to stop
> activity for other reasons, too...
> 
> > atomic copies. I thought that ide reacts to this message by putting a
> > hold on queues, but doesn't otherwise do anything to prepare a drive for
> > a restart. If that's true, using FREEZE here isn't going to stop drives
> > from doing their emergency shutdown actions. Don't we need PMSG_SUSPEND
> > instead?
> 
> Spinning disk down is not neccessary for reboot. Users will be angry
> if we do it before reboot...

Yes, but I understood (perhaps wrongly) that we were discussing the
shutdown path. Nevertheless, for rebooting, you don't want to simply
freeze the queue - you want to flush the queue and tell the drive to
flush too. For freeze, you may well flush the queue, but you might not
necessarily force the drive to flush its queue too.

Regards,

Nigel
-- 
Evolution.
Enumerate the requirements.
Consider the interdependencies.
Calculate the probabilities.

FYI: device_suspend(...) in kernel_power_off().

[Eric W. Biederman]

Early in the 2.6.13 process my kexec related patches were introduced
into the reboot path, and under the rule you touch it you fix it
it I have been involved in tracking quite a few regressions
on the reboot path.

Recently with Benjamin Herrenschmidt's removal of
device_suppend(PMSG_SUPPEND) from kernel_power_off(),
it seems the last of the issues went away.  I asked
for confirmation that reintroducing the patch below 
would break systems and I received it.

The experimental evidence then is that calling 
device_suspend(PMSG_SUSPEND) in kernel_power_off
when performing an acpi_power_off is actively harmful.

There as been a fair amount of consensus that calling
device_suspend(...) in the reboot path was inappropriate now, because
the device suspend code was too immature.   With this latest
piece of evidence it seems to me that introducing device_suspend(...)
in kernel_power_off, kernel_halt, kernel_reboot, or kernel_kexec
can never be appropriate.  I am including as many people as
I can on this so we in our collective wisdom don't forget this
lesson.

What lead us to this situation was a real problem, and a desire
to fix it.  Currently linux has a proliferation of interfaces
to place devices in different states.  The reboot notifiers,
device_suspend(...), device_shutdown+system_state, remove_one,
module_exit, and probably a few I'm not aware of.  Each interface
introduced by a different author wanting to solve a different problem.
Just writing this list of interfaces is confusing.  The implementation
task for device driver writers appears even harder as simultaneously
implementing all of these interfaces correctly seems not to happen.

The lesson: fixing this problem is heavy lifting, and that
device_suspend() in the reboot path is not the answer.

Eric


This is the patch that subtly and mysterously broke things.
> diff --git a/kernel/sys.c b/kernel/sys.c
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -404,6 +404,7 @@ void kernel_halt(void)
>  {
>  	notifier_call_chain(&reboot_notifier_list, SYS_HALT, NULL);
>  	system_state = SYSTEM_HALT;
> +	device_suspend(PMSG_SUSPEND);
>  	device_shutdown();
>  	printk(KERN_EMERG "System halted.\n");
>  	machine_halt();
> @@ -414,6 +415,7 @@ void kernel_power_off(void)
>  {
>  	notifier_call_chain(&reboot_notifier_list, SYS_POWER_OFF, NULL);
>  	system_state = SYSTEM_POWER_OFF;
> +	device_suspend(PMSG_SUSPEND);
>  	device_shutdown();
>  	printk(KERN_EMERG "Power down.\n");
>  	machine_power_off();
> 
> 

Re: FYI: device_suspend(...) in kernel_power_off().

[Pavel Machek]

Hi!

> There as been a fair amount of consensus that calling
> device_suspend(...) in the reboot path was inappropriate now, because
> the device suspend code was too immature.   With this latest
> piece of evidence it seems to me that introducing device_suspend(...)
> in kernel_power_off, kernel_halt, kernel_reboot, or kernel_kexec
> can never be appropriate.

Code is not ready now => it can never be fixed? Thats quite a strange
conclusion to make.
				Pavel
-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         

Re: FYI: device_suspend(...) in kernel_power_off().

[Eric W. Biederman]

Pavel Machek <pavel@suse.cz> writes:

> Hi!
>
>> There as been a fair amount of consensus that calling
>> device_suspend(...) in the reboot path was inappropriate now, because
>> the device suspend code was too immature.   With this latest
>> piece of evidence it seems to me that introducing device_suspend(...)
>> in kernel_power_off, kernel_halt, kernel_reboot, or kernel_kexec
>> can never be appropriate.
>
> Code is not ready now => it can never be fixed? Thats quite a strange
> conclusion to make.

It seems there is an fundamental incompatibility with ACPI power off.
As best as I can tell the normal case of device_suspend(PMSG_SUSPEND)
works reasonably well in 2.6.x.

>From what I can tell there are some fairly fundamental semantic
differences, on that code path.  The most peculiar problem I tracked
is someone had a machine that would go into power off state and then
wake right back up because of the device_suspend(PMSG_SUSPEND) change.
If acpi power off doesn't expect the hardware to be suspended I don't
see how you can make device_suspend(PMSG_SUSPEND) a default in 2.6.x.

I won't call it impossible to resolve the problems, but the people
doing it must be extremely sensitive to the subtle semantic
differences that exist and the burden of proof for showing things work
better need to be extremely high.  And the developer who reintroduces
it gets to own all of the reboot/halt/power off problems in the stable
tree when it gets merged.

Pavel the fact that you did not articulate a possibility that your
change could have caused most of the problems that were seen with
it is what scares me the most.  The fairly trivial and obvious
problems were not addressed when the patch was merged much less the
subtle ones.  Your initial patch did not even touch all of the code
paths necessary to achieve what it was trying to do.

So yes without a darn good argument as to why it should work.  I will
go with the experimental evidence that it fails miserably and
trivially because of semantic incompatibility and can therefore
never be fixed.

Eric

Re: FYI: device_suspend(...) in kernel_power_off().

[Pavel Machek]

Hi!

> >> There as been a fair amount of consensus that calling
> >> device_suspend(...) in the reboot path was inappropriate now, because
> >> the device suspend code was too immature.   With this latest
> >> piece of evidence it seems to me that introducing device_suspend(...)
> >> in kernel_power_off, kernel_halt, kernel_reboot, or kernel_kexec
> >> can never be appropriate.
> >
> > Code is not ready now => it can never be fixed? Thats quite a strange
> > conclusion to make.
> 
> It seems there is an fundamental incompatibility with ACPI power off.
> As best as I can tell the normal case of device_suspend(PMSG_SUSPEND)
> works reasonably well in 2.6.x.

Powerdown is going to have the same problems as the powerdown at the
end of suspend-to-disk. Can you ask people reporting broken shutdown
to try suspend-to-disk?

> >From what I can tell there are some fairly fundamental semantic
> differences, on that code path.  The most peculiar problem I tracked
> is someone had a machine that would go into power off state and then
> wake right back up because of the device_suspend(PMSG_SUSPEND)
> change.

So something is wrong with ACPI wakeup GPEs. It would hurt in
suspend-to-disk case, too.

> I won't call it impossible to resolve the problems, but the people

Good.

> So yes without a darn good argument as to why it should work.  I will
> go with the experimental evidence that it fails miserably and
> trivially because of semantic incompatibility and can therefore
> never be fixed.

I do not think any "semantic" issues exist. We need to pass detailed
info down to the drivers that care, and we need to fix all the bugs in
the drivers. That should be pretty much it.
								Pavel
-- 
if you have sharp zaurus hardware you don't need... you know my address

Re: FYI: device_suspend(...) in kernel_power_off().

[Eric W. Biederman]

Pavel Machek <pavel@ucw.cz> writes:

> Hi!
>
>> >> There as been a fair amount of consensus that calling
>> >> device_suspend(...) in the reboot path was inappropriate now, because
>> >> the device suspend code was too immature.   With this latest
>> >> piece of evidence it seems to me that introducing device_suspend(...)
>> >> in kernel_power_off, kernel_halt, kernel_reboot, or kernel_kexec
>> >> can never be appropriate.
>> >
>> > Code is not ready now => it can never be fixed? Thats quite a strange
>> > conclusion to make.
>> 
>> It seems there is an fundamental incompatibility with ACPI power off.
>> As best as I can tell the normal case of device_suspend(PMSG_SUSPEND)
>> works reasonably well in 2.6.x.
>
> Powerdown is going to have the same problems as the powerdown at the
> end of suspend-to-disk. Can you ask people reporting broken shutdown
> to try suspend-to-disk?

Everyone I know of who is affected has been copied on this thread.
However your request is just nonsense.  There is a device_resume in
the code before we get to the device_shutdown so there should be no
effect at all.  Are we looking at the same kernel?

>> >From what I can tell there are some fairly fundamental semantic
>> differences, on that code path.  The most peculiar problem I tracked
>> is someone had a machine that would go into power off state and then
>> wake right back up because of the device_suspend(PMSG_SUSPEND)
>> change.
>
> So something is wrong with ACPI wakeup GPEs. It would hurt in
> suspend-to-disk case, too.

Something was wrong.  I can't possibly see how the suspend-to-disk
case would be affected.

>> I won't call it impossible to resolve the problems, but the people
>
> Good.

Nope.  Now that I have read the code I would just call it nonsense.

>> So yes without a darn good argument as to why it should work.  I will
>> go with the experimental evidence that it fails miserably and
>> trivially because of semantic incompatibility and can therefore
>> never be fixed.
>
> I do not think any "semantic" issues exist. We need to pass detailed
> info down to the drivers that care, and we need to fix all the bugs in
> the drivers. That should be pretty much it.

Given that acpi and other platform firmware is involved there are
pieces we cannot fix.  We either match the spec or we are incorrect.

I haven't a clue how suspend/resume is expected to interact with
things in suspend to disk scenario.  Reading through the code
the power message is PMSG_FREEZE not PMSG_SUSPEND (as you
implemented).  All of the hardware is actually resumed before
we device_shutdown() is called.

I want to see the correlation between device_suspend(PMSG_FREEZE) and
the code in device_shutdown(), but I don't see it.
device_suspend(...) is all about allowing the state of a device to be
preserved.  device_shutdown() is really about stopping it.  These are
really quite different operations. 

With the pm_suspend_disk calling kernel_power_off it appears that we
currently have complete code reuse of the relevant code on that path.

Currently I see no true redundancy between the two cases at all.
The methods do different things for different purposes.  Which is
about the largest semantic difference I can think of.  The fact
that the methods at first glance look like they do the same
thing is probably the real surprise.

Calling device_suspend(...) from kernel_power_off, kernel_halt,
kernel_kexec, or kernel_restart seems pointless, useless and silly.

Eric

Re: FYI: device_suspend(...) in kernel_power_off().

[Nigel Cunningham]

Hi.

On Wed, 2005-08-10 at 03:25, Eric W. Biederman wrote:
> Pavel Machek <pavel@ucw.cz> writes:
> 
> > Hi!
> >
> >> >> There as been a fair amount of consensus that calling
> >> >> device_suspend(...) in the reboot path was inappropriate now, because
> >> >> the device suspend code was too immature.   With this latest
> >> >> piece of evidence it seems to me that introducing device_suspend(...)
> >> >> in kernel_power_off, kernel_halt, kernel_reboot, or kernel_kexec
> >> >> can never be appropriate.
> >> >
> >> > Code is not ready now => it can never be fixed? Thats quite a strange
> >> > conclusion to make.
> >> 
> >> It seems there is an fundamental incompatibility with ACPI power off.
> >> As best as I can tell the normal case of device_suspend(PMSG_SUSPEND)
> >> works reasonably well in 2.6.x.
> >
> > Powerdown is going to have the same problems as the powerdown at the
> > end of suspend-to-disk. Can you ask people reporting broken shutdown
> > to try suspend-to-disk?
> 
> Everyone I know of who is affected has been copied on this thread.
> However your request is just nonsense.  There is a device_resume in
> the code before we get to the device_shutdown so there should be no
> effect at all.  Are we looking at the same kernel?

My poweroff after suspend-to-disk was broken during 2.6.13-rcs, and came
right in rc6.

> >> >From what I can tell there are some fairly fundamental semantic
> >> differences, on that code path.  The most peculiar problem I tracked
> >> is someone had a machine that would go into power off state and then
> >> wake right back up because of the device_suspend(PMSG_SUSPEND)
> >> change.
> >
> > So something is wrong with ACPI wakeup GPEs. It would hurt in
> > suspend-to-disk case, too.
> 
> Something was wrong.  I can't possibly see how the suspend-to-disk
> case would be affected.
> 
> >> I won't call it impossible to resolve the problems, but the people
> >
> > Good.
> 
> Nope.  Now that I have read the code I would just call it nonsense.
> 
> >> So yes without a darn good argument as to why it should work.  I will
> >> go with the experimental evidence that it fails miserably and
> >> trivially because of semantic incompatibility and can therefore
> >> never be fixed.
> >
> > I do not think any "semantic" issues exist. We need to pass detailed
> > info down to the drivers that care, and we need to fix all the bugs in
> > the drivers. That should be pretty much it.
> 
> Given that acpi and other platform firmware is involved there are
> pieces we cannot fix.  We either match the spec or we are incorrect.
> 
> I haven't a clue how suspend/resume is expected to interact with
> things in suspend to disk scenario.  Reading through the code
> the power message is PMSG_FREEZE not PMSG_SUSPEND (as you
> implemented).  All of the hardware is actually resumed before
> we device_shutdown() is called.
> 
> I want to see the correlation between device_suspend(PMSG_FREEZE) and
> the code in device_shutdown(), but I don't see it.
> device_suspend(...) is all about allowing the state of a device to be
> preserved.  device_shutdown() is really about stopping it.  These are
> really quite different operations. 

Agreed here.

> With the pm_suspend_disk calling kernel_power_off it appears that we
> currently have complete code reuse of the relevant code on that path.
> 
> Currently I see no true redundancy between the two cases at all.
> The methods do different things for different purposes.  Which is
> about the largest semantic difference I can think of.  The fact
> that the methods at first glance look like they do the same
> thing is probably the real surprise.

If the suspend to disk code called kernel_power_off, it should be
exactly what it sounds like. We've already written the image and we now
went to simply power down the machine. Just as with a 'normal'
powerdown, we should do everything necessary to ensure all data
submitted to hard drives is really flushed and that emergency head
parking isn't done, and then power down (or reboot). This should, so far
as I can see, be exactly the same in both cases.

Regards,

Nigel

> Calling device_suspend(...) from kernel_power_off, kernel_halt,
> kernel_kexec, or kernel_restart seems pointless, useless and silly.
> 
> Eric
-- 
Evolution.
Enumerate the requirements.
Consider the interdependencies.
Calculate the probabilities.

Re: FYI: device_suspend(...) in kernel_power_off().

[Pavel Machek]

Hi!

> >> > Code is not ready now => it can never be fixed? Thats quite a strange
> >> > conclusion to make.
> >> 
> >> It seems there is an fundamental incompatibility with ACPI power off.
> >> As best as I can tell the normal case of device_suspend(PMSG_SUSPEND)
> >> works reasonably well in 2.6.x.
> >
> > Powerdown is going to have the same problems as the powerdown at the
> > end of suspend-to-disk. Can you ask people reporting broken shutdown
> > to try suspend-to-disk?
> 
> Everyone I know of who is affected has been copied on this thread.
> However your request is just nonsense.  There is a device_resume in
> the code before we get to the device_shutdown so there should be no
> effect at all.  Are we looking at the same kernel?

Sorry, my fault. kernel/power/disk.c:power_down(): it calls
device_shutdown even in PM_DISK_PLATFORM case. I thought it would do
device_suspend() to enable drivers doing something more clever.

So only missing piece of puzzle seems to be making sure disks are
properly spinned down in device_shutdown... and even that seems to be
there, not sure why it was broken before.
								Pavel
-- 
if you have sharp zaurus hardware you don't need... you know my address