lkml.org 
[lkml]   [2011]   [Oct]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] kdump: Fix crash_kexec - smp_send_stop race in panic
    On Wed, 26 Oct 2011 16:34:09 +0200
    Michael Holzheu <holzheu@linux.vnet.ibm.com> wrote:

    > Hello Andrew,
    >
    > After the discussion with Eric and Vivek the following patch
    > seems to be a good solution to me. Could you accept this patch?
    >
    > When two CPUs call panic at the same time there is a
    > possible race condition that can stop kdump. The first
    > CPU calls crash_kexec() and the second CPU calls
    > smp_send_stop() in panic() before crash_kexec() finished
    > on the first CPU. So the second CPU stops the first CPU
    > and therefore kdump fails:
    >
    > 1st CPU:
    > panic()->crash_kexec()->mutex_trylock(&kexec_mutex)-> do kdump
    >
    > 2nd CPU:
    > panic()->crash_kexec()->kexec_mutex already held by 1st CPU
    > ->smp_send_stop()-> stop 1st CPU (stop kdump)
    >
    > This patch fixes the problem by introducing a spinlock in
    > panic that allows only one CPU to process crash_kexec() and
    > the subsequent panic code.
    >
    > Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
    > ---
    > kernel/panic.c | 8 ++++++++
    > 1 file changed, 8 insertions(+)
    >
    > --- a/kernel/panic.c
    > +++ b/kernel/panic.c
    > @@ -59,6 +59,7 @@ EXPORT_SYMBOL(panic_blink);
    > */
    > NORET_TYPE void panic(const char * fmt, ...)
    > {
    > + static DEFINE_SPINLOCK(panic_lock);
    > static char buf[1024];
    > va_list args;
    > long i, i_next = 0;
    > @@ -82,6 +83,13 @@ NORET_TYPE void panic(const char * fmt,
    > #endif
    >
    > /*
    > + * Only one CPU is allowed to execute the panic code from here. For
    > + * multiple parallel invocations of panic all other CPUs will wait on
    > + * the panic_lock. They are stopped afterwards by smp_send_stop().
    > + */
    > + spin_lock(&panic_lock);
    > +

    hm. Boy. That'll stop 'em OK!

    Should this be done earlier in the function? As it stands we'll have
    multiple CPUs scribbling on buf[] at the same time and all trying to
    print the same thing at the same time, dumping their stacks, etc.
    Perhaps it would be better to single-thread all that stuff.

    Also... this patch affects all CPU architectures, all configs, etc.
    So we're expecting that every architecture's smp_send_stop() is able to
    stop a CPU which is spinning in spin_lock(), possibly with local
    interrupts disabled. Will this work?


    \
     
     \ /
      Last update: 2011-10-29 01:13    [W:0.024 / U:0.280 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site