Messages in this thread | | | Date | Tue, 29 Jul 2003 20:09:20 +0200 | From | Manfred Spraul <> | Subject | Re: [PATCH] select fix |
| |
Valdis.Kletnieks@vt.edu wrote:
>On Tue, 29 Jul 2003 10:36:30 PDT, Andrew Morton said: > > > >>>- if (tty->driver->chars_in_buffer(tty) < WAKEUP_CHARS) >>>+ if (!tty->stopped && tty->driver->chars_in_buffer(tty) < WAKEUP_CHARS) >>> mask |= POLLOUT | POLLWRNORM; >>> >>> >>Manfred sent a patch through esterday which addresses it this way: >> >>- if (tty->driver->chars_in_buffer(tty) < WAKEUP_CHARS) >>+ if (tty->driver->chars_in_buffer(tty) < WAKEUP_CHARS && >>+ tty->driver->write_room(tty) > 0) >> >>Any preferences? >> >> > >Would including all 3 conditions make sense? Not sure if it should be A&B&C, or >A&(B|C) though, but it certainly smells like the write_room() and tty->stopped >checks are covering 2 different corner cases.... > > No. select() and write() must agree when -EAGAIN happens. write() will fail if write_room() returns 0. Additionally, we want to delay wakeups a bit, to reduce context switches. The problem is that the console driver implements stopping by returning 0 from ->write_room() - therefore "less than WAKEUP_CHARS in buffer" is not equivalent to "write will not return -EAGAIN", and thus user space loops. My patch fixes that by checking ->write_room() in normal_poll.
Perhaps the Right Thing (tm) is > if (tty->driver->write_room(tty) > WAKEUP_CHARS) > mask |= POLLOUT | POLLWRNORM;
but I simply to not understand the tty layer at all, thus I proposed the minimal patch.
-- Manfred
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |