[4/4] ARM: timer-sp: Set dynamic irq affinity

Message ID 1361917047-29230-5-git-send-email-daniel.lezcano@linaro.org
State Superseded
Headers show

Commit Message

Daniel Lezcano Feb. 26, 2013, 10:17 p.m.
From: Viresh Kumar <viresh.kumar@linaro.org>

When a cpu goes to a deep idle state where its local timer is shutdown, it
notifies the time frame work to use the broadcast timer instead.

Unfortunately, the broadcast device could wake up any CPU, including an idle one
which is not concerned by the wake up at all.

This implies, in the worst case, an idle CPU will wake up to send an IPI to
another idle cpu.

This patch fixes this for ARM platforms using timer-sp, by setting
CLOCK_EVT_FEAT_DYNIRQ feature.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 arch/arm/common/timer-sp.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Santosh Shilimkar Feb. 27, 2013, 4:56 a.m. | #1
On Wednesday 27 February 2013 03:47 AM, Daniel Lezcano wrote:
> From: Viresh Kumar <viresh.kumar@linaro.org>
>
> When a cpu goes to a deep idle state where its local timer is shutdown, it
> notifies the time frame work to use the broadcast timer instead.
>
> Unfortunately, the broadcast device could wake up any CPU, including an idle one
> which is not concerned by the wake up at all.
>
Broad-cast device will only open the CPU for which the timer IRQ
affined to. And infact with subject series the affinity also is
updated for the CPU which owns the last timer expiry event.

> This implies, in the worst case, an idle CPU will wake up to send an IPI to
> another idle cpu.
>
> This patch fixes this for ARM platforms using timer-sp, by setting
> CLOCK_EVT_FEAT_DYNIRQ feature.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
What am I missing here ?

Regards,
Santosh
Viresh Kumar Feb. 27, 2013, 4:59 a.m. | #2
On 27 February 2013 10:26, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
> On Wednesday 27 February 2013 03:47 AM, Daniel Lezcano wrote:
>>
>> From: Viresh Kumar <viresh.kumar@linaro.org>
>>
>> When a cpu goes to a deep idle state where its local timer is shutdown, it
>> notifies the time frame work to use the broadcast timer instead.
>>
>> Unfortunately, the broadcast device could wake up any CPU, including an
>> idle one
>> which is not concerned by the wake up at all.
>>
> Broad-cast device will only open the CPU for which the timer IRQ
> affined to. And infact with subject series the affinity also is
> updated for the CPU which owns the last timer expiry event.
>
> What am I missing here ?

Dynamic affinity will work only if the following flag is set for a
clock_event_device: CLOCK_EVT_FEAT_DYNIRQ, otherwise wakeup
would happen on the cpu to which static affinity was set to.
Santosh Shilimkar Feb. 27, 2013, 5:04 a.m. | #3
On Wednesday 27 February 2013 10:29 AM, Viresh Kumar wrote:
> On 27 February 2013 10:26, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
>> On Wednesday 27 February 2013 03:47 AM, Daniel Lezcano wrote:
>>>
>>> From: Viresh Kumar <viresh.kumar@linaro.org>
>>>
>>> When a cpu goes to a deep idle state where its local timer is shutdown, it
>>> notifies the time frame work to use the broadcast timer instead.
>>>
>>> Unfortunately, the broadcast device could wake up any CPU, including an
>>> idle one
>>> which is not concerned by the wake up at all.
>>>
>> Broad-cast device will only open the CPU for which the timer IRQ
>> affined to. And infact with subject series the affinity also is
>> updated for the CPU which owns the last timer expiry event.
>>
>> What am I missing here ?
>
> Dynamic affinity will work only if the following flag is set for a
> clock_event_device: CLOCK_EVT_FEAT_DYNIRQ, otherwise wakeup
> would happen on the cpu to which static affinity was set to.
>
I should have looked at the patches in order first :)
Sorry for the noise.

Regards,
Santosh

Patch

diff --git a/arch/arm/common/timer-sp.c b/arch/arm/common/timer-sp.c
index 9d2d3ba..ae3c0f9 100644
--- a/arch/arm/common/timer-sp.c
+++ b/arch/arm/common/timer-sp.c
@@ -158,7 +158,8 @@  static int sp804_set_next_event(unsigned long next,
 }
 
 static struct clock_event_device sp804_clockevent = {
-	.features       = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
+	.features       = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT |
+		CLOCK_EVT_FEAT_DYNIRQ,
 	.set_mode	= sp804_set_mode,
 	.set_next_event	= sp804_set_next_event,
 	.rating		= 300,