rcu: Detect illegal uses of RCU in idle and fix some v5

Inform the user if an RCU usage error is detected by lockdep while in an extended quiescent state (in this case, the RCU-free window in idle). This is accomplished by adding a line to the RCU lockdep splat indicating whether or not the splat occurred in extended quiescent state.

Uses of RCU from within extended quiescent state mode are totally ignored by RCU, hence the importance of this diagnostic.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Paul E. McKenney <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Lai Jiangshan <[email protected]>

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

---
 kernel/lockdep.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

kernel/lockdep.c +22 -0

index 1e48f1c..8873f6e 100644
@@ -4026,6 +4026,28 @@ void lockdep_rcu_suspicious(const char *file, const int line, const char *s)
 	printk("%s:%d %s!\n", file, line, s);
 	printk("\nother info that might help us debug this:\n\n");
 	printk("\nrcu_scheduler_active = %d, debug_locks = %d\n", rcu_scheduler_active, debug_locks);
+
+	/*
+	 * If a CPU is in the RCU-free window in idle (ie: in the section
+	 * between rcu_idle_enter() and rcu_idle_exit(), then RCU
+	 * considers that CPU to be in an "extended quiescent state",
+	 * which means that RCU will be completely ignoring that CPU.
+	 * Therefore, rcu_read_lock() and friends have absolutely no
+	 * effect on a CPU running in that state. In other words, even if
+	 * such an RCU-idle CPU has called rcu_read_lock(), RCU might well
+	 * delete data structures out from under it.  RCU really has no
+	 * choice here: we need to keep an RCU-free window in idle where
+	 * the CPU may possibly enter into low power mode. This way we can
+	 * notice an extended quiescent state to other CPUs that started a grace
+	 * period. Otherwise we would delay any grace period as long as we run
+	 * in the idle task.
+	 *
+	 * So complain bitterly if someone does call rcu_read_lock(),
+	 * rcu_read_lock_bh() and so on from extended quiescent states.
+	 */
+	if (rcu_is_cpu_idle())
+		printk("RCU used illegally from extended quiescent state!\n");
+
 	lockdep_print_held_locks(curr);
 	printk("\nstack backtrace:\n");
 	dump_stack();

Signature · 1 line

1.7.5.4

It is assumed that rcu won't be used once we switch to tickless mode and until we restart the tick. However this is not always true, as in x86-64 where we dereference the idle notifiers after the tick is stopped.

To prepare for fixing this, add two new APIs:
tick_nohz_idle_enter_norcu() and tick_nohz_idle_exit_norcu().

If no use of RCU is made in the idle loop between tick_nohz_enter_idle() and tick_nohz_exit_idle() calls, the arch must instead call the new *_norcu() version such that the arch doesn't need to call rcu_idle_enter() and rcu_idle_exit().

Otherwise the arch must call tick_nohz_enter_idle() and tick_nohz_exit_idle() and also call explicitly:

- rcu_idle_enter() after its last use of RCU before the CPU is put to sleep.
- rcu_idle_exit() before the first use of RCU after the CPU is woken up.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Mike Frysinger <[email protected]>

Cc Guan Xuetao <[email protected]>

Cc David Miller <[email protected]>

Cc Chris Metcalf <[email protected]>

Cc Hans-Christian Egtvedt <[email protected]>

Cc Ralf Baechle <[email protected]>

Cc Paul E. McKenney <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Russell King <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Heiko Carstens <[email protected]>

Cc Paul Mundt <[email protected]>

---
 arch/arm/kernel/process.c              |    4 +-
 arch/avr32/kernel/process.c            |    4 +-
 arch/blackfin/kernel/process.c         |    4 +-
 arch/microblaze/kernel/process.c       |    4 +-
 arch/mips/kernel/process.c             |    4 +-
 arch/openrisc/kernel/idle.c            |    4 +-
 arch/powerpc/kernel/idle.c             |    4 +-
 arch/powerpc/platforms/iseries/setup.c |    8 +++---
 arch/s390/kernel/process.c             |    4 +-
 arch/sh/kernel/idle.c                  |    4 +-
 arch/sparc/kernel/process_64.c         |    4 +-
 arch/tile/kernel/process.c             |    4 +-
 arch/um/kernel/process.c               |    4 +-
 arch/unicore32/kernel/process.c        |    4 +-
 arch/x86/kernel/process_32.c           |    4 +-
 arch/x86/kernel/process_64.c           |    4 +-
 include/linux/tick.h                   |   45 +++++++++++++++++++++++++++++--
 kernel/time/tick-sched.c               |   25 +++++++++--------
 18 files changed, 89 insertions(+), 49 deletions(-)

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

index f9261d0..4f83362 100644
@@ -183,7 +183,7 @@ void cpu_idle(void)
 
 	/* endless idle loop with no priority at all */
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		leds_event(led_idle_start);
 		while (!need_resched()) {
 #ifdef CONFIG_HOTPLUG_CPU

@@ -210,7 +210,7 @@ void cpu_idle(void)
 			}
 		}
 		leds_event(led_idle_end);
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/avr32/kernel/process.c +2 -2

index 6ee7952..34c8c70 100644
@@ -34,10 +34,10 @@ void cpu_idle(void)
 {
 	/* endless idle loop with no priority at all */
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched())
 			cpu_idle_sleep();
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/blackfin/kernel/process.c +2 -2

index 7b141b5..57e0749 100644
@@ -88,10 +88,10 @@ void cpu_idle(void)
 #endif
 		if (!idle)
 			idle = default_idle;
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched())
 			idle();
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/microblaze/kernel/process.c +2 -2

index 6dc123e..c6ece38 100644
@@ -103,10 +103,10 @@ void cpu_idle(void)
 		if (!idle)
 			idle = default_idle;
 
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched())
 			idle();
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 
 		preempt_enable_no_resched();
 		schedule();

arch/mips/kernel/process.c +2 -2

index d50a005..7df2ffc 100644
@@ -56,7 +56,7 @@ void __noreturn cpu_idle(void)
 
 	/* endless idle loop with no priority at all */
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched() && cpu_online(cpu)) {
 #ifdef CONFIG_MIPS_MT_SMTC
 			extern void smtc_idle_loop_hook(void);

@@ -77,7 +77,7 @@ void __noreturn cpu_idle(void)
 		     system_state == SYSTEM_BOOTING))
 			play_dead();
 #endif
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/openrisc/kernel/idle.c +2 -2

index fb6a9bf..2e82cd0 100644
@@ -51,7 +51,7 @@ void cpu_idle(void)
 
 	/* endless idle loop with no priority at all */
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 
 		while (!need_resched()) {
 			check_pgt_cache();

@@ -69,7 +69,7 @@ void cpu_idle(void)
 			set_thread_flag(TIF_POLLING_NRFLAG);
 		}
 
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/powerpc/kernel/idle.c +2 -2

index 878572f..2e782a3 100644
@@ -56,7 +56,7 @@ void cpu_idle(void)
 
 	set_thread_flag(TIF_POLLING_NRFLAG);
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched() && !cpu_should_die()) {
 			ppc64_runlatch_off();
 

@@ -93,7 +93,7 @@ void cpu_idle(void)
 
 		HMT_medium();
 		ppc64_runlatch_on();
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		if (cpu_should_die())
 			cpu_die();

arch/powerpc/platforms/iseries/setup.c +4 -4

index e2f5fad..77ff6eb 100644
@@ -562,7 +562,7 @@ static void yield_shared_processor(void)
 static void iseries_shared_idle(void)
 {
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched() && !hvlpevent_is_pending()) {
 			local_irq_disable();
 			ppc64_runlatch_off();

@@ -576,7 +576,7 @@ static void iseries_shared_idle(void)
 		}
 
 		ppc64_runlatch_on();
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 
 		if (hvlpevent_is_pending())
 			process_iSeries_events();

@@ -592,7 +592,7 @@ static void iseries_dedicated_idle(void)
 	set_thread_flag(TIF_POLLING_NRFLAG);
 
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		if (!need_resched()) {
 			while (!need_resched()) {
 				ppc64_runlatch_off();

@@ -609,7 +609,7 @@ static void iseries_dedicated_idle(void)
 		}
 
 		ppc64_runlatch_on();
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/s390/kernel/process.c +2 -2

index db3e930..44028ae 100644
@@ -90,10 +90,10 @@ static void default_idle(void)
 void cpu_idle(void)
 {
 	for (;;) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched())
 			default_idle();
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/sh/kernel/idle.c +2 -2

index 6015743..ad58e75 100644
@@ -89,7 +89,7 @@ void cpu_idle(void)
 
 	/* endless idle loop with no priority at all */
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 
 		while (!need_resched()) {
 			check_pgt_cache();

@@ -111,7 +111,7 @@ void cpu_idle(void)
 			start_critical_timings();
 		}
 
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/sparc/kernel/process_64.c +2 -2

index 1235f63..78b1bc0 100644
@@ -95,12 +95,12 @@ void cpu_idle(void)
 	set_thread_flag(TIF_POLLING_NRFLAG);
 
 	while(1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 
 		while (!need_resched() && !cpu_is_offline(cpu))
 			sparc64_yield(cpu);
 
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 
 		preempt_enable_no_resched();
 

arch/tile/kernel/process.c +2 -2

index 920e674..53ac895 100644
@@ -85,7 +85,7 @@ void cpu_idle(void)
 
 	/* endless idle loop with no priority at all */
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched()) {
 			if (cpu_is_offline(cpu))
 				BUG();  /* no HOTPLUG_CPU */

@@ -105,7 +105,7 @@ void cpu_idle(void)
 				local_irq_enable();
 			current_thread_info()->status |= TS_POLLING;
 		}
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/um/kernel/process.c +2 -2

index 41acf59..9e7176b 100644
@@ -245,10 +245,10 @@ void default_idle(void)
 		if (need_resched())
 			schedule();
 
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		nsecs = disable_timer();
 		idle_sleep(nsecs);
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 	}
 }
 

arch/unicore32/kernel/process.c +2 -2

index 9999b9a..095ff5a 100644
@@ -55,7 +55,7 @@ void cpu_idle(void)
 {
 	/* endless idle loop with no priority at all */
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched()) {
 			local_irq_disable();
 			stop_critical_timings();

@@ -63,7 +63,7 @@ void cpu_idle(void)
 			local_irq_enable();
 			start_critical_timings();
 		}
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/x86/kernel/process_32.c +2 -2

index ad93205..f311d096 100644
@@ -98,7 +98,7 @@ void cpu_idle(void)
 
 	/* endless idle loop with no priority at all */
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched()) {
 
 			check_pgt_cache();

@@ -114,7 +114,7 @@ void cpu_idle(void)
 				pm_idle();
 			start_critical_timings();
 		}
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/x86/kernel/process_64.c +2 -2

index 9ca714e..e72daf9 100644
@@ -121,7 +121,7 @@ void cpu_idle(void)
 
 	/* endless idle loop with no priority at all */
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched()) {
 
 			rmb();

@@ -147,7 +147,7 @@ void cpu_idle(void)
 			__exit_idle();
 		}
 
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

include/linux/tick.h +42 -3

index 0df1d50..7224396 100644
@@ -7,6 +7,7 @@
 #define _LINUX_TICK_H
 
 #include <linux/clockchips.h>
+#include <linux/irqflags.h>
 
 #ifdef CONFIG_GENERIC_CLOCKEVENTS
 

@@ -121,18 +122,56 @@ static inline int tick_oneshot_mode_active(void) { return 0; }
 #endif /* !CONFIG_GENERIC_CLOCKEVENTS */
 
 # ifdef CONFIG_NO_HZ
-extern void tick_nohz_idle_enter(void);
+extern void __tick_nohz_idle_enter(void);
+static inline void tick_nohz_idle_enter(void)
+{
+	local_irq_disable();
+	__tick_nohz_idle_enter();
+	local_irq_enable();
+}
 extern void tick_nohz_idle_exit(void);
+
+/*
+ * Call this pair of function if the arch doesn't make any use
+ * of RCU in-between. You won't need to call rcu_idle_enter() and
+ * rcu_idle_exit().
+ * Otherwise you need to call tick_nohz_idle_enter() and tick_nohz_idle_exit()
+ * and explicitly tell RCU about the window around the place the CPU enters low
+ * power mode where no RCU use is made. This is done by calling rcu_idle_enter()
+ * after the last use of RCU before the CPU is put to sleep and by calling
+ * rcu_idle_exit() before the first use of RCU after the CPU woke up.
+ */
+static inline void tick_nohz_idle_enter_norcu(void)
+{
+	/*
+	 * Also call rcu_idle_enter() in the irq disabled section even
+	 * if it disables irq itself.
+	 * Just an optimization that prevents from an interrupt happening
+	 * between it and __tick_nohz_idle_enter() to lose time to help completing
+	 * a grace period while we could be in extended grace period already.
+	 */
+	local_irq_disable();
+	__tick_nohz_idle_enter();
+	rcu_idle_enter();
+	local_irq_enable();
+}
+static inline void tick_nohz_idle_exit_norcu(void)
+{
+	rcu_idle_exit();
+	tick_nohz_idle_exit();
+}
 extern void tick_nohz_irq_exit(void);
 extern ktime_t tick_nohz_get_sleep_length(void);
 extern u64 get_cpu_idle_time_us(int cpu, u64 *last_update_time);
 extern u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time);
 # else
-static inline void tick_nohz_idle_enter(void)
+static inline void tick_nohz_idle_enter(void) { }
+static inline void tick_nohz_idle_exit(void) { }
+static inline void tick_nohz_idle_enter_norcu(void)
 {
 	rcu_idle_enter();
 }
-static inline void tick_nohz_idle_exit(void)
+static inline void tick_nohz_idle_exit_norcu(void)
 {
 	rcu_idle_exit();
 }

kernel/time/tick-sched.c +13 -12

index 52b7ace..360d028 100644
@@ -424,18 +424,22 @@ out:
  *
  * When the next event is more than a tick into the future, stop the idle tick
  * Called when we start the idle loop.
- * This also enters into RCU extended quiescent state so that this CPU doesn't
- * need anymore to be part of any global grace period completion. This way
- * the tick can be stopped safely as we don't need to report quiescent states.
+ *
+ * If no use of RCU is made in the idle loop between
+ * tick_nohz_idle_enter() and tick_nohz_idle_exit() calls, then
+ * tick_nohz_idle_enter_norcu() should be called instead and the arch
+ * doesn't need to call rcu_idle_enter() and rcu_idle_exit() explicitly.
+ *
+ * Otherwise the arch is responsible of calling:
+ *
+ * - rcu_idle_enter() after its last use of RCU before the CPU is put
+ *  to sleep.
+ * - rcu_idle_exit() before the first use of RCU after the CPU is woken up.
  */
-void tick_nohz_idle_enter(void)
+void __tick_nohz_idle_enter(void)
 {
 	struct tick_sched *ts;
 
-	WARN_ON_ONCE(irqs_disabled());
-
-	local_irq_disable();
-
 	ts = &__get_cpu_var(tick_cpu_sched);
 	/*
 	 * set ts->inidle unconditionally. even if the system did not

@@ -444,9 +448,6 @@ void tick_nohz_idle_enter(void)
 	 */
 	ts->inidle = 1;
 	tick_nohz_stop_sched_tick(ts);
-	rcu_idle_enter();
-
-	local_irq_enable();
 }
 
 /**

@@ -522,7 +523,7 @@ void tick_nohz_idle_exit(void)
 	ktime_t now;
 
 	local_irq_disable();
-	rcu_idle_exit();
+
 	if (ts->idle_active || (ts->inidle && ts->tick_stopped))
 		now = ktime_get();
 

Signature · 7 lines

1.7.5.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Quoted 13 lines · Frederic Weisbecker

It is assumed that rcu won't be used once we switch to tickless mode and until we restart the tick. However this is not always true, as in x86-64 where we dereference the idle notifiers after the tick is stopped.

To prepare for fixing this, add two new APIs:
tick_nohz_idle_enter_norcu() and tick_nohz_idle_exit_norcu().

If no use of RCU is made in the idle loop between tick_nohz_enter_idle() and tick_nohz_exit_idle() calls, the arch must instead call the new *_norcu() version such that the arch doesn't need to call rcu_idle_enter() and rcu_idle_exit().

Quoted 7 lines

Otherwise the arch must call tick_nohz_enter_idle() and tick_nohz_exit_idle() and also call explicitly:

- rcu_idle_enter() after its last use of RCU before the CPU is put to sleep.
- rcu_idle_exit() before the first use of RCU after the CPU is woken up.

Thank you, Frederic! I have queued this to replace the earlier version. The set is available on branch rcu/dyntick of

https://github.com/paulmckrcu/linux

Quoted 513 lines

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Mike Frysinger <[email protected]>

Cc Guan Xuetao <[email protected]>

Cc David Miller <[email protected]>

Cc Chris Metcalf <[email protected]>

Cc Hans-Christian Egtvedt <[email protected]>

Cc Ralf Baechle <[email protected]>

Cc Paul E. McKenney <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Russell King <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Heiko Carstens <[email protected]>

Cc Paul Mundt <[email protected]>

---
 arch/arm/kernel/process.c              |    4 +-
 arch/avr32/kernel/process.c            |    4 +-
 arch/blackfin/kernel/process.c         |    4 +-
 arch/microblaze/kernel/process.c       |    4 +-
 arch/mips/kernel/process.c             |    4 +-
 arch/openrisc/kernel/idle.c            |    4 +-
 arch/powerpc/kernel/idle.c             |    4 +-
 arch/powerpc/platforms/iseries/setup.c |    8 +++---
 arch/s390/kernel/process.c             |    4 +-
 arch/sh/kernel/idle.c                  |    4 +-
 arch/sparc/kernel/process_64.c         |    4 +-
 arch/tile/kernel/process.c             |    4 +-
 arch/um/kernel/process.c               |    4 +-
 arch/unicore32/kernel/process.c        |    4 +-
 arch/x86/kernel/process_32.c           |    4 +-
 arch/x86/kernel/process_64.c           |    4 +-
 include/linux/tick.h                   |   45 +++++++++++++++++++++++++++++--
 kernel/time/tick-sched.c               |   25 +++++++++--------
 18 files changed, 89 insertions(+), 49 deletions(-)

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

index f9261d0..4f83362 100644
@@ -183,7 +183,7 @@ void cpu_idle(void)
 
 	/* endless idle loop with no priority at all */
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		leds_event(led_idle_start);
 		while (!need_resched()) {
 #ifdef CONFIG_HOTPLUG_CPU

@@ -210,7 +210,7 @@ void cpu_idle(void)
 			}
 		}
 		leds_event(led_idle_end);
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/avr32/kernel/process.c +2 -2

index 6ee7952..34c8c70 100644
@@ -34,10 +34,10 @@ void cpu_idle(void)
 {
 	/* endless idle loop with no priority at all */
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched())
 			cpu_idle_sleep();
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/blackfin/kernel/process.c +2 -2

index 7b141b5..57e0749 100644
@@ -88,10 +88,10 @@ void cpu_idle(void)
 #endif
 		if (!idle)
 			idle = default_idle;
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched())
 			idle();
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/microblaze/kernel/process.c +2 -2

index 6dc123e..c6ece38 100644
@@ -103,10 +103,10 @@ void cpu_idle(void)
 		if (!idle)
 			idle = default_idle;
 
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched())
 			idle();
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 
 		preempt_enable_no_resched();
 		schedule();

arch/mips/kernel/process.c +2 -2

index d50a005..7df2ffc 100644
@@ -56,7 +56,7 @@ void __noreturn cpu_idle(void)
 
 	/* endless idle loop with no priority at all */
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched() && cpu_online(cpu)) {
 #ifdef CONFIG_MIPS_MT_SMTC
 			extern void smtc_idle_loop_hook(void);

@@ -77,7 +77,7 @@ void __noreturn cpu_idle(void)
 		     system_state == SYSTEM_BOOTING))
 			play_dead();
 #endif
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/openrisc/kernel/idle.c +2 -2

index fb6a9bf..2e82cd0 100644
@@ -51,7 +51,7 @@ void cpu_idle(void)
 
 	/* endless idle loop with no priority at all */
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 
 		while (!need_resched()) {
 			check_pgt_cache();

@@ -69,7 +69,7 @@ void cpu_idle(void)
 			set_thread_flag(TIF_POLLING_NRFLAG);
 		}
 
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/powerpc/kernel/idle.c +2 -2

index 878572f..2e782a3 100644
@@ -56,7 +56,7 @@ void cpu_idle(void)
 
 	set_thread_flag(TIF_POLLING_NRFLAG);
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched() && !cpu_should_die()) {
 			ppc64_runlatch_off();
 

@@ -93,7 +93,7 @@ void cpu_idle(void)
 
 		HMT_medium();
 		ppc64_runlatch_on();
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		if (cpu_should_die())
 			cpu_die();

arch/powerpc/platforms/iseries/setup.c +4 -4

index e2f5fad..77ff6eb 100644
@@ -562,7 +562,7 @@ static void yield_shared_processor(void)
 static void iseries_shared_idle(void)
 {
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched() && !hvlpevent_is_pending()) {
 			local_irq_disable();
 			ppc64_runlatch_off();

@@ -576,7 +576,7 @@ static void iseries_shared_idle(void)
 		}
 
 		ppc64_runlatch_on();
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 
 		if (hvlpevent_is_pending())
 			process_iSeries_events();

@@ -592,7 +592,7 @@ static void iseries_dedicated_idle(void)
 	set_thread_flag(TIF_POLLING_NRFLAG);
 
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		if (!need_resched()) {
 			while (!need_resched()) {
 				ppc64_runlatch_off();

@@ -609,7 +609,7 @@ static void iseries_dedicated_idle(void)
 		}
 
 		ppc64_runlatch_on();
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/s390/kernel/process.c +2 -2

index db3e930..44028ae 100644
@@ -90,10 +90,10 @@ static void default_idle(void)
 void cpu_idle(void)
 {
 	for (;;) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched())
 			default_idle();
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/sh/kernel/idle.c +2 -2

index 6015743..ad58e75 100644
@@ -89,7 +89,7 @@ void cpu_idle(void)
 
 	/* endless idle loop with no priority at all */
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 
 		while (!need_resched()) {
 			check_pgt_cache();

@@ -111,7 +111,7 @@ void cpu_idle(void)
 			start_critical_timings();
 		}
 
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/sparc/kernel/process_64.c +2 -2

index 1235f63..78b1bc0 100644
@@ -95,12 +95,12 @@ void cpu_idle(void)
 	set_thread_flag(TIF_POLLING_NRFLAG);
 
 	while(1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 
 		while (!need_resched() && !cpu_is_offline(cpu))
 			sparc64_yield(cpu);
 
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 
 		preempt_enable_no_resched();
 

arch/tile/kernel/process.c +2 -2

index 920e674..53ac895 100644
@@ -85,7 +85,7 @@ void cpu_idle(void)
 
 	/* endless idle loop with no priority at all */
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched()) {
 			if (cpu_is_offline(cpu))
 				BUG();  /* no HOTPLUG_CPU */

@@ -105,7 +105,7 @@ void cpu_idle(void)
 				local_irq_enable();
 			current_thread_info()->status |= TS_POLLING;
 		}
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/um/kernel/process.c +2 -2

index 41acf59..9e7176b 100644
@@ -245,10 +245,10 @@ void default_idle(void)
 		if (need_resched())
 			schedule();
 
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		nsecs = disable_timer();
 		idle_sleep(nsecs);
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 	}
 }
 

arch/unicore32/kernel/process.c +2 -2

index 9999b9a..095ff5a 100644
@@ -55,7 +55,7 @@ void cpu_idle(void)
 {
 	/* endless idle loop with no priority at all */
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched()) {
 			local_irq_disable();
 			stop_critical_timings();

@@ -63,7 +63,7 @@ void cpu_idle(void)
 			local_irq_enable();
 			start_critical_timings();
 		}
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/x86/kernel/process_32.c +2 -2

index ad93205..f311d096 100644
@@ -98,7 +98,7 @@ void cpu_idle(void)
 
 	/* endless idle loop with no priority at all */
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched()) {
 
 			check_pgt_cache();

@@ -114,7 +114,7 @@ void cpu_idle(void)
 				pm_idle();
 			start_critical_timings();
 		}
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

arch/x86/kernel/process_64.c +2 -2

index 9ca714e..e72daf9 100644
@@ -121,7 +121,7 @@ void cpu_idle(void)
 
 	/* endless idle loop with no priority at all */
 	while (1) {
-		tick_nohz_idle_enter();
+		tick_nohz_idle_enter_norcu();
 		while (!need_resched()) {
 
 			rmb();

@@ -147,7 +147,7 @@ void cpu_idle(void)
 			__exit_idle();
 		}
 
-		tick_nohz_idle_exit();
+		tick_nohz_idle_exit_norcu();
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();

include/linux/tick.h +42 -3

index 0df1d50..7224396 100644
@@ -7,6 +7,7 @@
 #define _LINUX_TICK_H
 
 #include <linux/clockchips.h>
+#include <linux/irqflags.h>
 
 #ifdef CONFIG_GENERIC_CLOCKEVENTS
 

@@ -121,18 +122,56 @@ static inline int tick_oneshot_mode_active(void) { return 0; }
 #endif /* !CONFIG_GENERIC_CLOCKEVENTS */
 
 # ifdef CONFIG_NO_HZ
-extern void tick_nohz_idle_enter(void);
+extern void __tick_nohz_idle_enter(void);
+static inline void tick_nohz_idle_enter(void)
+{
+	local_irq_disable();
+	__tick_nohz_idle_enter();
+	local_irq_enable();
+}
 extern void tick_nohz_idle_exit(void);
+
+/*
+ * Call this pair of function if the arch doesn't make any use
+ * of RCU in-between. You won't need to call rcu_idle_enter() and
+ * rcu_idle_exit().
+ * Otherwise you need to call tick_nohz_idle_enter() and tick_nohz_idle_exit()
+ * and explicitly tell RCU about the window around the place the CPU enters low
+ * power mode where no RCU use is made. This is done by calling rcu_idle_enter()
+ * after the last use of RCU before the CPU is put to sleep and by calling
+ * rcu_idle_exit() before the first use of RCU after the CPU woke up.
+ */
+static inline void tick_nohz_idle_enter_norcu(void)
+{
+	/*
+	 * Also call rcu_idle_enter() in the irq disabled section even
+	 * if it disables irq itself.
+	 * Just an optimization that prevents from an interrupt happening
+	 * between it and __tick_nohz_idle_enter() to lose time to help completing
+	 * a grace period while we could be in extended grace period already.
+	 */
+	local_irq_disable();
+	__tick_nohz_idle_enter();
+	rcu_idle_enter();
+	local_irq_enable();
+}
+static inline void tick_nohz_idle_exit_norcu(void)
+{
+	rcu_idle_exit();
+	tick_nohz_idle_exit();
+}
 extern void tick_nohz_irq_exit(void);
 extern ktime_t tick_nohz_get_sleep_length(void);
 extern u64 get_cpu_idle_time_us(int cpu, u64 *last_update_time);
 extern u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time);
 # else
-static inline void tick_nohz_idle_enter(void)
+static inline void tick_nohz_idle_enter(void) { }
+static inline void tick_nohz_idle_exit(void) { }
+static inline void tick_nohz_idle_enter_norcu(void)
 {
 	rcu_idle_enter();
 }
-static inline void tick_nohz_idle_exit(void)
+static inline void tick_nohz_idle_exit_norcu(void)
 {
 	rcu_idle_exit();
 }

kernel/time/tick-sched.c +13 -12

index 52b7ace..360d028 100644
@@ -424,18 +424,22 @@ out:
  *
  * When the next event is more than a tick into the future, stop the idle tick
  * Called when we start the idle loop.
- * This also enters into RCU extended quiescent state so that this CPU doesn't
- * need anymore to be part of any global grace period completion. This way
- * the tick can be stopped safely as we don't need to report quiescent states.
+ *
+ * If no use of RCU is made in the idle loop between
+ * tick_nohz_idle_enter() and tick_nohz_idle_exit() calls, then
+ * tick_nohz_idle_enter_norcu() should be called instead and the arch
+ * doesn't need to call rcu_idle_enter() and rcu_idle_exit() explicitly.
+ *
+ * Otherwise the arch is responsible of calling:
+ *
+ * - rcu_idle_enter() after its last use of RCU before the CPU is put
+ *  to sleep.
+ * - rcu_idle_exit() before the first use of RCU after the CPU is woken up.
  */
-void tick_nohz_idle_enter(void)
+void __tick_nohz_idle_enter(void)
 {
 	struct tick_sched *ts;
 
-	WARN_ON_ONCE(irqs_disabled());
-
-	local_irq_disable();
-
 	ts = &__get_cpu_var(tick_cpu_sched);
 	/*
 	 * set ts->inidle unconditionally. even if the system did not

@@ -444,9 +448,6 @@ void tick_nohz_idle_enter(void)
 	 */
 	ts->inidle = 1;
 	tick_nohz_stop_sched_tick(ts);
-	rcu_idle_enter();
-
-	local_irq_enable();
 }
 
 /**

@@ -522,7 +523,7 @@ void tick_nohz_idle_exit(void)
 	ktime_t now;
 
 	local_irq_disable();
-	rcu_idle_exit();
+
 	if (ts->idle_active || (ts->inidle && ts->tick_stopped))
 		now = ktime_get();
 

Signature · 7 lines

1.7.5.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Quoted 22 lines · Paul E. McKenney

Quoted 13 lines · Frederic Weisbecker

It is assumed that rcu won't be used once we switch to tickless mode and until we restart the tick. However this is not always true, as in x86-64 where we dereference the idle notifiers after the tick is stopped.

To prepare for fixing this, add two new APIs:
tick_nohz_idle_enter_norcu() and tick_nohz_idle_exit_norcu().

If no use of RCU is made in the idle loop between tick_nohz_enter_idle() and tick_nohz_exit_idle() calls, the arch must instead call the new *_norcu() version such that the arch doesn't need to call rcu_idle_enter() and rcu_idle_exit().

Quoted 7 lines

Otherwise the arch must call tick_nohz_enter_idle() and tick_nohz_exit_idle() and also call explicitly:

- rcu_idle_enter() after its last use of RCU before the CPU is put to sleep.
- rcu_idle_exit() before the first use of RCU after the CPU is woken up.

Quoted 4 lines

Thank you, Frederic! I have queued this to replace the earlier version. The set is available on branch rcu/dyntick of

https://github.com/paulmckrcu/linux

Which reminds me... About the ultimate objective, getting tick-free operation. (Or, for the guys who want to eliminate the tick entirely, shutting up the hrtimer stuff that they want to replace it with.)

I believe that you will then need to have two levels of not-in-dynticks for processes, one for idle vs. not and another for when a process switches from user-space to kernel execution. Correct, or am I confused?

The reason I ask is that commit e11f5981 currently only allows one level of not-in-dynticks for processes. It is easy to add another level, but thought I should check beforehand.

Quoted 32 lines · Paul E. McKenney

Quoted 22 lines · Paul E. McKenney

Quoted 13 lines · Frederic Weisbecker

It is assumed that rcu won't be used once we switch to tickless mode and until we restart the tick. However this is not always true, as in x86-64 where we dereference the idle notifiers after the tick is stopped.

To prepare for fixing this, add two new APIs:
tick_nohz_idle_enter_norcu() and tick_nohz_idle_exit_norcu().

If no use of RCU is made in the idle loop between tick_nohz_enter_idle() and tick_nohz_exit_idle() calls, the arch must instead call the new *_norcu() version such that the arch doesn't need to call rcu_idle_enter() and rcu_idle_exit().

Quoted 7 lines

Otherwise the arch must call tick_nohz_enter_idle() and tick_nohz_exit_idle() and also call explicitly:

- rcu_idle_enter() after its last use of RCU before the CPU is put to sleep.
- rcu_idle_exit() before the first use of RCU after the CPU is woken up.

Quoted 4 lines

Thank you, Frederic! I have queued this to replace the earlier version. The set is available on branch rcu/dyntick of

https://github.com/paulmckrcu/linux

Which reminds me... About the ultimate objective, getting tick-free operation. (Or, for the guys who want to eliminate the tick entirely, shutting up the hrtimer stuff that they want to replace it with.)

Quoted 8 lines

I believe that you will then need to have two levels of not-in-dynticks for processes, one for idle vs. not and another for when a process switches from user-space to kernel execution. Correct, or am I confused?

The reason I ask is that commit e11f5981 currently only allows one level of not-in-dynticks for processes. It is easy to add another level, but thought I should check beforehand.

Hmm, yeah looking at that patch, it's going to be hard to have a nesting that looks like:

rcu_irq_enter();
rcu_user_enter();
rcu_irq_exit(); <-- with effective extended quiescent state starting there

I also need to be able to call rcu_user_enter() from non-irq path.

I don't truly understand the problem of the usermode helpers that mess up the dynticks counts. May be we can somewhow fix it differently from the offending callsite?

Quoted 45 lines · Frederic Weisbecker

Quoted 32 lines · Paul E. McKenney

Quoted 22 lines · Paul E. McKenney

Quoted 13 lines · Frederic Weisbecker

It is assumed that rcu won't be used once we switch to tickless mode and until we restart the tick. However this is not always true, as in x86-64 where we dereference the idle notifiers after the tick is stopped.

To prepare for fixing this, add two new APIs:
tick_nohz_idle_enter_norcu() and tick_nohz_idle_exit_norcu().

If no use of RCU is made in the idle loop between tick_nohz_enter_idle() and tick_nohz_exit_idle() calls, the arch must instead call the new *_norcu() version such that the arch doesn't need to call rcu_idle_enter() and rcu_idle_exit().

Quoted 7 lines

Otherwise the arch must call tick_nohz_enter_idle() and tick_nohz_exit_idle() and also call explicitly:

- rcu_idle_enter() after its last use of RCU before the CPU is put to sleep.
- rcu_idle_exit() before the first use of RCU after the CPU is woken up.

Quoted 4 lines

Thank you, Frederic! I have queued this to replace the earlier version. The set is available on branch rcu/dyntick of

https://github.com/paulmckrcu/linux

Which reminds me... About the ultimate objective, getting tick-free operation. (Or, for the guys who want to eliminate the tick entirely, shutting up the hrtimer stuff that they want to replace it with.)

Quoted 8 lines

I believe that you will then need to have two levels of not-in-dynticks for processes, one for idle vs. not and another for when a process switches from user-space to kernel execution. Correct, or am I confused?

The reason I ask is that commit e11f5981 currently only allows one level of not-in-dynticks for processes. It is easy to add another level, but thought I should check beforehand.

Hmm, yeah looking at that patch, it's going to be hard to have a nesting that looks like:

Quoted 3 lines

rcu_irq_enter();
rcu_user_enter();
rcu_irq_exit(); <-- with effective extended quiescent state starting there

OK, so the idea here is that there has been two runnable processes on the current CPU, but during the irq handler one of them moves or some such?

If so, how about a rcu_user_enter_fromirq() that sets the counter to 1 so that the rcu_irq_exit() cleans up properly? If need be, I could of course provide an argument to allow you to specify the count offset.

Quoted 1 line

I also need to be able to call rcu_user_enter() from non-irq path.

Then rcu_user_enter_fromirq() would be for the irq path and rcu_user_enter() from the non-irq path.

Would that work for you?

Quoted 4 lines

I don't truly understand the problem of the usermode helpers that mess up the dynticks counts. May be we can somewhow fix it differently from the offending callsite?

I tried a few approaches along these lines, but there were way too many opportunities for interruption and preemption along the way. The problem is that unless the fixup happens under a no-preempt region of code that includes the rcu_irq_enter() or rcu_irq_exit() call (as the case may be), then you end up messing up the idle-depth count of two CPUs rather than just one. :-(

But maybe I am missing something -- suggestions more than welcome!

Quoted 50 lines · Paul E. McKenney

Quoted 48 lines · Frederic Weisbecker

Quoted 32 lines · Paul E. McKenney

Quoted 22 lines · Paul E. McKenney

Quoted 13 lines · Frederic Weisbecker

It is assumed that rcu won't be used once we switch to tickless mode and until we restart the tick. However this is not always true, as in x86-64 where we dereference the idle notifiers after the tick is stopped.

To prepare for fixing this, add two new APIs:
tick_nohz_idle_enter_norcu() and tick_nohz_idle_exit_norcu().

If no use of RCU is made in the idle loop between tick_nohz_enter_idle() and tick_nohz_exit_idle() calls, the arch must instead call the new *_norcu() version such that the arch doesn't need to call rcu_idle_enter() and rcu_idle_exit().

Quoted 7 lines

Otherwise the arch must call tick_nohz_enter_idle() and tick_nohz_exit_idle() and also call explicitly:

- rcu_idle_enter() after its last use of RCU before the CPU is put to sleep.
- rcu_idle_exit() before the first use of RCU after the CPU is woken up.

Quoted 4 lines

Thank you, Frederic! I have queued this to replace the earlier version. The set is available on branch rcu/dyntick of

https://github.com/paulmckrcu/linux

Which reminds me... About the ultimate objective, getting tick-free operation. (Or, for the guys who want to eliminate the tick entirely, shutting up the hrtimer stuff that they want to replace it with.)

Quoted 8 lines

I believe that you will then need to have two levels of not-in-dynticks for processes, one for idle vs. not and another for when a process switches from user-space to kernel execution. Correct, or am I confused?

The reason I ask is that commit e11f5981 currently only allows one level of not-in-dynticks for processes. It is easy to add another level, but thought I should check beforehand.

Hmm, yeah looking at that patch, it's going to be hard to have a nesting that looks like:

rcu_irq_enter();
rcu_user_enter();
rcu_irq_exit(); <-- with effective extended quiescent state starting there

Quoted 3 lines

OK, so the idea here is that there has been two runnable processes on the current CPU, but during the irq handler one of them moves or some such?

No it happens when we have an irq in userspace and we stop the tick from that irq. Noticing we are in userspace, we want to be in extended quiescent state when we resume from the interrupt to userspace.

Quoted 3 lines

If so, how about a rcu_user_enter_fromirq() that sets the counter to 1 so that the rcu_irq_exit() cleans up properly? If need be, I could of course provide an argument to allow you to specify the count offset.

Yeah I think that should work.

Quoted 6 lines

Quoted 1 line

I also need to be able to call rcu_user_enter() from non-irq path.

Then rcu_user_enter_fromirq() would be for the irq path and rcu_user_enter() from the non-irq path.

Would that work for you?

Yep!

Quoted 11 lines

Quoted 3 lines

I don't truly understand the problem of the usermode helpers that mess up the dynticks counts. May be we can somewhow fix it differently from the offending callsite?

I tried a few approaches along these lines, but there were way too many opportunities for interruption and preemption along the way. The problem is that unless the fixup happens under a no-preempt region of code that includes the rcu_irq_enter() or rcu_irq_exit() call (as the case may be), then you end up messing up the idle-depth count of two CPUs rather than just one. :-(

Quoted 1 line

But maybe I am missing something -- suggestions more than welcome!

It's rather me missing everything :)
It happens when we call call_usermodehelper()? If so how? We have a call to rcu_irq_enter() that lacks an rcu_irq_exit() ?

Quoted 51 lines · Frederic Weisbecker

Quoted 50 lines · Paul E. McKenney

Quoted 48 lines · Frederic Weisbecker

Quoted 32 lines · Paul E. McKenney

Quoted 22 lines · Paul E. McKenney

Quoted 13 lines · Frederic Weisbecker

It is assumed that rcu won't be used once we switch to tickless mode and until we restart the tick. However this is not always true, as in x86-64 where we dereference the idle notifiers after the tick is stopped.

To prepare for fixing this, add two new APIs:
tick_nohz_idle_enter_norcu() and tick_nohz_idle_exit_norcu().

If no use of RCU is made in the idle loop between tick_nohz_enter_idle() and tick_nohz_exit_idle() calls, the arch must instead call the new *_norcu() version such that the arch doesn't need to call rcu_idle_enter() and rcu_idle_exit().

Quoted 7 lines

Otherwise the arch must call tick_nohz_enter_idle() and tick_nohz_exit_idle() and also call explicitly:

- rcu_idle_enter() after its last use of RCU before the CPU is put to sleep.
- rcu_idle_exit() before the first use of RCU after the CPU is woken up.

Quoted 4 lines

Thank you, Frederic! I have queued this to replace the earlier version. The set is available on branch rcu/dyntick of

https://github.com/paulmckrcu/linux

Which reminds me... About the ultimate objective, getting tick-free operation. (Or, for the guys who want to eliminate the tick entirely, shutting up the hrtimer stuff that they want to replace it with.)

Quoted 8 lines

I believe that you will then need to have two levels of not-in-dynticks for processes, one for idle vs. not and another for when a process switches from user-space to kernel execution. Correct, or am I confused?

The reason I ask is that commit e11f5981 currently only allows one level of not-in-dynticks for processes. It is easy to add another level, but thought I should check beforehand.

Hmm, yeah looking at that patch, it's going to be hard to have a nesting that looks like:

rcu_irq_enter();
rcu_user_enter();
rcu_irq_exit(); <-- with effective extended quiescent state starting there

Quoted 7 lines

Quoted 3 lines

OK, so the idea here is that there has been two runnable processes on the current CPU, but during the irq handler one of them moves or some such?

No it happens when we have an irq in userspace and we stop the tick from that irq. Noticing we are in userspace, we want to be in extended quiescent state when we resume from the interrupt to userspace.

Ah, OK!

Quoted 5 lines

Quoted 3 lines

If so, how about a rcu_user_enter_fromirq() that sets the counter to 1 so that the rcu_irq_exit() cleans up properly? If need be, I could of course provide an argument to allow you to specify the count offset.

Yeah I think that should work.

Very good. I will start off with no argument, easy enough to add it later if needed.

Quoted 7 lines

Quoted 6 lines

Quoted 1 line

I also need to be able to call rcu_user_enter() from non-irq path.

Then rcu_user_enter_fromirq() would be for the irq path and rcu_user_enter() from the non-irq path.

Would that work for you?

Quoted 1 line

Yep!

Very good, I will take a whack at it. BTW, testing is going quite well thus far with your current patches combined with my paranoid idle-count approach. One test in particular that previously failed reliably within minutes just successfully completed a ten-hour run. So things are looking up! (Famous last words...)

Quoted 12 lines

Quoted 12 lines

Quoted 4 lines

I don't truly understand the problem of the usermode helpers that mess up the dynticks counts. May be we can somewhow fix it differently from the offending callsite?

I tried a few approaches along these lines, but there were way too many opportunities for interruption and preemption along the way. The problem is that unless the fixup happens under a no-preempt region of code that includes the rcu_irq_enter() or rcu_irq_exit() call (as the case may be), then you end up messing up the idle-depth count of two CPUs rather than just one. :-(

Quoted 5 lines

Quoted 1 line

But maybe I am missing something -- suggestions more than welcome!

It's rather me missing everything :)
It happens when we call call_usermodehelper()? If so how? We have a call to rcu_irq_enter() that lacks an rcu_irq_exit() ?

On powerpc, it executes the "sc" ("system call") instruction from kernel mode, which results in an exception. But from what I can see, there is no corresponding return from exception, so my not-so-paranoid counting scheme would lose count. That said, please keep in mind that I in no way fully understand that code. It is also far from clear to me why my earlier dyntick-idle code worked in this situation -- perhaps the value of preempt_count() gets fixed up somehow -- I haven't really studied all the assembly language involved in detail, so there is lots of opportunity for such a fixup somewhere.

You asked! ;-)

Quoted 66 lines · Paul E. McKenney

Quoted 51 lines · Frederic Weisbecker

Quoted 50 lines · Paul E. McKenney

Quoted 48 lines · Frederic Weisbecker

Quoted 32 lines · Paul E. McKenney

Quoted 22 lines · Paul E. McKenney

Quoted 13 lines · Frederic Weisbecker

It is assumed that rcu won't be used once we switch to tickless mode and until we restart the tick. However this is not always true, as in x86-64 where we dereference the idle notifiers after the tick is stopped.

To prepare for fixing this, add two new APIs:
tick_nohz_idle_enter_norcu() and tick_nohz_idle_exit_norcu().

If no use of RCU is made in the idle loop between tick_nohz_enter_idle() and tick_nohz_exit_idle() calls, the arch must instead call the new *_norcu() version such that the arch doesn't need to call rcu_idle_enter() and rcu_idle_exit().

Quoted 7 lines

Otherwise the arch must call tick_nohz_enter_idle() and tick_nohz_exit_idle() and also call explicitly:

- rcu_idle_enter() after its last use of RCU before the CPU is put to sleep.
- rcu_idle_exit() before the first use of RCU after the CPU is woken up.

Quoted 4 lines

Thank you, Frederic! I have queued this to replace the earlier version. The set is available on branch rcu/dyntick of

https://github.com/paulmckrcu/linux

Which reminds me... About the ultimate objective, getting tick-free operation. (Or, for the guys who want to eliminate the tick entirely, shutting up the hrtimer stuff that they want to replace it with.)

Quoted 8 lines

I believe that you will then need to have two levels of not-in-dynticks for processes, one for idle vs. not and another for when a process switches from user-space to kernel execution. Correct, or am I confused?

The reason I ask is that commit e11f5981 currently only allows one level of not-in-dynticks for processes. It is easy to add another level, but thought I should check beforehand.

Hmm, yeah looking at that patch, it's going to be hard to have a nesting that looks like:

rcu_irq_enter();
rcu_user_enter();
rcu_irq_exit(); <-- with effective extended quiescent state starting there

Quoted 7 lines

Quoted 3 lines

OK, so the idea here is that there has been two runnable processes on the current CPU, but during the irq handler one of them moves or some such?

No it happens when we have an irq in userspace and we stop the tick from that irq. Noticing we are in userspace, we want to be in extended quiescent state when we resume from the interrupt to userspace.

Ah, OK!

Quoted 4 lines

Quoted 3 lines

If so, how about a rcu_user_enter_fromirq() that sets the counter to 1 so that the rcu_irq_exit() cleans up properly? If need be, I could of course provide an argument to allow you to specify the count offset.

Quoted 4 lines

Quoted 1 line

Yeah I think that should work.

Very good. I will start off with no argument, easy enough to add it later if needed.

Yeah, but for now, this is out of tree code. The functions seem quite trivial to write so I guess I can sort it out in my tree when I'll rebase on 3.2 (if the current queue goes to the next merge window)

Quoted 9 lines

Quoted 8 lines

Quoted 6 lines

Quoted 1 line

I also need to be able to call rcu_user_enter() from non-irq path.

Then rcu_user_enter_fromirq() would be for the irq path and rcu_user_enter() from the non-irq path.

Would that work for you?

Yep!

Quoted 5 lines

Very good, I will take a whack at it. BTW, testing is going quite well thus far with your current patches combined with my paranoid idle-count approach. One test in particular that previously failed reliably within minutes just successfully completed a ten-hour run. So things are looking up! (Famous last words...)

Great, I cross my fingers! :)

Quoted 27 lines

Quoted 11 lines

Quoted 11 lines

Quoted 3 lines

I don't truly understand the problem of the usermode helpers that mess up the dynticks counts. May be we can somewhow fix it differently from the offending callsite?

I tried a few approaches along these lines, but there were way too many opportunities for interruption and preemption along the way. The problem is that unless the fixup happens under a no-preempt region of code that includes the rcu_irq_enter() or rcu_irq_exit() call (as the case may be), then you end up messing up the idle-depth count of two CPUs rather than just one. :-(

Quoted 5 lines

Quoted 1 line

But maybe I am missing something -- suggestions more than welcome!

It's rather me missing everything :)
It happens when we call call_usermodehelper()? If so how? We have a call to rcu_irq_enter() that lacks an rcu_irq_exit() ?

On powerpc, it executes the "sc" ("system call") instruction from kernel mode, which results in an exception. But from what I can see, there is no corresponding return from exception, so my not-so-paranoid counting scheme would lose count. That said, please keep in mind that I in no way fully understand that code. It is also far from clear to me why my earlier dyntick-idle code worked in this situation -- perhaps the value of preempt_count() gets fixed up somehow -- I haven't really studied all the assembly language involved in detail, so there is lots of opportunity for such a fixup somewhere.

Quoted 1 line

You asked! ;-)

Haha! ;)

Hi Paul,

Here is the rebase. And it's more than just a rebase given the semantical changes after your patch that tracks idleness. And also because of the new API naming (with a new pair of tick_nohz_idle_enter() tick_nohz_idle_exit() with *_norcu suffixes).

I have reused and updated some comments (that you made on earlier versions of my patchset) that assumed that extended qs = dynticks idle, as it's not always true anymore. I've tried to bring a new wording to express what we are dealing with: "RCU-free window in idle" or "RCU-idle window". I let you update the comments if you think that's confusing or too scarce.

It has passed several hours of rcutorture with NO_HZ && SMP, and at least booted fine with all your configs.

Frederic Weisbecker (9):
rcu: Detect illegal rcu dereference in extended quiescent state
rcu: Inform the user about extended quiescent state on PROVE_RCU warning
rcu: Warn when rcu_read_lock() is used in extended quiescent state
rcu: Make srcu_read_lock_held() call common lockdep-enabled function
nohz: Separate out irq exit and idle loop dyntick logic
nohz: Allow rcu extended quiescent state handling seperately from tick stop
x86: Enter rcu extended qs after idle notifier call
x86: Call idle notifier after irq_enter()
rcu: Fix early call to rcu_idle_enter()

Paul E. McKenney (2):
rcu: Remove one layer of abstraction from PROVE_RCU checking
rcu: Warn when srcu_read_lock() is used in an extended quiescent state

 arch/arm/kernel/process.c                |    4 +-
 arch/avr32/kernel/process.c              |    4 +-
 arch/blackfin/kernel/process.c           |    4 +-
 arch/microblaze/kernel/process.c         |    4 +-
 arch/mips/kernel/process.c               |    4 +-
 arch/openrisc/kernel/idle.c              |    4 +-
 arch/powerpc/kernel/idle.c               |    4 +-
 arch/powerpc/platforms/iseries/setup.c   |    8 +-
 arch/s390/kernel/process.c               |    4 +-
 arch/sh/kernel/idle.c                    |    4 +-
 arch/sparc/kernel/process_64.c           |    4 +-
 arch/tile/kernel/process.c               |    4 +-
 arch/um/kernel/process.c                 |    4 +-
 arch/unicore32/kernel/process.c          |    4 +-
 arch/x86/kernel/apic/apic.c              |    6 +-
 arch/x86/kernel/apic/io_apic.c           |    2 +-
 arch/x86/kernel/cpu/mcheck/therm_throt.c |    2 +-
 arch/x86/kernel/cpu/mcheck/threshold.c   |    2 +-
 arch/x86/kernel/irq.c                    |    6 +-
 arch/x86/kernel/process_32.c             |    4 +-
 arch/x86/kernel/process_64.c             |   10 +++-
 include/linux/rcupdate.h                 |   74 +++++++++++++++--------
 include/linux/srcu.h                     |   38 ++++++++----
 include/linux/tick.h                     |   52 ++++++++++++++--
 kernel/lockdep.c                         |   22 +++++++
 kernel/rcupdate.c                        |    4 +
 kernel/rcutiny.c                         |    1 +
 kernel/rcutree.c                         |   19 +++++-
 kernel/softirq.c                         |    4 +-
 kernel/time/tick-sched.c                 |   96 ++++++++++++++++++-----------
 30 files changed, 273 insertions(+), 129 deletions(-)

Signature · 1 line

1.7.5.4