So this fixes some potential RCU stalls in a bunch of architectures.
When rcu_idle_enter()/rcu_idle_exit() became a requirement, we forgot
to handle the architectures that don't support CONFIG_NO_HZ.
I guess the set should be dispatched into arch maintainer trees.
I'm sorry I haven't built tested everywhere. But the changes are
small and need to be at least boot tested anyway.
Also many of these archs use the same kind of idle loop:
void cpu_idle(void)
{
while (1) {
rcu_idle_enter();
while (!need_resched())
//power saving function()
rcu_idle_exit();
schedule_preempt_disabled();
}
}
So once the set is merged, I'll probably try to consolidate this with a generic
simple cpu_idle() that does the above and calls the arch power saving
function. This will be only for archs that use this simple idle loop
of course.
Thanks.
Frederic Weisbecker (10):
alpha: Add missing RCU idle APIs on idle loop
cris: Add missing RCU idle APIs on idle loop
frv: Add missing RCU idle APIs on idle loop
h8300: Add missing RCU idle APIs on idle loop
m32r: Add missing RCU idle APIs on idle loop
m68k: Add missing RCU idle APIs on idle loop
mn10300: Add missing RCU idle APIs on idle loop
parisc: Add missing RCU idle APIs on idle loop
score: Add missing RCU idle APIs on idle loop
xtensa: Add missing RCU idle APIs on idle loop
arch/alpha/kernel/process.c | 6 +++++-
arch/cris/kernel/process.c | 3 +++
arch/frv/kernel/process.c | 3 +++
arch/h8300/kernel/process.c | 3 +++
arch/m32r/kernel/process.c | 3 +++
arch/m68k/kernel/process.c | 3 +++
arch/mn10300/kernel/process.c | 3 +++
arch/parisc/kernel/process.c | 3 +++
arch/score/kernel/process.c | 4 +++-
arch/xtensa/kernel/process.c | 3 +++
10 files changed, 32 insertions(+), 2 deletions(-)
--
1.7.5.4
In the old times, the whole idle task was considered
as an RCU quiescent state. But as RCU became more and
more successful overtime, some RCU read side critical
section have been added even in the code of some
architectures idle tasks, for tracing for example.
So nowadays, rcu_idle_enter() and rcu_idle_exit() must
be called by the architecture to tell RCU about the part
in the idle loop that doesn't make use of rcu read side
critical sections, typically the part that puts the CPU
in low power mode.
This is necessary for RCU to find the quiescent states in
idle in order to complete grace periods.
Add this missing pair of calls in the Alpha's idle loop.
Reported-by: Paul E. McKenney <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
Cc: Richard Henderson <[email protected]>
Cc: Ivan Kokshaysky <[email protected]>
Cc: Matt Turner <[email protected]>
Cc: alpha <[email protected]>
Cc: Paul E. McKenney <[email protected]>
Cc: 3.2.x.. <[email protected]>
---
arch/alpha/kernel/process.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/arch/alpha/kernel/process.c b/arch/alpha/kernel/process.c
index 153d3fc..2ebf7b5 100644
--- a/arch/alpha/kernel/process.c
+++ b/arch/alpha/kernel/process.c
@@ -28,6 +28,7 @@
#include <linux/tty.h>
#include <linux/console.h>
#include <linux/slab.h>
+#include <linux/rcupdate.h>
#include <asm/reg.h>
#include <asm/uaccess.h>
@@ -50,13 +51,16 @@ cpu_idle(void)
{
set_thread_flag(TIF_POLLING_NRFLAG);
+ preempt_disable();
while (1) {
/* FIXME -- EV6 and LCA45 know how to power down
the CPU. */
+ rcu_idle_enter();
while (!need_resched())
cpu_relax();
- schedule();
+ rcu_idle_exit();
+ schedule_preempt_disabled();
}
}
--
1.7.5.4
In the old times, the whole idle task was considered
as an RCU quiescent state. But as RCU became more and
more successful overtime, some RCU read side critical
section have been added even in the code of some
architectures idle tasks, for tracing for example.
So nowadays, rcu_idle_enter() and rcu_idle_exit() must
be called by the architecture to tell RCU about the part
in the idle loop that doesn't make use of rcu read side
critical sections, typically the part that puts the CPU
in low power mode.
This is necessary for RCU to find the quiescent states in
idle in order to complete grace periods.
Add this missing pair of calls in the h8300's idle loop.
Reported-by: Paul E. McKenney <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
Cc: Yoshinori Sato <[email protected]>
Cc: 3.2.x.. <[email protected]>
Cc: Paul E. McKenney <[email protected]>
---
arch/h8300/kernel/process.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/arch/h8300/kernel/process.c b/arch/h8300/kernel/process.c
index 0e9c315..f153ed1 100644
--- a/arch/h8300/kernel/process.c
+++ b/arch/h8300/kernel/process.c
@@ -36,6 +36,7 @@
#include <linux/reboot.h>
#include <linux/fs.h>
#include <linux/slab.h>
+#include <linux/rcupdate.h>
#include <asm/uaccess.h>
#include <asm/traps.h>
@@ -78,8 +79,10 @@ void (*idle)(void) = default_idle;
void cpu_idle(void)
{
while (1) {
+ rcu_idle_enter();
while (!need_resched())
idle();
+ rcu_idle_exit();
schedule_preempt_disabled();
}
}
--
1.7.5.4
In the old times, the whole idle task was considered
as an RCU quiescent state. But as RCU became more and
more successful overtime, some RCU read side critical
section have been added even in the code of some
architectures idle tasks, for tracing for example.
So nowadays, rcu_idle_enter() and rcu_idle_exit() must
be called by the architecture to tell RCU about the part
in the idle loop that doesn't make use of rcu read side
critical sections, typically the part that puts the CPU
in low power mode.
This is necessary for RCU to find the quiescent states in
idle in order to complete grace periods.
Add this missing pair of calls in the m68k's idle loop.
Reported-by: Paul E. McKenney <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Cc: m68k <[email protected]>
Cc: 3.2.x.. <[email protected]>
Cc: Paul E. McKenney <[email protected]>
---
arch/m68k/kernel/process.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/arch/m68k/kernel/process.c b/arch/m68k/kernel/process.c
index c488e3c..ac2892e 100644
--- a/arch/m68k/kernel/process.c
+++ b/arch/m68k/kernel/process.c
@@ -25,6 +25,7 @@
#include <linux/reboot.h>
#include <linux/init_task.h>
#include <linux/mqueue.h>
+#include <linux/rcupdate.h>
#include <asm/uaccess.h>
#include <asm/traps.h>
@@ -75,8 +76,10 @@ void cpu_idle(void)
{
/* endless idle loop with no priority at all */
while (1) {
+ rcu_idle_enter();
while (!need_resched())
idle();
+ rcu_idle_exit();
schedule_preempt_disabled();
}
}
--
1.7.5.4
In the old times, the whole idle task was considered
as an RCU quiescent state. But as RCU became more and
more successful overtime, some RCU read side critical
section have been added even in the code of some
architectures idle tasks, for tracing for example.
So nowadays, rcu_idle_enter() and rcu_idle_exit() must
be called by the architecture to tell RCU about the part
in the idle loop that doesn't make use of rcu read side
critical sections, typically the part that puts the CPU
in low power mode.
This is necessary for RCU to find the quiescent states in
idle in order to complete grace periods.
Add this missing pair of calls in the xtensa's idle loop.
Reported-by: Paul E. McKenney <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
Cc: Chris Zankel <[email protected]>
Cc: 3.2.x.. <[email protected]>
Cc: Paul E. McKenney <[email protected]>
---
arch/xtensa/kernel/process.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/arch/xtensa/kernel/process.c b/arch/xtensa/kernel/process.c
index 2c8d6a3..bc44311 100644
--- a/arch/xtensa/kernel/process.c
+++ b/arch/xtensa/kernel/process.c
@@ -31,6 +31,7 @@
#include <linux/mqueue.h>
#include <linux/fs.h>
#include <linux/slab.h>
+#include <linux/rcupdate.h>
#include <asm/pgtable.h>
#include <asm/uaccess.h>
@@ -110,8 +111,10 @@ void cpu_idle(void)
/* endless idle loop with no priority at all */
while (1) {
+ rcu_idle_enter();
while (!need_resched())
platform_idle();
+ rcu_idle_exit();
schedule_preempt_disabled();
}
}
--
1.7.5.4
In the old times, the whole idle task was considered
as an RCU quiescent state. But as RCU became more and
more successful overtime, some RCU read side critical
section have been added even in the code of some
architectures idle tasks, for tracing for example.
So nowadays, rcu_idle_enter() and rcu_idle_exit() must
be called by the architecture to tell RCU about the part
in the idle loop that doesn't make use of rcu read side
critical sections, typically the part that puts the CPU
in low power mode.
This is necessary for RCU to find the quiescent states in
idle in order to complete grace periods.
Add this missing pair of calls in the scores's idle loop.
Reported-by: Paul E. McKenney <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
Cc: Chen Liqin <[email protected]>
Cc: Lennox Wu <[email protected]>
Cc: 3.2.x.. <[email protected]>
Cc: Paul E. McKenney <[email protected]>
---
arch/score/kernel/process.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/arch/score/kernel/process.c b/arch/score/kernel/process.c
index 2707023..637970c 100644
--- a/arch/score/kernel/process.c
+++ b/arch/score/kernel/process.c
@@ -27,6 +27,7 @@
#include <linux/reboot.h>
#include <linux/elfcore.h>
#include <linux/pm.h>
+#include <linux/rcupdate.h>
void (*pm_power_off)(void);
EXPORT_SYMBOL(pm_power_off);
@@ -50,9 +51,10 @@ void __noreturn cpu_idle(void)
{
/* endless idle loop with no priority at all */
while (1) {
+ rcu_idle_enter();
while (!need_resched())
barrier();
-
+ rcu_idle_exit();
schedule_preempt_disabled();
}
}
--
1.7.5.4
In the old times, the whole idle task was considered
as an RCU quiescent state. But as RCU became more and
more successful overtime, some RCU read side critical
section have been added even in the code of some
architectures idle tasks, for tracing for example.
So nowadays, rcu_idle_enter() and rcu_idle_exit() must
be called by the architecture to tell RCU about the part
in the idle loop that doesn't make use of rcu read side
critical sections, typically the part that puts the CPU
in low power mode.
This is necessary for RCU to find the quiescent states in
idle in order to complete grace periods.
Add this missing pair of calls in the parisc's idle loop.
Reported-by: Paul E. McKenney <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
Cc: James E.J. Bottomley <[email protected]>
Cc: Helge Deller <[email protected]>
Cc: Parisc <[email protected]>
Cc: 3.2.x.. <[email protected]>
Cc: Paul E. McKenney <[email protected]>
---
arch/parisc/kernel/process.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/arch/parisc/kernel/process.c b/arch/parisc/kernel/process.c
index d4b94b3..c54a4db 100644
--- a/arch/parisc/kernel/process.c
+++ b/arch/parisc/kernel/process.c
@@ -48,6 +48,7 @@
#include <linux/unistd.h>
#include <linux/kallsyms.h>
#include <linux/uaccess.h>
+#include <linux/rcupdate.h>
#include <asm/io.h>
#include <asm/asm-offsets.h>
@@ -69,8 +70,10 @@ void cpu_idle(void)
/* endless idle loop with no priority at all */
while (1) {
+ rcu_idle_enter();
while (!need_resched())
barrier();
+ rcu_idle_exit();
schedule_preempt_disabled();
check_pgt_cache();
}
--
1.7.5.4
In the old times, the whole idle task was considered
as an RCU quiescent state. But as RCU became more and
more successful overtime, some RCU read side critical
section have been added even in the code of some
architectures idle tasks, for tracing for example.
So nowadays, rcu_idle_enter() and rcu_idle_exit() must
be called by the architecture to tell RCU about the part
in the idle loop that doesn't make use of rcu read side
critical sections, typically the part that puts the CPU
in low power mode.
This is necessary for RCU to find the quiescent states in
idle in order to complete grace periods.
Add this missing pair of calls in the mn10300's idle loop.
Reported-by: Paul E. McKenney <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
Cc: David Howells <[email protected]>
Cc: Koichi Yasutake <[email protected]>
Cc: 3.2.x.. <[email protected]>
Cc: Paul E. McKenney <[email protected]>
---
arch/mn10300/kernel/process.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/arch/mn10300/kernel/process.c b/arch/mn10300/kernel/process.c
index 7dab0cd..e9cceba 100644
--- a/arch/mn10300/kernel/process.c
+++ b/arch/mn10300/kernel/process.c
@@ -25,6 +25,7 @@
#include <linux/err.h>
#include <linux/fs.h>
#include <linux/slab.h>
+#include <linux/rcupdate.h>
#include <asm/uaccess.h>
#include <asm/pgtable.h>
#include <asm/io.h>
@@ -107,6 +108,7 @@ void cpu_idle(void)
{
/* endless idle loop with no priority at all */
for (;;) {
+ rcu_idle_enter();
while (!need_resched()) {
void (*idle)(void);
@@ -121,6 +123,7 @@ void cpu_idle(void)
}
idle();
}
+ rcu_idle_exit();
schedule_preempt_disabled();
}
--
1.7.5.4
In the old times, the whole idle task was considered
as an RCU quiescent state. But as RCU became more and
more successful overtime, some RCU read side critical
section have been added even in the code of some
architectures idle tasks, for tracing for example.
So nowadays, rcu_idle_enter() and rcu_idle_exit() must
be called by the architecture to tell RCU about the part
in the idle loop that doesn't make use of rcu read side
critical sections, typically the part that puts the CPU
in low power mode.
This is necessary for RCU to find the quiescent states in
idle in order to complete grace periods.
Add this missing pair of calls in the m32r's idle loop.
Reported-by: Paul E. McKenney <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
Cc: Hirokazu Takata <[email protected]>
Cc: 3.2.x.. <[email protected]>
Cc: Paul E. McKenney <[email protected]>
---
arch/m32r/kernel/process.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/arch/m32r/kernel/process.c b/arch/m32r/kernel/process.c
index 3a4a32b2..384e63f 100644
--- a/arch/m32r/kernel/process.c
+++ b/arch/m32r/kernel/process.c
@@ -26,6 +26,7 @@
#include <linux/ptrace.h>
#include <linux/unistd.h>
#include <linux/hardirq.h>
+#include <linux/rcupdate.h>
#include <asm/io.h>
#include <asm/uaccess.h>
@@ -82,6 +83,7 @@ void cpu_idle (void)
{
/* endless idle loop with no priority at all */
while (1) {
+ rcu_idle_enter();
while (!need_resched()) {
void (*idle)(void) = pm_idle;
@@ -90,6 +92,7 @@ void cpu_idle (void)
idle();
}
+ rcu_idle_exit();
schedule_preempt_disabled();
}
}
--
1.7.5.4
In the old times, the whole idle task was considered
as an RCU quiescent state. But as RCU became more and
more successful overtime, some RCU read side critical
section have been added even in the code of some
architectures idle tasks, for tracing for example.
So nowadays, rcu_idle_enter() and rcu_idle_exit() must
be called by the architecture to tell RCU about the part
in the idle loop that doesn't make use of rcu read side
critical sections, typically the part that puts the CPU
in low power mode.
This is necessary for RCU to find the quiescent states in
idle in order to complete grace periods.
Add this missing pair of calls in the Frv's idle loop.
Reported-by: Paul E. McKenney <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
Cc: David Howells <[email protected]>
Cc: 3.2.x.. <[email protected]>
Cc: Paul E. McKenney <[email protected]>
---
arch/frv/kernel/process.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/arch/frv/kernel/process.c b/arch/frv/kernel/process.c
index ff95f50..2eb7fa5 100644
--- a/arch/frv/kernel/process.c
+++ b/arch/frv/kernel/process.c
@@ -25,6 +25,7 @@
#include <linux/reboot.h>
#include <linux/interrupt.h>
#include <linux/pagemap.h>
+#include <linux/rcupdate.h>
#include <asm/asm-offsets.h>
#include <asm/uaccess.h>
@@ -69,12 +70,14 @@ void cpu_idle(void)
{
/* endless idle loop with no priority at all */
while (1) {
+ rcu_idle_enter();
while (!need_resched()) {
check_pgt_cache();
if (!frv_dma_inprogress && idle)
idle();
}
+ rcu_idle_exit();
schedule_preempt_disabled();
}
--
1.7.5.4
In the old times, the whole idle task was considered
as an RCU quiescent state. But as RCU became more and
more successful overtime, some RCU read side critical
section have been added even in the code of some
architectures idle tasks, for tracing for example.
So nowadays, rcu_idle_enter() and rcu_idle_exit() must
be called by the architecture to tell RCU about the part
in the idle loop that doesn't make use of rcu read side
critical sections, typically the part that puts the CPU
in low power mode.
This is necessary for RCU to find the quiescent states in
idle in order to complete grace periods.
Add this missing pair of calls in the Cris's idle loop.
Reported-by: Paul E. McKenney <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
Cc: Mikael Starvik <[email protected]>
Cc: Jesper Nilsson <[email protected]>
Cc: Cris <[email protected]>
Cc: 3.2.x.. <[email protected]>
Cc: Paul E. McKenney <[email protected]>
---
arch/cris/kernel/process.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/arch/cris/kernel/process.c b/arch/cris/kernel/process.c
index 66fd017..7f65be6 100644
--- a/arch/cris/kernel/process.c
+++ b/arch/cris/kernel/process.c
@@ -25,6 +25,7 @@
#include <linux/elfcore.h>
#include <linux/mqueue.h>
#include <linux/reboot.h>
+#include <linux/rcupdate.h>
//#define DEBUG
@@ -74,6 +75,7 @@ void cpu_idle (void)
{
/* endless idle loop with no priority at all */
while (1) {
+ rcu_idle_enter();
while (!need_resched()) {
void (*idle)(void);
/*
@@ -86,6 +88,7 @@ void cpu_idle (void)
idle = default_idle;
idle();
}
+ rcu_idle_exit();
schedule_preempt_disabled();
}
}
--
1.7.5.4
On Wed, Aug 22, 2012 at 6:23 PM, Frederic Weisbecker <[email protected]> wrote:
> So this fixes some potential RCU stalls in a bunch of architectures.
> When rcu_idle_enter()/rcu_idle_exit() became a requirement, we forgot
> to handle the architectures that don't support CONFIG_NO_HZ.
>
> I guess the set should be dispatched into arch maintainer trees.
I can take the m68k version, but are you sure you want it this way?
Each of them must be in mainline before they can enter stable.
> I'm sorry I haven't built tested everywhere. But the changes are
> small and need to be at least boot tested anyway.
Builds and boots fine on m68k under ARAnyM.
Acked-by: Geert Uytterhoeven <[email protected]> (for m68k)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Wed, Aug 22, 2012 at 06:23:39PM +0200, Frederic Weisbecker wrote:
> In the old times, the whole idle task was considered
> as an RCU quiescent state. But as RCU became more and
> more successful overtime, some RCU read side critical
> section have been added even in the code of some
> architectures idle tasks, for tracing for example.
>
> So nowadays, rcu_idle_enter() and rcu_idle_exit() must
> be called by the architecture to tell RCU about the part
> in the idle loop that doesn't make use of rcu read side
> critical sections, typically the part that puts the CPU
> in low power mode.
>
> This is necessary for RCU to find the quiescent states in
> idle in order to complete grace periods.
>
> Add this missing pair of calls in the Alpha's idle loop.
>
> Reported-by: Paul E. McKenney <[email protected]>
> Signed-off-by: Frederic Weisbecker <[email protected]>
> Cc: Richard Henderson <[email protected]>
> Cc: Ivan Kokshaysky <[email protected]>
> Cc: Matt Turner <[email protected]>
> Cc: alpha <[email protected]>
> Cc: Paul E. McKenney <[email protected]>
> Cc: 3.2.x.. <[email protected]>
> ---
> arch/alpha/kernel/process.c | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/arch/alpha/kernel/process.c b/arch/alpha/kernel/process.c
> index 153d3fc..2ebf7b5 100644
> --- a/arch/alpha/kernel/process.c
> +++ b/arch/alpha/kernel/process.c
> @@ -28,6 +28,7 @@
> #include <linux/tty.h>
> #include <linux/console.h>
> #include <linux/slab.h>
> +#include <linux/rcupdate.h>
>
> #include <asm/reg.h>
> #include <asm/uaccess.h>
> @@ -50,13 +51,16 @@ cpu_idle(void)
> {
> set_thread_flag(TIF_POLLING_NRFLAG);
>
> + preempt_disable();
I don't understand the above preempt_disable() not having a matching
preempt_enable() at exit, but the rest of the patches in this series
look good to me.
Thanx, Paul
> while (1) {
> /* FIXME -- EV6 and LCA45 know how to power down
> the CPU. */
>
> + rcu_idle_enter();
> while (!need_resched())
> cpu_relax();
> - schedule();
> + rcu_idle_exit();
> + schedule_preempt_disabled();
> }
> }
>
> --
> 1.7.5.4
>
On Wed, Aug 22, 2012 at 10:19:30AM -0700, Paul E. McKenney wrote:
> On Wed, Aug 22, 2012 at 06:23:39PM +0200, Frederic Weisbecker wrote:
> > In the old times, the whole idle task was considered
> > as an RCU quiescent state. But as RCU became more and
> > more successful overtime, some RCU read side critical
> > section have been added even in the code of some
> > architectures idle tasks, for tracing for example.
> >
> > So nowadays, rcu_idle_enter() and rcu_idle_exit() must
> > be called by the architecture to tell RCU about the part
> > in the idle loop that doesn't make use of rcu read side
> > critical sections, typically the part that puts the CPU
> > in low power mode.
> >
> > This is necessary for RCU to find the quiescent states in
> > idle in order to complete grace periods.
> >
> > Add this missing pair of calls in the Alpha's idle loop.
> >
> > Reported-by: Paul E. McKenney <[email protected]>
> > Signed-off-by: Frederic Weisbecker <[email protected]>
> > Cc: Richard Henderson <[email protected]>
> > Cc: Ivan Kokshaysky <[email protected]>
> > Cc: Matt Turner <[email protected]>
> > Cc: alpha <[email protected]>
> > Cc: Paul E. McKenney <[email protected]>
> > Cc: 3.2.x.. <[email protected]>
> > ---
> > arch/alpha/kernel/process.c | 6 +++++-
> > 1 files changed, 5 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/alpha/kernel/process.c b/arch/alpha/kernel/process.c
> > index 153d3fc..2ebf7b5 100644
> > --- a/arch/alpha/kernel/process.c
> > +++ b/arch/alpha/kernel/process.c
> > @@ -28,6 +28,7 @@
> > #include <linux/tty.h>
> > #include <linux/console.h>
> > #include <linux/slab.h>
> > +#include <linux/rcupdate.h>
> >
> > #include <asm/reg.h>
> > #include <asm/uaccess.h>
> > @@ -50,13 +51,16 @@ cpu_idle(void)
> > {
> > set_thread_flag(TIF_POLLING_NRFLAG);
> >
> > + preempt_disable();
>
> I don't understand the above preempt_disable() not having a matching
> preempt_enable() at exit, but the rest of the patches in this series
> look good to me.
The current code is preemptable, at least it appears so because it calls
schedule() directly. And if I call rcu_idle_enter() in a preemptable section,
I'm in trouble because I'll schedule while in extended QS.
Thus I need to disable preemption here at least until I call rcu_idle_exit().
Now this is an endless loop so there is no need to re-enable
preemption after the loop. And schedule_preempt_disabled()
takes care of enabling preemption before schedule() and redisabling
it afterward.
>
> Thanx, Paul
>
> > while (1) {
> > /* FIXME -- EV6 and LCA45 know how to power down
> > the CPU. */
> >
> > + rcu_idle_enter();
> > while (!need_resched())
> > cpu_relax();
> > - schedule();
> > + rcu_idle_exit();
> > + schedule_preempt_disabled();
> > }
> > }
> >
> > --
> > 1.7.5.4
> >
>
On Wed, Aug 22, 2012 at 07:35:45PM +0200, Frederic Weisbecker wrote:
> On Wed, Aug 22, 2012 at 10:19:30AM -0700, Paul E. McKenney wrote:
> > On Wed, Aug 22, 2012 at 06:23:39PM +0200, Frederic Weisbecker wrote:
> > > In the old times, the whole idle task was considered
> > > as an RCU quiescent state. But as RCU became more and
> > > more successful overtime, some RCU read side critical
> > > section have been added even in the code of some
> > > architectures idle tasks, for tracing for example.
> > >
> > > So nowadays, rcu_idle_enter() and rcu_idle_exit() must
> > > be called by the architecture to tell RCU about the part
> > > in the idle loop that doesn't make use of rcu read side
> > > critical sections, typically the part that puts the CPU
> > > in low power mode.
> > >
> > > This is necessary for RCU to find the quiescent states in
> > > idle in order to complete grace periods.
> > >
> > > Add this missing pair of calls in the Alpha's idle loop.
> > >
> > > Reported-by: Paul E. McKenney <[email protected]>
> > > Signed-off-by: Frederic Weisbecker <[email protected]>
> > > Cc: Richard Henderson <[email protected]>
> > > Cc: Ivan Kokshaysky <[email protected]>
> > > Cc: Matt Turner <[email protected]>
> > > Cc: alpha <[email protected]>
> > > Cc: Paul E. McKenney <[email protected]>
> > > Cc: 3.2.x.. <[email protected]>
> > > ---
> > > arch/alpha/kernel/process.c | 6 +++++-
> > > 1 files changed, 5 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/arch/alpha/kernel/process.c b/arch/alpha/kernel/process.c
> > > index 153d3fc..2ebf7b5 100644
> > > --- a/arch/alpha/kernel/process.c
> > > +++ b/arch/alpha/kernel/process.c
> > > @@ -28,6 +28,7 @@
> > > #include <linux/tty.h>
> > > #include <linux/console.h>
> > > #include <linux/slab.h>
> > > +#include <linux/rcupdate.h>
> > >
> > > #include <asm/reg.h>
> > > #include <asm/uaccess.h>
> > > @@ -50,13 +51,16 @@ cpu_idle(void)
> > > {
> > > set_thread_flag(TIF_POLLING_NRFLAG);
> > >
> > > + preempt_disable();
> >
> > I don't understand the above preempt_disable() not having a matching
> > preempt_enable() at exit, but the rest of the patches in this series
> > look good to me.
>
> The current code is preemptable, at least it appears so because it calls
> schedule() directly. And if I call rcu_idle_enter() in a preemptable section,
> I'm in trouble because I'll schedule while in extended QS.
>
> Thus I need to disable preemption here at least until I call rcu_idle_exit().
>
> Now this is an endless loop so there is no need to re-enable
> preemption after the loop. And schedule_preempt_disabled()
> takes care of enabling preemption before schedule() and redisabling
> it afterward.
>
>
> >
> > Thanx, Paul
> >
> > > while (1) {
> > > /* FIXME -- EV6 and LCA45 know how to power down
> > > the CPU. */
> > >
> > > + rcu_idle_enter();
> > > while (!need_resched())
> > > cpu_relax();
> > > - schedule();
> > > + rcu_idle_exit();
> > > + schedule_preempt_disabled();
> > > }
Understood, but what I don't understand is why you don't need a
preempt_enable() right here.
Thanx, Paul
> > > }
> > >
> > > --
> > > 1.7.5.4
> > >
> >
>
On 23/08/12 04:23, Frederic Weisbecker wrote:
> In the old times, the whole idle task was considered
> as an RCU quiescent state. But as RCU became more and
> more successful overtime, some RCU read side critical
> section have been added even in the code of some
> architectures idle tasks, for tracing for example.
Fantastic! It fixes RCU CPU stalls that we were seeing on the SMP
kernel when built for generic Alpha.
A build of glibc and running its test suite reliably triggers RCU CPU
stalls when running a kernel built for generic Alpha. I have just built
glibc and ran its test suite twice with no RCU CPU stalls with this
patch against a 3.5.2 kernel! Nice. Very nice.
I see the stable queue is CCed but I note the patch does not apply
cleanly to the 3.2.y kernel. It would be nice to have a backport of the
patches for the 3.2 stable kernel.
So feel free to add:
Tested-by: Michael Cree <[email protected]>
Cheers
Michael.
On Wed, Aug 22, 2012 at 12:01:09PM -0700, Paul E. McKenney wrote:
> > The current code is preemptable, at least it appears so because it calls
> > schedule() directly. And if I call rcu_idle_enter() in a preemptable section,
> > I'm in trouble because I'll schedule while in extended QS.
> >
> > Thus I need to disable preemption here at least until I call rcu_idle_exit().
> >
> > Now this is an endless loop so there is no need to re-enable
> > preemption after the loop. And schedule_preempt_disabled()
> > takes care of enabling preemption before schedule() and redisabling
> > it afterward.
> >
> >
> > >
> > > Thanx, Paul
> > >
> > > > while (1) {
> > > > /* FIXME -- EV6 and LCA45 know how to power down
> > > > the CPU. */
> > > >
> > > > + rcu_idle_enter();
> > > > while (!need_resched())
> > > > cpu_relax();
> > > > - schedule();
> > > > + rcu_idle_exit();
> > > > + schedule_preempt_disabled();
> > > > }
>
> Understood, but what I don't understand is why you don't need a
> preempt_enable() right here.
Look, let's inline the content of schedule_preempt_disabled(), the code
then looks like:
void cpu_idle(void)
{
set_thread_flag(TIF_POLLING_NRFLAG);
preempt_disable();
while (1) {
/* FIXME -- EV6 and LCA45 know how to power down
the CPU. */
rcu_idle_enter();
while (!need_resched())
cpu_relax();
rcu_idle_exit();
sched_preempt_enable_no_resched();
schedule();
preempt_disable();
}
}
So there is a preempt_enable() before we schedule, then we re-disable
preemption after schedule.
Now I realize cpu_idle() is supposed to be called with preemption disabled
already so I shouldn't add an explicit preempt_disable() or it's going to be worse.
But that means there is an existing bug here in alpha, it should call schedule_preempt_disabled()
instead of schedule(). cpu_idle() is called with preemption disabled on the boot CPU.
And it should as well from the secondary CPUs entry but alpha doesn't seem to do that.
So I need to fix that first. I'll respin.
On Thu, Aug 23, 2012 at 09:32:18PM +1200, Michael Cree wrote:
> On 23/08/12 04:23, Frederic Weisbecker wrote:
> > In the old times, the whole idle task was considered
> > as an RCU quiescent state. But as RCU became more and
> > more successful overtime, some RCU read side critical
> > section have been added even in the code of some
> > architectures idle tasks, for tracing for example.
>
> Fantastic! It fixes RCU CPU stalls that we were seeing on the SMP
> kernel when built for generic Alpha.
>
> A build of glibc and running its test suite reliably triggers RCU CPU
> stalls when running a kernel built for generic Alpha. I have just built
> glibc and ran its test suite twice with no RCU CPU stalls with this
> patch against a 3.5.2 kernel! Nice. Very nice.
>
> I see the stable queue is CCed but I note the patch does not apply
> cleanly to the 3.2.y kernel. It would be nice to have a backport of the
> patches for the 3.2 stable kernel.
Sure.
>
> So feel free to add:
>
> Tested-by: Michael Cree <[email protected]>
Thanks, but I need to refactor the patch, I suspect a problem with CONFIG_PREEMPT.
On Wed, Aug 22, 2012 at 07:18:04PM +0200, Geert Uytterhoeven wrote:
> On Wed, Aug 22, 2012 at 6:23 PM, Frederic Weisbecker <[email protected]> wrote:
> > So this fixes some potential RCU stalls in a bunch of architectures.
> > When rcu_idle_enter()/rcu_idle_exit() became a requirement, we forgot
> > to handle the architectures that don't support CONFIG_NO_HZ.
> >
> > I guess the set should be dispatched into arch maintainer trees.
>
> I can take the m68k version, but are you sure you want it this way?
> Each of them must be in mainline before they can enter stable.
Yeah, I was thinking the right route is for these patches to be
carried by arch maintainer who then push to Linus and then this goes
to stable.
Is that ok for you?
Otherwise I can carry the patches myself. In a tree of my own, or
Paul's or mmotm. As long as I have your ack.
Thanks.
>
> > I'm sorry I haven't built tested everywhere. But the changes are
> > small and need to be at least boot tested anyway.
>
> Builds and boots fine on m68k under ARAnyM.
> Acked-by: Geert Uytterhoeven <[email protected]> (for m68k)
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
On Thu, Aug 23, 2012 at 12:42:11PM +0200, Frederic Weisbecker wrote:
> On Wed, Aug 22, 2012 at 12:01:09PM -0700, Paul E. McKenney wrote:
> > > The current code is preemptable, at least it appears so because it calls
> > > schedule() directly. And if I call rcu_idle_enter() in a preemptable section,
> > > I'm in trouble because I'll schedule while in extended QS.
> > >
> > > Thus I need to disable preemption here at least until I call rcu_idle_exit().
> > >
> > > Now this is an endless loop so there is no need to re-enable
> > > preemption after the loop. And schedule_preempt_disabled()
> > > takes care of enabling preemption before schedule() and redisabling
> > > it afterward.
> > >
> > >
> > > >
> > > > Thanx, Paul
> > > >
> > > > > while (1) {
> > > > > /* FIXME -- EV6 and LCA45 know how to power down
> > > > > the CPU. */
> > > > >
> > > > > + rcu_idle_enter();
> > > > > while (!need_resched())
> > > > > cpu_relax();
> > > > > - schedule();
> > > > > + rcu_idle_exit();
> > > > > + schedule_preempt_disabled();
> > > > > }
> >
> > Understood, but what I don't understand is why you don't need a
> > preempt_enable() right here.
>
> Look, let's inline the content of schedule_preempt_disabled(), the code
> then looks like:
>
> void cpu_idle(void)
> {
> set_thread_flag(TIF_POLLING_NRFLAG);
>
> preempt_disable();
> while (1) {
> /* FIXME -- EV6 and LCA45 know how to power down
> the CPU. */
>
> rcu_idle_enter();
> while (!need_resched())
> cpu_relax();
> rcu_idle_exit();
>
> sched_preempt_enable_no_resched();
> schedule();
> preempt_disable();
> }
preempt_enable(); /* Why is this not needed? */
> }
>
> So there is a preempt_enable() before we schedule, then we re-disable
> preemption after schedule.
>
> Now I realize cpu_idle() is supposed to be called with preemption disabled
> already so I shouldn't add an explicit preempt_disable() or it's going to be worse.
> But that means there is an existing bug here in alpha, it should call schedule_preempt_disabled()
> instead of schedule(). cpu_idle() is called with preemption disabled on the boot CPU.
> And it should as well from the secondary CPUs entry but alpha doesn't seem to do that.
>
> So I need to fix that first. I'll respin.
OK, look forward to seeing the respin.
Thanx, Paul
Hi Frederic,
On Thu, Aug 23, 2012 at 1:02 PM, Frederic Weisbecker <[email protected]> wrote:
> On Wed, Aug 22, 2012 at 07:18:04PM +0200, Geert Uytterhoeven wrote:
>> On Wed, Aug 22, 2012 at 6:23 PM, Frederic Weisbecker <[email protected]> wrote:
>> > So this fixes some potential RCU stalls in a bunch of architectures.
>> > When rcu_idle_enter()/rcu_idle_exit() became a requirement, we forgot
>> > to handle the architectures that don't support CONFIG_NO_HZ.
>> >
>> > I guess the set should be dispatched into arch maintainer trees.
>>
>> I can take the m68k version, but are you sure you want it this way?
>> Each of them must be in mainline before they can enter stable.
>
> Yeah, I was thinking the right route is for these patches to be
> carried by arch maintainer who then push to Linus and then this goes
> to stable.
>
> Is that ok for you?
>
> Otherwise I can carry the patches myself. In a tree of my own, or
> Paul's or mmotm. As long as I have your ack.
I applied your patch to the m68k for-3.6/for-linus branch.
I'll ask Linus to pull later in the rc cycle (right now I don't have
anything else
queued for 3.6).
Still, I think it's better to just collect acks and send it to Linus
in one shot,
so it can go into stable in one shot too.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Thu, Aug 23, 2012 at 10:23:22PM +0200, Geert Uytterhoeven wrote:
> Hi Frederic,
>
> On Thu, Aug 23, 2012 at 1:02 PM, Frederic Weisbecker <[email protected]> wrote:
> > On Wed, Aug 22, 2012 at 07:18:04PM +0200, Geert Uytterhoeven wrote:
> >> On Wed, Aug 22, 2012 at 6:23 PM, Frederic Weisbecker <[email protected]> wrote:
> >> > So this fixes some potential RCU stalls in a bunch of architectures.
> >> > When rcu_idle_enter()/rcu_idle_exit() became a requirement, we forgot
> >> > to handle the architectures that don't support CONFIG_NO_HZ.
> >> >
> >> > I guess the set should be dispatched into arch maintainer trees.
> >>
> >> I can take the m68k version, but are you sure you want it this way?
> >> Each of them must be in mainline before they can enter stable.
> >
> > Yeah, I was thinking the right route is for these patches to be
> > carried by arch maintainer who then push to Linus and then this goes
> > to stable.
> >
> > Is that ok for you?
> >
> > Otherwise I can carry the patches myself. In a tree of my own, or
> > Paul's or mmotm. As long as I have your ack.
>
> I applied your patch to the m68k for-3.6/for-linus branch.
> I'll ask Linus to pull later in the rc cycle (right now I don't have
> anything else
> queued for 3.6).
> Still, I think it's better to just collect acks and send it to Linus
> in one shot,
> so it can go into stable in one shot too.
Sure I can do that if you prefer.
Thanks.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
On 8/22/2012 12:23 PM, Frederic Weisbecker wrote:
> In the old times, the whole idle task was considered
> as an RCU quiescent state. But as RCU became more and
> more successful overtime, some RCU read side critical
> section have been added even in the code of some
> architectures idle tasks, for tracing for example.
>
> So nowadays, rcu_idle_enter() and rcu_idle_exit() must
> be called by the architecture to tell RCU about the part
> in the idle loop that doesn't make use of rcu read side
> critical sections, typically the part that puts the CPU
> in low power mode.
>
> This is necessary for RCU to find the quiescent states in
> idle in order to complete grace periods.
>
> Add this missing pair of calls in the parisc's idle loop.
>
> Reported-by: Paul E. McKenney <[email protected]>
> Signed-off-by: Frederic Weisbecker <[email protected]>
> Cc: James E.J. Bottomley <[email protected]>
> Cc: Helge Deller <[email protected]>
> Cc: Parisc <[email protected]>
> Cc: 3.2.x.. <[email protected]>
> Cc: Paul E. McKenney <[email protected]>
> ---
> arch/parisc/kernel/process.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/arch/parisc/kernel/process.c b/arch/parisc/kernel/process.c
> index d4b94b3..c54a4db 100644
> --- a/arch/parisc/kernel/process.c
> +++ b/arch/parisc/kernel/process.c
> @@ -48,6 +48,7 @@
> #include <linux/unistd.h>
> #include <linux/kallsyms.h>
> #include <linux/uaccess.h>
> +#include <linux/rcupdate.h>
>
> #include <asm/io.h>
> #include <asm/asm-offsets.h>
> @@ -69,8 +70,10 @@ void cpu_idle(void)
>
> /* endless idle loop with no priority at all */
> while (1) {
> + rcu_idle_enter();
> while (!need_resched())
> barrier();
> + rcu_idle_exit();
> schedule_preempt_disabled();
> check_pgt_cache();
> }
Builds and boots fine on parisc.
Acked-by: John David Anglin<[email protected]>
Dave
--
John David Anglin [email protected]