On Thu, 2006-05-18 at 13:48 -0700, Zachary Amsden wrote:
> It also looks like s390 has another bug. When compiling the 32-bit
> kernel, doesn't this computation overflow:
>
> arch/s390/kernel/time.c, stop_hz_timer:274
>
> /*
> * This cpu is going really idle. Set up the clock comparator
> * for the next event.
> */
> next = next_timer_interrupt();
> do {
> seq = read_seqbegin_irqsave(&xtime_lock, flags);
> timer = (__u64)(next - jiffies) + jiffies_64;
> } while (read_seqretry_irqrestore(&xtime_lock, seq, flags));
>
>
> Since jiffies can advance between next_timer_interrupt and the read
> under xtime lock, next-jiffies could be negative. I would think you
> want to cast that to signed long instead of __u64, but I'm not totally
> qualified to talk about s390.
Seems like you are qualified to talk about s390 in this case. The
extension of (next - jiffies) to a 64 bit value needs to be done as a
signed extension, follow by a cast to u64. Blech. I think to cast next
and jiffies to u64 before subtracting them is cleaner. It takes a few
more cycles because we now do two 64 bit adds/subtracts but the code is
used for going idle so it doesn't matter. Patch attached, thanks Zach.
--
blue skies,
Martin.
Martin Schwidefsky
Linux for zSeries Development & Services
IBM Deutschland Entwicklung GmbH
"Reality continues to ruin my life." - Calvin.
--
From: Martin Schwidefsky <[email protected]>
[patch] s390: next_timer_interrupt overflow in stop_hz_timer.
The 32 bit unsigned substraction (next - jiffies) in stop_hz_timer
can overflow if jiffies gets advanced between next_timer_interrupt
and the read under the xtime lock. The cast to a u64 then results
in a large value which causes the cpu to wait too long.
Fix this by casting next and jiffies independently to u64 before
subtracting them.
Signed-off-by: Martin Schwidefsky <[email protected]>
---
arch/s390/kernel/time.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
diff -urpN linux-2.6/arch/s390/kernel/time.c linux-2.6-patched/arch/s390/kernel/time.c
--- linux-2.6/arch/s390/kernel/time.c 2006-05-16 09:44:29.000000000 +0200
+++ linux-2.6-patched/arch/s390/kernel/time.c 2006-05-19 11:04:04.000000000 +0200
@@ -272,7 +272,7 @@ static inline void stop_hz_timer(void)
next = next_timer_interrupt();
do {
seq = read_seqbegin_irqsave(&xtime_lock, flags);
- timer = (__u64)(next - jiffies) + jiffies_64;
+ timer = (__u64 next) - (__u64 jiffies) + jiffies_64;
} while (read_seqretry_irqrestore(&xtime_lock, seq, flags));
todval = -1ULL;
/* Be careful about overflows. */
-
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]