All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/11][GIT PULL] Timekeeping items for 4.5
@ 2015-12-18 21:39 John Stultz
  2015-12-18 21:39 ` [PATCH 01/11] MAINTAINERS: Add entry for kernel/time/alarmtimer.c John Stultz
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: John Stultz @ 2015-12-18 21:39 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Thomas Gleixner, Ingo Molnar, Miroslav Lichvar,
	Prarit Bhargava, Richard Cochran

Hey Thomas, Ingo,
  Given folks (me included) are getting ready for end of year
vacations, I wanted to send out my queue for 4.5 for
tip/timers/core

Let me know if you have any objections.

thanks
-john

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Miroslav Lichvar <mlichvar@redhat.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>

The following changes since commit 8005c49d9aea74d382f474ce11afbbc7d7130bec:

  Linux 4.4-rc1 (2015-11-15 17:00:27 -0800)

are available in the git repository at:

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

for you to fetch changes up to ec02b076ceab63f99e5b3d80fd223d777266c236:

  timekeeping: Cap adjustments so they don't exceed the maxadj value (2015-12-16 16:50:57 -0800)

----------------------------------------------------------------

Andrzej Hajda (1):
  selftests/timers: fix write return value handlng

David Gibson (1):
  time: Avoid signed overflow in timekeeping_get_ns()

DengChao (3):
  timekeeping: Provide internal function __ktime_get_real_seconds
  ntp: Change time_reftime to time64_t and utilize 64bit
    __ktime_get_real_seconds
  ntp: Fix second_overflow's input parameter type to be 64bits

John Stultz (3):
  MAINTAINERS: Add entry for kernel/time/alarmtimer.c
  time: Verify time values in adjtimex ADJ_SETOFFSET to avoid overflow
  timekeeping: Cap adjustments so they don't exceed the maxadj value

Sasha Levin (1):
  ntp: Verify offset doesn't overflow in ntp_update_offset

Seiichi Ikarashi (1):
  clocksource: Add CPU info to clocksource watchdog reporting

zhuo-hao (1):
  alarmtimer: Avoid unexpected rtc interrupt when system resume from S3

 MAINTAINERS                                        |  3 +-
 include/linux/time.h                               | 26 ++++++++++++
 kernel/time/alarmtimer.c                           | 17 ++++++++
 kernel/time/clocksource.c                          |  4 +-
 kernel/time/ntp.c                                  | 44 ++++++++++++-------
 kernel/time/ntp_internal.h                         |  2 +-
 kernel/time/timekeeping.c                          | 49 ++++++++++++++++++----
 kernel/time/timekeeping_internal.h                 |  2 +
 .../testing/selftests/timers/clocksource-switch.c  |  2 +-
 9 files changed, 121 insertions(+), 28 deletions(-)

-- 
1.9.1


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

* [PATCH 01/11] MAINTAINERS: Add entry for kernel/time/alarmtimer.c
  2015-12-18 21:39 [PATCH 00/11][GIT PULL] Timekeeping items for 4.5 John Stultz
@ 2015-12-18 21:39 ` John Stultz
  2015-12-18 21:39 ` [PATCH 02/11] alarmtimer: Avoid unexpected rtc interrupt when system resume from S3 John Stultz
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2015-12-18 21:39 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Thomas Gleixner, Ingo Molnar, Miroslav Lichvar,
	Prarit Bhargava, Richard Cochran

I've been missing patches against alarmtimer.c due to
a lack of a proper entry for it in the MAINTAINERS file.

So update MAINTAINERS to fix this, adding it in with the
timekeeping, ntp and core clocksource logic I share with
Thomas.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Miroslav Lichvar <mlichvar@redhat.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 MAINTAINERS | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index e9caa4b..e11c043 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9334,7 +9334,7 @@ M:	Andreas Noever <andreas.noever@gmail.com>
 S:	Maintained
 F:	drivers/thunderbolt/
 
-TIMEKEEPING, CLOCKSOURCE CORE, NTP
+TIMEKEEPING, CLOCKSOURCE CORE, NTP, ALARMTIMER
 M:	John Stultz <john.stultz@linaro.org>
 M:	Thomas Gleixner <tglx@linutronix.de>
 L:	linux-kernel@vger.kernel.org
@@ -9347,6 +9347,7 @@ F:	include/uapi/linux/time.h
 F:	include/uapi/linux/timex.h
 F:	kernel/time/clocksource.c
 F:	kernel/time/time*.c
+F:	kernel/time/alarmtimer.c
 F:	kernel/time/ntp.c
 F:	tools/testing/selftests/timers/
 
-- 
1.9.1


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

* [PATCH 02/11] alarmtimer: Avoid unexpected rtc interrupt when system resume from S3
  2015-12-18 21:39 [PATCH 00/11][GIT PULL] Timekeeping items for 4.5 John Stultz
  2015-12-18 21:39 ` [PATCH 01/11] MAINTAINERS: Add entry for kernel/time/alarmtimer.c John Stultz
@ 2015-12-18 21:39 ` John Stultz
  2015-12-18 21:39 ` [PATCH 03/11] time: Avoid signed overflow in timekeeping_get_ns() John Stultz
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2015-12-18 21:39 UTC (permalink / raw)
  To: lkml
  Cc: zhuo-hao, Thomas Gleixner, Ingo Molnar, Miroslav Lichvar,
	Prarit Bhargava, Richard Cochran, John Stultz

From: zhuo-hao <zhuo-hao.lee@intel.com>

Before the system go to suspend (S3), if user create a timer
with clockid CLOCK_REALTIME_ALARM/CLOCK_BOOTTIME_ALARM and set a
"large" timeout value to this timer. The function
alarmtimer_suspend will be called to setup a timeout value to
RTC timer to avoid the system sleep over time. However, if the
system wakeup early than RTC timeout, the RTC timer will not be
cleared. And this will cause the hpet_rtc_interrupt come
unexpectedly until the RTC timeout. To fix this problem, just
adding alarmtimer_resume to cancel the RTC timer.

This was noticed because the HPET RTC emulation fires an
interrupt every 16ms(=1/2^DEFAULT_RTC_SHIFT) up to the point
where the alarm time is reached.

This program always hits this situation
(https://lkml.org/lkml/2015/11/8/326), if system wake up earlier
than alarm time.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Miroslav Lichvar <mlichvar@redhat.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Zhuo-hao Lee <zhuo-hao.lee@intel.com>
[jstultz: Tweak commit subject & formatting slightly]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/alarmtimer.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 7fbba635..e840ed86 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -271,11 +271,27 @@ static int alarmtimer_suspend(struct device *dev)
 		__pm_wakeup_event(ws, MSEC_PER_SEC);
 	return ret;
 }
+
+static int alarmtimer_resume(struct device *dev)
+{
+	struct rtc_device *rtc;
+
+	rtc = alarmtimer_get_rtcdev();
+	if (rtc)
+		rtc_timer_cancel(rtc, &rtctimer);
+	return 0;
+}
+
 #else
 static int alarmtimer_suspend(struct device *dev)
 {
 	return 0;
 }
+
+static int alarmtimer_resume(struct device *dev)
+{
+	return 0;
+}
 #endif
 
 static void alarmtimer_freezerset(ktime_t absexp, enum alarmtimer_type type)
@@ -800,6 +816,7 @@ out:
 /* Suspend hook structures */
 static const struct dev_pm_ops alarmtimer_pm_ops = {
 	.suspend = alarmtimer_suspend,
+	.resume = alarmtimer_resume,
 };
 
 static struct platform_driver alarmtimer_driver = {
-- 
1.9.1


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

* [PATCH 03/11] time: Avoid signed overflow in timekeeping_get_ns()
  2015-12-18 21:39 [PATCH 00/11][GIT PULL] Timekeeping items for 4.5 John Stultz
  2015-12-18 21:39 ` [PATCH 01/11] MAINTAINERS: Add entry for kernel/time/alarmtimer.c John Stultz
  2015-12-18 21:39 ` [PATCH 02/11] alarmtimer: Avoid unexpected rtc interrupt when system resume from S3 John Stultz
@ 2015-12-18 21:39 ` John Stultz
  2015-12-18 21:39 ` [PATCH 04/11] clocksource: Add CPU info to clocksource watchdog reporting John Stultz
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2015-12-18 21:39 UTC (permalink / raw)
  To: lkml
  Cc: David Gibson, 3.7+ after 4.5-rc2, Thomas Gleixner, Ingo Molnar,
	Miroslav Lichvar, Prarit Bhargava, Richard Cochran, John Stultz

From: David Gibson <david@gibson.dropbear.id.au>

1e75fa8 "time: Condense timekeeper.xtime into xtime_sec" replaced a call to
clocksource_cyc2ns() from timekeeping_get_ns() with an open-coded version
of the same logic to avoid keeping a semi-redundant struct timespec
in struct timekeeper.

However, the commit also introduced a subtle semantic change - where
clocksource_cyc2ns() uses purely unsigned math, the new version introduces
a signed temporary, meaning that if (delta * tk->mult) has a 63-bit
overflow the following shift will still give a negative result.  The
choice of 'maxsec' in __clocksource_updatefreq_scale() means this will
generally happen if there's a ~10 minute pause in examining the
clocksource.

This can be triggered on a powerpc KVM guest by stopping it from qemu for
a bit over 10 minutes.  After resuming time has jumped backwards several
minutes causing numerous problems (jiffies does not advance, msleep()s can
be extended by minutes..).  It doesn't happen on x86 KVM guests, because
the guest TSC is effectively frozen while the guest is stopped, which is
not the case for the powerpc timebase.

Obviously an unsigned (64 bit) overflow will only take twice as long as a
signed, 63-bit overflow.  I don't know the time code well enough to know
if that will still cause incorrect calculations, or if a 64-bit overflow
is avoided elsewhere.

Still, an incorrect forwards clock adjustment will cause less trouble than
time going backwards.  So, this patch removes the potential for
intermediate signed overflow.

Cc: stable@vger.kernel.org  (3.7+ after 4.5-rc2)
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Miroslav Lichvar <mlichvar@redhat.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Suggested-by: Laurent Vivier <lvivier@redhat.com>
Tested-by: Laurent Vivier <lvivier@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/timekeeping.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index d563c19..99188ee 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -305,8 +305,7 @@ static inline s64 timekeeping_get_ns(struct tk_read_base *tkr)
 
 	delta = timekeeping_get_delta(tkr);
 
-	nsec = delta * tkr->mult + tkr->xtime_nsec;
-	nsec >>= tkr->shift;
+	nsec = (delta * tkr->mult + tkr->xtime_nsec) >> tkr->shift;
 
 	/* If arch requires, add in get_arch_timeoffset() */
 	return nsec + arch_gettimeoffset();
-- 
1.9.1


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

* [PATCH 04/11] clocksource: Add CPU info to clocksource watchdog reporting
  2015-12-18 21:39 [PATCH 00/11][GIT PULL] Timekeeping items for 4.5 John Stultz
                   ` (2 preceding siblings ...)
  2015-12-18 21:39 ` [PATCH 03/11] time: Avoid signed overflow in timekeeping_get_ns() John Stultz
@ 2015-12-18 21:39 ` John Stultz
  2015-12-18 21:39 ` [PATCH 05/11] selftests/timers: fix write return value handlng John Stultz
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2015-12-18 21:39 UTC (permalink / raw)
  To: lkml
  Cc: Seiichi Ikarashi, Thomas Gleixner, Ingo Molnar, Miroslav Lichvar,
	Prarit Bhargava, Richard Cochran, John Stultz

From: Seiichi Ikarashi <s.ikarashi@jp.fujitsu.com>

The clocksource watchdog reporting was improved by 0b046b217ad4c6.
I want to add the info of CPU where the watchdog detects a
deviation because it is necessary to identify the trouble spot
if the clocksource is TSC.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Miroslav Lichvar <mlichvar@redhat.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Seiichi Ikarashi <s.ikarashi@jp.fujitsu.com>
[jstultz: Tweaked commit message]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/clocksource.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index 1347882..664de53 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -218,8 +218,8 @@ static void clocksource_watchdog(unsigned long data)
 
 		/* Check the deviation from the watchdog clocksource. */
 		if (abs(cs_nsec - wd_nsec) > WATCHDOG_THRESHOLD) {
-			pr_warn("timekeeping watchdog: Marking clocksource '%s' as unstable because the skew is too large:\n",
-				cs->name);
+			pr_warn("timekeeping watchdog on CPU%d: Marking clocksource '%s' as unstable because the skew is too large:\n",
+				smp_processor_id(), cs->name);
 			pr_warn("                      '%s' wd_now: %llx wd_last: %llx mask: %llx\n",
 				watchdog->name, wdnow, wdlast, watchdog->mask);
 			pr_warn("                      '%s' cs_now: %llx cs_last: %llx mask: %llx\n",
-- 
1.9.1


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

* [PATCH 05/11] selftests/timers: fix write return value handlng
  2015-12-18 21:39 [PATCH 00/11][GIT PULL] Timekeeping items for 4.5 John Stultz
                   ` (3 preceding siblings ...)
  2015-12-18 21:39 ` [PATCH 04/11] clocksource: Add CPU info to clocksource watchdog reporting John Stultz
@ 2015-12-18 21:39 ` John Stultz
  2015-12-18 21:39 ` [PATCH 06/11] ntp: Verify offset doesn't overflow in ntp_update_offset John Stultz
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2015-12-18 21:39 UTC (permalink / raw)
  To: lkml
  Cc: Andrzej Hajda, Thomas Gleixner, Ingo Molnar, Miroslav Lichvar,
	Prarit Bhargava, Richard Cochran, John Stultz

From: Andrzej Hajda <a.hajda@samsung.com>

The function can return negative value.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Miroslav Lichvar <mlichvar@redhat.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 tools/testing/selftests/timers/clocksource-switch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/timers/clocksource-switch.c b/tools/testing/selftests/timers/clocksource-switch.c
index 627ec74..fd88e30 100644
--- a/tools/testing/selftests/timers/clocksource-switch.c
+++ b/tools/testing/selftests/timers/clocksource-switch.c
@@ -97,7 +97,7 @@ int get_cur_clocksource(char *buf, size_t size)
 int change_clocksource(char *clocksource)
 {
 	int fd;
-	size_t size;
+	ssize_t size;
 
 	fd = open("/sys/devices/system/clocksource/clocksource0/current_clocksource", O_WRONLY);
 
-- 
1.9.1


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

* [PATCH 06/11] ntp: Verify offset doesn't overflow in ntp_update_offset
  2015-12-18 21:39 [PATCH 00/11][GIT PULL] Timekeeping items for 4.5 John Stultz
                   ` (4 preceding siblings ...)
  2015-12-18 21:39 ` [PATCH 05/11] selftests/timers: fix write return value handlng John Stultz
@ 2015-12-18 21:39 ` John Stultz
  2015-12-18 21:39 ` [PATCH 07/11] time: Verify time values in adjtimex ADJ_SETOFFSET to avoid overflow John Stultz
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2015-12-18 21:39 UTC (permalink / raw)
  To: lkml
  Cc: Sasha Levin, Thomas Gleixner, Ingo Molnar, Miroslav Lichvar,
	Prarit Bhargava, Richard Cochran, John Stultz

From: Sasha Levin <sasha.levin@oracle.com>

We need to make sure that the offset is valid before manipulating it,
otherwise it might overflow on the multiplication.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Miroslav Lichvar <mlichvar@redhat.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
[jstultz: Reworked one of the checks so it makes more sense]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/ntp.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 149cc80..125fc03 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -297,15 +297,17 @@ static void ntp_update_offset(long offset)
 	if (!(time_status & STA_PLL))
 		return;
 
-	if (!(time_status & STA_NANO))
+	if (!(time_status & STA_NANO)) {
+		/* Make sure the multiplication below won't overflow */
+		offset = clamp(offset, -USEC_PER_SEC, USEC_PER_SEC);
 		offset *= NSEC_PER_USEC;
+	}
 
 	/*
 	 * Scale the phase adjustment and
 	 * clamp to the operating range.
 	 */
-	offset = min(offset, MAXPHASE);
-	offset = max(offset, -MAXPHASE);
+	offset = clamp(offset, -MAXPHASE, MAXPHASE);
 
 	/*
 	 * Select how the frequency is to be controlled
-- 
1.9.1


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

* [PATCH 07/11] time: Verify time values in adjtimex ADJ_SETOFFSET to avoid overflow
  2015-12-18 21:39 [PATCH 00/11][GIT PULL] Timekeeping items for 4.5 John Stultz
                   ` (5 preceding siblings ...)
  2015-12-18 21:39 ` [PATCH 06/11] ntp: Verify offset doesn't overflow in ntp_update_offset John Stultz
@ 2015-12-18 21:39 ` John Stultz
  2015-12-18 21:39 ` [PATCH 08/11] timekeeping: Provide internal function __ktime_get_real_seconds John Stultz
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2015-12-18 21:39 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Sasha Levin, Thomas Gleixner, Ingo Molnar,
	Miroslav Lichvar, Prarit Bhargava, Richard Cochran

For adjtimex()'s ADJ_SETOFFSET, make sure the tv_usec value is
sane. We might multiply them later which can cause an overflow
and undefined behavior.

This patch introduces new helper functions to simplify the
checking code and adds comments to clarify

Orginally this patch was by Sasha Levin, but I've basically
rewritten it, so he should get credit for finding the issue
and I should get the blame for any mistakes made since.

Also, credit to Richard Cochran for the phrasing used in the
comment for what is considered valid here.

Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Miroslav Lichvar <mlichvar@redhat.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Reported-by: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 include/linux/time.h      | 26 ++++++++++++++++++++++++++
 kernel/time/ntp.c         | 10 ++++++++--
 kernel/time/timekeeping.c |  2 +-
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/include/linux/time.h b/include/linux/time.h
index beebe3a..297f09f 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -125,6 +125,32 @@ static inline bool timeval_valid(const struct timeval *tv)
 
 extern struct timespec timespec_trunc(struct timespec t, unsigned gran);
 
+/*
+ * Validates if a timespec/timeval used to inject a time offset is valid.
+ * Offsets can be postive or negative. The value of the timeval/timespec
+ * is the sum of its fields, but *NOTE*: the field tv_usec/tv_nsec must
+ * always be non-negative.
+ */
+static inline bool timeval_inject_offset_valid(const struct timeval *tv)
+{
+	/* We don't check the tv_sec as it can be positive or negative */
+
+	/* Can't have more microseconds then a second */
+	if (tv->tv_usec < 0 || tv->tv_usec >= USEC_PER_SEC)
+		return false;
+	return true;
+}
+
+static inline bool timespec_inject_offset_valid(const struct timespec *ts)
+{
+	/* We don't check the tv_sec as it can be positive or negative */
+
+	/* Can't have more nanoseconds then a second */
+	if (ts->tv_nsec < 0 || ts->tv_nsec >= NSEC_PER_SEC)
+		return false;
+	return true;
+}
+
 #define CURRENT_TIME		(current_kernel_time())
 #define CURRENT_TIME_SEC	((struct timespec) { get_seconds(), 0 })
 
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 125fc03..4073c95 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -676,8 +676,14 @@ int ntp_validate_timex(struct timex *txc)
 			return -EINVAL;
 	}
 
-	if ((txc->modes & ADJ_SETOFFSET) && (!capable(CAP_SYS_TIME)))
-		return -EPERM;
+	if (txc->modes & ADJ_SETOFFSET) {
+		/* In order to inject time, you gotta be super-user! */
+		if (!capable(CAP_SYS_TIME))
+			return -EPERM;
+
+		if (!timeval_inject_offset_valid(&txc->time))
+			return -EINVAL;
+	}
 
 	/*
 	 * Check for potential multiplication overflows that can
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 99188ee..d9249da 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -958,7 +958,7 @@ int timekeeping_inject_offset(struct timespec *ts)
 	struct timespec64 ts64, tmp;
 	int ret = 0;
 
-	if ((unsigned long)ts->tv_nsec >= NSEC_PER_SEC)
+	if (!timespec_inject_offset_valid(ts))
 		return -EINVAL;
 
 	ts64 = timespec_to_timespec64(*ts);
-- 
1.9.1


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

* [PATCH 08/11] timekeeping: Provide internal function __ktime_get_real_seconds
  2015-12-18 21:39 [PATCH 00/11][GIT PULL] Timekeeping items for 4.5 John Stultz
                   ` (6 preceding siblings ...)
  2015-12-18 21:39 ` [PATCH 07/11] time: Verify time values in adjtimex ADJ_SETOFFSET to avoid overflow John Stultz
@ 2015-12-18 21:39 ` John Stultz
  2015-12-18 21:39 ` [PATCH 09/11] ntp: Change time_reftime to time64_t and utilize 64bit __ktime_get_real_seconds John Stultz
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2015-12-18 21:39 UTC (permalink / raw)
  To: lkml
  Cc: DengChao, Thomas Gleixner, Ingo Molnar, Miroslav Lichvar,
	Prarit Bhargava, Richard Cochran, John Stultz

From: DengChao <chao.deng@linaro.org>

In order to fix Y2038 issues in the ntp code we will need replace
get_seconds() with ktime_get_real_seconds() but as the ntp code uses
the timekeeping lock which is also used by ktime_get_real_seconds(),
we need a version without locking.
Add a new function __ktime_get_real_seconds() in timekeeping to
do this.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Miroslav Lichvar <mlichvar@redhat.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Reviewed-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: DengChao <chao.deng@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/timekeeping.c          | 13 +++++++++++++
 kernel/time/timekeeping_internal.h |  2 ++
 2 files changed, 15 insertions(+)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index d9249da..21cc239 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -845,6 +845,19 @@ time64_t ktime_get_real_seconds(void)
 }
 EXPORT_SYMBOL_GPL(ktime_get_real_seconds);
 
+/**
+ * __ktime_get_real_seconds - The same as ktime_get_real_seconds
+ * but without the sequence counter protect. This internal function
+ * is called just when timekeeping lock is already held.
+ */
+time64_t __ktime_get_real_seconds(void)
+{
+	struct timekeeper *tk = &tk_core.timekeeper;
+
+	return tk->xtime_sec;
+}
+
+
 #ifdef CONFIG_NTP_PPS
 
 /**
diff --git a/kernel/time/timekeeping_internal.h b/kernel/time/timekeeping_internal.h
index 4ea005a..e20466f 100644
--- a/kernel/time/timekeeping_internal.h
+++ b/kernel/time/timekeeping_internal.h
@@ -26,4 +26,6 @@ static inline cycle_t clocksource_delta(cycle_t now, cycle_t last, cycle_t mask)
 }
 #endif
 
+extern time64_t __ktime_get_real_seconds(void);
+
 #endif /* _TIMEKEEPING_INTERNAL_H */
-- 
1.9.1


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

* [PATCH 09/11] ntp: Change time_reftime to time64_t and utilize 64bit __ktime_get_real_seconds
  2015-12-18 21:39 [PATCH 00/11][GIT PULL] Timekeeping items for 4.5 John Stultz
                   ` (7 preceding siblings ...)
  2015-12-18 21:39 ` [PATCH 08/11] timekeeping: Provide internal function __ktime_get_real_seconds John Stultz
@ 2015-12-18 21:39 ` John Stultz
  2015-12-18 21:39 ` [PATCH 10/11] ntp: Fix second_overflow's input parameter type to be 64bits John Stultz
  2015-12-18 21:39 ` [PATCH 11/11] timekeeping: Cap adjustments so they don't exceed the maxadj value John Stultz
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2015-12-18 21:39 UTC (permalink / raw)
  To: lkml
  Cc: DengChao, Thomas Gleixner, Ingo Molnar, Miroslav Lichvar,
	Prarit Bhargava, Richard Cochran, John Stultz

From: DengChao <chao.deng@linaro.org>

The type of static variant "time_reftime" and the call of
get_seconds in ntp are both not y2038 safe.

So change the type of time_reftime to time64_t and replace
get_seconds with __ktime_get_real_seconds.

The local variant "secs" in ntp_update_offset represents
seconds between now and last ntp adjustment, it seems impossible
that this time will last more than 68 years, so keep its type as
"long".

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Miroslav Lichvar <mlichvar@redhat.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Reviewed-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: DengChao <chao.deng@linaro.org>
[jstultz: Tweaked commit message]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/ntp.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 4073c95..e947bfd 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -18,6 +18,8 @@
 #include <linux/rtc.h>
 
 #include "ntp_internal.h"
+#include "timekeeping_internal.h"
+
 
 /*
  * NTP timekeeping variables:
@@ -70,7 +72,7 @@ static long			time_esterror = NTP_PHASE_LIMIT;
 static s64			time_freq;
 
 /* time at last adjustment (secs):					*/
-static long			time_reftime;
+static time64_t		time_reftime;
 
 static long			time_adjust;
 
@@ -313,11 +315,11 @@ static void ntp_update_offset(long offset)
 	 * Select how the frequency is to be controlled
 	 * and in which mode (PLL or FLL).
 	 */
-	secs = get_seconds() - time_reftime;
+	secs = (long)(__ktime_get_real_seconds() - time_reftime);
 	if (unlikely(time_status & STA_FREQHOLD))
 		secs = 0;
 
-	time_reftime = get_seconds();
+	time_reftime = __ktime_get_real_seconds();
 
 	offset64    = offset;
 	freq_adj    = ntp_update_offset_fll(offset64, secs);
@@ -592,7 +594,7 @@ static inline void process_adj_status(struct timex *txc, struct timespec64 *ts)
 	 * reference time to current time.
 	 */
 	if (!(time_status & STA_PLL) && (txc->status & STA_PLL))
-		time_reftime = get_seconds();
+		time_reftime = __ktime_get_real_seconds();
 
 	/* only set allowed bits */
 	time_status &= STA_RONLY;
-- 
1.9.1


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

* [PATCH 10/11] ntp: Fix second_overflow's input parameter type to be 64bits
  2015-12-18 21:39 [PATCH 00/11][GIT PULL] Timekeeping items for 4.5 John Stultz
                   ` (8 preceding siblings ...)
  2015-12-18 21:39 ` [PATCH 09/11] ntp: Change time_reftime to time64_t and utilize 64bit __ktime_get_real_seconds John Stultz
@ 2015-12-18 21:39 ` John Stultz
  2015-12-18 21:39 ` [PATCH 11/11] timekeeping: Cap adjustments so they don't exceed the maxadj value John Stultz
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2015-12-18 21:39 UTC (permalink / raw)
  To: lkml
  Cc: DengChao, Thomas Gleixner, Ingo Molnar, Miroslav Lichvar,
	Prarit Bhargava, Richard Cochran, John Stultz

From: DengChao <chao.deng@linaro.org>

The function "second_overflow" uses "unsign long"
as its input parameter type which will overflow after
year 2106 on 32bit systems.

Thus this patch replaces it with time64_t type.

While the 64-bit division is expensive, "next_ntp_leap_sec"
has been calculated already, so we can just re-use it in the
TIME_INS/DEL cases, allowing one expensive division per
leapsecond instead of re-doing the divsion once a second after
the leap flag has been set.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Miroslav Lichvar <mlichvar@redhat.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: DengChao <chao.deng@linaro.org>
[jstultz: Tweaked commit message]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/ntp.c          | 16 +++++++++-------
 kernel/time/ntp_internal.h |  2 +-
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index e947bfd..36f2ca0 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -16,6 +16,7 @@
 #include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/rtc.h>
+#include <linux/math64.h>
 
 #include "ntp_internal.h"
 #include "timekeeping_internal.h"
@@ -394,10 +395,11 @@ ktime_t ntp_get_next_leap(void)
  *
  * Also handles leap second processing, and returns leap offset
  */
-int second_overflow(unsigned long secs)
+int second_overflow(time64_t secs)
 {
 	s64 delta;
 	int leap = 0;
+	s32 rem;
 
 	/*
 	 * Leap second processing. If in leap-insert state at the end of the
@@ -408,19 +410,19 @@ int second_overflow(unsigned long secs)
 	case TIME_OK:
 		if (time_status & STA_INS) {
 			time_state = TIME_INS;
-			ntp_next_leap_sec = secs + SECS_PER_DAY -
-						(secs % SECS_PER_DAY);
+			div_s64_rem(secs, SECS_PER_DAY, &rem);
+			ntp_next_leap_sec = secs + SECS_PER_DAY - rem;
 		} else if (time_status & STA_DEL) {
 			time_state = TIME_DEL;
-			ntp_next_leap_sec = secs + SECS_PER_DAY -
-						 ((secs+1) % SECS_PER_DAY);
+			div_s64_rem(secs + 1, SECS_PER_DAY, &rem);
+			ntp_next_leap_sec = secs + SECS_PER_DAY - rem;
 		}
 		break;
 	case TIME_INS:
 		if (!(time_status & STA_INS)) {
 			ntp_next_leap_sec = TIME64_MAX;
 			time_state = TIME_OK;
-		} else if (secs % SECS_PER_DAY == 0) {
+		} else if (secs == ntp_next_leap_sec) {
 			leap = -1;
 			time_state = TIME_OOP;
 			printk(KERN_NOTICE
@@ -431,7 +433,7 @@ int second_overflow(unsigned long secs)
 		if (!(time_status & STA_DEL)) {
 			ntp_next_leap_sec = TIME64_MAX;
 			time_state = TIME_OK;
-		} else if ((secs + 1) % SECS_PER_DAY == 0) {
+		} else if (secs == ntp_next_leap_sec) {
 			leap = 1;
 			ntp_next_leap_sec = TIME64_MAX;
 			time_state = TIME_WAIT;
diff --git a/kernel/time/ntp_internal.h b/kernel/time/ntp_internal.h
index af92447..d8a7c11 100644
--- a/kernel/time/ntp_internal.h
+++ b/kernel/time/ntp_internal.h
@@ -6,7 +6,7 @@ extern void ntp_clear(void);
 /* Returns how long ticks are at present, in ns / 2^NTP_SCALE_SHIFT. */
 extern u64 ntp_tick_length(void);
 extern ktime_t ntp_get_next_leap(void);
-extern int second_overflow(unsigned long secs);
+extern int second_overflow(time64_t secs);
 extern int ntp_validate_timex(struct timex *);
 extern int __do_adjtimex(struct timex *, struct timespec64 *, s32 *);
 extern void __hardpps(const struct timespec64 *, const struct timespec64 *);
-- 
1.9.1


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

* [PATCH 11/11] timekeeping: Cap adjustments so they don't exceed the maxadj value
  2015-12-18 21:39 [PATCH 00/11][GIT PULL] Timekeeping items for 4.5 John Stultz
                   ` (9 preceding siblings ...)
  2015-12-18 21:39 ` [PATCH 10/11] ntp: Fix second_overflow's input parameter type to be 64bits John Stultz
@ 2015-12-18 21:39 ` John Stultz
  10 siblings, 0 replies; 12+ messages in thread
From: John Stultz @ 2015-12-18 21:39 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Ingo Molnar, Miroslav Lichvar, Thomas Gleixner,
	Richard Cochran, Prarit Bhargava, Andy Lutomirski

Thus its been occasionally noted that users have seen
confusing warnings like:

    Adjusting tsc more than 11% (5941981 vs 7759439)

We try to limit the maximum total adjustment to 11% (10% tick
adjustment + 0.5% frequency adjustment). But this is done by
bounding the requested adjustment values, and the internal
steering that is done by tracking the error from what was
requested and what was applied, does not have any such limits.

This is usually not problematic, but in some cases has a risk
that an adjustment could cause the clocksource mult value to
overflow, so its an indication things are outside of what is
expected.

It ends up most of the reports of this 11% warning are on systems
using chrony, which utilizes the adjtimex() ADJ_TICK interface
(which allows a +-10% adjustment). The original rational for
ADJ_TICK unclear to me but my assumption it was originally added
to allow broken systems to get a big constant correction at boot
(see adjtimex userspace package for an example) which would allow
the system to work w/ ntpd's 0.5% adjustment limit.

Chrony uses ADJ_TICK to make very aggressive short term corrections
(usually right at startup). Which push us close enough to the max
bound that a few late ticks can cause the internal steering to push
past the max adjust value (tripping the warning).

Thus this patch adds some extra logic to enforce the max adjustment
cap in the internal steering.

Note: This has the potential to slow corrections when the ADJ_TICK
value is furthest away from the default value. So it would be good to
get some testing from folks using chrony, to make sure we don't
cause any troubles there.

Cc: Ingo Molnar <mingo@kernel.org>
Cc: Miroslav Lichvar <mlichvar@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Andy Lutomirski <luto@kernel.org>
Tested-by: Miroslav Lichvar <mlichvar@redhat.com>
Reported-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/timekeeping.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 21cc239..34b4ced 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1604,9 +1604,12 @@ static __always_inline void timekeeping_freqadjust(struct timekeeper *tk,
 {
 	s64 interval = tk->cycle_interval;
 	s64 xinterval = tk->xtime_interval;
+	u32 base = tk->tkr_mono.clock->mult;
+	u32 max = tk->tkr_mono.clock->maxadj;
+	u32 cur_adj = tk->tkr_mono.mult;
 	s64 tick_error;
 	bool negative;
-	u32 adj;
+	u32 adj_scale;
 
 	/* Remove any current error adj from freq calculation */
 	if (tk->ntp_err_mult)
@@ -1625,13 +1628,33 @@ static __always_inline void timekeeping_freqadjust(struct timekeeper *tk,
 	/* preserve the direction of correction */
 	negative = (tick_error < 0);
 
-	/* Sort out the magnitude of the correction */
+	/* If any adjustment would pass the max, just return */
+	if (negative && (cur_adj - 1) <= (base - max))
+		return;
+	if (!negative && (cur_adj + 1) >= (base + max))
+		return;
+	/*
+	 * Sort out the magnitude of the correction, but
+	 * avoid making so large a correction that we go
+	 * over the max adjustment.
+	 */
+	adj_scale = 0;
 	tick_error = abs(tick_error);
-	for (adj = 0; tick_error > interval; adj++)
+	while (tick_error > interval) {
+		u32 adj = 1 << (adj_scale + 1);
+
+		/* Check if adjustment gets us within 1 unit from the max */
+		if (negative && (cur_adj - adj) <= (base - max))
+			break;
+		if (!negative && (cur_adj + adj) >= (base + max))
+			break;
+
+		adj_scale++;
 		tick_error >>= 1;
+	}
 
 	/* scale the corrections */
-	timekeeping_apply_adjustment(tk, offset, negative, adj);
+	timekeeping_apply_adjustment(tk, offset, negative, adj_scale);
 }
 
 /*
-- 
1.9.1


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

end of thread, other threads:[~2015-12-18 21:42 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-18 21:39 [PATCH 00/11][GIT PULL] Timekeeping items for 4.5 John Stultz
2015-12-18 21:39 ` [PATCH 01/11] MAINTAINERS: Add entry for kernel/time/alarmtimer.c John Stultz
2015-12-18 21:39 ` [PATCH 02/11] alarmtimer: Avoid unexpected rtc interrupt when system resume from S3 John Stultz
2015-12-18 21:39 ` [PATCH 03/11] time: Avoid signed overflow in timekeeping_get_ns() John Stultz
2015-12-18 21:39 ` [PATCH 04/11] clocksource: Add CPU info to clocksource watchdog reporting John Stultz
2015-12-18 21:39 ` [PATCH 05/11] selftests/timers: fix write return value handlng John Stultz
2015-12-18 21:39 ` [PATCH 06/11] ntp: Verify offset doesn't overflow in ntp_update_offset John Stultz
2015-12-18 21:39 ` [PATCH 07/11] time: Verify time values in adjtimex ADJ_SETOFFSET to avoid overflow John Stultz
2015-12-18 21:39 ` [PATCH 08/11] timekeeping: Provide internal function __ktime_get_real_seconds John Stultz
2015-12-18 21:39 ` [PATCH 09/11] ntp: Change time_reftime to time64_t and utilize 64bit __ktime_get_real_seconds John Stultz
2015-12-18 21:39 ` [PATCH 10/11] ntp: Fix second_overflow's input parameter type to be 64bits John Stultz
2015-12-18 21:39 ` [PATCH 11/11] timekeeping: Cap adjustments so they don't exceed the maxadj value John Stultz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.