Hi,
On Sat, Sep 24, 2005 at 09:52:05PM +0200, Willy Tarreau wrote:
> This patch simplifies the overflow fix which was applied to sys_poll() in
> 2.6.14-rc2-mm1 by relying on the overflow detection code added to jiffies.h
> by patch 1/3. This also removes a useless divide for HZ <= 1000.
>
> It might be worth applying too. It's only for -mm1, and does *not* apply
> to 2.6.14-rc2 because this code has already received patches in -mm1.
I noticed that the overflow check in the original code is either useless or
buggy, so this patch is even more worth applying. Look below at original
code : if HZ <= 1000, the overflow is set only when
msecs_to_jiffies() >= MAX_SCHEDULE_TIMEOUT.
But the maximal value that msecs_to_jiffies() can return is MAX_JIFFY_OFFSET.
Guess what ?
include/linux/jiffies.h:#define MAX_JIFFY_OFFSET ((~0UL >> 1)-1)
include/linux/sched.h:#define MAX_SCHEDULE_TIMEOUT LONG_MAX
include/linux/kernel.h:#define LONG_MAX ((long)(~0UL>>1))
=> MAX_JIFFY_OFFSET == (MAX_SCHEDULE_TIMEOUT - 1)
=> So msecs_to_jiffies() cannot be >= MAX_SCHEDULE_TIMEOUT, and 'overflow'
can never be set for HZ <= 1000.
Given how msecs_to_jiffies() is used everywhere, I wonder if we should not
provide a separate overflow detection function which would be used where
needed, and possibly remove the test from msecs_to_jiffies() which is mostly
called with constants or small delays which will never overflow.
Has anyone an opinion on this ?
Regards,
Willy
> diff -purN linux-2.6.14-rc2-mm1/fs/select.c linux-2.6.14-rc2-mm1-poll/fs/select.c
> --- linux-2.6.14-rc2-mm1/fs/select.c Sat Sep 24 21:12:36 2005
> +++ linux-2.6.14-rc2-mm1-poll/fs/select.c Sat Sep 24 21:23:21 2005
> @@ -469,7 +469,6 @@ asmlinkage long sys_poll(struct pollfd _
> {
> struct poll_wqueues table;
> int fdcount, err;
> - int overflow;
> unsigned int i;
> struct poll_list *head;
> struct poll_list *walk;
> @@ -486,22 +485,11 @@ asmlinkage long sys_poll(struct pollfd _
> return -EINVAL;
>
> /*
> - * We compare HZ with 1000 to work out which side of the
> - * expression needs conversion. Because we want to avoid
> - * converting any value to a numerically higher value, which
> - * could overflow.
> - */
> -#if HZ > 1000
> - overflow = timeout_msecs >= jiffies_to_msecs(MAX_SCHEDULE_TIMEOUT);
> -#else
> - overflow = msecs_to_jiffies(timeout_msecs) >= MAX_SCHEDULE_TIMEOUT;
> -#endif
> -
> - /*
> * If we would overflow in the conversion or a negative timeout
> - * is requested, sleep indefinitely.
> + * is requested, sleep indefinitely. Note: msecs_to_jiffies checks
> + * for the overflow.
> */
> - if (overflow || timeout_msecs < 0)
> + if (timeout_msecs < 0)
> timeout_jiffies = MAX_SCHEDULE_TIMEOUT;
> else
> timeout_jiffies = msecs_to_jiffies(timeout_msecs) + 1;
>
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
