hrtimer: fix recursion deadlock by re-introducing the softirq

Impact: fix rare runtime deadlock There are a few sites that do: spin_lock_irq(&foo) hrtimer_start(&bar) __run_hrtimer(&bar) func() spin_lock(&foo) which obviously deadlocks. In order to avoid this, never call __run_hrtimer() from hrtimer_start*() context, but instead defer this to softirq context. Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl> Signed-off-by: Ingo Molnar <mingo@elte.hu>

hrtimer: fix recursion deadlock by re-introducing the softirq
Impact: fix rare runtime deadlock There are a few sites that do: spin_lock_irq(&foo) hrtimer_start(&bar) __run_hrtimer(&bar) func() spin_lock(&foo) which obviously deadlocks. In order to avoid this, never call __run_hrtimer() from hrtimer_start*() context, but instead defer this to softirq context. Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl> Signed-off-by: Ingo Molnar <mingo@elte.hu>
Peter Zijlstra · Ingo Molnar
1 parent 731a55ba0f
Showing 2 changed files with 29 additions and 34 deletions Side-by-side Diff
include/linux/interrupt.h
kernel/hrtimer.c
@@ -253,7 +253,8 @@
 	BLOCK_SOFTIRQ,
 	TASKLET_SOFTIRQ,
 	SCHED_SOFTIRQ,
-	RCU_SOFTIRQ, 	/* Preferable RCU should always be the last softirq */
+	HRTIMER_SOFTIRQ,
+	RCU_SOFTIRQ,	/* Preferable RCU should always be the last softirq */
  
 	NR_SOFTIRQS
 };
@@ -634,7 +634,6 @@
 {
 }
  
-static void __run_hrtimer(struct hrtimer *timer);
  
 /*
  * When High resolution timers are active, try to reprogram. Note, that in case
@@ -646,13 +645,9 @@
 					    struct hrtimer_clock_base *base)
 {
 	if (base->cpu_base->hres_active && hrtimer_reprogram(timer, base)) {
-		/*
-		 * XXX: recursion check?
-		 * hrtimer_forward() should round up with timer granularity
-		 * so that we never get into inf recursion here,
-		 * it doesn't do that though
-		 */
-		__run_hrtimer(timer);
+		spin_unlock(&base->cpu_base->lock);
+		raise_softirq_irqoff(HRTIMER_SOFTIRQ);
+		spin_lock(&base->cpu_base->lock);
 		return 1;
 	}
 	return 0;
@@ -705,11 +700,6 @@
 }
 static inline void hrtimer_init_hres(struct hrtimer_cpu_base *base) { }
 static inline void hrtimer_init_timer_hres(struct hrtimer *timer) { }
-static inline int hrtimer_reprogram(struct hrtimer *timer,
-				    struct hrtimer_clock_base *base)
-{
-	return 0;
-}
  
 #endif /* CONFIG_HIGH_RES_TIMERS */
  
  
@@ -780,9 +770,11 @@
  *
  * The timer is inserted in expiry order. Insertion into the
  * red black tree is O(log(n)). Must hold the base lock.
+ *
+ * Returns 1 when the new timer is the leftmost timer in the tree.
  */
-static void enqueue_hrtimer(struct hrtimer *timer,
-			    struct hrtimer_clock_base *base, int reprogram)
+static int enqueue_hrtimer(struct hrtimer *timer,
+			   struct hrtimer_clock_base *base)
 {
 	struct rb_node **link = &base->active.rb_node;
 	struct rb_node *parent = NULL;
  
@@ -814,20 +806,8 @@
 	 * Insert the timer to the rbtree and check whether it
 	 * replaces the first pending timer
 	 */
-	if (leftmost) {
-		/*
-		 * Reprogram the clock event device. When the timer is already
-		 * expired hrtimer_enqueue_reprogram has either called the
-		 * callback or added it to the pending list and raised the
-		 * softirq.
-		 *
-		 * This is a NOP for !HIGHRES
-		 */
-		if (reprogram && hrtimer_enqueue_reprogram(timer, base))
-			return;
-
+	if (leftmost)
 		base->first = &timer->node;
-	}
  
 	rb_link_node(&timer->node, parent, link);
 	rb_insert_color(&timer->node, &base->active);
@@ -836,6 +816,8 @@
 	 * state of a possibly running callback.
 	 */
 	timer->state |= HRTIMER_STATE_ENQUEUED;
+
+	return leftmost;
 }
  
 /*
@@ -912,7 +894,7 @@
 {
 	struct hrtimer_clock_base *base, *new_base;
 	unsigned long flags;
-	int ret;
+	int ret, leftmost;
  
 	base = lock_hrtimer_base(timer, &flags);
  
  
  
@@ -940,12 +922,16 @@
  
 	timer_stats_hrtimer_set_start_info(timer);
  
+	leftmost = enqueue_hrtimer(timer, new_base);
+
 	/*
 	 * Only allow reprogramming if the new base is on this CPU.
 	 * (it might still be on another CPU if the timer was pending)
+	 *
+	 * XXX send_remote_softirq() ?
 	 */
-	enqueue_hrtimer(timer, new_base,
-			new_base->cpu_base == &__get_cpu_var(hrtimer_bases));
+	if (leftmost && new_base->cpu_base == &__get_cpu_var(hrtimer_bases))
+		hrtimer_enqueue_reprogram(timer, new_base);
  
 	unlock_hrtimer_base(timer, &flags);
  
@@ -1163,7 +1149,7 @@
 	 */
 	if (restart != HRTIMER_NORESTART) {
 		BUG_ON(timer->state != HRTIMER_STATE_CALLBACK);
-		enqueue_hrtimer(timer, base, 0);
+		enqueue_hrtimer(timer, base);
 	}
 	timer->state &= ~HRTIMER_STATE_CALLBACK;
 }
@@ -1277,6 +1263,11 @@
 	local_irq_restore(flags);
 }
  
+static void run_hrtimer_softirq(struct softirq_action *h)
+{
+	hrtimer_peek_ahead_timers();
+}
+
 #endif	/* CONFIG_HIGH_RES_TIMERS */
  
 /*
@@ -1532,7 +1523,7 @@
 		 * is done, which will run all expired timers and re-programm
 		 * the timer device.
 		 */
-		enqueue_hrtimer(timer, new_base, 0);
+		enqueue_hrtimer(timer, new_base);
  
 		/* Clear the migration state bit */
 		timer->state &= ~HRTIMER_STATE_MIGRATE;
@@ -1610,6 +1601,9 @@
 	hrtimer_cpu_notify(&hrtimers_nb, (unsigned long)CPU_UP_PREPARE,
 			  (void *)(long)smp_processor_id());
 	register_cpu_notifier(&hrtimers_nb);
+#ifdef CONFIG_HIGH_RES_TIMERS
+	open_softirq(HRTIMER_SOFTIRQ, run_hrtimer_softirq);
+#endif
 }
  
 /**
...	...	@@ -253,7 +253,8 @@
253	253	BLOCK_SOFTIRQ,
254	254	TASKLET_SOFTIRQ,
255	255	SCHED_SOFTIRQ,
256		- RCU_SOFTIRQ, /* Preferable RCU should always be the last softirq */
	256	+ HRTIMER_SOFTIRQ,
	257	+ RCU_SOFTIRQ, /* Preferable RCU should always be the last softirq */
257	258
258	259	NR_SOFTIRQS
259	260	};
...	...	@@ -634,7 +634,6 @@
634	634	{
635	635	}
636	636
637		-static void __run_hrtimer(struct hrtimer *timer);
638	637
639	638	/*
640	639	* When High resolution timers are active, try to reprogram. Note, that in case
...	...	@@ -646,13 +645,9 @@
646	645	struct hrtimer_clock_base *base)
647	646	{
648	647	if (base->cpu_base->hres_active && hrtimer_reprogram(timer, base)) {
649		- /*
650		- * XXX: recursion check?
651		- * hrtimer_forward() should round up with timer granularity
652		- * so that we never get into inf recursion here,
653		- * it doesn't do that though
654		- */
655		- __run_hrtimer(timer);
	648	+ spin_unlock(&base->cpu_base->lock);
	649	+ raise_softirq_irqoff(HRTIMER_SOFTIRQ);
	650	+ spin_lock(&base->cpu_base->lock);
656	651	return 1;
657	652	}
658	653	return 0;
...	...	@@ -705,11 +700,6 @@
705	700	}
706	701	static inline void hrtimer_init_hres(struct hrtimer_cpu_base *base) { }
707	702	static inline void hrtimer_init_timer_hres(struct hrtimer *timer) { }
708		-static inline int hrtimer_reprogram(struct hrtimer *timer,
709		- struct hrtimer_clock_base *base)
710		-{
711		- return 0;
712		-}
713	703
714	704	#endif /* CONFIG_HIGH_RES_TIMERS */
715	705
716	706
...	...	@@ -780,9 +770,11 @@
780	770	*
781	771	* The timer is inserted in expiry order. Insertion into the
782	772	* red black tree is O(log(n)). Must hold the base lock.
	773	+ *
	774	+ * Returns 1 when the new timer is the leftmost timer in the tree.
783	775	*/
784		-static void enqueue_hrtimer(struct hrtimer *timer,
785		- struct hrtimer_clock_base *base, int reprogram)
	776	+static int enqueue_hrtimer(struct hrtimer *timer,
	777	+ struct hrtimer_clock_base *base)
786	778	{
787	779	struct rb_node **link = &base->active.rb_node;
788	780	struct rb_node *parent = NULL;
789	781
...	...	@@ -814,20 +806,8 @@
814	806	* Insert the timer to the rbtree and check whether it
815	807	* replaces the first pending timer
816	808	*/
817		- if (leftmost) {
818		- /*
819		- * Reprogram the clock event device. When the timer is already
820		- * expired hrtimer_enqueue_reprogram has either called the
821		- * callback or added it to the pending list and raised the
822		- * softirq.
823		- *
824		- * This is a NOP for !HIGHRES
825		- */
826		- if (reprogram && hrtimer_enqueue_reprogram(timer, base))
827		- return;
828		-
	809	+ if (leftmost)
829	810	base->first = &timer->node;
830		- }
831	811
832	812	rb_link_node(&timer->node, parent, link);
833	813	rb_insert_color(&timer->node, &base->active);
...	...	@@ -836,6 +816,8 @@
836	816	* state of a possibly running callback.
837	817	*/
838	818	timer->state \|= HRTIMER_STATE_ENQUEUED;
	819	+
	820	+ return leftmost;
839	821	}
840	822
841	823	/*
...	...	@@ -912,7 +894,7 @@
912	894	{
913	895	struct hrtimer_clock_base base, new_base;
914	896	unsigned long flags;
915		- int ret;
	897	+ int ret, leftmost;
916	898
917	899	base = lock_hrtimer_base(timer, &flags);
918	900
919	901
920	902
...	...	@@ -940,12 +922,16 @@
940	922
941	923	timer_stats_hrtimer_set_start_info(timer);
942	924
	925	+ leftmost = enqueue_hrtimer(timer, new_base);
	926	+
943	927	/*
944	928	* Only allow reprogramming if the new base is on this CPU.
945	929	* (it might still be on another CPU if the timer was pending)
	930	+ *
	931	+ * XXX send_remote_softirq() ?
946	932	*/
947		- enqueue_hrtimer(timer, new_base,
948		- new_base->cpu_base == &__get_cpu_var(hrtimer_bases));
	933	+ if (leftmost && new_base->cpu_base == &__get_cpu_var(hrtimer_bases))
	934	+ hrtimer_enqueue_reprogram(timer, new_base);
949	935
950	936	unlock_hrtimer_base(timer, &flags);
951	937
...	...	@@ -1163,7 +1149,7 @@
1163	1149	*/
1164	1150	if (restart != HRTIMER_NORESTART) {
1165	1151	BUG_ON(timer->state != HRTIMER_STATE_CALLBACK);
1166		- enqueue_hrtimer(timer, base, 0);
	1152	+ enqueue_hrtimer(timer, base);
1167	1153	}
1168	1154	timer->state &= ~HRTIMER_STATE_CALLBACK;
1169	1155	}
...	...	@@ -1277,6 +1263,11 @@
1277	1263	local_irq_restore(flags);
1278	1264	}
1279	1265
	1266	+static void run_hrtimer_softirq(struct softirq_action *h)
	1267	+{
	1268	+ hrtimer_peek_ahead_timers();
	1269	+}
	1270	+
1280	1271	#endif /* CONFIG_HIGH_RES_TIMERS */
1281	1272
1282	1273	/*
...	...	@@ -1532,7 +1523,7 @@
1532	1523	* is done, which will run all expired timers and re-programm
1533	1524	* the timer device.
1534	1525	*/
1535		- enqueue_hrtimer(timer, new_base, 0);
	1526	+ enqueue_hrtimer(timer, new_base);
1536	1527
1537	1528	/* Clear the migration state bit */
1538	1529	timer->state &= ~HRTIMER_STATE_MIGRATE;
...	...	@@ -1610,6 +1601,9 @@
1610	1601	hrtimer_cpu_notify(&hrtimers_nb, (unsigned long)CPU_UP_PREPARE,
1611	1602	(void *)(long)smp_processor_id());
1612	1603	register_cpu_notifier(&hrtimers_nb);
	1604	+#ifdef CONFIG_HIGH_RES_TIMERS
	1605	+ open_softirq(HRTIMER_SOFTIRQ, run_hrtimer_softirq);
	1606	+#endif
1613	1607	}
1614	1608
1615	1609	/**