Remove ARM local timer API

On an SMP system with only one global clockevent and a dummy clockevent per CPU we run into problems. We want the dummy clockevents to be registered as the per CPU tick devices, but we can only achieve that if we register the dummy clockevents before the global clockevent or if we artificially inflate the rating of the dummy clockevents to be higher than the rating of the global clockevent. Failure to do so leads to boot hangs when the dummy timers are registered on all other CPUs besides the CPU that accepted the global clockevent as its tick device and there is no broadcast timer to poke the dummy devices.

If we're registering multiple clockevents and one clockevent is global and the other is local to a particular CPU we should choose to use the local clockevent regardless of the rating of the device. This way, if the clockevent is a dummy it will take the tick device duty as long as there isn't a higher rated tick device and any global clockevent will be bumped out into broadcast mode, fixing the problem described above.

Reported-by Mark Rutland <[email protected]>

Tested-by Mark Rutland <[email protected]>

Tested-by S?ren Brinkmann <[email protected]>

Acked-by Marc Zyngier <[email protected]>,

Cc John Stultz <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Daniel Lezcano <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---
 kernel/time/tick-common.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

kernel/time/tick-common.c +3 -2

index 5d3fb10..3da62de 100644
@@ -254,9 +254,10 @@ static int tick_check_new_device(struct clock_event_device *newdev)
 		    !(newdev->features & CLOCK_EVT_FEAT_ONESHOT))
 			goto out_bc;
 		/*
-		 * Check the rating
+		 * Check the rating, but prefer CPU local devices
 		 */
-		if (curdev->rating >= newdev->rating)
+		if (curdev->rating >= newdev->rating &&
+		    cpumask_equal(curdev->cpumask, newdev->cpumask))
 			goto out_bc;
 	}
 

Quoted 12 lines · Stephen Boyd

On an SMP system with only one global clockevent and a dummy clockevent per CPU we run into problems. We want the dummy clockevents to be registered as the per CPU tick devices, but we can only achieve that if we register the dummy clockevents before the global clockevent or if we artificially inflate the rating of the dummy clockevents to be higher than the rating of the global clockevent. Failure to do so leads to boot hangs when the dummy timers are registered on all other CPUs besides the CPU that accepted the global clockevent as its tick device and there is no broadcast timer to poke the dummy devices.

Quoted 7 lines

If we're registering multiple clockevents and one clockevent is global and the other is local to a particular CPU we should choose to use the local clockevent regardless of the rating of the device. This way, if the clockevent is a dummy it will take the tick device duty as long as there isn't a higher rated tick device and any global clockevent will be bumped out into broadcast mode, fixing the problem described above.

It is not clear the connection between the changelog, the patch and the comment. Could you clarify a bit ?

Quoted 21 lines · Daniel Lezcano

Quoted 12 lines · Stephen Boyd

On an SMP system with only one global clockevent and a dummy clockevent per CPU we run into problems. We want the dummy clockevents to be registered as the per CPU tick devices, but we can only achieve that if we register the dummy clockevents before the global clockevent or if we artificially inflate the rating of the dummy clockevents to be higher than the rating of the global clockevent. Failure to do so leads to boot hangs when the dummy timers are registered on all other CPUs besides the CPU that accepted the global clockevent as its tick device and there is no broadcast timer to poke the dummy devices.

Quoted 7 lines

If we're registering multiple clockevents and one clockevent is global and the other is local to a particular CPU we should choose to use the local clockevent regardless of the rating of the device. This way, if the clockevent is a dummy it will take the tick device duty as long as there isn't a higher rated tick device and any global clockevent will be bumped out into broadcast mode, fixing the problem described above.

Quoted 3 lines

It is not clear the connection between the changelog, the patch and the comment. Could you clarify a bit ?

There is one tick device per-cpu and one broadcast device. The broadcast device can only be a global clockevent, whereas the per-cpu tick device can be a global clockevent or a per-cpu clockevent. The code tries hard to keep per-cpu clockevents in the tick device slots but it has an ordering/rating requirement that doesn't work when there are only dummy per-cpu devices and one global device.

Perhaps an example will help. Let's say you only have one global clockevent such as the sp804, and you have SMP enabled. To support SMP we have to register dummy clockevents on each CPU so that the sp804 can go into broadcast mode. If we don't do this, only the CPU that registered the sp804 will get interrupts while the other CPUs will be left with no tick device and thus no scheduling. To fix this we register dummy clockevents on all the CPUs _before_ we register the sp804 to force the sp804 into the broadcast slot. Or we give the dummy clockevents a higher rating than the sp804 so that when we register them after the sp804 the sp804 is bumped out to broadcast duty.

If the dummy devices are registered before the sp804 we can give the dummies a low rating and the sp804 will still go into the broadcast slot due to this code:

	/*
	 * If we have a cpu local device already, do not replace it
	 * by a non cpu local device
	 */
	if (curdev && cpumask_equal(curdev->cpumask, cpumask_of(cpu)))
		goto out_bc;

If we register the sp804 before the dummies we're also fine as long as the rating of the dummy is more than the sp804. Playing games with the dummy rating is not very nice so this patch fixes it by allowing the per-cpu device to replace the global device no matter what the rating of the global device is.

This fixes the sp804 case when the dummy is rated lower than sp804 and it removes any ordering requirement from the registration of clockevents. It also completes the logic above where we prefer cpu local devices over non cpu local devices.

Quoted 63 lines · Stephen Boyd

Quoted 21 lines · Daniel Lezcano

Quoted 12 lines · Stephen Boyd

On an SMP system with only one global clockevent and a dummy clockevent per CPU we run into problems. We want the dummy clockevents to be registered as the per CPU tick devices, but we can only achieve that if we register the dummy clockevents before the global clockevent or if we artificially inflate the rating of the dummy clockevents to be higher than the rating of the global clockevent. Failure to do so leads to boot hangs when the dummy timers are registered on all other CPUs besides the CPU that accepted the global clockevent as its tick device and there is no broadcast timer to poke the dummy devices.

Quoted 7 lines

If we're registering multiple clockevents and one clockevent is global and the other is local to a particular CPU we should choose to use the local clockevent regardless of the rating of the device. This way, if the clockevent is a dummy it will take the tick device duty as long as there isn't a higher rated tick device and any global clockevent will be bumped out into broadcast mode, fixing the problem described above.

Quoted 3 lines

It is not clear the connection between the changelog, the patch and the comment. Could you clarify a bit ?

There is one tick device per-cpu and one broadcast device. The broadcast device can only be a global clockevent, whereas the per-cpu tick device can be a global clockevent or a per-cpu clockevent. The code tries hard to keep per-cpu clockevents in the tick device slots but it has an ordering/rating requirement that doesn't work when there are only dummy per-cpu devices and one global device.

Perhaps an example will help. Let's say you only have one global clockevent such as the sp804, and you have SMP enabled. To support SMP we have to register dummy clockevents on each CPU so that the sp804 can go into broadcast mode. If we don't do this, only the CPU that registered the sp804 will get interrupts while the other CPUs will be left with no tick device and thus no scheduling. To fix this we register dummy clockevents on all the CPUs _before_ we register the sp804 to force the sp804 into the broadcast slot. Or we give the dummy clockevents a higher rating than the sp804 so that when we register them after the sp804 the sp804 is bumped out to broadcast duty.

If the dummy devices are registered before the sp804 we can give the dummies a low rating and the sp804 will still go into the broadcast slot due to this code:

	/*
	 * If we have a cpu local device already, do not replace it
	 * by a non cpu local device
	 */
	if (curdev && cpumask_equal(curdev->cpumask, cpumask_of(cpu)))
		goto out_bc;

If we register the sp804 before the dummies we're also fine as long as the rating of the dummy is more than the sp804. Playing games with the dummy rating is not very nice so this patch fixes it by allowing the per-cpu device to replace the global device no matter what the rating of the global device is.

Quoted 4 lines

This fixes the sp804 case when the dummy is rated lower than sp804 and it removes any ordering requirement from the registration of clockevents. It also completes the logic above where we prefer cpu local devices over non cpu local devices.

Thanks for the detailed explanation.

Did Thomas reacted to this patch ?

Quoted 69 lines · Daniel Lezcano

Quoted 67 lines · Stephen Boyd

Quoted 21 lines · Daniel Lezcano

Quoted 12 lines · Stephen Boyd

On an SMP system with only one global clockevent and a dummy clockevent per CPU we run into problems. We want the dummy clockevents to be registered as the per CPU tick devices, but we can only achieve that if we register the dummy clockevents before the global clockevent or if we artificially inflate the rating of the dummy clockevents to be higher than the rating of the global clockevent. Failure to do so leads to boot hangs when the dummy timers are registered on all other CPUs besides the CPU that accepted the global clockevent as its tick device and there is no broadcast timer to poke the dummy devices.

Quoted 7 lines

If we're registering multiple clockevents and one clockevent is global and the other is local to a particular CPU we should choose to use the local clockevent regardless of the rating of the device. This way, if the clockevent is a dummy it will take the tick device duty as long as there isn't a higher rated tick device and any global clockevent will be bumped out into broadcast mode, fixing the problem described above.

Quoted 3 lines

It is not clear the connection between the changelog, the patch and the comment. Could you clarify a bit ?

There is one tick device per-cpu and one broadcast device. The broadcast device can only be a global clockevent, whereas the per-cpu tick device can be a global clockevent or a per-cpu clockevent. The code tries hard to keep per-cpu clockevents in the tick device slots but it has an ordering/rating requirement that doesn't work when there are only dummy per-cpu devices and one global device.

Perhaps an example will help. Let's say you only have one global clockevent such as the sp804, and you have SMP enabled. To support SMP we have to register dummy clockevents on each CPU so that the sp804 can go into broadcast mode. If we don't do this, only the CPU that registered the sp804 will get interrupts while the other CPUs will be left with no tick device and thus no scheduling. To fix this we register dummy clockevents on all the CPUs _before_ we register the sp804 to force the sp804 into the broadcast slot. Or we give the dummy clockevents a higher rating than the sp804 so that when we register them after the sp804 the sp804 is bumped out to broadcast duty.

If the dummy devices are registered before the sp804 we can give the dummies a low rating and the sp804 will still go into the broadcast slot due to this code:

	/*
	 * If we have a cpu local device already, do not replace it
	 * by a non cpu local device
	 */
	if (curdev && cpumask_equal(curdev->cpumask, cpumask_of(cpu)))
		goto out_bc;

If we register the sp804 before the dummies we're also fine as long as the rating of the dummy is more than the sp804. Playing games with the dummy rating is not very nice so this patch fixes it by allowing the per-cpu device to replace the global device no matter what the rating of the global device is.

This fixes the sp804 case when the dummy is rated lower than sp804 and it removes any ordering requirement from the registration of clockevents. It also completes the logic above where we prefer cpu local devices over non cpu local devices.

Quoted 4 lines

Thanks for the detailed explanation.

Did Thomas reacted to this patch ?

So far there has been no response from Thomas.

Quoted 72 lines · Stephen Boyd

Quoted 71 lines · Daniel Lezcano

Quoted 63 lines · Stephen Boyd

Quoted 21 lines · Daniel Lezcano

Quoted 12 lines · Stephen Boyd

On an SMP system with only one global clockevent and a dummy clockevent per CPU we run into problems. We want the dummy clockevents to be registered as the per CPU tick devices, but we can only achieve that if we register the dummy clockevents before the global clockevent or if we artificially inflate the rating of the dummy clockevents to be higher than the rating of the global clockevent. Failure to do so leads to boot hangs when the dummy timers are registered on all other CPUs besides the CPU that accepted the global clockevent as its tick device and there is no broadcast timer to poke the dummy devices.

Quoted 7 lines

If we're registering multiple clockevents and one clockevent is global and the other is local to a particular CPU we should choose to use the local clockevent regardless of the rating of the device. This way, if the clockevent is a dummy it will take the tick device duty as long as there isn't a higher rated tick device and any global clockevent will be bumped out into broadcast mode, fixing the problem described above.

Quoted 3 lines

It is not clear the connection between the changelog, the patch and the comment. Could you clarify a bit ?

There is one tick device per-cpu and one broadcast device. The broadcast device can only be a global clockevent, whereas the per-cpu tick device can be a global clockevent or a per-cpu clockevent. The code tries hard to keep per-cpu clockevents in the tick device slots but it has an ordering/rating requirement that doesn't work when there are only dummy per-cpu devices and one global device.

Perhaps an example will help. Let's say you only have one global clockevent such as the sp804, and you have SMP enabled. To support SMP we have to register dummy clockevents on each CPU so that the sp804 can go into broadcast mode. If we don't do this, only the CPU that registered the sp804 will get interrupts while the other CPUs will be left with no tick device and thus no scheduling. To fix this we register dummy clockevents on all the CPUs _before_ we register the sp804 to force the sp804 into the broadcast slot. Or we give the dummy clockevents a higher rating than the sp804 so that when we register them after the sp804 the sp804 is bumped out to broadcast duty.

If the dummy devices are registered before the sp804 we can give the dummies a low rating and the sp804 will still go into the broadcast slot due to this code:

	/*
	 * If we have a cpu local device already, do not replace it
	 * by a non cpu local device
	 */
	if (curdev && cpumask_equal(curdev->cpumask, cpumask_of(cpu)))
		goto out_bc;

If we register the sp804 before the dummies we're also fine as long as the rating of the dummy is more than the sp804. Playing games with the dummy rating is not very nice so this patch fixes it by allowing the per-cpu device to replace the global device no matter what the rating of the global device is.

Quoted 4 lines

This fixes the sp804 case when the dummy is rated lower than sp804 and it removes any ordering requirement from the registration of clockevents. It also completes the logic above where we prefer cpu local devices over non cpu local devices.

Thanks for the detailed explanation.

Quoted 5 lines

Quoted 2 lines

Did Thomas reacted to this patch ?

So far there has been no response from Thomas.

Will you ack this patch anyway? Or do we need Thomas to review this patch? It seems that this patch series has stalled again.

I prefer Thomas to have a look at it and ack it. I changed Cc to To for Thomas.

Quoted 2 lines · Daniel Lezcano

I prefer Thomas to have a look at it and ack it. I changed Cc to To for Thomas.

The patch does not apply on tip timers/core. The code has been reworked a month ago. Please work against tip timers/core. That's where this stuff ends up.

Quoted 8 lines · Thomas Gleixner

Quoted 2 lines · Daniel Lezcano

I prefer Thomas to have a look at it and ack it. I changed Cc to To for Thomas.

The patch does not apply on tip timers/core. The code has been reworked a month ago. Please work against tip timers/core. That's where this stuff ends up.

Ah, I thought your patch series had stalled. Here is a refreshed patch. Every other patch in this series applies cleanly to tip timers/core so I don't want to resend them again unless absolutely necessary.

-----8<-----
Subject: [PATCH v8] clockevents: Prefer CPU local devices over global devices

On an SMP system with only one global clockevent and a dummy clockevent per CPU we run into problems. We want the dummy clockevents to be registered as the per CPU tick devices, but we can only achieve that if we register the dummy clockevents before the global clockevent or if we artificially inflate the rating of the dummy clockevents to be higher than the rating of the global clockevent. Failure to do so leads to boot hangs when the dummy timers are registered on all other CPUs besides the CPU that accepted the global clockevent as its tick device and there is no broadcast timer to poke the dummy devices.

If we're registering multiple clockevents and one clockevent is global and the other is local to a particular CPU we should choose to use the local clockevent regardless of the rating of the device. This way, if the clockevent is a dummy it will take the tick device duty as long as there isn't a higher rated tick device and any global clockevent will be bumped out into broadcast mode, fixing the problem described above.

Reported-by Mark Rutland <[email protected]>

Cc John Stultz <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Daniel Lezcano <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

Quoted 8 lines · Thomas Gleixner

Quoted 2 lines · Daniel Lezcano

I prefer Thomas to have a look at it and ack it. I changed Cc to To for Thomas.

The patch does not apply on tip timers/core. The code has been reworked a month ago. Please work against tip timers/core. That's where this stuff ends up.

Ah, I thought your patch series had stalled. Here is a refreshed patch. Every other patch in this series applies cleanly to tip timers/core so I don't want to resend them again unless absolutely necessary.

-----8<-----
Subject: [PATCH v8] clockevents: Prefer CPU local devices over global devices

On an SMP system with only one global clockevent and a dummy clockevent per CPU we run into problems. We want the dummy clockevents to be registered as the per CPU tick devices, but we can only achieve that if we register the dummy clockevents before the global clockevent or if we artificially inflate the rating of the dummy clockevents to be higher than the rating of the global clockevent. Failure to do so leads to boot hangs when the dummy timers are registered on all other CPUs besides the CPU that accepted the global clockevent as its tick device and there is no broadcast timer to poke the dummy devices.

If we're registering multiple clockevents and one clockevent is global and the other is local to a particular CPU we should choose to use the local clockevent regardless of the rating of the device. This way, if the clockevent is a dummy it will take the tick device duty as long as there isn't a higher rated tick device and any global clockevent will be bumped out into broadcast mode, fixing the problem described above.

Reported-by Mark Rutland <[email protected]>

Cc John Stultz <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Daniel Lezcano <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---
 kernel/time/tick-common.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

kernel/time/tick-common.c +7 -2

index 5edfb48..edd45f6 100644
@@ -243,8 +243,13 @@ static bool tick_check_preferred(struct clock_event_device *curdev,
 			return false;
 	}
 
-	/* Use the higher rated one */
-	return !curdev || newdev->rating > curdev->rating;
+	/*
+	 * Use the higher rated one, but prefer a CPU local device with a lower
+	 * rating than a non-CPU local device
+	 */
+	return !curdev ||
+		newdev->rating > curdev->rating ||
+	       !cpumask_equal(curdev->cpumask, newdev->cpumask);
 }
 
 /*

Signature · 3 lines

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Quoted 38 lines · Stephen Boyd

Quoted 8 lines · Thomas Gleixner

Quoted 2 lines · Daniel Lezcano

I prefer Thomas to have a look at it and ack it. I changed Cc to To for Thomas.

The patch does not apply on tip timers/core. The code has been reworked a month ago. Please work against tip timers/core. That's where this stuff ends up.

Ah, I thought your patch series had stalled. Here is a refreshed patch. Every other patch in this series applies cleanly to tip timers/core so I don't want to resend them again unless absolutely necessary.

-----8<-----
Subject: [PATCH v8] clockevents: Prefer CPU local devices over global devices

On an SMP system with only one global clockevent and a dummy clockevent per CPU we run into problems. We want the dummy clockevents to be registered as the per CPU tick devices, but we can only achieve that if we register the dummy clockevents before the global clockevent or if we artificially inflate the rating of the dummy clockevents to be higher than the rating of the global clockevent. Failure to do so leads to boot hangs when the dummy timers are registered on all other CPUs besides the CPU that accepted the global clockevent as its tick device and there is no broadcast timer to poke the dummy devices.

If we're registering multiple clockevents and one clockevent is global and the other is local to a particular CPU we should choose to use the local clockevent regardless of the rating of the device. This way, if the clockevent is a dummy it will take the tick device duty as long as there isn't a higher rated tick device and any global clockevent will be bumped out into broadcast mode, fixing the problem described above.

Quoted 5 lines

Reported-by Mark Rutland <[email protected]>

Cc John Stultz <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Daniel Lezcano <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

I've just tested this atop of tip/timers/core on a tc2, using only the sp804. As previously, without the patch boot hangs, and with the patch I'm able to reach userspace and do useful things.

Tested-by Mark Rutland <[email protected]>

Thanks for working on this, Stephen.

Can you pick up the first two patches in this series please? And preferably make a stable branch that can be pulled into arm-soc? Then I can send the rest through the arm-soc tree.

Quoted 4 lines · Stephen Boyd

Can you pick up the first two patches in this series please? And preferably make a stable branch that can be pulled into arm-soc? Then I can send the rest through the arm-soc tree.

ping?

From: Mark Rutland <[email protected]>

Several architectures have a dummy timer driver tightly coupled with their broadcast code to support machines without cpu-local timers (or where there is a lack of driver support).

Since 12ad100046: "clockevents: Add generic timer broadcast function" it's been possible to write broadcast-capable timer drivers decoupled from the broadcast mechanism. We can use this functionality to implement a generic dummy timer driver that can be shared by all architectures with generic tick broadcast (ARCH_HAS_TICK_BROADCAST).

This patch implements a generic dummy timer using this facility.

Signed-off-by Mark Rutland <[email protected]>

Acked-by Marc Zyngier <[email protected]>,

Cc John Stultz <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Daniel Lezcano <[email protected]>

[sboyd: Make percpu data static, use __this_cpu_ptr(), move to
early_initcall to properly register on each CPU, only register if more than one CPU possible]

Signed-off-by Stephen Boyd <[email protected]>

---

 drivers/clocksource/Makefile      |  1 +
 drivers/clocksource/dummy_timer.c | 69 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+)
 create mode 100644 drivers/clocksource/dummy_timer.c

drivers/clocksource/Makefile +1 -0

index 8d979c7..1a17d76 100644
@@ -29,3 +29,4 @@ obj-$(CONFIG_CLKSRC_SAMSUNG_PWM)	+= samsung_pwm_timer.o
 
 obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
 obj-$(CONFIG_CLKSRC_METAG_GENERIC)	+= metag_generic.o
+obj-$(CONFIG_ARCH_HAS_TICK_BROADCAST)	+= dummy_timer.o

drivers/clocksource/dummy_timer.c new file +69 -0

new file mode 100644
index 0000000..1f55f96
@@ -0,0 +1,69 @@
+/*
+ *  linux/drivers/clocksource/dummy_timer.c
+ *
+ *  Copyright (C) 2013 ARM Ltd.
+ *  All Rights Reserved
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/clockchips.h>
+#include <linux/cpu.h>
+#include <linux/init.h>
+#include <linux/percpu.h>
+#include <linux/cpumask.h>
+
+static DEFINE_PER_CPU(struct clock_event_device, dummy_timer_evt);
+
+static void dummy_timer_set_mode(enum clock_event_mode mode,
+			   struct clock_event_device *evt)
+{
+	/*
+	 * Core clockevents code will call this when exchanging timer devices.
+	 * We don't need to do anything here.
+	 */
+}
+
+static void __cpuinit dummy_timer_setup(void)
+{
+	int cpu = smp_processor_id();
+	struct clock_event_device *evt = __this_cpu_ptr(&dummy_timer_evt);
+
+	evt->name	= "dummy_timer";
+	evt->features	= CLOCK_EVT_FEAT_PERIODIC |
+			  CLOCK_EVT_FEAT_ONESHOT |
+			  CLOCK_EVT_FEAT_DUMMY;
+	evt->rating	= 100;
+	evt->set_mode	= dummy_timer_set_mode;
+	evt->cpumask	= cpumask_of(cpu);
+
+	clockevents_register_device(evt);
+}
+
+static int __cpuinit dummy_timer_cpu_notify(struct notifier_block *self,
+				      unsigned long action, void *hcpu)
+{
+	if ((action & ~CPU_TASKS_FROZEN) == CPU_STARTING)
+		dummy_timer_setup();
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block dummy_timer_cpu_nb __cpuinitdata = {
+	.notifier_call = dummy_timer_cpu_notify,
+};
+
+static int __init dummy_timer_register(void)
+{
+	int err = register_cpu_notifier(&dummy_timer_cpu_nb);
+	if (err)
+		return err;
+
+	/* We won't get a call on the boot CPU, so register immediately */
+	if (num_possible_cpus() > 1)
+		dummy_timer_setup();
+
+	return 0;
+}
+early_initcall(dummy_timer_register);