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

Willy Tarreau
2005-09-25T08:06:53Z

Hi,

On Sun, Sep 25, 2005 at 12:08:03AM -0700, Vadim Lobanov wrote:
(...)
> > +/* Maximum msec timeout value storeable in a long int */
> > +#define EP_MAX_MSTIMEO min(1000ULL * MAX_SCHEDULE_TIMEOUT / HZ, LONG_MAX / HZ - 1000ULL)
>
> This should instead be:
> #define EP_MAX_MSTIMEO min(1000ULL * MAX_SCHEDULE_TIMEOUT / HZ, (LONG_MAX - 999ULL) / HZ)
> Here's why:
> We want to avoid overflow of (timeout * HZ + 999), or, in other words,
> the case where (timeout * HZ + 999) >= LONG_MAX
> Unwrapping the equation, we get timeout >= (LONG_MAX - 999) / HZ
>
> The original code isn't _wrong_, but more restrictive than it should be.
> In any case, better to fix up the base patch now, before all the other
> patches go in. I could do this, or Davide can... it's all good. :-)

I think it's because with the numerous changes we brought, the '>' test
became '>=' but the old timeout was still used with '>'. With '>=', I
agree with you that it must be -999.

Andrew, Vadim is right. Anyway, this proves why we must really move all
those complicated tests to jiffies.h ASAP !

BTW, Andrew, could you merge the jiffies fix before -mm3, so that we can
remove those annoying tests quickly ?

Thanks in advance,
Willy