Re: tty update speed regression (was: 2.6.14-rc2-mm1)

[Alexey Dobriyan]

On Thu, Sep 22, 2005 at 11:50:29PM +0400, Alexey Dobriyan wrote:
> I see regression in tty update speed with ADOM (ncurses based
> roguelike) [1].
> 
> Messages at the top ("goblin hits you") are printed slowly. An eye can
> notice letter after letter printing.
> 
> 2.6.14-rc2 is OK.
> 
> I'll try to revert tty-layer-buffering-revamp*.patch pieces and see if
> it'll change something.
> 
> [1] http://adom.de/adom/download/linux/adom-111-elf.tar.gz (binary only)

Scratch TTY revamp, the sucker is
fix-sys_poll-large-timeout-handling.patch

HZ=250 here.
------------------------------------------------------------------------
From: Nishanth Aravamudan <nacc@xxxxxxxxxx>

The @timeout parameter to sys_poll() is in milliseconds but we compare it
to (MAX_SCHEDULE_TIMEOUT / HZ), which is (jiffies/jiffies-per-sec) or
seconds.  That seems blatantly broken.  This led to improper overflow
checking for @timeout.  As Andrew Morton pointed out, the best fix is to to
check for potential overflow first, then either select an indefinite value
or convert @timeout.

To achieve this and clean-up the code, change the prototype of the sys_poll
to make it clear that the parameter is in milliseconds and introduce a
variable, timeout_jiffies to hold the corresonding jiffies value.

Signed-off-by: Nishanth Aravamudan <nacc@xxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxx>
---

 fs/select.c              |   36 ++++++++++++++++++++++++++----------
 include/linux/syscalls.h |    2 +-
 2 files changed, 27 insertions(+), 11 deletions(-)

diff -puN fs/select.c~fix-sys_poll-large-timeout-handling fs/select.c
--- devel/fs/select.c~fix-sys_poll-large-timeout-handling	2005-09-10 02:35:19.000000000 -0700
+++ devel-akpm/fs/select.c	2005-09-10 03:26:17.000000000 -0700
@@ -464,15 +464,18 @@ static int do_poll(unsigned int nfds,  s
 	return count;
 }
 
-asmlinkage long sys_poll(struct pollfd __user * ufds, unsigned int nfds, long timeout)
+asmlinkage long sys_poll(struct pollfd __user *ufds, unsigned int nfds,
+				long timeout_msecs)
 {
 	struct poll_wqueues table;
- 	int fdcount, err;
+	int fdcount, err;
+	int overflow;
  	unsigned int i;
 	struct poll_list *head;
  	struct poll_list *walk;
 	struct fdtable *fdt;
 	int max_fdset;
+	unsigned long timeout_jiffies;
 
 	/* Do a sanity check on nfds ... */
 	rcu_read_lock();
@@ -482,13 +485,26 @@ asmlinkage long sys_poll(struct pollfd _
 	if (nfds > max_fdset && nfds > OPEN_MAX)
 		return -EINVAL;
 
-	if (timeout) {
-		/* Careful about overflow in the intermediate values */
-		if ((unsigned long) timeout < MAX_SCHEDULE_TIMEOUT / HZ)
-			timeout = (unsigned long)(timeout*HZ+999)/1000+1;
-		else /* Negative or overflow */
-			timeout = MAX_SCHEDULE_TIMEOUT;
-	}
+	/*
+	 * We compare HZ with 1000 to work out which side of the
+	 * expression needs conversion.  Because we want to avoid
+	 * converting any value to a numerically higher value, which
+	 * could overflow.
+	 */
+#if HZ > 1000
+	overflow = timeout_msecs >= jiffies_to_msecs(MAX_SCHEDULE_TIMEOUT);
+#else
+	overflow = msecs_to_jiffies(timeout_msecs) >= MAX_SCHEDULE_TIMEOUT;
+#endif
+
+	/*
+	 * If we would overflow in the conversion or a negative timeout
+	 * is requested, sleep indefinitely.
+	 */
+	if (overflow || timeout_msecs < 0)
+		timeout_jiffies = MAX_SCHEDULE_TIMEOUT;
+	else
+		timeout_jiffies = msecs_to_jiffies(timeout_msecs) + 1;
 
 	poll_initwait(&table);
 
@@ -519,7 +535,7 @@ asmlinkage long sys_poll(struct pollfd _
 		}
 		i -= pp->len;
 	}
-	fdcount = do_poll(nfds, head, &table, timeout);
+	fdcount = do_poll(nfds, head, &table, timeout_jiffies);
 
 	/* OK, now copy the revents fields back to user space. */
 	walk = head;
diff -puN include/linux/syscalls.h~fix-sys_poll-large-timeout-handling include/linux/syscalls.h
--- devel/include/linux/syscalls.h~fix-sys_poll-large-timeout-handling	2005-09-10 02:35:19.000000000 -0700
+++ devel-akpm/include/linux/syscalls.h	2005-09-10 02:35:19.000000000 -0700
@@ -420,7 +420,7 @@ asmlinkage long sys_socketpair(int, int,
 asmlinkage long sys_socketcall(int call, unsigned long __user *args);
 asmlinkage long sys_listen(int, int);
 asmlinkage long sys_poll(struct pollfd __user *ufds, unsigned int nfds,
-				long timeout);
+				long timeout_msecs);
 asmlinkage long sys_select(int n, fd_set __user *inp, fd_set __user *outp,
 			fd_set __user *exp, struct timeval __user *tvp);
 asmlinkage long sys_epoll_create(int size);
_

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/