Re: arca-24 [Re: new arca-23 released]

Linus Torvalds (torvalds@transmeta.com)
Fri, 20 Nov 1998 08:25:04 -0800 (PST)

On Fri, 20 Nov 1998, Andrea Arcangeli wrote:
> @@ -16,21 +17,47 @@
> * change timeval to jiffies, trying to avoid the
> * most obvious overflows..
> */
> -static __inline__ unsigned long
> +static __inline__ long
> timespec_to_jiffies(struct timespec *value)
> {
> - unsigned long sec = value->tv_sec;
> - long nsec = value->tv_nsec;
> + long sec = value->tv_sec, nsec = value->tv_nsec, retval;
>
> - if (sec > ((long)(~0UL >> 1) / HZ))
> - return ~0UL >> 1;
> nsec += 1000000000L / HZ - 1;
> nsec /= 1000000000L / HZ;
> - return HZ * sec + nsec;
> + retval = HZ * sec + nsec;
> + /*
> + * Here we are careful to return a positive value. This helps
> + * when the retval will be a parameter of schedule_timeout()
> + */
> + if (retval < 0)
> + return LONG_MAX;
> + else
> + return retval;
> }

This is still not right.

The reason we have the

if (sec > ((long)(~0UL >> 1) / HZ))

test early is that it's _not_ sufficient to test the sign after the
multiplication. The multiplication by HZ may overflow a _lot_ (ie wrap
around multiple times in whatever the word size of the machine is), and
the overflow is incorrect - even if that overflow happened to be positive.

So the right (as far as I can tell) timespec function is to test
beforehand:

static __inline__ unsigned long
timespec_to_jiffies(struct timespec *value)
{
unsigned long sec = value->tv_sec;
long nsec = value->tv_nsec;

if (sec >= (MAX_JIFFY_OFFSET / HZ))
return MAX_JIFFY_OFFSET;
nsec += 1000000000L / HZ - 1;
nsec /= 1000000000L / HZ;
return HZ * sec + nsec;
}

(You can test after the multiply too: you could test something like

mult = HZ * sec + nsec;
if (mult / HZ < sec)
return MAX_JIFFY_OFFSET;
return mult;

instead, but that would imply a division that can't be done at compile
time (which can be turned into a multiply-high, but the point being that
it's still "expensive").

And no, I don't guarantee that the above is right either. Maybe
MAX_JIFFY_OFFSET should be MAX_LONG-1-(1000000/HZ), to take care of the
nsec case causing problems..

Yes, getting all the overflow cases right is really fairly nasty.

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/