2014-11-29 16:14:00

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH urgent v2] sched: Add missing rcu protection to wake_up_all_idle_cpus

Locklessly doing is_idle_task(rq->curr) is only okay because of RCU
protection. The older variant of the broken code checked
rq->curr == rq->idle instead and therefore didn't need RCU.

Fixes: f6be8af1c95d sched: Add new API wake_up_if_idle() to wake up the idle cpu
Cc: Chuansheng Liu <[email protected]>
Signed-off-by: Andy Lutomirski <[email protected]>
---

This fixes a 3.18 regression, I think. This code seems to only be called
inside preempt_disable, so I think it's unlikely to be a problem in practice.

Compile-tested only, although the kbuild bot is chewing on it.

Changes from v1:
- Cc the right people (whoops)
- Add rcu_read_lock and rcu_read_unlock.

kernel/sched/core.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 240157c13ddc..b7937512965f 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1623,8 +1623,10 @@ void wake_up_if_idle(int cpu)
struct rq *rq = cpu_rq(cpu);
unsigned long flags;

- if (!is_idle_task(rq->curr))
- return;
+ rcu_read_lock();
+
+ if (!is_idle_task(rcu_dereference(rq->curr)))
+ goto out;

if (set_nr_if_polling(rq->idle)) {
trace_sched_wake_idle_without_ipi(cpu);
@@ -1635,6 +1637,9 @@ void wake_up_if_idle(int cpu)
/* Else cpu is not in idle, do nothing here */
raw_spin_unlock_irqrestore(&rq->lock, flags);
}
+
+out:
+ rcu_read_unlock();
}

bool cpus_share_cache(int this_cpu, int that_cpu)
--
1.9.3


2014-12-01 03:13:43

by Liu, Chuansheng

[permalink] [raw]
Subject: RE: [PATCH urgent v2] sched: Add missing rcu protection to wake_up_all_idle_cpus



> -----Original Message-----
> From: Andy Lutomirski [mailto:[email protected]]
> Sent: Sunday, November 30, 2014 12:14 AM
> To: Ingo Molnar; Thomas Gleixner; [email protected]
> Cc: Peter Zijlstra; Andy Lutomirski; Liu, Chuansheng
> Subject: [PATCH urgent v2] sched: Add missing rcu protection to
> wake_up_all_idle_cpus
>
> Locklessly doing is_idle_task(rq->curr) is only okay because of RCU
> protection. The older variant of the broken code checked
> rq->curr == rq->idle instead and therefore didn't need RCU.
>
> Fixes: f6be8af1c95d sched: Add new API wake_up_if_idle() to wake up the idle
> cpu
> Cc: Chuansheng Liu <[email protected]>
> Signed-off-by: Andy Lutomirski <[email protected]>
> ---
Reviewed-by: Chuansheng Liu <[email protected]>

Thanks Andy.

Subject: [tip:sched/core] sched: Add missing rcu protection to wake_up_all_idle_cpus

Commit-ID: fd7de1e8d5b2b2b35e71332fafb899f584597150
Gitweb: http://git.kernel.org/tip/fd7de1e8d5b2b2b35e71332fafb899f584597150
Author: Andy Lutomirski <[email protected]>
AuthorDate: Sat, 29 Nov 2014 08:13:51 -0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Mon, 8 Dec 2014 11:44:19 +0100

sched: Add missing rcu protection to wake_up_all_idle_cpus

Locklessly doing is_idle_task(rq->curr) is only okay because of
RCU protection. The older variant of the broken code checked
rq->curr == rq->idle instead and therefore didn't need RCU.

Fixes: f6be8af1c95d ("sched: Add new API wake_up_if_idle() to wake up the idle cpu")
Signed-off-by: Andy Lutomirski <[email protected]>
Reviewed-by: Chuansheng Liu <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/729365dddca178506dfd0a9451006344cd6808bc.1417277372.git.luto@amacapital.net
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/sched/core.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index d44d0c5..88f49bc 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1628,8 +1628,10 @@ void wake_up_if_idle(int cpu)
struct rq *rq = cpu_rq(cpu);
unsigned long flags;

- if (!is_idle_task(rq->curr))
- return;
+ rcu_read_lock();
+
+ if (!is_idle_task(rcu_dereference(rq->curr)))
+ goto out;

if (set_nr_if_polling(rq->idle)) {
trace_sched_wake_idle_without_ipi(cpu);
@@ -1640,6 +1642,9 @@ void wake_up_if_idle(int cpu)
/* Else cpu is not in idle, do nothing here */
raw_spin_unlock_irqrestore(&rq->lock, flags);
}
+
+out:
+ rcu_read_unlock();
}

bool cpus_share_cache(int this_cpu, int that_cpu)