Make arm, sh, and x86 stop using RCU when idle

From: "Paul E. McKenney" <[email protected]>

The idle loop is a quiscent state for RCU, which means that RCU ignores CPUs that have told RCU that they are idle via rcu_idle_enter(). There are nevertheless quite a few places where idle CPUs use RCU, most commonly indirectly via tracing. This patch fixes these problems for ARM.

Many of these bugs have been in the kernel for quite some time, but Frederic's recent change now gives warnings.

This patch takes the straightforward approach of pushing the rcu_idle_enter()/rcu_idle_exit() pair further down into the core of the idle loop.

Signed-off-by Paul E. McKenney <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

Cc Russell King <[email protected]>

Cc Andrew Victor <[email protected]>

Cc Nicolas Ferre <[email protected]>

Cc Jean-Christophe Plagniol-Villard <[email protected]>

Cc Sekhar Nori <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Kukjin Kim <[email protected]>

Cc Rob Herring <[email protected]>

Cc Sascha Hauer <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Lennert Buytenhek <[email protected]>

Cc Nicolas Pitre <[email protected]>

Cc Amit Kucheria <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Barry Song <[email protected]>

Cc Paul Mundt <[email protected]>

Cc Magnus Damm <[email protected]>

Cc Will Deacon <[email protected]>

Cc H Hartley Sweeten <[email protected]>

Cc Trinabh Gupta <[email protected]>

Cc Deepthi Dharwar <[email protected]>

Cc Len Brown <[email protected]>

Cc linux-arm-kernel at lists.infradead.org

Cc linux-samsung-soc at vger.kernel.org

Cc linux-omap at vger.kernel.org

---
 arch/arm/kernel/process.c          |    2 --
 arch/arm/mach-at91/cpuidle.c       |    3 +++
 arch/arm/mach-davinci/cpuidle.c    |    3 +++
 arch/arm/mach-exynos/common.c      |    2 ++
 arch/arm/mach-highbank/pm.c        |   12 ++++++++++++
 arch/arm/mach-imx/mm-imx3.c        |    3 +++
 arch/arm/mach-imx/pm-imx27.c       |    4 ++++
 arch/arm/mach-imx/pm-imx6q.c       |    4 ++++
 arch/arm/mach-kirkwood/cpuidle.c   |    3 +++
 arch/arm/mach-mx5/mm.c             |    3 +++
 arch/arm/mach-mx5/pm-imx5.c        |    3 +++
 arch/arm/mach-mxs/pm.c             |    4 ++++
 arch/arm/mach-omap1/pm.c           |    4 ++++
 arch/arm/mach-omap2/pm24xx.c       |    2 ++
 arch/arm/mach-omap2/pm34xx.c       |    2 ++
 arch/arm/mach-omap2/pm44xx.c       |    3 +++

 arch/arm/mach-pnx4008/pm.c         |    2 ++
 arch/arm/mach-prima2/pm.c          |    4 ++++
 arch/arm/mach-s5p64x0/common.c     |    2 ++
 arch/arm/mach-s5pc100/common.c     |    2 ++
 arch/arm/mach-s5pv210/common.c     |    2 ++
 arch/arm/mach-shmobile/cpuidle.c   |    3 +++
 arch/arm/mach-shmobile/pm-sh7372.c |    8 ++++++++
 23 files changed, 78 insertions(+), 2 deletions(-)

arch/arm/kernel/process.c +0 -1

 		leds_event(led_idle_start);
 		while (!need_resched()) {
 #ifdef CONFIG_HOTPLUG_CPU

@@ -237,7 +236,6 @@ void cpu_idle(void)
 			}
 		}
 		leds_event(led_idle_end);
-		rcu_idle_exit();
 		tick_nohz_idle_exit();
 		preempt_enable_no_resched();
 		schedule();

arch/arm/mach-at91/cpuidle.c +3 -0

index a851e6c..8feff6b 100644
@@ -20,6 +20,7 @@
 #include <asm/proc-fns.h>
 #include <linux/io.h>
 #include <linux/export.h>
+#include <linux/rcupdate.h>
 
 #include "pm.h"
 

@@ -43,6 +44,7 @@ static int at91_enter_idle(struct cpuidle_device *dev,
 
 	local_irq_disable();
 	do_gettimeofday(&before);
+	rcu_idle_enter();
 	if (index == 0)
 		/* Wait for interrupt state */
 		cpu_do_idle();

@@ -53,6 +55,7 @@ static int at91_enter_idle(struct cpuidle_device *dev,
 		cpu_do_idle();
 		sdram_selfrefresh_disable(saved_lpr);
 	}
+	rcu_idle_exit();
 	do_gettimeofday(&after);
 	local_irq_enable();
 	idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +

arch/arm/mach-davinci/cpuidle.c +3 -0

index a30c7c5..6594b7e 100644
@@ -17,6 +17,7 @@
 #include <linux/cpuidle.h>
 #include <linux/io.h>
 #include <linux/export.h>
+#include <linux/rcupdate.h>
 #include <asm/proc-fns.h>
 
 #include <mach/cpuidle.h>

@@ -90,12 +91,14 @@ static int davinci_enter_idle(struct cpuidle_device *dev,
 	local_irq_disable();
 	do_gettimeofday(&before);
 
+	rcu_idle_enter();
 	if (ops && ops->enter)
 		ops->enter(ops->flags);
 	/* Wait for interrupt state */
 	cpu_do_idle();
 	if (ops && ops->exit)
 		ops->exit(ops->flags);
+	rcu_idle_exit();
 
 	do_gettimeofday(&after);
 	local_irq_enable();

arch/arm/mach-exynos/common.c +2 -0

index c59e188..1e5844a 100644
@@ -203,8 +203,10 @@ static struct map_desc exynos4_iodesc1[] __initdata = {
 
 static void exynos_idle(void)
 {
+	rcu_idle_enter();
 	if (!need_resched())
 		cpu_do_idle();
+	rcu_idle_exit();
 
 	local_irq_enable();
 }

arch/arm/mach-highbank/pm.c +12 -0

index 33b3beb..5ecc4bc 100644
@@ -17,6 +17,7 @@
 #include <linux/init.h>
 #include <linux/io.h>
 #include <linux/suspend.h>
+#include <linux/rcupdate.h>
 
 #include <asm/proc-fns.h>
 #include <asm/smp_scu.h>

@@ -33,12 +34,23 @@ static int highbank_suspend_finish(unsigned long val)
 
 static int highbank_pm_enter(suspend_state_t state)
 {
+	/*
+	 * Some of the functions preceding the cpu_suspend() can
+	 * invoke RCU, but only in case of failure to disable preemption
+	 * or preempt_disable() misnesting.  Assume that these errors
+	 * are not committed in the following code, because RCU just
+	 * doesn't want to run on a CPU that has its caches disabled.
+	 */
+	rcu_idle_enter();
+
 	hignbank_set_pwr_suspend();
 	highbank_set_cpu_jump(0, cpu_resume);
 
 	scu_power_mode(scu_base_addr, SCU_PM_POWEROFF);
 	cpu_suspend(0, highbank_suspend_finish);
 
+	rcu_idle_exit();
+
 	return 0;
 }
 

arch/arm/mach-imx/mm-imx3.c +3 -0

index 31807d2..0945b23 100644
@@ -19,6 +19,7 @@
 #include <linux/mm.h>
 #include <linux/init.h>
 #include <linux/err.h>
+#include <linux/rcupdate.h>
 
 #include <asm/pgtable.h>
 #include <asm/hardware/cache-l2x0.h>

@@ -34,6 +35,7 @@ static void imx3_idle(void)
 {
 	unsigned long reg = 0;
 
+	rcu_idle_enter();
 	if (!need_resched())
 		__asm__ __volatile__(
 			/* disable I and D cache */

@@ -58,6 +60,7 @@ static void imx3_idle(void)
 			"orr %0, %0, #0x00000004\n"
 			"mcr p15, 0, %0, c1, c0, 0\n"
 			: "=r" (reg));
+	rcu_idle_exit();
 	local_irq_enable();
 }
 

arch/arm/mach-imx/pm-imx27.c +4 -0

index e455d2f..c4adc05 100644
@@ -10,12 +10,14 @@
 #include <linux/kernel.h>
 #include <linux/suspend.h>
 #include <linux/io.h>
+#include <linux/rcupdate.h>
 #include <mach/system.h>
 #include <mach/hardware.h>
 
 static int mx27_suspend_enter(suspend_state_t state)
 {
 	u32 cscr;
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		/* Clear MPEN and SPEN to disable MPLL/SPLL */

@@ -24,9 +26,11 @@ static int mx27_suspend_enter(suspend_state_t state)
 		__raw_writel(cscr, MX27_IO_ADDRESS(MX27_CCM_BASE_ADDR));
 		/* Executes WFI */
 		arch_idle();
+		rcu_idle_exit();
 		break;
 
 	default:
+		rcu_idle_exit();
 		return -EINVAL;
 	}
 	return 0;

arch/arm/mach-imx/pm-imx6q.c +4 -0

index f7b0c2b..b9c2589 100644
@@ -14,6 +14,7 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/suspend.h>
+#include <linux/rcupdate.h>
 #include <asm/cacheflush.h>
 #include <asm/proc-fns.h>
 #include <asm/suspend.h>

@@ -31,6 +32,7 @@ static int imx6q_suspend_finish(unsigned long val)
 
 static int imx6q_pm_enter(suspend_state_t state)
 {
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		imx6q_set_lpm(STOP_POWER_OFF);

@@ -40,8 +42,10 @@ static int imx6q_pm_enter(suspend_state_t state)
 		cpu_suspend(0, imx6q_suspend_finish);
 		imx_smp_prepare();
 		imx_gpc_post_resume();
+		rcu_idle_exit();
 		break;
 	default:
+		rcu_idle_exit();
 		return -EINVAL;
 	}
 

arch/arm/mach-kirkwood/cpuidle.c +3 -0

index 7088180..de8e317 100644
@@ -19,6 +19,7 @@
 #include <linux/cpuidle.h>
 #include <linux/io.h>
 #include <linux/export.h>
+#include <linux/rcupdate.h>
 #include <asm/proc-fns.h>
 #include <mach/kirkwood.h>
 

@@ -41,6 +42,7 @@ static int kirkwood_enter_idle(struct cpuidle_device *dev,
 
 	local_irq_disable();
 	do_gettimeofday(&before);
+	rcu_idle_enter();
 	if (index == 0)
 		/* Wait for interrupt state */
 		cpu_do_idle();

@@ -55,6 +57,7 @@ static int kirkwood_enter_idle(struct cpuidle_device *dev,
 		writel(0x7, DDR_OPERATION_BASE);
 		cpu_do_idle();
 	}
+	rcu_idle_exit();
 	do_gettimeofday(&after);
 	local_irq_enable();
 	idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +

arch/arm/mach-mx5/mm.c +3 -0

index bc17dfe..d919648 100644
@@ -14,6 +14,7 @@
 #include <linux/mm.h>
 #include <linux/init.h>
 #include <linux/clk.h>
+#include <linux/rcupdate.h>
 
 #include <asm/mach/map.h>
 

@@ -37,7 +38,9 @@ static void imx5_idle(void)
 		mx5_cpu_lp_set(WAIT_UNCLOCKED_POWER_OFF);
 		if (tzic_enable_wake())
 			goto err1;
+		rcu_idle_enter();  /* No tracing until rcu_idle_exit(). */
 		cpu_do_idle();
+		rcu_idle_exit();
 err1:
 		clk_disable(gpc_dvfs_clk);
 	}

arch/arm/mach-mx5/pm-imx5.c +3 -0

index 98052fc..b4adf42 100644
@@ -12,6 +12,7 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/err.h>
+#include <linux/rcupdate.h>
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
 #include <mach/common.h>

@@ -27,6 +28,7 @@ static int mx5_suspend_prepare(void)
 
 static int mx5_suspend_enter(suspend_state_t state)
 {
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		mx5_cpu_lp_set(STOP_POWER_OFF);

@@ -47,6 +49,7 @@ static int mx5_suspend_enter(suspend_state_t state)
 		__raw_writel(0, MXC_SRPG_EMPGC1_SRPGCR);
 	}
 	cpu_do_idle();
+	rcu_idle_exit();
 	return 0;
 }
 

arch/arm/mach-mxs/pm.c +4 -0

index fb042da..36a3a57 100644
@@ -15,16 +15,20 @@
 #include <linux/kernel.h>
 #include <linux/suspend.h>
 #include <linux/io.h>
+#include <linux/rcupdate.h>
 #include <mach/system.h>
 
 static int mxs_suspend_enter(suspend_state_t state)
 {
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		arch_idle();
+		rcu_idle_exit();
 		break;
 
 	default:
+		rcu_idle_exit();
 		return -EINVAL;
 	}
 	return 0;

arch/arm/mach-omap1/pm.c +4 -0

index 89ea20c..c661eba 100644
@@ -108,11 +108,13 @@ void omap1_pm_idle(void)
 	__u32 use_idlect1 = arm_idlect1_mask;
 	int do_sleep = 0;
 
+	rcu_idle_enter();
 	local_irq_disable();
 	local_fiq_disable();
 	if (need_resched()) {
 		local_fiq_enable();
 		local_irq_enable();
+		rcu_idle_exit();
 		return;
 	}
 

@@ -158,6 +160,7 @@ void omap1_pm_idle(void)
 
 		local_fiq_enable();
 		local_irq_enable();
+		rcu_idle_exit();
 		return;
 	}
 	omap_sram_suspend(omap_readl(ARM_IDLECT1),

@@ -165,6 +168,7 @@ void omap1_pm_idle(void)
 
 	local_fiq_enable();
 	local_irq_enable();
+	rcu_idle_exit();
 }
 
 /*

arch/arm/mach-omap2/pm24xx.c +2 -0

index b8822f8..2139e9d 100644
@@ -232,6 +232,7 @@ static int omap2_can_sleep(void)
 
 static void omap2_pm_idle(void)
 {
+	rcu_idle_enter();
 	local_irq_disable();
 	local_fiq_disable();
 

@@ -250,6 +251,7 @@ static void omap2_pm_idle(void)
 out:
 	local_fiq_enable();
 	local_irq_enable();
+	rcu_idle_exit();
 }
 
 #ifdef CONFIG_SUSPEND

arch/arm/mach-omap2/pm34xx.c +2 -0

index fc69875..58a8689 100644
@@ -427,7 +427,9 @@ static void omap3_pm_idle(void)
 	trace_power_start(POWER_CSTATE, 1, smp_processor_id());
 	trace_cpu_idle(1, smp_processor_id());
 
+	rcu_idle_enter();
 	omap_sram_idle();
+	rcu_idle_exit();
 
 	trace_power_end(smp_processor_id());
 	trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());

arch/arm/mach-omap2/pm44xx.c +3 -0

index c264ef7..038796c 100644
@@ -16,6 +16,7 @@
 #include <linux/list.h>
 #include <linux/err.h>
 #include <linux/slab.h>
+#include <linux/rcupdate.h>
 
 #include "common.h"
 #include "clockdomain.h"

@@ -178,6 +179,7 @@ static int __init pwrdms_setup(struct powerdomain *pwrdm, void *unused)
  */
 static void omap_default_idle(void)
 {
+	rcu_idle_enter();
 	local_irq_disable();
 	local_fiq_disable();
 

@@ -185,6 +187,7 @@ static void omap_default_idle(void)
 
 	local_fiq_enable();
 	local_irq_enable();
+	rcu_idle_exit();
 }
 
 /**

arch/arm/mach-pnx4008/pm.c +2 -0

index f3e60a0..0b8703f 100644
@@ -102,6 +102,7 @@ static inline void pnx4008_suspend(void)
 
 static int pnx4008_pm_enter(suspend_state_t state)
 {
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_STANDBY:
 		pnx4008_standby();

@@ -110,6 +111,7 @@ static int pnx4008_pm_enter(suspend_state_t state)
 		pnx4008_suspend();
 		break;
 	}
+	rcu_idle_exit();
 	return 0;
 }
 

arch/arm/mach-prima2/pm.c +4 -0

index 26ebb57..1e35c8a 100644
@@ -15,6 +15,7 @@
 #include <linux/of_device.h>
 #include <linux/of_platform.h>
 #include <linux/io.h>
+#include <linux/rcupdate.h>
 #include <linux/rtc/sirfsoc_rtciobrg.h>
 #include <asm/suspend.h>
 #include <asm/hardware/cache-l2x0.h>

@@ -64,6 +65,7 @@ static int sirfsoc_pre_suspend_power_off(void)
 
 static int sirfsoc_pm_enter(suspend_state_t state)
 {
+	rcu_idle_enter(); /* No good place for this: locking->lockdep->RCU. */
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		sirfsoc_pre_suspend_power_off();

@@ -73,8 +75,10 @@ static int sirfsoc_pm_enter(suspend_state_t state)
 		/* go zzz */
 		cpu_suspend(0, sirfsoc_finish_suspend);
 		outer_resume();
+		rcu_idle_exit();
 		break;
 	default:
+		rcu_idle_exit();
 		return -EINVAL;
 	}
 	return 0;

arch/arm/mach-s5p64x0/common.c +2 -0

index 52b89a3..c59a7f2 100644
@@ -146,6 +146,7 @@ static void s5p64x0_idle(void)
 {
 	unsigned long val;
 
+	rcu_idle_enter();
 	if (!need_resched()) {
 		val = __raw_readl(S5P64X0_PWR_CFG);
 		val &= ~(0x3 << 5);

@@ -154,6 +155,7 @@ static void s5p64x0_idle(void)
 
 		cpu_do_idle();
 	}
+	rcu_idle_exit();
 	local_irq_enable();
 }
 

arch/arm/mach-s5pc100/common.c +2 -0

index c909573..7bf02ff 100644
@@ -131,9 +131,11 @@ static struct map_desc s5pc100_iodesc[] __initdata = {
 
 static void s5pc100_idle(void)
 {
+	rcu_idle_enter();
 	if (!need_resched())
 		cpu_do_idle();
 
+	rcu_idle_exit();
 	local_irq_enable();
 }
 

arch/arm/mach-s5pv210/common.c +2 -0

index 9c1bcdc..ccd6b4d 100644
@@ -144,9 +144,11 @@ static struct map_desc s5pv210_iodesc[] __initdata = {
 
 static void s5pv210_idle(void)
 {
+	rcu_idle_enter();
 	if (!need_resched())
 		cpu_do_idle();
 
+	rcu_idle_exit();
 	local_irq_enable();
 }
 

arch/arm/mach-shmobile/cpuidle.c +3 -0

index 1b23342..651e92b 100644
@@ -13,6 +13,7 @@
 #include <linux/suspend.h>
 #include <linux/module.h>
 #include <linux/err.h>
+#include <linux/rcupdate.h>
 #include <asm/system.h>
 #include <asm/io.h>
 

@@ -33,6 +34,7 @@ static int shmobile_cpuidle_enter(struct cpuidle_device *dev,
 
 	before = ktime_get();
 
+	rcu_idle_enter();
 	local_irq_disable();
 	local_fiq_disable();
 

@@ -40,6 +42,7 @@ static int shmobile_cpuidle_enter(struct cpuidle_device *dev,
 
 	local_irq_enable();
 	local_fiq_enable();
+	rcu_idle_exit();
 
 	after = ktime_get();
 	dev->last_residency = ktime_to_ns(ktime_sub(after, before)) >> 10;

arch/arm/mach-shmobile/pm-sh7372.c +8 -0

index fcf8b17..e11507e 100644
@@ -21,6 +21,7 @@
 #include <linux/irq.h>
 #include <linux/bitrev.h>
 #include <linux/console.h>
+#include <linux/rcupdate.h>
 #include <asm/system.h>
 #include <asm/io.h>
 #include <asm/tlbflush.h>

@@ -520,17 +521,24 @@ static int sh7372_enter_suspend(suspend_state_t suspend_state)
 		    sh7372_a4s.genpd.status == GPD_STATE_POWER_OFF) {
 			/* enter A4S sleep with PLLC0 off */
 			pr_debug("entering A4S\n");
+			rcu_idle_enter();
 			sh7372_enter_a4s_common(0);
+			rcu_idle_exit();
 		} else {
 			/* enter A3SM sleep with PLLC0 off */
 			pr_debug("entering A3SM\n");
+			rcu_idle_enter();
 			sh7372_enter_a3sm_common(0);
+			rcu_idle_exit();
 		}
 	} else {
 		/* default to Core Standby that supports all wakeup sources */
 		pr_debug("entering Core Standby\n");
+		rcu_idle_enter();
 		sh7372_enter_core_standby();
+		rcu_idle_exit();
 	}
+
 	return 0;
 }
 

Paul,

Quoted 6 lines · Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

The idle loop is a quiscent state for RCU, which means that RCU ignores CPUs that have told RCU that they are idle via rcu_idle_enter(). There are nevertheless quite a few places where idle CPUs use RCU, most commonly indirectly via tracing. This patch fixes these problems for ARM.

Do you have some specific examples of actual problems? It's not possible to restrict tracing at this low level?

This really seems like the wrong direction as we are trying to pull common pieces up out of platform specific code.

Quoted 51 lines

Many of these bugs have been in the kernel for quite some time, but Frederic's recent change now gives warnings.

This patch takes the straightforward approach of pushing the rcu_idle_enter()/rcu_idle_exit() pair further down into the core of the idle loop.

Signed-off-by Paul E. McKenney <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

Cc Russell King <[email protected]>

Cc Andrew Victor <[email protected]>

Cc Nicolas Ferre <[email protected]>

Cc Jean-Christophe Plagniol-Villard <[email protected]>

Cc Sekhar Nori <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Kukjin Kim <[email protected]>

Cc Rob Herring <[email protected]>

Cc Sascha Hauer <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Lennert Buytenhek <[email protected]>

Cc Nicolas Pitre <[email protected]>

Cc Amit Kucheria <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Barry Song <[email protected]>

Cc Paul Mundt <[email protected]>

Cc Magnus Damm <[email protected]>

Cc Will Deacon <[email protected]>

Cc H Hartley Sweeten <[email protected]>

Cc Trinabh Gupta <[email protected]>

Cc Deepthi Dharwar <[email protected]>

Cc Len Brown <[email protected]>

Cc linux-arm-kernel at lists.infradead.org

Cc linux-samsung-soc at vger.kernel.org

Cc linux-omap at vger.kernel.org

---
 arch/arm/kernel/process.c          |    2 --
 arch/arm/mach-at91/cpuidle.c       |    3 +++
 arch/arm/mach-davinci/cpuidle.c    |    3 +++
 arch/arm/mach-exynos/common.c      |    2 ++
 arch/arm/mach-highbank/pm.c        |   12 ++++++++++++
 arch/arm/mach-imx/mm-imx3.c        |    3 +++
 arch/arm/mach-imx/pm-imx27.c       |    4 ++++
 arch/arm/mach-imx/pm-imx6q.c       |    4 ++++
 arch/arm/mach-kirkwood/cpuidle.c   |    3 +++
 arch/arm/mach-mx5/mm.c             |    3 +++
 arch/arm/mach-mx5/pm-imx5.c        |    3 +++
 arch/arm/mach-mxs/pm.c             |    4 ++++
 arch/arm/mach-omap1/pm.c           |    4 ++++
 arch/arm/mach-omap2/pm24xx.c       |    2 ++
 arch/arm/mach-omap2/pm34xx.c       |    2 ++
 arch/arm/mach-omap2/pm44xx.c       |    3 +++

Quoted 9 lines

 arch/arm/mach-pnx4008/pm.c         |    2 ++
 arch/arm/mach-prima2/pm.c          |    4 ++++
 arch/arm/mach-s5p64x0/common.c     |    2 ++
 arch/arm/mach-s5pc100/common.c     |    2 ++
 arch/arm/mach-s5pv210/common.c     |    2 ++
 arch/arm/mach-shmobile/cpuidle.c   |    3 +++
 arch/arm/mach-shmobile/pm-sh7372.c |    8 ++++++++
 23 files changed, 78 insertions(+), 2 deletions(-)

This is clearly not all platforms and you're missing some cpuidle drivers like omap. Some platform's idle code don't need this?

I'd guess this conflicts with Nico's idle clean-up which is in Russell's for-next branch now.

arch/arm/kernel/process.c void cpu_idle(void) · line 210

Quoted 9 lines

arch/arm/kernel/process.c +0 -1

index 971d65c..8241df7 100644
@@ -207,7 +207,6 @@ void cpu_idle(void)
 	/* endless idle loop with no priority at all */
 	while (1) {
 		tick_nohz_idle_enter();
-		rcu_idle_enter();

In some cases with the clean-up, there is no platform specific idle code, so rcu_idle_enter would never be called.

Rob

Quoted 574 lines

arch/arm/kernel/process.c +0 -1

 		leds_event(led_idle_start);
 		while (!need_resched()) {
 #ifdef CONFIG_HOTPLUG_CPU

@@ -237,7 +236,6 @@ void cpu_idle(void)
 			}
 		}
 		leds_event(led_idle_end);
-		rcu_idle_exit();
 		tick_nohz_idle_exit();
 		preempt_enable_no_resched();
 		schedule();

arch/arm/mach-at91/cpuidle.c +3 -0

index a851e6c..8feff6b 100644
@@ -20,6 +20,7 @@
 #include <asm/proc-fns.h>
 #include <linux/io.h>
 #include <linux/export.h>
+#include <linux/rcupdate.h>
 
 #include "pm.h"
 

@@ -43,6 +44,7 @@ static int at91_enter_idle(struct cpuidle_device *dev,
 
 	local_irq_disable();
 	do_gettimeofday(&before);
+	rcu_idle_enter();
 	if (index == 0)
 		/* Wait for interrupt state */
 		cpu_do_idle();

@@ -53,6 +55,7 @@ static int at91_enter_idle(struct cpuidle_device *dev,
 		cpu_do_idle();
 		sdram_selfrefresh_disable(saved_lpr);
 	}
+	rcu_idle_exit();
 	do_gettimeofday(&after);
 	local_irq_enable();
 	idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +

arch/arm/mach-davinci/cpuidle.c +3 -0

index a30c7c5..6594b7e 100644
@@ -17,6 +17,7 @@
 #include <linux/cpuidle.h>
 #include <linux/io.h>
 #include <linux/export.h>
+#include <linux/rcupdate.h>
 #include <asm/proc-fns.h>
 
 #include <mach/cpuidle.h>

@@ -90,12 +91,14 @@ static int davinci_enter_idle(struct cpuidle_device *dev,
 	local_irq_disable();
 	do_gettimeofday(&before);
 
+	rcu_idle_enter();
 	if (ops && ops->enter)
 		ops->enter(ops->flags);
 	/* Wait for interrupt state */
 	cpu_do_idle();
 	if (ops && ops->exit)
 		ops->exit(ops->flags);
+	rcu_idle_exit();
 
 	do_gettimeofday(&after);
 	local_irq_enable();

arch/arm/mach-exynos/common.c +2 -0

index c59e188..1e5844a 100644
@@ -203,8 +203,10 @@ static struct map_desc exynos4_iodesc1[] __initdata = {
 
 static void exynos_idle(void)
 {
+	rcu_idle_enter();
 	if (!need_resched())
 		cpu_do_idle();
+	rcu_idle_exit();
 
 	local_irq_enable();
 }

arch/arm/mach-highbank/pm.c +12 -0

index 33b3beb..5ecc4bc 100644
@@ -17,6 +17,7 @@
 #include <linux/init.h>
 #include <linux/io.h>
 #include <linux/suspend.h>
+#include <linux/rcupdate.h>
 
 #include <asm/proc-fns.h>
 #include <asm/smp_scu.h>

@@ -33,12 +34,23 @@ static int highbank_suspend_finish(unsigned long val)
 
 static int highbank_pm_enter(suspend_state_t state)
 {
+	/*
+	 * Some of the functions preceding the cpu_suspend() can
+	 * invoke RCU, but only in case of failure to disable preemption
+	 * or preempt_disable() misnesting.  Assume that these errors
+	 * are not committed in the following code, because RCU just
+	 * doesn't want to run on a CPU that has its caches disabled.
+	 */
+	rcu_idle_enter();
+
 	hignbank_set_pwr_suspend();
 	highbank_set_cpu_jump(0, cpu_resume);
 
 	scu_power_mode(scu_base_addr, SCU_PM_POWEROFF);
 	cpu_suspend(0, highbank_suspend_finish);
 
+	rcu_idle_exit();
+
 	return 0;
 }
 

arch/arm/mach-imx/mm-imx3.c +3 -0

index 31807d2..0945b23 100644
@@ -19,6 +19,7 @@
 #include <linux/mm.h>
 #include <linux/init.h>
 #include <linux/err.h>
+#include <linux/rcupdate.h>
 
 #include <asm/pgtable.h>
 #include <asm/hardware/cache-l2x0.h>

@@ -34,6 +35,7 @@ static void imx3_idle(void)
 {
 	unsigned long reg = 0;
 
+	rcu_idle_enter();
 	if (!need_resched())
 		__asm__ __volatile__(
 			/* disable I and D cache */

@@ -58,6 +60,7 @@ static void imx3_idle(void)
 			"orr %0, %0, #0x00000004\n"
 			"mcr p15, 0, %0, c1, c0, 0\n"
 			: "=r" (reg));
+	rcu_idle_exit();
 	local_irq_enable();
 }
 

arch/arm/mach-imx/pm-imx27.c +4 -0

index e455d2f..c4adc05 100644
@@ -10,12 +10,14 @@
 #include <linux/kernel.h>
 #include <linux/suspend.h>
 #include <linux/io.h>
+#include <linux/rcupdate.h>
 #include <mach/system.h>
 #include <mach/hardware.h>
 
 static int mx27_suspend_enter(suspend_state_t state)
 {
 	u32 cscr;
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		/* Clear MPEN and SPEN to disable MPLL/SPLL */

@@ -24,9 +26,11 @@ static int mx27_suspend_enter(suspend_state_t state)
 		__raw_writel(cscr, MX27_IO_ADDRESS(MX27_CCM_BASE_ADDR));
 		/* Executes WFI */
 		arch_idle();
+		rcu_idle_exit();
 		break;
 
 	default:
+		rcu_idle_exit();
 		return -EINVAL;
 	}
 	return 0;

arch/arm/mach-imx/pm-imx6q.c +4 -0

index f7b0c2b..b9c2589 100644
@@ -14,6 +14,7 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/suspend.h>
+#include <linux/rcupdate.h>
 #include <asm/cacheflush.h>
 #include <asm/proc-fns.h>
 #include <asm/suspend.h>

@@ -31,6 +32,7 @@ static int imx6q_suspend_finish(unsigned long val)
 
 static int imx6q_pm_enter(suspend_state_t state)
 {
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		imx6q_set_lpm(STOP_POWER_OFF);

@@ -40,8 +42,10 @@ static int imx6q_pm_enter(suspend_state_t state)
 		cpu_suspend(0, imx6q_suspend_finish);
 		imx_smp_prepare();
 		imx_gpc_post_resume();
+		rcu_idle_exit();
 		break;
 	default:
+		rcu_idle_exit();
 		return -EINVAL;
 	}
 

arch/arm/mach-kirkwood/cpuidle.c +3 -0

index 7088180..de8e317 100644
@@ -19,6 +19,7 @@
 #include <linux/cpuidle.h>
 #include <linux/io.h>
 #include <linux/export.h>
+#include <linux/rcupdate.h>
 #include <asm/proc-fns.h>
 #include <mach/kirkwood.h>
 

@@ -41,6 +42,7 @@ static int kirkwood_enter_idle(struct cpuidle_device *dev,
 
 	local_irq_disable();
 	do_gettimeofday(&before);
+	rcu_idle_enter();
 	if (index == 0)
 		/* Wait for interrupt state */
 		cpu_do_idle();

@@ -55,6 +57,7 @@ static int kirkwood_enter_idle(struct cpuidle_device *dev,
 		writel(0x7, DDR_OPERATION_BASE);
 		cpu_do_idle();
 	}
+	rcu_idle_exit();
 	do_gettimeofday(&after);
 	local_irq_enable();
 	idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +

arch/arm/mach-mx5/mm.c +3 -0

index bc17dfe..d919648 100644
@@ -14,6 +14,7 @@
 #include <linux/mm.h>
 #include <linux/init.h>
 #include <linux/clk.h>
+#include <linux/rcupdate.h>
 
 #include <asm/mach/map.h>
 

@@ -37,7 +38,9 @@ static void imx5_idle(void)
 		mx5_cpu_lp_set(WAIT_UNCLOCKED_POWER_OFF);
 		if (tzic_enable_wake())
 			goto err1;
+		rcu_idle_enter();  /* No tracing until rcu_idle_exit(). */
 		cpu_do_idle();
+		rcu_idle_exit();
 err1:
 		clk_disable(gpc_dvfs_clk);
 	}

arch/arm/mach-mx5/pm-imx5.c +3 -0

index 98052fc..b4adf42 100644
@@ -12,6 +12,7 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/err.h>
+#include <linux/rcupdate.h>
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
 #include <mach/common.h>

@@ -27,6 +28,7 @@ static int mx5_suspend_prepare(void)
 
 static int mx5_suspend_enter(suspend_state_t state)
 {
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		mx5_cpu_lp_set(STOP_POWER_OFF);

@@ -47,6 +49,7 @@ static int mx5_suspend_enter(suspend_state_t state)
 		__raw_writel(0, MXC_SRPG_EMPGC1_SRPGCR);
 	}
 	cpu_do_idle();
+	rcu_idle_exit();
 	return 0;
 }
 

arch/arm/mach-mxs/pm.c +4 -0

index fb042da..36a3a57 100644
@@ -15,16 +15,20 @@
 #include <linux/kernel.h>
 #include <linux/suspend.h>
 #include <linux/io.h>
+#include <linux/rcupdate.h>
 #include <mach/system.h>
 
 static int mxs_suspend_enter(suspend_state_t state)
 {
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		arch_idle();
+		rcu_idle_exit();
 		break;
 
 	default:
+		rcu_idle_exit();
 		return -EINVAL;
 	}
 	return 0;

arch/arm/mach-omap1/pm.c +4 -0

index 89ea20c..c661eba 100644
@@ -108,11 +108,13 @@ void omap1_pm_idle(void)
 	__u32 use_idlect1 = arm_idlect1_mask;
 	int do_sleep = 0;
 
+	rcu_idle_enter();
 	local_irq_disable();
 	local_fiq_disable();
 	if (need_resched()) {
 		local_fiq_enable();
 		local_irq_enable();
+		rcu_idle_exit();
 		return;
 	}
 

@@ -158,6 +160,7 @@ void omap1_pm_idle(void)
 
 		local_fiq_enable();
 		local_irq_enable();
+		rcu_idle_exit();
 		return;
 	}
 	omap_sram_suspend(omap_readl(ARM_IDLECT1),

@@ -165,6 +168,7 @@ void omap1_pm_idle(void)
 
 	local_fiq_enable();
 	local_irq_enable();
+	rcu_idle_exit();
 }
 
 /*

arch/arm/mach-omap2/pm24xx.c +2 -0

index b8822f8..2139e9d 100644
@@ -232,6 +232,7 @@ static int omap2_can_sleep(void)
 
 static void omap2_pm_idle(void)
 {
+	rcu_idle_enter();
 	local_irq_disable();
 	local_fiq_disable();
 

@@ -250,6 +251,7 @@ static void omap2_pm_idle(void)
 out:
 	local_fiq_enable();
 	local_irq_enable();
+	rcu_idle_exit();
 }
 
 #ifdef CONFIG_SUSPEND

arch/arm/mach-omap2/pm34xx.c +2 -0

index fc69875..58a8689 100644
@@ -427,7 +427,9 @@ static void omap3_pm_idle(void)
 	trace_power_start(POWER_CSTATE, 1, smp_processor_id());
 	trace_cpu_idle(1, smp_processor_id());
 
+	rcu_idle_enter();
 	omap_sram_idle();
+	rcu_idle_exit();
 
 	trace_power_end(smp_processor_id());
 	trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());

arch/arm/mach-omap2/pm44xx.c +3 -0

index c264ef7..038796c 100644
@@ -16,6 +16,7 @@
 #include <linux/list.h>
 #include <linux/err.h>
 #include <linux/slab.h>
+#include <linux/rcupdate.h>
 
 #include "common.h"
 #include "clockdomain.h"

@@ -178,6 +179,7 @@ static int __init pwrdms_setup(struct powerdomain *pwrdm, void *unused)
  */
 static void omap_default_idle(void)
 {
+	rcu_idle_enter();
 	local_irq_disable();
 	local_fiq_disable();
 

@@ -185,6 +187,7 @@ static void omap_default_idle(void)
 
 	local_fiq_enable();
 	local_irq_enable();
+	rcu_idle_exit();
 }
 
 /**

arch/arm/mach-pnx4008/pm.c +2 -0

index f3e60a0..0b8703f 100644
@@ -102,6 +102,7 @@ static inline void pnx4008_suspend(void)
 
 static int pnx4008_pm_enter(suspend_state_t state)
 {
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_STANDBY:
 		pnx4008_standby();

@@ -110,6 +111,7 @@ static int pnx4008_pm_enter(suspend_state_t state)
 		pnx4008_suspend();
 		break;
 	}
+	rcu_idle_exit();
 	return 0;
 }
 

arch/arm/mach-prima2/pm.c +4 -0

index 26ebb57..1e35c8a 100644
@@ -15,6 +15,7 @@
 #include <linux/of_device.h>
 #include <linux/of_platform.h>
 #include <linux/io.h>
+#include <linux/rcupdate.h>
 #include <linux/rtc/sirfsoc_rtciobrg.h>
 #include <asm/suspend.h>
 #include <asm/hardware/cache-l2x0.h>

@@ -64,6 +65,7 @@ static int sirfsoc_pre_suspend_power_off(void)
 
 static int sirfsoc_pm_enter(suspend_state_t state)
 {
+	rcu_idle_enter(); /* No good place for this: locking->lockdep->RCU. */
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		sirfsoc_pre_suspend_power_off();

@@ -73,8 +75,10 @@ static int sirfsoc_pm_enter(suspend_state_t state)
 		/* go zzz */
 		cpu_suspend(0, sirfsoc_finish_suspend);
 		outer_resume();
+		rcu_idle_exit();
 		break;
 	default:
+		rcu_idle_exit();
 		return -EINVAL;
 	}
 	return 0;

arch/arm/mach-s5p64x0/common.c +2 -0

index 52b89a3..c59a7f2 100644
@@ -146,6 +146,7 @@ static void s5p64x0_idle(void)
 {
 	unsigned long val;
 
+	rcu_idle_enter();
 	if (!need_resched()) {
 		val = __raw_readl(S5P64X0_PWR_CFG);
 		val &= ~(0x3 << 5);

@@ -154,6 +155,7 @@ static void s5p64x0_idle(void)
 
 		cpu_do_idle();
 	}
+	rcu_idle_exit();
 	local_irq_enable();
 }
 

arch/arm/mach-s5pc100/common.c +2 -0

index c909573..7bf02ff 100644
@@ -131,9 +131,11 @@ static struct map_desc s5pc100_iodesc[] __initdata = {
 
 static void s5pc100_idle(void)
 {
+	rcu_idle_enter();
 	if (!need_resched())
 		cpu_do_idle();
 
+	rcu_idle_exit();
 	local_irq_enable();
 }
 

arch/arm/mach-s5pv210/common.c +2 -0

index 9c1bcdc..ccd6b4d 100644
@@ -144,9 +144,11 @@ static struct map_desc s5pv210_iodesc[] __initdata = {
 
 static void s5pv210_idle(void)
 {
+	rcu_idle_enter();
 	if (!need_resched())
 		cpu_do_idle();
 
+	rcu_idle_exit();
 	local_irq_enable();
 }
 

arch/arm/mach-shmobile/cpuidle.c +3 -0

index 1b23342..651e92b 100644
@@ -13,6 +13,7 @@
 #include <linux/suspend.h>
 #include <linux/module.h>
 #include <linux/err.h>
+#include <linux/rcupdate.h>
 #include <asm/system.h>
 #include <asm/io.h>
 

@@ -33,6 +34,7 @@ static int shmobile_cpuidle_enter(struct cpuidle_device *dev,
 
 	before = ktime_get();
 
+	rcu_idle_enter();
 	local_irq_disable();
 	local_fiq_disable();
 

@@ -40,6 +42,7 @@ static int shmobile_cpuidle_enter(struct cpuidle_device *dev,
 
 	local_irq_enable();
 	local_fiq_enable();
+	rcu_idle_exit();
 
 	after = ktime_get();
 	dev->last_residency = ktime_to_ns(ktime_sub(after, before)) >> 10;

arch/arm/mach-shmobile/pm-sh7372.c +8 -0

index fcf8b17..e11507e 100644
@@ -21,6 +21,7 @@
 #include <linux/irq.h>
 #include <linux/bitrev.h>
 #include <linux/console.h>
+#include <linux/rcupdate.h>
 #include <asm/system.h>
 #include <asm/io.h>
 #include <asm/tlbflush.h>

@@ -520,17 +521,24 @@ static int sh7372_enter_suspend(suspend_state_t suspend_state)
 		    sh7372_a4s.genpd.status == GPD_STATE_POWER_OFF) {
 			/* enter A4S sleep with PLLC0 off */
 			pr_debug("entering A4S\n");
+			rcu_idle_enter();
 			sh7372_enter_a4s_common(0);
+			rcu_idle_exit();
 		} else {
 			/* enter A3SM sleep with PLLC0 off */
 			pr_debug("entering A3SM\n");
+			rcu_idle_enter();
 			sh7372_enter_a3sm_common(0);
+			rcu_idle_exit();
 		}
 	} else {
 		/* default to Core Standby that supports all wakeup sources */
 		pr_debug("entering Core Standby\n");
+		rcu_idle_enter();
 		sh7372_enter_core_standby();
+		rcu_idle_exit();
 	}
+
 	return 0;
 }
 

From: "Paul E. McKenney" <[email protected]>

The idle loop is a quiscent state for RCU, which means that RCU ignores CPUs that have told RCU that they are idle via rcu_idle_enter(). There are nevertheless quite a few places where idle CPUs use RCU, most commonly indirectly via tracing. This patch fixes these problems for ARM.

Many of these bugs have been in the kernel for quite some time, but Frederic's recent change now gives warnings.

This patch takes the straightforward approach of pushing the rcu_idle_enter()/rcu_idle_exit() pair further down into the core of the idle loop.

Signed-off-by Paul E. McKenney <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

Cc Russell King <[email protected]>

Cc Andrew Victor <[email protected]>

Cc Nicolas Ferre <[email protected]>

Cc Jean-Christophe Plagniol-Villard <[email protected]>

Cc Sekhar Nori <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Kukjin Kim <[email protected]>

Cc Rob Herring <[email protected]>

Cc Sascha Hauer <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Lennert Buytenhek <[email protected]>

Cc Nicolas Pitre <[email protected]>

Cc Amit Kucheria <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Barry Song <[email protected]>

Cc Paul Mundt <[email protected]>

Cc Magnus Damm <[email protected]>

Cc Will Deacon <[email protected]>

Cc H Hartley Sweeten <[email protected]>

Cc Trinabh Gupta <[email protected]>

Cc Deepthi Dharwar <[email protected]>

Cc Len Brown <[email protected]>

Cc linux-arm-kernel at lists.infradead.org

Cc linux-samsung-soc at vger.kernel.org

Cc linux-omap at vger.kernel.org

---
 arch/arm/kernel/process.c          |    2 --
 arch/arm/mach-at91/cpuidle.c       |    3 +++
 arch/arm/mach-davinci/cpuidle.c    |    3 +++
 arch/arm/mach-exynos/common.c      |    2 ++
 arch/arm/mach-highbank/pm.c        |   12 ++++++++++++
 arch/arm/mach-imx/mm-imx3.c        |    3 +++
 arch/arm/mach-imx/pm-imx27.c       |    4 ++++
 arch/arm/mach-imx/pm-imx6q.c       |    4 ++++
 arch/arm/mach-kirkwood/cpuidle.c   |    3 +++
 arch/arm/mach-mx5/mm.c             |    3 +++
 arch/arm/mach-mx5/pm-imx5.c        |    3 +++
 arch/arm/mach-mxs/pm.c             |    4 ++++
 arch/arm/mach-omap1/pm.c           |    4 ++++
 arch/arm/mach-omap2/pm24xx.c       |    2 ++
 arch/arm/mach-omap2/pm34xx.c       |    2 ++
 arch/arm/mach-omap2/pm44xx.c       |    3 +++
 arch/arm/mach-pnx4008/pm.c         |    2 ++
 arch/arm/mach-prima2/pm.c          |    4 ++++
 arch/arm/mach-s5p64x0/common.c     |    2 ++
 arch/arm/mach-s5pc100/common.c     |    2 ++
 arch/arm/mach-s5pv210/common.c     |    2 ++
 arch/arm/mach-shmobile/cpuidle.c   |    3 +++
 arch/arm/mach-shmobile/pm-sh7372.c |    8 ++++++++
 23 files changed, 78 insertions(+), 2 deletions(-)

arch/arm/kernel/process.c +0 -2

index 971d65c..8241df7 100644
@@ -207,7 +207,6 @@ void cpu_idle(void)
 	/* endless idle loop with no priority at all */
 	while (1) {
 		tick_nohz_idle_enter();
-		rcu_idle_enter();
 		leds_event(led_idle_start);
 		while (!need_resched()) {
 #ifdef CONFIG_HOTPLUG_CPU

@@ -237,7 +236,6 @@ void cpu_idle(void)
 			}
 		}
 		leds_event(led_idle_end);
-		rcu_idle_exit();
 		tick_nohz_idle_exit();
 		preempt_enable_no_resched();
 		schedule();

arch/arm/mach-at91/cpuidle.c +3 -0

index a851e6c..8feff6b 100644
@@ -20,6 +20,7 @@
 #include <asm/proc-fns.h>
 #include <linux/io.h>
 #include <linux/export.h>
+#include <linux/rcupdate.h>
 
 #include "pm.h"
 

@@ -43,6 +44,7 @@ static int at91_enter_idle(struct cpuidle_device *dev,
 
 	local_irq_disable();
 	do_gettimeofday(&before);
+	rcu_idle_enter();
 	if (index == 0)
 		/* Wait for interrupt state */
 		cpu_do_idle();

@@ -53,6 +55,7 @@ static int at91_enter_idle(struct cpuidle_device *dev,
 		cpu_do_idle();
 		sdram_selfrefresh_disable(saved_lpr);
 	}
+	rcu_idle_exit();
 	do_gettimeofday(&after);
 	local_irq_enable();
 	idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +

arch/arm/mach-davinci/cpuidle.c +3 -0

index a30c7c5..6594b7e 100644
@@ -17,6 +17,7 @@
 #include <linux/cpuidle.h>
 #include <linux/io.h>
 #include <linux/export.h>
+#include <linux/rcupdate.h>
 #include <asm/proc-fns.h>
 
 #include <mach/cpuidle.h>

@@ -90,12 +91,14 @@ static int davinci_enter_idle(struct cpuidle_device *dev,
 	local_irq_disable();
 	do_gettimeofday(&before);
 
+	rcu_idle_enter();
 	if (ops && ops->enter)
 		ops->enter(ops->flags);
 	/* Wait for interrupt state */
 	cpu_do_idle();
 	if (ops && ops->exit)
 		ops->exit(ops->flags);
+	rcu_idle_exit();
 
 	do_gettimeofday(&after);
 	local_irq_enable();

arch/arm/mach-exynos/common.c +2 -0

index c59e188..1e5844a 100644
@@ -203,8 +203,10 @@ static struct map_desc exynos4_iodesc1[] __initdata = {
 
 static void exynos_idle(void)
 {
+	rcu_idle_enter();
 	if (!need_resched())
 		cpu_do_idle();
+	rcu_idle_exit();
 
 	local_irq_enable();
 }

arch/arm/mach-highbank/pm.c +12 -0

index 33b3beb..5ecc4bc 100644
@@ -17,6 +17,7 @@
 #include <linux/init.h>
 #include <linux/io.h>
 #include <linux/suspend.h>
+#include <linux/rcupdate.h>
 
 #include <asm/proc-fns.h>
 #include <asm/smp_scu.h>

@@ -33,12 +34,23 @@ static int highbank_suspend_finish(unsigned long val)
 
 static int highbank_pm_enter(suspend_state_t state)
 {
+	/*
+	 * Some of the functions preceding the cpu_suspend() can
+	 * invoke RCU, but only in case of failure to disable preemption
+	 * or preempt_disable() misnesting.  Assume that these errors
+	 * are not committed in the following code, because RCU just
+	 * doesn't want to run on a CPU that has its caches disabled.
+	 */
+	rcu_idle_enter();
+
 	hignbank_set_pwr_suspend();
 	highbank_set_cpu_jump(0, cpu_resume);
 
 	scu_power_mode(scu_base_addr, SCU_PM_POWEROFF);
 	cpu_suspend(0, highbank_suspend_finish);
 
+	rcu_idle_exit();
+
 	return 0;
 }
 

arch/arm/mach-imx/mm-imx3.c +3 -0

index 31807d2..0945b23 100644
@@ -19,6 +19,7 @@
 #include <linux/mm.h>
 #include <linux/init.h>
 #include <linux/err.h>
+#include <linux/rcupdate.h>
 
 #include <asm/pgtable.h>
 #include <asm/hardware/cache-l2x0.h>

@@ -34,6 +35,7 @@ static void imx3_idle(void)
 {
 	unsigned long reg = 0;
 
+	rcu_idle_enter();
 	if (!need_resched())
 		__asm__ __volatile__(
 			/* disable I and D cache */

@@ -58,6 +60,7 @@ static void imx3_idle(void)
 			"orr %0, %0, #0x00000004\n"
 			"mcr p15, 0, %0, c1, c0, 0\n"
 			: "=r" (reg));
+	rcu_idle_exit();
 	local_irq_enable();
 }
 

arch/arm/mach-imx/pm-imx27.c +4 -0

index e455d2f..c4adc05 100644
@@ -10,12 +10,14 @@
 #include <linux/kernel.h>
 #include <linux/suspend.h>
 #include <linux/io.h>
+#include <linux/rcupdate.h>
 #include <mach/system.h>
 #include <mach/hardware.h>
 
 static int mx27_suspend_enter(suspend_state_t state)
 {
 	u32 cscr;
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		/* Clear MPEN and SPEN to disable MPLL/SPLL */

@@ -24,9 +26,11 @@ static int mx27_suspend_enter(suspend_state_t state)
 		__raw_writel(cscr, MX27_IO_ADDRESS(MX27_CCM_BASE_ADDR));
 		/* Executes WFI */
 		arch_idle();
+		rcu_idle_exit();
 		break;
 
 	default:
+		rcu_idle_exit();
 		return -EINVAL;
 	}
 	return 0;

arch/arm/mach-imx/pm-imx6q.c +4 -0

index f7b0c2b..b9c2589 100644
@@ -14,6 +14,7 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/suspend.h>
+#include <linux/rcupdate.h>
 #include <asm/cacheflush.h>
 #include <asm/proc-fns.h>
 #include <asm/suspend.h>

@@ -31,6 +32,7 @@ static int imx6q_suspend_finish(unsigned long val)
 
 static int imx6q_pm_enter(suspend_state_t state)
 {
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		imx6q_set_lpm(STOP_POWER_OFF);

@@ -40,8 +42,10 @@ static int imx6q_pm_enter(suspend_state_t state)
 		cpu_suspend(0, imx6q_suspend_finish);
 		imx_smp_prepare();
 		imx_gpc_post_resume();
+		rcu_idle_exit();
 		break;
 	default:
+		rcu_idle_exit();
 		return -EINVAL;
 	}
 

arch/arm/mach-kirkwood/cpuidle.c +3 -0

index 7088180..de8e317 100644
@@ -19,6 +19,7 @@
 #include <linux/cpuidle.h>
 #include <linux/io.h>
 #include <linux/export.h>
+#include <linux/rcupdate.h>
 #include <asm/proc-fns.h>
 #include <mach/kirkwood.h>
 

@@ -41,6 +42,7 @@ static int kirkwood_enter_idle(struct cpuidle_device *dev,
 
 	local_irq_disable();
 	do_gettimeofday(&before);
+	rcu_idle_enter();
 	if (index == 0)
 		/* Wait for interrupt state */
 		cpu_do_idle();

@@ -55,6 +57,7 @@ static int kirkwood_enter_idle(struct cpuidle_device *dev,
 		writel(0x7, DDR_OPERATION_BASE);
 		cpu_do_idle();
 	}
+	rcu_idle_exit();
 	do_gettimeofday(&after);
 	local_irq_enable();
 	idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +

arch/arm/mach-mx5/mm.c +3 -0

index bc17dfe..d919648 100644
@@ -14,6 +14,7 @@
 #include <linux/mm.h>
 #include <linux/init.h>
 #include <linux/clk.h>
+#include <linux/rcupdate.h>
 
 #include <asm/mach/map.h>
 

@@ -37,7 +38,9 @@ static void imx5_idle(void)
 		mx5_cpu_lp_set(WAIT_UNCLOCKED_POWER_OFF);
 		if (tzic_enable_wake())
 			goto err1;
+		rcu_idle_enter();  /* No tracing until rcu_idle_exit(). */
 		cpu_do_idle();
+		rcu_idle_exit();
 err1:
 		clk_disable(gpc_dvfs_clk);
 	}

arch/arm/mach-mx5/pm-imx5.c +3 -0

index 98052fc..b4adf42 100644
@@ -12,6 +12,7 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/err.h>
+#include <linux/rcupdate.h>
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
 #include <mach/common.h>

@@ -27,6 +28,7 @@ static int mx5_suspend_prepare(void)
 
 static int mx5_suspend_enter(suspend_state_t state)
 {
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		mx5_cpu_lp_set(STOP_POWER_OFF);

@@ -47,6 +49,7 @@ static int mx5_suspend_enter(suspend_state_t state)
 		__raw_writel(0, MXC_SRPG_EMPGC1_SRPGCR);
 	}
 	cpu_do_idle();
+	rcu_idle_exit();
 	return 0;
 }
 

arch/arm/mach-mxs/pm.c +4 -0

index fb042da..36a3a57 100644
@@ -15,16 +15,20 @@
 #include <linux/kernel.h>
 #include <linux/suspend.h>
 #include <linux/io.h>
+#include <linux/rcupdate.h>
 #include <mach/system.h>
 
 static int mxs_suspend_enter(suspend_state_t state)
 {
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		arch_idle();
+		rcu_idle_exit();
 		break;
 
 	default:
+		rcu_idle_exit();
 		return -EINVAL;
 	}
 	return 0;

arch/arm/mach-omap1/pm.c +4 -0

index 89ea20c..c661eba 100644
@@ -108,11 +108,13 @@ void omap1_pm_idle(void)
 	__u32 use_idlect1 = arm_idlect1_mask;
 	int do_sleep = 0;
 
+	rcu_idle_enter();
 	local_irq_disable();
 	local_fiq_disable();
 	if (need_resched()) {
 		local_fiq_enable();
 		local_irq_enable();
+		rcu_idle_exit();
 		return;
 	}
 

@@ -158,6 +160,7 @@ void omap1_pm_idle(void)
 
 		local_fiq_enable();
 		local_irq_enable();
+		rcu_idle_exit();
 		return;
 	}
 	omap_sram_suspend(omap_readl(ARM_IDLECT1),

@@ -165,6 +168,7 @@ void omap1_pm_idle(void)
 
 	local_fiq_enable();
 	local_irq_enable();
+	rcu_idle_exit();
 }
 
 /*

arch/arm/mach-omap2/pm24xx.c +2 -0

index b8822f8..2139e9d 100644
@@ -232,6 +232,7 @@ static int omap2_can_sleep(void)
 
 static void omap2_pm_idle(void)
 {
+	rcu_idle_enter();
 	local_irq_disable();
 	local_fiq_disable();
 

@@ -250,6 +251,7 @@ static void omap2_pm_idle(void)
 out:
 	local_fiq_enable();
 	local_irq_enable();
+	rcu_idle_exit();
 }
 
 #ifdef CONFIG_SUSPEND

arch/arm/mach-omap2/pm34xx.c +2 -0

index fc69875..58a8689 100644
@@ -427,7 +427,9 @@ static void omap3_pm_idle(void)
 	trace_power_start(POWER_CSTATE, 1, smp_processor_id());
 	trace_cpu_idle(1, smp_processor_id());
 
+	rcu_idle_enter();
 	omap_sram_idle();
+	rcu_idle_exit();
 
 	trace_power_end(smp_processor_id());
 	trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());

arch/arm/mach-omap2/pm44xx.c +3 -0

index c264ef7..038796c 100644
@@ -16,6 +16,7 @@
 #include <linux/list.h>
 #include <linux/err.h>
 #include <linux/slab.h>
+#include <linux/rcupdate.h>
 
 #include "common.h"
 #include "clockdomain.h"

@@ -178,6 +179,7 @@ static int __init pwrdms_setup(struct powerdomain *pwrdm, void *unused)
  */
 static void omap_default_idle(void)
 {
+	rcu_idle_enter();
 	local_irq_disable();
 	local_fiq_disable();
 

@@ -185,6 +187,7 @@ static void omap_default_idle(void)
 
 	local_fiq_enable();
 	local_irq_enable();
+	rcu_idle_exit();
 }
 
 /**

arch/arm/mach-pnx4008/pm.c +2 -0

index f3e60a0..0b8703f 100644
@@ -102,6 +102,7 @@ static inline void pnx4008_suspend(void)
 
 static int pnx4008_pm_enter(suspend_state_t state)
 {
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_STANDBY:
 		pnx4008_standby();

@@ -110,6 +111,7 @@ static int pnx4008_pm_enter(suspend_state_t state)
 		pnx4008_suspend();
 		break;
 	}
+	rcu_idle_exit();
 	return 0;
 }
 

arch/arm/mach-prima2/pm.c +4 -0

index 26ebb57..1e35c8a 100644
@@ -15,6 +15,7 @@
 #include <linux/of_device.h>
 #include <linux/of_platform.h>
 #include <linux/io.h>
+#include <linux/rcupdate.h>
 #include <linux/rtc/sirfsoc_rtciobrg.h>
 #include <asm/suspend.h>
 #include <asm/hardware/cache-l2x0.h>

@@ -64,6 +65,7 @@ static int sirfsoc_pre_suspend_power_off(void)
 
 static int sirfsoc_pm_enter(suspend_state_t state)
 {
+	rcu_idle_enter(); /* No good place for this: locking->lockdep->RCU. */
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		sirfsoc_pre_suspend_power_off();

@@ -73,8 +75,10 @@ static int sirfsoc_pm_enter(suspend_state_t state)
 		/* go zzz */
 		cpu_suspend(0, sirfsoc_finish_suspend);
 		outer_resume();
+		rcu_idle_exit();
 		break;
 	default:
+		rcu_idle_exit();
 		return -EINVAL;
 	}
 	return 0;

arch/arm/mach-s5p64x0/common.c +2 -0

index 52b89a3..c59a7f2 100644
@@ -146,6 +146,7 @@ static void s5p64x0_idle(void)
 {
 	unsigned long val;
 
+	rcu_idle_enter();
 	if (!need_resched()) {
 		val = __raw_readl(S5P64X0_PWR_CFG);
 		val &= ~(0x3 << 5);

@@ -154,6 +155,7 @@ static void s5p64x0_idle(void)
 
 		cpu_do_idle();
 	}
+	rcu_idle_exit();
 	local_irq_enable();
 }
 

arch/arm/mach-s5pc100/common.c +2 -0

index c909573..7bf02ff 100644
@@ -131,9 +131,11 @@ static struct map_desc s5pc100_iodesc[] __initdata = {
 
 static void s5pc100_idle(void)
 {
+	rcu_idle_enter();
 	if (!need_resched())
 		cpu_do_idle();
 
+	rcu_idle_exit();
 	local_irq_enable();
 }
 

arch/arm/mach-s5pv210/common.c +2 -0

index 9c1bcdc..ccd6b4d 100644
@@ -144,9 +144,11 @@ static struct map_desc s5pv210_iodesc[] __initdata = {
 
 static void s5pv210_idle(void)
 {
+	rcu_idle_enter();
 	if (!need_resched())
 		cpu_do_idle();
 
+	rcu_idle_exit();
 	local_irq_enable();
 }
 

arch/arm/mach-shmobile/cpuidle.c +3 -0

index 1b23342..651e92b 100644
@@ -13,6 +13,7 @@
 #include <linux/suspend.h>
 #include <linux/module.h>
 #include <linux/err.h>
+#include <linux/rcupdate.h>
 #include <asm/system.h>
 #include <asm/io.h>
 

@@ -33,6 +34,7 @@ static int shmobile_cpuidle_enter(struct cpuidle_device *dev,
 
 	before = ktime_get();
 
+	rcu_idle_enter();
 	local_irq_disable();
 	local_fiq_disable();
 

@@ -40,6 +42,7 @@ static int shmobile_cpuidle_enter(struct cpuidle_device *dev,
 
 	local_irq_enable();
 	local_fiq_enable();
+	rcu_idle_exit();
 
 	after = ktime_get();
 	dev->last_residency = ktime_to_ns(ktime_sub(after, before)) >> 10;

arch/arm/mach-shmobile/pm-sh7372.c +8 -0

index fcf8b17..e11507e 100644
@@ -21,6 +21,7 @@
 #include <linux/irq.h>
 #include <linux/bitrev.h>
 #include <linux/console.h>
+#include <linux/rcupdate.h>
 #include <asm/system.h>
 #include <asm/io.h>
 #include <asm/tlbflush.h>

@@ -520,17 +521,24 @@ static int sh7372_enter_suspend(suspend_state_t suspend_state)
 		    sh7372_a4s.genpd.status == GPD_STATE_POWER_OFF) {
 			/* enter A4S sleep with PLLC0 off */
 			pr_debug("entering A4S\n");
+			rcu_idle_enter();
 			sh7372_enter_a4s_common(0);
+			rcu_idle_exit();
 		} else {
 			/* enter A3SM sleep with PLLC0 off */
 			pr_debug("entering A3SM\n");
+			rcu_idle_enter();
 			sh7372_enter_a3sm_common(0);
+			rcu_idle_exit();
 		}
 	} else {
 		/* default to Core Standby that supports all wakeup sources */
 		pr_debug("entering Core Standby\n");
+		rcu_idle_enter();
 		sh7372_enter_core_standby();
+		rcu_idle_exit();
 	}
+
 	return 0;
 }
 

Signature · 1 line

1.7.8

Quoted 7 lines · Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

The idle loop is a quiscent state for RCU, which means that RCU ignores CPUs that have told RCU that they are idle via rcu_idle_enter(). There are nevertheless quite a few places where idle CPUs use RCU, most commonly indirectly via tracing. This patch fixes these problems for ARM.

Quoted 6 lines

Many of these bugs have been in the kernel for quite some time, but Frederic's recent change now gives warnings.

This patch takes the straightforward approach of pushing the rcu_idle_enter()/rcu_idle_exit() pair further down into the core of the idle loop.

NAK.

1) This is going to conflict with a patch series I've pushed to RMK already. You can see its result in linux-next.

2) The purpose of (1) is to do precisely the very opposite i.e. move as much common knowledge as possible up the idle call paths and leave the platform specific quirks as bare as possible, if any.

So I'd much prefer if this change was constrained to be inside cpu_idle(), or at least in its close vicinity.

Quoted 639 lines

Signed-off-by Paul E. McKenney <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

Cc Russell King <[email protected]>

Cc Andrew Victor <[email protected]>

Cc Nicolas Ferre <[email protected]>

Cc Jean-Christophe Plagniol-Villard <[email protected]>

Cc Sekhar Nori <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Kukjin Kim <[email protected]>

Cc Rob Herring <[email protected]>

Cc Sascha Hauer <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Lennert Buytenhek <[email protected]>

Cc Nicolas Pitre <[email protected]>

Cc Amit Kucheria <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Barry Song <[email protected]>

Cc Paul Mundt <[email protected]>

Cc Magnus Damm <[email protected]>

Cc Will Deacon <[email protected]>

Cc H Hartley Sweeten <[email protected]>

Cc Trinabh Gupta <[email protected]>

Cc Deepthi Dharwar <[email protected]>

Cc Len Brown <[email protected]>

Cc linux-arm-kernel at lists.infradead.org

Cc linux-samsung-soc at vger.kernel.org

Cc linux-omap at vger.kernel.org

---
 arch/arm/kernel/process.c          |    2 --
 arch/arm/mach-at91/cpuidle.c       |    3 +++
 arch/arm/mach-davinci/cpuidle.c    |    3 +++
 arch/arm/mach-exynos/common.c      |    2 ++
 arch/arm/mach-highbank/pm.c        |   12 ++++++++++++
 arch/arm/mach-imx/mm-imx3.c        |    3 +++
 arch/arm/mach-imx/pm-imx27.c       |    4 ++++
 arch/arm/mach-imx/pm-imx6q.c       |    4 ++++
 arch/arm/mach-kirkwood/cpuidle.c   |    3 +++
 arch/arm/mach-mx5/mm.c             |    3 +++
 arch/arm/mach-mx5/pm-imx5.c        |    3 +++
 arch/arm/mach-mxs/pm.c             |    4 ++++
 arch/arm/mach-omap1/pm.c           |    4 ++++
 arch/arm/mach-omap2/pm24xx.c       |    2 ++
 arch/arm/mach-omap2/pm34xx.c       |    2 ++
 arch/arm/mach-omap2/pm44xx.c       |    3 +++
 arch/arm/mach-pnx4008/pm.c         |    2 ++
 arch/arm/mach-prima2/pm.c          |    4 ++++
 arch/arm/mach-s5p64x0/common.c     |    2 ++
 arch/arm/mach-s5pc100/common.c     |    2 ++
 arch/arm/mach-s5pv210/common.c     |    2 ++
 arch/arm/mach-shmobile/cpuidle.c   |    3 +++
 arch/arm/mach-shmobile/pm-sh7372.c |    8 ++++++++
 23 files changed, 78 insertions(+), 2 deletions(-)

arch/arm/kernel/process.c +0 -2

index 971d65c..8241df7 100644
@@ -207,7 +207,6 @@ void cpu_idle(void)
 	/* endless idle loop with no priority at all */
 	while (1) {
 		tick_nohz_idle_enter();
-		rcu_idle_enter();
 		leds_event(led_idle_start);
 		while (!need_resched()) {
 #ifdef CONFIG_HOTPLUG_CPU

@@ -237,7 +236,6 @@ void cpu_idle(void)
 			}
 		}
 		leds_event(led_idle_end);
-		rcu_idle_exit();
 		tick_nohz_idle_exit();
 		preempt_enable_no_resched();
 		schedule();

arch/arm/mach-at91/cpuidle.c +3 -0

index a851e6c..8feff6b 100644
@@ -20,6 +20,7 @@
 #include <asm/proc-fns.h>
 #include <linux/io.h>
 #include <linux/export.h>
+#include <linux/rcupdate.h>
 
 #include "pm.h"
 

@@ -43,6 +44,7 @@ static int at91_enter_idle(struct cpuidle_device *dev,
 
 	local_irq_disable();
 	do_gettimeofday(&before);
+	rcu_idle_enter();
 	if (index == 0)
 		/* Wait for interrupt state */
 		cpu_do_idle();

@@ -53,6 +55,7 @@ static int at91_enter_idle(struct cpuidle_device *dev,
 		cpu_do_idle();
 		sdram_selfrefresh_disable(saved_lpr);
 	}
+	rcu_idle_exit();
 	do_gettimeofday(&after);
 	local_irq_enable();
 	idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +

arch/arm/mach-davinci/cpuidle.c +3 -0

index a30c7c5..6594b7e 100644
@@ -17,6 +17,7 @@
 #include <linux/cpuidle.h>
 #include <linux/io.h>
 #include <linux/export.h>
+#include <linux/rcupdate.h>
 #include <asm/proc-fns.h>
 
 #include <mach/cpuidle.h>

@@ -90,12 +91,14 @@ static int davinci_enter_idle(struct cpuidle_device *dev,
 	local_irq_disable();
 	do_gettimeofday(&before);
 
+	rcu_idle_enter();
 	if (ops && ops->enter)
 		ops->enter(ops->flags);
 	/* Wait for interrupt state */
 	cpu_do_idle();
 	if (ops && ops->exit)
 		ops->exit(ops->flags);
+	rcu_idle_exit();
 
 	do_gettimeofday(&after);
 	local_irq_enable();

arch/arm/mach-exynos/common.c +2 -0

index c59e188..1e5844a 100644
@@ -203,8 +203,10 @@ static struct map_desc exynos4_iodesc1[] __initdata = {
 
 static void exynos_idle(void)
 {
+	rcu_idle_enter();
 	if (!need_resched())
 		cpu_do_idle();
+	rcu_idle_exit();
 
 	local_irq_enable();
 }

arch/arm/mach-highbank/pm.c +12 -0

index 33b3beb..5ecc4bc 100644
@@ -17,6 +17,7 @@
 #include <linux/init.h>
 #include <linux/io.h>
 #include <linux/suspend.h>
+#include <linux/rcupdate.h>
 
 #include <asm/proc-fns.h>
 #include <asm/smp_scu.h>

@@ -33,12 +34,23 @@ static int highbank_suspend_finish(unsigned long val)
 
 static int highbank_pm_enter(suspend_state_t state)
 {
+	/*
+	 * Some of the functions preceding the cpu_suspend() can
+	 * invoke RCU, but only in case of failure to disable preemption
+	 * or preempt_disable() misnesting.  Assume that these errors
+	 * are not committed in the following code, because RCU just
+	 * doesn't want to run on a CPU that has its caches disabled.
+	 */
+	rcu_idle_enter();
+
 	hignbank_set_pwr_suspend();
 	highbank_set_cpu_jump(0, cpu_resume);
 
 	scu_power_mode(scu_base_addr, SCU_PM_POWEROFF);
 	cpu_suspend(0, highbank_suspend_finish);
 
+	rcu_idle_exit();
+
 	return 0;
 }
 

arch/arm/mach-imx/mm-imx3.c +3 -0

index 31807d2..0945b23 100644
@@ -19,6 +19,7 @@
 #include <linux/mm.h>
 #include <linux/init.h>
 #include <linux/err.h>
+#include <linux/rcupdate.h>
 
 #include <asm/pgtable.h>
 #include <asm/hardware/cache-l2x0.h>

@@ -34,6 +35,7 @@ static void imx3_idle(void)
 {
 	unsigned long reg = 0;
 
+	rcu_idle_enter();
 	if (!need_resched())
 		__asm__ __volatile__(
 			/* disable I and D cache */

@@ -58,6 +60,7 @@ static void imx3_idle(void)
 			"orr %0, %0, #0x00000004\n"
 			"mcr p15, 0, %0, c1, c0, 0\n"
 			: "=r" (reg));
+	rcu_idle_exit();
 	local_irq_enable();
 }
 

arch/arm/mach-imx/pm-imx27.c +4 -0

index e455d2f..c4adc05 100644
@@ -10,12 +10,14 @@
 #include <linux/kernel.h>
 #include <linux/suspend.h>
 #include <linux/io.h>
+#include <linux/rcupdate.h>
 #include <mach/system.h>
 #include <mach/hardware.h>
 
 static int mx27_suspend_enter(suspend_state_t state)
 {
 	u32 cscr;
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		/* Clear MPEN and SPEN to disable MPLL/SPLL */

@@ -24,9 +26,11 @@ static int mx27_suspend_enter(suspend_state_t state)
 		__raw_writel(cscr, MX27_IO_ADDRESS(MX27_CCM_BASE_ADDR));
 		/* Executes WFI */
 		arch_idle();
+		rcu_idle_exit();
 		break;
 
 	default:
+		rcu_idle_exit();
 		return -EINVAL;
 	}
 	return 0;

arch/arm/mach-imx/pm-imx6q.c +4 -0

index f7b0c2b..b9c2589 100644
@@ -14,6 +14,7 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/suspend.h>
+#include <linux/rcupdate.h>
 #include <asm/cacheflush.h>
 #include <asm/proc-fns.h>
 #include <asm/suspend.h>

@@ -31,6 +32,7 @@ static int imx6q_suspend_finish(unsigned long val)
 
 static int imx6q_pm_enter(suspend_state_t state)
 {
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		imx6q_set_lpm(STOP_POWER_OFF);

@@ -40,8 +42,10 @@ static int imx6q_pm_enter(suspend_state_t state)
 		cpu_suspend(0, imx6q_suspend_finish);
 		imx_smp_prepare();
 		imx_gpc_post_resume();
+		rcu_idle_exit();
 		break;
 	default:
+		rcu_idle_exit();
 		return -EINVAL;
 	}
 

arch/arm/mach-kirkwood/cpuidle.c +3 -0

index 7088180..de8e317 100644
@@ -19,6 +19,7 @@
 #include <linux/cpuidle.h>
 #include <linux/io.h>
 #include <linux/export.h>
+#include <linux/rcupdate.h>
 #include <asm/proc-fns.h>
 #include <mach/kirkwood.h>
 

@@ -41,6 +42,7 @@ static int kirkwood_enter_idle(struct cpuidle_device *dev,
 
 	local_irq_disable();
 	do_gettimeofday(&before);
+	rcu_idle_enter();
 	if (index == 0)
 		/* Wait for interrupt state */
 		cpu_do_idle();

@@ -55,6 +57,7 @@ static int kirkwood_enter_idle(struct cpuidle_device *dev,
 		writel(0x7, DDR_OPERATION_BASE);
 		cpu_do_idle();
 	}
+	rcu_idle_exit();
 	do_gettimeofday(&after);
 	local_irq_enable();
 	idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +

arch/arm/mach-mx5/mm.c +3 -0

index bc17dfe..d919648 100644
@@ -14,6 +14,7 @@
 #include <linux/mm.h>
 #include <linux/init.h>
 #include <linux/clk.h>
+#include <linux/rcupdate.h>
 
 #include <asm/mach/map.h>
 

@@ -37,7 +38,9 @@ static void imx5_idle(void)
 		mx5_cpu_lp_set(WAIT_UNCLOCKED_POWER_OFF);
 		if (tzic_enable_wake())
 			goto err1;
+		rcu_idle_enter();  /* No tracing until rcu_idle_exit(). */
 		cpu_do_idle();
+		rcu_idle_exit();
 err1:
 		clk_disable(gpc_dvfs_clk);
 	}

arch/arm/mach-mx5/pm-imx5.c +3 -0

index 98052fc..b4adf42 100644
@@ -12,6 +12,7 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/err.h>
+#include <linux/rcupdate.h>
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
 #include <mach/common.h>

@@ -27,6 +28,7 @@ static int mx5_suspend_prepare(void)
 
 static int mx5_suspend_enter(suspend_state_t state)
 {
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		mx5_cpu_lp_set(STOP_POWER_OFF);

@@ -47,6 +49,7 @@ static int mx5_suspend_enter(suspend_state_t state)
 		__raw_writel(0, MXC_SRPG_EMPGC1_SRPGCR);
 	}
 	cpu_do_idle();
+	rcu_idle_exit();
 	return 0;
 }
 

arch/arm/mach-mxs/pm.c +4 -0

index fb042da..36a3a57 100644
@@ -15,16 +15,20 @@
 #include <linux/kernel.h>
 #include <linux/suspend.h>
 #include <linux/io.h>
+#include <linux/rcupdate.h>
 #include <mach/system.h>
 
 static int mxs_suspend_enter(suspend_state_t state)
 {
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		arch_idle();
+		rcu_idle_exit();
 		break;
 
 	default:
+		rcu_idle_exit();
 		return -EINVAL;
 	}
 	return 0;

arch/arm/mach-omap1/pm.c +4 -0

index 89ea20c..c661eba 100644
@@ -108,11 +108,13 @@ void omap1_pm_idle(void)
 	__u32 use_idlect1 = arm_idlect1_mask;
 	int do_sleep = 0;
 
+	rcu_idle_enter();
 	local_irq_disable();
 	local_fiq_disable();
 	if (need_resched()) {
 		local_fiq_enable();
 		local_irq_enable();
+		rcu_idle_exit();
 		return;
 	}
 

@@ -158,6 +160,7 @@ void omap1_pm_idle(void)
 
 		local_fiq_enable();
 		local_irq_enable();
+		rcu_idle_exit();
 		return;
 	}
 	omap_sram_suspend(omap_readl(ARM_IDLECT1),

@@ -165,6 +168,7 @@ void omap1_pm_idle(void)
 
 	local_fiq_enable();
 	local_irq_enable();
+	rcu_idle_exit();
 }
 
 /*

arch/arm/mach-omap2/pm24xx.c +2 -0

index b8822f8..2139e9d 100644
@@ -232,6 +232,7 @@ static int omap2_can_sleep(void)
 
 static void omap2_pm_idle(void)
 {
+	rcu_idle_enter();
 	local_irq_disable();
 	local_fiq_disable();
 

@@ -250,6 +251,7 @@ static void omap2_pm_idle(void)
 out:
 	local_fiq_enable();
 	local_irq_enable();
+	rcu_idle_exit();
 }
 
 #ifdef CONFIG_SUSPEND

arch/arm/mach-omap2/pm34xx.c +2 -0

index fc69875..58a8689 100644
@@ -427,7 +427,9 @@ static void omap3_pm_idle(void)
 	trace_power_start(POWER_CSTATE, 1, smp_processor_id());
 	trace_cpu_idle(1, smp_processor_id());
 
+	rcu_idle_enter();
 	omap_sram_idle();
+	rcu_idle_exit();
 
 	trace_power_end(smp_processor_id());
 	trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());

arch/arm/mach-omap2/pm44xx.c +3 -0

index c264ef7..038796c 100644
@@ -16,6 +16,7 @@
 #include <linux/list.h>
 #include <linux/err.h>
 #include <linux/slab.h>
+#include <linux/rcupdate.h>
 
 #include "common.h"
 #include "clockdomain.h"

@@ -178,6 +179,7 @@ static int __init pwrdms_setup(struct powerdomain *pwrdm, void *unused)
  */
 static void omap_default_idle(void)
 {
+	rcu_idle_enter();
 	local_irq_disable();
 	local_fiq_disable();
 

@@ -185,6 +187,7 @@ static void omap_default_idle(void)
 
 	local_fiq_enable();
 	local_irq_enable();
+	rcu_idle_exit();
 }
 
 /**

arch/arm/mach-pnx4008/pm.c +2 -0

index f3e60a0..0b8703f 100644
@@ -102,6 +102,7 @@ static inline void pnx4008_suspend(void)
 
 static int pnx4008_pm_enter(suspend_state_t state)
 {
+	rcu_idle_enter();
 	switch (state) {
 	case PM_SUSPEND_STANDBY:
 		pnx4008_standby();

@@ -110,6 +111,7 @@ static int pnx4008_pm_enter(suspend_state_t state)
 		pnx4008_suspend();
 		break;
 	}
+	rcu_idle_exit();
 	return 0;
 }
 

arch/arm/mach-prima2/pm.c +4 -0

index 26ebb57..1e35c8a 100644
@@ -15,6 +15,7 @@
 #include <linux/of_device.h>
 #include <linux/of_platform.h>
 #include <linux/io.h>
+#include <linux/rcupdate.h>
 #include <linux/rtc/sirfsoc_rtciobrg.h>
 #include <asm/suspend.h>
 #include <asm/hardware/cache-l2x0.h>

@@ -64,6 +65,7 @@ static int sirfsoc_pre_suspend_power_off(void)
 
 static int sirfsoc_pm_enter(suspend_state_t state)
 {
+	rcu_idle_enter(); /* No good place for this: locking->lockdep->RCU. */
 	switch (state) {
 	case PM_SUSPEND_MEM:
 		sirfsoc_pre_suspend_power_off();

@@ -73,8 +75,10 @@ static int sirfsoc_pm_enter(suspend_state_t state)
 		/* go zzz */
 		cpu_suspend(0, sirfsoc_finish_suspend);
 		outer_resume();
+		rcu_idle_exit();
 		break;
 	default:
+		rcu_idle_exit();
 		return -EINVAL;
 	}
 	return 0;

arch/arm/mach-s5p64x0/common.c +2 -0

index 52b89a3..c59a7f2 100644
@@ -146,6 +146,7 @@ static void s5p64x0_idle(void)
 {
 	unsigned long val;
 
+	rcu_idle_enter();
 	if (!need_resched()) {
 		val = __raw_readl(S5P64X0_PWR_CFG);
 		val &= ~(0x3 << 5);

@@ -154,6 +155,7 @@ static void s5p64x0_idle(void)
 
 		cpu_do_idle();
 	}
+	rcu_idle_exit();
 	local_irq_enable();
 }
 

arch/arm/mach-s5pc100/common.c +2 -0

index c909573..7bf02ff 100644
@@ -131,9 +131,11 @@ static struct map_desc s5pc100_iodesc[] __initdata = {
 
 static void s5pc100_idle(void)
 {
+	rcu_idle_enter();
 	if (!need_resched())
 		cpu_do_idle();
 
+	rcu_idle_exit();
 	local_irq_enable();
 }
 

arch/arm/mach-s5pv210/common.c +2 -0

index 9c1bcdc..ccd6b4d 100644
@@ -144,9 +144,11 @@ static struct map_desc s5pv210_iodesc[] __initdata = {
 
 static void s5pv210_idle(void)
 {
+	rcu_idle_enter();
 	if (!need_resched())
 		cpu_do_idle();
 
+	rcu_idle_exit();
 	local_irq_enable();
 }
 

arch/arm/mach-shmobile/cpuidle.c +3 -0

index 1b23342..651e92b 100644
@@ -13,6 +13,7 @@
 #include <linux/suspend.h>
 #include <linux/module.h>
 #include <linux/err.h>
+#include <linux/rcupdate.h>
 #include <asm/system.h>
 #include <asm/io.h>
 

@@ -33,6 +34,7 @@ static int shmobile_cpuidle_enter(struct cpuidle_device *dev,
 
 	before = ktime_get();
 
+	rcu_idle_enter();
 	local_irq_disable();
 	local_fiq_disable();
 

@@ -40,6 +42,7 @@ static int shmobile_cpuidle_enter(struct cpuidle_device *dev,
 
 	local_irq_enable();
 	local_fiq_enable();
+	rcu_idle_exit();
 
 	after = ktime_get();
 	dev->last_residency = ktime_to_ns(ktime_sub(after, before)) >> 10;

arch/arm/mach-shmobile/pm-sh7372.c +8 -0

index fcf8b17..e11507e 100644
@@ -21,6 +21,7 @@
 #include <linux/irq.h>
 #include <linux/bitrev.h>
 #include <linux/console.h>
+#include <linux/rcupdate.h>
 #include <asm/system.h>
 #include <asm/io.h>
 #include <asm/tlbflush.h>

@@ -520,17 +521,24 @@ static int sh7372_enter_suspend(suspend_state_t suspend_state)
 		    sh7372_a4s.genpd.status == GPD_STATE_POWER_OFF) {
 			/* enter A4S sleep with PLLC0 off */
 			pr_debug("entering A4S\n");
+			rcu_idle_enter();
 			sh7372_enter_a4s_common(0);
+			rcu_idle_exit();
 		} else {
 			/* enter A3SM sleep with PLLC0 off */
 			pr_debug("entering A3SM\n");
+			rcu_idle_enter();
 			sh7372_enter_a3sm_common(0);
+			rcu_idle_exit();
 		}
 	} else {
 		/* default to Core Standby that supports all wakeup sources */
 		pr_debug("entering Core Standby\n");
+		rcu_idle_enter();
 		sh7372_enter_core_standby();
+		rcu_idle_exit();
 	}
+
 	return 0;
 }
 

Signature · 1 line

1.7.8

Quoted 25 lines · Nicolas Pitre

Quoted 7 lines · Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

The idle loop is a quiscent state for RCU, which means that RCU ignores CPUs that have told RCU that they are idle via rcu_idle_enter(). There are nevertheless quite a few places where idle CPUs use RCU, most commonly indirectly via tracing. This patch fixes these problems for ARM.

Quoted 6 lines

Many of these bugs have been in the kernel for quite some time, but Frederic's recent change now gives warnings.

This patch takes the straightforward approach of pushing the rcu_idle_enter()/rcu_idle_exit() pair further down into the core of the idle loop.

NAK.

1) This is going to conflict with a patch series I've pushed to RMK already. You can see its result in linux-next.

2) The purpose of (1) is to do precisely the very opposite i.e. move as much common knowledge as possible up the idle call paths and leave the platform specific quirks as bare as possible, if any.

Quoted 2 lines

So I'd much prefer if this change was constrained to be inside cpu_idle(), or at least in its close vicinity.

OK. Then the tracing in the inner idle loop needs to go. Other restructuring might be required as well.

The long and short of it is that you absolutely cannot use RCU between the time you invoke rcu_idle_enter() and the time you invoke rcu_idle_exit(). The ARM idle code currently does just that and therefore must change.

Whatever change you choose that causes the code to meet that constraint is met is fine by me.

But that constraint absolutely must be met.

Quoted 26 lines · Paul E. McKenney

Quoted 25 lines · Nicolas Pitre

Quoted 7 lines · Paul E. McKenney

From: "Paul E. McKenney" <[email protected]>

The idle loop is a quiscent state for RCU, which means that RCU ignores CPUs that have told RCU that they are idle via rcu_idle_enter(). There are nevertheless quite a few places where idle CPUs use RCU, most commonly indirectly via tracing. This patch fixes these problems for ARM.

Quoted 6 lines

Many of these bugs have been in the kernel for quite some time, but Frederic's recent change now gives warnings.

This patch takes the straightforward approach of pushing the rcu_idle_enter()/rcu_idle_exit() pair further down into the core of the idle loop.

NAK.

1) This is going to conflict with a patch series I've pushed to RMK already. You can see its result in linux-next.

2) The purpose of (1) is to do precisely the very opposite i.e. move as much common knowledge as possible up the idle call paths and leave the platform specific quirks as bare as possible, if any.

Quoted 5 lines

Quoted 2 lines

So I'd much prefer if this change was constrained to be inside cpu_idle(), or at least in its close vicinity.

OK. Then the tracing in the inner idle loop needs to go. Other restructuring might be required as well.

Let me expand a bit more on the "if any" in my #2 above. Most ARM sub-architectures don't have any special idle drivers and they simply rely on the default cpu_do_idle call. What this proposed patch is doing is to move the rcu_idle_enter()/rcu_idle_exit() pair down into the few subarchs with an existing cpu-idle callback. This means in practice that rcu_idle_enter()/rcu_idle_exit() wouldn't be called at all anymore on most ARM targets. Is that appropriate?

Quoted 5 lines

The long and short of it is that you absolutely cannot use RCU between the time you invoke rcu_idle_enter() and the time you invoke rcu_idle_exit(). The ARM idle code currently does just that and therefore must change.

Quoted 4 lines

Whatever change you choose that causes the code to meet that constraint is met is fine by me.

But that constraint absolutely must be met.

I would have to know more about what the rcu_idle_*() calls imply before suggesting an alternative.

Quoted 2 lines · Nicolas Pitre

I would have to know more about what the rcu_idle_*() calls imply before suggesting an alternative.

After a call to rcu_idle_enter(), RCU ignores the CPU completely until the next call to rcu_idle_exit(). This means that in the interval between rcu_idle_enter() and rcu_idle_exit(), you can say rcu_read_lock() all you want, but your data structures will get absolutely no RCU protection. This will result in random memory corruption, and for all I know might already be resultin in random memory corruption. So a fix is required.

So why does RCU need to ignore idle CPUs? Because otherwise, RCU needs to periodically wake them up to check their status. Waking them up periodically defeats CONFIG_NO_HZ's attempts to conserve power. Avoiding waking them up and avoiding ignoring them extended RCU grace periods, eventually OOMing the systems.

Why this new imposition? It is not new. It has always been illegal to use RCU in the idle loop from the beginning. But people happily ignored that restriction, partly because it is not always immediately obvious what is using RCU. Event tracing does, but unless you know that...

So we added diagnostics to check for illegal uses of RCU in the idle loop, and also separated rcu_idle_enter() and rcu_idle_exit() from tick_nohz_stop_sched_tick(), so that things like idle notifiers can work. The diagnostics promptly located all sorts of problems, including these.

The two options I see are:

1. Rip tracing out of the inner idle loops and everything that
they invoke.

2. Push rcu_idle_enter() and rcu_idle_exit() further down into
the idle loops. As you say, -all- of them.

My patch set was an attempt at #2, but clearly a very poorly conceived attempt. But I had to start somewhere.

Other ideas?

Quoted 17 lines · Paul E. McKenney

Quoted 2 lines · Nicolas Pitre

I would have to know more about what the rcu_idle_*() calls imply before suggesting an alternative.

After a call to rcu_idle_enter(), RCU ignores the CPU completely until the next call to rcu_idle_exit(). This means that in the interval between rcu_idle_enter() and rcu_idle_exit(), you can say rcu_read_lock() all you want, but your data structures will get absolutely no RCU protection. This will result in random memory corruption, and for all I know might already be resultin in random memory corruption. So a fix is required.

So why does RCU need to ignore idle CPUs? Because otherwise, RCU needs to periodically wake them up to check their status. Waking them up periodically defeats CONFIG_NO_HZ's attempts to conserve power. Avoiding waking them up and avoiding ignoring them extended RCU grace periods, eventually OOMing the systems.

Quoted 4 lines

Why this new imposition? It is not new. It has always been illegal to use RCU in the idle loop from the beginning. But people happily ignored that restriction, partly because it is not always immediately obvious what is using RCU. Event tracing does, but unless you know that...

Not having the slightest idea about the tracing context, I'd simply suggest flaming the people who ignored the restriction harder. Or turn that into a BUG() to get their attention.

Quoted 4 lines

So we added diagnostics to check for illegal uses of RCU in the idle loop, and also separated rcu_idle_enter() and rcu_idle_exit() from tick_nohz_stop_sched_tick(), so that things like idle notifiers can work. The diagnostics promptly located all sorts of problems, including these.

Good.

Quoted 4 lines

The two options I see are:

1. Rip tracing out of the inner idle loops and everything that
they invoke.

What I suggested above. But as I said I know sh*t about that tracing implementation so that's an easy suggestion for me to make.

Quoted 5 lines

2. Push rcu_idle_enter() and rcu_idle_exit() further down into
the idle loops. As you say, -all- of them.

My patch set was an attempt at #2, but clearly a very poorly conceived attempt. But I had to start somewhere.

I'm pretty opposed to #2 in any case. Spreading knowledge about generic kernel infrastructure requirements across 53 or so different ARM subarchitectures is what has put ARM in trouble in the past, as core kernel folks called it a maintenance hell. Just imagine that you do put your rcu_idle_enter() call in those subarchs, and a year from now you need to modify its semantics. At that point you would have to audit all those 53 subarchs which might have evolved their idle support in the mean time, and the new ones that might have appeared with buggy usage of rcu_idle_enter() just because they copied it from somewhere else without thinking it through.

Now we're working very hard to kill that trend and move things in the other direction so to keep only minimal and very platform specific knowledge in the subarch code. If rcu were to push its requirements down there again instead of keeping things abstracted in one place higher the stack then that would be a big step backward.

Quoted 1 line

Other ideas?

Have a special tracing API just for the idle code that queues its events in a per-CPU buffer (there should not be that many events to trace in the idle code) and have rcu_idle_exit() flush that buffer back in the actual tracing infrastructure. Maybe rcu_idle_enter() could set things in a way so the regular tracing API could still be used regardless. This has the advantage of keeping the knowledge about rcu restrictions in a central place that can be much more easily maintained.

Quoted 7 lines

Quoted 4 lines

The two options I see are:

1. Rip tracing out of the inner idle loops and everything that
they invoke.

What I suggested above. But as I said I know sh*t about that tracing implementation so that's an easy suggestion for me to make.

Works for me as well. ;-)

"Paul E. McKenney" <[email protected]> writes:

[...]

Quoted 8 lines

Quoted 7 lines

Quoted 4 lines

The two options I see are:

1. Rip tracing out of the inner idle loops and everything that
they invoke.

What I suggested above. But as I said I know sh*t about that tracing implementation so that's an easy suggestion for me to make.

Quoted 1 line

Works for me as well. ;-)

While I must admit not having a better suggestion, I for one would vote strongly against removing tracing from the idle path.

Being a PM developer and maintainer, much of the code I work on and maintain happens to be run in the bowels of the idle path. Not having the ability to trace this code would be a major step backwards IMO.

Quoted 12 lines · Kevin Hilman

"Paul E. McKenney" <[email protected]> writes:

[...]

Quoted 8 lines

Quoted 7 lines

Quoted 4 lines

The two options I see are:

1. Rip tracing out of the inner idle loops and everything that
they invoke.

What I suggested above. But as I said I know sh*t about that tracing implementation so that's an easy suggestion for me to make.

Quoted 8 lines

Quoted 1 line

Works for me as well. ;-)

While I must admit not having a better suggestion, I for one would vote strongly against removing tracing from the idle path.

Being a PM developer and maintainer, much of the code I work on and maintain happens to be run in the bowels of the idle path. Not having the ability to trace this code would be a major step backwards IMO.

How is it a step backwards if it is already broken. Obviously you haven't actually used any tracing here because it doesn't work right with things as is.

What exactly do you want to trace at this level. By the point you are in this code, the path is somewhat known and problems you have are likely h/w issues. If you are trying to go thru a very precise sequence of saving cpu state and flushing caches, you don't want calls out to tracing code that could very easily change the behavior.

It would be nice to understand a real example of how to hit this problem. Why type of tracing? Are there other known examples?

Anyway, one answer is (and I was talking with Paul about this on IRC) is that we can create a special "TRACE_EVENT_IDLE()" that will explicitly call "rcu_idle_exit/enter()" as it expects to be called with it off.

This should solve most issues I believe.

On Thu, Feb 02, 2012 at 06:03:39PM -0500, Steven Rostedt wrote:

[ . . . ]

Quoted 4 lines

Anyway, one answer is (and I was talking with Paul about this on IRC) is that we can create a special "TRACE_EVENT_IDLE()" that will explicitly call "rcu_idle_exit/enter()" as it expects to be called with it off.

Quoted 1 line

This should solve most issues I believe.

You OK with something like RCU_NONIDLE() rather than RCU_EVENT_IDLE()? I have this funny feeling that tracing won't be the only thing using RCU from idle. :-/

Something like this, perhaps?

On Thu, Feb 02, 2012 at 06:03:39PM -0500, Steven Rostedt wrote:

[ . . . ]

Quoted 5 lines

Anyway, one answer is (and I was talking with Paul about this on IRC) is that we can create a special "TRACE_EVENT_IDLE()" that will explicitly call "rcu_idle_exit/enter()" as it expects to be called with it off.

This should solve most issues I believe.

You OK with something like RCU_NONIDLE() rather than RCU_EVENT_IDLE()? I have this funny feeling that tracing won't be the only thing using RCU from idle. :-/

Something like this, perhaps?

	#define RCU_NONIDLE(a) \
		do { \
			rcu_idle_exit(); \
			do { a; } while (0); \
			rcu_idle_enter(); \
		}

Quoted 14 lines · Paul E. McKenney

On Thu, Feb 02, 2012 at 06:03:39PM -0500, Steven Rostedt wrote:

[ . . . ]

Quoted 5 lines

Anyway, one answer is (and I was talking with Paul about this on IRC) is that we can create a special "TRACE_EVENT_IDLE()" that will explicitly call "rcu_idle_exit/enter()" as it expects to be called with it off.

This should solve most issues I believe.

You OK with something like RCU_NONIDLE() rather than RCU_EVENT_IDLE()? I have this funny feeling that tracing won't be the only thing using RCU from idle. :-/

Quoted 9 lines

Something like this, perhaps?

	#define RCU_NONIDLE(a) \
		do { \
			rcu_idle_exit(); \
			do { a; } while (0); \
			rcu_idle_enter(); \
		}

I'm fine with this. But what is the overhead for doing such a thing. Remember, to encapsulate a tracepoint means that you will be doing that work everytime regardless if the tracepoint is ever activated or not. Or even worse, not even compiled into the kernel.

Now this is entering and exiting idle, so maybe it's not that critical?

On Thu, Feb 02, 2012 at 09:45:31PM -0500, Steven Rostedt wrote:

It is an atomic instruction or two, plus some memory barriers. Entering idle is more heavyweight for RCU_FAST_NO_HZ. But as you say, it is entering and exiting idle.

But should I make an empty definition of RCU_NONIDLE() for some #define or another?

	#ifdef CONFIG_YOU_TELL_ME
	#define RCU_NONIDLE(a) \
		do { \
			rcu_idle_exit(); \
			do { a; } while (0); \
			rcu_idle_enter(); \
		} while (0)
	#else /* #ifdef CONFIG_YOU_TELL_ME */
	#define RCU_NONIDLE(a) do { } while (0);
	#endif /* #else #ifdef CONFIG_YOU_TELL_ME */

Or is event tracing unconditional these days?

Quoted 20 lines · Paul E. McKenney

On Thu, Feb 02, 2012 at 09:45:31PM -0500, Steven Rostedt wrote:

It is an atomic instruction or two, plus some memory barriers. Entering idle is more heavyweight for RCU_FAST_NO_HZ. But as you say, it is entering and exiting idle.

But should I make an empty definition of RCU_NONIDLE() for some #define or another?

	#ifdef CONFIG_YOU_TELL_ME
	#define RCU_NONIDLE(a) \
		do { \
			rcu_idle_exit(); \
			do { a; } while (0); \
			rcu_idle_enter(); \
		} while (0)
	#else /* #ifdef CONFIG_YOU_TELL_ME */
	#define RCU_NONIDLE(a) do { } while (0);
	#endif /* #else #ifdef CONFIG_YOU_TELL_ME */

Quoted 1 line

Or is event tracing unconditional these days?

I don't like it. As it binds the RCU_NONIDLE to tracepoints only without any annotation that they are bound. Still doesn't help when tracepoints are configured but not enabled.

I have no problem in making a special TRACE_EVENT_IDLE() that does this inside the jump label. Basically what we have today is:

	if (static_branch(tracepoint_key)) {
		rcu_read_lock_sched_notrace();
		for (all attached tracepoints) {
			[...]
		}
		rcu_read_unlock_sched_notrace();
	}

Ideally we want the enter/exit idle inside that static_branch() condition:

	if (static_branch(tracepoint_key)) {
		rcu_idle_exit();
		rcu_read_lock_sched_notrace();
		for (all attached tracepoints) {
			[...]
		}
		rcu_read_unlock_sched_notrace();
		rcu_idle_enter();
	}

The static_branch() is the jump label code when it's a nop when disabled and a jump to the tracing code when enabled:

	nop; /* or jmp 2f */  <<--- jump label
1:	[ normal code ]
	ret;

2: [trace code]
jmp 1b

The jump label when disabled is just a nop that ignores the trace code (although current gcc has a bug that it currently doesn't do it this elegantly). When tracing is enabled the nop is converted to a jump to the tracing code. This makes tracepoints very light weight in hot paths.

Ideally, we want the exit/enter rcu idle with in the [trace code], which makes it not used when not needed.

So the idea is that if you have a trace event that is to be used in idle, you use TRACE_EVENT_IDLE() rather than TRACE_EVENT() to declare that trace event? That would work for me, and might make for fewer changes for the architecture guys. Also, this should address the code-size concerns we discussed yesterday.

So sounds good!

Is a DEFINE_EVENT_IDLE() also needed? Or prehaps a DECLARE_EVENT_CLASS_IDLE()? My guess is "yes" for at least one of the two based on include/trace/events/power.h.