From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: peterz@infradead.org, mingo@elte.hu
Cc: linux-kernel@vger.kernel.org, rjw@rjwysocki.net,
linux-pm@vger.kernel.org, alex.shi@linaro.org,
vincent.guittot@linaro.org, morten.rasmussen@arm.com,
linaro-kernel@lists.linaro.org
Subject: [PATCH 3/3] sched: idle: Store the idle state the cpu is
Date: Thu, 24 Apr 2014 14:24:51 +0200 [thread overview]
Message-ID: <1398342291-16322-4-git-send-email-daniel.lezcano@linaro.org> (raw)
In-Reply-To: <1398342291-16322-1-git-send-email-daniel.lezcano@linaro.org>
When the cpu enters idle it stores the cpuidle state pointer in the struct
rq which in turn could be used to take a right decision when balancing
a task.
As soon as the cpu exits the idle state, the structure is filled back with the
NULL pointer.
There are a couple of situations where the idle state pointer could be changed
while looking at it:
1. For x86/acpi with dynamic c-states, when a laptop switches from battery
to AC that could result on removing the deeper idle state. The acpi driver
triggers:
'acpi_processor_cst_has_changed'
'cpuidle_pause_and_lock'
'cpuidle_uninstall_idle_handler'
'kick_all_cpus_sync'.
All cpus will exit their idle state and the pointed object will be set to NULL.
2. The cpuidle driver is unloaded. Logically that could happen but not in
practice because the drivers are always compiled in and 95% of the drivers are
not coded to unregister the driver. In any case, the unloading code must call
'cpuidle_unregister_device', that calls 'cpuidle_pause_and_lock' leading to
'kick_all_cpus_sync' as mentioned above.
The race can happen if we use the pointer and then one of these two situations
occurs at the same moment.
In order to be safe, the idle state pointer stored in the rq must be used inside
a rcu_read_lock section where we are protected with the 'rcu_barrier' in the
'cpuidle_uninstall_idle_handler' function.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/cpuidle/cpuidle.c | 6 ++++++
kernel/sched/idle.c | 8 ++++++++
kernel/sched/sched.h | 5 +++++
3 files changed, 19 insertions(+)
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 8236746..6a13f40 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -190,6 +190,12 @@ void cpuidle_install_idle_handler(void)
*/
void cpuidle_uninstall_idle_handler(void)
{
+ /*
+ * Wait for the scheduler to finish to use the idle state he
+ * may pointing to when looking for the cpu idle states
+ */
+ rcu_barrier();
+
if (enabled_devices) {
initialized = 0;
kick_all_cpus_sync();
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index e877dd4..4c14ec0 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -12,6 +12,8 @@
#include <trace/events/power.h>
+#include "sched.h"
+
static int __read_mostly cpu_idle_force_poll;
void cpu_idle_poll_ctrl(bool enable)
@@ -66,6 +68,8 @@ void __weak arch_cpu_idle(void)
#ifdef CONFIG_CPU_IDLE
static int __cpuidle_idle_call(void)
{
+ struct rq *rq = this_rq();
+ struct cpuidle_state **idle_state = &rq->idle_state;
struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
int next_state, entered_state, ret;
@@ -114,6 +118,8 @@ static int __cpuidle_idle_call(void)
if (!ret) {
trace_cpu_idle_rcuidle(next_state, dev->cpu);
+ *idle_state = &drv->states[next_state];
+
/*
* Enter the idle state previously returned by
* the governor decision. This function will
@@ -123,6 +129,8 @@ static int __cpuidle_idle_call(void)
*/
entered_state = cpuidle_enter(drv, dev, next_state);
+ *idle_state = NULL;
+
trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu);
if (broadcast)
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 456e492..bace64a 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -14,6 +14,7 @@
#include "cpuacct.h"
struct rq;
+struct cpuidle_state;
extern __read_mostly int scheduler_running;
@@ -643,6 +644,10 @@ struct rq {
#ifdef CONFIG_SMP
struct llist_head wake_list;
#endif
+
+#ifdef CONFIG_CPU_IDLE
+ struct cpuidle_state *idle_state;
+#endif
};
static inline int cpu_of(struct rq *rq)
--
1.7.9.5
next prev parent reply other threads:[~2014-04-24 12:27 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-24 12:24 [PATCH 0/3] sched: idle: Provide the basis to integrate cpuidle Daniel Lezcano
2014-04-24 12:24 ` [PATCH 1/3] sched: idle: Encapsulate the code to compile it out Daniel Lezcano
2014-04-30 17:39 ` Nicolas Pitre
2014-04-24 12:24 ` [PATCH 2/3] sched: idle: Add sched balance option Daniel Lezcano
[not found] ` <CAP245DX9wewQFhcyGj5ZuNE7hHC4fRn90POC32HLF6ugja6nJg@mail.gmail.com>
2014-04-24 13:30 ` Daniel Lezcano
2014-04-25 10:54 ` Amit Kucheria
2014-04-25 11:46 ` Rafael J. Wysocki
2014-04-25 12:17 ` Daniel Lezcano
2014-04-25 13:20 ` Peter Zijlstra
2014-04-25 17:01 ` Daniel Lezcano
2014-04-25 18:43 ` Peter Zijlstra
2014-04-28 10:09 ` Daniel Lezcano
2014-04-28 10:28 ` Peter Zijlstra
2014-04-28 11:07 ` Daniel Lezcano
2014-04-28 11:21 ` Peter Zijlstra
2014-04-28 23:11 ` Rafael J. Wysocki
2014-04-29 10:00 ` Morten Rasmussen
2014-04-29 22:19 ` Rafael J. Wysocki
2014-04-29 10:25 ` Daniel Lezcano
2014-05-05 0:32 ` Rafael J. Wysocki
2014-04-29 9:26 ` Morten Rasmussen
2014-04-26 0:18 ` Rafael J. Wysocki
2014-04-26 6:17 ` Ingo Molnar
2014-04-27 13:23 ` Rafael J. Wysocki
2014-04-29 10:50 ` Morten Rasmussen
2014-04-29 22:02 ` Rafael J. Wysocki
2014-04-26 10:55 ` Peter Zijlstra
2014-04-27 13:54 ` Rafael J. Wysocki
2014-04-27 19:39 ` Peter Zijlstra
2014-04-25 13:22 ` Peter Zijlstra
2014-04-24 12:24 ` Daniel Lezcano [this message]
2014-04-24 16:16 ` [PATCH 3/3] sched: idle: Store the idle state the cpu is Rafael J. Wysocki
2014-04-24 16:13 ` [PATCH 0/3] sched: idle: Provide the basis to integrate cpuidle Rafael J. Wysocki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1398342291-16322-4-git-send-email-daniel.lezcano@linaro.org \
--to=daniel.lezcano@linaro.org \
--cc=alex.shi@linaro.org \
--cc=linaro-kernel@lists.linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=morten.rasmussen@arm.com \
--cc=peterz@infradead.org \
--cc=rjw@rjwysocki.net \
--cc=vincent.guittot@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).