Re: [uml-devel] [BUG: UML 2.6.11-rc4-bk-latest] sleeping function called from invalid context and segmentation fault
From: Blaisorblade
Date: Wed Feb 16 2005 - 13:42:04 EST
On Monday 14 February 2005 12:48, Anton Altaparmakov wrote:
> Hi,
>
> I get a few Debug messages of the form from UML:
>
> Debug: sleeping function called from invalid context at
> include/asm/arch/semaphore.h:107
> in_atomic():0, irqs_disabled():1
> Call Trace:
> 087d77b0: [<0809aaa5>] __might_sleep+0x135/0x180
> 087d77d8: [<084d377f>] mcount+0xf/0x20
> 087d77e0: [<0807cc13>] uml_console_write+0x33/0x80
> Most are coming via uml_console_write.
The problem is that the UML tty drivers use a semaphore instead of a spinlock
for the locking, which also causes some other problems.
The attached patch should fix this, but I've not yet made sure it is not
deadlock-prone (I didn't hit any during some very limited testing).
So it's not yet ready for 2.6.11.
--
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade
Signed-off-by: Paolo 'Blaisorblade' Giarrusso <blaisorblade@xxxxxxxx>
---
linux-2.6.11-paolo/arch/um/drivers/line.c | 49 +++++++++------------
linux-2.6.11-paolo/arch/um/drivers/ssl.c | 16 +-----
linux-2.6.11-paolo/arch/um/drivers/stdio_console.c | 16 +-----
linux-2.6.11-paolo/arch/um/include/line.h | 16 ------
4 files changed, 31 insertions(+), 66 deletions(-)
diff -puN arch/um/drivers/line.c~uml-console-redo-locking arch/um/drivers/line.c
--- linux-2.6.11/arch/um/drivers/line.c~uml-console-redo-locking 2005-02-07 19:16:49.954057296 +0100
+++ linux-2.6.11-paolo/arch/um/drivers/line.c 2005-02-07 19:16:49.969055016 +0100
@@ -52,6 +52,7 @@ static int write_room(struct line *dev)
return (n - 1);
}
+/* Must be called while holding line->lock!*/
static int buffer_data(struct line *line, const char *buf, int len)
{
int end, room;
@@ -84,6 +85,7 @@ static int buffer_data(struct line *line
return(len);
}
+/* Must be called while holding line->lock!*/
static int flush_buffer(struct line *line)
{
int n, count;
@@ -122,16 +124,13 @@ int line_write(struct tty_struct *tty, c
if(tty->stopped) return 0;
- down(&line->sem);
- if(line->head != line->tail){
- local_irq_save(flags);
+ spin_lock_irqsave(&line->lock, flags);
+ if (line->head != line->tail){
ret = buffer_data(line, buf, len);
err = flush_buffer(line);
- local_irq_restore(flags);
if(err <= 0)
ret = err;
- }
- else {
+ } else {
n = write_chan(&line->chan_list, buf, len,
line->driver->write_irq);
if(n < 0){
@@ -144,8 +143,8 @@ int line_write(struct tty_struct *tty, c
if(len > 0)
ret += buffer_data(line, buf + n, len);
}
- out_up:
- up(&line->sem);
+out_up:
+ spin_unlock_irqrestore(&line->lock, flags);
return(ret);
}
@@ -260,6 +259,7 @@ static irqreturn_t line_write_interrupt(
struct line *line = tty->driver_data;
int err;
+ spin_lock(&line->lock);
err = flush_buffer(line);
if(err == 0)
return(IRQ_NONE);
@@ -267,6 +267,7 @@ static irqreturn_t line_write_interrupt(
line->head = line->buffer;
line->tail = line->buffer;
}
+ spin_unlock(&line->lock);
if(tty == NULL)
return(IRQ_NONE);
@@ -336,7 +337,9 @@ int line_open(struct line *lines, struct
line = &lines[tty->index];
tty->driver_data = line;
- down(&line->sem);
+ /* The IRQ which takes this lock is not yet enabled and won't be run
+ * before the end, so we don't need to use spin_lock_irq.*/
+ spin_lock(&line->lock);
if (tty->count == 1) {
if (!line->valid) {
err = -ENODEV;
@@ -349,6 +352,7 @@ int line_open(struct line *lines, struct
err = open_chan(&line->chan_list);
if(err) goto out;
}
+ /* Here the interrupt is registered.*/
enable_chan(&line->chan_list, tty);
INIT_WORK(&line->task, line_timer_cb, tty);
}
@@ -362,7 +366,7 @@ int line_open(struct line *lines, struct
line->count++;
out:
- up(&line->sem);
+ spin_unlock(&line->lock);
return(err);
}
@@ -370,13 +374,17 @@ void line_close(struct tty_struct *tty,
{
struct line *line = tty->driver_data;
- down(&line->sem);
+ spin_lock_irq(&line->lock);
+
+ /* We ignore the error anyway! */
+ flush_buffer(line);
+
line->count--;
if (tty->count == 1) {
line_disable(tty, -1);
tty->driver_data = NULL;
}
- up(&line->sem);
+ spin_unlock_irq(&line->lock);
}
void close_lines(struct line *lines, int nlines)
@@ -474,13 +482,13 @@ int line_get_config(char *name, struct l
line = &lines[dev];
- down(&line->sem);
+ spin_lock(&line->lock);
if(!line->valid)
CONFIG_CHUNK(str, size, n, "none", 1);
else if(line->count == 0)
CONFIG_CHUNK(str, size, n, line->init_str, 1);
else n = chan_config_string(&line->chan_list, str, size, error_out);
- up(&line->sem);
+ spin_unlock(&line->lock);
return(n);
}
@@ -553,7 +561,7 @@ void lines_init(struct line *lines, int
for(i = 0; i < nlines; i++){
line = &lines[i];
INIT_LIST_HEAD(&line->chan_list);
- sema_init(&line->sem, 1);
+ spin_lock_init(&line->lock);
if(line->init_str != NULL){
line->init_str = uml_strdup(line->init_str);
if(line->init_str == NULL)
@@ -668,14 +676,3 @@ char *add_xterm_umid(char *base)
snprintf(title, len, "%s (%s)", base, umid);
return(title);
}
-
-/*
- * Overrides for Emacs so that we follow Linus's tabbing style.
- * Emacs will notice this stuff at the end of the file and automatically
- * adjust the settings for this buffer only. This must remain at the end
- * of the file.
- * ---------------------------------------------------------------------------
- * Local variables:
- * c-file-style: "linux"
- * End:
- */
diff -puN arch/um/include/line.h~uml-console-redo-locking arch/um/include/line.h
--- linux-2.6.11/arch/um/include/line.h~uml-console-redo-locking 2005-02-07 19:16:49.961056232 +0100
+++ linux-2.6.11-paolo/arch/um/include/line.h 2005-02-07 19:16:49.969055016 +0100
@@ -10,7 +10,7 @@
#include "linux/workqueue.h"
#include "linux/tty.h"
#include "linux/interrupt.h"
-#include "asm/semaphore.h"
+#include "linux/spinlock.h"
#include "chan_user.h"
#include "mconsole_kern.h"
@@ -37,7 +37,7 @@ struct line {
struct list_head chan_list;
int valid;
int count;
- struct semaphore sem;
+ spinlock_t lock;
char *buffer;
char *head;
char *tail;
@@ -52,7 +52,6 @@ struct line {
init_pri : INIT_STATIC, \
chan_list : { }, \
valid : 1, \
- sem : { }, \
buffer : NULL, \
head : NULL, \
tail : NULL, \
@@ -95,14 +94,3 @@ extern int line_get_config(char *dev, st
int size, char **error_out);
#endif
-
-/*
- * Overrides for Emacs so that we follow Linus's tabbing style.
- * Emacs will notice this stuff at the end of the file and automatically
- * adjust the settings for this buffer only. This must remain at the end
- * of the file.
- * ---------------------------------------------------------------------------
- * Local variables:
- * c-file-style: "linux"
- * End:
- */
diff -puN arch/um/drivers/ssl.c~uml-console-redo-locking arch/um/drivers/ssl.c
--- linux-2.6.11/arch/um/drivers/ssl.c~uml-console-redo-locking 2005-02-07 19:16:49.963055928 +0100
+++ linux-2.6.11-paolo/arch/um/drivers/ssl.c 2005-02-07 19:16:49.970054864 +0100
@@ -171,10 +171,11 @@ static void ssl_console_write(struct con
unsigned len)
{
struct line *line = &serial_lines[c->index];
+ unsigned long flags;
- down(&line->sem);
+ spin_lock_irqsave(&line->lock, flags);
console_write_chan(&line->chan_list, string, len);
- up(&line->sem);
+ spin_unlock_irqrestore(&line->lock, flags);
}
static struct tty_driver *ssl_console_device(struct console *c, int *index)
@@ -238,14 +239,3 @@ static int ssl_chan_setup(char *str)
__setup("ssl", ssl_chan_setup);
__channel_help(ssl_chan_setup, "ssl");
-
-/*
- * Overrides for Emacs so that we follow Linus's tabbing style.
- * Emacs will notice this stuff at the end of the file and automatically
- * adjust the settings for this buffer only. This must remain at the end
- * of the file.
- * ---------------------------------------------------------------------------
- * Local variables:
- * c-file-style: "linux"
- * End:
- */
diff -puN arch/um/drivers/stdio_console.c~uml-console-redo-locking arch/um/drivers/stdio_console.c
--- linux-2.6.11/arch/um/drivers/stdio_console.c~uml-console-redo-locking 2005-02-07 19:16:49.966055472 +0100
+++ linux-2.6.11-paolo/arch/um/drivers/stdio_console.c 2005-02-07 19:16:49.970054864 +0100
@@ -126,10 +126,11 @@ static void uml_console_write(struct con
unsigned len)
{
struct line *line = &vts[console->index];
+ unsigned long flags;
- down(&line->sem);
+ spin_lock_irqsave(&line->lock, flags);
console_write_chan(&line->chan_list, string, len);
- up(&line->sem);
+ spin_unlock_irqrestore(&line->lock, flags);
}
static struct tty_driver *uml_console_device(struct console *c, int *index)
@@ -192,14 +193,3 @@ static int console_chan_setup(char *str)
}
__setup("con", console_chan_setup);
__channel_help(console_chan_setup, "con");
-
-/*
- * Overrides for Emacs so that we follow Linus's tabbing style.
- * Emacs will notice this stuff at the end of the file and automatically
- * adjust the settings for this buffer only. This must remain at the end
- * of the file.
- * ---------------------------------------------------------------------------
- * Local variables:
- * c-file-style: "linux"
- * End:
- */
_