diff mbox

[21/21] time: Rework debugging variables so they aren't global

Message ID 1427945681-29972-22-git-send-email-john.stultz@linaro.org
State New
Headers show

Commit Message

John Stultz April 2, 2015, 3:34 a.m. UTC 
Ingo suggested that the timekeeping debugging variables
recently added should not be global, and should be tied
to the timekeeper's read_base.

Thus this patch implements that suggestion.

I'm a little hesitant here, since the tkr structure
has been carefully designed to fit in a cacheline.
However, these additions are all at the end of the
structure and are conditionally compiled.

Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 include/linux/timekeeper_internal.h | 18 +++++++++++++++++-
 kernel/time/timekeeping.c           | 33 ++++++++++-----------------------
 2 files changed, 27 insertions(+), 24 deletions(-)

Comments

John Stultz April 2, 2015, 5:32 p.m. UTC | #1 
On Thu, Apr 2, 2015 at 12:51 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Peter Zijlstra <peterz@infradead.org> wrote:
>
>> On Wed, Apr 01, 2015 at 08:34:41PM -0700, John Stultz wrote:
>> > Ingo suggested that the timekeeping debugging variables
>> > recently added should not be global, and should be tied
>> > to the timekeeper's read_base.
>>
>> But why? its the same hardware clock for both tkr's. Surely if one goes
>> funny the other will too.
>>
>> It doesn't make sense to duplicate this.
>
> Well, could it be moved to the timekeeper data structure? What I was
> opposed to was making it super-global, after all the (nice) effort we
> made to tidy up global data structures in this area.

Ok. I'll rework this then and resend.  Though if there's no objections
to Xunlei's portion of the series, would you mind merging those into
-tip? I'd like to make sure they get some time in -next before the
merge window opens.

thanks
-john
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
diff mbox

Patch

diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index fb86963..9b33027 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -20,9 +20,13 @@ 
  * @shift:	Shift value for scaled math conversion
  * @xtime_nsec: Shifted (fractional) nano seconds offset for readout
  * @base:	ktime_t (nanoseconds) base time for readout
+ * @last_warning:   Warning ratelimiter (DEBUG_TIMEKEEPING)
+ * @underflow_seen: Underflow warning flag (DEBUG_TIMEKEEPING)
+ * @overflow_seen:  Overflow warning flag (DEBUG_TIMEKEEPING)
  *
  * This struct has size 56 byte on 64 bit. Together with a seqcount it
- * occupies a single 64byte cache line.
+ * occupies a single 64byte cache line (when DEBUG_TIMEKEEPING is not
+ * enabled).
  *
  * The struct is separate from struct timekeeper as it is also used
  * for a fast NMI safe accessors.
@@ -36,6 +40,18 @@  struct tk_read_base {
 	u32			shift;
 	u64			xtime_nsec;
 	ktime_t			base;
+#ifdef CONFIG_DEBUG_TIMEKEEPING
+	long			last_warning;
+	/*
+	 * These simple flag variables are managed
+	 * without locks, which is racy, but ok since
+	 * we don't really care about being super
+	 * precise about how many events were seen,
+	 * just that a problem was observed.
+	 */
+	int			underflow_seen;
+	int			overflow_seen;
+#endif
 };
 
 /**
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 676896e..ad7a80f 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -118,19 +118,6 @@  static inline void tk_update_sleep_time(struct timekeeper *tk, ktime_t delta)
 
 #ifdef CONFIG_DEBUG_TIMEKEEPING
 #define WARNING_FREQ (HZ*300) /* 5 minute rate-limiting */
-/*
- * These simple flag variables are managed
- * without locks, which is racy, but ok since
- * we don't really care about being super
- * precise about how many events were seen,
- * just that a problem was observed.
- */
-static int timekeeping_underflow_seen;
-static int timekeeping_overflow_seen;
-
-/* last_warning is only modified under the timekeeping lock */
-static long timekeeping_last_warning;
-
 static void timekeeping_check_update(struct tk_read_base *tkr, cycle_t offset)
 {
 
@@ -149,24 +136,24 @@  static void timekeeping_check_update(struct tk_read_base *tkr, cycle_t offset)
 		}
 	}
 
-	if (timekeeping_underflow_seen) {
-		if (jiffies - timekeeping_last_warning > WARNING_FREQ) {
+	if (tkr->underflow_seen) {
+		if (jiffies - tkr->last_warning > WARNING_FREQ) {
 			printk_deferred("WARNING: Underflow in clocksource '%s' observed, time update ignored.\n", name);
 			printk_deferred("         Please report this, consider using a different clocksource, if possible.\n");
 			printk_deferred("         Your kernel is probably still fine.\n");
-			timekeeping_last_warning = jiffies;
+			tkr->last_warning = jiffies;
 		}
-		timekeeping_underflow_seen = 0;
+		tkr->underflow_seen = 0;
 	}
 
-	if (timekeeping_overflow_seen) {
-		if (jiffies - timekeeping_last_warning > WARNING_FREQ) {
+	if (tkr->overflow_seen) {
+		if (jiffies - tkr->last_warning > WARNING_FREQ) {
 			printk_deferred("WARNING: Overflow in clocksource '%s' observed, time update capped.\n", name);
 			printk_deferred("         Please report this, consider using a different clocksource, if possible.\n");
 			printk_deferred("         Your kernel is probably still fine.\n");
-			timekeeping_last_warning = jiffies;
+			tkr->last_warning = jiffies;
 		}
-		timekeeping_overflow_seen = 0;
+		tkr->overflow_seen = 0;
 	}
 }
 
@@ -197,13 +184,13 @@  static inline cycle_t timekeeping_get_delta(struct tk_read_base *tkr)
 	 * mask-relative negative values.
 	 */
 	if (unlikely((~delta & mask) < (mask >> 3))) {
-		timekeeping_underflow_seen = 1;
+		tkr->underflow_seen = 1;
 		delta = 0;
 	}
 
 	/* Cap delta value to the max_cycles values to avoid mult overflows */
 	if (unlikely(delta > max)) {
-		timekeeping_overflow_seen = 1;
+		tkr->overflow_seen = 1;
 		delta = tkr->clock->max_cycles;
 	}