Re: [PATCH] clockevents: Leave broadcast device shtudown only ifthe current device is always running.

From: Suresh Siddha
Date: Mon Apr 09 2012 - 18:39:49 EST


On Mon, 2012-04-09 at 11:33 +0530, Santosh Shilimkar wrote:
> Commit 77b0d60{clockevents: Leave the broadcast device in shutdown mode
> when not needed} was intended to leave the broadcast device in shutdown mode
> when the per-cpu clockevent devices are always running.
>
> This breaks the systems where per cpu clock events stop in low power states.
>
> Hence revert 77b0d60 and implement the same requirement with use
> of C3STOP feature flag.

Problem you encountered is not related to leaving the broadcast device
in shutdown mode. Problem is that we didn't track the mode change to
oneshot and later during idle entry/exit, when we request the broadcast
device services using CLOCK_EVT_NOTIFY_BROADCAST_ENTER etc,
tick_broadcast_oneshot_control() returns with out doing much as it
thinks broadcast device is in periodic mode.

Can you please check if the appended patch fixes the issue? I could
reproduce the issue on my NHM platform which doesn't have always running
apic timer and with the appended fix, all is well with cpu's going into
tickless idle etc. Thanks.
---

From: Suresh Siddha <suresh.b.siddha@xxxxxxxxx>
Subject: clockevents: track broadcast device mode change in tick_broadcast_switch_to_oneshot()

In the commit 77b0d60c5adf39c74039e2142a1d3cd1e4d53799,
"clockevents: Leave the broadcast device in shutdown mode when not needed",
we were bailing out too quickly in tick_broadcast_switch_to_oneshot(),
with out tracking the broadcast device mode change to 'TICKDEV_MODE_ONESHOT'.

This breaks the platforms which need broadcast device oneshot services during
deep idle states. tick_broadcast_oneshot_control() thinks that it is
in periodic mode and fails to take proper decisions based on the
CLOCK_EVT_NOTIFY_BROADCAST_[ENTER, EXIT] notifications during deep
idle entry/exit.

Fix this by tracking the broadcast device mode as 'TICKDEV_MODE_ONESHOT',
before leaving the broadcast HW device in shutdown mode if there are no active
requests for the moment.

Reported-by: Santosh Shilimkar <santosh.shilimkar@xxxxxx>
Signed-off-by: Suresh Siddha <suresh.b.siddha@xxxxxxxxx>
---
kernel/time/tick-broadcast.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index e883f57..bf57abd 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -575,10 +575,12 @@ void tick_broadcast_switch_to_oneshot(void)
unsigned long flags;

raw_spin_lock_irqsave(&tick_broadcast_lock, flags);
+
+ tick_broadcast_device.mode = TICKDEV_MODE_ONESHOT;
+
if (cpumask_empty(tick_get_broadcast_mask()))
goto end;

- tick_broadcast_device.mode = TICKDEV_MODE_ONESHOT;
bc = tick_broadcast_device.evtdev;
if (bc)
tick_broadcast_setup_oneshot(bc);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/