I have tracked down why my ppp-over-pty-over-socket-over-ethernet test
setup was clagging up on SMP machines with 2.2.15pre9. The problem is
actually in drivers/char/n_tty.c.
The way the setup works is that I have a pty master/slave pair with the
slave side set to PPP line discipline. The master side is in the normal
(N_TTY) line discipline. There is a usermode process transferring
characters between the master side and a socket.
The n_tty.c code has a receive buffer (tty->read_buf) which is used as a
ring buffer addressed by tty->read_head and tty->read_tail, with
tty->read_cnt being the number of characters in the buffer. What I was
seeing was that these three were getting out of sync, so that for example
at one stage read_head was 225, read_tail was 475 but read_cnt was 0.
The problem was basically the concurrency between the read_chan() function
and the n_tty_receive_buf function. read_chan() attempts to control this
by setting the TTY_DONT_FLIP bit, which prevents n_tty_receive_buf from
being called from flush_to_ldisc in tty_io.c. But the pty_write function
calls ldisc.receive_buf without checking the TTY_DONT_FLIP bit.
The patch below fixes the problem using a spinlock around all the sections
in n_tty.c that update tty->read_{head,tail,cnt}.
The patch also fixes a potential problem in pty.c where it calls
to->ldisc.receive_room() inside the MIN macro like this:
c = MIN(count, to->ldisc.receive_room(to));
Since receive_room can return a different value each time, we could end up
with c > count.
With this patch, I can run my ppp-over-pty setup without any problems on
an SMP machine.
Paul.
diff -urN official/include/linux/tty.h linux/include/linux/tty.h
--- official/include/linux/tty.h Wed Jan 5 09:19:53 2000
+++ linux/include/linux/tty.h Wed Feb 23 13:45:08 2000
@@ -304,6 +304,7 @@
unsigned int canon_column;
struct semaphore atomic_read;
struct semaphore atomic_write;
+ spinlock_t read_lock;
};
/* tty magic number */
diff -urN official/drivers/char/n_tty.c linux/drivers/char/n_tty.c
--- official/drivers/char/n_tty.c Mon Feb 21 11:15:45 2000
+++ linux/drivers/char/n_tty.c Wed Feb 23 13:48:30 2000
@@ -62,8 +62,6 @@
#define TTY_THRESHOLD_THROTTLE 128 /* now based on remaining room */
#define TTY_THRESHOLD_UNTHROTTLE 128
-static spinlock_t __attribute((unused)) tty_put_lock = SPIN_LOCK_UNLOCKED;
-
static inline void put_tty_queue(unsigned char c, struct tty_struct *tty)
{
unsigned long flags;
@@ -71,13 +69,13 @@
* The problem of stomping on the buffers ends here.
* Why didn't anyone see this one comming? --AJK
*/
- spin_lock_irqsave(&tty_put_lock, flags);
+ spin_lock_irqsave(&tty->read_lock, flags);
if (tty->read_cnt < N_TTY_BUF_SIZE) {
tty->read_buf[tty->read_head] = c;
tty->read_head = (tty->read_head + 1) & (N_TTY_BUF_SIZE-1);
tty->read_cnt++;
}
- spin_unlock_irqrestore(&tty_put_lock, flags);
+ spin_unlock_irqrestore(&tty->read_lock, flags);
}
/*
@@ -100,7 +98,11 @@
*/
static void reset_buffer_flags(struct tty_struct *tty)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&tty->read_lock, flags);
tty->read_head = tty->read_tail = tty->read_cnt = 0;
+ spin_unlock_irqrestore(&tty->read_lock, flags);
tty->canon_head = tty->canon_data = tty->erasing = 0;
memset(&tty->read_flags, 0, sizeof tty->read_flags);
check_unthrottle(tty);
@@ -128,14 +130,19 @@
*/
ssize_t n_tty_chars_in_buffer(struct tty_struct *tty)
{
- if (tty->icanon) {
- if (!tty->canon_data) return 0;
+ unsigned long flags;
+ ssize_t n = 0;
- return (tty->canon_head > tty->read_tail) ?
+ spin_lock_irqsave(&tty->read_lock, flags);
+ if (!tty->icanon) {
+ n = tty->read_cnt;
+ } else if (tty->canon_data) {
+ n = (tty->canon_head > tty->read_tail) ?
tty->canon_head - tty->read_tail :
tty->canon_head + (N_TTY_BUF_SIZE - tty->read_tail);
}
- return tty->read_cnt;
+ spin_unlock_irqrestore(&tty->read_lock, flags);
+ return n;
}
/*
@@ -297,6 +304,7 @@
{
enum { ERASE, WERASE, KILL } kill_type;
int head, seen_alnums;
+ unsigned long flags;
if (tty->read_head == tty->canon_head) {
/* opost('\a', tty); */ /* what do you think? */
@@ -308,15 +316,19 @@
kill_type = WERASE;
else {
if (!L_ECHO(tty)) {
+ spin_lock_irqsave(&tty->read_lock, flags);
tty->read_cnt -= ((tty->read_head - tty->canon_head) &
(N_TTY_BUF_SIZE - 1));
tty->read_head = tty->canon_head;
+ spin_unlock_irqrestore(&tty->read_lock, flags);
return;
}
if (!L_ECHOK(tty) || !L_ECHOKE(tty) || !L_ECHOE(tty)) {
+ spin_lock_irqsave(&tty->read_lock, flags);
tty->read_cnt -= ((tty->read_head - tty->canon_head) &
(N_TTY_BUF_SIZE - 1));
tty->read_head = tty->canon_head;
+ spin_unlock_irqrestore(&tty->read_lock, flags);
finish_erasing(tty);
echo_char(KILL_CHAR(tty), tty);
/* Add a newline if ECHOK is on and ECHOKE is off. */
@@ -338,8 +350,10 @@
else if (seen_alnums)
break;
}
+ spin_lock_irqsave(&tty->read_lock, flags);
tty->read_head = head;
tty->read_cnt--;
+ spin_unlock_irqrestore(&tty->read_lock, flags);
if (L_ECHO(tty)) {
if (L_ECHOPRT(tty)) {
if (!tty->erasing) {
@@ -672,11 +686,13 @@
char *f, flags = TTY_NORMAL;
int i;
char buf[64];
+ unsigned long cpuflags;
if (!tty->read_buf)
return;
if (tty->real_raw) {
+ spin_lock_irqsave(&tty->read_lock, cpuflags);
i = MIN(count, MIN(N_TTY_BUF_SIZE - tty->read_cnt,
N_TTY_BUF_SIZE - tty->read_head));
memcpy(tty->read_buf + tty->read_head, cp, i);
@@ -690,6 +706,7 @@
memcpy(tty->read_buf + tty->read_head, cp, i);
tty->read_head = (tty->read_head + i) & (N_TTY_BUF_SIZE-1);
tty->read_cnt += i;
+ spin_unlock_irqrestore(&tty->read_lock, cpuflags);
} else {
for (i=count, p = cp, f = fp; i; i--, p++) {
if (f)
@@ -864,15 +881,20 @@
{
int retval;
ssize_t n;
+ unsigned long flags;
retval = 0;
+ spin_lock_irqsave(&tty->read_lock, flags);
n = MIN(*nr, MIN(tty->read_cnt, N_TTY_BUF_SIZE - tty->read_tail));
+ spin_unlock_irqrestore(&tty->read_lock, flags);
if (n) {
mb();
retval = copy_to_user(*b, &tty->read_buf[tty->read_tail], n);
n -= retval;
+ spin_lock_irqsave(&tty->read_lock, flags);
tty->read_tail = (tty->read_tail + n) & (N_TTY_BUF_SIZE-1);
tty->read_cnt -= n;
+ spin_unlock_irqrestore(&tty->read_lock, flags);
*b += n;
*nr -= n;
}
@@ -889,6 +911,7 @@
ssize_t retval = 0;
ssize_t size;
long timeout;
+ unsigned long flags;
do_it_again:
@@ -1007,9 +1030,11 @@
eol = test_and_clear_bit(tty->read_tail,
&tty->read_flags);
c = tty->read_buf[tty->read_tail];
+ spin_lock_irqsave(&tty->read_lock, flags);
tty->read_tail = ((tty->read_tail+1) &
(N_TTY_BUF_SIZE-1));
tty->read_cnt--;
+ spin_unlock_irqrestore(&tty->read_lock, flags);
if (!eol || (c != __DISABLED_CHAR)) {
put_user(c, b++);
diff -urN official/drivers/char/pty.c linux/drivers/char/pty.c
--- official/drivers/char/pty.c Wed Feb 24 03:35:10 1999
+++ linux/drivers/char/pty.c Wed Feb 23 12:47:25 2000
@@ -168,7 +168,9 @@
}
up(&tty->flip.pty_sem);
} else {
- c = MIN(count, to->ldisc.receive_room(to));
+ c = to->ldisc.receive_room(to);
+ if (c > count)
+ c = count;
to->ldisc.receive_buf(to, buf, 0, c);
}
diff -urN official/drivers/char/tty_io.c linux/drivers/char/tty_io.c
--- official/drivers/char/tty_io.c Mon Feb 21 11:15:46 2000
+++ linux/drivers/char/tty_io.c Wed Feb 23 12:44:58 2000
@@ -1943,6 +1943,7 @@
tty->tq_hangup.data = tty;
sema_init(&tty->atomic_read, 1);
sema_init(&tty->atomic_write, 1);
+ spin_lock_init(&tty->read_lock);
}
/*
-- Paul Mackerras, Senior Open Source Researcher, Linuxcare, Inc. +61 2 6262 8990 tel, +61 2 6262 8991 fax paulus@linuxcare.com.au, http://www.linuxcare.com.au/ Linuxcare. Support for the revolution.- 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/
This archive was generated by hypermail 2b29 : Wed Feb 23 2000 - 21:00:32 EST