linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] Time fixes and cleanups for 3.6
@ 2012-07-13  5:21 John Stultz
  2012-07-13  5:21 ` [PATCH 1/8] ntp: Fix STA_INS/DEL clearing bug John Stultz
                   ` (8 more replies)
  0 siblings, 9 replies; 34+ messages in thread
From: John Stultz @ 2012-07-13  5:21 UTC (permalink / raw)
  To: Linux Kernel
  Cc: John Stultz, Ingo Molnar, Peter Zijlstra, Richard Cochran,
	Prarit Bhargava, Thomas Gleixner

Hey Thomas,
	Since you're offline next week, I wanted to send you my updated
queue for the 3.6 merge window.

These are mostly the same changes I sent you with my earlier 3.6 pull
request mid-last month, but reordered and trimmed down to focus on
cleanups (no new features).

There is one NTP bug fix, from a change in 3.4, but given its non critical
and we're late in the 3.5-rc cycle, I wanted to hold it off for 3.6 and
backport it once merged.

Since these expect the leapsecond hrtimer/time changes currently in
tip/timers/urgent, it is based off of that branch, rather then
tip/timers/core. Although  tip/timers/core merges fairly cleanly (there's
only a minor collision with already upstream 3.5 changes).

Let me know if these look ok. With the exception of the NTP fix, the rest
can wait till 3.7 if you'd prefer, so they're nothing urgent.

thanks
-john

CC: Ingo Molnar <mingo@kernel.org>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Richard Cochran <richardcochran@gmail.com>
CC: Prarit Bhargava <prarit@redhat.com>
CC: Thomas Gleixner <tglx@linutronix.de>

If you want to git pull:
The following changes since commit 5baefd6d84163443215f4a99f6a20f054ef11236:

  hrtimer: Update hrtimer base offsets each hrtimer_interrupt (2012-07-11 23:34:39 +0200)

are available in the git repository at:

  git://git.linaro.org/people/jstultz/linux.git fortglx/3.6/time

for you to fetch changes up to 96dbf98929ac690a7d0d0ad54cf24631763da16c:

  time: Rework timekeeping functions to take timekeeper ptr as argument (2012-07-13 01:15:46 -0400)

----------------------------------------------------------------
John Stultz (8):
      ntp: Fix STA_INS/DEL clearing bug
      time: Whitespace cleanups per Ingo's requests
      time: Explicitly use u32 instead of int for shift values
      time: Condense timekeeper.xtime into xtime_sec
      time: Refactor accumulation of nsecs to secs
      time: Move arch_gettimeoffset() usage into timekeeping_get_ns()
      time: Move xtime_nsec adjustment underflow handling timekeeping_adjust
      time: Rework timekeeping functions to take timekeeper ptr as argument

 kernel/time/ntp.c         |    8 +-
 kernel/time/timekeeping.c |  492 ++++++++++++++++++++++++---------------------
 2 files changed, 273 insertions(+), 227 deletions(-)


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 1/8] ntp: Fix STA_INS/DEL clearing bug
  2012-07-13  5:21 [PATCH 0/8] Time fixes and cleanups for 3.6 John Stultz
@ 2012-07-13  5:21 ` John Stultz
  2012-07-13  5:58   ` Ingo Molnar
  2012-07-15  7:57   ` [tip:timers/urgent] " tip-bot for John Stultz
  2012-07-13  5:21 ` [PATCH 2/8] time: Whitespace cleanups per Ingo's requests John Stultz
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 34+ messages in thread
From: John Stultz @ 2012-07-13  5:21 UTC (permalink / raw)
  To: Linux Kernel
  Cc: John Stultz, Ingo Molnar, Peter Zijlstra, Richard Cochran,
	Prarit Bhargava, Thomas Gleixner, stable

From: John Stultz <johnstul@us.ibm.com>

In commit 6b43ae8a619d17c4935c3320d2ef9e92bdeed05d, I
introduced a bug that kept the STA_INS or STA_DEL bit
from being cleared from time_status via adjtimex()
without forcing STA_PLL first.

Usually once the STA_INS is set, it isn't cleared
until the leap second is applied, so its unlikely this
affected anyone. However during testing I noticed it
took some effort to cancel a leap second once STA_INS
was set.

This issue affects 3.4 and up.

Since this isn't urgent (issue is only observed in testing,
the behavior doesn't affect ntpd, nor is a leapsecond due
for at least ~6 months), and we're late in the 3.5-rc
cycle, I'm holding this off for 3.6 merge window,
where I'll then backport to 3.5-stable and 3.4-stable.

CC: Ingo Molnar <mingo@kernel.org>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Richard Cochran <richardcochran@gmail.com>
CC: Prarit Bhargava <prarit@redhat.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: stable@vger.kernel.org
Signed-off-by: John Stultz <johnstul@us.ibm.com>
---
 kernel/time/ntp.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 70b33ab..b7fbadc 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -409,7 +409,9 @@ int second_overflow(unsigned long secs)
 			time_state = TIME_DEL;
 		break;
 	case TIME_INS:
-		if (secs % 86400 == 0) {
+		if (!(time_status & STA_INS))
+			time_state = TIME_OK;
+		else if (secs % 86400 == 0) {
 			leap = -1;
 			time_state = TIME_OOP;
 			time_tai++;
@@ -418,7 +420,9 @@ int second_overflow(unsigned long secs)
 		}
 		break;
 	case TIME_DEL:
-		if ((secs + 1) % 86400 == 0) {
+		if (!(time_status & STA_DEL))
+			time_state = TIME_OK;
+		else if ((secs + 1) % 86400 == 0) {
 			leap = 1;
 			time_tai--;
 			time_state = TIME_WAIT;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 2/8] time: Whitespace cleanups per Ingo's requests
  2012-07-13  5:21 [PATCH 0/8] Time fixes and cleanups for 3.6 John Stultz
  2012-07-13  5:21 ` [PATCH 1/8] ntp: Fix STA_INS/DEL clearing bug John Stultz
@ 2012-07-13  5:21 ` John Stultz
  2012-07-15  8:56   ` [tip:timers/core] time: Whitespace cleanups per Ingo%27s requests tip-bot for John Stultz
  2012-07-13  5:21 ` [PATCH 3/8] time: Explicitly use u32 instead of int for shift values John Stultz
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: John Stultz @ 2012-07-13  5:21 UTC (permalink / raw)
  To: Linux Kernel
  Cc: John Stultz, Ingo Molnar, Peter Zijlstra, Richard Cochran,
	Prarit Bhargava, Thomas Gleixner

Ingo noted a number of places where there is inconsistent
use of whitespace. This patch tries to address the main
culprits.

CC: Ingo Molnar <mingo@kernel.org>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Richard Cochran <richardcochran@gmail.com>
CC: Prarit Bhargava <prarit@redhat.com>
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/timekeeping.c |   39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 269b1fe..c2f12aa 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -24,32 +24,31 @@
 /* Structure holding internal timekeeping values. */
 struct timekeeper {
 	/* Current clocksource used for timekeeping. */
-	struct clocksource *clock;
+	struct clocksource	*clock;
 	/* NTP adjusted clock multiplier */
-	u32	mult;
+	u32			mult;
 	/* The shift value of the current clocksource. */
-	int	shift;
-
+	int			shift;
 	/* Number of clock cycles in one NTP interval. */
-	cycle_t cycle_interval;
+	cycle_t			cycle_interval;
 	/* Number of clock shifted nano seconds in one NTP interval. */
-	u64	xtime_interval;
+	u64			xtime_interval;
 	/* shifted nano seconds left over when rounding cycle_interval */
-	s64	xtime_remainder;
+	s64			xtime_remainder;
 	/* Raw nano seconds accumulated per NTP interval. */
-	u32	raw_interval;
+	u32			raw_interval;
 
 	/* Clock shifted nano seconds remainder not stored in xtime.tv_nsec. */
-	u64	xtime_nsec;
+	u64			xtime_nsec;
 	/* Difference between accumulated time and NTP time in ntp
 	 * shifted nano seconds. */
-	s64	ntp_error;
+	s64			ntp_error;
 	/* Shift conversion between clock shifted nano seconds and
 	 * ntp shifted nano seconds. */
-	int	ntp_error_shift;
+	int			ntp_error_shift;
 
 	/* The current time */
-	struct timespec xtime;
+	struct timespec 	xtime;
 	/*
 	 * wall_to_monotonic is what we need to add to xtime (or xtime corrected
 	 * for sub jiffie times) to get to monotonic time.  Monotonic is pegged
@@ -64,20 +63,17 @@ struct timekeeper {
 	 * - wall_to_monotonic is no longer the boot time, getboottime must be
 	 * used instead.
 	 */
-	struct timespec wall_to_monotonic;
+	struct timespec		wall_to_monotonic;
 	/* time spent in suspend */
-	struct timespec total_sleep_time;
+	struct timespec		total_sleep_time;
 	/* The raw monotonic time for the CLOCK_MONOTONIC_RAW posix clock. */
-	struct timespec raw_time;
-
+	struct timespec		raw_time;
 	/* Offset clock monotonic -> clock realtime */
-	ktime_t offs_real;
-
+	ktime_t			offs_real;
 	/* Offset clock monotonic -> clock boottime */
-	ktime_t offs_boot;
-
+	ktime_t			offs_boot;
 	/* Seqlock for all timekeeper values */
-	seqlock_t lock;
+	seqlock_t		lock;
 };
 
 static struct timekeeper timekeeper;
@@ -547,6 +543,7 @@ u64 timekeeping_max_deferment(void)
 {
 	unsigned long seq;
 	u64 ret;
+
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 3/8] time: Explicitly use u32 instead of int for shift values
  2012-07-13  5:21 [PATCH 0/8] Time fixes and cleanups for 3.6 John Stultz
  2012-07-13  5:21 ` [PATCH 1/8] ntp: Fix STA_INS/DEL clearing bug John Stultz
  2012-07-13  5:21 ` [PATCH 2/8] time: Whitespace cleanups per Ingo's requests John Stultz
@ 2012-07-13  5:21 ` John Stultz
  2012-07-15  8:57   ` [tip:timers/core] " tip-bot for John Stultz
  2012-07-13  5:21 ` [PATCH 4/8] time: Condense timekeeper.xtime into xtime_sec John Stultz
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: John Stultz @ 2012-07-13  5:21 UTC (permalink / raw)
  To: Linux Kernel
  Cc: John Stultz, Ingo Molnar, Peter Zijlstra, Richard Cochran,
	Prarit Bhargava, Thomas Gleixner

Ingo noted that using a u32 instead of int for shift values
would be better to make sure the compiler doesn't unnecessarily
use complex signed arithmetic.

CC: Ingo Molnar <mingo@kernel.org>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Richard Cochran <richardcochran@gmail.com>
CC: Prarit Bhargava <prarit@redhat.com>
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/timekeeping.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index c2f12aa..4fd83df 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -28,7 +28,7 @@ struct timekeeper {
 	/* NTP adjusted clock multiplier */
 	u32			mult;
 	/* The shift value of the current clocksource. */
-	int			shift;
+	u32			shift;
 	/* Number of clock cycles in one NTP interval. */
 	cycle_t			cycle_interval;
 	/* Number of clock shifted nano seconds in one NTP interval. */
@@ -45,7 +45,7 @@ struct timekeeper {
 	s64			ntp_error;
 	/* Shift conversion between clock shifted nano seconds and
 	 * ntp shifted nano seconds. */
-	int			ntp_error_shift;
+	u32			ntp_error_shift;
 
 	/* The current time */
 	struct timespec 	xtime;
@@ -960,7 +960,7 @@ static void timekeeping_adjust(s64 offset)
  *
  * Returns the unconsumed cycles.
  */
-static cycle_t logarithmic_accumulation(cycle_t offset, int shift)
+static cycle_t logarithmic_accumulation(cycle_t offset, u32 shift)
 {
 	u64 nsecps = (u64)NSEC_PER_SEC << timekeeper.shift;
 	u64 raw_nsecs;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 4/8] time: Condense timekeeper.xtime into xtime_sec
  2012-07-13  5:21 [PATCH 0/8] Time fixes and cleanups for 3.6 John Stultz
                   ` (2 preceding siblings ...)
  2012-07-13  5:21 ` [PATCH 3/8] time: Explicitly use u32 instead of int for shift values John Stultz
@ 2012-07-13  5:21 ` John Stultz
  2012-07-13  6:00   ` Ingo Molnar
                     ` (2 more replies)
  2012-07-13  5:21 ` [PATCH 5/8] time: Refactor accumulation of nsecs to secs John Stultz
                   ` (4 subsequent siblings)
  8 siblings, 3 replies; 34+ messages in thread
From: John Stultz @ 2012-07-13  5:21 UTC (permalink / raw)
  To: Linux Kernel
  Cc: John Stultz, Ingo Molnar, Peter Zijlstra, Richard Cochran,
	Prarit Bhargava, Thomas Gleixner

The timekeeper struct has a xtime_nsec, which keeps the
sub-nanosecond remainder.  This ends up being somewhat
duplicative of the timekeeper.xtime.tv_nsec value, and we
have to do extra work to keep them apart, copying the full
nsec portion out and back in over and over.

This patch simplifies some of the logic by taking the timekeeper
xtime value and splitting it into timekeeper.xtime_sec and
reuses the timekeeper.xtime_nsec for the sub-second portion
(stored in higher res shifted nanoseconds).

This simplifies some of the accumulation logic. And will
allow for more accurate timekeeping once the vsyscall code
is updated to use the shifted nanosecond remainder.

CC: Ingo Molnar <mingo@kernel.org>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Richard Cochran <richardcochran@gmail.com>
CC: Prarit Bhargava <prarit@redhat.com>
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/timekeeping.c |  183 ++++++++++++++++++++++++++++-----------------
 1 file changed, 113 insertions(+), 70 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 4fd83df..80d0c78 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -38,8 +38,11 @@ struct timekeeper {
 	/* Raw nano seconds accumulated per NTP interval. */
 	u32			raw_interval;
 
-	/* Clock shifted nano seconds remainder not stored in xtime.tv_nsec. */
+	/* Current CLOCK_REALTIME time in seconds */
+	u64			xtime_sec;
+	/* Clock shifted nano seconds */
 	u64			xtime_nsec;
+
 	/* Difference between accumulated time and NTP time in ntp
 	 * shifted nano seconds. */
 	s64			ntp_error;
@@ -47,8 +50,6 @@ struct timekeeper {
 	 * ntp shifted nano seconds. */
 	u32			ntp_error_shift;
 
-	/* The current time */
-	struct timespec 	xtime;
 	/*
 	 * wall_to_monotonic is what we need to add to xtime (or xtime corrected
 	 * for sub jiffie times) to get to monotonic time.  Monotonic is pegged
@@ -89,6 +90,36 @@ __cacheline_aligned_in_smp DEFINE_SEQLOCK(xtime_lock);
 int __read_mostly timekeeping_suspended;
 
 
+static inline void tk_normalize_xtime(struct timekeeper *tk)
+{
+	while (tk->xtime_nsec >= ((u64)NSEC_PER_SEC << tk->shift)) {
+		tk->xtime_nsec -= (u64)NSEC_PER_SEC << tk->shift;
+		tk->xtime_sec++;
+	}
+}
+
+static struct timespec tk_xtime(struct timekeeper *tk)
+{
+	struct timespec ts;
+
+	ts.tv_sec = tk->xtime_sec;
+	ts.tv_nsec = (long)(tk->xtime_nsec >> tk->shift);
+	return ts;
+}
+
+static void tk_set_xtime(struct timekeeper *tk, const struct timespec *ts)
+{
+	tk->xtime_sec = ts->tv_sec;
+	tk->xtime_nsec = ts->tv_nsec << tk->shift;
+}
+
+
+static void tk_xtime_add(struct timekeeper *tk, const struct timespec *ts)
+{
+	tk->xtime_sec += ts->tv_sec;
+	tk->xtime_nsec += ts->tv_nsec << tk->shift;
+}
+
 
 /**
  * timekeeper_setup_internals - Set up internals to use clocksource clock.
@@ -104,7 +135,9 @@ static void timekeeper_setup_internals(struct clocksource *clock)
 {
 	cycle_t interval;
 	u64 tmp, ntpinterval;
+	struct clocksource *old_clock;
 
+	old_clock = timekeeper.clock;
 	timekeeper.clock = clock;
 	clock->cycle_last = clock->read(clock);
 
@@ -126,7 +159,14 @@ static void timekeeper_setup_internals(struct clocksource *clock)
 	timekeeper.raw_interval =
 		((u64) interval * clock->mult) >> clock->shift;
 
-	timekeeper.xtime_nsec = 0;
+	 /* if changing clocks, convert xtime_nsec shift units */
+	if (old_clock) {
+		int shift_change = clock->shift - old_clock->shift;
+		if (shift_change < 0)
+			timekeeper.xtime_nsec >>= -shift_change;
+		else
+			timekeeper.xtime_nsec <<= shift_change;
+	}
 	timekeeper.shift = clock->shift;
 
 	timekeeper.ntp_error = 0;
@@ -145,6 +185,7 @@ static inline s64 timekeeping_get_ns(void)
 {
 	cycle_t cycle_now, cycle_delta;
 	struct clocksource *clock;
+	s64 nsec;
 
 	/* read clocksource: */
 	clock = timekeeper.clock;
@@ -153,9 +194,8 @@ static inline s64 timekeeping_get_ns(void)
 	/* calculate the delta since the last update_wall_time: */
 	cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
 
-	/* return delta convert to nanoseconds using ntp adjusted mult. */
-	return clocksource_cyc2ns(cycle_delta, timekeeper.mult,
-				  timekeeper.shift);
+	nsec = cycle_delta * timekeeper.mult + timekeeper.xtime_nsec;
+	return nsec >> timekeeper.shift;
 }
 
 static inline s64 timekeeping_get_ns_raw(void)
@@ -185,12 +225,15 @@ static void update_rt_offset(void)
 /* must hold write on timekeeper.lock */
 static void timekeeping_update(bool clearntp)
 {
+	struct timespec xt;
+
 	if (clearntp) {
 		timekeeper.ntp_error = 0;
 		ntp_clear();
 	}
 	update_rt_offset();
-	update_vsyscall(&timekeeper.xtime, &timekeeper.wall_to_monotonic,
+	xt = tk_xtime(&timekeeper);
+	update_vsyscall(&xt, &timekeeper.wall_to_monotonic,
 			 timekeeper.clock, timekeeper.mult);
 }
 
@@ -213,13 +256,12 @@ static void timekeeping_forward_now(void)
 	cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
 	clock->cycle_last = cycle_now;
 
-	nsec = clocksource_cyc2ns(cycle_delta, timekeeper.mult,
-				  timekeeper.shift);
+	timekeeper.xtime_nsec += cycle_delta * timekeeper.mult;
 
 	/* If arch requires, add in gettimeoffset() */
-	nsec += arch_gettimeoffset();
+	timekeeper.xtime_nsec += arch_gettimeoffset() << timekeeper.shift;
 
-	timespec_add_ns(&timekeeper.xtime, nsec);
+	tk_normalize_xtime(&timekeeper);
 
 	nsec = clocksource_cyc2ns(cycle_delta, clock->mult, clock->shift);
 	timespec_add_ns(&timekeeper.raw_time, nsec);
@@ -234,15 +276,15 @@ static void timekeeping_forward_now(void)
 void getnstimeofday(struct timespec *ts)
 {
 	unsigned long seq;
-	s64 nsecs;
+	s64 nsecs = 0;
 
 	WARN_ON(timekeeping_suspended);
 
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
 
-		*ts = timekeeper.xtime;
-		nsecs = timekeeping_get_ns();
+		ts->tv_sec = timekeeper.xtime_sec;
+		ts->tv_nsec = timekeeping_get_ns();
 
 		/* If arch requires, add in gettimeoffset() */
 		nsecs += arch_gettimeoffset();
@@ -262,11 +304,10 @@ ktime_t ktime_get(void)
 
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
-		secs = timekeeper.xtime.tv_sec +
+		secs = timekeeper.xtime_sec +
 				timekeeper.wall_to_monotonic.tv_sec;
-		nsecs = timekeeper.xtime.tv_nsec +
+		nsecs = timekeeping_get_ns() +
 				timekeeper.wall_to_monotonic.tv_nsec;
-		nsecs += timekeeping_get_ns();
 		/* If arch requires, add in gettimeoffset() */
 		nsecs += arch_gettimeoffset();
 
@@ -291,22 +332,21 @@ void ktime_get_ts(struct timespec *ts)
 {
 	struct timespec tomono;
 	unsigned int seq;
-	s64 nsecs;
 
 	WARN_ON(timekeeping_suspended);
 
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
-		*ts = timekeeper.xtime;
+		ts->tv_sec = timekeeper.xtime_sec;
+		ts->tv_nsec = timekeeping_get_ns();
 		tomono = timekeeper.wall_to_monotonic;
-		nsecs = timekeeping_get_ns();
 		/* If arch requires, add in gettimeoffset() */
-		nsecs += arch_gettimeoffset();
+		ts->tv_nsec += arch_gettimeoffset();
 
 	} while (read_seqretry(&timekeeper.lock, seq));
 
 	set_normalized_timespec(ts, ts->tv_sec + tomono.tv_sec,
-				ts->tv_nsec + tomono.tv_nsec + nsecs);
+				ts->tv_nsec + tomono.tv_nsec);
 }
 EXPORT_SYMBOL_GPL(ktime_get_ts);
 
@@ -334,7 +374,8 @@ void getnstime_raw_and_real(struct timespec *ts_raw, struct timespec *ts_real)
 		seq = read_seqbegin(&timekeeper.lock);
 
 		*ts_raw = timekeeper.raw_time;
-		*ts_real = timekeeper.xtime;
+		ts_real->tv_sec = timekeeper.xtime_sec;
+		ts_real->tv_nsec = 0;
 
 		nsecs_raw = timekeeping_get_ns_raw();
 		nsecs_real = timekeeping_get_ns();
@@ -377,7 +418,7 @@ EXPORT_SYMBOL(do_gettimeofday);
  */
 int do_settimeofday(const struct timespec *tv)
 {
-	struct timespec ts_delta;
+	struct timespec ts_delta, xt;
 	unsigned long flags;
 
 	if ((unsigned long)tv->tv_nsec >= NSEC_PER_SEC)
@@ -387,12 +428,15 @@ int do_settimeofday(const struct timespec *tv)
 
 	timekeeping_forward_now();
 
-	ts_delta.tv_sec = tv->tv_sec - timekeeper.xtime.tv_sec;
-	ts_delta.tv_nsec = tv->tv_nsec - timekeeper.xtime.tv_nsec;
+	xt = tk_xtime(&timekeeper);
+	ts_delta.tv_sec = tv->tv_sec - xt.tv_sec;
+	ts_delta.tv_nsec = tv->tv_nsec - xt.tv_nsec;
+
 	timekeeper.wall_to_monotonic =
 			timespec_sub(timekeeper.wall_to_monotonic, ts_delta);
 
-	timekeeper.xtime = *tv;
+	tk_set_xtime(&timekeeper, tv);
+
 	timekeeping_update(true);
 
 	write_sequnlock_irqrestore(&timekeeper.lock, flags);
@@ -422,7 +466,8 @@ int timekeeping_inject_offset(struct timespec *ts)
 
 	timekeeping_forward_now();
 
-	timekeeper.xtime = timespec_add(timekeeper.xtime, *ts);
+
+	tk_xtime_add(&timekeeper, ts);
 	timekeeper.wall_to_monotonic =
 				timespec_sub(timekeeper.wall_to_monotonic, *ts);
 
@@ -606,14 +651,12 @@ void __init timekeeping_init(void)
 		clock->enable(clock);
 	timekeeper_setup_internals(clock);
 
-	timekeeper.xtime.tv_sec = now.tv_sec;
-	timekeeper.xtime.tv_nsec = now.tv_nsec;
+	tk_set_xtime(&timekeeper, &now);
 	timekeeper.raw_time.tv_sec = 0;
 	timekeeper.raw_time.tv_nsec = 0;
-	if (boot.tv_sec == 0 && boot.tv_nsec == 0) {
-		boot.tv_sec = timekeeper.xtime.tv_sec;
-		boot.tv_nsec = timekeeper.xtime.tv_nsec;
-	}
+	if (boot.tv_sec == 0 && boot.tv_nsec == 0)
+		boot = tk_xtime(&timekeeper);
+
 	set_normalized_timespec(&timekeeper.wall_to_monotonic,
 				-boot.tv_sec, -boot.tv_nsec);
 	update_rt_offset();
@@ -646,7 +689,7 @@ static void __timekeeping_inject_sleeptime(struct timespec *delta)
 		return;
 	}
 
-	timekeeper.xtime = timespec_add(timekeeper.xtime, *delta);
+	tk_xtime_add(&timekeeper, delta);
 	timekeeper.wall_to_monotonic =
 			timespec_sub(timekeeper.wall_to_monotonic, *delta);
 	update_sleep_time(timespec_add(timekeeper.total_sleep_time, *delta));
@@ -742,7 +785,7 @@ static int timekeeping_suspend(void)
 	 * try to compensate so the difference in system time
 	 * and persistent_clock time stays close to constant.
 	 */
-	delta = timespec_sub(timekeeper.xtime, timekeeping_suspend_time);
+	delta = timespec_sub(tk_xtime(&timekeeper), timekeeping_suspend_time);
 	delta_delta = timespec_sub(delta, old_delta);
 	if (abs(delta_delta.tv_sec)  >= 2) {
 		/*
@@ -977,9 +1020,9 @@ static cycle_t logarithmic_accumulation(cycle_t offset, u32 shift)
 	while (timekeeper.xtime_nsec >= nsecps) {
 		int leap;
 		timekeeper.xtime_nsec -= nsecps;
-		timekeeper.xtime.tv_sec++;
-		leap = second_overflow(timekeeper.xtime.tv_sec);
-		timekeeper.xtime.tv_sec += leap;
+		timekeeper.xtime_sec++;
+		leap = second_overflow(timekeeper.xtime_sec);
+		timekeeper.xtime_sec += leap;
 		timekeeper.wall_to_monotonic.tv_sec -= leap;
 		if (leap)
 			clock_was_set_delayed();
@@ -1015,6 +1058,7 @@ static void update_wall_time(void)
 	cycle_t offset;
 	int shift = 0, maxshift;
 	unsigned long flags;
+	s64 remainder;
 
 	write_seqlock_irqsave(&timekeeper.lock, flags);
 
@@ -1029,8 +1073,6 @@ static void update_wall_time(void)
 #else
 	offset = (clock->read(clock) - clock->cycle_last) & clock->mask;
 #endif
-	timekeeper.xtime_nsec = (s64)timekeeper.xtime.tv_nsec <<
-						timekeeper.shift;
 
 	/*
 	 * With NO_HZ we may have to accumulate many cycle_intervals
@@ -1076,28 +1118,31 @@ static void update_wall_time(void)
 		timekeeper.ntp_error += neg << timekeeper.ntp_error_shift;
 	}
 
-
 	/*
-	 * Store full nanoseconds into xtime after rounding it up and
-	 * add the remainder to the error difference.
-	 */
-	timekeeper.xtime.tv_nsec = ((s64)timekeeper.xtime_nsec >>
-						timekeeper.shift) + 1;
-	timekeeper.xtime_nsec -= (s64)timekeeper.xtime.tv_nsec <<
-						timekeeper.shift;
-	timekeeper.ntp_error +=	timekeeper.xtime_nsec <<
-				timekeeper.ntp_error_shift;
+	* Store only full nanoseconds into xtime_nsec after rounding
+	* it up and add the remainder to the error difference.
+	* XXX - This is necessary to avoid small 1ns inconsistnecies caused
+	* by truncating the remainder in vsyscalls. However, it causes
+	* additional work to be done in timekeeping_adjust(). Once
+	* the vsyscall implementations are converted to use xtime_nsec
+	* (shifted nanoseconds), this can be killed.
+	*/
+	remainder = timekeeper.xtime_nsec & ((1 << timekeeper.shift) - 1);
+	timekeeper.xtime_nsec -= remainder;
+	timekeeper.xtime_nsec += 1 << timekeeper.shift;
+	timekeeper.ntp_error += remainder << timekeeper.ntp_error_shift;
 
 	/*
 	 * Finally, make sure that after the rounding
-	 * xtime.tv_nsec isn't larger than NSEC_PER_SEC
+	 * xtime_nsec isn't larger than NSEC_PER_SEC
 	 */
-	if (unlikely(timekeeper.xtime.tv_nsec >= NSEC_PER_SEC)) {
+	if (unlikely(timekeeper.xtime_nsec >=
+			((u64)NSEC_PER_SEC << timekeeper.shift))) {
 		int leap;
-		timekeeper.xtime.tv_nsec -= NSEC_PER_SEC;
-		timekeeper.xtime.tv_sec++;
-		leap = second_overflow(timekeeper.xtime.tv_sec);
-		timekeeper.xtime.tv_sec += leap;
+		timekeeper.xtime_nsec -= (u64)NSEC_PER_SEC << timekeeper.shift;
+		timekeeper.xtime_sec++;
+		leap = second_overflow(timekeeper.xtime_sec);
+		timekeeper.xtime_sec += leap;
 		timekeeper.wall_to_monotonic.tv_sec -= leap;
 		if (leap)
 			clock_was_set_delayed();
@@ -1148,21 +1193,20 @@ void get_monotonic_boottime(struct timespec *ts)
 {
 	struct timespec tomono, sleep;
 	unsigned int seq;
-	s64 nsecs;
 
 	WARN_ON(timekeeping_suspended);
 
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
-		*ts = timekeeper.xtime;
+		ts->tv_sec = timekeeper.xtime_sec;
+		ts->tv_nsec = timekeeping_get_ns();
 		tomono = timekeeper.wall_to_monotonic;
 		sleep = timekeeper.total_sleep_time;
-		nsecs = timekeeping_get_ns();
 
 	} while (read_seqretry(&timekeeper.lock, seq));
 
 	set_normalized_timespec(ts, ts->tv_sec + tomono.tv_sec + sleep.tv_sec,
-			ts->tv_nsec + tomono.tv_nsec + sleep.tv_nsec + nsecs);
+			ts->tv_nsec + tomono.tv_nsec + sleep.tv_nsec);
 }
 EXPORT_SYMBOL_GPL(get_monotonic_boottime);
 
@@ -1195,13 +1239,13 @@ EXPORT_SYMBOL_GPL(monotonic_to_bootbased);
 
 unsigned long get_seconds(void)
 {
-	return timekeeper.xtime.tv_sec;
+	return timekeeper.xtime_sec;
 }
 EXPORT_SYMBOL(get_seconds);
 
 struct timespec __current_kernel_time(void)
 {
-	return timekeeper.xtime;
+	return tk_xtime(&timekeeper);
 }
 
 struct timespec current_kernel_time(void)
@@ -1212,7 +1256,7 @@ struct timespec current_kernel_time(void)
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
 
-		now = timekeeper.xtime;
+		now = tk_xtime(&timekeeper);
 	} while (read_seqretry(&timekeeper.lock, seq));
 
 	return now;
@@ -1227,7 +1271,7 @@ struct timespec get_monotonic_coarse(void)
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
 
-		now = timekeeper.xtime;
+		now = tk_xtime(&timekeeper);
 		mono = timekeeper.wall_to_monotonic;
 	} while (read_seqretry(&timekeeper.lock, seq));
 
@@ -1262,7 +1306,7 @@ void get_xtime_and_monotonic_and_sleep_offset(struct timespec *xtim,
 
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
-		*xtim = timekeeper.xtime;
+		*xtim = tk_xtime(&timekeeper);
 		*wtom = timekeeper.wall_to_monotonic;
 		*sleep = timekeeper.total_sleep_time;
 	} while (read_seqretry(&timekeeper.lock, seq));
@@ -1286,9 +1330,8 @@ ktime_t ktime_get_update_offsets(ktime_t *offs_real, ktime_t *offs_boot)
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
 
-		secs = timekeeper.xtime.tv_sec;
-		nsecs = timekeeper.xtime.tv_nsec;
-		nsecs += timekeeping_get_ns();
+		secs = timekeeper.xtime_sec;
+		nsecs = timekeeping_get_ns();
 		/* If arch requires, add in gettimeoffset() */
 		nsecs += arch_gettimeoffset();
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 5/8] time: Refactor accumulation of nsecs to secs
  2012-07-13  5:21 [PATCH 0/8] Time fixes and cleanups for 3.6 John Stultz
                   ` (3 preceding siblings ...)
  2012-07-13  5:21 ` [PATCH 4/8] time: Condense timekeeper.xtime into xtime_sec John Stultz
@ 2012-07-13  5:21 ` John Stultz
  2012-07-13  5:56   ` Ingo Molnar
                     ` (2 more replies)
  2012-07-13  5:21 ` [PATCH 6/8] time: Move arch_gettimeoffset() usage into timekeeping_get_ns() John Stultz
                   ` (3 subsequent siblings)
  8 siblings, 3 replies; 34+ messages in thread
From: John Stultz @ 2012-07-13  5:21 UTC (permalink / raw)
  To: Linux Kernel
  Cc: John Stultz, Ingo Molnar, Peter Zijlstra, Richard Cochran,
	Prarit Bhargava, Thomas Gleixner

We do the exact same logic moving nsecs to secs in the
timekeeper in multiple places, so condense this into a
single function.

CC: Ingo Molnar <mingo@kernel.org>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Richard Cochran <richardcochran@gmail.com>
CC: Prarit Bhargava <prarit@redhat.com>
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/timekeeping.c |   54 +++++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 22 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 80d0c78..5ffb2b6 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -995,6 +995,35 @@ static void timekeeping_adjust(s64 offset)
 
 
 /**
+ * accumulate_nsecs_to_secs - Accumulates nsecs into secs
+ *
+ * Helper function that accumulates a the nsecs greater then a second
+ * from the xtime_nsec field to the xtime_secs field.
+ * It also calls into the NTP code to handle leapsecond processing.
+ *
+ */
+static inline void accumulate_nsecs_to_secs(struct timekeeper *tk)
+{
+	u64 nsecps = (u64)NSEC_PER_SEC << tk->shift;
+
+	while (tk->xtime_nsec >= nsecps) {
+		int leap;
+
+		tk->xtime_nsec -= nsecps;
+		tk->xtime_sec++;
+
+		/* Figure out if its a leap sec and apply if needed */
+		leap = second_overflow(tk->xtime_sec);
+		tk->xtime_sec += leap;
+		tk->wall_to_monotonic.tv_sec -= leap;
+		if (leap)
+			clock_was_set_delayed();
+
+	}
+}
+
+
+/**
  * logarithmic_accumulation - shifted accumulation of cycles
  *
  * This functions accumulates a shifted interval of cycles into
@@ -1005,7 +1034,6 @@ static void timekeeping_adjust(s64 offset)
  */
 static cycle_t logarithmic_accumulation(cycle_t offset, u32 shift)
 {
-	u64 nsecps = (u64)NSEC_PER_SEC << timekeeper.shift;
 	u64 raw_nsecs;
 
 	/* If the offset is smaller than a shifted interval, do nothing */
@@ -1017,16 +1045,8 @@ static cycle_t logarithmic_accumulation(cycle_t offset, u32 shift)
 	timekeeper.clock->cycle_last += timekeeper.cycle_interval << shift;
 
 	timekeeper.xtime_nsec += timekeeper.xtime_interval << shift;
-	while (timekeeper.xtime_nsec >= nsecps) {
-		int leap;
-		timekeeper.xtime_nsec -= nsecps;
-		timekeeper.xtime_sec++;
-		leap = second_overflow(timekeeper.xtime_sec);
-		timekeeper.xtime_sec += leap;
-		timekeeper.wall_to_monotonic.tv_sec -= leap;
-		if (leap)
-			clock_was_set_delayed();
-	}
+
+	accumulate_nsecs_to_secs(&timekeeper);
 
 	/* Accumulate raw time */
 	raw_nsecs = timekeeper.raw_interval << shift;
@@ -1136,17 +1156,7 @@ static void update_wall_time(void)
 	 * Finally, make sure that after the rounding
 	 * xtime_nsec isn't larger than NSEC_PER_SEC
 	 */
-	if (unlikely(timekeeper.xtime_nsec >=
-			((u64)NSEC_PER_SEC << timekeeper.shift))) {
-		int leap;
-		timekeeper.xtime_nsec -= (u64)NSEC_PER_SEC << timekeeper.shift;
-		timekeeper.xtime_sec++;
-		leap = second_overflow(timekeeper.xtime_sec);
-		timekeeper.xtime_sec += leap;
-		timekeeper.wall_to_monotonic.tv_sec -= leap;
-		if (leap)
-			clock_was_set_delayed();
-	}
+	accumulate_nsecs_to_secs(&timekeeper);
 
 	timekeeping_update(false);
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 6/8] time: Move arch_gettimeoffset() usage into timekeeping_get_ns()
  2012-07-13  5:21 [PATCH 0/8] Time fixes and cleanups for 3.6 John Stultz
                   ` (4 preceding siblings ...)
  2012-07-13  5:21 ` [PATCH 5/8] time: Refactor accumulation of nsecs to secs John Stultz
@ 2012-07-13  5:21 ` John Stultz
  2012-07-13  6:03   ` Ingo Molnar
  2012-07-15  8:59   ` [tip:timers/core] " tip-bot for John Stultz
  2012-07-13  5:21 ` [PATCH 7/8] time: Move xtime_nsec adjustment underflow handling timekeeping_adjust John Stultz
                   ` (2 subsequent siblings)
  8 siblings, 2 replies; 34+ messages in thread
From: John Stultz @ 2012-07-13  5:21 UTC (permalink / raw)
  To: Linux Kernel
  Cc: John Stultz, Ingo Molnar, Peter Zijlstra, Richard Cochran,
	Prarit Bhargava, Thomas Gleixner, John Stultz

Since we call arch_gettimeoffset() in all the accessor
functions, move arch_gettimeoffset() calls into
timekeeping_get_ns() and timekeeping_get_ns_raw() to simplify
the code.

This also makes the code easier to maintain as we don't have to
worry about forgetting the arch_gettimeoffset() as has happened
in the past.

CC: Ingo Molnar <mingo@kernel.org>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Richard Cochran <richardcochran@gmail.com>
CC: Prarit Bhargava <prarit@redhat.com>
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <johnstul@us.ibm.com>
---
 kernel/time/timekeeping.c |   32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 5ffb2b6..dd119355 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -195,13 +195,18 @@ static inline s64 timekeeping_get_ns(void)
 	cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
 
 	nsec = cycle_delta * timekeeper.mult + timekeeper.xtime_nsec;
-	return nsec >> timekeeper.shift;
+	nsec >>= timekeeper.shift;
+
+	/* If arch requires, add in gettimeoffset() */
+	nsec += arch_gettimeoffset();
+	return nsec;
 }
 
 static inline s64 timekeeping_get_ns_raw(void)
 {
 	cycle_t cycle_now, cycle_delta;
 	struct clocksource *clock;
+	s64 nsec;
 
 	/* read clocksource: */
 	clock = timekeeper.clock;
@@ -210,8 +215,13 @@ static inline s64 timekeeping_get_ns_raw(void)
 	/* calculate the delta since the last update_wall_time: */
 	cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
 
-	/* return delta convert to nanoseconds. */
-	return clocksource_cyc2ns(cycle_delta, clock->mult, clock->shift);
+	/* convert delta to nanoseconds. */
+	nsec = clocksource_cyc2ns(cycle_delta, clock->mult, clock->shift);
+
+	/* If arch requires, add in gettimeoffset() */
+	nsec += arch_gettimeoffset();
+	return nsec;
+
 }
 
 static void update_rt_offset(void)
@@ -286,9 +296,6 @@ void getnstimeofday(struct timespec *ts)
 		ts->tv_sec = timekeeper.xtime_sec;
 		ts->tv_nsec = timekeeping_get_ns();
 
-		/* If arch requires, add in gettimeoffset() */
-		nsecs += arch_gettimeoffset();
-
 	} while (read_seqretry(&timekeeper.lock, seq));
 
 	timespec_add_ns(ts, nsecs);
@@ -308,8 +315,6 @@ ktime_t ktime_get(void)
 				timekeeper.wall_to_monotonic.tv_sec;
 		nsecs = timekeeping_get_ns() +
 				timekeeper.wall_to_monotonic.tv_nsec;
-		/* If arch requires, add in gettimeoffset() */
-		nsecs += arch_gettimeoffset();
 
 	} while (read_seqretry(&timekeeper.lock, seq));
 	/*
@@ -340,8 +345,6 @@ void ktime_get_ts(struct timespec *ts)
 		ts->tv_sec = timekeeper.xtime_sec;
 		ts->tv_nsec = timekeeping_get_ns();
 		tomono = timekeeper.wall_to_monotonic;
-		/* If arch requires, add in gettimeoffset() */
-		ts->tv_nsec += arch_gettimeoffset();
 
 	} while (read_seqretry(&timekeeper.lock, seq));
 
@@ -369,8 +372,6 @@ void getnstime_raw_and_real(struct timespec *ts_raw, struct timespec *ts_real)
 	WARN_ON_ONCE(timekeeping_suspended);
 
 	do {
-		u32 arch_offset;
-
 		seq = read_seqbegin(&timekeeper.lock);
 
 		*ts_raw = timekeeper.raw_time;
@@ -380,11 +381,6 @@ void getnstime_raw_and_real(struct timespec *ts_raw, struct timespec *ts_real)
 		nsecs_raw = timekeeping_get_ns_raw();
 		nsecs_real = timekeeping_get_ns();
 
-		/* If arch requires, add in gettimeoffset() */
-		arch_offset = arch_gettimeoffset();
-		nsecs_raw += arch_offset;
-		nsecs_real += arch_offset;
-
 	} while (read_seqretry(&timekeeper.lock, seq));
 
 	timespec_add_ns(ts_raw, nsecs_raw);
@@ -1342,8 +1338,6 @@ ktime_t ktime_get_update_offsets(ktime_t *offs_real, ktime_t *offs_boot)
 
 		secs = timekeeper.xtime_sec;
 		nsecs = timekeeping_get_ns();
-		/* If arch requires, add in gettimeoffset() */
-		nsecs += arch_gettimeoffset();
 
 		*offs_real = timekeeper.offs_real;
 		*offs_boot = timekeeper.offs_boot;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 7/8] time: Move xtime_nsec adjustment underflow handling timekeeping_adjust
  2012-07-13  5:21 [PATCH 0/8] Time fixes and cleanups for 3.6 John Stultz
                   ` (5 preceding siblings ...)
  2012-07-13  5:21 ` [PATCH 6/8] time: Move arch_gettimeoffset() usage into timekeeping_get_ns() John Stultz
@ 2012-07-13  5:21 ` John Stultz
  2012-07-13  6:04   ` Ingo Molnar
  2012-07-15  9:00   ` [tip:timers/core] " tip-bot for John Stultz
  2012-07-13  5:21 ` [PATCH 8/8] time: Rework timekeeping functions to take timekeeper ptr as argument John Stultz
  2012-07-13  6:05 ` [PATCH 0/8] Time fixes and cleanups for 3.6 Ingo Molnar
  8 siblings, 2 replies; 34+ messages in thread
From: John Stultz @ 2012-07-13  5:21 UTC (permalink / raw)
  To: Linux Kernel
  Cc: John Stultz, Ingo Molnar, Peter Zijlstra, Richard Cochran,
	Prarit Bhargava, Thomas Gleixner, John Stultz

When we make adjustments speeding up the clock, its possible
for xtime_nsec to underflow. We already handle this properly,
but we do so from update_wall_time() instead of the more logical
timekeeping_adjust(), where the possible underflow actually
occurs.

Thus, move the correction logic to the timekeeping_adjust, which
is the function that causes the issue. Making update_wall_time()
more readable.

CC: Ingo Molnar <mingo@kernel.org>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Richard Cochran <richardcochran@gmail.com>
CC: Prarit Bhargava <prarit@redhat.com>
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <johnstul@us.ibm.com>
---
 kernel/time/timekeeping.c |   42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index dd119355..4b76432 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -987,6 +987,27 @@ static void timekeeping_adjust(s64 offset)
 	timekeeper.xtime_nsec -= offset;
 	timekeeper.ntp_error -= (interval - offset) <<
 				timekeeper.ntp_error_shift;
+
+	/*
+	 * It may be possible that when we entered this function, xtime_nsec
+	 * was very small.  Further, if we're slightly speeding the clocksource
+	 * in the code above, its possible the required corrective factor to
+	 * xtime_nsec could cause it to underflow.
+	 *
+	 * Now, since we already accumulated the second, cannot simply roll
+	 * the accumulated second back, since the NTP subsystem has been
+	 * notified via second_overflow. So instead we push xtime_nsec forward
+	 * by the amount we underflowed, and add that amount into the error.
+	 *
+	 * We'll correct this error next time through this function, when
+	 * xtime_nsec is not as small.
+	 */
+	if (unlikely((s64)timekeeper.xtime_nsec < 0)) {
+		s64 neg = -(s64)timekeeper.xtime_nsec;
+		timekeeper.xtime_nsec = 0;
+		timekeeper.ntp_error += neg << timekeeper.ntp_error_shift;
+	}
+
 }
 
 
@@ -1112,27 +1133,6 @@ static void update_wall_time(void)
 	/* correct the clock when NTP error is too big */
 	timekeeping_adjust(offset);
 
-	/*
-	 * Since in the loop above, we accumulate any amount of time
-	 * in xtime_nsec over a second into xtime.tv_sec, its possible for
-	 * xtime_nsec to be fairly small after the loop. Further, if we're
-	 * slightly speeding the clocksource up in timekeeping_adjust(),
-	 * its possible the required corrective factor to xtime_nsec could
-	 * cause it to underflow.
-	 *
-	 * Now, we cannot simply roll the accumulated second back, since
-	 * the NTP subsystem has been notified via second_overflow. So
-	 * instead we push xtime_nsec forward by the amount we underflowed,
-	 * and add that amount into the error.
-	 *
-	 * We'll correct this error next time through this function, when
-	 * xtime_nsec is not as small.
-	 */
-	if (unlikely((s64)timekeeper.xtime_nsec < 0)) {
-		s64 neg = -(s64)timekeeper.xtime_nsec;
-		timekeeper.xtime_nsec = 0;
-		timekeeper.ntp_error += neg << timekeeper.ntp_error_shift;
-	}
 
 	/*
 	* Store only full nanoseconds into xtime_nsec after rounding
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 8/8] time: Rework timekeeping functions to take timekeeper ptr as argument
  2012-07-13  5:21 [PATCH 0/8] Time fixes and cleanups for 3.6 John Stultz
                   ` (6 preceding siblings ...)
  2012-07-13  5:21 ` [PATCH 7/8] time: Move xtime_nsec adjustment underflow handling timekeeping_adjust John Stultz
@ 2012-07-13  5:21 ` John Stultz
  2012-07-15  9:01   ` [tip:timers/core] " tip-bot for John Stultz
  2012-07-13  6:05 ` [PATCH 0/8] Time fixes and cleanups for 3.6 Ingo Molnar
  8 siblings, 1 reply; 34+ messages in thread
From: John Stultz @ 2012-07-13  5:21 UTC (permalink / raw)
  To: Linux Kernel
  Cc: John Stultz, Ingo Molnar, Peter Zijlstra, Richard Cochran,
	Prarit Bhargava, Thomas Gleixner

As part of cleaning up the timekeeping code, this patch converts
a number of internal functions to takei a timekeeper ptr as an
argument, so that the internal functions don't access the global
timekeeper structure directly. This allows for further optimizations
to reduce lock hold time later.

This patch has been updated to include more consistent usage of the
timekeeper value, by making sure it is always passed as a argument
to non top-level functions.

CC: Ingo Molnar <mingo@kernel.org>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Richard Cochran <richardcochran@gmail.com>
CC: Prarit Bhargava <prarit@redhat.com>
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/timekeeping.c |  210 ++++++++++++++++++++++-----------------------
 1 file changed, 104 insertions(+), 106 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 4b76432..7f08972 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -122,7 +122,7 @@ static void tk_xtime_add(struct timekeeper *tk, const struct timespec *ts)
 
 
 /**
- * timekeeper_setup_internals - Set up internals to use clocksource clock.
+ * tk_setup_internals - Set up internals to use clocksource clock.
  *
  * @clock:		Pointer to clocksource.
  *
@@ -131,14 +131,14 @@ static void tk_xtime_add(struct timekeeper *tk, const struct timespec *ts)
  *
  * Unless you're the timekeeping code, you should not be using this!
  */
-static void timekeeper_setup_internals(struct clocksource *clock)
+static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
 {
 	cycle_t interval;
 	u64 tmp, ntpinterval;
 	struct clocksource *old_clock;
 
-	old_clock = timekeeper.clock;
-	timekeeper.clock = clock;
+	old_clock = tk->clock;
+	tk->clock = clock;
 	clock->cycle_last = clock->read(clock);
 
 	/* Do the ns -> cycle conversion first, using original mult */
@@ -151,65 +151,65 @@ static void timekeeper_setup_internals(struct clocksource *clock)
 		tmp = 1;
 
 	interval = (cycle_t) tmp;
-	timekeeper.cycle_interval = interval;
+	tk->cycle_interval = interval;
 
 	/* Go back from cycles -> shifted ns */
-	timekeeper.xtime_interval = (u64) interval * clock->mult;
-	timekeeper.xtime_remainder = ntpinterval - timekeeper.xtime_interval;
-	timekeeper.raw_interval =
+	tk->xtime_interval = (u64) interval * clock->mult;
+	tk->xtime_remainder = ntpinterval - tk->xtime_interval;
+	tk->raw_interval =
 		((u64) interval * clock->mult) >> clock->shift;
 
 	 /* if changing clocks, convert xtime_nsec shift units */
 	if (old_clock) {
 		int shift_change = clock->shift - old_clock->shift;
 		if (shift_change < 0)
-			timekeeper.xtime_nsec >>= -shift_change;
+			tk->xtime_nsec >>= -shift_change;
 		else
-			timekeeper.xtime_nsec <<= shift_change;
+			tk->xtime_nsec <<= shift_change;
 	}
-	timekeeper.shift = clock->shift;
+	tk->shift = clock->shift;
 
-	timekeeper.ntp_error = 0;
-	timekeeper.ntp_error_shift = NTP_SCALE_SHIFT - clock->shift;
+	tk->ntp_error = 0;
+	tk->ntp_error_shift = NTP_SCALE_SHIFT - clock->shift;
 
 	/*
 	 * The timekeeper keeps its own mult values for the currently
 	 * active clocksource. These value will be adjusted via NTP
 	 * to counteract clock drifting.
 	 */
-	timekeeper.mult = clock->mult;
+	tk->mult = clock->mult;
 }
 
 /* Timekeeper helper functions. */
-static inline s64 timekeeping_get_ns(void)
+static inline s64 timekeeping_get_ns(struct timekeeper *tk)
 {
 	cycle_t cycle_now, cycle_delta;
 	struct clocksource *clock;
 	s64 nsec;
 
 	/* read clocksource: */
-	clock = timekeeper.clock;
+	clock = tk->clock;
 	cycle_now = clock->read(clock);
 
 	/* calculate the delta since the last update_wall_time: */
 	cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
 
-	nsec = cycle_delta * timekeeper.mult + timekeeper.xtime_nsec;
-	nsec >>= timekeeper.shift;
+	nsec = cycle_delta * tk->mult + tk->xtime_nsec;
+	nsec >>= tk->shift;
 
 	/* If arch requires, add in gettimeoffset() */
 	nsec += arch_gettimeoffset();
 	return nsec;
 }
 
-static inline s64 timekeeping_get_ns_raw(void)
+static inline s64 timekeeping_get_ns_raw(struct timekeeper *tk)
 {
 	cycle_t cycle_now, cycle_delta;
 	struct clocksource *clock;
 	s64 nsec;
 
 	/* read clocksource: */
-	clock = timekeeper.clock;
+	clock = tk->clock;
 	cycle_now = clock->read(clock);
 
 	/* calculate the delta since the last update_wall_time: */
@@ -224,27 +224,26 @@ static inline s64 timekeeping_get_ns_raw(void)
 
 }
 
-static void update_rt_offset(void)
+static void update_rt_offset(struct timekeeper *tk)
 {
-	struct timespec tmp, *wtm = &timekeeper.wall_to_monotonic;
+	struct timespec tmp, *wtm = &tk->wall_to_monotonic;
 
 	set_normalized_timespec(&tmp, -wtm->tv_sec, -wtm->tv_nsec);
-	timekeeper.offs_real = timespec_to_ktime(tmp);
+	tk->offs_real = timespec_to_ktime(tmp);
 }
 
 /* must hold write on timekeeper.lock */
-static void timekeeping_update(bool clearntp)
+static void timekeeping_update(struct timekeeper *tk, bool clearntp)
 {
 	struct timespec xt;
 
 	if (clearntp) {
-		timekeeper.ntp_error = 0;
+		tk->ntp_error = 0;
 		ntp_clear();
 	}
-	update_rt_offset();
-	xt = tk_xtime(&timekeeper);
-	update_vsyscall(&xt, &timekeeper.wall_to_monotonic,
-			 timekeeper.clock, timekeeper.mult);
+	update_rt_offset(tk);
+	xt = tk_xtime(tk);
+	update_vsyscall(&xt, &tk->wall_to_monotonic, tk->clock, tk->mult);
 }
 
 
@@ -255,26 +254,26 @@ static void timekeeping_update(bool clearntp)
  * update_wall_time(). This is useful before significant clock changes,
  * as it avoids having to deal with this time offset explicitly.
  */
-static void timekeeping_forward_now(void)
+static void timekeeping_forward_now(struct timekeeper *tk)
 {
 	cycle_t cycle_now, cycle_delta;
 	struct clocksource *clock;
 	s64 nsec;
 
-	clock = timekeeper.clock;
+	clock = tk->clock;
 	cycle_now = clock->read(clock);
 	cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
 	clock->cycle_last = cycle_now;
 
-	timekeeper.xtime_nsec += cycle_delta * timekeeper.mult;
+	tk->xtime_nsec += cycle_delta * tk->mult;
 
 	/* If arch requires, add in gettimeoffset() */
-	timekeeper.xtime_nsec += arch_gettimeoffset() << timekeeper.shift;
+	tk->xtime_nsec += arch_gettimeoffset() << tk->shift;
 
-	tk_normalize_xtime(&timekeeper);
+	tk_normalize_xtime(tk);
 
 	nsec = clocksource_cyc2ns(cycle_delta, clock->mult, clock->shift);
-	timespec_add_ns(&timekeeper.raw_time, nsec);
+	timespec_add_ns(&tk->raw_time, nsec);
 }
 
 /**
@@ -294,7 +293,7 @@ void getnstimeofday(struct timespec *ts)
 		seq = read_seqbegin(&timekeeper.lock);
 
 		ts->tv_sec = timekeeper.xtime_sec;
-		ts->tv_nsec = timekeeping_get_ns();
+		ts->tv_nsec = timekeeping_get_ns(&timekeeper);
 
 	} while (read_seqretry(&timekeeper.lock, seq));
 
@@ -313,7 +312,7 @@ ktime_t ktime_get(void)
 		seq = read_seqbegin(&timekeeper.lock);
 		secs = timekeeper.xtime_sec +
 				timekeeper.wall_to_monotonic.tv_sec;
-		nsecs = timekeeping_get_ns() +
+		nsecs = timekeeping_get_ns(&timekeeper) +
 				timekeeper.wall_to_monotonic.tv_nsec;
 
 	} while (read_seqretry(&timekeeper.lock, seq));
@@ -343,7 +342,7 @@ void ktime_get_ts(struct timespec *ts)
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
 		ts->tv_sec = timekeeper.xtime_sec;
-		ts->tv_nsec = timekeeping_get_ns();
+		ts->tv_nsec = timekeeping_get_ns(&timekeeper);
 		tomono = timekeeper.wall_to_monotonic;
 
 	} while (read_seqretry(&timekeeper.lock, seq));
@@ -378,8 +377,8 @@ void getnstime_raw_and_real(struct timespec *ts_raw, struct timespec *ts_real)
 		ts_real->tv_sec = timekeeper.xtime_sec;
 		ts_real->tv_nsec = 0;
 
-		nsecs_raw = timekeeping_get_ns_raw();
-		nsecs_real = timekeeping_get_ns();
+		nsecs_raw = timekeeping_get_ns_raw(&timekeeper);
+		nsecs_real = timekeeping_get_ns(&timekeeper);
 
 	} while (read_seqretry(&timekeeper.lock, seq));
 
@@ -422,7 +421,7 @@ int do_settimeofday(const struct timespec *tv)
 
 	write_seqlock_irqsave(&timekeeper.lock, flags);
 
-	timekeeping_forward_now();
+	timekeeping_forward_now(&timekeeper);
 
 	xt = tk_xtime(&timekeeper);
 	ts_delta.tv_sec = tv->tv_sec - xt.tv_sec;
@@ -433,7 +432,7 @@ int do_settimeofday(const struct timespec *tv)
 
 	tk_set_xtime(&timekeeper, tv);
 
-	timekeeping_update(true);
+	timekeeping_update(&timekeeper, true);
 
 	write_sequnlock_irqrestore(&timekeeper.lock, flags);
 
@@ -460,14 +459,14 @@ int timekeeping_inject_offset(struct timespec *ts)
 
 	write_seqlock_irqsave(&timekeeper.lock, flags);
 
-	timekeeping_forward_now();
+	timekeeping_forward_now(&timekeeper);
 
 
 	tk_xtime_add(&timekeeper, ts);
 	timekeeper.wall_to_monotonic =
 				timespec_sub(timekeeper.wall_to_monotonic, *ts);
 
-	timekeeping_update(true);
+	timekeeping_update(&timekeeper, true);
 
 	write_sequnlock_irqrestore(&timekeeper.lock, flags);
 
@@ -492,14 +491,14 @@ static int change_clocksource(void *data)
 
 	write_seqlock_irqsave(&timekeeper.lock, flags);
 
-	timekeeping_forward_now();
+	timekeeping_forward_now(&timekeeper);
 	if (!new->enable || new->enable(new) == 0) {
 		old = timekeeper.clock;
-		timekeeper_setup_internals(new);
+		tk_setup_internals(&timekeeper, new);
 		if (old->disable)
 			old->disable(old);
 	}
-	timekeeping_update(true);
+	timekeeping_update(&timekeeper, true);
 
 	write_sequnlock_irqrestore(&timekeeper.lock, flags);
 
@@ -549,7 +548,7 @@ void getrawmonotonic(struct timespec *ts)
 
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
-		nsecs = timekeeping_get_ns_raw();
+		nsecs = timekeeping_get_ns_raw(&timekeeper);
 		*ts = timekeeper.raw_time;
 
 	} while (read_seqretry(&timekeeper.lock, seq));
@@ -645,7 +644,7 @@ void __init timekeeping_init(void)
 	clock = clocksource_default_clock();
 	if (clock->enable)
 		clock->enable(clock);
-	timekeeper_setup_internals(clock);
+	tk_setup_internals(&timekeeper, clock);
 
 	tk_set_xtime(&timekeeper, &now);
 	timekeeper.raw_time.tv_sec = 0;
@@ -655,7 +654,7 @@ void __init timekeeping_init(void)
 
 	set_normalized_timespec(&timekeeper.wall_to_monotonic,
 				-boot.tv_sec, -boot.tv_nsec);
-	update_rt_offset();
+	update_rt_offset(&timekeeper);
 	timekeeper.total_sleep_time.tv_sec = 0;
 	timekeeper.total_sleep_time.tv_nsec = 0;
 	write_sequnlock_irqrestore(&timekeeper.lock, flags);
@@ -677,7 +676,8 @@ static void update_sleep_time(struct timespec t)
  * Takes a timespec offset measuring a suspend interval and properly
  * adds the sleep offset to the timekeeping variables.
  */
-static void __timekeeping_inject_sleeptime(struct timespec *delta)
+static void __timekeeping_inject_sleeptime(struct timekeeper *tk,
+							struct timespec *delta)
 {
 	if (!timespec_valid(delta)) {
 		printk(KERN_WARNING "__timekeeping_inject_sleeptime: Invalid "
@@ -685,10 +685,9 @@ static void __timekeeping_inject_sleeptime(struct timespec *delta)
 		return;
 	}
 
-	tk_xtime_add(&timekeeper, delta);
-	timekeeper.wall_to_monotonic =
-			timespec_sub(timekeeper.wall_to_monotonic, *delta);
-	update_sleep_time(timespec_add(timekeeper.total_sleep_time, *delta));
+	tk_xtime_add(tk, delta);
+	tk->wall_to_monotonic = timespec_sub(tk->wall_to_monotonic, *delta);
+	update_sleep_time(timespec_add(tk->total_sleep_time, *delta));
 }
 
 
@@ -714,11 +713,11 @@ void timekeeping_inject_sleeptime(struct timespec *delta)
 
 	write_seqlock_irqsave(&timekeeper.lock, flags);
 
-	timekeeping_forward_now();
+	timekeeping_forward_now(&timekeeper);
 
-	__timekeeping_inject_sleeptime(delta);
+	__timekeeping_inject_sleeptime(&timekeeper, delta);
 
-	timekeeping_update(true);
+	timekeeping_update(&timekeeper, true);
 
 	write_sequnlock_irqrestore(&timekeeper.lock, flags);
 
@@ -747,7 +746,7 @@ static void timekeeping_resume(void)
 
 	if (timespec_compare(&ts, &timekeeping_suspend_time) > 0) {
 		ts = timespec_sub(ts, timekeeping_suspend_time);
-		__timekeeping_inject_sleeptime(&ts);
+		__timekeeping_inject_sleeptime(&timekeeper, &ts);
 	}
 	/* re-base the last cycle value */
 	timekeeper.clock->cycle_last = timekeeper.clock->read(timekeeper.clock);
@@ -772,7 +771,7 @@ static int timekeeping_suspend(void)
 	read_persistent_clock(&timekeeping_suspend_time);
 
 	write_seqlock_irqsave(&timekeeper.lock, flags);
-	timekeeping_forward_now();
+	timekeeping_forward_now(&timekeeper);
 	timekeeping_suspended = 1;
 
 	/*
@@ -820,7 +819,8 @@ device_initcall(timekeeping_init_ops);
  * If the error is already larger, we look ahead even further
  * to compensate for late or lost adjustments.
  */
-static __always_inline int timekeeping_bigadjust(s64 error, s64 *interval,
+static __always_inline int timekeeping_bigadjust(struct timekeeper *tk,
+						 s64 error, s64 *interval,
 						 s64 *offset)
 {
 	s64 tick_error, i;
@@ -836,7 +836,7 @@ static __always_inline int timekeeping_bigadjust(s64 error, s64 *interval,
 	 * here.  This is tuned so that an error of about 1 msec is adjusted
 	 * within about 1 sec (or 2^20 nsec in 2^SHIFT_HZ ticks).
 	 */
-	error2 = timekeeper.ntp_error >> (NTP_SCALE_SHIFT + 22 - 2 * SHIFT_HZ);
+	error2 = tk->ntp_error >> (NTP_SCALE_SHIFT + 22 - 2 * SHIFT_HZ);
 	error2 = abs(error2);
 	for (look_ahead = 0; error2 > 0; look_ahead++)
 		error2 >>= 2;
@@ -845,8 +845,8 @@ static __always_inline int timekeeping_bigadjust(s64 error, s64 *interval,
 	 * Now calculate the error in (1 << look_ahead) ticks, but first
 	 * remove the single look ahead already included in the error.
 	 */
-	tick_error = ntp_tick_length() >> (timekeeper.ntp_error_shift + 1);
-	tick_error -= timekeeper.xtime_interval >> 1;
+	tick_error = ntp_tick_length() >> (tk->ntp_error_shift + 1);
+	tick_error -= tk->xtime_interval >> 1;
 	error = ((error - tick_error) >> look_ahead) + tick_error;
 
 	/* Finally calculate the adjustment shift value.  */
@@ -871,9 +871,9 @@ static __always_inline int timekeeping_bigadjust(s64 error, s64 *interval,
  * this is optimized for the most common adjustments of -1,0,1,
  * for other values we can do a bit more work.
  */
-static void timekeeping_adjust(s64 offset)
+static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
 {
-	s64 error, interval = timekeeper.cycle_interval;
+	s64 error, interval = tk->cycle_interval;
 	int adj;
 
 	/*
@@ -889,7 +889,7 @@ static void timekeeping_adjust(s64 offset)
 	 *
 	 * Note: It does not "save" on aggravation when reading the code.
 	 */
-	error = timekeeper.ntp_error >> (timekeeper.ntp_error_shift - 1);
+	error = tk->ntp_error >> (tk->ntp_error_shift - 1);
 	if (error > interval) {
 		/*
 		 * We now divide error by 4(via shift), which checks if
@@ -911,7 +911,8 @@ static void timekeeping_adjust(s64 offset)
 		if (likely(error <= interval))
 			adj = 1;
 		else
-			adj = timekeeping_bigadjust(error, &interval, &offset);
+			adj = timekeeping_bigadjust(tk, error, &interval,
+							&offset);
 	} else if (error < -interval) {
 		/* See comment above, this is just switched for the negative */
 		error >>= 2;
@@ -920,18 +921,17 @@ static void timekeeping_adjust(s64 offset)
 			interval = -interval;
 			offset = -offset;
 		} else
-			adj = timekeeping_bigadjust(error, &interval, &offset);
-	} else /* No adjustment needed */
+			adj = timekeeping_bigadjust(tk, error, &interval,
+							&offset);
+	} else
 		return;
 
-	if (unlikely(timekeeper.clock->maxadj &&
-			(timekeeper.mult + adj >
-			timekeeper.clock->mult + timekeeper.clock->maxadj))) {
+	if (unlikely(tk->clock->maxadj &&
+		(tk->mult + adj > tk->clock->mult + tk->clock->maxadj))) {
 		printk_once(KERN_WARNING
 			"Adjusting %s more than 11%% (%ld vs %ld)\n",
-			timekeeper.clock->name, (long)timekeeper.mult + adj,
-			(long)timekeeper.clock->mult +
-				timekeeper.clock->maxadj);
+			tk->clock->name, (long)tk->mult + adj,
+			(long)tk->clock->mult + tk->clock->maxadj);
 	}
 	/*
 	 * So the following can be confusing.
@@ -982,11 +982,10 @@ static void timekeeping_adjust(s64 offset)
 	 *
 	 * XXX - TODO: Doc ntp_error calculation.
 	 */
-	timekeeper.mult += adj;
-	timekeeper.xtime_interval += interval;
-	timekeeper.xtime_nsec -= offset;
-	timekeeper.ntp_error -= (interval - offset) <<
-				timekeeper.ntp_error_shift;
+	tk->mult += adj;
+	tk->xtime_interval += interval;
+	tk->xtime_nsec -= offset;
+	tk->ntp_error -= (interval - offset) << tk->ntp_error_shift;
 
 	/*
 	 * It may be possible that when we entered this function, xtime_nsec
@@ -1002,10 +1001,10 @@ static void timekeeping_adjust(s64 offset)
 	 * We'll correct this error next time through this function, when
 	 * xtime_nsec is not as small.
 	 */
-	if (unlikely((s64)timekeeper.xtime_nsec < 0)) {
-		s64 neg = -(s64)timekeeper.xtime_nsec;
-		timekeeper.xtime_nsec = 0;
-		timekeeper.ntp_error += neg << timekeeper.ntp_error_shift;
+	if (unlikely((s64)tk->xtime_nsec < 0)) {
+		s64 neg = -(s64)tk->xtime_nsec;
+		tk->xtime_nsec = 0;
+		tk->ntp_error += neg << tk->ntp_error_shift;
 	}
 
 }
@@ -1049,37 +1048,36 @@ static inline void accumulate_nsecs_to_secs(struct timekeeper *tk)
  *
  * Returns the unconsumed cycles.
  */
-static cycle_t logarithmic_accumulation(cycle_t offset, u32 shift)
+static cycle_t logarithmic_accumulation(struct timekeeper *tk, cycle_t offset,
+						u32 shift)
 {
 	u64 raw_nsecs;
 
-	/* If the offset is smaller than a shifted interval, do nothing */
-	if (offset < timekeeper.cycle_interval<<shift)
+	/* If the offset is smaller then a shifted interval, do nothing */
+	if (offset < tk->cycle_interval<<shift)
 		return offset;
 
 	/* Accumulate one shifted interval */
-	offset -= timekeeper.cycle_interval << shift;
-	timekeeper.clock->cycle_last += timekeeper.cycle_interval << shift;
+	offset -= tk->cycle_interval << shift;
+	tk->clock->cycle_last += tk->cycle_interval << shift;
 
-	timekeeper.xtime_nsec += timekeeper.xtime_interval << shift;
-
-	accumulate_nsecs_to_secs(&timekeeper);
+	tk->xtime_nsec += tk->xtime_interval << shift;
+	accumulate_nsecs_to_secs(tk);
 
 	/* Accumulate raw time */
-	raw_nsecs = timekeeper.raw_interval << shift;
-	raw_nsecs += timekeeper.raw_time.tv_nsec;
+	raw_nsecs = tk->raw_interval << shift;
+	raw_nsecs += tk->raw_time.tv_nsec;
 	if (raw_nsecs >= NSEC_PER_SEC) {
 		u64 raw_secs = raw_nsecs;
 		raw_nsecs = do_div(raw_secs, NSEC_PER_SEC);
-		timekeeper.raw_time.tv_sec += raw_secs;
+		tk->raw_time.tv_sec += raw_secs;
 	}
-	timekeeper.raw_time.tv_nsec = raw_nsecs;
+	tk->raw_time.tv_nsec = raw_nsecs;
 
 	/* Accumulate error between NTP and clock interval */
-	timekeeper.ntp_error += ntp_tick_length() << shift;
-	timekeeper.ntp_error -=
-	    (timekeeper.xtime_interval + timekeeper.xtime_remainder) <<
-				(timekeeper.ntp_error_shift + shift);
+	tk->ntp_error += ntp_tick_length() << shift;
+	tk->ntp_error -= (tk->xtime_interval + tk->xtime_remainder) <<
+						(tk->ntp_error_shift + shift);
 
 	return offset;
 }
@@ -1125,13 +1123,13 @@ static void update_wall_time(void)
 	maxshift = (64 - (ilog2(ntp_tick_length())+1)) - 1;
 	shift = min(shift, maxshift);
 	while (offset >= timekeeper.cycle_interval) {
-		offset = logarithmic_accumulation(offset, shift);
+		offset = logarithmic_accumulation(&timekeeper, offset, shift);
 		if(offset < timekeeper.cycle_interval<<shift)
 			shift--;
 	}
 
 	/* correct the clock when NTP error is too big */
-	timekeeping_adjust(offset);
+	timekeeping_adjust(&timekeeper, offset);
 
 
 	/*
@@ -1154,7 +1152,7 @@ static void update_wall_time(void)
 	 */
 	accumulate_nsecs_to_secs(&timekeeper);
 
-	timekeeping_update(false);
+	timekeeping_update(&timekeeper, false);
 
 out:
 	write_sequnlock_irqrestore(&timekeeper.lock, flags);
@@ -1205,7 +1203,7 @@ void get_monotonic_boottime(struct timespec *ts)
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
 		ts->tv_sec = timekeeper.xtime_sec;
-		ts->tv_nsec = timekeeping_get_ns();
+		ts->tv_nsec = timekeeping_get_ns(&timekeeper);
 		tomono = timekeeper.wall_to_monotonic;
 		sleep = timekeeper.total_sleep_time;
 
@@ -1337,7 +1335,7 @@ ktime_t ktime_get_update_offsets(ktime_t *offs_real, ktime_t *offs_boot)
 		seq = read_seqbegin(&timekeeper.lock);
 
 		secs = timekeeper.xtime_sec;
-		nsecs = timekeeping_get_ns();
+		nsecs = timekeeping_get_ns(&timekeeper);
 
 		*offs_real = timekeeper.offs_real;
 		*offs_boot = timekeeper.offs_boot;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 5/8] time: Refactor accumulation of nsecs to secs
  2012-07-13  5:21 ` [PATCH 5/8] time: Refactor accumulation of nsecs to secs John Stultz
@ 2012-07-13  5:56   ` Ingo Molnar
  2012-07-13  6:01   ` Ingo Molnar
  2012-07-15  8:58   ` [tip:timers/core] " tip-bot for John Stultz
  2 siblings, 0 replies; 34+ messages in thread
From: Ingo Molnar @ 2012-07-13  5:56 UTC (permalink / raw)
  To: John Stultz
  Cc: Linux Kernel, Peter Zijlstra, Richard Cochran, Prarit Bhargava,
	Thomas Gleixner


* John Stultz <john.stultz@linaro.org> wrote:

> We do the exact same logic moving nsecs to secs in the
> timekeeper in multiple places, so condense this into a
> single function.
> 
> CC: Ingo Molnar <mingo@kernel.org>
> CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
> CC: Richard Cochran <richardcochran@gmail.com>
> CC: Prarit Bhargava <prarit@redhat.com>
> CC: Thomas Gleixner <tglx@linutronix.de>

Please make that Cc:, not CC:.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/8] ntp: Fix STA_INS/DEL clearing bug
  2012-07-13  5:21 ` [PATCH 1/8] ntp: Fix STA_INS/DEL clearing bug John Stultz
@ 2012-07-13  5:58   ` Ingo Molnar
  2012-07-13 18:36     ` John Stultz
  2012-07-15  7:57   ` [tip:timers/urgent] " tip-bot for John Stultz
  1 sibling, 1 reply; 34+ messages in thread
From: Ingo Molnar @ 2012-07-13  5:58 UTC (permalink / raw)
  To: John Stultz
  Cc: Linux Kernel, John Stultz, Peter Zijlstra, Richard Cochran,
	Prarit Bhargava, Thomas Gleixner, stable


* John Stultz <john.stultz@linaro.org> wrote:

> From: John Stultz <johnstul@us.ibm.com>
> 
> In commit 6b43ae8a619d17c4935c3320d2ef9e92bdeed05d, I
> introduced a bug that kept the STA_INS or STA_DEL bit
> from being cleared from time_status via adjtimex()
> without forcing STA_PLL first.
> 
> Usually once the STA_INS is set, it isn't cleared
> until the leap second is applied, so its unlikely this
> affected anyone. However during testing I noticed it
> took some effort to cancel a leap second once STA_INS
> was set.
> 
> This issue affects 3.4 and up.
> 
> Since this isn't urgent (issue is only observed in testing,
> the behavior doesn't affect ntpd, nor is a leapsecond due
> for at least ~6 months), and we're late in the 3.5-rc
> cycle, I'm holding this off for 3.6 merge window,
> where I'll then backport to 3.5-stable and 3.4-stable.

> CC: stable@vger.kernel.org

We generally don't do such a workflow. Either it's valid for 
tip:timers/urgent and it can have a -stable tag, or it should 
not be backported, and not have a -stable tag.

The rule is: if it's important enough for -stable then it's 
doubly important for the current -rc kernel!

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 4/8] time: Condense timekeeper.xtime into xtime_sec
  2012-07-13  5:21 ` [PATCH 4/8] time: Condense timekeeper.xtime into xtime_sec John Stultz
@ 2012-07-13  6:00   ` Ingo Molnar
  2012-07-15  8:57   ` [tip:timers/core] " tip-bot for John Stultz
  2012-08-19 21:02   ` [PATCH 4/8] " Andreas Schwab
  2 siblings, 0 replies; 34+ messages in thread
From: Ingo Molnar @ 2012-07-13  6:00 UTC (permalink / raw)
  To: John Stultz
  Cc: Linux Kernel, Peter Zijlstra, Richard Cochran, Prarit Bhargava,
	Thomas Gleixner


* John Stultz <john.stultz@linaro.org> wrote:

> +static void tk_set_xtime(struct timekeeper *tk, const struct timespec *ts)
> +{
> +	tk->xtime_sec = ts->tv_sec;
> +	tk->xtime_nsec = ts->tv_nsec << tk->shift;
> +}
> +
> +
> +static void tk_xtime_add(struct timekeeper *tk, const struct timespec *ts)

Small nit: there's a stray newline here.

> +{
> +	tk->xtime_sec += ts->tv_sec;
> +	tk->xtime_nsec += ts->tv_nsec << tk->shift;
> +}
> +
>  
>  /**

Ditto.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 5/8] time: Refactor accumulation of nsecs to secs
  2012-07-13  5:21 ` [PATCH 5/8] time: Refactor accumulation of nsecs to secs John Stultz
  2012-07-13  5:56   ` Ingo Molnar
@ 2012-07-13  6:01   ` Ingo Molnar
  2012-07-15  8:58   ` [tip:timers/core] " tip-bot for John Stultz
  2 siblings, 0 replies; 34+ messages in thread
From: Ingo Molnar @ 2012-07-13  6:01 UTC (permalink / raw)
  To: John Stultz
  Cc: Linux Kernel, Peter Zijlstra, Richard Cochran, Prarit Bhargava,
	Thomas Gleixner


* John Stultz <john.stultz@linaro.org> wrote:

> +}
> +
> +
> +/**

Stray newline added here as well.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 6/8] time: Move arch_gettimeoffset() usage into timekeeping_get_ns()
  2012-07-13  5:21 ` [PATCH 6/8] time: Move arch_gettimeoffset() usage into timekeeping_get_ns() John Stultz
@ 2012-07-13  6:03   ` Ingo Molnar
  2012-07-15  8:59   ` [tip:timers/core] " tip-bot for John Stultz
  1 sibling, 0 replies; 34+ messages in thread
From: Ingo Molnar @ 2012-07-13  6:03 UTC (permalink / raw)
  To: John Stultz
  Cc: Linux Kernel, Peter Zijlstra, Richard Cochran, Prarit Bhargava,
	Thomas Gleixner, John Stultz


* John Stultz <john.stultz@linaro.org> wrote:

> +	nsec >>= timekeeper.shift;
> +
> +	/* If arch requires, add in gettimeoffset() */
> +	nsec += arch_gettimeoffset();
> +	return nsec;

As a factoring out bonus, this could be further shortened to:

	return nsec + arch_gettimeoffset();

> +	/* If arch requires, add in gettimeoffset() */
> +	nsec += arch_gettimeoffset();
> +	return nsec;

Ditto.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 7/8] time: Move xtime_nsec adjustment underflow handling timekeeping_adjust
  2012-07-13  5:21 ` [PATCH 7/8] time: Move xtime_nsec adjustment underflow handling timekeeping_adjust John Stultz
@ 2012-07-13  6:04   ` Ingo Molnar
  2012-07-15  9:00   ` [tip:timers/core] " tip-bot for John Stultz
  1 sibling, 0 replies; 34+ messages in thread
From: Ingo Molnar @ 2012-07-13  6:04 UTC (permalink / raw)
  To: John Stultz
  Cc: Linux Kernel, Peter Zijlstra, Richard Cochran, Prarit Bhargava,
	Thomas Gleixner, John Stultz


* John Stultz <john.stultz@linaro.org> wrote:

> When we make adjustments speeding up the clock, its possible
> for xtime_nsec to underflow. We already handle this properly,
> but we do so from update_wall_time() instead of the more logical
> timekeeping_adjust(), where the possible underflow actually
> occurs.
> 
> Thus, move the correction logic to the timekeeping_adjust, which
> is the function that causes the issue. Making update_wall_time()
> more readable.
> 
> CC: Ingo Molnar <mingo@kernel.org>
> CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
> CC: Richard Cochran <richardcochran@gmail.com>
> CC: Prarit Bhargava <prarit@redhat.com>
> CC: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: John Stultz <johnstul@us.ibm.com>
> ---
>  kernel/time/timekeeping.c |   42 +++++++++++++++++++++---------------------
>  1 file changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index dd119355..4b76432 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -987,6 +987,27 @@ static void timekeeping_adjust(s64 offset)
>  	timekeeper.xtime_nsec -= offset;
>  	timekeeper.ntp_error -= (interval - offset) <<
>  				timekeeper.ntp_error_shift;
> +
> +	/*
> +	 * It may be possible that when we entered this function, xtime_nsec
> +	 * was very small.  Further, if we're slightly speeding the clocksource
> +	 * in the code above, its possible the required corrective factor to
> +	 * xtime_nsec could cause it to underflow.

s/slightly speeding/slightly speeding up ?

> +	 *
> +	 * Now, since we already accumulated the second, cannot simply roll
> +	 * the accumulated second back, since the NTP subsystem has been

s/cannot/we cannot ?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/8] Time fixes and cleanups for 3.6
  2012-07-13  5:21 [PATCH 0/8] Time fixes and cleanups for 3.6 John Stultz
                   ` (7 preceding siblings ...)
  2012-07-13  5:21 ` [PATCH 8/8] time: Rework timekeeping functions to take timekeeper ptr as argument John Stultz
@ 2012-07-13  6:05 ` Ingo Molnar
  2012-07-13 20:49   ` John Stultz
  8 siblings, 1 reply; 34+ messages in thread
From: Ingo Molnar @ 2012-07-13  6:05 UTC (permalink / raw)
  To: John Stultz
  Cc: Linux Kernel, Peter Zijlstra, Richard Cochran, Prarit Bhargava,
	Thomas Gleixner


Looks sensible.

Reviewed-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/8] ntp: Fix STA_INS/DEL clearing bug
  2012-07-13  5:58   ` Ingo Molnar
@ 2012-07-13 18:36     ` John Stultz
  0 siblings, 0 replies; 34+ messages in thread
From: John Stultz @ 2012-07-13 18:36 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Linux Kernel, Peter Zijlstra, Richard Cochran, Prarit Bhargava,
	Thomas Gleixner, stable

On 07/12/2012 10:58 PM, Ingo Molnar wrote:
>
> * John Stultz <john.stultz@linaro.org> wrote:
>
>> From: John Stultz <johnstul@us.ibm.com>
>>
>> In commit 6b43ae8a619d17c4935c3320d2ef9e92bdeed05d, I
>> introduced a bug that kept the STA_INS or STA_DEL bit
>> from being cleared from time_status via adjtimex()
>> without forcing STA_PLL first.
>>
>> Usually once the STA_INS is set, it isn't cleared
>> until the leap second is applied, so its unlikely this
>> affected anyone. However during testing I noticed it
>> took some effort to cancel a leap second once STA_INS
>> was set.
>>
>> This issue affects 3.4 and up.
>>
>> Since this isn't urgent (issue is only observed in testing,
>> the behavior doesn't affect ntpd, nor is a leapsecond due
>> for at least ~6 months), and we're late in the 3.5-rc
>> cycle, I'm holding this off for 3.6 merge window,
>> where I'll then backport to 3.5-stable and 3.4-stable.
>
>> CC: stable@vger.kernel.org
>
> We generally don't do such a workflow. Either it's valid for
> tip:timers/urgent and it can have a -stable tag, or it should
> not be backported, and not have a -stable tag.
>
> The rule is: if it's important enough for -stable then it's
> doubly important for the current -rc kernel!

Ok. My concern here was that if the time/hrtimer leapsecond changes were 
possibly too large for merging this late in 3.5-rc, I didn't want to add 
anything more to that queue.

So if you're comfortable pushing this one upstream for 3.5-rc, I'd be 
happy to have it merged sooner.

thanks
-john


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 0/8] Time fixes and cleanups for 3.6
  2012-07-13  6:05 ` [PATCH 0/8] Time fixes and cleanups for 3.6 Ingo Molnar
@ 2012-07-13 20:49   ` John Stultz
  0 siblings, 0 replies; 34+ messages in thread
From: John Stultz @ 2012-07-13 20:49 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Linux Kernel, Peter Zijlstra, Richard Cochran, Prarit Bhargava,
	Thomas Gleixner

On 07/12/2012 11:05 PM, Ingo Molnar wrote:
>
> Looks sensible.
>
> Reviewed-by: Ingo Molnar <mingo@kernel.org>
>
> Thanks,
>
> 	Ingo
>


Thanks for the review! I've added your changes and your reviewed by tag 
and pushed it out to my fortglx/3.6/time branch.

Here's the updated pull request below (Sorry for the word-wrap, 
thunderbrid on my laptop isn't wanting to preserve the preformatting for 
some reason).

thanks
-john


The following changes since commit 5baefd6d84163443215f4a99f6a20f054ef11236:

   hrtimer: Update hrtimer base offsets each hrtimer_interrupt 
(2012-07-11 23:34:39 +0200)

are available in the git repository at:

   git://git.linaro.org/people/jstultz/linux.git fortglx/3.6/time

for you to fetch changes up to bbe1c99bd1000f64b117d462ea1c5041f81fb179:

   time: Rework timekeeping functions to take timekeeper ptr as argument 
(2012-07-13 16:39:02 -0400)

----------------------------------------------------------------
John Stultz (8):
       ntp: Fix STA_INS/DEL clearing bug
       time: Whitespace cleanups per Ingo's requests
       time: Explicitly use u32 instead of int for shift values
       time: Condense timekeeper.xtime into xtime_sec
       time: Refactor accumulation of nsecs to secs
       time: Move arch_gettimeoffset() usage into timekeeping_get_ns()
       time: Move xtime_nsec adjustment underflow handling 
timekeeping_adjust
       time: Rework timekeeping functions to take timekeeper ptr as argument

  kernel/time/ntp.c         |    8 +-
  kernel/time/timekeeping.c |  486 
++++++++++++++++++++++++---------------------
  2 files changed, 267 insertions(+), 227 deletions(-)



^ permalink raw reply	[flat|nested] 34+ messages in thread

* [tip:timers/urgent] ntp: Fix STA_INS/DEL clearing bug
  2012-07-13  5:21 ` [PATCH 1/8] ntp: Fix STA_INS/DEL clearing bug John Stultz
  2012-07-13  5:58   ` Ingo Molnar
@ 2012-07-15  7:57   ` tip-bot for John Stultz
  1 sibling, 0 replies; 34+ messages in thread
From: tip-bot for John Stultz @ 2012-07-15  7:57 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, johnstul, richardcochran,
	tglx, prarit

Commit-ID:  6b1859dba01c7d512b72d77e3fd7da8354235189
Gitweb:     http://git.kernel.org/tip/6b1859dba01c7d512b72d77e3fd7da8354235189
Author:     John Stultz <johnstul@us.ibm.com>
AuthorDate: Fri, 13 Jul 2012 01:21:50 -0400
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 15 Jul 2012 09:48:49 +0200

ntp: Fix STA_INS/DEL clearing bug

In commit 6b43ae8a619d17c4935c3320d2ef9e92bdeed05d, I
introduced a bug that kept the STA_INS or STA_DEL bit
from being cleared from time_status via adjtimex()
without forcing STA_PLL first.

Usually once the STA_INS is set, it isn't cleared
until the leap second is applied, so its unlikely this
affected anyone. However during testing I noticed it
took some effort to cancel a leap second once STA_INS
was set.

Signed-off-by: John Stultz <johnstul@us.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
CC: stable@vger.kernel.org # 3.4
Link: http://lkml.kernel.org/r/1342156917-25092-2-git-send-email-john.stultz@linaro.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/ntp.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 70b33ab..b7fbadc 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -409,7 +409,9 @@ int second_overflow(unsigned long secs)
 			time_state = TIME_DEL;
 		break;
 	case TIME_INS:
-		if (secs % 86400 == 0) {
+		if (!(time_status & STA_INS))
+			time_state = TIME_OK;
+		else if (secs % 86400 == 0) {
 			leap = -1;
 			time_state = TIME_OOP;
 			time_tai++;
@@ -418,7 +420,9 @@ int second_overflow(unsigned long secs)
 		}
 		break;
 	case TIME_DEL:
-		if ((secs + 1) % 86400 == 0) {
+		if (!(time_status & STA_DEL))
+			time_state = TIME_OK;
+		else if ((secs + 1) % 86400 == 0) {
 			leap = 1;
 			time_tai--;
 			time_state = TIME_WAIT;

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [tip:timers/core] time: Whitespace cleanups per Ingo%27s requests
  2012-07-13  5:21 ` [PATCH 2/8] time: Whitespace cleanups per Ingo's requests John Stultz
@ 2012-07-15  8:56   ` tip-bot for John Stultz
  0 siblings, 0 replies; 34+ messages in thread
From: tip-bot for John Stultz @ 2012-07-15  8:56 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, richardcochran,
	john.stultz, tglx, prarit

Commit-ID:  42e71e81f5bb5125ca7c194b5ccf1c93511ff8fb
Gitweb:     http://git.kernel.org/tip/42e71e81f5bb5125ca7c194b5ccf1c93511ff8fb
Author:     John Stultz <john.stultz@linaro.org>
AuthorDate: Fri, 13 Jul 2012 01:21:51 -0400
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 15 Jul 2012 10:39:05 +0200

time: Whitespace cleanups per Ingo%27s requests

Ingo noted a number of places where there is inconsistent
use of whitespace. This patch tries to address the main
culprits.

Signed-off-by: John Stultz <john.stultz@linaro.org>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Link: http://lkml.kernel.org/r/1342156917-25092-3-git-send-email-john.stultz@linaro.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/timekeeping.c |   39 ++++++++++++++++++---------------------
 1 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 269b1fe..c2f12aa 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -24,32 +24,31 @@
 /* Structure holding internal timekeeping values. */
 struct timekeeper {
 	/* Current clocksource used for timekeeping. */
-	struct clocksource *clock;
+	struct clocksource	*clock;
 	/* NTP adjusted clock multiplier */
-	u32	mult;
+	u32			mult;
 	/* The shift value of the current clocksource. */
-	int	shift;
-
+	int			shift;
 	/* Number of clock cycles in one NTP interval. */
-	cycle_t cycle_interval;
+	cycle_t			cycle_interval;
 	/* Number of clock shifted nano seconds in one NTP interval. */
-	u64	xtime_interval;
+	u64			xtime_interval;
 	/* shifted nano seconds left over when rounding cycle_interval */
-	s64	xtime_remainder;
+	s64			xtime_remainder;
 	/* Raw nano seconds accumulated per NTP interval. */
-	u32	raw_interval;
+	u32			raw_interval;
 
 	/* Clock shifted nano seconds remainder not stored in xtime.tv_nsec. */
-	u64	xtime_nsec;
+	u64			xtime_nsec;
 	/* Difference between accumulated time and NTP time in ntp
 	 * shifted nano seconds. */
-	s64	ntp_error;
+	s64			ntp_error;
 	/* Shift conversion between clock shifted nano seconds and
 	 * ntp shifted nano seconds. */
-	int	ntp_error_shift;
+	int			ntp_error_shift;
 
 	/* The current time */
-	struct timespec xtime;
+	struct timespec 	xtime;
 	/*
 	 * wall_to_monotonic is what we need to add to xtime (or xtime corrected
 	 * for sub jiffie times) to get to monotonic time.  Monotonic is pegged
@@ -64,20 +63,17 @@ struct timekeeper {
 	 * - wall_to_monotonic is no longer the boot time, getboottime must be
 	 * used instead.
 	 */
-	struct timespec wall_to_monotonic;
+	struct timespec		wall_to_monotonic;
 	/* time spent in suspend */
-	struct timespec total_sleep_time;
+	struct timespec		total_sleep_time;
 	/* The raw monotonic time for the CLOCK_MONOTONIC_RAW posix clock. */
-	struct timespec raw_time;
-
+	struct timespec		raw_time;
 	/* Offset clock monotonic -> clock realtime */
-	ktime_t offs_real;
-
+	ktime_t			offs_real;
 	/* Offset clock monotonic -> clock boottime */
-	ktime_t offs_boot;
-
+	ktime_t			offs_boot;
 	/* Seqlock for all timekeeper values */
-	seqlock_t lock;
+	seqlock_t		lock;
 };
 
 static struct timekeeper timekeeper;
@@ -547,6 +543,7 @@ u64 timekeeping_max_deferment(void)
 {
 	unsigned long seq;
 	u64 ret;
+
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
 

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [tip:timers/core] time: Explicitly use u32 instead of int for shift values
  2012-07-13  5:21 ` [PATCH 3/8] time: Explicitly use u32 instead of int for shift values John Stultz
@ 2012-07-15  8:57   ` tip-bot for John Stultz
  0 siblings, 0 replies; 34+ messages in thread
From: tip-bot for John Stultz @ 2012-07-15  8:57 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, richardcochran,
	john.stultz, tglx, prarit

Commit-ID:  fee84c43e6afc42295ae8058cbbef9ea5633926c
Gitweb:     http://git.kernel.org/tip/fee84c43e6afc42295ae8058cbbef9ea5633926c
Author:     John Stultz <john.stultz@linaro.org>
AuthorDate: Fri, 13 Jul 2012 01:21:52 -0400
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 15 Jul 2012 10:39:05 +0200

time: Explicitly use u32 instead of int for shift values

Ingo noted that using a u32 instead of int for shift values
would be better to make sure the compiler doesn't unnecessarily
use complex signed arithmetic.

Signed-off-by: John Stultz <john.stultz@linaro.org>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Link: http://lkml.kernel.org/r/1342156917-25092-4-git-send-email-john.stultz@linaro.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/timekeeping.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index c2f12aa..4fd83df 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -28,7 +28,7 @@ struct timekeeper {
 	/* NTP adjusted clock multiplier */
 	u32			mult;
 	/* The shift value of the current clocksource. */
-	int			shift;
+	u32			shift;
 	/* Number of clock cycles in one NTP interval. */
 	cycle_t			cycle_interval;
 	/* Number of clock shifted nano seconds in one NTP interval. */
@@ -45,7 +45,7 @@ struct timekeeper {
 	s64			ntp_error;
 	/* Shift conversion between clock shifted nano seconds and
 	 * ntp shifted nano seconds. */
-	int			ntp_error_shift;
+	u32			ntp_error_shift;
 
 	/* The current time */
 	struct timespec 	xtime;
@@ -960,7 +960,7 @@ static void timekeeping_adjust(s64 offset)
  *
  * Returns the unconsumed cycles.
  */
-static cycle_t logarithmic_accumulation(cycle_t offset, int shift)
+static cycle_t logarithmic_accumulation(cycle_t offset, u32 shift)
 {
 	u64 nsecps = (u64)NSEC_PER_SEC << timekeeper.shift;
 	u64 raw_nsecs;

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [tip:timers/core] time: Condense timekeeper.xtime into xtime_sec
  2012-07-13  5:21 ` [PATCH 4/8] time: Condense timekeeper.xtime into xtime_sec John Stultz
  2012-07-13  6:00   ` Ingo Molnar
@ 2012-07-15  8:57   ` tip-bot for John Stultz
  2012-08-19 21:02   ` [PATCH 4/8] " Andreas Schwab
  2 siblings, 0 replies; 34+ messages in thread
From: tip-bot for John Stultz @ 2012-07-15  8:57 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, richardcochran,
	john.stultz, tglx, prarit

Commit-ID:  1e75fa8be9fb61e1af46b5b3b176347a4c958ca1
Gitweb:     http://git.kernel.org/tip/1e75fa8be9fb61e1af46b5b3b176347a4c958ca1
Author:     John Stultz <john.stultz@linaro.org>
AuthorDate: Fri, 13 Jul 2012 01:21:53 -0400
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 15 Jul 2012 10:39:06 +0200

time: Condense timekeeper.xtime into xtime_sec

The timekeeper struct has a xtime_nsec, which keeps the
sub-nanosecond remainder.  This ends up being somewhat
duplicative of the timekeeper.xtime.tv_nsec value, and we
have to do extra work to keep them apart, copying the full
nsec portion out and back in over and over.

This patch simplifies some of the logic by taking the timekeeper
xtime value and splitting it into timekeeper.xtime_sec and
reuses the timekeeper.xtime_nsec for the sub-second portion
(stored in higher res shifted nanoseconds).

This simplifies some of the accumulation logic. And will
allow for more accurate timekeeping once the vsyscall code
is updated to use the shifted nanosecond remainder.

Signed-off-by: John Stultz <john.stultz@linaro.org>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Link: http://lkml.kernel.org/r/1342156917-25092-5-git-send-email-john.stultz@linaro.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/timekeeping.c |  181 +++++++++++++++++++++++++++------------------
 1 files changed, 110 insertions(+), 71 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 4fd83df..b98d9bd 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -38,8 +38,11 @@ struct timekeeper {
 	/* Raw nano seconds accumulated per NTP interval. */
 	u32			raw_interval;
 
-	/* Clock shifted nano seconds remainder not stored in xtime.tv_nsec. */
+	/* Current CLOCK_REALTIME time in seconds */
+	u64			xtime_sec;
+	/* Clock shifted nano seconds */
 	u64			xtime_nsec;
+
 	/* Difference between accumulated time and NTP time in ntp
 	 * shifted nano seconds. */
 	s64			ntp_error;
@@ -47,8 +50,6 @@ struct timekeeper {
 	 * ntp shifted nano seconds. */
 	u32			ntp_error_shift;
 
-	/* The current time */
-	struct timespec 	xtime;
 	/*
 	 * wall_to_monotonic is what we need to add to xtime (or xtime corrected
 	 * for sub jiffie times) to get to monotonic time.  Monotonic is pegged
@@ -84,11 +85,37 @@ static struct timekeeper timekeeper;
  */
 __cacheline_aligned_in_smp DEFINE_SEQLOCK(xtime_lock);
 
-
 /* flag for if timekeeping is suspended */
 int __read_mostly timekeeping_suspended;
 
+static inline void tk_normalize_xtime(struct timekeeper *tk)
+{
+	while (tk->xtime_nsec >= ((u64)NSEC_PER_SEC << tk->shift)) {
+		tk->xtime_nsec -= (u64)NSEC_PER_SEC << tk->shift;
+		tk->xtime_sec++;
+	}
+}
+
+static struct timespec tk_xtime(struct timekeeper *tk)
+{
+	struct timespec ts;
+
+	ts.tv_sec = tk->xtime_sec;
+	ts.tv_nsec = (long)(tk->xtime_nsec >> tk->shift);
+	return ts;
+}
 
+static void tk_set_xtime(struct timekeeper *tk, const struct timespec *ts)
+{
+	tk->xtime_sec = ts->tv_sec;
+	tk->xtime_nsec = ts->tv_nsec << tk->shift;
+}
+
+static void tk_xtime_add(struct timekeeper *tk, const struct timespec *ts)
+{
+	tk->xtime_sec += ts->tv_sec;
+	tk->xtime_nsec += ts->tv_nsec << tk->shift;
+}
 
 /**
  * timekeeper_setup_internals - Set up internals to use clocksource clock.
@@ -104,7 +131,9 @@ static void timekeeper_setup_internals(struct clocksource *clock)
 {
 	cycle_t interval;
 	u64 tmp, ntpinterval;
+	struct clocksource *old_clock;
 
+	old_clock = timekeeper.clock;
 	timekeeper.clock = clock;
 	clock->cycle_last = clock->read(clock);
 
@@ -126,7 +155,14 @@ static void timekeeper_setup_internals(struct clocksource *clock)
 	timekeeper.raw_interval =
 		((u64) interval * clock->mult) >> clock->shift;
 
-	timekeeper.xtime_nsec = 0;
+	 /* if changing clocks, convert xtime_nsec shift units */
+	if (old_clock) {
+		int shift_change = clock->shift - old_clock->shift;
+		if (shift_change < 0)
+			timekeeper.xtime_nsec >>= -shift_change;
+		else
+			timekeeper.xtime_nsec <<= shift_change;
+	}
 	timekeeper.shift = clock->shift;
 
 	timekeeper.ntp_error = 0;
@@ -145,6 +181,7 @@ static inline s64 timekeeping_get_ns(void)
 {
 	cycle_t cycle_now, cycle_delta;
 	struct clocksource *clock;
+	s64 nsec;
 
 	/* read clocksource: */
 	clock = timekeeper.clock;
@@ -153,9 +190,8 @@ static inline s64 timekeeping_get_ns(void)
 	/* calculate the delta since the last update_wall_time: */
 	cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
 
-	/* return delta convert to nanoseconds using ntp adjusted mult. */
-	return clocksource_cyc2ns(cycle_delta, timekeeper.mult,
-				  timekeeper.shift);
+	nsec = cycle_delta * timekeeper.mult + timekeeper.xtime_nsec;
+	return nsec >> timekeeper.shift;
 }
 
 static inline s64 timekeeping_get_ns_raw(void)
@@ -185,12 +221,15 @@ static void update_rt_offset(void)
 /* must hold write on timekeeper.lock */
 static void timekeeping_update(bool clearntp)
 {
+	struct timespec xt;
+
 	if (clearntp) {
 		timekeeper.ntp_error = 0;
 		ntp_clear();
 	}
 	update_rt_offset();
-	update_vsyscall(&timekeeper.xtime, &timekeeper.wall_to_monotonic,
+	xt = tk_xtime(&timekeeper);
+	update_vsyscall(&xt, &timekeeper.wall_to_monotonic,
 			 timekeeper.clock, timekeeper.mult);
 }
 
@@ -213,13 +252,12 @@ static void timekeeping_forward_now(void)
 	cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
 	clock->cycle_last = cycle_now;
 
-	nsec = clocksource_cyc2ns(cycle_delta, timekeeper.mult,
-				  timekeeper.shift);
+	timekeeper.xtime_nsec += cycle_delta * timekeeper.mult;
 
 	/* If arch requires, add in gettimeoffset() */
-	nsec += arch_gettimeoffset();
+	timekeeper.xtime_nsec += arch_gettimeoffset() << timekeeper.shift;
 
-	timespec_add_ns(&timekeeper.xtime, nsec);
+	tk_normalize_xtime(&timekeeper);
 
 	nsec = clocksource_cyc2ns(cycle_delta, clock->mult, clock->shift);
 	timespec_add_ns(&timekeeper.raw_time, nsec);
@@ -234,15 +272,15 @@ static void timekeeping_forward_now(void)
 void getnstimeofday(struct timespec *ts)
 {
 	unsigned long seq;
-	s64 nsecs;
+	s64 nsecs = 0;
 
 	WARN_ON(timekeeping_suspended);
 
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
 
-		*ts = timekeeper.xtime;
-		nsecs = timekeeping_get_ns();
+		ts->tv_sec = timekeeper.xtime_sec;
+		ts->tv_nsec = timekeeping_get_ns();
 
 		/* If arch requires, add in gettimeoffset() */
 		nsecs += arch_gettimeoffset();
@@ -262,11 +300,10 @@ ktime_t ktime_get(void)
 
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
-		secs = timekeeper.xtime.tv_sec +
+		secs = timekeeper.xtime_sec +
 				timekeeper.wall_to_monotonic.tv_sec;
-		nsecs = timekeeper.xtime.tv_nsec +
+		nsecs = timekeeping_get_ns() +
 				timekeeper.wall_to_monotonic.tv_nsec;
-		nsecs += timekeeping_get_ns();
 		/* If arch requires, add in gettimeoffset() */
 		nsecs += arch_gettimeoffset();
 
@@ -291,22 +328,21 @@ void ktime_get_ts(struct timespec *ts)
 {
 	struct timespec tomono;
 	unsigned int seq;
-	s64 nsecs;
 
 	WARN_ON(timekeeping_suspended);
 
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
-		*ts = timekeeper.xtime;
+		ts->tv_sec = timekeeper.xtime_sec;
+		ts->tv_nsec = timekeeping_get_ns();
 		tomono = timekeeper.wall_to_monotonic;
-		nsecs = timekeeping_get_ns();
 		/* If arch requires, add in gettimeoffset() */
-		nsecs += arch_gettimeoffset();
+		ts->tv_nsec += arch_gettimeoffset();
 
 	} while (read_seqretry(&timekeeper.lock, seq));
 
 	set_normalized_timespec(ts, ts->tv_sec + tomono.tv_sec,
-				ts->tv_nsec + tomono.tv_nsec + nsecs);
+				ts->tv_nsec + tomono.tv_nsec);
 }
 EXPORT_SYMBOL_GPL(ktime_get_ts);
 
@@ -334,7 +370,8 @@ void getnstime_raw_and_real(struct timespec *ts_raw, struct timespec *ts_real)
 		seq = read_seqbegin(&timekeeper.lock);
 
 		*ts_raw = timekeeper.raw_time;
-		*ts_real = timekeeper.xtime;
+		ts_real->tv_sec = timekeeper.xtime_sec;
+		ts_real->tv_nsec = 0;
 
 		nsecs_raw = timekeeping_get_ns_raw();
 		nsecs_real = timekeeping_get_ns();
@@ -377,7 +414,7 @@ EXPORT_SYMBOL(do_gettimeofday);
  */
 int do_settimeofday(const struct timespec *tv)
 {
-	struct timespec ts_delta;
+	struct timespec ts_delta, xt;
 	unsigned long flags;
 
 	if ((unsigned long)tv->tv_nsec >= NSEC_PER_SEC)
@@ -387,12 +424,15 @@ int do_settimeofday(const struct timespec *tv)
 
 	timekeeping_forward_now();
 
-	ts_delta.tv_sec = tv->tv_sec - timekeeper.xtime.tv_sec;
-	ts_delta.tv_nsec = tv->tv_nsec - timekeeper.xtime.tv_nsec;
+	xt = tk_xtime(&timekeeper);
+	ts_delta.tv_sec = tv->tv_sec - xt.tv_sec;
+	ts_delta.tv_nsec = tv->tv_nsec - xt.tv_nsec;
+
 	timekeeper.wall_to_monotonic =
 			timespec_sub(timekeeper.wall_to_monotonic, ts_delta);
 
-	timekeeper.xtime = *tv;
+	tk_set_xtime(&timekeeper, tv);
+
 	timekeeping_update(true);
 
 	write_sequnlock_irqrestore(&timekeeper.lock, flags);
@@ -422,7 +462,8 @@ int timekeeping_inject_offset(struct timespec *ts)
 
 	timekeeping_forward_now();
 
-	timekeeper.xtime = timespec_add(timekeeper.xtime, *ts);
+
+	tk_xtime_add(&timekeeper, ts);
 	timekeeper.wall_to_monotonic =
 				timespec_sub(timekeeper.wall_to_monotonic, *ts);
 
@@ -606,14 +647,12 @@ void __init timekeeping_init(void)
 		clock->enable(clock);
 	timekeeper_setup_internals(clock);
 
-	timekeeper.xtime.tv_sec = now.tv_sec;
-	timekeeper.xtime.tv_nsec = now.tv_nsec;
+	tk_set_xtime(&timekeeper, &now);
 	timekeeper.raw_time.tv_sec = 0;
 	timekeeper.raw_time.tv_nsec = 0;
-	if (boot.tv_sec == 0 && boot.tv_nsec == 0) {
-		boot.tv_sec = timekeeper.xtime.tv_sec;
-		boot.tv_nsec = timekeeper.xtime.tv_nsec;
-	}
+	if (boot.tv_sec == 0 && boot.tv_nsec == 0)
+		boot = tk_xtime(&timekeeper);
+
 	set_normalized_timespec(&timekeeper.wall_to_monotonic,
 				-boot.tv_sec, -boot.tv_nsec);
 	update_rt_offset();
@@ -646,7 +685,7 @@ static void __timekeeping_inject_sleeptime(struct timespec *delta)
 		return;
 	}
 
-	timekeeper.xtime = timespec_add(timekeeper.xtime, *delta);
+	tk_xtime_add(&timekeeper, delta);
 	timekeeper.wall_to_monotonic =
 			timespec_sub(timekeeper.wall_to_monotonic, *delta);
 	update_sleep_time(timespec_add(timekeeper.total_sleep_time, *delta));
@@ -742,7 +781,7 @@ static int timekeeping_suspend(void)
 	 * try to compensate so the difference in system time
 	 * and persistent_clock time stays close to constant.
 	 */
-	delta = timespec_sub(timekeeper.xtime, timekeeping_suspend_time);
+	delta = timespec_sub(tk_xtime(&timekeeper), timekeeping_suspend_time);
 	delta_delta = timespec_sub(delta, old_delta);
 	if (abs(delta_delta.tv_sec)  >= 2) {
 		/*
@@ -977,9 +1016,9 @@ static cycle_t logarithmic_accumulation(cycle_t offset, u32 shift)
 	while (timekeeper.xtime_nsec >= nsecps) {
 		int leap;
 		timekeeper.xtime_nsec -= nsecps;
-		timekeeper.xtime.tv_sec++;
-		leap = second_overflow(timekeeper.xtime.tv_sec);
-		timekeeper.xtime.tv_sec += leap;
+		timekeeper.xtime_sec++;
+		leap = second_overflow(timekeeper.xtime_sec);
+		timekeeper.xtime_sec += leap;
 		timekeeper.wall_to_monotonic.tv_sec -= leap;
 		if (leap)
 			clock_was_set_delayed();
@@ -1015,6 +1054,7 @@ static void update_wall_time(void)
 	cycle_t offset;
 	int shift = 0, maxshift;
 	unsigned long flags;
+	s64 remainder;
 
 	write_seqlock_irqsave(&timekeeper.lock, flags);
 
@@ -1029,8 +1069,6 @@ static void update_wall_time(void)
 #else
 	offset = (clock->read(clock) - clock->cycle_last) & clock->mask;
 #endif
-	timekeeper.xtime_nsec = (s64)timekeeper.xtime.tv_nsec <<
-						timekeeper.shift;
 
 	/*
 	 * With NO_HZ we may have to accumulate many cycle_intervals
@@ -1076,28 +1114,31 @@ static void update_wall_time(void)
 		timekeeper.ntp_error += neg << timekeeper.ntp_error_shift;
 	}
 
-
 	/*
-	 * Store full nanoseconds into xtime after rounding it up and
-	 * add the remainder to the error difference.
-	 */
-	timekeeper.xtime.tv_nsec = ((s64)timekeeper.xtime_nsec >>
-						timekeeper.shift) + 1;
-	timekeeper.xtime_nsec -= (s64)timekeeper.xtime.tv_nsec <<
-						timekeeper.shift;
-	timekeeper.ntp_error +=	timekeeper.xtime_nsec <<
-				timekeeper.ntp_error_shift;
+	* Store only full nanoseconds into xtime_nsec after rounding
+	* it up and add the remainder to the error difference.
+	* XXX - This is necessary to avoid small 1ns inconsistnecies caused
+	* by truncating the remainder in vsyscalls. However, it causes
+	* additional work to be done in timekeeping_adjust(). Once
+	* the vsyscall implementations are converted to use xtime_nsec
+	* (shifted nanoseconds), this can be killed.
+	*/
+	remainder = timekeeper.xtime_nsec & ((1 << timekeeper.shift) - 1);
+	timekeeper.xtime_nsec -= remainder;
+	timekeeper.xtime_nsec += 1 << timekeeper.shift;
+	timekeeper.ntp_error += remainder << timekeeper.ntp_error_shift;
 
 	/*
 	 * Finally, make sure that after the rounding
-	 * xtime.tv_nsec isn't larger than NSEC_PER_SEC
+	 * xtime_nsec isn't larger than NSEC_PER_SEC
 	 */
-	if (unlikely(timekeeper.xtime.tv_nsec >= NSEC_PER_SEC)) {
+	if (unlikely(timekeeper.xtime_nsec >=
+			((u64)NSEC_PER_SEC << timekeeper.shift))) {
 		int leap;
-		timekeeper.xtime.tv_nsec -= NSEC_PER_SEC;
-		timekeeper.xtime.tv_sec++;
-		leap = second_overflow(timekeeper.xtime.tv_sec);
-		timekeeper.xtime.tv_sec += leap;
+		timekeeper.xtime_nsec -= (u64)NSEC_PER_SEC << timekeeper.shift;
+		timekeeper.xtime_sec++;
+		leap = second_overflow(timekeeper.xtime_sec);
+		timekeeper.xtime_sec += leap;
 		timekeeper.wall_to_monotonic.tv_sec -= leap;
 		if (leap)
 			clock_was_set_delayed();
@@ -1148,21 +1189,20 @@ void get_monotonic_boottime(struct timespec *ts)
 {
 	struct timespec tomono, sleep;
 	unsigned int seq;
-	s64 nsecs;
 
 	WARN_ON(timekeeping_suspended);
 
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
-		*ts = timekeeper.xtime;
+		ts->tv_sec = timekeeper.xtime_sec;
+		ts->tv_nsec = timekeeping_get_ns();
 		tomono = timekeeper.wall_to_monotonic;
 		sleep = timekeeper.total_sleep_time;
-		nsecs = timekeeping_get_ns();
 
 	} while (read_seqretry(&timekeeper.lock, seq));
 
 	set_normalized_timespec(ts, ts->tv_sec + tomono.tv_sec + sleep.tv_sec,
-			ts->tv_nsec + tomono.tv_nsec + sleep.tv_nsec + nsecs);
+			ts->tv_nsec + tomono.tv_nsec + sleep.tv_nsec);
 }
 EXPORT_SYMBOL_GPL(get_monotonic_boottime);
 
@@ -1195,13 +1235,13 @@ EXPORT_SYMBOL_GPL(monotonic_to_bootbased);
 
 unsigned long get_seconds(void)
 {
-	return timekeeper.xtime.tv_sec;
+	return timekeeper.xtime_sec;
 }
 EXPORT_SYMBOL(get_seconds);
 
 struct timespec __current_kernel_time(void)
 {
-	return timekeeper.xtime;
+	return tk_xtime(&timekeeper);
 }
 
 struct timespec current_kernel_time(void)
@@ -1212,7 +1252,7 @@ struct timespec current_kernel_time(void)
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
 
-		now = timekeeper.xtime;
+		now = tk_xtime(&timekeeper);
 	} while (read_seqretry(&timekeeper.lock, seq));
 
 	return now;
@@ -1227,7 +1267,7 @@ struct timespec get_monotonic_coarse(void)
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
 
-		now = timekeeper.xtime;
+		now = tk_xtime(&timekeeper);
 		mono = timekeeper.wall_to_monotonic;
 	} while (read_seqretry(&timekeeper.lock, seq));
 
@@ -1262,7 +1302,7 @@ void get_xtime_and_monotonic_and_sleep_offset(struct timespec *xtim,
 
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
-		*xtim = timekeeper.xtime;
+		*xtim = tk_xtime(&timekeeper);
 		*wtom = timekeeper.wall_to_monotonic;
 		*sleep = timekeeper.total_sleep_time;
 	} while (read_seqretry(&timekeeper.lock, seq));
@@ -1286,9 +1326,8 @@ ktime_t ktime_get_update_offsets(ktime_t *offs_real, ktime_t *offs_boot)
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
 
-		secs = timekeeper.xtime.tv_sec;
-		nsecs = timekeeper.xtime.tv_nsec;
-		nsecs += timekeeping_get_ns();
+		secs = timekeeper.xtime_sec;
+		nsecs = timekeeping_get_ns();
 		/* If arch requires, add in gettimeoffset() */
 		nsecs += arch_gettimeoffset();
 

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [tip:timers/core] time: Refactor accumulation of nsecs to secs
  2012-07-13  5:21 ` [PATCH 5/8] time: Refactor accumulation of nsecs to secs John Stultz
  2012-07-13  5:56   ` Ingo Molnar
  2012-07-13  6:01   ` Ingo Molnar
@ 2012-07-15  8:58   ` tip-bot for John Stultz
  2 siblings, 0 replies; 34+ messages in thread
From: tip-bot for John Stultz @ 2012-07-15  8:58 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, richardcochran,
	john.stultz, tglx, prarit

Commit-ID:  1f4f948706bcec1b51bf6492bf04057d2e21e273
Gitweb:     http://git.kernel.org/tip/1f4f948706bcec1b51bf6492bf04057d2e21e273
Author:     John Stultz <john.stultz@linaro.org>
AuthorDate: Fri, 13 Jul 2012 01:21:54 -0400
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 15 Jul 2012 10:39:06 +0200

time: Refactor accumulation of nsecs to secs

We do the exact same logic moving nsecs to secs in the
timekeeper in multiple places, so condense this into a
single function.

Signed-off-by: John Stultz <john.stultz@linaro.org>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Link: http://lkml.kernel.org/r/1342156917-25092-6-git-send-email-john.stultz@linaro.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/timekeeping.c |   54 ++++++++++++++++++++++++++------------------
 1 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index b98d9bd..cb4a433ba 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -991,6 +991,35 @@ static void timekeeping_adjust(s64 offset)
 
 
 /**
+ * accumulate_nsecs_to_secs - Accumulates nsecs into secs
+ *
+ * Helper function that accumulates a the nsecs greater then a second
+ * from the xtime_nsec field to the xtime_secs field.
+ * It also calls into the NTP code to handle leapsecond processing.
+ *
+ */
+static inline void accumulate_nsecs_to_secs(struct timekeeper *tk)
+{
+	u64 nsecps = (u64)NSEC_PER_SEC << tk->shift;
+
+	while (tk->xtime_nsec >= nsecps) {
+		int leap;
+
+		tk->xtime_nsec -= nsecps;
+		tk->xtime_sec++;
+
+		/* Figure out if its a leap sec and apply if needed */
+		leap = second_overflow(tk->xtime_sec);
+		tk->xtime_sec += leap;
+		tk->wall_to_monotonic.tv_sec -= leap;
+		if (leap)
+			clock_was_set_delayed();
+
+	}
+}
+
+
+/**
  * logarithmic_accumulation - shifted accumulation of cycles
  *
  * This functions accumulates a shifted interval of cycles into
@@ -1001,7 +1030,6 @@ static void timekeeping_adjust(s64 offset)
  */
 static cycle_t logarithmic_accumulation(cycle_t offset, u32 shift)
 {
-	u64 nsecps = (u64)NSEC_PER_SEC << timekeeper.shift;
 	u64 raw_nsecs;
 
 	/* If the offset is smaller than a shifted interval, do nothing */
@@ -1013,16 +1041,8 @@ static cycle_t logarithmic_accumulation(cycle_t offset, u32 shift)
 	timekeeper.clock->cycle_last += timekeeper.cycle_interval << shift;
 
 	timekeeper.xtime_nsec += timekeeper.xtime_interval << shift;
-	while (timekeeper.xtime_nsec >= nsecps) {
-		int leap;
-		timekeeper.xtime_nsec -= nsecps;
-		timekeeper.xtime_sec++;
-		leap = second_overflow(timekeeper.xtime_sec);
-		timekeeper.xtime_sec += leap;
-		timekeeper.wall_to_monotonic.tv_sec -= leap;
-		if (leap)
-			clock_was_set_delayed();
-	}
+
+	accumulate_nsecs_to_secs(&timekeeper);
 
 	/* Accumulate raw time */
 	raw_nsecs = timekeeper.raw_interval << shift;
@@ -1132,17 +1152,7 @@ static void update_wall_time(void)
 	 * Finally, make sure that after the rounding
 	 * xtime_nsec isn't larger than NSEC_PER_SEC
 	 */
-	if (unlikely(timekeeper.xtime_nsec >=
-			((u64)NSEC_PER_SEC << timekeeper.shift))) {
-		int leap;
-		timekeeper.xtime_nsec -= (u64)NSEC_PER_SEC << timekeeper.shift;
-		timekeeper.xtime_sec++;
-		leap = second_overflow(timekeeper.xtime_sec);
-		timekeeper.xtime_sec += leap;
-		timekeeper.wall_to_monotonic.tv_sec -= leap;
-		if (leap)
-			clock_was_set_delayed();
-	}
+	accumulate_nsecs_to_secs(&timekeeper);
 
 	timekeeping_update(false);
 

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [tip:timers/core] time: Move arch_gettimeoffset() usage into timekeeping_get_ns()
  2012-07-13  5:21 ` [PATCH 6/8] time: Move arch_gettimeoffset() usage into timekeeping_get_ns() John Stultz
  2012-07-13  6:03   ` Ingo Molnar
@ 2012-07-15  8:59   ` tip-bot for John Stultz
  1 sibling, 0 replies; 34+ messages in thread
From: tip-bot for John Stultz @ 2012-07-15  8:59 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, johnstul, richardcochran,
	john.stultz, tglx, prarit

Commit-ID:  f2a5a0854efc62abe7f69e9947842cb135837f9a
Gitweb:     http://git.kernel.org/tip/f2a5a0854efc62abe7f69e9947842cb135837f9a
Author:     John Stultz <john.stultz@linaro.org>
AuthorDate: Fri, 13 Jul 2012 01:21:55 -0400
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 15 Jul 2012 10:39:06 +0200

time: Move arch_gettimeoffset() usage into timekeeping_get_ns()

Since we call arch_gettimeoffset() in all the accessor
functions, move arch_gettimeoffset() calls into
timekeeping_get_ns() and timekeeping_get_ns_raw() to simplify
the code.

This also makes the code easier to maintain as we don't have to
worry about forgetting the arch_gettimeoffset() as has happened
in the past.

Signed-off-by: John Stultz <johnstul@us.ibm.com>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Link: http://lkml.kernel.org/r/1342156917-25092-7-git-send-email-john.stultz@linaro.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/timekeeping.c |   29 ++++++++++-------------------
 1 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index cb4a433ba..e43289d 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -191,13 +191,17 @@ static inline s64 timekeeping_get_ns(void)
 	cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
 
 	nsec = cycle_delta * timekeeper.mult + timekeeper.xtime_nsec;
-	return nsec >> timekeeper.shift;
+	nsec >>= timekeeper.shift;
+
+	/* If arch requires, add in gettimeoffset() */
+	return nsec + arch_gettimeoffset();
 }
 
 static inline s64 timekeeping_get_ns_raw(void)
 {
 	cycle_t cycle_now, cycle_delta;
 	struct clocksource *clock;
+	s64 nsec;
 
 	/* read clocksource: */
 	clock = timekeeper.clock;
@@ -206,8 +210,11 @@ static inline s64 timekeeping_get_ns_raw(void)
 	/* calculate the delta since the last update_wall_time: */
 	cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
 
-	/* return delta convert to nanoseconds. */
-	return clocksource_cyc2ns(cycle_delta, clock->mult, clock->shift);
+	/* convert delta to nanoseconds. */
+	nsec = clocksource_cyc2ns(cycle_delta, clock->mult, clock->shift);
+
+	/* If arch requires, add in gettimeoffset() */
+	return nsec + arch_gettimeoffset();
 }
 
 static void update_rt_offset(void)
@@ -282,9 +289,6 @@ void getnstimeofday(struct timespec *ts)
 		ts->tv_sec = timekeeper.xtime_sec;
 		ts->tv_nsec = timekeeping_get_ns();
 
-		/* If arch requires, add in gettimeoffset() */
-		nsecs += arch_gettimeoffset();
-
 	} while (read_seqretry(&timekeeper.lock, seq));
 
 	timespec_add_ns(ts, nsecs);
@@ -304,8 +308,6 @@ ktime_t ktime_get(void)
 				timekeeper.wall_to_monotonic.tv_sec;
 		nsecs = timekeeping_get_ns() +
 				timekeeper.wall_to_monotonic.tv_nsec;
-		/* If arch requires, add in gettimeoffset() */
-		nsecs += arch_gettimeoffset();
 
 	} while (read_seqretry(&timekeeper.lock, seq));
 	/*
@@ -336,8 +338,6 @@ void ktime_get_ts(struct timespec *ts)
 		ts->tv_sec = timekeeper.xtime_sec;
 		ts->tv_nsec = timekeeping_get_ns();
 		tomono = timekeeper.wall_to_monotonic;
-		/* If arch requires, add in gettimeoffset() */
-		ts->tv_nsec += arch_gettimeoffset();
 
 	} while (read_seqretry(&timekeeper.lock, seq));
 
@@ -365,8 +365,6 @@ void getnstime_raw_and_real(struct timespec *ts_raw, struct timespec *ts_real)
 	WARN_ON_ONCE(timekeeping_suspended);
 
 	do {
-		u32 arch_offset;
-
 		seq = read_seqbegin(&timekeeper.lock);
 
 		*ts_raw = timekeeper.raw_time;
@@ -376,11 +374,6 @@ void getnstime_raw_and_real(struct timespec *ts_raw, struct timespec *ts_real)
 		nsecs_raw = timekeeping_get_ns_raw();
 		nsecs_real = timekeeping_get_ns();
 
-		/* If arch requires, add in gettimeoffset() */
-		arch_offset = arch_gettimeoffset();
-		nsecs_raw += arch_offset;
-		nsecs_real += arch_offset;
-
 	} while (read_seqretry(&timekeeper.lock, seq));
 
 	timespec_add_ns(ts_raw, nsecs_raw);
@@ -1338,8 +1331,6 @@ ktime_t ktime_get_update_offsets(ktime_t *offs_real, ktime_t *offs_boot)
 
 		secs = timekeeper.xtime_sec;
 		nsecs = timekeeping_get_ns();
-		/* If arch requires, add in gettimeoffset() */
-		nsecs += arch_gettimeoffset();
 
 		*offs_real = timekeeper.offs_real;
 		*offs_boot = timekeeper.offs_boot;

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [tip:timers/core] time: Move xtime_nsec adjustment underflow handling timekeeping_adjust
  2012-07-13  5:21 ` [PATCH 7/8] time: Move xtime_nsec adjustment underflow handling timekeeping_adjust John Stultz
  2012-07-13  6:04   ` Ingo Molnar
@ 2012-07-15  9:00   ` tip-bot for John Stultz
  1 sibling, 0 replies; 34+ messages in thread
From: tip-bot for John Stultz @ 2012-07-15  9:00 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, johnstul, richardcochran,
	john.stultz, tglx, prarit

Commit-ID:  2a8c0883c3cfffcc148ea606e2a4e7453cd75e73
Gitweb:     http://git.kernel.org/tip/2a8c0883c3cfffcc148ea606e2a4e7453cd75e73
Author:     John Stultz <john.stultz@linaro.org>
AuthorDate: Fri, 13 Jul 2012 01:21:56 -0400
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 15 Jul 2012 10:39:07 +0200

time: Move xtime_nsec adjustment underflow handling timekeeping_adjust

When we make adjustments speeding up the clock, its possible
for xtime_nsec to underflow. We already handle this properly,
but we do so from update_wall_time() instead of the more logical
timekeeping_adjust(), where the possible underflow actually
occurs.

Thus, move the correction logic to the timekeeping_adjust, which
is the function that causes the issue. Making update_wall_time()
more readable.

Signed-off-by: John Stultz <johnstul@us.ibm.com>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Link: http://lkml.kernel.org/r/1342156917-25092-8-git-send-email-john.stultz@linaro.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/timekeeping.c |   42 +++++++++++++++++++++---------------------
 1 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index e43289d..aeeaab8 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -980,6 +980,27 @@ static void timekeeping_adjust(s64 offset)
 	timekeeper.xtime_nsec -= offset;
 	timekeeper.ntp_error -= (interval - offset) <<
 				timekeeper.ntp_error_shift;
+
+	/*
+	 * It may be possible that when we entered this function, xtime_nsec
+	 * was very small.  Further, if we're slightly speeding the clocksource
+	 * in the code above, its possible the required corrective factor to
+	 * xtime_nsec could cause it to underflow.
+	 *
+	 * Now, since we already accumulated the second, cannot simply roll
+	 * the accumulated second back, since the NTP subsystem has been
+	 * notified via second_overflow. So instead we push xtime_nsec forward
+	 * by the amount we underflowed, and add that amount into the error.
+	 *
+	 * We'll correct this error next time through this function, when
+	 * xtime_nsec is not as small.
+	 */
+	if (unlikely((s64)timekeeper.xtime_nsec < 0)) {
+		s64 neg = -(s64)timekeeper.xtime_nsec;
+		timekeeper.xtime_nsec = 0;
+		timekeeper.ntp_error += neg << timekeeper.ntp_error_shift;
+	}
+
 }
 
 
@@ -1105,27 +1126,6 @@ static void update_wall_time(void)
 	/* correct the clock when NTP error is too big */
 	timekeeping_adjust(offset);
 
-	/*
-	 * Since in the loop above, we accumulate any amount of time
-	 * in xtime_nsec over a second into xtime.tv_sec, its possible for
-	 * xtime_nsec to be fairly small after the loop. Further, if we're
-	 * slightly speeding the clocksource up in timekeeping_adjust(),
-	 * its possible the required corrective factor to xtime_nsec could
-	 * cause it to underflow.
-	 *
-	 * Now, we cannot simply roll the accumulated second back, since
-	 * the NTP subsystem has been notified via second_overflow. So
-	 * instead we push xtime_nsec forward by the amount we underflowed,
-	 * and add that amount into the error.
-	 *
-	 * We'll correct this error next time through this function, when
-	 * xtime_nsec is not as small.
-	 */
-	if (unlikely((s64)timekeeper.xtime_nsec < 0)) {
-		s64 neg = -(s64)timekeeper.xtime_nsec;
-		timekeeper.xtime_nsec = 0;
-		timekeeper.ntp_error += neg << timekeeper.ntp_error_shift;
-	}
 
 	/*
 	* Store only full nanoseconds into xtime_nsec after rounding

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [tip:timers/core] time: Rework timekeeping functions to take timekeeper ptr as argument
  2012-07-13  5:21 ` [PATCH 8/8] time: Rework timekeeping functions to take timekeeper ptr as argument John Stultz
@ 2012-07-15  9:01   ` tip-bot for John Stultz
  0 siblings, 0 replies; 34+ messages in thread
From: tip-bot for John Stultz @ 2012-07-15  9:01 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, richardcochran,
	john.stultz, tglx, prarit

Commit-ID:  f726a697d06102e7a1fc0a87308cb30a84580205
Gitweb:     http://git.kernel.org/tip/f726a697d06102e7a1fc0a87308cb30a84580205
Author:     John Stultz <john.stultz@linaro.org>
AuthorDate: Fri, 13 Jul 2012 01:21:57 -0400
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 15 Jul 2012 10:39:07 +0200

time: Rework timekeeping functions to take timekeeper ptr as argument

As part of cleaning up the timekeeping code, this patch converts
a number of internal functions to takei a timekeeper ptr as an
argument, so that the internal functions don't access the global
timekeeper structure directly. This allows for further optimizations
to reduce lock hold time later.

This patch has been updated to include more consistent usage of the
timekeeper value, by making sure it is always passed as a argument
to non top-level functions.

Signed-off-by: John Stultz <john.stultz@linaro.org>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Link: http://lkml.kernel.org/r/1342156917-25092-9-git-send-email-john.stultz@linaro.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/timekeeping.c |  208 ++++++++++++++++++++++-----------------------
 1 files changed, 103 insertions(+), 105 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index aeeaab8..5980e90 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -127,14 +127,14 @@ static void tk_xtime_add(struct timekeeper *tk, const struct timespec *ts)
  *
  * Unless you're the timekeeping code, you should not be using this!
  */
-static void timekeeper_setup_internals(struct clocksource *clock)
+static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
 {
 	cycle_t interval;
 	u64 tmp, ntpinterval;
 	struct clocksource *old_clock;
 
-	old_clock = timekeeper.clock;
-	timekeeper.clock = clock;
+	old_clock = tk->clock;
+	tk->clock = clock;
 	clock->cycle_last = clock->read(clock);
 
 	/* Do the ns -> cycle conversion first, using original mult */
@@ -147,64 +147,64 @@ static void timekeeper_setup_internals(struct clocksource *clock)
 		tmp = 1;
 
 	interval = (cycle_t) tmp;
-	timekeeper.cycle_interval = interval;
+	tk->cycle_interval = interval;
 
 	/* Go back from cycles -> shifted ns */
-	timekeeper.xtime_interval = (u64) interval * clock->mult;
-	timekeeper.xtime_remainder = ntpinterval - timekeeper.xtime_interval;
-	timekeeper.raw_interval =
+	tk->xtime_interval = (u64) interval * clock->mult;
+	tk->xtime_remainder = ntpinterval - tk->xtime_interval;
+	tk->raw_interval =
 		((u64) interval * clock->mult) >> clock->shift;
 
 	 /* if changing clocks, convert xtime_nsec shift units */
 	if (old_clock) {
 		int shift_change = clock->shift - old_clock->shift;
 		if (shift_change < 0)
-			timekeeper.xtime_nsec >>= -shift_change;
+			tk->xtime_nsec >>= -shift_change;
 		else
-			timekeeper.xtime_nsec <<= shift_change;
+			tk->xtime_nsec <<= shift_change;
 	}
-	timekeeper.shift = clock->shift;
+	tk->shift = clock->shift;
 
-	timekeeper.ntp_error = 0;
-	timekeeper.ntp_error_shift = NTP_SCALE_SHIFT - clock->shift;
+	tk->ntp_error = 0;
+	tk->ntp_error_shift = NTP_SCALE_SHIFT - clock->shift;
 
 	/*
 	 * The timekeeper keeps its own mult values for the currently
 	 * active clocksource. These value will be adjusted via NTP
 	 * to counteract clock drifting.
 	 */
-	timekeeper.mult = clock->mult;
+	tk->mult = clock->mult;
 }
 
 /* Timekeeper helper functions. */
-static inline s64 timekeeping_get_ns(void)
+static inline s64 timekeeping_get_ns(struct timekeeper *tk)
 {
 	cycle_t cycle_now, cycle_delta;
 	struct clocksource *clock;
 	s64 nsec;
 
 	/* read clocksource: */
-	clock = timekeeper.clock;
+	clock = tk->clock;
 	cycle_now = clock->read(clock);
 
 	/* calculate the delta since the last update_wall_time: */
 	cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
 
-	nsec = cycle_delta * timekeeper.mult + timekeeper.xtime_nsec;
-	nsec >>= timekeeper.shift;
+	nsec = cycle_delta * tk->mult + tk->xtime_nsec;
+	nsec >>= tk->shift;
 
 	/* If arch requires, add in gettimeoffset() */
 	return nsec + arch_gettimeoffset();
 }
 
-static inline s64 timekeeping_get_ns_raw(void)
+static inline s64 timekeeping_get_ns_raw(struct timekeeper *tk)
 {
 	cycle_t cycle_now, cycle_delta;
 	struct clocksource *clock;
 	s64 nsec;
 
 	/* read clocksource: */
-	clock = timekeeper.clock;
+	clock = tk->clock;
 	cycle_now = clock->read(clock);
 
 	/* calculate the delta since the last update_wall_time: */
@@ -217,27 +217,26 @@ static inline s64 timekeeping_get_ns_raw(void)
 	return nsec + arch_gettimeoffset();
 }
 
-static void update_rt_offset(void)
+static void update_rt_offset(struct timekeeper *tk)
 {
-	struct timespec tmp, *wtm = &timekeeper.wall_to_monotonic;
+	struct timespec tmp, *wtm = &tk->wall_to_monotonic;
 
 	set_normalized_timespec(&tmp, -wtm->tv_sec, -wtm->tv_nsec);
-	timekeeper.offs_real = timespec_to_ktime(tmp);
+	tk->offs_real = timespec_to_ktime(tmp);
 }
 
 /* must hold write on timekeeper.lock */
-static void timekeeping_update(bool clearntp)
+static void timekeeping_update(struct timekeeper *tk, bool clearntp)
 {
 	struct timespec xt;
 
 	if (clearntp) {
-		timekeeper.ntp_error = 0;
+		tk->ntp_error = 0;
 		ntp_clear();
 	}
-	update_rt_offset();
-	xt = tk_xtime(&timekeeper);
-	update_vsyscall(&xt, &timekeeper.wall_to_monotonic,
-			 timekeeper.clock, timekeeper.mult);
+	update_rt_offset(tk);
+	xt = tk_xtime(tk);
+	update_vsyscall(&xt, &tk->wall_to_monotonic, tk->clock, tk->mult);
 }
 
 
@@ -248,26 +247,26 @@ static void timekeeping_update(bool clearntp)
  * update_wall_time(). This is useful before significant clock changes,
  * as it avoids having to deal with this time offset explicitly.
  */
-static void timekeeping_forward_now(void)
+static void timekeeping_forward_now(struct timekeeper *tk)
 {
 	cycle_t cycle_now, cycle_delta;
 	struct clocksource *clock;
 	s64 nsec;
 
-	clock = timekeeper.clock;
+	clock = tk->clock;
 	cycle_now = clock->read(clock);
 	cycle_delta = (cycle_now - clock->cycle_last) & clock->mask;
 	clock->cycle_last = cycle_now;
 
-	timekeeper.xtime_nsec += cycle_delta * timekeeper.mult;
+	tk->xtime_nsec += cycle_delta * tk->mult;
 
 	/* If arch requires, add in gettimeoffset() */
-	timekeeper.xtime_nsec += arch_gettimeoffset() << timekeeper.shift;
+	tk->xtime_nsec += arch_gettimeoffset() << tk->shift;
 
-	tk_normalize_xtime(&timekeeper);
+	tk_normalize_xtime(tk);
 
 	nsec = clocksource_cyc2ns(cycle_delta, clock->mult, clock->shift);
-	timespec_add_ns(&timekeeper.raw_time, nsec);
+	timespec_add_ns(&tk->raw_time, nsec);
 }
 
 /**
@@ -287,7 +286,7 @@ void getnstimeofday(struct timespec *ts)
 		seq = read_seqbegin(&timekeeper.lock);
 
 		ts->tv_sec = timekeeper.xtime_sec;
-		ts->tv_nsec = timekeeping_get_ns();
+		ts->tv_nsec = timekeeping_get_ns(&timekeeper);
 
 	} while (read_seqretry(&timekeeper.lock, seq));
 
@@ -306,7 +305,7 @@ ktime_t ktime_get(void)
 		seq = read_seqbegin(&timekeeper.lock);
 		secs = timekeeper.xtime_sec +
 				timekeeper.wall_to_monotonic.tv_sec;
-		nsecs = timekeeping_get_ns() +
+		nsecs = timekeeping_get_ns(&timekeeper) +
 				timekeeper.wall_to_monotonic.tv_nsec;
 
 	} while (read_seqretry(&timekeeper.lock, seq));
@@ -336,7 +335,7 @@ void ktime_get_ts(struct timespec *ts)
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
 		ts->tv_sec = timekeeper.xtime_sec;
-		ts->tv_nsec = timekeeping_get_ns();
+		ts->tv_nsec = timekeeping_get_ns(&timekeeper);
 		tomono = timekeeper.wall_to_monotonic;
 
 	} while (read_seqretry(&timekeeper.lock, seq));
@@ -371,8 +370,8 @@ void getnstime_raw_and_real(struct timespec *ts_raw, struct timespec *ts_real)
 		ts_real->tv_sec = timekeeper.xtime_sec;
 		ts_real->tv_nsec = 0;
 
-		nsecs_raw = timekeeping_get_ns_raw();
-		nsecs_real = timekeeping_get_ns();
+		nsecs_raw = timekeeping_get_ns_raw(&timekeeper);
+		nsecs_real = timekeeping_get_ns(&timekeeper);
 
 	} while (read_seqretry(&timekeeper.lock, seq));
 
@@ -415,7 +414,7 @@ int do_settimeofday(const struct timespec *tv)
 
 	write_seqlock_irqsave(&timekeeper.lock, flags);
 
-	timekeeping_forward_now();
+	timekeeping_forward_now(&timekeeper);
 
 	xt = tk_xtime(&timekeeper);
 	ts_delta.tv_sec = tv->tv_sec - xt.tv_sec;
@@ -426,7 +425,7 @@ int do_settimeofday(const struct timespec *tv)
 
 	tk_set_xtime(&timekeeper, tv);
 
-	timekeeping_update(true);
+	timekeeping_update(&timekeeper, true);
 
 	write_sequnlock_irqrestore(&timekeeper.lock, flags);
 
@@ -453,14 +452,14 @@ int timekeeping_inject_offset(struct timespec *ts)
 
 	write_seqlock_irqsave(&timekeeper.lock, flags);
 
-	timekeeping_forward_now();
+	timekeeping_forward_now(&timekeeper);
 
 
 	tk_xtime_add(&timekeeper, ts);
 	timekeeper.wall_to_monotonic =
 				timespec_sub(timekeeper.wall_to_monotonic, *ts);
 
-	timekeeping_update(true);
+	timekeeping_update(&timekeeper, true);
 
 	write_sequnlock_irqrestore(&timekeeper.lock, flags);
 
@@ -485,14 +484,14 @@ static int change_clocksource(void *data)
 
 	write_seqlock_irqsave(&timekeeper.lock, flags);
 
-	timekeeping_forward_now();
+	timekeeping_forward_now(&timekeeper);
 	if (!new->enable || new->enable(new) == 0) {
 		old = timekeeper.clock;
-		timekeeper_setup_internals(new);
+		tk_setup_internals(&timekeeper, new);
 		if (old->disable)
 			old->disable(old);
 	}
-	timekeeping_update(true);
+	timekeeping_update(&timekeeper, true);
 
 	write_sequnlock_irqrestore(&timekeeper.lock, flags);
 
@@ -542,7 +541,7 @@ void getrawmonotonic(struct timespec *ts)
 
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
-		nsecs = timekeeping_get_ns_raw();
+		nsecs = timekeeping_get_ns_raw(&timekeeper);
 		*ts = timekeeper.raw_time;
 
 	} while (read_seqretry(&timekeeper.lock, seq));
@@ -638,7 +637,7 @@ void __init timekeeping_init(void)
 	clock = clocksource_default_clock();
 	if (clock->enable)
 		clock->enable(clock);
-	timekeeper_setup_internals(clock);
+	tk_setup_internals(&timekeeper, clock);
 
 	tk_set_xtime(&timekeeper, &now);
 	timekeeper.raw_time.tv_sec = 0;
@@ -648,7 +647,7 @@ void __init timekeeping_init(void)
 
 	set_normalized_timespec(&timekeeper.wall_to_monotonic,
 				-boot.tv_sec, -boot.tv_nsec);
-	update_rt_offset();
+	update_rt_offset(&timekeeper);
 	timekeeper.total_sleep_time.tv_sec = 0;
 	timekeeper.total_sleep_time.tv_nsec = 0;
 	write_sequnlock_irqrestore(&timekeeper.lock, flags);
@@ -670,7 +669,8 @@ static void update_sleep_time(struct timespec t)
  * Takes a timespec offset measuring a suspend interval and properly
  * adds the sleep offset to the timekeeping variables.
  */
-static void __timekeeping_inject_sleeptime(struct timespec *delta)
+static void __timekeeping_inject_sleeptime(struct timekeeper *tk,
+							struct timespec *delta)
 {
 	if (!timespec_valid(delta)) {
 		printk(KERN_WARNING "__timekeeping_inject_sleeptime: Invalid "
@@ -678,10 +678,9 @@ static void __timekeeping_inject_sleeptime(struct timespec *delta)
 		return;
 	}
 
-	tk_xtime_add(&timekeeper, delta);
-	timekeeper.wall_to_monotonic =
-			timespec_sub(timekeeper.wall_to_monotonic, *delta);
-	update_sleep_time(timespec_add(timekeeper.total_sleep_time, *delta));
+	tk_xtime_add(tk, delta);
+	tk->wall_to_monotonic = timespec_sub(tk->wall_to_monotonic, *delta);
+	update_sleep_time(timespec_add(tk->total_sleep_time, *delta));
 }
 
 
@@ -707,11 +706,11 @@ void timekeeping_inject_sleeptime(struct timespec *delta)
 
 	write_seqlock_irqsave(&timekeeper.lock, flags);
 
-	timekeeping_forward_now();
+	timekeeping_forward_now(&timekeeper);
 
-	__timekeeping_inject_sleeptime(delta);
+	__timekeeping_inject_sleeptime(&timekeeper, delta);
 
-	timekeeping_update(true);
+	timekeeping_update(&timekeeper, true);
 
 	write_sequnlock_irqrestore(&timekeeper.lock, flags);
 
@@ -740,7 +739,7 @@ static void timekeeping_resume(void)
 
 	if (timespec_compare(&ts, &timekeeping_suspend_time) > 0) {
 		ts = timespec_sub(ts, timekeeping_suspend_time);
-		__timekeeping_inject_sleeptime(&ts);
+		__timekeeping_inject_sleeptime(&timekeeper, &ts);
 	}
 	/* re-base the last cycle value */
 	timekeeper.clock->cycle_last = timekeeper.clock->read(timekeeper.clock);
@@ -765,7 +764,7 @@ static int timekeeping_suspend(void)
 	read_persistent_clock(&timekeeping_suspend_time);
 
 	write_seqlock_irqsave(&timekeeper.lock, flags);
-	timekeeping_forward_now();
+	timekeeping_forward_now(&timekeeper);
 	timekeeping_suspended = 1;
 
 	/*
@@ -813,7 +812,8 @@ device_initcall(timekeeping_init_ops);
  * If the error is already larger, we look ahead even further
  * to compensate for late or lost adjustments.
  */
-static __always_inline int timekeeping_bigadjust(s64 error, s64 *interval,
+static __always_inline int timekeeping_bigadjust(struct timekeeper *tk,
+						 s64 error, s64 *interval,
 						 s64 *offset)
 {
 	s64 tick_error, i;
@@ -829,7 +829,7 @@ static __always_inline int timekeeping_bigadjust(s64 error, s64 *interval,
 	 * here.  This is tuned so that an error of about 1 msec is adjusted
 	 * within about 1 sec (or 2^20 nsec in 2^SHIFT_HZ ticks).
 	 */
-	error2 = timekeeper.ntp_error >> (NTP_SCALE_SHIFT + 22 - 2 * SHIFT_HZ);
+	error2 = tk->ntp_error >> (NTP_SCALE_SHIFT + 22 - 2 * SHIFT_HZ);
 	error2 = abs(error2);
 	for (look_ahead = 0; error2 > 0; look_ahead++)
 		error2 >>= 2;
@@ -838,8 +838,8 @@ static __always_inline int timekeeping_bigadjust(s64 error, s64 *interval,
 	 * Now calculate the error in (1 << look_ahead) ticks, but first
 	 * remove the single look ahead already included in the error.
 	 */
-	tick_error = ntp_tick_length() >> (timekeeper.ntp_error_shift + 1);
-	tick_error -= timekeeper.xtime_interval >> 1;
+	tick_error = ntp_tick_length() >> (tk->ntp_error_shift + 1);
+	tick_error -= tk->xtime_interval >> 1;
 	error = ((error - tick_error) >> look_ahead) + tick_error;
 
 	/* Finally calculate the adjustment shift value.  */
@@ -864,9 +864,9 @@ static __always_inline int timekeeping_bigadjust(s64 error, s64 *interval,
  * this is optimized for the most common adjustments of -1,0,1,
  * for other values we can do a bit more work.
  */
-static void timekeeping_adjust(s64 offset)
+static void timekeeping_adjust(struct timekeeper *tk, s64 offset)
 {
-	s64 error, interval = timekeeper.cycle_interval;
+	s64 error, interval = tk->cycle_interval;
 	int adj;
 
 	/*
@@ -882,7 +882,7 @@ static void timekeeping_adjust(s64 offset)
 	 *
 	 * Note: It does not "save" on aggravation when reading the code.
 	 */
-	error = timekeeper.ntp_error >> (timekeeper.ntp_error_shift - 1);
+	error = tk->ntp_error >> (tk->ntp_error_shift - 1);
 	if (error > interval) {
 		/*
 		 * We now divide error by 4(via shift), which checks if
@@ -904,7 +904,8 @@ static void timekeeping_adjust(s64 offset)
 		if (likely(error <= interval))
 			adj = 1;
 		else
-			adj = timekeeping_bigadjust(error, &interval, &offset);
+			adj = timekeeping_bigadjust(tk, error, &interval,
+							&offset);
 	} else if (error < -interval) {
 		/* See comment above, this is just switched for the negative */
 		error >>= 2;
@@ -913,18 +914,17 @@ static void timekeeping_adjust(s64 offset)
 			interval = -interval;
 			offset = -offset;
 		} else
-			adj = timekeeping_bigadjust(error, &interval, &offset);
-	} else /* No adjustment needed */
+			adj = timekeeping_bigadjust(tk, error, &interval,
+							&offset);
+	} else
 		return;
 
-	if (unlikely(timekeeper.clock->maxadj &&
-			(timekeeper.mult + adj >
-			timekeeper.clock->mult + timekeeper.clock->maxadj))) {
+	if (unlikely(tk->clock->maxadj &&
+		(tk->mult + adj > tk->clock->mult + tk->clock->maxadj))) {
 		printk_once(KERN_WARNING
 			"Adjusting %s more than 11%% (%ld vs %ld)\n",
-			timekeeper.clock->name, (long)timekeeper.mult + adj,
-			(long)timekeeper.clock->mult +
-				timekeeper.clock->maxadj);
+			tk->clock->name, (long)tk->mult + adj,
+			(long)tk->clock->mult + tk->clock->maxadj);
 	}
 	/*
 	 * So the following can be confusing.
@@ -975,11 +975,10 @@ static void timekeeping_adjust(s64 offset)
 	 *
 	 * XXX - TODO: Doc ntp_error calculation.
 	 */
-	timekeeper.mult += adj;
-	timekeeper.xtime_interval += interval;
-	timekeeper.xtime_nsec -= offset;
-	timekeeper.ntp_error -= (interval - offset) <<
-				timekeeper.ntp_error_shift;
+	tk->mult += adj;
+	tk->xtime_interval += interval;
+	tk->xtime_nsec -= offset;
+	tk->ntp_error -= (interval - offset) << tk->ntp_error_shift;
 
 	/*
 	 * It may be possible that when we entered this function, xtime_nsec
@@ -995,10 +994,10 @@ static void timekeeping_adjust(s64 offset)
 	 * We'll correct this error next time through this function, when
 	 * xtime_nsec is not as small.
 	 */
-	if (unlikely((s64)timekeeper.xtime_nsec < 0)) {
-		s64 neg = -(s64)timekeeper.xtime_nsec;
-		timekeeper.xtime_nsec = 0;
-		timekeeper.ntp_error += neg << timekeeper.ntp_error_shift;
+	if (unlikely((s64)tk->xtime_nsec < 0)) {
+		s64 neg = -(s64)tk->xtime_nsec;
+		tk->xtime_nsec = 0;
+		tk->ntp_error += neg << tk->ntp_error_shift;
 	}
 
 }
@@ -1042,37 +1041,36 @@ static inline void accumulate_nsecs_to_secs(struct timekeeper *tk)
  *
  * Returns the unconsumed cycles.
  */
-static cycle_t logarithmic_accumulation(cycle_t offset, u32 shift)
+static cycle_t logarithmic_accumulation(struct timekeeper *tk, cycle_t offset,
+						u32 shift)
 {
 	u64 raw_nsecs;
 
-	/* If the offset is smaller than a shifted interval, do nothing */
-	if (offset < timekeeper.cycle_interval<<shift)
+	/* If the offset is smaller then a shifted interval, do nothing */
+	if (offset < tk->cycle_interval<<shift)
 		return offset;
 
 	/* Accumulate one shifted interval */
-	offset -= timekeeper.cycle_interval << shift;
-	timekeeper.clock->cycle_last += timekeeper.cycle_interval << shift;
+	offset -= tk->cycle_interval << shift;
+	tk->clock->cycle_last += tk->cycle_interval << shift;
 
-	timekeeper.xtime_nsec += timekeeper.xtime_interval << shift;
-
-	accumulate_nsecs_to_secs(&timekeeper);
+	tk->xtime_nsec += tk->xtime_interval << shift;
+	accumulate_nsecs_to_secs(tk);
 
 	/* Accumulate raw time */
-	raw_nsecs = timekeeper.raw_interval << shift;
-	raw_nsecs += timekeeper.raw_time.tv_nsec;
+	raw_nsecs = tk->raw_interval << shift;
+	raw_nsecs += tk->raw_time.tv_nsec;
 	if (raw_nsecs >= NSEC_PER_SEC) {
 		u64 raw_secs = raw_nsecs;
 		raw_nsecs = do_div(raw_secs, NSEC_PER_SEC);
-		timekeeper.raw_time.tv_sec += raw_secs;
+		tk->raw_time.tv_sec += raw_secs;
 	}
-	timekeeper.raw_time.tv_nsec = raw_nsecs;
+	tk->raw_time.tv_nsec = raw_nsecs;
 
 	/* Accumulate error between NTP and clock interval */
-	timekeeper.ntp_error += ntp_tick_length() << shift;
-	timekeeper.ntp_error -=
-	    (timekeeper.xtime_interval + timekeeper.xtime_remainder) <<
-				(timekeeper.ntp_error_shift + shift);
+	tk->ntp_error += ntp_tick_length() << shift;
+	tk->ntp_error -= (tk->xtime_interval + tk->xtime_remainder) <<
+						(tk->ntp_error_shift + shift);
 
 	return offset;
 }
@@ -1118,13 +1116,13 @@ static void update_wall_time(void)
 	maxshift = (64 - (ilog2(ntp_tick_length())+1)) - 1;
 	shift = min(shift, maxshift);
 	while (offset >= timekeeper.cycle_interval) {
-		offset = logarithmic_accumulation(offset, shift);
+		offset = logarithmic_accumulation(&timekeeper, offset, shift);
 		if(offset < timekeeper.cycle_interval<<shift)
 			shift--;
 	}
 
 	/* correct the clock when NTP error is too big */
-	timekeeping_adjust(offset);
+	timekeeping_adjust(&timekeeper, offset);
 
 
 	/*
@@ -1147,7 +1145,7 @@ static void update_wall_time(void)
 	 */
 	accumulate_nsecs_to_secs(&timekeeper);
 
-	timekeeping_update(false);
+	timekeeping_update(&timekeeper, false);
 
 out:
 	write_sequnlock_irqrestore(&timekeeper.lock, flags);
@@ -1198,7 +1196,7 @@ void get_monotonic_boottime(struct timespec *ts)
 	do {
 		seq = read_seqbegin(&timekeeper.lock);
 		ts->tv_sec = timekeeper.xtime_sec;
-		ts->tv_nsec = timekeeping_get_ns();
+		ts->tv_nsec = timekeeping_get_ns(&timekeeper);
 		tomono = timekeeper.wall_to_monotonic;
 		sleep = timekeeper.total_sleep_time;
 
@@ -1330,7 +1328,7 @@ ktime_t ktime_get_update_offsets(ktime_t *offs_real, ktime_t *offs_boot)
 		seq = read_seqbegin(&timekeeper.lock);
 
 		secs = timekeeper.xtime_sec;
-		nsecs = timekeeping_get_ns();
+		nsecs = timekeeping_get_ns(&timekeeper);
 
 		*offs_real = timekeeper.offs_real;
 		*offs_boot = timekeeper.offs_boot;

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 4/8] time: Condense timekeeper.xtime into xtime_sec
  2012-07-13  5:21 ` [PATCH 4/8] time: Condense timekeeper.xtime into xtime_sec John Stultz
  2012-07-13  6:00   ` Ingo Molnar
  2012-07-15  8:57   ` [tip:timers/core] " tip-bot for John Stultz
@ 2012-08-19 21:02   ` Andreas Schwab
  2012-08-20 18:58     ` John Stultz
  2 siblings, 1 reply; 34+ messages in thread
From: Andreas Schwab @ 2012-08-19 21:02 UTC (permalink / raw)
  To: John Stultz
  Cc: Linux Kernel, Ingo Molnar, Peter Zijlstra, Richard Cochran,
	Prarit Bhargava, Thomas Gleixner, linuxppc-dev

John Stultz <john.stultz@linaro.org> writes:

> The timekeeper struct has a xtime_nsec, which keeps the
> sub-nanosecond remainder.  This ends up being somewhat
> duplicative of the timekeeper.xtime.tv_nsec value, and we
> have to do extra work to keep them apart, copying the full
> nsec portion out and back in over and over.
>
> This patch simplifies some of the logic by taking the timekeeper
> xtime value and splitting it into timekeeper.xtime_sec and
> reuses the timekeeper.xtime_nsec for the sub-second portion
> (stored in higher res shifted nanoseconds).
>
> This simplifies some of the accumulation logic. And will
> allow for more accurate timekeeping once the vsyscall code
> is updated to use the shifted nanosecond remainder.

This (together with b44d50d "time: Fix casting issue in tk_set_xtime and
tk_xtime_add") is causing resume to hang on the iBook (PowerBook6,7).
The fact that the add-on commit is needed to uncover the bug might give
a hint, but I'm unable to decipher it.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 4/8] time: Condense timekeeper.xtime into xtime_sec
  2012-08-19 21:02   ` [PATCH 4/8] " Andreas Schwab
@ 2012-08-20 18:58     ` John Stultz
  2012-08-20 19:45       ` Andreas Schwab
  0 siblings, 1 reply; 34+ messages in thread
From: John Stultz @ 2012-08-20 18:58 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: Linux Kernel, Ingo Molnar, Peter Zijlstra, Richard Cochran,
	Prarit Bhargava, Thomas Gleixner, linuxppc-dev

On 08/19/2012 02:02 PM, Andreas Schwab wrote:
> John Stultz <john.stultz@linaro.org> writes:
>
>> The timekeeper struct has a xtime_nsec, which keeps the
>> sub-nanosecond remainder.  This ends up being somewhat
>> duplicative of the timekeeper.xtime.tv_nsec value, and we
>> have to do extra work to keep them apart, copying the full
>> nsec portion out and back in over and over.
>>
>> This patch simplifies some of the logic by taking the timekeeper
>> xtime value and splitting it into timekeeper.xtime_sec and
>> reuses the timekeeper.xtime_nsec for the sub-second portion
>> (stored in higher res shifted nanoseconds).
>>
>> This simplifies some of the accumulation logic. And will
>> allow for more accurate timekeeping once the vsyscall code
>> is updated to use the shifted nanosecond remainder.
> This (together with b44d50d "time: Fix casting issue in tk_set_xtime and
> tk_xtime_add") is causing resume to hang on the iBook (PowerBook6,7).
> The fact that the add-on commit is needed to uncover the bug might give
> a hint, but I'm unable to decipher it.

Thanks for the bug report and narrowing this down.

I'm not very familiar w/ the iBook hardware, but does it use a 
clocksource, or does it use arch_gettimeoffset()?

I suspect that the casting has avoided clipping some strange values from 
the persistent clock.

Could you try with the following patch against Linus' HEAD? I suspect it 
will let the box resume (although it will seem as though no time was 
spent in resume) and then let me know what the JDB lines print out?

thanks
-john

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index e16af19..03f5a82 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -753,10 +753,14 @@ static void timekeeping_resume(void)
  	clocksource_resume();
  
  	write_seqlock_irqsave(&tk->lock, flags);
+	printk("JDB: suspend_time: %ld:%ld  resume_time: %ld:%ld\n",
+		timekeeping_suspend_time.tv_sec, timekeeping_suspend_time.tv_nsec,
+		ts.tv_sec, ts.tv_nsec);
  
  	if (timespec_compare(&ts, &timekeeping_suspend_time) > 0) {
  		ts = timespec_sub(ts, timekeeping_suspend_time);
-		__timekeeping_inject_sleeptime(tk, &ts);
+		printk("JDB: Trying to add: %ld:%ld\n", ts.tv_sec, ts.tv_nsec);
+		//__timekeeping_inject_sleeptime(tk, &ts);
  	}
  	/* re-base the last cycle value */
  	tk->clock->cycle_last = tk->clock->read(tk->clock);


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 4/8] time: Condense timekeeper.xtime into xtime_sec
  2012-08-20 18:58     ` John Stultz
@ 2012-08-20 19:45       ` Andreas Schwab
  2012-08-20 19:57         ` John Stultz
  0 siblings, 1 reply; 34+ messages in thread
From: Andreas Schwab @ 2012-08-20 19:45 UTC (permalink / raw)
  To: John Stultz
  Cc: Linux Kernel, Ingo Molnar, Peter Zijlstra, Richard Cochran,
	Prarit Bhargava, Thomas Gleixner, linuxppc-dev

John Stultz <john.stultz@linaro.org> writes:

> I'm not very familiar w/ the iBook hardware, but does it use a 
> clocksource, or does it use arch_gettimeoffset()?

clocksource: timebase mult[3640e38e] shift[24] registered

> I suspect that the casting has avoided clipping some strange values from 
> the persistent clock.

That's my guess as well.

> Could you try with the following patch against Linus' HEAD? I suspect it 
> will let the box resume (although it will seem as though no time was 
> spent in resume) and then let me know what the JDB lines print out?

JDB: suspend_time: 1345491706:0  resume_time: 1345491737:0
JDB: Trying to add: 31:0

(Looks reasonable.)

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 4/8] time: Condense timekeeper.xtime into xtime_sec
  2012-08-20 19:45       ` Andreas Schwab
@ 2012-08-20 19:57         ` John Stultz
  2012-08-20 20:04           ` Andreas Schwab
  0 siblings, 1 reply; 34+ messages in thread
From: John Stultz @ 2012-08-20 19:57 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: Linux Kernel, Ingo Molnar, Peter Zijlstra, Richard Cochran,
	Prarit Bhargava, Thomas Gleixner, linuxppc-dev

On 08/20/2012 12:45 PM, Andreas Schwab wrote:
> John Stultz <john.stultz@linaro.org> writes:
>
>> I'm not very familiar w/ the iBook hardware, but does it use a
>> clocksource, or does it use arch_gettimeoffset()?
> clocksource: timebase mult[3640e38e] shift[24] registered
>
>> I suspect that the casting has avoided clipping some strange values from
>> the persistent clock.
> That's my guess as well.
>
>> Could you try with the following patch against Linus' HEAD? I suspect it
>> will let the box resume (although it will seem as though no time was
>> spent in resume) and then let me know what the JDB lines print out?
> JDB: suspend_time: 1345491706:0  resume_time: 1345491737:0
> JDB: Trying to add: 31:0
>
> (Looks reasonable.)
Huh.  Yea, that looks fine.  And without the 
__timekeeping_inject_sleeptime() call, the system resumed ok?

Thanks for the testing!  I'll keep looking here.
-john






^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 4/8] time: Condense timekeeper.xtime into xtime_sec
  2012-08-20 19:57         ` John Stultz
@ 2012-08-20 20:04           ` Andreas Schwab
  2012-08-21  3:38             ` John Stultz
  0 siblings, 1 reply; 34+ messages in thread
From: Andreas Schwab @ 2012-08-20 20:04 UTC (permalink / raw)
  To: John Stultz
  Cc: Linux Kernel, Ingo Molnar, Peter Zijlstra, Richard Cochran,
	Prarit Bhargava, Thomas Gleixner, linuxppc-dev

John Stultz <john.stultz@linaro.org> writes:

> Huh.  Yea, that looks fine.  And without the
> __timekeeping_inject_sleeptime() call, the system resumed ok?

Yes, it does.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 4/8] time: Condense timekeeper.xtime into xtime_sec
  2012-08-20 20:04           ` Andreas Schwab
@ 2012-08-21  3:38             ` John Stultz
  2012-08-21  7:14               ` Andreas Schwab
  0 siblings, 1 reply; 34+ messages in thread
From: John Stultz @ 2012-08-21  3:38 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: Linux Kernel, Ingo Molnar, Peter Zijlstra, Richard Cochran,
	Prarit Bhargava, Thomas Gleixner, linuxppc-dev

On 08/20/2012 01:04 PM, Andreas Schwab wrote:
> John Stultz <john.stultz@linaro.org> writes:
>
>> Huh.  Yea, that looks fine.  And without the
>> __timekeeping_inject_sleeptime() call, the system resumed ok?
> Yes, it does.

So I'm mostly still stumped on this. But I did find one possible related 
bugfix that maybe you can try?

Let me know if the patch below dodges the problem, and if not, please 
send me the JDB printk output.
(if the resume hangs without any output, add a "return;" before the 
tk_xtime_add() call between the JDB printks).

thanks
-john



diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index e16af19..1a9b9c5 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -115,6 +115,7 @@ static void tk_xtime_add(struct timekeeper *tk, const struct timespec *ts)
  {
  	tk->xtime_sec += ts->tv_sec;
  	tk->xtime_nsec += (u64)ts->tv_nsec << tk->shift;
+	tk_normalize_xtime(tk);
  }
  
  static void tk_set_wall_to_mono(struct timekeeper *tk, struct timespec wtm)
@@ -695,9 +696,22 @@ static void __timekeeping_inject_sleeptime(struct timekeeper *tk,
  					"sleep delta value!\n");
  		return;
  	}
+
+	printk("JDB: pre xt %ld:%ld  wtm: %ld:%ld st: %ld:%ld\n",
+		tk_xtime(tk).tv_sec, tk_xtime(tk).tv_nsec,
+		tk->wall_to_monotonic.tv_sec,tk->wall_to_monotonic.tv_nsec,
+		tk->total_sleep_time.tv_sec, tk->total_sleep_time.tv_nsec);
+	printk("JDB: Adding %ld:%ld\n", delta->tv_sec, delta->tv_nsec);
+
  	tk_xtime_add(tk, delta);
  	tk_set_wall_to_mono(tk, timespec_sub(tk->wall_to_monotonic, *delta));
  	tk_set_sleep_time(tk, timespec_add(tk->total_sleep_time, *delta));
+
+	printk("JDB: post xt %ld:%ld  wtm: %ld:%ld st: %ld:%ld\n",
+		tk_xtime(tk).tv_sec, tk_xtime(tk).tv_nsec,
+		tk->wall_to_monotonic.tv_sec,tk->wall_to_monotonic.tv_nsec,
+		tk->total_sleep_time.tv_sec, tk->total_sleep_time.tv_nsec);
+
  }
  
  /**


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 4/8] time: Condense timekeeper.xtime into xtime_sec
  2012-08-21  3:38             ` John Stultz
@ 2012-08-21  7:14               ` Andreas Schwab
  2012-08-21 18:14                 ` John Stultz
  0 siblings, 1 reply; 34+ messages in thread
From: Andreas Schwab @ 2012-08-21  7:14 UTC (permalink / raw)
  To: John Stultz
  Cc: Linux Kernel, Ingo Molnar, Peter Zijlstra, Richard Cochran,
	Prarit Bhargava, Thomas Gleixner, linuxppc-dev

John Stultz <john.stultz@linaro.org> writes:

> @@ -115,6 +115,7 @@ static void tk_xtime_add(struct timekeeper *tk, const struct timespec *ts)
>  {
>  	tk->xtime_sec += ts->tv_sec;
>  	tk->xtime_nsec += (u64)ts->tv_nsec << tk->shift;
> +	tk_normalize_xtime(tk);
>  }

Yes, that does it.  Failure to normalize is always bad.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 4/8] time: Condense timekeeper.xtime into xtime_sec
  2012-08-21  7:14               ` Andreas Schwab
@ 2012-08-21 18:14                 ` John Stultz
  0 siblings, 0 replies; 34+ messages in thread
From: John Stultz @ 2012-08-21 18:14 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: Linux Kernel, Ingo Molnar, Peter Zijlstra, Richard Cochran,
	Prarit Bhargava, Thomas Gleixner, linuxppc-dev

On 08/21/2012 12:14 AM, Andreas Schwab wrote:
> John Stultz <john.stultz@linaro.org> writes:
>
>> @@ -115,6 +115,7 @@ static void tk_xtime_add(struct timekeeper *tk, const struct timespec *ts)
>>   {
>>   	tk->xtime_sec += ts->tv_sec;
>>   	tk->xtime_nsec += (u64)ts->tv_nsec << tk->shift;
>> +	tk_normalize_xtime(tk);
>>   }
> Yes, that does it.  Failure to normalize is always bad.

Great. Thanks for the testing and the bug report! I'll be sending out 
the patch to -tip later today.

thanks
-john


^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2012-08-21 18:15 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-13  5:21 [PATCH 0/8] Time fixes and cleanups for 3.6 John Stultz
2012-07-13  5:21 ` [PATCH 1/8] ntp: Fix STA_INS/DEL clearing bug John Stultz
2012-07-13  5:58   ` Ingo Molnar
2012-07-13 18:36     ` John Stultz
2012-07-15  7:57   ` [tip:timers/urgent] " tip-bot for John Stultz
2012-07-13  5:21 ` [PATCH 2/8] time: Whitespace cleanups per Ingo's requests John Stultz
2012-07-15  8:56   ` [tip:timers/core] time: Whitespace cleanups per Ingo%27s requests tip-bot for John Stultz
2012-07-13  5:21 ` [PATCH 3/8] time: Explicitly use u32 instead of int for shift values John Stultz
2012-07-15  8:57   ` [tip:timers/core] " tip-bot for John Stultz
2012-07-13  5:21 ` [PATCH 4/8] time: Condense timekeeper.xtime into xtime_sec John Stultz
2012-07-13  6:00   ` Ingo Molnar
2012-07-15  8:57   ` [tip:timers/core] " tip-bot for John Stultz
2012-08-19 21:02   ` [PATCH 4/8] " Andreas Schwab
2012-08-20 18:58     ` John Stultz
2012-08-20 19:45       ` Andreas Schwab
2012-08-20 19:57         ` John Stultz
2012-08-20 20:04           ` Andreas Schwab
2012-08-21  3:38             ` John Stultz
2012-08-21  7:14               ` Andreas Schwab
2012-08-21 18:14                 ` John Stultz
2012-07-13  5:21 ` [PATCH 5/8] time: Refactor accumulation of nsecs to secs John Stultz
2012-07-13  5:56   ` Ingo Molnar
2012-07-13  6:01   ` Ingo Molnar
2012-07-15  8:58   ` [tip:timers/core] " tip-bot for John Stultz
2012-07-13  5:21 ` [PATCH 6/8] time: Move arch_gettimeoffset() usage into timekeeping_get_ns() John Stultz
2012-07-13  6:03   ` Ingo Molnar
2012-07-15  8:59   ` [tip:timers/core] " tip-bot for John Stultz
2012-07-13  5:21 ` [PATCH 7/8] time: Move xtime_nsec adjustment underflow handling timekeeping_adjust John Stultz
2012-07-13  6:04   ` Ingo Molnar
2012-07-15  9:00   ` [tip:timers/core] " tip-bot for John Stultz
2012-07-13  5:21 ` [PATCH 8/8] time: Rework timekeeping functions to take timekeeper ptr as argument John Stultz
2012-07-15  9:01   ` [tip:timers/core] " tip-bot for John Stultz
2012-07-13  6:05 ` [PATCH 0/8] Time fixes and cleanups for 3.6 Ingo Molnar
2012-07-13 20:49   ` John Stultz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).