From: Andrea Arcangeli <andrea@suse.de>
>Your patch is not complete. You can drop the big kernel lock from
>do_syslog and you must grab the console_lock also in other places.
Ok.
But I think it's a bad idea to grab the console lock while
calling console->setup().
>Also there's no need to change the internals of syslog(case 3:). You only
>care to have a coherent `j' value to not overflow the static kernel ring
>buffer.
I don't know what we want to do here:
* my patch guarantees that syslog(3) will only return valid data, i.e.
it will refuse to return data which was already overwritten.
* your patch is simpler, but it could return overwritten data.
Please note that there is no overhead caused by the &(LEN-1), because
the &(LEN-1) is required anyway. [Obviously, I prefer my patch]
> Manfred I would appreciate if you could check that I didn't missed
> something from your original patch.
You missed nothing, but I found new problems:
* register_console(CON_PRINTBUFFER) could use the wrong msg_level for
the first line.
* console_print() must not use spin_lock_irq(), it needs
spin_lock_irqsave().
[e.g. drivers/block/cpqarray.c, line 1021. I installed the linux cross
reference from lxr.linux.no, really nice]
I've updated my own patch.
--
Manfred
--------------2AB83B7FD27C008B57FBF406
Content-Type: text/plain; charset=us-ascii;
name="patch-console_lock"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="patch-console_lock"
// $Header: /pub/cvs/ms/patches/patch-console_lock,v 1.2 1999/08/12 17:23:21 manfreds Exp $
// kernel 2.3.13
--- 2.3/kernel/printk.c Thu Aug 12 19:16:12 1999
+++ build-2.3/kernel/printk.c Thu Aug 12 19:00:48 1999
@@ -10,6 +10,8 @@
* elsewhere, in preparation for a serial line console (someday).
* Ted Ts'o, 2/11/93.
* Modified for sysctl support, 1/8/97, Chris Horn.
+ * Fixed SMP synchronization, 08/08/99, Manfred Spraul
+ * manfreds@colorfullife.com
*/
#include <linux/mm.h>
@@ -40,6 +42,8 @@
int minimum_console_loglevel = MINIMUM_CONSOLE_LOGLEVEL;
int default_console_loglevel = DEFAULT_CONSOLE_LOGLEVEL;
+spinlock_t console_lock;
+
struct console *console_drivers = NULL;
static char log_buf[LOG_BUF_LEN];
static unsigned long log_start = 0;
@@ -117,12 +121,11 @@
*/
int do_syslog(int type, char * buf, int len)
{
- unsigned long i, j, count, flags;
+ unsigned long i, j, limit, count;
int do_clear = 0;
char c;
int error = -EPERM;
- lock_kernel();
error = 0;
switch (type) {
case 0: /* Close log */
@@ -143,18 +146,18 @@
if (error)
goto out;
i = 0;
+ spin_lock_irq(&console_lock);
while (log_size && i < len) {
- c = *((char *) log_buf+log_start);
+ c = *((char *) log_buf+(log_start & LOG_BUF_LEN-1));
log_start++;
log_size--;
- log_start &= LOG_BUF_LEN-1;
- sti();
+ spin_unlock_irq(&console_lock);
__put_user(c,buf);
buf++;
i++;
- cli();
+ spin_lock_irq(&console_lock);
}
- sti();
+ spin_unlock_irq(&console_lock);
error = i;
break;
case 4: /* Read/clear last kernel messages */
@@ -170,35 +173,52 @@
error = verify_area(VERIFY_WRITE,buf,len);
if (error)
goto out;
- /*
- * The logged_chars, log_start, and log_size values may
- * change from an interrupt, so we disable interrupts.
- */
- __save_flags(flags);
- __cli();
+ spin_lock_irq(&console_lock);
count = len;
if (count > LOG_BUF_LEN)
count = LOG_BUF_LEN;
if (count > logged_chars)
count = logged_chars;
- j = log_start + log_size - count;
- __restore_flags(flags);
- for (i = 0; i < count; i++) {
- c = *((char *) log_buf+(j++ & (LOG_BUF_LEN-1)));
- __put_user(c, buf++);
- }
if (do_clear)
logged_chars = 0;
+ limit = log_start + log_size;
+ /*
+ * __put_user() could sleep, and while we sleep
+ * the printk() could overwrite the data
+ * we try to copy to user space. Therefore
+ * I must copy the data in reverse. <manfreds>
+ */
+ for(i=0;i < count;i++) {
+ j = limit-1-i;
+ if(j+LOG_BUF_LEN < log_start+log_size)
+ break;
+ c = *((char*) log_buf+( j & (LOG_BUF_LEN-1)));
+ spin_unlock_irq(&console_lock);
+ __put_user(c,&buf[count-1-i]);
+ spin_lock_irq(&console_lock);
+ }
+ spin_unlock_irq(&console_lock);
+ if(i != count) {
+ /* buffer overflow during copy, correct user buffer. */
+ memmove(buf,&buf[count-i],i);
+ }
error = i;
+
break;
case 5: /* Clear ring buffer */
+ spin_lock_irq(&console_lock);
logged_chars = 0;
+ spin_unlock_irq(&console_lock);
break;
case 6: /* Disable logging to console */
+ spin_lock_irq(&console_lock);
console_loglevel = minimum_console_loglevel;
+ spin_unlock_irq(&console_lock);
break;
case 7: /* Enable logging to console */
+ spin_lock_irq(&console_lock);
console_loglevel = default_console_loglevel;
+ spin_unlock_irq(&console_lock);
break;
case 8:
error = -EINVAL;
@@ -206,7 +226,9 @@
goto out;
if (len < minimum_console_loglevel)
len = minimum_console_loglevel;
+ spin_lock_irq(&console_lock);
console_loglevel = len;
+ spin_unlock_irq(&console_lock);
error = 0;
break;
default:
@@ -214,7 +236,6 @@
break;
}
out:
- unlock_kernel();
return error;
}
@@ -225,9 +246,6 @@
return do_syslog(type, buf, len);
}
-
-spinlock_t console_lock;
-
asmlinkage int printk(const char *fmt, ...)
{
va_list args;
@@ -264,10 +282,9 @@
log_buf[(log_start+log_size) & (LOG_BUF_LEN-1)] = *p;
if (log_size < LOG_BUF_LEN)
log_size++;
- else {
+ else
log_start++;
- log_start &= LOG_BUF_LEN-1;
- }
+
logged_chars++;
if (*p == '\n') {
line_feed = 1;
@@ -292,24 +309,33 @@
void console_print(const char *s)
{
- struct console *c = console_drivers;
+ struct console *c;
+ unsigned long flags;
int len = strlen(s);
+ spin_lock_irqsave(&console_lock,flags);
+ c = console_drivers;
while(c) {
if ((c->flags & CON_ENABLED) && c->write)
c->write(c, s, len);
c = c->next;
}
+ spin_unlock_irqrestore(&console_lock,flags);
}
void unblank_console(void)
{
- struct console *c = console_drivers;
+ struct console *c;
+ unsigned long flags;
+
+ spin_lock_irqsave(&console_lock,flags);
+ c = console_drivers;
while(c) {
if ((c->flags & CON_ENABLED) && c->unblank)
c->unblank();
c = c->next;
}
+ spin_unlock_irqrestore(&console_lock,flags);
}
/*
@@ -320,11 +346,8 @@
*/
void register_console(struct console * console)
{
- int i,j,len;
- int p = log_start;
- char buf[16];
- signed char msg_level = -1;
- char *q;
+ int i;
+ unsigned long flags;
/*
* See if we want to use this console driver. If we
@@ -370,6 +393,7 @@
* Put this console in the list - keep the
* preferred driver at the head of the list.
*/
+ spin_lock_irqsave(&console_lock,flags);
if ((console->flags & CON_CONSDEV) || console_drivers == NULL) {
console->next = console_drivers;
console_drivers = console;
@@ -377,50 +401,71 @@
console->next = console_drivers->next;
console_drivers->next = console;
}
- if ((console->flags & CON_PRINTBUFFER) == 0) return;
-
- /*
- * Print out buffered log messages.
- */
- for (i=0,j=0; i < log_size; i++) {
- buf[j++] = log_buf[p];
- p++; p &= LOG_BUF_LEN-1;
- if (buf[j-1] != '\n' && i < log_size - 1 && j < sizeof(buf)-1)
- continue;
- buf[j] = 0;
- q = buf;
- len = j;
- if (msg_level < 0) {
- msg_level = buf[1] - '0';
- q = buf + 3;
- len -= 3;
+ if ((console->flags & CON_PRINTBUFFER) != 0) {
+ /*
+ * Print out buffered log messages.
+ */
+ int j,len;
+ int p = (log_start& LOG_BUF_LEN-1);
+ char buf[16];
+ signed char msg_level = -1;
+ char *q;
+ for (i=0,j=0; i < log_size; i++) {
+ buf[j++] = log_buf[p];
+ p++; p &= LOG_BUF_LEN-1;
+ if (buf[j-1] != '\n' && i < log_size - 1 && j < sizeof(buf)-1)
+ continue;
+ buf[j] = 0;
+ q = buf;
+ len = j;
+ if (msg_level < 0) {
+ if(buf[0] == '<' &&
+ buf[1] >= '0' &&
+ buf[1] <= '7' &&
+ buf[2] == '>') {
+ msg_level = buf[1] - '0';
+ q = buf + 3;
+ len -= 3;
+ } else
+ {
+ msg_level = default_message_loglevel;
+ }
+ }
+ if (msg_level < console_loglevel)
+ console->write(console, q, len);
+ if (buf[j-1] == '\n')
+ msg_level = -1;
+ j = 0;
}
- if (msg_level < console_loglevel)
- console->write(console, q, len);
- if (buf[j-1] == '\n')
- msg_level = -1;
- j = 0;
}
+ spin_unlock_irqrestore(&console_lock,flags);
}
int unregister_console(struct console * console)
{
struct console *a,*b;
-
+ unsigned long flags;
+ int res = 1;
+
+ spin_lock_irqsave(&console_lock,flags);
if (console_drivers == console) {
console_drivers=console->next;
- return (0);
- }
- for (a=console_drivers->next, b=console_drivers ;
- a; b=a, a=b->next) {
- if (a == console) {
- b->next = a->next;
- return 0;
- }
+ res = 0;
+ } else
+ {
+ for (a=console_drivers->next, b=console_drivers ;
+ a; b=a, a=b->next) {
+ if (a == console) {
+ b->next = a->next;
+ res = 0;
+ break;
+ }
+ }
}
- return (1);
+ spin_unlock_irqrestore(&console_lock,flags);
+ return res;
}
/*
--------------2AB83B7FD27C008B57FBF406--
-
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/