Re: [patch] sys_epoll_wait() timeout saga ...

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, 24 Sep 2005, Willy Tarreau wrote:

On Sat, Sep 24, 2005 at 08:10:32AM -0700, Davide Libenzi wrote:
+       jtimeout = timeout < 0 || \
+                    timeout >= (1000ULL * MAX_SCHEDULE_TIMEOUT / HZ) ||
\
+                    timeout >= (LONG_MAX / HZ - 1000) ?
                 MAX_SCHEDULE_TIMEOUT: (timeout * HZ + 999) / 1000;

as both are constants, they can be optimized. Otherwise, we can resort to
using a MAX() macro to reduce this to only one test which will catch all
corner cases.

Using the MIN() macro would be better so we have a single check, and the
compiler optimize that automatically.

you're right, it's MIN() not MAX() ;-)
Anyway, I've checked the code and the compiler does a single test with -O2.

Or we can force 'timeout * HZ' to use ULL math. I don't think it makes a lot of difference for something that is in a likely sleep path ;)

"likely", yes, but not necessarily. Under a high load, you can have enough
events queued so that epoll() will not wait at all. I've already encountered
such cases during benchmarks, and I noticed that epoll() took more time than
select() for small numbers of FDs (something like 20% below 100 FDs), but of
course, it is considerably faster above. So turning the multiply to an ULL
may increase this overhead on some architectures, while the double check
will leave the code identical.

The attached patch uses the kernel min() macro, that is optimized has single compare by gcc-O2. Andrew, this goes over (hopefully ;) the bits you already have in -mm.

PS: It might be possible to move the currently epoll-local EP_MAX_MSTIMEO macro, to a MAX_LONG_MSTIMEO (or whatever name you like) somewhere in the tree, to be used where needed.


Signed-off-by: Davide Libenzi <[email protected]>


- Davide


--- a/fs/eventpoll.c	2005-09-24 11:07:04.000000000 -0700
+++ b/fs/eventpoll.c	2005-09-24 11:11:06.000000000 -0700
@@ -101,6 +101,10 @@
 /* Maximum number of poll wake up nests we are allowing */
 #define EP_MAX_POLLWAKE_NESTS 4
 
+/* Maximum msec timeout value storeable in a long int */
+#define EP_MAX_MSTIMEO min(1000ULL * MAX_SCHEDULE_TIMEOUT / HZ, LONG_MAX / HZ - 1000ULL)
+
+
 struct epoll_filefd {
 	struct file *file;
 	int fd;
@@ -1507,8 +1511,7 @@
 	 * and the overflow condition. The passed timeout is in milliseconds,
 	 * that why (t * HZ) / 1000.
 	 */
-	jtimeout = (timeout < 0 ||
-		    (timeout / 1000) >= (MAX_SCHEDULE_TIMEOUT / HZ)) ?
+	jtimeout = (timeout < 0 || timeout >= EP_MAX_MSTIMEO) ?
 		MAX_SCHEDULE_TIMEOUT: (timeout * HZ + 999) / 1000;
 
 retry:

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]
  Powered by Linux