Groups > linux.kernel > #1208866 > unrolled thread

[PATCH 0/9] Time items for 4.3

Started by	John Stultz <john.stultz@linaro.org>
First post	2015-08-17 22:50 +0200
Last post	2015-08-18 22:20 +0200
Articles	20 on this page of 22 — 4 participants

Back to article view | Back to linux.kernel

  [PATCH 0/9] Time items for 4.3 John Stultz <john.stultz@linaro.org> - 2015-08-17 22:50 +0200
    [PATCH 2/9] time: Fix nanosecond file time rounding in timespec_trunc() John Stultz <john.stultz@linaro.org> - 2015-08-17 22:50 +0200
      Re: [PATCH 2/9] time: Fix nanosecond file time rounding in  timespec_trunc() Thomas Gleixner <tglx@linutronix.de> - 2015-08-18 00:20 +0200
    [PATCH 6/9] time: Introduce current_kernel_time64() John Stultz <john.stultz@linaro.org> - 2015-08-17 22:50 +0200
    [PATCH 1/9] timer_list: Add the base offset so remaining nsecs are accurate for non monotonic timers John Stultz <john.stultz@linaro.org> - 2015-08-17 22:50 +0200
      Re: [PATCH 1/9] timer_list: Add the base offset so remaining nsecs  are accurate for non monotonic timers Shuah Khan <shuahkh@osg.samsung.com> - 2015-08-17 23:10 +0200
        Re: [PATCH 1/9] timer_list: Add the base offset so remaining nsecs  are accurate for non monotonic timers John Stultz <john.stultz@linaro.org> - 2015-08-17 23:10 +0200
        Re: [PATCH 1/9] timer_list: Add the base offset so remaining nsecs  are accurate for non monotonic timers Shuah Khan <shuahkh@osg.samsung.com> - 2015-08-17 23:10 +0200
    [PATCH 3/9] time: Always make sure wall_to_monotonic isn't positive John Stultz <john.stultz@linaro.org> - 2015-08-17 22:50 +0200
    [PATCH 7/9] time: Introduce timespec64_to_jiffies()/jiffies_to_timespec64() John Stultz <john.stultz@linaro.org> - 2015-08-17 22:50 +0200
    [PATCH 4/9] time: Add the common weak version of update_persistent_clock() John Stultz <john.stultz@linaro.org> - 2015-08-17 22:50 +0200
    [PATCH 8/9] clocksource: Improve unstable clocksource detection John Stultz <john.stultz@linaro.org> - 2015-08-17 22:50 +0200
      Re: [PATCH 8/9] clocksource: Improve unstable clocksource  detection Thomas Gleixner <tglx@linutronix.de> - 2015-08-18 00:10 +0200
        Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection John Stultz <john.stultz@linaro.org> - 2015-08-18 00:20 +0200
          Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection Shaohua Li <shli@fb.com> - 2015-08-18 05:00 +0200
            Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection John Stultz <john.stultz@linaro.org> - 2015-08-18 05:40 +0200
            Re: [PATCH 8/9] clocksource: Improve unstable clocksource  detection Thomas Gleixner <tglx@linutronix.de> - 2015-08-18 11:00 +0200
          Re: [PATCH 8/9] clocksource: Improve unstable clocksource  detection Thomas Gleixner <tglx@linutronix.de> - 2015-08-18 10:40 +0200
            Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection John Stultz <john.stultz@linaro.org> - 2015-08-18 19:50 +0200
              Re: [PATCH 8/9] clocksource: Improve unstable clocksource  detection Thomas Gleixner <tglx@linutronix.de> - 2015-08-18 21:30 +0200
                Re: [PATCH 8/9] clocksource: Improve unstable clocksource  detection Thomas Gleixner <tglx@linutronix.de> - 2015-08-18 22:20 +0200
                Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection John Stultz <john.stultz@linaro.org> - 2015-08-18 22:20 +0200

Page 1 of 2 [1] 2 Next page →

#1208866 — [PATCH 0/9] Time items for 4.3

From	John Stultz <john.stultz@linaro.org>
Date	2015-08-17 22:50 +0200
Subject	[PATCH 0/9] Time items for 4.3
Message-ID	<pYzvX-1vo-3@gated-at.bofh.it>

Hey Ingo,
    I got back from vacation last week, so this is a little later
then I wanted it to be. But here is my queue of items I've collected
for 4.3

There's a handful of y2038-prep related items. Then some small fixes
for timespec_trunc(), timer_list output, and clocksource watchdog false
positives caused by processing stalls or delays.

Let me know if there's anything that you object to.

thanks
-john

Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Karsten Blees <blees@dcon.de>
Cc: Wang YanQing <udknight@gmail.com>
Cc: Xunlei Pang <pang.xunlei@linaro.org>
Cc: Baolin Wang <baolin.wang@linaro.org>
Cc: Shaohua Li <shli@fb.com>

If you'd like to pull this set:
The following changes since commit d770e558e21961ad6cfdf0ff7df0eb5d7d4f0754:

  Linux 4.2-rc1 (2015-07-05 11:01:52 -0700)

are available in the git repository at:

  https://git.linaro.org/people/john.stultz/linux.git fortglx/4.3/time

for you to fetch changes up to bad86629c40a9fd8887c742ffd958e867592959a:

  clocksource: Sanity check watchdog clocksource (2015-08-17 11:28:30 -0700)

Baolin Wang (3):
  time: Introduce struct itimerspec64
  time: Introduce current_kernel_time64()
  time: Introduce timespec64_to_jiffies()/jiffies_to_timespec64()

John Stultz (1):
  timer_list: Add the base offset so remaining nsecs are accurate for
    non monotonic timers

Karsten Blees (1):
  time: Fix nanosecond file time rounding in timespec_trunc()

Shaohua Li (2):
  clocksource: Improve unstable clocksource detection
  clocksource: Sanity check watchdog clocksource

Wang YanQing (1):
  time: Always make sure wall_to_monotonic isn't positive

Xunlei Pang (1):
  time: Add the common weak version of update_persistent_clock()

 include/linux/jiffies.h     | 22 +++++++++++++++++++---
 include/linux/time64.h      | 35 +++++++++++++++++++++++++++++++++++
 include/linux/timekeeping.h |  9 ++++++++-
 kernel/time/clocksource.c   | 10 +++++++---
 kernel/time/ntp.c           |  5 +++++
 kernel/time/time.c          | 43 +++++++++++++++++++++----------------------
 kernel/time/timekeeping.c   | 19 +++++++++++++------
 kernel/time/timer_list.c    |  2 +-
 8 files changed, 109 insertions(+), 36 deletions(-)

-- 
1.9.1

--
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/

[toc] | [next] | [standalone]

#1208867 — [PATCH 2/9] time: Fix nanosecond file time rounding in timespec_trunc()

From	John Stultz <john.stultz@linaro.org>
Date	2015-08-17 22:50 +0200
Subject	[PATCH 2/9] time: Fix nanosecond file time rounding in timespec_trunc()
Message-ID	<pYzvY-1vo-7@gated-at.bofh.it>
In reply to	#1208866

From: Karsten Blees <karsten.blees@gmail.com>

timespec_trunc() avoids rounding if granularity <= nanoseconds-per-jiffie
(or TICK_NSEC). This optimization assumes that:

 1. current_kernel_time().tv_nsec is already rounded to TICK_NSEC (i.e.
    with HZ=1000 you'd get 1000000, 2000000, 3000000... but never 1000001).
    This is no longer true (probably since hrtimers introduced in 2.6.16).

 2. TICK_NSEC is evenly divisible by all possible granularities. This may
    be true for HZ=100, 250, 1000, but obviously not for HZ=300 /
    TICK_NSEC=3333333 (introduced in 2.6.20).

Thus, sub-second portions of in-core file times are not rounded to on-disk
granularity. I.e. file times may change when the inode is re-read from disk
or when the file system is remounted.

This affects all file systems with file time granularities > 1 ns and < 1s,
e.g. CEPH (1000 ns), UDF (1000 ns), CIFS (100 ns), NTFS (100 ns) and FUSE
(configurable from user mode via struct fuse_init_out.time_gran).

Steps to reproduce with e.g. UDF:

  $ dd if=/dev/zero of=udfdisk count=10000 && mkudffs udfdisk
  $ mkdir udf && mount udfdisk udf
  $ touch udf/test && stat -c %y udf/test
  2015-06-09 10:22:56.130006767 +0200
  $ umount udf && mount udfdisk udf
  $ stat -c %y udf/test
  2015-06-09 10:22:56.130006000 +0200

Remounting truncates the mtime to 1 µs.

Fix the rounding in timespec_trunc() and update the documentation.

timespec_trunc() is exclusively used to calculate inode's [acm]time (mostly
via current_fs_time()), and always with super_block.s_time_gran as second
argument. So this can safely be changed without side effects.

Note: This does _not_ fix the issue for FAT's 2 second mtime resolution,
as super_block.s_time_gran isn't prepared to handle different ctime /
mtime / atime resolutions nor resolutions > 1 second.

Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/time.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/kernel/time/time.c b/kernel/time/time.c
index 85d5bb1..34dbd42 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -287,26 +287,20 @@ EXPORT_SYMBOL(jiffies_to_usecs);
  * @t: Timespec
  * @gran: Granularity in ns.
  *
- * Truncate a timespec to a granularity. gran must be smaller than a second.
- * Always rounds down.
- *
- * This function should be only used for timestamps returned by
- * current_kernel_time() or CURRENT_TIME, not with do_gettimeofday() because
- * it doesn't handle the better resolution of the latter.
+ * Truncate a timespec to a granularity. Always rounds down. gran must
+ * not be 0 nor greater than a second (NSEC_PER_SEC, or 10^9 ns).
  */
 struct timespec timespec_trunc(struct timespec t, unsigned gran)
 {
-	/*
-	 * Division is pretty slow so avoid it for common cases.
-	 * Currently current_kernel_time() never returns better than
-	 * jiffies resolution. Exploit that.
-	 */
-	if (gran <= jiffies_to_usecs(1) * 1000) {
+	/* Avoid division in the common cases 1 ns and 1 s. */
+	if (gran == 1) {
 		/* nothing */
-	} else if (gran == 1000000000) {
+	} else if (gran == NSEC_PER_SEC) {
 		t.tv_nsec = 0;
-	} else {
+	} else if (gran > 1 && gran < NSEC_PER_SEC) {
 		t.tv_nsec -= t.tv_nsec % gran;
+	} else {
+		WARN(1, "illegal file time granularity: %u", gran);
 	}
 	return t;
 }
-- 
1.9.1

--
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/

[toc] | [prev] | [next] | [standalone]

#1208919 — Re: [PATCH 2/9] time: Fix nanosecond file time rounding in timespec_trunc()

From	Thomas Gleixner <tglx@linutronix.de>
Date	2015-08-18 00:20 +0200
Subject	Re: [PATCH 2/9] time: Fix nanosecond file time rounding in timespec_trunc()
Message-ID	<pYAV4-3DY-13@gated-at.bofh.it>
In reply to	#1208867

On Mon, 17 Aug 2015, John Stultz wrote:
> +	} else {
> +		WARN(1, "illegal file time granularity: %u", gran);

s/illegal/Invalid/

Thanks,

	tglx
--
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/

[toc] | [prev] | [next] | [standalone]

#1208870 — [PATCH 6/9] time: Introduce current_kernel_time64()

From	John Stultz <john.stultz@linaro.org>
Date	2015-08-17 22:50 +0200
Subject	[PATCH 6/9] time: Introduce current_kernel_time64()
Message-ID	<pYzvY-1vo-15@gated-at.bofh.it>
In reply to	#1208866

From: Baolin Wang <baolin.wang@linaro.org>

The current_kernel_time() is not year 2038 safe on 32bit systems
since it returns a timespec value. Introduce current_kernel_time64()
which returns a timespec64 value.

Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 include/linux/timekeeping.h | 9 ++++++++-
 kernel/time/timekeeping.c   | 6 +++---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index 3aa72e6..657ea03 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -18,10 +18,17 @@ extern int do_sys_settimeofday(const struct timespec *tv,
  * Kernel time accessors
  */
 unsigned long get_seconds(void);
-struct timespec current_kernel_time(void);
+struct timespec64 current_kernel_time64(void);
 /* does not take xtime_lock */
 struct timespec __current_kernel_time(void);
 
+static inline struct timespec current_kernel_time(void)
+{
+	struct timespec64 now = current_kernel_time64();
+
+	return timespec64_to_timespec(now);
+}
+
 /*
  * timespec based interfaces
  */
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 4cdb771..f6ee2e6 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1881,7 +1881,7 @@ struct timespec __current_kernel_time(void)
 	return timespec64_to_timespec(tk_xtime(tk));
 }
 
-struct timespec current_kernel_time(void)
+struct timespec64 current_kernel_time64(void)
 {
 	struct timekeeper *tk = &tk_core.timekeeper;
 	struct timespec64 now;
@@ -1893,9 +1893,9 @@ struct timespec current_kernel_time(void)
 		now = tk_xtime(tk);
 	} while (read_seqcount_retry(&tk_core.seq, seq));
 
-	return timespec64_to_timespec(now);
+	return now;
 }
-EXPORT_SYMBOL(current_kernel_time);
+EXPORT_SYMBOL(current_kernel_time64);
 
 struct timespec64 get_monotonic_coarse64(void)
 {
-- 
1.9.1

--
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/

[toc] | [prev] | [next] | [standalone]

#1208871 — [PATCH 1/9] timer_list: Add the base offset so remaining nsecs are accurate for non monotonic timers

From	John Stultz <john.stultz@linaro.org>
Date	2015-08-17 22:50 +0200
Subject	[PATCH 1/9] timer_list: Add the base offset so remaining nsecs are accurate for non monotonic timers
Message-ID	<pYzvY-1vo-17@gated-at.bofh.it>
In reply to	#1208866

I noticed for non-monotonic timers in timer_list, some of the
output looked a little confusing.

For example:
 #1: <0000000000000000>, posix_timer_fn, S:01, hrtimer_start_range_ns, leap-a-day/2360
 # expires at 1434412800000000000-1434412800000000000 nsecs [in 1434410725062375469 to 1434410725062375469 nsecs]

You'll note the relative time till the expiration "[in xxx to
yyy nsecs]" is incorrect. This is because its printing the delta
between CLOCK_MONOTONIC time to the CLOCK_REALTIME expiration.

This patch fixes this issue by adding the clock offset to the
"now" time which we use to calculate the delta.

Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Jiri Bohac <jbohac@suse.cz>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/timer_list.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/timer_list.c b/kernel/time/timer_list.c
index a4536e1..129c960 100644
--- a/kernel/time/timer_list.c
+++ b/kernel/time/timer_list.c
@@ -137,7 +137,7 @@ print_base(struct seq_file *m, struct hrtimer_clock_base *base, u64 now)
 		   (unsigned long long) ktime_to_ns(base->offset));
 #endif
 	SEQ_printf(m,   "active timers:\n");
-	print_active_timers(m, base, now);
+	print_active_timers(m, base, now + ktime_to_ns(base->offset));
 }
 
 static void print_cpu(struct seq_file *m, int cpu, u64 now)
-- 
1.9.1

--
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/

[toc] | [prev] | [next] | [standalone]

#1208884 — Re: [PATCH 1/9] timer_list: Add the base offset so remaining nsecs are accurate for non monotonic timers

From	Shuah Khan <shuahkh@osg.samsung.com>
Date	2015-08-17 23:10 +0200
Subject	Re: [PATCH 1/9] timer_list: Add the base offset so remaining nsecs are accurate for non monotonic timers
Message-ID	<pYzPj-27r-13@gated-at.bofh.it>
In reply to	#1208871

On 08/17/2015 02:40 PM, John Stultz wrote:
> I noticed for non-monotonic timers in timer_list, some of the
> output looked a little confusing.
> 
> For example:
>  #1: <0000000000000000>, posix_timer_fn, S:01, hrtimer_start_range_ns, leap-a-day/2360
>  # expires at 1434412800000000000-1434412800000000000 nsecs [in 1434410725062375469 to 1434410725062375469 nsecs]
> 
> You'll note the relative time till the expiration "[in xxx to
> yyy nsecs]" is incorrect. This is because its printing the delta
> between CLOCK_MONOTONIC time to the CLOCK_REALTIME expiration.
> 
> This patch fixes this issue by adding the clock offset to the
> "now" time which we use to calculate the delta.
> 
> Cc: Prarit Bhargava <prarit@redhat.com>
> Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
> Cc: Richard Cochran <richardcochran@gmail.com>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Jiri Bohac <jbohac@suse.cz>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Shuah Khan <shuahkh@osg.samsung.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---

Hi John,

I see just this patch in the series and not the others. Could you
please make sure I am on the cc for all of them. I will review and
try to get these into 4.3

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978
--
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/

[toc] | [prev] | [next] | [standalone]

#1208885 — Re: [PATCH 1/9] timer_list: Add the base offset so remaining nsecs are accurate for non monotonic timers

From	John Stultz <john.stultz@linaro.org>
Date	2015-08-17 23:10 +0200
Subject	Re: [PATCH 1/9] timer_list: Add the base offset so remaining nsecs are accurate for non monotonic timers
Message-ID	<pYzPk-27r-21@gated-at.bofh.it>
In reply to	#1208884

On Mon, Aug 17, 2015 at 2:01 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> On 08/17/2015 02:40 PM, John Stultz wrote:
>> I noticed for non-monotonic timers in timer_list, some of the
>> output looked a little confusing.
>>
>> For example:
>>  #1: <0000000000000000>, posix_timer_fn, S:01, hrtimer_start_range_ns, leap-a-day/2360
>>  # expires at 1434412800000000000-1434412800000000000 nsecs [in 1434410725062375469 to 1434410725062375469 nsecs]
>>
>> You'll note the relative time till the expiration "[in xxx to
>> yyy nsecs]" is incorrect. This is because its printing the delta
>> between CLOCK_MONOTONIC time to the CLOCK_REALTIME expiration.
>>
>> This patch fixes this issue by adding the clock offset to the
>> "now" time which we use to calculate the delta.
>>
>> Cc: Prarit Bhargava <prarit@redhat.com>
>> Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
>> Cc: Richard Cochran <richardcochran@gmail.com>
>> Cc: Jan Kara <jack@suse.cz>
>> Cc: Jiri Bohac <jbohac@suse.cz>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Shuah Khan <shuahkh@osg.samsung.com>
>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>> ---
>
> Hi John,
>
> I see just this patch in the series and not the others. Could you
> please make sure I am on the cc for all of them. I will review and
> try to get these into 4.3

So sorry here. I actually had you on CC for this from a previous patch
series this patch was included in, and didn't remove you before
sending. I'm hoping Ingo queues these for 4.3, so you shouldn't have
to do anything (unless you want to review it and provide any feedback
:).

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/

[toc] | [prev] | [next] | [standalone]

#1208887 — Re: [PATCH 1/9] timer_list: Add the base offset so remaining nsecs are accurate for non monotonic timers

From	Shuah Khan <shuahkh@osg.samsung.com>
Date	2015-08-17 23:10 +0200
Subject	Re: [PATCH 1/9] timer_list: Add the base offset so remaining nsecs are accurate for non monotonic timers
Message-ID	<pYzPk-27r-27@gated-at.bofh.it>
In reply to	#1208884

On 08/17/2015 03:01 PM, Shuah Khan wrote:
> On 08/17/2015 02:40 PM, John Stultz wrote:
>> I noticed for non-monotonic timers in timer_list, some of the
>> output looked a little confusing.
>>
>> For example:
>>  #1: <0000000000000000>, posix_timer_fn, S:01, hrtimer_start_range_ns, leap-a-day/2360
>>  # expires at 1434412800000000000-1434412800000000000 nsecs [in 1434410725062375469 to 1434410725062375469 nsecs]
>>
>> You'll note the relative time till the expiration "[in xxx to
>> yyy nsecs]" is incorrect. This is because its printing the delta
>> between CLOCK_MONOTONIC time to the CLOCK_REALTIME expiration.
>>
>> This patch fixes this issue by adding the clock offset to the
>> "now" time which we use to calculate the delta.
>>
>> Cc: Prarit Bhargava <prarit@redhat.com>
>> Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
>> Cc: Richard Cochran <richardcochran@gmail.com>
>> Cc: Jan Kara <jack@suse.cz>
>> Cc: Jiri Bohac <jbohac@suse.cz>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Shuah Khan <shuahkh@osg.samsung.com>
>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>> ---
> 
> Hi John,
> 
> I see just this patch in the series and not the others. Could you
> please make sure I am on the cc for all of them. I will review and
> try to get these into 4.3
> 

Never mind. Looks like this one at least is better suited to go through
timer git.

-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978
--
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/

[toc] | [prev] | [next] | [standalone]

#1208872 — [PATCH 3/9] time: Always make sure wall_to_monotonic isn't positive

From	John Stultz <john.stultz@linaro.org>
Date	2015-08-17 22:50 +0200
Subject	[PATCH 3/9] time: Always make sure wall_to_monotonic isn't positive
Message-ID	<pYzvY-1vo-19@gated-at.bofh.it>
In reply to	#1208866

From: Wang YanQing <udknight@gmail.com>

Two issues were found on an IMX6 development board without an
enabled RTC device(resulting in the boot time and monotonic
time being initialized to 0).

Issue 1:exportfs -a generate:
       "exportfs: /opt/nfs/arm does not support NFS export"
Issue 2:cat /proc/stat:
       "btime 4294967236"

The same issues can be reproduced on x86 after running the
following code:
	int main(void)
	{
	    struct timeval val;
	    int ret;

	    val.tv_sec = 0;
	    val.tv_usec = 0;
	    ret = settimeofday(&val, NULL);
	    return 0;
	}

Two issues are different symptoms of same problem:
The reason is a positive wall_to_monotonic pushes boot time back
to the time before Epoch, and getboottime will return negative
value.

In symptom 1:
          negative boot time cause get_expiry() to overflow time_t
          when input expire time is 2147483647, then cache_flush()
          always clears entries just added in ip_map_parse.
In symptom 2:
          show_stat() uses "unsigned long" to print negative btime
          value returned by getboottime.

This patch fix the problem by prohibiting time from being set to a value which
would cause a negative boot time. As a result one can't set the CLOCK_REALTIME
time prior to (1970 + system uptime).

Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Wang YanQing <udknight@gmail.com>
[jstultz: reworded commit message]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/timekeeping.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index bca3667..4cdb771 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -911,6 +911,7 @@ int do_settimeofday64(const struct timespec64 *ts)
 	struct timekeeper *tk = &tk_core.timekeeper;
 	struct timespec64 ts_delta, xt;
 	unsigned long flags;
+	int ret = 0;
 
 	if (!timespec64_valid_strict(ts))
 		return -EINVAL;
@@ -924,10 +925,15 @@ int do_settimeofday64(const struct timespec64 *ts)
 	ts_delta.tv_sec = ts->tv_sec - xt.tv_sec;
 	ts_delta.tv_nsec = ts->tv_nsec - xt.tv_nsec;
 
+	if (timespec64_compare(&tk->wall_to_monotonic, &ts_delta) > 0) {
+		ret = -EINVAL;
+		goto out;
+	}
+
 	tk_set_wall_to_mono(tk, timespec64_sub(tk->wall_to_monotonic, ts_delta));
 
 	tk_set_xtime(tk, ts);
-
+out:
 	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
 
 	write_seqcount_end(&tk_core.seq);
@@ -936,7 +942,7 @@ int do_settimeofday64(const struct timespec64 *ts)
 	/* signal hrtimers about time change */
 	clock_was_set();
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL(do_settimeofday64);
 
@@ -965,7 +971,8 @@ int timekeeping_inject_offset(struct timespec *ts)
 
 	/* Make sure the proposed value is valid */
 	tmp = timespec64_add(tk_xtime(tk),  ts64);
-	if (!timespec64_valid_strict(&tmp)) {
+	if (timespec64_compare(&tk->wall_to_monotonic, &ts64) > 0 ||
+	    !timespec64_valid_strict(&tmp)) {
 		ret = -EINVAL;
 		goto error;
 	}
-- 
1.9.1

--
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/

[toc] | [prev] | [next] | [standalone]

#1208874 — [PATCH 7/9] time: Introduce timespec64_to_jiffies()/jiffies_to_timespec64()

From	John Stultz <john.stultz@linaro.org>
Date	2015-08-17 22:50 +0200
Subject	[PATCH 7/9] time: Introduce timespec64_to_jiffies()/jiffies_to_timespec64()
Message-ID	<pYzvY-1vo-23@gated-at.bofh.it>
In reply to	#1208866

From: Baolin Wang <baolin.wang@linaro.org>

The conversion between struct timespec and jiffies is not year 2038
safe on 32bit systems. Introduce timespec64_to_jiffies() and
jiffies_to_timespec64() functions which use struct timespec64 to
make it ready for 2038 issue.

Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 include/linux/jiffies.h | 22 +++++++++++++++++++---
 kernel/time/time.c      | 21 +++++++++++++--------
 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/include/linux/jiffies.h b/include/linux/jiffies.h
index 535fd3b..bf96d9f 100644
--- a/include/linux/jiffies.h
+++ b/include/linux/jiffies.h
@@ -416,9 +416,25 @@ static inline unsigned long usecs_to_jiffies(const unsigned int u)
 	}
 }
 
-extern unsigned long timespec_to_jiffies(const struct timespec *value);
-extern void jiffies_to_timespec(const unsigned long jiffies,
-				struct timespec *value);
+extern unsigned long timespec64_to_jiffies(const struct timespec64 *value);
+extern void jiffies_to_timespec64(const unsigned long jiffies,
+				  struct timespec64 *value);
+static inline unsigned long timespec_to_jiffies(const struct timespec *value)
+{
+	struct timespec64 ts = timespec_to_timespec64(*value);
+
+	return timespec64_to_jiffies(&ts);
+}
+
+static inline void jiffies_to_timespec(const unsigned long jiffies,
+				       struct timespec *value)
+{
+	struct timespec64 ts;
+
+	jiffies_to_timespec64(jiffies, &ts);
+	*value = timespec64_to_timespec(ts);
+}
+
 extern unsigned long timeval_to_jiffies(const struct timeval *value);
 extern void jiffies_to_timeval(const unsigned long jiffies,
 			       struct timeval *value);
diff --git a/kernel/time/time.c b/kernel/time/time.c
index 34dbd42..f18ab10 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -540,7 +540,7 @@ EXPORT_SYMBOL(__usecs_to_jiffies);
  * value to a scaled second value.
  */
 static unsigned long
-__timespec_to_jiffies(unsigned long sec, long nsec)
+__timespec64_to_jiffies(u64 sec, long nsec)
 {
 	nsec = nsec + TICK_NSEC - 1;
 
@@ -548,22 +548,27 @@ __timespec_to_jiffies(unsigned long sec, long nsec)
 		sec = MAX_SEC_IN_JIFFIES;
 		nsec = 0;
 	}
-	return (((u64)sec * SEC_CONVERSION) +
+	return ((sec * SEC_CONVERSION) +
 		(((u64)nsec * NSEC_CONVERSION) >>
 		 (NSEC_JIFFIE_SC - SEC_JIFFIE_SC))) >> SEC_JIFFIE_SC;
 
 }
 
-unsigned long
-timespec_to_jiffies(const struct timespec *value)
+static unsigned long
+__timespec_to_jiffies(unsigned long sec, long nsec)
 {
-	return __timespec_to_jiffies(value->tv_sec, value->tv_nsec);
+	return __timespec64_to_jiffies((u64)sec, nsec);
 }
 
-EXPORT_SYMBOL(timespec_to_jiffies);
+unsigned long
+timespec64_to_jiffies(const struct timespec64 *value)
+{
+	return __timespec64_to_jiffies(value->tv_sec, value->tv_nsec);
+}
+EXPORT_SYMBOL(timespec64_to_jiffies);
 
 void
-jiffies_to_timespec(const unsigned long jiffies, struct timespec *value)
+jiffies_to_timespec64(const unsigned long jiffies, struct timespec64 *value)
 {
 	/*
 	 * Convert jiffies to nanoseconds and separate with
@@ -574,7 +579,7 @@ jiffies_to_timespec(const unsigned long jiffies, struct timespec *value)
 				    NSEC_PER_SEC, &rem);
 	value->tv_nsec = rem;
 }
-EXPORT_SYMBOL(jiffies_to_timespec);
+EXPORT_SYMBOL(jiffies_to_timespec64);
 
 /*
  * We could use a similar algorithm to timespec_to_jiffies (with a
-- 
1.9.1

--
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/

[toc] | [prev] | [next] | [standalone]

#1208875 — [PATCH 4/9] time: Add the common weak version of update_persistent_clock()

From	John Stultz <john.stultz@linaro.org>
Date	2015-08-17 22:50 +0200
Subject	[PATCH 4/9] time: Add the common weak version of update_persistent_clock()
Message-ID	<pYzvY-1vo-25@gated-at.bofh.it>
In reply to	#1208866

From: Xunlei Pang <pang.xunlei@linaro.org>

The weak update_persistent_clock64() calls update_persistent_clock(),
if the architecture defines an update_persistent_clock64() to replace
and remove its update_persistent_clock() version, when building the
kernel the linker will throw an undefined symbol error, that is, any
arch that switches to update_persistent_clock64() will have this issue.

To solve the issue, we add the common weak update_persistent_clock().

Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Xunlei Pang <pang.xunlei@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/ntp.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index fb4d98c..df68cb8 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -487,6 +487,11 @@ out:
 }
 
 #ifdef CONFIG_GENERIC_CMOS_UPDATE
+int __weak update_persistent_clock(struct timespec now)
+{
+	return -ENODEV;
+}
+
 int __weak update_persistent_clock64(struct timespec64 now64)
 {
 	struct timespec now;
-- 
1.9.1

--
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/

[toc] | [prev] | [next] | [standalone]

#1208877 — [PATCH 8/9] clocksource: Improve unstable clocksource detection

From	John Stultz <john.stultz@linaro.org>
Date	2015-08-17 22:50 +0200
Subject	[PATCH 8/9] clocksource: Improve unstable clocksource detection
Message-ID	<pYzvZ-1vo-29@gated-at.bofh.it>
In reply to	#1208866

From: Shaohua Li <shli@fb.com>

From time to time we saw TSC is marked as unstable in our systems, while
the CPUs declare to have stable TSC. Looking at the clocksource unstable
detection, there are two problems:
- watchdog clock source wrap. HPET is the most common watchdog clock
  source. It's 32-bit and runs in 14.3Mhz. That means the hpet counter
  can wrap in about 5 minutes.
- threshold isn't scaled against interval. The threshold is 0.0625s in
  0.5s interval. What if the actual interval is bigger than 0.5s?

The watchdog runs in a timer bh, so hard/soft irq can defer its running.
Heavy network stack softirq can hog a cpu. IPMI driver can disable
interrupt for a very long time. The first problem is mostly we are
suffering I think.

Here is a simple patch to fix the issues. If the waterdog doesn't run
for a long time, we ignore the detection. This should work for the two
problems. For the second one, we probably doen't need to scale if the
interval isn't very long.

Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Shaohua Li <shli@fb.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/clocksource.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index 841b72f..8417c83 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -122,9 +122,10 @@ static int clocksource_watchdog_kthread(void *data);
 static void __clocksource_change_rating(struct clocksource *cs, int rating);
 
 /*
- * Interval: 0.5sec Threshold: 0.0625s
+ * Interval: 0.5sec MaxInterval: 1s Threshold: 0.0625s
  */
 #define WATCHDOG_INTERVAL (HZ >> 1)
+#define WATCHDOG_MAX_INTERVAL_NS (NSEC_PER_SEC)
 #define WATCHDOG_THRESHOLD (NSEC_PER_SEC >> 4)
 
 static void clocksource_watchdog_work(struct work_struct *work)
@@ -217,7 +218,9 @@ static void clocksource_watchdog(unsigned long data)
 			continue;
 
 		/* Check the deviation from the watchdog clocksource. */
-		if ((abs(cs_nsec - wd_nsec) > WATCHDOG_THRESHOLD)) {
+		if ((abs(cs_nsec - wd_nsec) > WATCHDOG_THRESHOLD) &&
+		    cs_nsec < WATCHDOG_MAX_INTERVAL_NS &&
+		    wd_nsec < WATCHDOG_MAX_INTERVAL_NS) {
 			pr_warn("timekeeping watchdog: Marking clocksource '%s' as unstable because the skew is too large:\n",
 				cs->name);
 			pr_warn("                      '%s' wd_now: %llx wd_last: %llx mask: %llx\n",
-- 
1.9.1

--
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/

[toc] | [prev] | [next] | [standalone]

#1208915 — Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection

From	Thomas Gleixner <tglx@linutronix.de>
Date	2015-08-18 00:10 +0200
Subject	Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection
Message-ID	<pYALo-3sv-11@gated-at.bofh.it>
In reply to	#1208877

On Mon, 17 Aug 2015, John Stultz wrote:

> From: Shaohua Li <shli@fb.com>
> 
> >From time to time we saw TSC is marked as unstable in our systems, while

Stray '>'

> the CPUs declare to have stable TSC. Looking at the clocksource unstable
> detection, there are two problems:
> - watchdog clock source wrap. HPET is the most common watchdog clock
>   source. It's 32-bit and runs in 14.3Mhz. That means the hpet counter
>   can wrap in about 5 minutes.
> - threshold isn't scaled against interval. The threshold is 0.0625s in
>   0.5s interval. What if the actual interval is bigger than 0.5s?
> 
> The watchdog runs in a timer bh, so hard/soft irq can defer its running.
> Heavy network stack softirq can hog a cpu. IPMI driver can disable
> interrupt for a very long time.

And they hold off the timer softirq for more than a second? Don't you
think that's the problem which needs to be fixed?

> The first problem is mostly we are suffering I think.

So you think that's the root cause and because your patch makes it go
away it's not necessary to know for sure, right?

> Here is a simple patch to fix the issues. If the waterdog doesn't run

waterdog?

> for a long time, we ignore the detection. 

What's 'long time'? Please explain the numbers chosen.

> This should work for the two

Emphasis on 'should'? 

> problems. For the second one, we probably doen't need to scale if the
> interval isn't very long.

-ENOPARSE
 
> @@ -122,9 +122,10 @@ static int clocksource_watchdog_kthread(void *data);
>  static void __clocksource_change_rating(struct clocksource *cs, int rating);
>  
>  /*
> - * Interval: 0.5sec Threshold: 0.0625s
> + * Interval: 0.5sec MaxInterval: 1s Threshold: 0.0625s
>   */
>  #define WATCHDOG_INTERVAL (HZ >> 1)
> +#define WATCHDOG_MAX_INTERVAL_NS (NSEC_PER_SEC)
>  #define WATCHDOG_THRESHOLD (NSEC_PER_SEC >> 4)
>  
>  static void clocksource_watchdog_work(struct work_struct *work)
> @@ -217,7 +218,9 @@ static void clocksource_watchdog(unsigned long data)
>  			continue;
>  
>  		/* Check the deviation from the watchdog clocksource. */
> -		if ((abs(cs_nsec - wd_nsec) > WATCHDOG_THRESHOLD)) {
> +		if ((abs(cs_nsec - wd_nsec) > WATCHDOG_THRESHOLD) &&
> +		    cs_nsec < WATCHDOG_MAX_INTERVAL_NS &&
> +		    wd_nsec < WATCHDOG_MAX_INTERVAL_NS) {

So that adds a new opportunity for undiscovered wreckage:

   clocksource_watchdog();
   ....				<--- SMI skews TSC
   looong_irq_disabled_region();
   ....
   clocksource_watchdog();      <--- Does not detect skew

and it will not detect it later on if that SMI was a one time event.

So 'fixing' the watchdog is the wrong approach. Fixing the stuff which
prevents the watchdog to run is the proper thing to do.

Thanks,

	tglx
--
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/

[toc] | [prev] | [next] | [standalone]

#1208917 — Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection

From	John Stultz <john.stultz@linaro.org>
Date	2015-08-18 00:20 +0200
Subject	Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection
Message-ID	<pYAV3-3DY-5@gated-at.bofh.it>
In reply to	#1208915

On Mon, Aug 17, 2015 at 3:04 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Mon, 17 Aug 2015, John Stultz wrote:
>
>> From: Shaohua Li <shli@fb.com>
>>
>> >From time to time we saw TSC is marked as unstable in our systems, while
>
> Stray '>'
>
>> the CPUs declare to have stable TSC. Looking at the clocksource unstable
>> detection, there are two problems:
>> - watchdog clock source wrap. HPET is the most common watchdog clock
>>   source. It's 32-bit and runs in 14.3Mhz. That means the hpet counter
>>   can wrap in about 5 minutes.
>> - threshold isn't scaled against interval. The threshold is 0.0625s in
>>   0.5s interval. What if the actual interval is bigger than 0.5s?
>>
>> The watchdog runs in a timer bh, so hard/soft irq can defer its running.
>> Heavy network stack softirq can hog a cpu. IPMI driver can disable
>> interrupt for a very long time.
>
> And they hold off the timer softirq for more than a second? Don't you
> think that's the problem which needs to be fixed?

Though this is an issue I've experienced (and tried unsuccessfully to
fix in a more complicated way) with the RT kernel, where high priority
tasks blocked the watchdog long enough that we'd disqualify the TSC.

Ideally that sort of high-priority RT busyness would be avoided, but
its also a pain to have false positive trigger when doing things like
stress testing.


>> The first problem is mostly we are suffering I think.
>
> So you think that's the root cause and because your patch makes it go
> away it's not necessary to know for sure, right?
>
>> Here is a simple patch to fix the issues. If the waterdog doesn't run
>
> waterdog?

Allergen-free. :)


>> for a long time, we ignore the detection.
>
> What's 'long time'? Please explain the numbers chosen.
>
>> This should work for the two
>
> Emphasis on 'should'?
>
>> problems. For the second one, we probably doen't need to scale if the
>> interval isn't very long.
>
> -ENOPARSE
>
>> @@ -122,9 +122,10 @@ static int clocksource_watchdog_kthread(void *data);
>>  static void __clocksource_change_rating(struct clocksource *cs, int rating);
>>
>>  /*
>> - * Interval: 0.5sec Threshold: 0.0625s
>> + * Interval: 0.5sec MaxInterval: 1s Threshold: 0.0625s
>>   */
>>  #define WATCHDOG_INTERVAL (HZ >> 1)
>> +#define WATCHDOG_MAX_INTERVAL_NS (NSEC_PER_SEC)
>>  #define WATCHDOG_THRESHOLD (NSEC_PER_SEC >> 4)
>>
>>  static void clocksource_watchdog_work(struct work_struct *work)
>> @@ -217,7 +218,9 @@ static void clocksource_watchdog(unsigned long data)
>>                       continue;
>>
>>               /* Check the deviation from the watchdog clocksource. */
>> -             if ((abs(cs_nsec - wd_nsec) > WATCHDOG_THRESHOLD)) {
>> +             if ((abs(cs_nsec - wd_nsec) > WATCHDOG_THRESHOLD) &&
>> +                 cs_nsec < WATCHDOG_MAX_INTERVAL_NS &&
>> +                 wd_nsec < WATCHDOG_MAX_INTERVAL_NS) {
>
> So that adds a new opportunity for undiscovered wreckage:
>
>    clocksource_watchdog();
>    ....                         <--- SMI skews TSC
>    looong_irq_disabled_region();
>    ....
>    clocksource_watchdog();      <--- Does not detect skew
>
> and it will not detect it later on if that SMI was a one time event.
>
> So 'fixing' the watchdog is the wrong approach. Fixing the stuff which
> prevents the watchdog to run is the proper thing to do.

I'm not sure here. I feel like these delay-caused false positives
(I've seen similar reports w/ VMs being stalled) are more common then
one-off SMI TSC skews.

There are hard lines in the timekeeping code, where we do say: Don't
delay us past X or we can't really handle it, but in this case, the
main clocksource is fine and the limit is being caused by the
watchdog. So I think some sort of a solution to remove this
restriction would be good. We don't want to needlessly punish fine
hardware because our checks for bad hardware add extra restrictions.

That said, I agree the "should"s and other vague qualifiers in the
commit description you point out should have more specifics to back
things up. And I'm fine delaying this (and the follow-on) patch until
those details are provided.

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/

[toc] | [prev] | [next] | [standalone]

#1209018 — Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection

From	Shaohua Li <shli@fb.com>
Date	2015-08-18 05:00 +0200
Subject	Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection
Message-ID	<pYFi2-1yY-7@gated-at.bofh.it>
In reply to	#1208917

On Mon, Aug 17, 2015 at 03:17:28PM -0700, John Stultz wrote:
> On Mon, Aug 17, 2015 at 3:04 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Mon, 17 Aug 2015, John Stultz wrote:
> >
> >> From: Shaohua Li <shli@fb.com>
> >>
> >> >From time to time we saw TSC is marked as unstable in our systems, while
> >
> > Stray '>'
> >
> >> the CPUs declare to have stable TSC. Looking at the clocksource unstable
> >> detection, there are two problems:
> >> - watchdog clock source wrap. HPET is the most common watchdog clock
> >>   source. It's 32-bit and runs in 14.3Mhz. That means the hpet counter
> >>   can wrap in about 5 minutes.
> >> - threshold isn't scaled against interval. The threshold is 0.0625s in
> >>   0.5s interval. What if the actual interval is bigger than 0.5s?
> >>
> >> The watchdog runs in a timer bh, so hard/soft irq can defer its running.
> >> Heavy network stack softirq can hog a cpu. IPMI driver can disable
> >> interrupt for a very long time.
> >
> > And they hold off the timer softirq for more than a second? Don't you
> > think that's the problem which needs to be fixed?
> 
> Though this is an issue I've experienced (and tried unsuccessfully to
> fix in a more complicated way) with the RT kernel, where high priority
> tasks blocked the watchdog long enough that we'd disqualify the TSC.
> 
> Ideally that sort of high-priority RT busyness would be avoided, but
> its also a pain to have false positive trigger when doing things like
> stress testing.
> 
> 
> >> The first problem is mostly we are suffering I think.
> >
> > So you think that's the root cause and because your patch makes it go
> > away it's not necessary to know for sure, right?
> >
> >> Here is a simple patch to fix the issues. If the waterdog doesn't run
> >
> > waterdog?
> 
> Allergen-free. :)
> 
> 
> >> for a long time, we ignore the detection.
> >
> > What's 'long time'? Please explain the numbers chosen.
> >
> >> This should work for the two
> >
> > Emphasis on 'should'?
> >
> >> problems. For the second one, we probably doen't need to scale if the
> >> interval isn't very long.
> >
> > -ENOPARSE
> >
> >> @@ -122,9 +122,10 @@ static int clocksource_watchdog_kthread(void *data);
> >>  static void __clocksource_change_rating(struct clocksource *cs, int rating);
> >>
> >>  /*
> >> - * Interval: 0.5sec Threshold: 0.0625s
> >> + * Interval: 0.5sec MaxInterval: 1s Threshold: 0.0625s
> >>   */
> >>  #define WATCHDOG_INTERVAL (HZ >> 1)
> >> +#define WATCHDOG_MAX_INTERVAL_NS (NSEC_PER_SEC)
> >>  #define WATCHDOG_THRESHOLD (NSEC_PER_SEC >> 4)
> >>
> >>  static void clocksource_watchdog_work(struct work_struct *work)
> >> @@ -217,7 +218,9 @@ static void clocksource_watchdog(unsigned long data)
> >>                       continue;
> >>
> >>               /* Check the deviation from the watchdog clocksource. */
> >> -             if ((abs(cs_nsec - wd_nsec) > WATCHDOG_THRESHOLD)) {
> >> +             if ((abs(cs_nsec - wd_nsec) > WATCHDOG_THRESHOLD) &&
> >> +                 cs_nsec < WATCHDOG_MAX_INTERVAL_NS &&
> >> +                 wd_nsec < WATCHDOG_MAX_INTERVAL_NS) {
> >
> > So that adds a new opportunity for undiscovered wreckage:
> >
> >    clocksource_watchdog();
> >    ....                         <--- SMI skews TSC
> >    looong_irq_disabled_region();
> >    ....
> >    clocksource_watchdog();      <--- Does not detect skew
> >
> > and it will not detect it later on if that SMI was a one time event.
> >
> > So 'fixing' the watchdog is the wrong approach. Fixing the stuff which
> > prevents the watchdog to run is the proper thing to do.
> 
> I'm not sure here. I feel like these delay-caused false positives
> (I've seen similar reports w/ VMs being stalled) are more common then
> one-off SMI TSC skews.
> 
> There are hard lines in the timekeeping code, where we do say: Don't
> delay us past X or we can't really handle it, but in this case, the
> main clocksource is fine and the limit is being caused by the
> watchdog. So I think some sort of a solution to remove this
> restriction would be good. We don't want to needlessly punish fine
> hardware because our checks for bad hardware add extra restrictions.
> 
> That said, I agree the "should"s and other vague qualifiers in the
> commit description you point out should have more specifics to back
> things up. And I'm fine delaying this (and the follow-on) patch until
> those details are provided.

It's not something I guess. We do see the issue from time to time. The
IPMI driver accesses some IO ports in softirq and hog cpu for a very
long time, then the watchdog alert. The false alert on the other hand
has very worse effect. It forces to use HPET as clocksource, which has
very big performance penality. We can't even manually switch back to TSC
as current interface doesn't allow us to do it, then we can only reboot
the system. I agree the driver should be fixed, but the watchdog has
false alert, we definitively should fix it.

The 1s interval is arbitary. If you think there is better way to fix the
issue, please let me know.

Thanks,
Shaohua
--
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/

[toc] | [prev] | [next] | [standalone]

#1209025 — Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection

From	John Stultz <john.stultz@linaro.org>
Date	2015-08-18 05:40 +0200
Subject	Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection
Message-ID	<pYFUJ-2x9-9@gated-at.bofh.it>
In reply to	#1209018

On Mon, Aug 17, 2015 at 7:57 PM, Shaohua Li <shli@fb.com> wrote:
> On Mon, Aug 17, 2015 at 03:17:28PM -0700, John Stultz wrote:
>> That said, I agree the "should"s and other vague qualifiers in the
>> commit description you point out should have more specifics to back
>> things up. And I'm fine delaying this (and the follow-on) patch until
>> those details are provided.
>
> It's not something I guess. We do see the issue from time to time. The
> IPMI driver accesses some IO ports in softirq and hog cpu for a very
> long time, then the watchdog alert. The false alert on the other hand
> has very worse effect. It forces to use HPET as clocksource, which has
> very big performance penality. We can't even manually switch back to TSC
> as current interface doesn't allow us to do it, then we can only reboot
> the system. I agree the driver should be fixed, but the watchdog has
> false alert, we definitively should fix it.

I think Thomas is requesting that some of the vague terms be
quantified. Seeing the issue "from time to time" isn't super
informative. When the IPMI driver hogs the cpu "for a very long time",
how long does that  actually take?  You've provided the HPET
frequency, and  the wrapping interval on your hardware. Do these
intervals all line up properly?

I sympathize that the "show-your-work" math problem aspect of this
request might be a little remedial and irritating, esp when the patch
fixes the problem for you. But its important, so later on when some
bug crops up in near by code, folks can easily repeat your calculation
and know the problem isn't from your code.

> The 1s interval is arbitary. If you think there is better way to fix the
> issue, please let me know.

I don't think 1s is necessarily arbitrary. Maybe not much conscious
thought was put into it, but clearly .001 sec wasn't chosen, nor
10minutes for a reason.

So given the intervals you're seeing the problem with, would maybe a
larger max interval (say 30-seconds) make more or less sense? What
would the tradeoffs be? (ie: Would that exclude clocksources with
faster wraps from being used as watchdogs, with your patches?).

I'm sure an good interval could be chosen with some thought, and the
rational be explained. :)

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/

[toc] | [prev] | [next] | [standalone]

#1209142 — Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection

From	Thomas Gleixner <tglx@linutronix.de>
Date	2015-08-18 11:00 +0200
Subject	Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection
Message-ID	<pYKUr-1e1-25@gated-at.bofh.it>
In reply to	#1209018

On Mon, 17 Aug 2015, Shaohua Li wrote:
> On Mon, Aug 17, 2015 at 03:17:28PM -0700, John Stultz wrote:
> > That said, I agree the "should"s and other vague qualifiers in the
> > commit description you point out should have more specifics to back
> > things up. And I'm fine delaying this (and the follow-on) patch until
> > those details are provided.
> 
> It's not something I guess. We do see the issue from time to time. The
> IPMI driver accesses some IO ports in softirq and hog cpu for a very
> long time, then the watchdog alert.

You still fail to provide proper numbers. 'very long time' does not
qualify as an argument at all.

> The false alert on the other hand has very worse effect. It forces
> to use HPET as clocksource, which has very big performance
> penality. We can't even manually switch back to TSC as current
> interface doesn't allow us to do it, then we can only reboot the
> system. I agree the driver should be fixed, but the watchdog has
> false alert, we definitively should fix it.

I tend to disagree. The watchdog has constraints and the driver is
violating these constraints, so the first thing which wants to be
addressed is the driver itself.

The behaviour of the watchdog in the case of constraint violations is
definitely suboptimal and can lead to false positives. I'm not against
making that more robust, but I'm not accepting your 'watchdog is
broken' argumentation at all.

Thanks,

	tglx
--
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/

[toc] | [prev] | [next] | [standalone]

#1209127 — Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection

From	Thomas Gleixner <tglx@linutronix.de>
Date	2015-08-18 10:40 +0200
Subject	Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection
Message-ID	<pYKB5-Rg-25@gated-at.bofh.it>
In reply to	#1208917

On Mon, 17 Aug 2015, John Stultz wrote:
> On Mon, Aug 17, 2015 at 3:04 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Mon, 17 Aug 2015, John Stultz wrote:
> >
> >> From: Shaohua Li <shli@fb.com>
> >>
> >> >From time to time we saw TSC is marked as unstable in our systems, while
> >
> > Stray '>'
> >
> >> the CPUs declare to have stable TSC. Looking at the clocksource unstable
> >> detection, there are two problems:
> >> - watchdog clock source wrap. HPET is the most common watchdog clock
> >>   source. It's 32-bit and runs in 14.3Mhz. That means the hpet counter
> >>   can wrap in about 5 minutes.
> >> - threshold isn't scaled against interval. The threshold is 0.0625s in
> >>   0.5s interval. What if the actual interval is bigger than 0.5s?
> >>
> >> The watchdog runs in a timer bh, so hard/soft irq can defer its running.
> >> Heavy network stack softirq can hog a cpu. IPMI driver can disable
> >> interrupt for a very long time.
> >
> > And they hold off the timer softirq for more than a second? Don't you
> > think that's the problem which needs to be fixed?
> 
> Though this is an issue I've experienced (and tried unsuccessfully to
> fix in a more complicated way) with the RT kernel, where high priority
> tasks blocked the watchdog long enough that we'd disqualify the TSC.

Did it disqualify the watchdog due to HPET wraparounds (5 minutes) or
due to the fixed threshold being applied?

> > So 'fixing' the watchdog is the wrong approach. Fixing the stuff which
> > prevents the watchdog to run is the proper thing to do.
> 
> I'm not sure here. I feel like these delay-caused false positives
> (I've seen similar reports w/ VMs being stalled) are more common then
> one-off SMI TSC skews.

Yes, they are more common, but the other issues are reality as well.

> There are hard lines in the timekeeping code, where we do say: Don't
> delay us past X or we can't really handle it, but in this case, the
> main clocksource is fine and the limit is being caused by the
> watchdog. So I think some sort of a solution to remove this
> restriction would be good. We don't want to needlessly punish fine
> hardware because our checks for bad hardware add extra restrictions.

No argument here. Though fine hardware has an escape route already to
avoid the watchdog business alltogether (tsc=reliable on the command
line).

> That said, I agree the "should"s and other vague qualifiers in the
> commit description you point out should have more specifics to back
> things up. And I'm fine delaying this (and the follow-on) patch until
> those details are provided.

Fair enough.

Thanks,

	tglx


--
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/

[toc] | [prev] | [next] | [standalone]

#1209409 — Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection

From	John Stultz <john.stultz@linaro.org>
Date	2015-08-18 19:50 +0200
Subject	Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection
Message-ID	<pYTbk-4Se-17@gated-at.bofh.it>
In reply to	#1209127

On Tue, Aug 18, 2015 at 1:38 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Mon, 17 Aug 2015, John Stultz wrote:
>> On Mon, Aug 17, 2015 at 3:04 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > On Mon, 17 Aug 2015, John Stultz wrote:
>> >
>> >> From: Shaohua Li <shli@fb.com>
>> >>
>> >> >From time to time we saw TSC is marked as unstable in our systems, while
>> >
>> > Stray '>'
>> >
>> >> the CPUs declare to have stable TSC. Looking at the clocksource unstable
>> >> detection, there are two problems:
>> >> - watchdog clock source wrap. HPET is the most common watchdog clock
>> >>   source. It's 32-bit and runs in 14.3Mhz. That means the hpet counter
>> >>   can wrap in about 5 minutes.
>> >> - threshold isn't scaled against interval. The threshold is 0.0625s in
>> >>   0.5s interval. What if the actual interval is bigger than 0.5s?
>> >>
>> >> The watchdog runs in a timer bh, so hard/soft irq can defer its running.
>> >> Heavy network stack softirq can hog a cpu. IPMI driver can disable
>> >> interrupt for a very long time.
>> >
>> > And they hold off the timer softirq for more than a second? Don't you
>> > think that's the problem which needs to be fixed?
>>
>> Though this is an issue I've experienced (and tried unsuccessfully to
>> fix in a more complicated way) with the RT kernel, where high priority
>> tasks blocked the watchdog long enough that we'd disqualify the TSC.
>
> Did it disqualify the watchdog due to HPET wraparounds (5 minutes) or
> due to the fixed threshold being applied?

This was years ago, but in my experience, the watchdog false positives
were due to HPET wraparounds.

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/

[toc] | [prev] | [next] | [standalone]

#1209443 — Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection

From	Thomas Gleixner <tglx@linutronix.de>
Date	2015-08-18 21:30 +0200
Subject	Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection
Message-ID	<pYUK5-7cl-7@gated-at.bofh.it>
In reply to	#1209409

On Tue, 18 Aug 2015, John Stultz wrote:
> On Tue, Aug 18, 2015 at 1:38 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Mon, 17 Aug 2015, John Stultz wrote:
> >> On Mon, Aug 17, 2015 at 3:04 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >> > On Mon, 17 Aug 2015, John Stultz wrote:
> >> >
> >> >> From: Shaohua Li <shli@fb.com>
> >> >>
> >> >> >From time to time we saw TSC is marked as unstable in our systems, while
> >> >
> >> > Stray '>'
> >> >
> >> >> the CPUs declare to have stable TSC. Looking at the clocksource unstable
> >> >> detection, there are two problems:
> >> >> - watchdog clock source wrap. HPET is the most common watchdog clock
> >> >>   source. It's 32-bit and runs in 14.3Mhz. That means the hpet counter
> >> >>   can wrap in about 5 minutes.
> >> >> - threshold isn't scaled against interval. The threshold is 0.0625s in
> >> >>   0.5s interval. What if the actual interval is bigger than 0.5s?
> >> >>
> >> >> The watchdog runs in a timer bh, so hard/soft irq can defer its running.
> >> >> Heavy network stack softirq can hog a cpu. IPMI driver can disable
> >> >> interrupt for a very long time.
> >> >
> >> > And they hold off the timer softirq for more than a second? Don't you
> >> > think that's the problem which needs to be fixed?
> >>
> >> Though this is an issue I've experienced (and tried unsuccessfully to
> >> fix in a more complicated way) with the RT kernel, where high priority
> >> tasks blocked the watchdog long enough that we'd disqualify the TSC.
> >
> > Did it disqualify the watchdog due to HPET wraparounds (5 minutes) or
> > due to the fixed threshold being applied?
> 
> This was years ago, but in my experience, the watchdog false positives
> were due to HPET wraparounds.

Blocking stuff for 5 minutes is insane ....
 
--
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/

[toc] | [prev] | [next] | [standalone]

Page 1 of 2 [1] 2 Next page →

csiph-web

[PATCH 0/9] Time items for 4.3

Contents

#1208866 — [PATCH 0/9] Time items for 4.3

#1208867 — [PATCH 2/9] time: Fix nanosecond file time rounding in timespec_trunc()

#1208919 — Re: [PATCH 2/9] time: Fix nanosecond file time rounding in timespec_trunc()

#1208870 — [PATCH 6/9] time: Introduce current_kernel_time64()

#1208871 — [PATCH 1/9] timer_list: Add the base offset so remaining nsecs are accurate for non monotonic timers

#1208884 — Re: [PATCH 1/9] timer_list: Add the base offset so remaining nsecs are accurate for non monotonic timers

#1208885 — Re: [PATCH 1/9] timer_list: Add the base offset so remaining nsecs are accurate for non monotonic timers

#1208887 — Re: [PATCH 1/9] timer_list: Add the base offset so remaining nsecs are accurate for non monotonic timers

#1208872 — [PATCH 3/9] time: Always make sure wall_to_monotonic isn't positive

#1208874 — [PATCH 7/9] time: Introduce timespec64_to_jiffies()/jiffies_to_timespec64()

#1208875 — [PATCH 4/9] time: Add the common weak version of update_persistent_clock()

#1208877 — [PATCH 8/9] clocksource: Improve unstable clocksource detection

#1208915 — Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection

#1208917 — Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection

#1209018 — Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection

#1209025 — Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection

#1209142 — Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection

#1209127 — Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection

#1209409 — Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection

#1209443 — Re: [PATCH 8/9] clocksource: Improve unstable clocksource detection