Re: [PATCH 2/2] kdump shutdown hook support
From: Olof Johansson <hidden>
Date: 2007-12-12 06:56:41
Hi, A couple of comments below. -Olof On Wed, Dec 12, 2007 at 04:45:12PM +1100, Michael Neuling wrote:
quoted hunk ↗ jump to hunk
Index: linux-2.6-ozlabs/arch/powerpc/kernel/crash.c ===================================================================--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/crash.c +++ linux-2.6-ozlabs/arch/powerpc/kernel/crash.c@@ -32,6 +32,8 @@ #include <asm/lmb.h> #include <asm/firmware.h> #include <asm/smp.h> +#include <asm/system.h> +#include <asm/setjmp.h> #ifdef DEBUG #include <asm/udbg.h>@@ -45,6 +47,11 @@ int crashing_cpu = -1; static cpumask_t cpus_in_crash = CPU_MASK_NONE; cpumask_t cpus_in_sr = CPU_MASK_NONE; +#define CRASH_SHUTDOWN_HANDLES_NUM 1 +/* NULL terminated list of shutdown handles */ +static crash_shutdown_t crash_shutdown_handles[CRASH_SHUTDOWN_HANDLES_NUM+1]; +static DEFINE_SPINLOCK(crash_handles_lock);
Not 'handlers'?
quoted hunk ↗ jump to hunk
+ #ifdef CONFIG_SMP static atomic_t enter_on_soft_reset = ATOMIC_INIT(0);@@ -285,9 +292,69 @@ static inline void crash_kexec_stop_spus } #endif /* CONFIG_SPU_BASE */ +/* + * Register a function to be called on shutdown. Only use this if you + * can't reset your device in the second kernel. + */ +int crash_shutdown_register(crash_shutdown_t handler) +{ + unsigned int i; + + spin_lock(&crash_handles_lock); + for(i = 0 ; i <= CRASH_SHUTDOWN_HANDLES_NUM; i++)
Missing space after for. There's a handful more of these through the patch.
quoted hunk ↗ jump to hunk
+ if (!crash_shutdown_handles[i]) + break; + + if (i == CRASH_SHUTDOWN_HANDLES_NUM){ + printk(KERN_ERR "Crash shutdown handles full, " + "not registered.\n"); + spin_unlock(&crash_handles_lock); + return 1; + } + + /* Insert handle at end */ + crash_shutdown_handles[i] = handler; + spin_unlock(&crash_handles_lock); + return 0; +} +EXPORT_SYMBOL(crash_shutdown_register); + +int crash_shutdown_unregister(crash_shutdown_t handler) +{ + unsigned int i; + + spin_lock(&crash_handles_lock); + for(i = 0 ; i <= CRASH_SHUTDOWN_HANDLES_NUM; i++) + if (crash_shutdown_handles[i] == handler) + break; + + if (i == CRASH_SHUTDOWN_HANDLES_NUM){ + printk(KERN_ERR "Crash shutdown handle not found\n"); + spin_unlock(&crash_handles_lock); + return 1; + } + + /* Shift handles down */ + while(crash_shutdown_handles[i]) { + crash_shutdown_handles[i] = crash_shutdown_handles[i+1]; + i++; + } + spin_unlock(&crash_handles_lock); + return 0; +} +EXPORT_SYMBOL(crash_shutdown_unregister); + +static long crash_shutdown_buf[SETJMP_BUF_LEN]; + +static int handle_fault(struct pt_regs *regs) +{ + longjmp(crash_shutdown_buf, 1); + return 0; +} + void default_machine_crash_shutdown(struct pt_regs *regs) { - unsigned int irq; + unsigned int i; /* * This function is only called after the system@@ -301,14 +368,27 @@ void default_machine_crash_shutdown(stru */ hard_irq_disable(); - for_each_irq(irq) { - struct irq_desc *desc = irq_desc + irq; + for_each_irq(i) { + struct irq_desc *desc = irq_desc + i; if (desc->status & IRQ_INPROGRESS) - desc->chip->eoi(irq); + desc->chip->eoi(i); if (!(desc->status & IRQ_DISABLED)) - desc->chip->disable(irq); + desc->chip->disable(i); + } + + /* Call registered shutdown routines */ + __debugger_fault_handler = handle_fault; + i = 0; + while(crash_shutdown_handles[i]){
This could do nicely as a for loop instead:
for (i = 0; crash_shutdown_handles[i]; i++) {
+ if (setjmp(crash_shutdown_buf) == 0) {
+ asm volatile("sync; isync");
+ crash_shutdown_handles[i]();
+ asm volatile("sync; isync");
+ __delay(200);This looks a bit random. Why the handcoded barriers, and why the delay? At least comment why the delay is needed (and why just 200 is sufficient). I don't see a need for the barriers at all here?
quoted hunk ↗ jump to hunk
+ } + i++; } /* Index: linux-2.6-ozlabs/include/asm-powerpc/kexec.h ===================================================================--- linux-2.6-ozlabs.orig/include/asm-powerpc/kexec.h +++ linux-2.6-ozlabs/include/asm-powerpc/kexec.h@@ -123,6 +123,9 @@ struct pt_regs; extern void default_machine_kexec(struct kimage *image); extern int default_machine_kexec_prepare(struct kimage *image); extern void default_machine_crash_shutdown(struct pt_regs *regs); +typedef void (*crash_shutdown_t)(void); +extern int crash_shutdown_register(crash_shutdown_t handler); +extern int crash_shutdown_unregister(crash_shutdown_t handler); extern void machine_kexec_simple(struct kimage *image); extern void crash_kexec_secondary(struct pt_regs *regs);_______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev