* [patch] small tty irq race fix
@ 2003-03-03 18:09 Nicolas Pitre
2003-03-03 20:06 ` Alan Cox
0 siblings, 1 reply; 6+ messages in thread
From: Nicolas Pitre @ 2003-03-03 18:09 UTC (permalink / raw)
To: torvalds; +Cc: lkml
--- linux-2.5.63/drivers/char/tty_io.c.orig Mon Feb 24 14:05:34 2003
+++ linux-2.5.63/drivers/char/tty_io.c Mon Mar 3 13:02:31 2003
@@ -1947,17 +1947,17 @@
if (tty->flip.buf_num) {
cp = tty->flip.char_buf + TTY_FLIPBUF_SIZE;
fp = tty->flip.flag_buf + TTY_FLIPBUF_SIZE;
- tty->flip.buf_num = 0;
local_irq_save(flags); // FIXME: is this safe?
+ tty->flip.buf_num = 0;
tty->flip.char_buf_ptr = tty->flip.char_buf;
tty->flip.flag_buf_ptr = tty->flip.flag_buf;
} else {
cp = tty->flip.char_buf;
fp = tty->flip.flag_buf;
- tty->flip.buf_num = 1;
local_irq_save(flags); // FIXME: is this safe?
+ tty->flip.buf_num = 1;
tty->flip.char_buf_ptr = tty->flip.char_buf + TTY_FLIPBUF_SIZE;
tty->flip.flag_buf_ptr = tty->flip.flag_buf + TTY_FLIPBUF_SIZE;
}
Nicolas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] small tty irq race fix
2003-03-03 20:06 ` Alan Cox
@ 2003-03-03 19:15 ` Nicolas Pitre
2003-03-03 21:20 ` Nicolas Pitre
1 sibling, 0 replies; 6+ messages in thread
From: Nicolas Pitre @ 2003-03-03 19:15 UTC (permalink / raw)
To: Alan Cox; +Cc: Linus Torvalds, lkml
On 3 Mar 2003, Alan Cox wrote:
>
> > fp = tty->flip.flag_buf + TTY_FLIPBUF_SIZE;
> > - tty->flip.buf_num = 0;
> >
> > local_irq_save(flags); // FIXME: is this safe?
> > + tty->flip.buf_num = 0;
>
> The other CPU can be touching these fields too surely. Its a
> useful note that the spinlocks need putting in the right spot
> but its still broken 8(
Well I only care about UP at the moment and the patch makes it right for UP
at least. Someone with his brain around the tty locking requirements can
look at replacing the local_irq_save() (there and elsewhere as well), which
is sort of a different issue.
Nicolas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] small tty irq race fix
2003-03-03 18:09 [patch] small tty irq race fix Nicolas Pitre
@ 2003-03-03 20:06 ` Alan Cox
2003-03-03 19:15 ` Nicolas Pitre
2003-03-03 21:20 ` Nicolas Pitre
0 siblings, 2 replies; 6+ messages in thread
From: Alan Cox @ 2003-03-03 20:06 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Linus Torvalds, lkml
> fp = tty->flip.flag_buf + TTY_FLIPBUF_SIZE;
> - tty->flip.buf_num = 0;
>
> local_irq_save(flags); // FIXME: is this safe?
> + tty->flip.buf_num = 0;
The other CPU can be touching these fields too surely. Its a
useful note that the spinlocks need putting in the right spot
but its still broken 8(
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] small tty irq race fix
2003-03-03 20:06 ` Alan Cox
2003-03-03 19:15 ` Nicolas Pitre
@ 2003-03-03 21:20 ` Nicolas Pitre
2003-03-03 21:39 ` Linus Torvalds
1 sibling, 1 reply; 6+ messages in thread
From: Nicolas Pitre @ 2003-03-03 21:20 UTC (permalink / raw)
To: Alan Cox; +Cc: Linus Torvalds, lkml
On Mon, 3 Mar 2003, Alan Cox wrote:
>
> > fp = tty->flip.flag_buf + TTY_FLIPBUF_SIZE;
> > - tty->flip.buf_num = 0;
> >
> > local_irq_save(flags); // FIXME: is this safe?
> > + tty->flip.buf_num = 0;
>
> The other CPU can be touching these fields too surely. Its a
> useful note that the spinlocks need putting in the right spot
> but its still broken 8(
What about this one? It just happens that tty->read_lock is actually used
deeper in the same call instance (in n_tty.c) so this looks to be the best
lock to use.
--- linux-2.5.63/drivers/char/tty_io.c.orig Mon Feb 24 14:05:34 2003
+++ linux-2.5.63/drivers/char/tty_io.c Mon Mar 3 16:13:30 2003
@@ -1947,23 +1947,23 @@
if (tty->flip.buf_num) {
cp = tty->flip.char_buf + TTY_FLIPBUF_SIZE;
fp = tty->flip.flag_buf + TTY_FLIPBUF_SIZE;
- tty->flip.buf_num = 0;
- local_irq_save(flags); // FIXME: is this safe?
+ spin_lock_irqsave(&tty->read_lock, flags);
+ tty->flip.buf_num = 0;
tty->flip.char_buf_ptr = tty->flip.char_buf;
tty->flip.flag_buf_ptr = tty->flip.flag_buf;
} else {
cp = tty->flip.char_buf;
fp = tty->flip.flag_buf;
- tty->flip.buf_num = 1;
- local_irq_save(flags); // FIXME: is this safe?
+ spin_lock_irqsave(&tty->read_lock, flags);
+ tty->flip.buf_num = 1;
tty->flip.char_buf_ptr = tty->flip.char_buf + TTY_FLIPBUF_SIZE;
tty->flip.flag_buf_ptr = tty->flip.flag_buf + TTY_FLIPBUF_SIZE;
}
count = tty->flip.count;
tty->flip.count = 0;
- local_irq_restore(flags); // FIXME: is this safe?
+ spin_unlock_irqrestore(&tty->read_lock, flags);
tty->ldisc.receive_buf(tty, cp, fp, count);
}
Nicolas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] small tty irq race fix
2003-03-03 21:20 ` Nicolas Pitre
@ 2003-03-03 21:39 ` Linus Torvalds
2003-03-03 21:51 ` Nicolas Pitre
0 siblings, 1 reply; 6+ messages in thread
From: Linus Torvalds @ 2003-03-03 21:39 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Alan Cox, lkml
On Mon, 3 Mar 2003, Nicolas Pitre wrote:
>
> What about this one? It just happens that tty->read_lock is actually used
> deeper in the same call instance (in n_tty.c) so this looks to be the best
> lock to use.
Looks ok. I would suggest moving the "spin_lock_irqsave()" to outside the
'if'-statement, though, since that should make the code a lot more
readable, and if the lock is supposed to protect tty->flip.buf_num, then
let's do it right and protect the read as well, no?
Linus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] small tty irq race fix
2003-03-03 21:39 ` Linus Torvalds
@ 2003-03-03 21:51 ` Nicolas Pitre
0 siblings, 0 replies; 6+ messages in thread
From: Nicolas Pitre @ 2003-03-03 21:51 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Alan Cox, lkml
On Mon, 3 Mar 2003, Linus Torvalds wrote:
>
> On Mon, 3 Mar 2003, Nicolas Pitre wrote:
> >
> > What about this one? It just happens that tty->read_lock is actually used
> > deeper in the same call instance (in n_tty.c) so this looks to be the best
> > lock to use.
>
> Looks ok. I would suggest moving the "spin_lock_irqsave()" to outside the
> 'if'-statement, though, since that should make the code a lot more
> readable, and if the lock is supposed to protect tty->flip.buf_num, then
> let's do it right and protect the read as well, no?
Oh sure.
--- linux-2.5.63/drivers/char/tty_io.c.orig Mon Feb 24 14:05:34 2003
+++ linux-2.5.63/drivers/char/tty_io.c Mon Mar 3 16:49:44 2003
@@ -1944,27 +1944,25 @@
schedule_delayed_work(&tty->flip.work, 1);
return;
}
+
+ spin_lock_irqsave(&tty->read_lock, flags);
if (tty->flip.buf_num) {
cp = tty->flip.char_buf + TTY_FLIPBUF_SIZE;
fp = tty->flip.flag_buf + TTY_FLIPBUF_SIZE;
tty->flip.buf_num = 0;
-
- local_irq_save(flags); // FIXME: is this safe?
tty->flip.char_buf_ptr = tty->flip.char_buf;
tty->flip.flag_buf_ptr = tty->flip.flag_buf;
} else {
cp = tty->flip.char_buf;
fp = tty->flip.flag_buf;
tty->flip.buf_num = 1;
-
- local_irq_save(flags); // FIXME: is this safe?
tty->flip.char_buf_ptr = tty->flip.char_buf + TTY_FLIPBUF_SIZE;
tty->flip.flag_buf_ptr = tty->flip.flag_buf + TTY_FLIPBUF_SIZE;
}
count = tty->flip.count;
tty->flip.count = 0;
- local_irq_restore(flags); // FIXME: is this safe?
-
+ spin_unlock_irqrestore(&tty->read_lock, flags);
+
tty->ldisc.receive_buf(tty, cp, fp, count);
}
Nicolas
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2003-03-03 21:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-03-03 18:09 [patch] small tty irq race fix Nicolas Pitre
2003-03-03 20:06 ` Alan Cox
2003-03-03 19:15 ` Nicolas Pitre
2003-03-03 21:20 ` Nicolas Pitre
2003-03-03 21:39 ` Linus Torvalds
2003-03-03 21:51 ` Nicolas Pitre
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).