WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [PATCH] Fix a NO_IDLE_HZ timer bug

To: Zachary Amsden <zach@xxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH] Fix a NO_IDLE_HZ timer bug
From: Martin Schwidefsky <schwidefsky@xxxxxxxxxx>
Date: Fri, 19 May 2006 11:26:04 +0200
Cc: Andrew Morton <akpm@xxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Linux Kernel Mailing List <linux-kernel@xxxxxxxxxxxxxxx>, george@xxxxxxxxxx
Delivery-date: Fri, 19 May 2006 04:35:58 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <446CDDAE.1070908@xxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Organization: IBM Corporation
References: <446CDDAE.1070908@xxxxxxxxxx>
Reply-to: schwidefsky@xxxxxxxxxx
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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 <schwidefsky@xxxxxxxxxx>

[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 <schwidefsky@xxxxxxxxxx>
---

 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. */



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>