On Fri, 2007-03-16 at 12:43 -0800, Andrew Morton wrote:
> On Wed, 14 Mar 2007 11:00:12 +0100 Thomas Gleixner <[EMAIL PROTECTED]> wrote:
>
> > rtimer_forward() does not check for the possible overflow of
> > timer->expires. This can happen on 64 bit machines with large interval
> > values and results currently in an endless loop in the softirq because
> > the expiry value becomes negative and therefor the timer is expired all
> > the time.
> >
> > Check for this condition and set the expiry value to the max. expiry
> > time in the future.
> >
> > The fix should be applied to stable kernel series as well.
> >
> > Signed-off-by: Thomas Gleixner <[EMAIL PROTECTED],de>
> >
> > diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
> > index ec4cb9f..5e7122d 100644
> > --- a/kernel/hrtimer.c
> > +++ b/kernel/hrtimer.c
> > @@ -644,6 +644,12 @@ hrtimer_forward(struct hrtimer *timer, k
> > orun++;
> > }
> > timer->expires = ktime_add(timer->expires, interval);
> > + /*
> > + * Make sure, that the result did not wrap with a very large
> > + * interval.
> > + */
> > + if (timer->expires.tv64 < 0)
> > + timer->expires = ktime_set(KTIME_SEC_MAX, 0);
> >
> > return orun;
> > }
>
> kernel/hrtimer.c: In function 'hrtimer_forward':
> kernel/hrtimer.c:652: warning: overflow in implicit constant conversion
>
> problem is, KTIME_SEC_MAX is 9,223,372,036 and ktime_set() takes a `long'.
Stupid me :(
> This?
>
> --- a/include/linux/ktime.h~ktime_set-fix-arg-type
> +++ a/include/linux/ktime.h
> @@ -72,13 +72,13 @@ typedef union {
> *
> * Return the ktime_t representation of the value
> */
> -static inline ktime_t ktime_set(const long secs, const unsigned long nsecs)
> +static inline ktime_t ktime_set(const s64 secs, const unsigned long nsecs)
> {
> #if (BITS_PER_LONG == 64)
> if (unlikely(secs >= KTIME_SEC_MAX))
> return (ktime_t){ .tv64 = KTIME_MAX };
> #endif
> - return (ktime_t) { .tv64 = (s64)secs * NSEC_PER_SEC + (s64)nsecs };
> + return (ktime_t) { .tv64 = secs * NSEC_PER_SEC + (s64)nsecs };
> }
>
> /* Subtract two ktime_t variables. rem = lhs -rhs: */
> _
>
> I worry about that `secs >= KTIME_SEC_MAX' comparison in there, too. Both
> operands are signed.
I'd prefer this one: The maximum seconds value we can handle on 32bit is
LONG_MAX.
diff --git a/include/linux/ktime.h b/include/linux/ktime.h
index c68c7ac..248305b 100644
--- a/include/linux/ktime.h
+++ b/include/linux/ktime.h
@@ -57,7 +57,11 @@ typedef union {
} ktime_t;
#define KTIME_MAX ((s64)~((u64)1 << 63))
-#define KTIME_SEC_MAX (KTIME_MAX / NSEC_PER_SEC)
+#if (BITS_PER_LONG == 64)
+# define KTIME_SEC_MAX (KTIME_MAX / NSEC_PER_SEC)
+#else
+# define KTIME_SEC_MAX LONG_MAX
+#endif
/*
* ktime_t definitions when using the 64-bit scalar representation:
-
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/
