Re: [PATCH 0/3] fixes for overflow in poll(), epoll(), and msec_to_jiffies()

[Willy Tarreau]

On Sun, Sep 25, 2005 at 01:55:18PM -0700, Nishanth Aravamudan wrote:
> On 24.09.2005 [21:38:39 +0200], Willy Tarreau wrote:
> > Hello,
> > 
> > After the discussion around epoll() timeout, I noticed that the functions used
> > to detect the timeout could themselves overflow for some values of HZ.
> > 
> > So I decided to fix them by defining a macro which represents the maximal
> > acceptable argument which is guaranteed not to overflow. As an added bonus,
> > those functions can now be used in poll() and ep_poll() and remove the divide
> > if HZ == 1000, or replace it with a shift if (1000 % HZ) or (HZ % 1000) is a
> > power of two.
> > 
> > Patches against 2.6.14-rc2-mm1 sent as replies to this mail.
> 
> These look really good, Willy. Thanks for the fixes!

Thanks.
I noticed that there still existed a high risk of overflow with HZ values
for which (HZ % 1000) != 0 && (1000 % HZ) != 0, because we hit the only
situation where we start with a multiply by HZ, which is even worse as HZ
grows. The only cases I've found are :

  - alpha : 1024 or 1200
  - ia64  : 1024

Fortunately, they're both 64 bits, and since the result is an unsigned long,
the multiply returns 64 bits result so it won't overflow in a while. However,
we should avoid those combinations on 32-bits archs, because the limit is
then rather low. For example, using HZ=1200 on an x86 will set MAX_MSEC_OFFSET
to 3579138 ms, which is just below one hour. For reference, with HZ=1000,
MAX_MSEC_OFFSET would be 2^32-1 ms, or 49.7 days, which makes quite a
difference.

It is possible that some self-made setups hit the overflow before the patch.
For this reason, I wonder how we could discourage people from using such
bastard HZ values on 32 bits platforms :-/

Regards,
Willy

-
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/