On Tuesday, April 28, 2015 02:37:10 PM Linus Walleij wrote:
> On Tue, Apr 28, 2015 at 2:19 PM, Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
> > Sudeep:
> >> At-least I observed issue only when I am using hardware broadcast timer.
> >> It doesn't hang when I am using hrtimer as broadcast timer in which case
> >> one of the cpu will be not enter deeper idle states that lose timer.
> >> I will rerun on v4.1-rc1 and post the complete log.
> >
> > So the bug here is that cpuidle_enter() enables interrupts, so the
> > assumption about them being not enabled made by
> > tick_broadcast_oneshot_control() is actually not valid.
> >
> > It looks like we need to acquire the clockevents_lock at least in this
> > particular case. Let me see where to put it and I'll send a patch for
> > testing.
>
> Aha that looks very much like it. Put me on the patch and I'll
> take it for a spin.
OK, so something like the below for starters (the _irqsave variant is used to
avoid adding one more WARN_ON(irqs_disabled()) in there).
I haven't tested it, but then I can't reproduce the original issue in the
first place.
---
include/linux/clockchips.h | 2 ++
kernel/sched/idle.c | 2 +-
kernel/time/clockevents.c | 15 +++++++++++++++
3 files changed, 18 insertions(+), 1 deletion(-)
Index: linux-pm/include/linux/clockchips.h
===================================================================
--- linux-pm.orig/include/linux/clockchips.h
+++ linux-pm/include/linux/clockchips.h
@@ -198,9 +198,11 @@ extern int tick_receive_broadcast(void);
# if defined(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST) && defined(CONFIG_TICK_ONESHOT)
extern void tick_setup_hrtimer_broadcast(void);
extern int tick_check_broadcast_expired(void);
+extern void tick_broadcast_exit_idle(void);
# else
static inline int tick_check_broadcast_expired(void) { return 0; }
static inline void tick_setup_hrtimer_broadcast(void) { }
+static inline void tick_broadcast_exit_idle(void) { }
# endif
extern int clockevents_notify(unsigned long reason, void *arg);
Index: linux-pm/kernel/time/clockevents.c
===================================================================
--- linux-pm.orig/kernel/time/clockevents.c
+++ linux-pm/kernel/time/clockevents.c
@@ -735,6 +735,21 @@ static ssize_t sysfs_unbind_tick_dev(str
static DEVICE_ATTR(unbind_device, 0200, NULL, sysfs_unbind_tick_dev);
#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
+/**
+ * tick_broadcast_exit_idle - Broadcast oneshot mode exit for the idle loop.
+ *
+ * The idle loop exits the broadcast oneshot mode with interrupts enabled, so
+ * clockevents_lock needs to be acquired around that.
+ */
+void tick_broadcast_exit_idle(void)
+{
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&clockevents_lock, flags);
+ tick_broadcast_exit();
+ raw_spin_unlock_irqrestore(&clockevents_lock, flags);
+}
+
static struct device tick_bc_dev = {
.init_name = "broadcast",
.id = 0,
Index: linux-pm/kernel/sched/idle.c
===================================================================
--- linux-pm.orig/kernel/sched/idle.c
+++ linux-pm/kernel/sched/idle.c
@@ -175,7 +175,7 @@ static void cpuidle_idle_call(void)
idle_set_state(this_rq(), NULL);
if (broadcast)
- tick_broadcast_exit();
+ tick_broadcast_exit_idle();
/*
* Give the governor an opportunity to reflect on the outcome
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
