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.046 / U:0.120 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site