Messages in this thread Patch in this message | | | Date | Sat, 24 Sep 2005 11:19:17 -0700 (PDT) | From | Davide Libenzi <> | Subject | Re: [patch] sys_epoll_wait() timeout saga ... |
| |
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 <davidel@xmailserver.org>
- 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: | |