Subject: kern/12285: long-term tty timeout (c_cc[VTIME]) doesn't work on some platforms
To: None <gnats-bugs@gnats.netbsd.org>
From: None <itohy@netbsd.org>
List: netbsd-bugs
Date: 02/26/2001 13:33:17
>Number:         12285
>Category:       kern
>Synopsis:       long-term tty timeout (c_cc[VTIME]) doesn't work on some platforms
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Feb 25 20:34:00 PST 2001
>Closed-Date:
>Last-Modified:
>Originator:     ITOH Yasufumi
>Release:        1.5S (Feb. 23, 2001)
>Organization:
>Environment:
System: NetBSD acha.my.domain 1.5S NetBSD 1.5S (ACHA) #75: Sun Feb 25 17:17:31 JST 2001 itohy@pino.my.domain:/w/src/sys/arch/x68k/compile/ACHA x68k
Architecture: m68k
Machine: x68k
hz = 100

>Description:
	tty(4) (termios) has ``read timer'' which allows timeouts
	of tty read in noncanonical mode.
	The timer value is set to the c_cc[VTIME] in struct termios,
	which takes a value from 1 (0.1s) upto 255 (25.5s).

	On some platforms, however, the timer is set at
	rather long-term period, it doesn't work as expected.

	For example, on hz = 100 and 32bit long platforms,
	the max timer value which works is 214 (21.4s) and
	if is set to [214,255] it doesn't work (read(2) returns
	immediately).
	On hz = 256 and 32bit long platform (pmax) the max value
	will be as short as 8.3s.


	These platforms are NOT affected by this bug:
		alpha, sparc64		64bit long
		arm32 (Shark), atari	HZ=64
		evbsh3			HZ=50,64
		hpcsh			HZ=50
		mac68k			HZ=60
		mipsco			HZ=25
		mmeye			HZ=10,60

	These platforms are affected:
		All the other ports of NetBSD 1.0 and later
		(NetBSD 0.9 didn't have VMIN/VTIME support)

		FreeBSD/i386 seems also affected :)

>How-To-Repeat:
	Here's a sample program (ttytest.c):

	% uname -srm
	NetBSD 1.5S x68k
	% cat /kern/hz
	100
	% cat ttytest.c
	[[ see below ]]
	% cc -o ttytest ttytest.c
	% time ./ttytest 214
	[[ program sleeps about 20s  (OK) ]]
	read: nread 0, errno 0
	0.2u 0.1s 0:21.89 2.0% 0+0k 0+0io 0pf+0w
	% time ./ttytest 215
	[[ program exits immediately  (NG) ]]
	read: nread 0, errno 0
	0.2u 0.2s 0:00.51 90.1% 0+0k 0+0io 0pf+0w
	%

ttytest.c:
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/termios.h>
#include <err.h>
#include <errno.h>

int
main(argc, argv)
	int argc;
	char *argv[];
{
	struct termios t, tbak;
	ssize_t nread;
	unsigned char c;

	if (tcgetattr(0, &t))
		err(1, "tcgetattr");

	tbak = t;
	cfmakeraw(&t);
	t.c_cc[VMIN] = 0;
	t.c_cc[VTIME] = atoi(argv[1]);

	if (tcsetattr(0, TCSANOW, &t))
		err(1, "tcsetattr");

	errno = 0;
	nread = read(0, &c, 1);

	if (nread > 0)
		printf("read: nread %d, char %d\r\n", (int) nread, c);
	else
		printf("read: nread %d, errno %d\r\n", (int) nread, errno);

	if (tcsetattr(0, TCSANOW, &tbak))
		warn("tcsetattr");

	return 0;
}

>Fix:
	Fix #1.  Just fix the expression as the comment says.
	This works only on hz < 169 machine.

--- sys/kern/tty.c.orig	Mon Dec 25 16:22:33 2000
+++ sys/kern/tty.c	Sun Feb 25 19:17:26 2001
@@ -1428,7 +1428,7 @@
 			 *
 			 * Also, use plain wakeup() not ttwakeup().
 			 */
-			slp = (long) (((u_long)slp * hz) + 999999) / 1000000;
+			slp = (((u_long)slp * hz) + 999999) / 1000000;
 			goto sleep;
 		}
 	} else if ((qp = &tp->t_canq)->c_cc <= 0) {

	Fix #2.  This also fixes on pmax.

--- sys/kern/tty.c.orig	Mon Dec 25 16:22:33 2000
+++ sys/kern/tty.c	Sun Feb 25 19:59:02 2001
@@ -1428,7 +1428,19 @@
 			 *
 			 * Also, use plain wakeup() not ttwakeup().
 			 */
-			slp = (long) (((u_long)slp * hz) + 999999) / 1000000;
+#ifdef pmax
+#define SFT	1	/* hz < 337 */
+#else
+#define SFT	0	/* hz < 169 */
+#endif
+#ifdef DIAGNOSTIC
+			if (hz > (ULONG_MAX - ((1000000 >> SFT) - 1)) /
+			    (255 * 100000 + ((1 << SFT) - 1)))
+				printf("ttread: enlarge SFT\n");
+#endif
+			slp = ((((u_long)slp + ((1 << SFT) - 1)) >> SFT) * hz +
+			    ((1000000 >> SFT) - 1)) / (1000000 >> SFT);
+#undef SFT
 			goto sleep;
 		}
 	} else if ((qp = &tp->t_canq)->c_cc <= 0) {

	Any better fix?

>Release-Note:
>Audit-Trail:
>Unformatted: