Messages in this thread | | | Date | Mon, 3 Jul 2000 17:36:40 -0700 (PDT) | From | Chris Lattner <> | Subject | Re: [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; }
| |