lkml.org 
[lkml]   [2000]   [Jul]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH #2] console lock grabbed too early in printk...

    Here's "PATCH #3", that basically counts recursive printk entries and
    printks the number of failed printks upon the next successful printk...

    Please, let me know if it helps!

    -Chris


    On Tue, 4 Jul 2000, Roger Larsson wrote:

    > Hi,
    >
    > But WHERE is the patch?
    > (I like to try it with my latency-profiling patch)
    >
    > /RogerL
    >
    > Chris Lattner wrote:
    > >
    > > Okay, try two. :)
    > >
    > > I don't think that completely redesigning the console before 2.4 goes out
    > > is such a good idea... so here is a much smaller fix that also happens to
    > > fix my previous oversight (yeeeouch! :).
    > >
    > > Basically, printk needs to be locked more finely... there is no reason in
    > > the world for the console lock to protect "buf" in addition to all the
    > > console stuff... so this patch adds protection to buf, moves buf into
    > > printk (as a static array), AND allows _SAFE_ recursion by kmalloc'ing a
    > > new buffer if "buf" is in use.
    > >
    > > This should reduce some console latency by making the console lock unheld
    > > for the vsprintf but held for the real console stuff... This patch keeps
    > > the common case nearly identical in performance: it only does a kmalloc
    > > during the extremely unlikely cases that are not handled now... [okay, I
    > > guess deadlock is "handling" it... but... :]
    > >
    > > Personally, I didn't like the idea of having one "buf" per proc, because
    > > it doesn't fix the recursion problem, it expands the needed data space
    > > (albeit not by much), and (if that approach were to be allied more
    > > generally) would bloat the kernel by a lot. The one thing it had going
    > > for it was the fact that you could be vsprintf'ing in parrellel! :)
    > >
    > > Anyways, let me know if I did something stupid again. :)
    > >
    > > -Chris
    > >
    > > -
    > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    > > the body of a message to majordomo@vger.rutgers.edu
    > > Please read the FAQ at http://www.tux.org/lkml/
    >
    > --
    > Home page:
    > http://www.norran.net/nra02596/
    >

    *** printk.c~ Fri Jun 30 16:24:38 2000
    --- printk.c Sun Jul 2 05:20:00 2000
    ***************
    *** 19,32 ****
    #include <linux/smp_lock.h>
    #include <linux/console.h>
    #include <linux/init.h>

    #include <asm/uaccess.h>

    #define LOG_BUF_LEN (16384)
    #define LOG_BUF_MASK (LOG_BUF_LEN-1)

    - static char buf[1024];
    -
    /* printk's without a loglevel use this.. */
    #define DEFAULT_MESSAGE_LOGLEVEL 4 /* KERN_WARNING */

    --- 19,31 ----
    #include <linux/smp_lock.h>
    #include <linux/console.h>
    #include <linux/init.h>
    + #include <linux/slab.h>

    #include <asm/uaccess.h>

    #define LOG_BUF_LEN (16384)
    #define LOG_BUF_MASK (LOG_BUF_LEN-1)

    /* printk's without a loglevel use this.. */
    #define DEFAULT_MESSAGE_LOGLEVEL 4 /* KERN_WARNING */

    ***************
    *** 253,271 ****

    asmlinkage int printk(const char *fmt, ...)
    {
    va_list args;
    int i;
    char *msg, *p, *buf_end;
    int line_feed;
    static signed char msg_level = -1;
    long flags;

    va_start(args, fmt);
    i = vsprintf(buf + 3, fmt, args); /* hopefully i < sizeof(buf)-4 */
    buf_end = buf + 3 + i;
    va_end(args);
    !
    spin_lock_irqsave(&console_lock, flags);
    for (p = buf + 3; p < buf_end; p++) {
    msg = p;
    if (msg_level < 0) {
    --- 252,318 ----

    asmlinkage int printk(const char *fmt, ...)
    {
    + static spinlock_t printk_buf_lock = SPIN_LOCK_UNLOCKED;
    + static struct task_struct *printk_buf_holder = 0;
    + static char printk_buf[1024];
    + static atomic_t lost_message = ATOMIC_INIT(0);
    +
    va_list args;
    int i;
    char *msg, *p, *buf_end;
    int line_feed;
    static signed char msg_level = -1;
    long flags;
    + char *buf = printk_buf;
    +
    + if (!spin_trylock(&printk_buf_lock)) {
    + /* Couldn't get spinlock... do we already have it? */
    + if (printk_buf_holder == current) {
    + /* We are already in printk... so we would be deadlocking
    + * mark this as such and we'll print a message later...
    + */
    + atomic_inc(&lost_message);
    + return 0;
    + } else {
    + /* Nope, wait for someone to release it... */
    + spin_lock(&printk_buf_lock);
    + }
    + }
    +
    + /* We are now the proud holders of the printk_buf_lock! :) */
    + printk_buf_holder = current;
    +
    + if (atomic_read(&lost_message)) {
    + struct console *c = console_drivers;
    + int Count = atomic_read(&lost_message);
    + char dropped[] =
    + "XX recursive printk's detected, message(s) lost\n";
    + /* Small window to lose message... not critical though */
    + atomic_set(&lost_message, 0);
    +
    + if (Count < 100) {
    + if (Count > 9)
    + dropped[0] = (char)(Count / 10);
    + else
    + dropped[0] = ' ';
    +
    + dropped[1] = Count % 10;
    + }
    +
    + while(c) {
    + if ((c->flags & CON_ENABLED) && c->write)
    + c->write(c, dropped, 49);
    + c = c->next;
    + }
    + }

    va_start(args, fmt);
    i = vsprintf(buf + 3, fmt, args); /* hopefully i < sizeof(buf)-4 */
    buf_end = buf + 3 + i;
    va_end(args);
    !
    spin_lock_irqsave(&console_lock, flags);
    +
    for (p = buf + 3; p < buf_end; p++) {
    msg = p;
    if (msg_level < 0) {
    ***************
    *** 308,315 ****
    --- 355,366 ----
    if (line_feed)
    msg_level = -1;
    }
    +
    spin_unlock_irqrestore(&console_lock, flags);
    wake_up_interruptible(&log_wait);
    +
    + spin_unlock(&printk_buf_lock);
    +
    return i;
    }

    \
     
     \ /
      Last update: 2005-03-22 13:57    [W:0.033 / U:0.252 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site