lkml.org 
[lkml]   [2011]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [debug patch] printk: Add a printk killswitch to robustify NMI watchdog messages
    From
    Date
    On Mon, 2011-06-06 at 17:09 +0200, Peter Zijlstra wrote:
    > On Mon, 2011-06-06 at 16:58 +0200, Ingo Molnar wrote:
    > > * Peter Zijlstra <peterz@infradead.org> wrote:
    > >
    > > > On Sun, 2011-06-05 at 17:13 +0200, Ingo Molnar wrote:
    > > >
    > > > > Now, this patch alone just removes a debugging check - but i'm not
    > > > > sure the debugging check is correct - we take the pi_lock in a raw
    > > > > way - which means it's not lockdep covered.
    > > >
    > > > Ever since tglx did s/raw_/arch_/g raw_ is covered by lockdep.
    > >
    > > It's not lockdep covered due to the lockdep_off(), or am i missing
    > > something?
    >
    > Your initial stmt was about the raw_ part, raw_ locks are tracked by
    > lockdep ever since tglx renamed them to arch_ and introduced new raw_
    > primitives.
    >
    > But yeah, the lockdep_off() stuff also disables all tracking, on top of
    > that it also makes lock_is_held() return an unconditional false (even if
    > the lock was acquired before lockdep_off and thus registered).
    >
    > My patch that fixes lock_is_held() should avoid false
    > lockdep_assert_held() explosions and this this printk() while rq->lock
    > problem.
    >
    > Removing lockdep_off() usage from printk() would also be nice, but Mike
    > triggered logbuf_lock <-> rq->lock inversion with that due to the
    > up(&console_sem) wakeup muck.
    >
    > Ideally we'd pull the up() out from under logbuf_lock, am looking at
    > that.

    something like so,.. but then there's a comment about console_sem and
    logbuf_lock interlocking in interating ways, but it fails to mention how
    and why. But I think it should maybe work..

    Needs more staring at, preferably by someone who actually understands
    that horrid mess :/ Also, this all still doesn't make printk() work
    reliably while holding rq->lock.

    ---
    Index: linux-2.6/kernel/printk.c
    ===================================================================
    --- linux-2.6.orig/kernel/printk.c
    +++ linux-2.6/kernel/printk.c
    @@ -686,6 +686,7 @@ static void zap_locks(void)

    oops_timestamp = jiffies;

    + debug_locks_off();
    /* If a crash is occurring, make sure we can't deadlock */
    spin_lock_init(&logbuf_lock);
    /* And make sure that we print immediately */
    @@ -782,7 +783,7 @@ static inline int can_use_console(unsign
    static int console_trylock_for_printk(unsigned int cpu)
    __releases(&logbuf_lock)
    {
    - int retval = 0;
    + int retval = 0, wake = 0;

    if (console_trylock()) {
    retval = 1;
    @@ -795,12 +796,14 @@ static int console_trylock_for_printk(un
    */
    if (!can_use_console(cpu)) {
    console_locked = 0;
    - up(&console_sem);
    + wake = 1;
    retval = 0;
    }
    }
    printk_cpu = UINT_MAX;
    spin_unlock(&logbuf_lock);
    + if (wake)
    + up(&console_sem);
    return retval;
    }
    static const char recursion_bug_msg [] =
    @@ -836,9 +839,8 @@ asmlinkage int vprintk(const char *fmt,
    boot_delay_msec();
    printk_delay();

    - preempt_disable();
    /* This stops the holder of console_sem just where we want him */
    - raw_local_irq_save(flags);
    + local_irq_save(flags);
    this_cpu = smp_processor_id();

    /*
    @@ -859,7 +861,6 @@ asmlinkage int vprintk(const char *fmt,
    zap_locks();
    }

    - lockdep_off();
    spin_lock(&logbuf_lock);
    printk_cpu = this_cpu;

    @@ -956,11 +957,9 @@ asmlinkage int vprintk(const char *fmt,
    if (console_trylock_for_printk(this_cpu))
    console_unlock();

    - lockdep_on();
    out_restore_irqs:
    - raw_local_irq_restore(flags);
    + local_irq_restore(flags);

    - preempt_enable();
    return printed_len;
    }
    EXPORT_SYMBOL(printk);
    @@ -1271,8 +1270,8 @@ void console_unlock(void)
    if (unlikely(exclusive_console))
    exclusive_console = NULL;

    - up(&console_sem);
    spin_unlock_irqrestore(&logbuf_lock, flags);
    + up(&console_sem);
    if (wake_klogd)
    wake_up_klogd();
    }


    \
     
     \ /
      Last update: 2011-06-06 17:49    [W:0.027 / U:60.288 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site