All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3][RFC] Fix for leapsecond caused futex issue (v4)
@ 2012-07-04  6:21 John Stultz
  2012-07-04  6:21 ` [PATCH 1/3] [RFC] hrtimer: Fix clock_was_set so it is safe to call from irq context John Stultz
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: John Stultz @ 2012-07-04  6:21 UTC (permalink / raw)
  To: Linux Kernel; +Cc: John Stultz, Prarit Bhargava, stable, Thomas Gleixner, linux

Ok, made a few tweaks to address issues caught by Prarit's and my
testing. This has run for a number of hours now w/ my leap-a-day.c
test on a few machines.

I'd really appreciate any extra testing, review, or acks at this point.
I'm targeting mid-late Thursday (to give folks in the US a chance to
review & test) as a point when I'll submit this upstream if no other
issues are found.


As widely reported on the internet, many Linux systems after
the leapsecond was inserted are experiencing futex related load
spikes (usually connected to MySQL, Firefox, Thunderbird, Java, etc).

An apparent  workaround for this issue is running:
$ date -s "`date`"

Credit: http://www.sheeri.com/content/mysql-and-leap-second-high-cpu-and-fix


To address this issue I'm proposing we do three things:
1) Fix the clock_was_set() call to remove the limitation that kept
us from calling it from update_wall_time().

2) Call clock_was_set() when we add/remove a leapsecond.

3) Change hrtimer_interrupt to update the hrtimer base offset values.
This third item provides additional robustness should the
clock_was_set() notification (done via a timer if we're in_atomic)
be delayed significantly.


This third item is new and tries to better address the fact that
the hrtimer code caches its sense of time separately from the
timekeeping core. This is necessary for performance reasons, as
hrtimer code is a very hot path, but opens up races between when
the time offsets have changed and when the hrtimer code updates
its bases on each cpu. By updating the base offsets prior to
doing any expiration, we ensure no timers are expired early.

Close review, however, would be appreciated.

I'm fairly happy with this set of changes, so if there's no
objections, I'd propose merging these for 3.5, and I'll
start generating backports for -stable (unfortunately
these won't apply trivially to 3.3 and prior kernels).

I'm also looking to see if we can consolidate the per-cpu base
offset values, so they are not per-cpu and are protected by their
own lock, allowing us to update them quickly from atomic context, 
even while holding the timekeeper.lock (currently I believe there's
the risk of having an ABBA deadlock between the base.lock and the
timekeeper.lock if we try to update the base offsets under
the timekeepr lock). However this will be potentially a more
significant change and wouldn't be appropriate for backporting,
so I want to get these three changes to fix the issue merged first.


NOTE: Some reports have been of a hard hang right at or before
the leapsecond. I've not been able to reproduce or diagnose
this, so this fix does not likely address the reported hard
hangs (unless they end up being connected to the futex/hrtimer
issue). Please email lkml and me if you experienced this.


TODOs:
* Collect feedback & acks
* Submit for merging.
* Generate a backports for pre-v3.4 kernels


v2:
* Address the issue w/ calling clock_was_set from atomic context,
pointed out by Prarit and Ben.
* Rework fix so its simpler.

v3:
* Change from using a work item to a timer for scheduling the
do_clock_was_set() call sooner.
* Add hrtimer_interrupt base offset updating

v4:
* Fix clock_was_set_timer initialization bug found by Prarit
* Switch from is_atomic() to irqs_disabled(), since is_atomic()
  isn't a sufficient check prior to calling smp_call_function()
  

CC: Prarit Bhargava <prarit@redhat.com>
CC: stable@vger.kernel.org
CC: Thomas Gleixner <tglx@linutronix.de>
CC: linux@openhuawei.org

John Stultz (3):
  [RFC] hrtimer: Fix clock_was_set so it is safe to call from irq
    context
  [RFC] time: Fix leapsecond triggered hrtimer/futex load spike issue
  [RFC] hrtimer: Update hrtimer base offsets each hrtimer_interrupt

 include/linux/hrtimer.h   |    3 +++
 kernel/hrtimer.c          |   31 +++++++++++++++++++++++++++----
 kernel/time/timekeeping.c |   38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 4 deletions(-)

-- 
1.7.9.5


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

* [PATCH 1/3] [RFC] hrtimer: Fix clock_was_set so it is safe to call from irq context
  2012-07-04  6:21 [PATCH 0/3][RFC] Fix for leapsecond caused futex issue (v4) John Stultz
@ 2012-07-04  6:21 ` John Stultz
  2012-07-05 14:29   ` Prarit Bhargava
  2012-07-04  6:21 ` [PATCH 2/3] [RFC] time: Fix leapsecond triggered hrtimer/futex load spike issue John Stultz
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: John Stultz @ 2012-07-04  6:21 UTC (permalink / raw)
  To: Linux Kernel; +Cc: John Stultz, Prarit Bhargava, stable, Thomas Gleixner, linux

NOTE:This is a prerequisite patch that's required to
address the widely observed leap-second related futex/hrtimer
issues.

Currently clock_was_set() is unsafe to be called from irq
context, as it calls on_each_cpu(). This causes problems when
we need to adjust the time from update_wall_time().

To fix this, if clock_was_set is called when irqs are
disabled, we schedule a timer to fire for immedately after
we're out of interrupt context to then notify the hrtimer
subsystem.

CC: Prarit Bhargava <prarit@redhat.com>
CC: stable@vger.kernel.org
CC: Thomas Gleixner <tglx@linutronix.de>
CC: linux@openhuawei.org
Reported-by: Jan Engelhardt <jengelh@inai.de>
Signed-off-by: John Stultz <johnstul@us.ibm.com>
---
 kernel/hrtimer.c |   17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index ae34bf5..d730678 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -746,7 +746,7 @@ static inline void retrigger_next_event(void *arg) { }
  * resolution timer interrupts. On UP we just disable interrupts and
  * call the high resolution interrupt code.
  */
-void clock_was_set(void)
+static void do_clock_was_set(unsigned long data)
 {
 #ifdef CONFIG_HIGH_RES_TIMERS
 	/* Retrigger the CPU local events everywhere */
@@ -755,6 +755,21 @@ void clock_was_set(void)
 	timerfd_clock_was_set();
 }
 
+static DEFINE_TIMER(clock_was_set_timer, do_clock_was_set , 0, 0);
+
+void clock_was_set(void)
+{
+	/*
+	 * We can't call on_each_cpu() from irq context,
+	 * so if irqs are disabled , schedule the clock_was_set
+	 * via a timer_list timer for right after.
+	 */
+	if (irqs_disabled())
+		mod_timer(&clock_was_set_timer, jiffies);
+	else
+		do_clock_was_set(0);
+}
+
 /*
  * During resume we might have to reprogram the high resolution timer
  * interrupt (on the local CPU):
-- 
1.7.9.5


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

* [PATCH 2/3] [RFC] time: Fix leapsecond triggered hrtimer/futex load spike issue
  2012-07-04  6:21 [PATCH 0/3][RFC] Fix for leapsecond caused futex issue (v4) John Stultz
  2012-07-04  6:21 ` [PATCH 1/3] [RFC] hrtimer: Fix clock_was_set so it is safe to call from irq context John Stultz
@ 2012-07-04  6:21 ` John Stultz
  2012-07-05 14:29   ` Prarit Bhargava
  2012-07-04  6:21 ` [PATCH 3/3] [RFC] hrtimer: Update hrtimer base offsets each hrtimer_interrupt John Stultz
  2012-07-04  6:23 ` [PATCH 0/3][RFC] Fix for leapsecond caused futex issue (v4) John Stultz
  3 siblings, 1 reply; 11+ messages in thread
From: John Stultz @ 2012-07-04  6:21 UTC (permalink / raw)
  To: Linux Kernel; +Cc: John Stultz, Prarit Bhargava, stable, Thomas Gleixner, linux

As widely reported on the internet, some Linux systems after
the leapsecond was inserted are experiencing futex related load
spikes (usually connected to MySQL, Firefox, Thunderbird, Java, etc).

An apparent for this issue workaround is running:
$ date -s "`date`"

Credit: http://www.sheeri.com/content/mysql-and-leap-second-high-cpu-and-fix

I this issue is due to the leapsecond being added without
calling clock_was_set() to notify the hrtimer subsystem of the
change.

The workaround functions as it forces a clock_was_set()
call from settimeofday().

This fix adds the required clock_was_set() calls to where
we adjust for leapseconds.

NOTE: This fix *depends* on the previous fix, which allows
clock_was_set to be called from atomic context. Do not try
to apply just this patch.

CC: Prarit Bhargava <prarit@redhat.com>
CC: stable@vger.kernel.org
CC: Thomas Gleixner <tglx@linutronix.de>
CC: linux@openhuawei.org
Reported-by: Jan Engelhardt <jengelh@inai.de>
Signed-off-by: John Stultz <johnstul@us.ibm.com>
---
 kernel/time/timekeeping.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 6f46a00..cc2991d 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -963,6 +963,8 @@ static cycle_t logarithmic_accumulation(cycle_t offset, int shift)
 		leap = second_overflow(timekeeper.xtime.tv_sec);
 		timekeeper.xtime.tv_sec += leap;
 		timekeeper.wall_to_monotonic.tv_sec -= leap;
+		if (leap)
+			clock_was_set();
 	}
 
 	/* Accumulate raw time */
@@ -1079,6 +1081,8 @@ static void update_wall_time(void)
 		leap = second_overflow(timekeeper.xtime.tv_sec);
 		timekeeper.xtime.tv_sec += leap;
 		timekeeper.wall_to_monotonic.tv_sec -= leap;
+		if (leap)
+			clock_was_set();
 	}
 
 	timekeeping_update(false);
-- 
1.7.9.5


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

* [PATCH 3/3] [RFC] hrtimer: Update hrtimer base offsets each hrtimer_interrupt
  2012-07-04  6:21 [PATCH 0/3][RFC] Fix for leapsecond caused futex issue (v4) John Stultz
  2012-07-04  6:21 ` [PATCH 1/3] [RFC] hrtimer: Fix clock_was_set so it is safe to call from irq context John Stultz
  2012-07-04  6:21 ` [PATCH 2/3] [RFC] time: Fix leapsecond triggered hrtimer/futex load spike issue John Stultz
@ 2012-07-04  6:21 ` John Stultz
  2012-07-05 14:30   ` Prarit Bhargava
  2012-07-04  6:23 ` [PATCH 0/3][RFC] Fix for leapsecond caused futex issue (v4) John Stultz
  3 siblings, 1 reply; 11+ messages in thread
From: John Stultz @ 2012-07-04  6:21 UTC (permalink / raw)
  To: Linux Kernel; +Cc: John Stultz, Prarit Bhargava, stable, Thomas Gleixner, linux

This patch introduces a new funciton which captures the
CLOCK_MONOTONIC time, along with the CLOCK_REALTIME and
CLOCK_BOOTTIME offsets at the same moment. This new function
is then used in place of ktime_get() when hrtimer_interrupt()
is expiring timers.

This ensures that any changes to realtime or boottime offsets
are noticed and stored into the per-cpu hrtimer base structures,
prior to doing any hrtimer expiration. This should ensure that
timers are not expired early if the offsets changes under us.

This is useful in the case where clock_was_set() is called from
atomic context and have to schedule the hrtimer base offset update
via a timer, as it provides extra robustness in the face of any
possible timer delay.

CC: Prarit Bhargava <prarit@redhat.com>
CC: stable@vger.kernel.org
CC: Thomas Gleixner <tglx@linutronix.de>
CC: linux@openhuawei.org
Signed-off-by: John Stultz <johnstul@us.ibm.com>
---
 include/linux/hrtimer.h   |    3 +++
 kernel/hrtimer.c          |   14 +++++++++++---
 kernel/time/timekeeping.c |   34 ++++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index fd0dc30..f6b2a74 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -320,6 +320,9 @@ extern ktime_t ktime_get(void);
 extern ktime_t ktime_get_real(void);
 extern ktime_t ktime_get_boottime(void);
 extern ktime_t ktime_get_monotonic_offset(void);
+extern void ktime_get_and_real_and_sleep_offset(ktime_t *monotonic,
+						ktime_t *real_offset,
+						ktime_t *sleep_offset);
 
 DECLARE_PER_CPU(struct tick_device, tick_cpu_device);
 
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index d730678..56600c4 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1258,18 +1258,26 @@ static void __run_hrtimer(struct hrtimer *timer, ktime_t *now)
 void hrtimer_interrupt(struct clock_event_device *dev)
 {
 	struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
-	ktime_t expires_next, now, entry_time, delta;
+	ktime_t expires_next, now, entry_time, delta, real_offset, sleep_offset;
 	int i, retries = 0;
 
 	BUG_ON(!cpu_base->hres_active);
 	cpu_base->nr_events++;
 	dev->next_event.tv64 = KTIME_MAX;
 
-	entry_time = now = ktime_get();
+
+	ktime_get_and_real_and_sleep_offset(&now, &real_offset, &sleep_offset);
+
+	entry_time = now;
 retry:
 	expires_next.tv64 = KTIME_MAX;
 
 	raw_spin_lock(&cpu_base->lock);
+
+	/* Update base offsets, to avoid early wakeups */
+	cpu_base->clock_base[HRTIMER_BASE_REALTIME].offset = real_offset;
+	cpu_base->clock_base[HRTIMER_BASE_BOOTTIME].offset = sleep_offset;
+
 	/*
 	 * We set expires_next to KTIME_MAX here with cpu_base->lock
 	 * held to prevent that a timer is enqueued in our queue via
@@ -1346,7 +1354,7 @@ retry:
 	 * interrupt routine. We give it 3 attempts to avoid
 	 * overreacting on some spurious event.
 	 */
-	now = ktime_get();
+	ktime_get_and_real_and_sleep_offset(&now, &real_offset, &sleep_offset);
 	cpu_base->nr_retries++;
 	if (++retries < 3)
 		goto retry;
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index cc2991d..b3404cf 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1251,6 +1251,40 @@ void get_xtime_and_monotonic_and_sleep_offset(struct timespec *xtim,
 }
 
 /**
+ * ktime_get_and_real_and_sleep_offset() - hrtimer helper, gets monotonic ktime,
+ *	realtime offset, and sleep offsets.
+ */
+void ktime_get_and_real_and_sleep_offset(ktime_t *monotonic,
+						ktime_t *real_offset,
+						ktime_t *sleep_offset)
+{
+	unsigned long seq;
+	struct timespec wtom, sleep;
+	u64 secs, nsecs;
+
+	do {
+		seq = read_seqbegin(&timekeeper.lock);
+
+		secs = timekeeper.xtime.tv_sec +
+				timekeeper.wall_to_monotonic.tv_sec;
+		nsecs = timekeeper.xtime.tv_nsec +
+				timekeeper.wall_to_monotonic.tv_nsec;
+		nsecs += timekeeping_get_ns();
+		/* If arch requires, add in gettimeoffset() */
+		nsecs += arch_gettimeoffset();
+
+		wtom = timekeeper.wall_to_monotonic;
+		sleep = timekeeper.total_sleep_time;
+	} while (read_seqretry(&timekeeper.lock, seq));
+
+	*monotonic = ktime_add_ns(ktime_set(secs, 0), nsecs);
+	set_normalized_timespec(&wtom, -wtom.tv_sec, -wtom.tv_nsec);
+	*real_offset =	timespec_to_ktime(wtom);
+	*sleep_offset = timespec_to_ktime(sleep);
+}
+
+
+/**
  * ktime_get_monotonic_offset() - get wall_to_monotonic in ktime_t format
  */
 ktime_t ktime_get_monotonic_offset(void)
-- 
1.7.9.5


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

* Re: [PATCH 0/3][RFC] Fix for leapsecond caused futex issue (v4)
  2012-07-04  6:21 [PATCH 0/3][RFC] Fix for leapsecond caused futex issue (v4) John Stultz
                   ` (2 preceding siblings ...)
  2012-07-04  6:21 ` [PATCH 3/3] [RFC] hrtimer: Update hrtimer base offsets each hrtimer_interrupt John Stultz
@ 2012-07-04  6:23 ` John Stultz
  2012-07-05 14:41   ` Prarit Bhargava
  3 siblings, 1 reply; 11+ messages in thread
From: John Stultz @ 2012-07-04  6:23 UTC (permalink / raw)
  To: John Stultz; +Cc: Linux Kernel, Prarit Bhargava, stable, Thomas Gleixner, linux

[-- Attachment #1: Type: text/plain, Size: 598 bytes --]

On 07/03/2012 11:21 PM, John Stultz wrote:
> Ok, made a few tweaks to address issues caught by Prarit's and my
> testing. This has run for a number of hours now w/ my leap-a-day.c
> test on a few machines.
>
> I'd really appreciate any extra testing, review, or acks at this point.
> I'm targeting mid-late Thursday (to give folks in the US a chance to
> review & test) as a point when I'll submit this upstream if no other
> issues are found.

And again, here's my leap-a-day.c test case which can be used to trigger 
a leapsecond every day, or every ~13 seconds via the -s option.

thanks
-john


[-- Attachment #2: leap-a-day.c --]
[-- Type: text/x-csrc, Size: 5228 bytes --]

/* Leap second stress test
 *              by: john stultz (johnstul@us.ibm.com)
 *              (C) Copyright IBM 2012
 *              Licensed under the GPLv2
 *
 *  This test signals the kernel to insert a leap second
 *  every day at midnight GMT. This allows for stessing the
 *  kernel's leap-second behavior, as well as how well applications
 *  handle the leap-second discontinuity.
 *
 *  Usage: leap-a-day [-s]
 *
 *  Options:
 *	-s:	Each iteration, set the date to 10 seconds before midnight GMT.
 *		This speeds up the number of leapsecond transitions tested,
 *		but because it calls settimeofday frequently, advancing the
 *		time by 24 hours every ~16 seconds, it may cause application
 *		disruption.
 *
 *  Other notes: Disabling NTP prior to running this is advised, as the two
 *		 may conflict in thier commands to the kernel.
 *
 *  To build:
 *	$ gcc leap-a-day.c -o leap-a-day -lrt
 */



#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <sys/time.h>
#include <sys/timex.h>
#include <string.h>
#include <signal.h>

#define NSEC_PER_SEC 1000000000ULL

/* returns 1 if a <= b, 0 otherwise */
static inline int in_order(struct timespec a, struct timespec b)
{
        if(a.tv_sec < b.tv_sec)
                return 1;
        if(a.tv_sec > b.tv_sec)
                return 0;
        if(a.tv_nsec > b.tv_nsec)
                return 0;
        return 1;
}

struct timespec timespec_add(struct timespec ts, unsigned long long ns)
{
	ts.tv_nsec += ns;
	while(ts.tv_nsec >= NSEC_PER_SEC) {
		ts.tv_nsec -= NSEC_PER_SEC;
		ts.tv_sec++;
	}
	return ts;
}


char* time_state_str(int state)
{
	switch (state) {
		case TIME_OK:	return "TIME_OK";
		case TIME_INS:	return "TIME_INS";
		case TIME_DEL:	return "TIME_DEL";
		case TIME_OOP:	return "TIME_OOP";
		case TIME_WAIT:	return "TIME_WAIT";
		case TIME_BAD:	return "TIME_BAD";
	}
	return "ERROR"; 
}

/* clear NTP time_status & time_state */
void clear_time_state(void)
{
	struct timex tx;
	int ret;

	/*
	 * XXX - The fact we have to call this twice seems
	 * to point to a slight issue in the kernel's ntp state
	 * managment. Needs to be investigated further.
	 */

	tx.modes = ADJ_STATUS;
	tx.status = STA_PLL;
	ret = adjtimex(&tx);

	tx.modes = ADJ_STATUS;
	tx.status = 0;
	ret = adjtimex(&tx);
}

/* Make sure we cleanup on ctrl-c */
void handler(int unused)
{
	clear_time_state();
	exit(0);
}


/* Test for known hrtimer failure */
void test_hrtimer_failure(void)
{
	struct timespec now, target;

	clock_gettime(CLOCK_REALTIME, &now);
	target = timespec_add(now, NSEC_PER_SEC/2);
	clock_nanosleep(CLOCK_REALTIME, TIMER_ABSTIME, &target, NULL);
	clock_gettime(CLOCK_REALTIME, &now);

	if (!in_order(target, now)) {
		printf("Note: hrtimer early expiration failure observed.\n");
	}

}



int main(int argc, char** argv) 
{
	struct timeval tv;
	struct timex tx;
	struct timespec ts;
	int settime = 0;

	signal(SIGINT, handler);
	signal(SIGKILL, handler);
	printf("This runs continuously. Press ctrl-c to stop\n");


	/* Process arguments */
	if (argc > 1) {
		if (!strncmp(argv[1], "-s", 2)) {
			printf("Setting time to speed up testing\n");
			settime = 1;
		} else {
			printf("Usage: %s [-s]\n", argv[0]);
			printf("	-s: Set time to right before leap second each iteration\n");
		}
	}

	printf("\n");
	while (1) {
		int ret;
		time_t now, next_leap;
		/* Get the current time */
		gettimeofday(&tv, NULL);

		/* Calculate the next possible leap second 23:59:60 GMT */
		tv.tv_sec += 86400 - (tv.tv_sec % 86400);
		next_leap = ts.tv_sec = tv.tv_sec;

		if (settime) {
			tv.tv_sec -= 10;
			settimeofday(&tv, NULL);
			printf("Setting time to %s", ctime(&ts.tv_sec));
		}

		/* Reset NTP time state */
		clear_time_state();

		/* Set the leap second insert flag */
		tx.modes = ADJ_STATUS;
		tx.status = STA_INS;
		ret = adjtimex(&tx);
		if (ret) {
			printf("Error: Problem setting STA_INS!: %s\n",
							time_state_str(ret));
			return -1;
		}

		/* Validate STA_INS was set */
		ret = adjtimex(&tx);
		if (tx.status != STA_INS) {
			printf("Error: STA_INS not set!: %s\n",
							time_state_str(ret));
			return -1;
		}

		printf("Scheduling leap second for %s", ctime(&next_leap));
	
		/* Wake up 3 seconds before leap */
		ts.tv_sec -= 3;
		while(clock_nanosleep(CLOCK_REALTIME, TIMER_ABSTIME, &ts, NULL))
			printf("Something woke us up, returning to sleep\n");

		/* Validate STA_INS is still set */
		ret = adjtimex(&tx);
		if (tx.status != STA_INS) {
			printf("Something cleared STA_INS, setting it again.\n");
			tx.modes = ADJ_STATUS;
			tx.status = STA_INS;
			ret = adjtimex(&tx);
		}

		/* Check adjtimex output every half second */
		now = tx.time.tv_sec;
		while (now < next_leap+2) {
			char buf[26];
			ret = adjtimex(&tx);

			ctime_r(&tx.time.tv_sec, buf);
			buf[strlen(buf)-1] = 0; /*remove trailing\n */

			printf("%s + %6ld us\t%s\n",
					buf,
					tx.time.tv_usec, 
					time_state_str(ret));
			now = tx.time.tv_sec;
			/* Sleep for another half second */
			ts.tv_sec = 0;
			ts.tv_nsec = NSEC_PER_SEC/2;
			clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL);			
		}

		/* Note if kernel has known hrtimer failure */
		test_hrtimer_failure();

		printf("Leap complete\n\n");
	}

	clear_time_state();
	return 0;
}

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

* Re: [PATCH 1/3] [RFC] hrtimer: Fix clock_was_set so it is safe to call from irq context
  2012-07-04  6:21 ` [PATCH 1/3] [RFC] hrtimer: Fix clock_was_set so it is safe to call from irq context John Stultz
@ 2012-07-05 14:29   ` Prarit Bhargava
  0 siblings, 0 replies; 11+ messages in thread
From: Prarit Bhargava @ 2012-07-05 14:29 UTC (permalink / raw)
  To: John Stultz; +Cc: Linux Kernel, stable, Thomas Gleixner, linux



On 07/04/2012 02:21 AM, John Stultz wrote:
> NOTE:This is a prerequisite patch that's required to
> address the widely observed leap-second related futex/hrtimer
> issues.
> 
> Currently clock_was_set() is unsafe to be called from irq
> context, as it calls on_each_cpu(). This causes problems when
> we need to adjust the time from update_wall_time().
> 
> To fix this, if clock_was_set is called when irqs are
> disabled, we schedule a timer to fire for immedately after
> we're out of interrupt context to then notify the hrtimer
> subsystem.
> 
> CC: Prarit Bhargava <prarit@redhat.com>
> CC: stable@vger.kernel.org
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: linux@openhuawei.org
> Reported-by: Jan Engelhardt <jengelh@inai.de>
> Signed-off-by: John Stultz <johnstul@us.ibm.com>
> ---
>  kernel/hrtimer.c |   17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
> index ae34bf5..d730678 100644
> --- a/kernel/hrtimer.c
> +++ b/kernel/hrtimer.c
> @@ -746,7 +746,7 @@ static inline void retrigger_next_event(void *arg) { }
>   * resolution timer interrupts. On UP we just disable interrupts and
>   * call the high resolution interrupt code.
>   */
> -void clock_was_set(void)
> +static void do_clock_was_set(unsigned long data)
>  {
>  #ifdef CONFIG_HIGH_RES_TIMERS
>  	/* Retrigger the CPU local events everywhere */
> @@ -755,6 +755,21 @@ void clock_was_set(void)
>  	timerfd_clock_was_set();
>  }
>  
> +static DEFINE_TIMER(clock_was_set_timer, do_clock_was_set , 0, 0);
> +
> +void clock_was_set(void)
> +{
> +	/*
> +	 * We can't call on_each_cpu() from irq context,
> +	 * so if irqs are disabled , schedule the clock_was_set
> +	 * via a timer_list timer for right after.
> +	 */
> +	if (irqs_disabled())

I was wondering about this in earlier versions of the patchset.  I thought it
was supposed to be irqs_disabled.  Thanks for changing this.

Acked-by: Prarit Bhargava <prarit@redhat.com>

P.

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

* Re: [PATCH 2/3] [RFC] time: Fix leapsecond triggered hrtimer/futex load spike issue
  2012-07-04  6:21 ` [PATCH 2/3] [RFC] time: Fix leapsecond triggered hrtimer/futex load spike issue John Stultz
@ 2012-07-05 14:29   ` Prarit Bhargava
  0 siblings, 0 replies; 11+ messages in thread
From: Prarit Bhargava @ 2012-07-05 14:29 UTC (permalink / raw)
  To: John Stultz; +Cc: Linux Kernel, stable, Thomas Gleixner, linux



On 07/04/2012 02:21 AM, John Stultz wrote:
> As widely reported on the internet, some Linux systems after
> the leapsecond was inserted are experiencing futex related load
> spikes (usually connected to MySQL, Firefox, Thunderbird, Java, etc).
> 
> An apparent for this issue workaround is running:
> $ date -s "`date`"
> 
> Credit: http://www.sheeri.com/content/mysql-and-leap-second-high-cpu-and-fix
> 
> I this issue is due to the leapsecond being added without
> calling clock_was_set() to notify the hrtimer subsystem of the
> change.
> 
> The workaround functions as it forces a clock_was_set()
> call from settimeofday().
> 
> This fix adds the required clock_was_set() calls to where
> we adjust for leapseconds.
> 
> NOTE: This fix *depends* on the previous fix, which allows
> clock_was_set to be called from atomic context. Do not try
> to apply just this patch.
> 
> CC: Prarit Bhargava <prarit@redhat.com>
> CC: stable@vger.kernel.org
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: linux@openhuawei.org
> Reported-by: Jan Engelhardt <jengelh@inai.de>
> Signed-off-by: John Stultz <johnstul@us.ibm.com>


Acked-by: Prarit Bhargava <prarit@redhat.com>

P.

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

* Re: [PATCH 3/3] [RFC] hrtimer: Update hrtimer base offsets each hrtimer_interrupt
  2012-07-04  6:21 ` [PATCH 3/3] [RFC] hrtimer: Update hrtimer base offsets each hrtimer_interrupt John Stultz
@ 2012-07-05 14:30   ` Prarit Bhargava
  0 siblings, 0 replies; 11+ messages in thread
From: Prarit Bhargava @ 2012-07-05 14:30 UTC (permalink / raw)
  To: John Stultz; +Cc: Linux Kernel, stable, Thomas Gleixner, linux



On 07/04/2012 02:21 AM, John Stultz wrote:
> This patch introduces a new funciton which captures the
> CLOCK_MONOTONIC time, along with the CLOCK_REALTIME and
> CLOCK_BOOTTIME offsets at the same moment. This new function
> is then used in place of ktime_get() when hrtimer_interrupt()
> is expiring timers.
> 
> This ensures that any changes to realtime or boottime offsets
> are noticed and stored into the per-cpu hrtimer base structures,
> prior to doing any hrtimer expiration. This should ensure that
> timers are not expired early if the offsets changes under us.
> 
> This is useful in the case where clock_was_set() is called from
> atomic context and have to schedule the hrtimer base offset update
> via a timer, as it provides extra robustness in the face of any
> possible timer delay.
> 
> CC: Prarit Bhargava <prarit@redhat.com>
> CC: stable@vger.kernel.org
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: linux@openhuawei.org
> Signed-off-by: John Stultz <johnstul@us.ibm.com>

Acked-by: Prarit Bhargava <prarit@redhat.com>

P.

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

* Re: [PATCH 0/3][RFC] Fix for leapsecond caused futex issue (v4)
  2012-07-04  6:23 ` [PATCH 0/3][RFC] Fix for leapsecond caused futex issue (v4) John Stultz
@ 2012-07-05 14:41   ` Prarit Bhargava
  2012-07-05 16:27     ` John Stultz
  0 siblings, 1 reply; 11+ messages in thread
From: Prarit Bhargava @ 2012-07-05 14:41 UTC (permalink / raw)
  To: John Stultz; +Cc: Linux Kernel, stable, Thomas Gleixner, linux



On 07/04/2012 02:23 AM, John Stultz wrote:
> On 07/03/2012 11:21 PM, John Stultz wrote:
>> Ok, made a few tweaks to address issues caught by Prarit's and my
>> testing. This has run for a number of hours now w/ my leap-a-day.c
>> test on a few machines.
>>
>> I'd really appreciate any extra testing, review, or acks at this point.
>> I'm targeting mid-late Thursday (to give folks in the US a chance to
>> review & test) as a point when I'll submit this upstream if no other
>> issues are found.
> 
> And again, here's my leap-a-day.c test case which can be used to trigger a
> leapsecond every day, or every ~13 seconds via the -s option.
> 

I tested this patchset for ~22 hours without any issues using my own leap second
test code, John's previous leaptest.c and this new leap-a-day test with and
without the -s option.

In all x86 systems' cases (both AMD and Intel, large cpu count[1] and small cpu
count) I also ran Firefox in the background and monitored it using top and
occasionally breaking in to check the status of the futexes.  I haven't seen
anything wrong with any of the systems.

John, my apologies for not getting to exactly 24 hours.  I've usurped these
systems long enough and other people need them :/.  I'm going to continue to run
two systems (one AMD and one Intel) for a longer stretch but I don't anticipate
any issues at this point.

So ...

Acked-by: Prarit Bhargava <prarit@redhat.com>

For the entire patchset.

P.

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

* Re: [PATCH 0/3][RFC] Fix for leapsecond caused futex issue (v4)
  2012-07-05 14:41   ` Prarit Bhargava
@ 2012-07-05 16:27     ` John Stultz
  0 siblings, 0 replies; 11+ messages in thread
From: John Stultz @ 2012-07-05 16:27 UTC (permalink / raw)
  To: Prarit Bhargava; +Cc: Linux Kernel, stable, Thomas Gleixner, linux

On 07/05/2012 07:41 AM, Prarit Bhargava wrote:
> I tested this patchset for ~22 hours without any issues using my own leap second
> test code, John's previous leaptest.c and this new leap-a-day test with and
> without the -s option.
>
> In all x86 systems' cases (both AMD and Intel, large cpu count[1] and small cpu
> count) I also ran Firefox in the background and monitored it using top and
> occasionally breaking in to check the status of the futexes.  I haven't seen
> anything wrong with any of the systems.
>
> John, my apologies for not getting to exactly 24 hours.  I've usurped these
> systems long enough and other people need them :/.  I'm going to continue to run
> two systems (one AMD and one Intel) for a longer stretch but I don't anticipate
> any issues at this point.
>
> So ...
>
> Acked-by: Prarit Bhargava <prarit@redhat.com>
>
> For the entire patchset.
Thanks so much! I greatly appreciate the extra testing and review you've 
provided!

-john



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

* [PATCH 3/3] [RFC] hrtimer: Update hrtimer base offsets each hrtimer_interrupt
  2012-07-03  2:16 [PATCH 0/3][RFC] Potential fix for leapsecond caused futex issue (v3) John Stultz
@ 2012-07-03  2:16 ` John Stultz
  0 siblings, 0 replies; 11+ messages in thread
From: John Stultz @ 2012-07-03  2:16 UTC (permalink / raw)
  To: Linux Kernel; +Cc: John Stultz, Prarit Bhargava, stable, Thomas Gleixner

This patch introduces a new funciton which captures the
CLOCK_MONOTONIC time, along with the CLOCK_REALTIME and
CLOCK_BOOTTIME offsets at the same moment. This new function
is then used in place of ktime_get() when hrtimer_interrupt()
is expiring timers.

This ensures that any changes to realtime or boottime offsets
are noticed and stored into the per-cpu hrtimer base structures,
prior to doing any hrtimer expiration. This should ensure that
timers are not expired early if the offsets changes under us.

This is useful in the case where clock_was_set() is called from
atomic context and have to schedule the hrtimer base offset update
via a timer, as it provides extra robustness in the face of any
possible timer delay.

CC: Prarit Bhargava <prarit@redhat.com>
CC: stable@vger.kernel.org
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <johnstul@us.ibm.com>
---
 include/linux/hrtimer.h   |    3 +++
 kernel/hrtimer.c          |   14 +++++++++++---
 kernel/time/timekeeping.c |   35 +++++++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index fd0dc30..f6b2a74 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -320,6 +320,9 @@ extern ktime_t ktime_get(void);
 extern ktime_t ktime_get_real(void);
 extern ktime_t ktime_get_boottime(void);
 extern ktime_t ktime_get_monotonic_offset(void);
+extern void ktime_get_and_real_and_sleep_offset(ktime_t *monotonic,
+						ktime_t *real_offset,
+						ktime_t *sleep_offset);
 
 DECLARE_PER_CPU(struct tick_device, tick_cpu_device);
 
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 393fd4d..0e78b3e 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1260,18 +1260,26 @@ static void __run_hrtimer(struct hrtimer *timer, ktime_t *now)
 void hrtimer_interrupt(struct clock_event_device *dev)
 {
 	struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
-	ktime_t expires_next, now, entry_time, delta;
+	ktime_t expires_next, now, entry_time, delta, real_offset, sleep_offset;
 	int i, retries = 0;
 
 	BUG_ON(!cpu_base->hres_active);
 	cpu_base->nr_events++;
 	dev->next_event.tv64 = KTIME_MAX;
 
-	entry_time = now = ktime_get();
+
+	ktime_get_and_real_and_sleep_offset(&now, &real_offset, &sleep_offset);
+
+	entry_time = now;
 retry:
 	expires_next.tv64 = KTIME_MAX;
 
 	raw_spin_lock(&cpu_base->lock);
+
+	/* Update base offsets, to avoid early wakeups */
+	cpu_base->clock_base[HRTIMER_BASE_REALTIME].offset = real_offset;
+	cpu_base->clock_base[HRTIMER_BASE_BOOTTIME].offset = sleep_offset;
+
 	/*
 	 * We set expires_next to KTIME_MAX here with cpu_base->lock
 	 * held to prevent that a timer is enqueued in our queue via
@@ -1348,7 +1356,7 @@ retry:
 	 * interrupt routine. We give it 3 attempts to avoid
 	 * overreacting on some spurious event.
 	 */
-	now = ktime_get();
+	ktime_get_and_real_and_sleep_offset(&now, &real_offset, &sleep_offset);
 	cpu_base->nr_retries++;
 	if (++retries < 3)
 		goto retry;
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index cc2991d..c2ba132 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1250,6 +1250,41 @@ void get_xtime_and_monotonic_and_sleep_offset(struct timespec *xtim,
 	} while (read_seqretry(&timekeeper.lock, seq));
 }
 
+
+void ktime_get_and_real_and_sleep_offset(ktime_t *monotonic,
+						ktime_t *real_offset,
+						ktime_t *sleep_offset)
+{
+	unsigned long seq;
+	struct timespec wtom, sleep;
+	u64 secs, nsecs;
+
+	do {
+		seq = read_seqbegin(&timekeeper.lock);
+
+		secs = timekeeper.xtime.tv_sec +
+				timekeeper.wall_to_monotonic.tv_sec;
+		nsecs = timekeeper.xtime.tv_nsec +
+				timekeeper.wall_to_monotonic.tv_nsec;
+		nsecs += timekeeping_get_ns();
+		/* If arch requires, add in gettimeoffset() */
+		nsecs += arch_gettimeoffset();
+
+		wtom = timekeeper.wall_to_monotonic;
+		sleep = timekeeper.total_sleep_time;
+	} while (read_seqretry(&timekeeper.lock, seq));
+
+
+	*monotonic = ktime_add_ns(ktime_set(secs, 0), nsecs);
+	set_normalized_timespec(&wtom, -wtom.tv_sec, -wtom.tv_nsec);
+	*real_offset =	timespec_to_ktime(wtom);
+	*sleep_offset = timespec_to_ktime(sleep);
+}
+
+
+
+
+
 /**
  * ktime_get_monotonic_offset() - get wall_to_monotonic in ktime_t format
  */
-- 
1.7.9.5


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

end of thread, other threads:[~2012-07-05 16:40 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-04  6:21 [PATCH 0/3][RFC] Fix for leapsecond caused futex issue (v4) John Stultz
2012-07-04  6:21 ` [PATCH 1/3] [RFC] hrtimer: Fix clock_was_set so it is safe to call from irq context John Stultz
2012-07-05 14:29   ` Prarit Bhargava
2012-07-04  6:21 ` [PATCH 2/3] [RFC] time: Fix leapsecond triggered hrtimer/futex load spike issue John Stultz
2012-07-05 14:29   ` Prarit Bhargava
2012-07-04  6:21 ` [PATCH 3/3] [RFC] hrtimer: Update hrtimer base offsets each hrtimer_interrupt John Stultz
2012-07-05 14:30   ` Prarit Bhargava
2012-07-04  6:23 ` [PATCH 0/3][RFC] Fix for leapsecond caused futex issue (v4) John Stultz
2012-07-05 14:41   ` Prarit Bhargava
2012-07-05 16:27     ` John Stultz
  -- strict thread matches above, loose matches on Subject: below --
2012-07-03  2:16 [PATCH 0/3][RFC] Potential fix for leapsecond caused futex issue (v3) John Stultz
2012-07-03  2:16 ` [PATCH 3/3] [RFC] hrtimer: Update hrtimer base offsets each hrtimer_interrupt 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.