[PATCH] Fix leap second message hang

[PATCH] Fix leap second message hang

[Chris Adams]

Resending this outside the big long thread to make sure it gets added
(it isn't critical right now but I don't want to forget it).

From: Chris Adams <cmadams@hiwaay.net>

The code to handle leap seconds printks an information message when the
second is inserted or deleted.  It does this while holding xtime_lock.
However, printk wakes up klogd, and in some cases, the scheduler tries
to get the current kernel time, trying to get xtime_lock (which results
in a deadlock).  This moved the printks outside of the lock.  It also
adds a comment to not use printk while holding xtime_lock.

Signed-off-by: Chris Adams <cmadams@hiwaay.net>
Acked-by: Linas Vepstas <linasvepstas@gmail.com>
---
diff -urpN linux-2.6.28-git5-vanilla/include/linux/time.h linux-2.6.28-git5/include/linux/time.h
--- linux-2.6.28-git5-vanilla/include/linux/time.h	2009-01-02 22:09:10.000000000 -0600
+++ linux-2.6.28-git5/include/linux/time.h	2009-01-03 11:57:27.000000000 -0600
@@ -99,6 +99,12 @@ static inline struct timespec timespec_s
 
 extern struct timespec xtime;
 extern struct timespec wall_to_monotonic;
+
+/*
+ * Do not call printk while holding this lock; it wakes klogd and the
+ * scheduler may try to get the current kernel time, which will try to get
+ * this lock.
+ */
 extern seqlock_t xtime_lock;
 
 extern unsigned long read_persistent_clock(void);
diff -urpN linux-2.6.28-git5-vanilla/kernel/time/ntp.c linux-2.6.28-git5/kernel/time/ntp.c
--- linux-2.6.28-git5-vanilla/kernel/time/ntp.c	2009-01-02 22:09:34.000000000 -0600
+++ linux-2.6.28-git5/kernel/time/ntp.c	2009-01-03 11:57:46.000000000 -0600
@@ -130,6 +130,7 @@ void ntp_clear(void)
 static enum hrtimer_restart ntp_leap_second(struct hrtimer *timer)
 {
 	enum hrtimer_restart res = HRTIMER_NORESTART;
+	const char *msg = NULL;
 
 	write_seqlock(&xtime_lock);
 
@@ -140,8 +141,7 @@ static enum hrtimer_restart ntp_leap_sec
 		xtime.tv_sec--;
 		wall_to_monotonic.tv_sec++;
 		time_state = TIME_OOP;
-		printk(KERN_NOTICE "Clock: "
-		       "inserting leap second 23:59:60 UTC\n");
+		msg = "Clock: inserting leap second 23:59:60 UTC";
 		hrtimer_add_expires_ns(&leap_timer, NSEC_PER_SEC);
 		res = HRTIMER_RESTART;
 		break;
@@ -150,8 +150,7 @@ static enum hrtimer_restart ntp_leap_sec
 		time_tai--;
 		wall_to_monotonic.tv_sec--;
 		time_state = TIME_WAIT;
-		printk(KERN_NOTICE "Clock: "
-		       "deleting leap second 23:59:59 UTC\n");
+		msg = "Clock: deleting leap second 23:59:59 UTC";
 		break;
 	case TIME_OOP:
 		time_tai++;
@@ -166,6 +165,9 @@ static enum hrtimer_restart ntp_leap_sec
 
 	write_sequnlock(&xtime_lock);
 
+	if (msg)
+		printk(KERN_NOTICE "%s\n", msg);
+
 	return res;
 }
 

Re: [PATCH] Fix leap second message hang

[Peter Zijlstra]

On Fri, 2009-01-09 at 14:31 -0600, Chris Adams wrote:
> Resending this outside the big long thread to make sure it gets added
> (it isn't critical right now but I don't want to forget it).

Best way to get stuff added is to CC the right people -- loose patches
get lost easily.

> The code to handle leap seconds printks an information message when the
> second is inserted or deleted.  It does this while holding xtime_lock.
> However, printk wakes up klogd, and in some cases, the scheduler tries
> to get the current kernel time, trying to get xtime_lock (which results
> in a deadlock).  This moved the printks outside of the lock.  It also
> adds a comment to not use printk while holding xtime_lock.

That should actually work these days, we don't actually do a direct wake
up anymore.

printk should work from all places, including holding rq->lock and
xtime_lock.