From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: discuss@x86-64.org
Subject: Re: 2.6.15-rc5-mm1 (x86_64-hpet-overflow.patch breaks resume from disk)
Date: Thu, 8 Dec 2005 23:35:49 +0100
User-Agent: KMail/1.9
Cc: "Jan Beulich" <JBeulich@novell.com>, "Andrew Morton" <akpm@osdl.org>,
       "Andi Kleen" <ak@suse.de>, linux-kernel@vger.kernel.org
References: <20051204232153.258cd554.akpm@osdl.org> <43980058.76F0.0078.0@novell.com> <200512081153.51397.rjw@sisk.pl>
In-Reply-To: <200512081153.51397.rjw@sisk.pl>
MIME-Version: 1.0
Content-Type: text/plain;
  charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
Message-Id: <200512082335.50417.rjw@sisk.pl>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3140
Lines: 78

Update:

On Thursday, 8 December 2005 11:53, Rafael J. Wysocki wrote:
> On Thursday, 8 December 2005 09:43, Jan Beulich wrote:
> > I don't know how resume normally handles the re-syncing of the wall
> > clock, but the problem here is obvious: do_timer runs a loop to
> > increment jiffies, which may require significant amounts of time
> > (depending how long the system was sleeping).
> > 
> > Whatever mechanism was previously used to adjust the wall clock during
> > resume, this (a) has to happen before enabling interrupts and (b) has to
> > communicate to the timer interrupt handler to adjust its last time stamp
> > taken (to compare against in the next run). Clearly, the code was broken
> > already before, as it doesn't handle the resume case (where the HPET
> > value read is entirely unrelated to the one read during the last timer
> > interrupt before suspend) at all, and even in the TSC timer case it
> > simply checks whether the TSC delta is negative (which is not a valid
> > condition, as, between the booting of the system and the OS resume there
> > may elapse more time than the system was running altogether prior to
> > suspend).
> 
> Well, I'm not an expert, but I think I understand your argumentation.
> However, the problem is that resume _works_ without the patch
> and doesn't work with it, which is a regression.  (BTW, without
> the patch the wall clock is evidently correct after resume.)
> 
> Frankly I don't know who should fix the broken code,

FWIW, I have tried to fix it myself.

The appended patch seems to work on my box, but I'm afraid
it's not the right fix (at least in general).  Please advise.

Greetings,
Rafael


 arch/x86_64/kernel/time.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+)

Index: linux-2.6.15-rc5-mm1/arch/x86_64/kernel/time.c
===================================================================
--- linux-2.6.15-rc5-mm1.orig/arch/x86_64/kernel/time.c	2005-12-08 21:39:29.000000000 +0100
+++ linux-2.6.15-rc5-mm1/arch/x86_64/kernel/time.c	2005-12-08 22:44:48.000000000 +0100
@@ -1117,6 +1117,7 @@
 	unsigned long sec;
 	unsigned long ctime = get_cmos_time();
 	unsigned long sleep_length = (ctime - sleep_start) * HZ;
+	long offset = 0;
 
 	if (vxtime.hpet_address)
 		hpet_reenable();
@@ -1125,6 +1126,20 @@
 
 	sec = ctime + clock_cmos_diff;
 	write_seqlock_irqsave(&xtime_lock,flags);
+	if (vxtime.hpet_address)
+		offset = hpet_readl(HPET_COUNTER);
+	if (hpet_use_timer) {
+		unsigned int hi1 = hpet64 > 0 ? hpet_readl(HPET_COUNTER+4) : 0;
+
+		offset = hpet_readl(HPET_T0_CMP) - hpet_tick;
+		if (hpet64 > 0)
+			offset += (long)(offset >= 0 ? hi1 : hpet_readl(HPET_COUNTER+4)) << 32;
+		else
+			offset = (unsigned int)offset;
+	}
+	if (vxtime.mode == VXTIME_HPET)
+		vxtime.last = offset;
+	rdtscll_sync(&vxtime.last_tsc);
 	xtime.tv_sec = sec;
 	xtime.tv_nsec = 0;
 	write_sequnlock_irqrestore(&xtime_lock,flags);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/