hrtimer: removing all ur callback modes, fix hotplug

Impact: fix hrtimer locking (reported by lockdep) in the CPU hotplug case This addition fixes the hotplug locking issue on my machine Signed-off-by: Peter Zijlstra <peterz@infradead.org> Signed-off-by: Ingo Molnar <mingo@elte.hu>

hrtimer: removing all ur callback modes, fix hotplug
Impact: fix hrtimer locking (reported by lockdep) in the CPU hotplug case This addition fixes the hotplug locking issue on my machine Signed-off-by: Peter Zijlstra <peterz@infradead.org> Signed-off-by: Ingo Molnar <mingo@elte.hu>
Peter Zijlstra · Ingo Molnar
1 parent ca109491f6
Showing 1 changed file with 37 additions and 28 deletions Side-by-side Diff
kernel/hrtimer.c
@@ -1496,7 +1496,7 @@
 #ifdef CONFIG_HOTPLUG_CPU
  
 static void migrate_hrtimer_list(struct hrtimer_clock_base *old_base,
-				 struct hrtimer_clock_base *new_base, int dcpu)
+				struct hrtimer_clock_base *new_base)
 {
 	struct hrtimer *timer;
 	struct rb_node *node;
  
  
  
  
  
  
@@ -1514,40 +1514,34 @@
 		__remove_hrtimer(timer, old_base, HRTIMER_STATE_MIGRATE, 0);
 		timer->base = new_base;
 		/*
-		 * Enqueue the timer. Allow reprogramming of the event device
+		 * Enqueue the timers on the new cpu, but do not reprogram 
+		 * the timer as that would enable a deadlock between
+		 * hrtimer_enqueue_reprogramm() running the timer and us still
+		 * holding a nested base lock.
+		 *
+		 * Instead we tickle the hrtimer interrupt after the migration
+		 * is done, which will run all expired timers and re-programm
+		 * the timer device.
 		 */
-		enqueue_hrtimer(timer, new_base, 1);
+		enqueue_hrtimer(timer, new_base, 0);
  
-#ifdef CONFIG_HIGH_RES_TIMERS
-		/*
-		 * Happens with high res enabled when the timer was
-		 * already expired and the callback mode is
-		 * HRTIMER_CB_IRQSAFE_UNLOCKED (hrtimer_sleeper). The
-		 * enqueue code does not move them to the soft irq
-		 * pending list for performance/latency reasons, but
-		 * in the migration state, we need to do that
-		 * otherwise we end up with a stale timer.
-		 */
-		if (timer->state == HRTIMER_STATE_MIGRATE) {
-			/* XXX: running on offline cpu */
-			__run_hrtimer(timer);
-		}
-#endif
 		/* Clear the migration state bit */
 		timer->state &= ~HRTIMER_STATE_MIGRATE;
 	}
 }
  
-static void migrate_hrtimers(int cpu)
+static int migrate_hrtimers(int scpu)
 {
 	struct hrtimer_cpu_base *old_base, *new_base;
-	int i;
+	int dcpu, i;
  
-	BUG_ON(cpu_online(cpu));
-	old_base = &per_cpu(hrtimer_bases, cpu);
+	BUG_ON(cpu_online(scpu));
+	old_base = &per_cpu(hrtimer_bases, scpu);
 	new_base = &get_cpu_var(hrtimer_bases);
  
-	tick_cancel_sched_timer(cpu);
+	dcpu = smp_processor_id();
+
+	tick_cancel_sched_timer(scpu);
 	/*
 	 * The caller is globally serialized and nobody else
 	 * takes two locks at once, deadlock is not possible.
  
  
  
  
  
@@ -1557,32 +1551,47 @@
  
 	for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++) {
 		migrate_hrtimer_list(&old_base->clock_base[i],
-				     &new_base->clock_base[i], cpu);
+				     &new_base->clock_base[i]);
 	}
  
 	spin_unlock(&old_base->lock);
 	spin_unlock_irq(&new_base->lock);
 	put_cpu_var(hrtimer_bases);
+
+	return dcpu;
 }
+
+static void tickle_timers(void *arg)
+{
+	hrtimer_peek_ahead_timers();
+}
+
 #endif /* CONFIG_HOTPLUG_CPU */
  
 static int __cpuinit hrtimer_cpu_notify(struct notifier_block *self,
 					unsigned long action, void *hcpu)
 {
-	unsigned int cpu = (long)hcpu;
+	int dcpu = -1, scpu = (long)hcpu;
  
 	switch (action) {
  
 	case CPU_UP_PREPARE:
 	case CPU_UP_PREPARE_FROZEN:
-		init_hrtimers_cpu(cpu);
+		init_hrtimers_cpu(scpu);
 		break;
  
 #ifdef CONFIG_HOTPLUG_CPU
 	case CPU_DEAD:
 	case CPU_DEAD_FROZEN:
-		clockevents_notify(CLOCK_EVT_NOTIFY_CPU_DEAD, &cpu);
-		migrate_hrtimers(cpu);
+		clockevents_notify(CLOCK_EVT_NOTIFY_CPU_DEAD, &scpu);
+		dcpu = migrate_hrtimers(scpu);
+		break;
+
+	case CPU_POST_DEAD:
+		if (dcpu == -1)
+			break;
+
+		smp_call_function_single(dcpu, tickle_timers, NULL, 0);
 		break;
 #endif
...	...	@@ -1496,7 +1496,7 @@
1496	1496	#ifdef CONFIG_HOTPLUG_CPU
1497	1497
1498	1498	static void migrate_hrtimer_list(struct hrtimer_clock_base *old_base,
1499		- struct hrtimer_clock_base *new_base, int dcpu)
	1499	+ struct hrtimer_clock_base *new_base)
1500	1500	{
1501	1501	struct hrtimer *timer;
1502	1502	struct rb_node *node;
1503	1503
1504	1504
1505	1505
1506	1506
1507	1507
1508	1508
...	...	@@ -1514,40 +1514,34 @@
1514	1514	__remove_hrtimer(timer, old_base, HRTIMER_STATE_MIGRATE, 0);
1515	1515	timer->base = new_base;
1516	1516	/*
1517		- * Enqueue the timer. Allow reprogramming of the event device
	1517	+ * Enqueue the timers on the new cpu, but do not reprogram
	1518	+ * the timer as that would enable a deadlock between
	1519	+ * hrtimer_enqueue_reprogramm() running the timer and us still
	1520	+ * holding a nested base lock.
	1521	+ *
	1522	+ * Instead we tickle the hrtimer interrupt after the migration
	1523	+ * is done, which will run all expired timers and re-programm
	1524	+ * the timer device.
1518	1525	*/
1519		- enqueue_hrtimer(timer, new_base, 1);
	1526	+ enqueue_hrtimer(timer, new_base, 0);
1520	1527
1521		-#ifdef CONFIG_HIGH_RES_TIMERS
1522		- /*
1523		- * Happens with high res enabled when the timer was
1524		- * already expired and the callback mode is
1525		- * HRTIMER_CB_IRQSAFE_UNLOCKED (hrtimer_sleeper). The
1526		- * enqueue code does not move them to the soft irq
1527		- * pending list for performance/latency reasons, but
1528		- * in the migration state, we need to do that
1529		- * otherwise we end up with a stale timer.
1530		- */
1531		- if (timer->state == HRTIMER_STATE_MIGRATE) {
1532		- /* XXX: running on offline cpu */
1533		- __run_hrtimer(timer);
1534		- }
1535		-#endif
1536	1528	/* Clear the migration state bit */
1537	1529	timer->state &= ~HRTIMER_STATE_MIGRATE;
1538	1530	}
1539	1531	}
1540	1532
1541		-static void migrate_hrtimers(int cpu)
	1533	+static int migrate_hrtimers(int scpu)
1542	1534	{
1543	1535	struct hrtimer_cpu_base old_base, new_base;
1544		- int i;
	1536	+ int dcpu, i;
1545	1537
1546		- BUG_ON(cpu_online(cpu));
1547		- old_base = &per_cpu(hrtimer_bases, cpu);
	1538	+ BUG_ON(cpu_online(scpu));
	1539	+ old_base = &per_cpu(hrtimer_bases, scpu);
1548	1540	new_base = &get_cpu_var(hrtimer_bases);
1549	1541
1550		- tick_cancel_sched_timer(cpu);
	1542	+ dcpu = smp_processor_id();
	1543	+
	1544	+ tick_cancel_sched_timer(scpu);
1551	1545	/*
1552	1546	* The caller is globally serialized and nobody else
1553	1547	* takes two locks at once, deadlock is not possible.
1554	1548
1555	1549
1556	1550
1557	1551
1558	1552
...	...	@@ -1557,32 +1551,47 @@
1557	1551
1558	1552	for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++) {
1559	1553	migrate_hrtimer_list(&old_base->clock_base[i],
1560		- &new_base->clock_base[i], cpu);
	1554	+ &new_base->clock_base[i]);
1561	1555	}
1562	1556
1563	1557	spin_unlock(&old_base->lock);
1564	1558	spin_unlock_irq(&new_base->lock);
1565	1559	put_cpu_var(hrtimer_bases);
	1560	+
	1561	+ return dcpu;
1566	1562	}
	1563	+
	1564	+static void tickle_timers(void *arg)
	1565	+{
	1566	+ hrtimer_peek_ahead_timers();
	1567	+}
	1568	+
1567	1569	#endif /* CONFIG_HOTPLUG_CPU */
1568	1570
1569	1571	static int __cpuinit hrtimer_cpu_notify(struct notifier_block *self,
1570	1572	unsigned long action, void *hcpu)
1571	1573	{
1572		- unsigned int cpu = (long)hcpu;
	1574	+ int dcpu = -1, scpu = (long)hcpu;
1573	1575
1574	1576	switch (action) {
1575	1577
1576	1578	case CPU_UP_PREPARE:
1577	1579	case CPU_UP_PREPARE_FROZEN:
1578		- init_hrtimers_cpu(cpu);
	1580	+ init_hrtimers_cpu(scpu);
1579	1581	break;
1580	1582
1581	1583	#ifdef CONFIG_HOTPLUG_CPU
1582	1584	case CPU_DEAD:
1583	1585	case CPU_DEAD_FROZEN:
1584		- clockevents_notify(CLOCK_EVT_NOTIFY_CPU_DEAD, &cpu);
1585		- migrate_hrtimers(cpu);
	1586	+ clockevents_notify(CLOCK_EVT_NOTIFY_CPU_DEAD, &scpu);
	1587	+ dcpu = migrate_hrtimers(scpu);
	1588	+ break;
	1589	+
	1590	+ case CPU_POST_DEAD:
	1591	+ if (dcpu == -1)
	1592	+ break;
	1593	+
	1594	+ smp_call_function_single(dcpu, tickle_timers, NULL, 0);
1586	1595	break;
1587	1596	#endif
1588	1597