lkml.org 
[lkml]   [2008]   [Jun]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] X86: reboot-notify additions

* Cliff Wickman <cpw@sgi.com> wrote:

> From: Cliff Wickman <cpw@sgi.com>
>
> X86 reboot-notify additions.
>
> This patch adds scans of the "reboot_notifier_list" callback chain in
> a three other places where the kernel is being stopped and/or restarted.
>
> Adds calls to blocking_notifier_call_chain() in:
> crash_kexec(), emergency_restart(), sys_kexec_load()
>
> In the crash_kexec() and emergency_restart() cases it is indicated to the
> called-back function that the system is not in a sane state, so that
> it can avoid taking a lock or some such potentially blocking action.
>
> These callbacks are important to a partition system. The stopped kernel needs
> to inform other partitions of their need to disconnect (stop sharing memory).
>
> Diffed against 2.6.26-rc6
>
> Signed-off-by: Cliff Wickman <cpw@sgi.com>
> ---
> include/linux/notifier.h | 4 ++++
> kernel/kexec.c | 5 +++++
> kernel/sys.c | 1 +
> 3 files changed, 10 insertions(+)
>
> Index: linux/include/linux/notifier.h
> ===================================================================
> --- linux.orig/include/linux/notifier.h
> +++ linux/include/linux/notifier.h
> @@ -202,6 +202,10 @@ static inline int notifier_to_errno(int
> #define SYS_RESTART SYS_DOWN
> #define SYS_HALT 0x0002 /* Notify of system halt */
> #define SYS_POWER_OFF 0x0003 /* Notify of system power off */
> +#define SYS_INSANE 0x0004 /* Notify of system error/panic/oops */
> +/* For the SYS_INSANE case, no locks should be taken by the called-back
> + * function. The kernel is ready for an immediate reboot.
> + */
>
> #define NETLINK_URELEASE 0x0001 /* Unicast netlink socket released */
>
> Index: linux/kernel/kexec.c
> ===================================================================
> --- linux.orig/kernel/kexec.c
> +++ linux/kernel/kexec.c
> @@ -1001,6 +1001,9 @@ asmlinkage long sys_kexec_load(unsigned
> if (result)
> goto out;
> }
> +
> + blocking_notifier_call_chain(&reboot_notifier_list, SYS_RESTART, NULL);
> +
> /* Install the new kernel, and Uninstall the old */
> image = xchg(dest_image, image);
>
> @@ -1068,6 +1071,8 @@ void crash_kexec(struct pt_regs *regs)
> if (!locked) {
> if (kexec_crash_image) {
> struct pt_regs fixed_regs;
> + blocking_notifier_call_chain(&reboot_notifier_list,
> + SYS_INSANE, NULL);
> crash_setup_regs(&fixed_regs, regs);
> crash_save_vmcoreinfo();
> machine_crash_shutdown(&fixed_regs);
> Index: linux/kernel/sys.c
> ===================================================================
> --- linux.orig/kernel/sys.c
> +++ linux/kernel/sys.c
> @@ -270,6 +270,7 @@ out_unlock:
> */
> void emergency_restart(void)
> {
> + blocking_notifier_call_chain(&reboot_notifier_list, SYS_INSANE, NULL);
> machine_emergency_restart();
> }
> EXPORT_SYMBOL_GPL(emergency_restart);

i dont think this is a good idea. reboot_notifier_list is a blocking
notifier, i.e. it comes with a notifier->rwsem read-write mutex that is
taken when blocking_notifier_call_chain() is executed.

i.e. this patch puts a sleeping mutex operation (a down_read()) into a
highly critical code path of the kernel. This will decrease the
reliability of the kernel.

what exactly are you trying to achieve?

Ingo


\
 
 \ /
  Last update: 2008-06-19 13:05    [from the cache]
©2003-2011 Jasper Spaans