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

From: Willy TARREAU
Date: Sat Sep 24 2005 - 15:23:29 EST


On Sat, Sep 24, 2005 at 01:08:22PM -0700, Davide Libenzi wrote:
> On Sat, 24 Sep 2005, 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.
>
> Why all that code, when you can have it with:
>
> #define MAX_LONG_MSTIMEO (long) min(1000ULL * MAX_SCHEDULE_TIMEOUT / HZ,
> LONG_MAX / HZ - 1000ULL)
>
> that gcc-O2 collapses into a single constant?

It is because I wanted to ensure that it matched exactly the limits of the
functions that we call. And since msec_to_jiffies() is defined in 3 possible
ways depending on HZ, 1000 % HZ and HZ % 1000, there are 3 different limits.

Then, once the msec_to_jiffies() function is fixed, it's valuable to use it
in ep_poll(), because its worst case does exactly what you already have
(timeout * HZ + 999) / 1000, and other optimal cases can do better (for
HZ=100, 250, 1000, there will be no divide at all).

Don't worry, in my case, gcc also produces a single constant. That's just
that it depends on how it will be used (check include/linux/jiffies.h,
you'll understand what I mean).

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/