diff mbox

[RFC,idle] Make arm, sh, and x86 stop using RCU when idle

Message ID 20120202022959.GA24442@linux-sh.org
State New
Headers show

Commit Message

Paul Mundt Feb. 2, 2012, 2:29 a.m. UTC
On Wed, Feb 01, 2012 at 04:42:53PM -0800, Paul E. McKenney wrote:
> Hello!
> 
> RCU's shiny new diagnostics (thank you, Frederic!) for using RCU when idle
> located a few problems in arm, sh, and x86.  This patch series contains
> alleged fixes for these problems.  And they are real problems -- if RCU
> believes that the CPU is idle, it is ignoring it.  Which means that the
> idle CPU can say "rcu_read_lock()" all it like, but there will be no
> useful effect.
> 
> I was tempted to break these up, but doing so is bad for bisectability.
> 
Presumably the same changes will also need to be reflected in cpuidle?

If so, here's a start:

Signed-off-by: Paul Mundt <lethal@linux-sh.org>

---

Comments

Paul E. McKenney Feb. 2, 2012, 4:58 a.m. UTC | #1
On Thu, Feb 02, 2012 at 11:29:59AM +0900, Paul Mundt wrote:
> On Wed, Feb 01, 2012 at 04:42:53PM -0800, Paul E. McKenney wrote:
> > Hello!
> > 
> > RCU's shiny new diagnostics (thank you, Frederic!) for using RCU when idle
> > located a few problems in arm, sh, and x86.  This patch series contains
> > alleged fixes for these problems.  And they are real problems -- if RCU
> > believes that the CPU is idle, it is ignoring it.  Which means that the
> > idle CPU can say "rcu_read_lock()" all it like, but there will be no
> > useful effect.
> > 
> > I was tempted to break these up, but doing so is bad for bisectability.
> > 
> Presumably the same changes will also need to be reflected in cpuidle?

Hmmm...  I do need to check that...

> If so, here's a start:
> 
> Signed-off-by: Paul Mundt <lethal@linux-sh.org>

The ARM guys are choking on this, so there might be an update.  Some
of them are thinking in terms of removing the tracing from the inner
idle loop.  Any solution works for me -- as long as there is no use
of RCU betwenen the rcu_idle_enter() and the matching rcu_idle_exit(),
both RCU and I are happy.  ;-)

							Thanx, Paul

> ---
> 
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 59f4261..97adcd4 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -18,6 +18,7 @@
>  #include <linux/ktime.h>
>  #include <linux/hrtimer.h>
>  #include <linux/module.h>
> +#include <linux/rcupdate.h>
>  #include <trace/events/power.h>
> 
>  #include "cpuidle.h"
> @@ -89,6 +90,8 @@ int cpuidle_idle_call(void)
>  	next_state = cpuidle_curr_governor->select(drv, dev);
>  	if (need_resched()) {
>  		local_irq_enable();
> +		rcu_idle_enter();
> +		rcu_idle_exit();
>  		return 0;
>  	}
> 
> @@ -96,9 +99,11 @@ int cpuidle_idle_call(void)
> 
>  	trace_power_start(POWER_CSTATE, next_state, dev->cpu);
>  	trace_cpu_idle(next_state, dev->cpu);
> +	rcu_idle_enter();
> 
>  	entered_state = target_state->enter(dev, drv, next_state);
> 
> +	rcu_idle_exit();
>  	trace_power_end(dev->cpu);
>  	trace_cpu_idle(PWR_EVENT_EXIT, dev->cpu);
> 
> @@ -173,8 +178,10 @@ static int poll_idle(struct cpuidle_device *dev,
> 
>  	t1 = ktime_get();
>  	local_irq_enable();
> +	rcu_idle_enter();
>  	while (!need_resched())
>  		cpu_relax();
> +	rcu_idle_exit();
> 
>  	t2 = ktime_get();
>  	diff = ktime_to_us(ktime_sub(t2, t1));
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
diff mbox

Patch

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 59f4261..97adcd4 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -18,6 +18,7 @@ 
 #include <linux/ktime.h>
 #include <linux/hrtimer.h>
 #include <linux/module.h>
+#include <linux/rcupdate.h>
 #include <trace/events/power.h>
 
 #include "cpuidle.h"
@@ -89,6 +90,8 @@  int cpuidle_idle_call(void)
 	next_state = cpuidle_curr_governor->select(drv, dev);
 	if (need_resched()) {
 		local_irq_enable();
+		rcu_idle_enter();
+		rcu_idle_exit();
 		return 0;
 	}
 
@@ -96,9 +99,11 @@  int cpuidle_idle_call(void)
 
 	trace_power_start(POWER_CSTATE, next_state, dev->cpu);
 	trace_cpu_idle(next_state, dev->cpu);
+	rcu_idle_enter();
 
 	entered_state = target_state->enter(dev, drv, next_state);
 
+	rcu_idle_exit();
 	trace_power_end(dev->cpu);
 	trace_cpu_idle(PWR_EVENT_EXIT, dev->cpu);
 
@@ -173,8 +178,10 @@  static int poll_idle(struct cpuidle_device *dev,
 
 	t1 = ktime_get();
 	local_irq_enable();
+	rcu_idle_enter();
 	while (!need_resched())
 		cpu_relax();
+	rcu_idle_exit();
 
 	t2 = ktime_get();
 	diff = ktime_to_us(ktime_sub(t2, t1));