do_gettimeofday() fails to compensate for lost ticks

do_gettimeofday() needs to account for lost ticks before returning current time and it fails to do that. do_gettimeofday() on other architectures compensate for lost ticks correctly. Due to this bug, if you repeatedly do clock_settime() immediately followed by clock_gettime() and compare the time returned by clock_gettime() to the time set by clock_settime(), you will eventually see clock going backwards. I am attaching a test program from POSIX testsuite at the end that exposes this bug. Run this test in a continuous loop that stops when test fails.

Following patch should address this issue.

--
Khalid

===========

@@ -132,6 +132,7 @@
 
 		sec = xtime.tv_sec;
 		usec += xtime.tv_usec;
+		usec += (jiffies - wall_jiffies) * (1000000 / HZ);
 	}
 	read_unlock_irqrestore(&xtime_lock, flags);
 

========= Test program =========
/*
* Copyright (c) 2002, Intel Corporation. All rights reserved.
* Created by: julie.n.fleischer REMOVE-THIS AT intel DOT com
* This file is licensed under the GPL license. For the full content
* of this license, see the COPYING file at the top level of this
* source tree.

* Test that clock_settime() sets clock_id to tp.
*
* The clock_id chosen for this test is CLOCK_REALTIME.
* The date chosen is Nov 12, 2002 ~11:13am (date when test was first
* written).
*/
#include <stdio.h>
#include <time.h>
/*#include "posixtest.h"
#include "helpers.h"*/

#define TESTTIME 1037128358
#define ACCEPTABLEDELTA 1
#define PTS_UNRESOLVED 2
#define PTS_PASS 0
#define PTS_FAIL 1

int getBeforeTime(struct timespec *tpget)

{
	if (clock_gettime(CLOCK_REALTIME, tpget) != 0) {
		perror("clock_gettime() did not return success\n");
		perror("clock may not be reset properly\n");
		return PTS_UNRESOLVED;
	}
	return PTS_PASS;
}

int setBackTime(struct timespec tpset)

{
	if (clock_settime(CLOCK_REALTIME, &tpset) != 0) {
		perror("clock_settime() did not return success\n");
		perror("clock may not be reset properly\n");
		return PTS_UNRESOLVED;
	}
	return PTS_PASS;
}

int main(int argc, char *argv[])

{
	struct timespec tpset, tpget, tpreset;
	int delta;

	getBeforeTime(&tpreset);

	tpset.tv_sec = TESTTIME;
	tpset.tv_nsec = 0;
	if (clock_settime(CLOCK_REALTIME, &tpset) = 0) {
		if (clock_gettime(CLOCK_REALTIME, &tpget) = -1) {
			perror("Error in clock_gettime()");
			return 2;
		}
		delta = tpget.tv_sec-tpset.tv_sec;
		if ( (delta <= ACCEPTABLEDELTA) && (delta >= 0) ) {
			printf("Test PASSED\n");
			setBackTime(tpreset);
			return 0;
		} else {
			printf("delta = %d, tpget=%d,%d, tpset=%d,%d\n", delta, tpget.tv_sec, tpget.tv_nsec, tpset.tv_sec, tpset.tv_nsec);
			printf("clock does not appear to be set\n");
			setBackTime(tpreset);
			return 1;
		}
	} else {
		printf("clock_settime() failed\n");
		return 2;
	}

	printf("This code should not be executed.\n");
	return 2;
}

Khalid> do_gettimeofday() needs to account for lost ticks before Khalid> returning current time and it fails to do
Khalid> that. do_gettimeofday() on other architectures compensate Khalid> for lost ticks correctly. Due to this bug, if you repeatedly Khalid> do clock_settime() immediately followed by clock_gettime() Khalid> and compare the time returned by clock_gettime() to the time Khalid> set by clock_settime(), you will eventually see clock going Khalid> backwards. I am attaching a test program from POSIX Khalid> testsuite at the end that exposes this bug. Run this test in Khalid> a continuous loop that stops when test fails.

This explanation doesn't sound right. See the "lost" variable in gettimeoffset(). It's supposed to account for lost ticks.

	--david

David,

So it would seem. What would then explain clock going backwards? Here is what I see when the test fails:

tpget37128357,995121000, tpset37128358,0

where tpget is what was returned by gettimeofday() and tpset is what was set by settimeofday(). Clock seems to have moved back by 4.879 msec.

Khalid

==================================
Khalid Aziz                                Linux and Open Source Lab
(970)898-9214                                        Hewlett-Packard
[email protected]                                       Fort Collins, CO

"The Linux kernel is subject to relentless development" 
				- Alessandro Rubini

Just FYI, Martin Hicks noticed recently that 2.6 on a machine with itc drift also has this problem. I think John Hawkes has some ideas on how to fix it, but I haven't had time to look at it yet.

Thanks,
Jesse

Khalid> So it would seem. What would then explain clock going Khalid> backwards? Here is what I see when the test fails:

Khalid> tpget37128357,995121000, tpset37128358,0

Khalid> where tpget is what was returned by gettimeofday() and tpset Khalid> is what was set by settimeofday(). Clock seems to have moved Khalid> back by 4.879 msec.

Not sure off-hand and I haven't looked at the 2.4 code-base in a while, but aren't we double-compensating in do_settimeofday()? There, we have:

		tv->tv_usec -= gettimeoffset();
		tv->tv_usec -= (jiffies - wall_jiffies) * (1000000 / HZ);

Perhaps the second line just needs to go?

	--david

I bet you are right. I will try taking that line out.

--
Khalid

==================================
Khalid Aziz Linux and Open Source Lab
(970)898-9214 Hewlett-Packard
[email protected] Fort Collins, CO

"The Linux kernel is subject to relentless development"
- Alessandro Rubini

That was it. The same test that failed in less than 30 seconds, now has been running for more than 10 minutes without failure. Thanks, David.

Here is a new patch.

--
Khalid

================--- linux-2.4.22/arch/ia64/kernel/time.c Mon Aug 25 05:44:39 2003
+++ linux-2.4.22-clock_fix/arch/ia64/kernel/time.c Wed Sep 24 12:38:20 2003
@@ -93,7 +93,6 @@
* it!
*/
tv->tv_usec -= gettimeoffset();
- tv->tv_usec -= (jiffies - wall_jiffies) * (1000000 / HZ);

while (tv->tv_usec < 0) {
tv->tv_usec += 1000000;

Khalid> That was it. The same test that failed in less than 30 Khalid> seconds, now has been running for more than 10 minutes Khalid> without failure.

Great! Thanks for tracking this down. I'm not surprised this bug has gone undetected so long---settimeofday() isn't really called all that often. I guess this shows the value of a good test-suite...

	--david