Bug 8479 - gettimeofday returning 1000000 in tv_usec on core2duo
Summary: gettimeofday returning 1000000 in tv_usec on core2duo
Status: CLOSED CODE_FIX
Alias: None
Product: Platform Specific/Hardware
Classification: Unclassified
Component: x86-64 (show other bugs)
Hardware: i386 Linux
: P2 normal
Assignee: Andi Kleen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-05-14 21:19 UTC by Daniel Gryniewicz
Modified: 2008-01-02 06:40 UTC (History)
1 user (show)

See Also:
Kernel Version: 2.6.21
Subsystem:
Regression: ---
Bisected commit-id:


Attachments
test program, used to get output. (340 bytes, text/plain)
2007-05-14 21:21 UTC, Daniel Gryniewicz
Details
.config from failing 2.6.21 kernel (37.84 KB, text/plain)
2007-05-14 21:24 UTC, Daniel Gryniewicz
Details

Description Daniel Gryniewicz 2007-05-14 21:19:58 UTC
Most recent kernel where this bug did *NOT* occur: 2.6.20
Distribution: Gentoo
Hardware Environment: core2duo T7200 (all reporters had this same CPU)
Software Environment: Linux 2.6.21, glibc 2.5
Problem Description:

gettimeofday returns 1 - 1000000 in tv_usec, not 0 - 999999 
This does not happen on any of my AMD-based 32 or 64 bit boxes, only on my
core2duo; I have 2 other reports of this problem, all on T7200's

Steps to reproduce:

call gettimeofday a lot.  Eventually, you'll get 1000000 returned in tv_usec. My
average is ~1 in 1000000 calls.  I've attached my test program, with output from
various boxes.  One of the other reporters tried the test program too, and got
similar output.  .config will be attached too.
Comment 1 Daniel Gryniewicz 2007-05-14 21:21:02 UTC
Created attachment 11501 [details]
test program, used to get output.
Comment 2 Daniel Gryniewicz 2007-05-14 21:23:51 UTC
Here's my output from running the program on the boxes I have available:

athena (core2duo, amd64)
    glibc-2.5-r2
    kernel 2.6.21-ck1
    done 10000000
    big 9 small 0

    glibc-2.5-r2
    kernel 2.6.21
    done 10000000
    big 13 small 0

    glibc-2.5-r2
    kernel 2.6.20
    done 10000000
    big 0 small 9

nemesis (athlon xp, x86)
    glibc-2.3.6-r4
    kernel 2.6.16-ck3
    done 10000000
    big 0 small 12

poseidon (sempron, amd64)
    glibc-2.5
    kernel 2.6.15-gentoo-r1
    done 10000000
    big 0 small 8

thanatos (athlon X2, amd64)
    glibc-2.5-r2
    kernel 2.6.20-ck1
    done 10000000
    big 0 small 13

    glibc-2.5-r2
    kernel 2.6.21
    done 10000000
    big 0 small 12

    glibc-2.5-r2
    kernel 2.6.21-ck1-r1
    done 10000000
    big 0 small 10

I consistently get overruns on my core2duo on 2.6.21 (and I *never* get 0
returned for tv_usec) and I always get valid values for all other combinations
of hardware and kernels.
Comment 3 Daniel Gryniewicz 2007-05-14 21:24:39 UTC
Created attachment 11502 [details]
.config from failing 2.6.21 kernel
Comment 4 Anonymous Emailer 2007-05-14 23:09:26 UTC
Reply-To: dada1@cosmosbay.com

Andrew Morton a 
Comment 5 Andi Kleen 2007-05-15 00:25:35 UTC
> >
> > I remember I already hit this and corrected it
> >
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blobdi
> >ff;f=arch/x86_64/kernel/vsyscall.c;h=dc32cef961950915fbaa185e36ab802d5f7ce
> >a3b;hp=ba330f87067996a17495f7d03466d646c718b52c;hb=c8118c6c07f2edfd697aaa0
> >b93e08c3b65a5a675;hpb=272a3713bb9e302e0455c894c41180a482d2c8a3
>
> Oh, OK.
>
> > Maybe a stable push is necessary ?
>
> yup.  Please always think of -stable when preparing fixes.  I'm sure many
> useful fixes are slipping past simply because those who _are_ looking out
> for backportable fixes are missing things.
>
> Greg, Chris: please consider c8118c6c07f2edfd697aaa0b93e08c3b65a5a675
> for -stable, if it isn't already there.

The full patch is overkill because 99% of it is an totally unrelated 
optimization. Only the > -> >= change should be backported

-Andi

Comment 6 Anonymous Emailer 2007-05-15 02:10:31 UTC
Reply-To: dada1@cosmosbay.com

On Tue, 15 May 2007 09:22:47 +0200
Andi Kleen <ak@suse.de> wrote:

> 
> > >
> > > I remember I already hit this and corrected it
> > >
> > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blobdi
> > >ff;f=arch/x86_64/kernel/vsyscall.c;h=dc32cef961950915fbaa185e36ab802d5f7ce
> > >a3b;hp=ba330f87067996a17495f7d03466d646c718b52c;hb=c8118c6c07f2edfd697aaa0
> > >b93e08c3b65a5a675;hpb=272a3713bb9e302e0455c894c41180a482d2c8a3
> >
> > Oh, OK.
> >
> > > Maybe a stable push is necessary ?
> >
> > yup.  Please always think of -stable when preparing fixes.  I'm sure many
> > useful fixes are slipping past simply because those who _are_ looking out
> > for backportable fixes are missing things.
> >
> > Greg, Chris: please consider c8118c6c07f2edfd697aaa0b93e08c3b65a5a675
> > for -stable, if it isn't already there.
> 
> The full patch is overkill because 99% of it is an totally unrelated 
> optimization. Only the > -> >= change should be backported

OK, here is the fix only patch, for linux-2.6.21-stable only, since 2.6.22 is already fixed.

[PATCH] x86_64 : Fix vgettimeofday()

vgettimeofday() may return some bad timeval values, (tv_usec = 1000000), because of a wrong compare.

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>

--- arch/x86_64/kernel/vsyscall.c
+++ arch/x86_64/kernel/vsyscall.c
@@ -132,7 +132,7 @@ static __always_inline void do_vgettimeo
 
 	/* convert to usecs and add to timespec: */
 	tv->tv_usec += nsec_delta / NSEC_PER_USEC;
-	while (tv->tv_usec > USEC_PER_SEC) {
+	while (tv->tv_usec >= USEC_PER_SEC) {
 		tv->tv_sec += 1;
 		tv->tv_usec -= USEC_PER_SEC;
 	}

Comment 7 Daniel Gryniewicz 2007-05-15 07:58:22 UTC
On Tue, 2007-05-15 at 10:17 +0200, Eric Dumazet wrote:
> OK, here is the fix only patch, for linux-2.6.21-stable only, since 2.6.22 is already fixed.
> 
> [PATCH] x86_64 : Fix vgettimeofday()
> 
> vgettimeofday() may return some bad timeval values, (tv_usec = 1000000), because of a wrong compare.
> 
> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
> 
> --- arch/x86_64/kernel/vsyscall.c
> +++ arch/x86_64/kernel/vsyscall.c
> @@ -132,7 +132,7 @@ static __always_inline void do_vgettimeo
>  
>  	/* convert to usecs and add to timespec: */
>  	tv->tv_usec += nsec_delta / NSEC_PER_USEC;
> -	while (tv->tv_usec > USEC_PER_SEC) {
> +	while (tv->tv_usec >= USEC_PER_SEC) {
>  		tv->tv_sec += 1;
>  		tv->tv_usec -= USEC_PER_SEC;
>  	}

That fixed it, thanks.

Daniel

Comment 8 Anonymous Emailer 2007-05-19 07:32:06 UTC
Reply-To: davidsen@tmr.com

Andrew Morton wrote:
> On Tue, 15 May 2007 08:06:52 +0200 Eric Dumazet <dada1@cosmosbay.com> wrote:
> 
>> Andrew Morton a 
Comment 9 Anonymous Emailer 2007-05-19 11:03:12 UTC
Reply-To: dada1@cosmosbay.com

Bill Davidsen a 

Note You need to log in before you can comment on or make changes to this bug.