lkml.org 
[lkml]   [2015]   [Apr]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [PATCH 16/20] sched/idle: Use explicit broadcast oneshot control function
Date
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@kernel.org> 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


\
 
 \ /
  Last update: 2015-04-28 15:41    [W:0.090 / U:0.588 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site