Skip to contentSkip to conversation

Additional RCU commits for 3.4

PATCH In review Paul E. McKenney 2 participants 10 messages Feb 21, 2012

lore
Share & tools
lore
#0 lore raw

cover Additional RCU commits for 3.4

Hello!

This series contains an additional four commits for 3.4. They are all small fixes. This series is in addition to the set posted earlier at
https://lkml.org/lkml/2012/2/3/459, and there is some possibility that the direct-drive SRCU at https://lkml.org/lkml/2012/2/12/117 will also make it. The four new patches are as follows:

1. Fix RCU_FAST_NO_HZ to avoid continually retrying dyntick-idle
entry if softirqs are pending.
2. Fix RCU_FAST_NO_HZ to avoid retrying dyntick-idle entry when
idle is re-entered after a timer was posted. For both #1 and #2, the philosophy is that it is better to take a scheduling-clock interrupt than to burn lots of CPU in a futile attempt to process RCU callbacks.
3. Eliminate spurious warnings from synchronize_sched_expedited(),
courtesy of Hugh Dickens.
4. Apply several fixes to rcutorture based on initial inspection.

For a testing-only version of this patchset from git, please see the following subject-to-rebase branch:

git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git rcu/next

Thanx, Paul

------------------------------------------------------------------------

 b/kernel/rcutorture.c     |   19 ++++++++++++++++---
 b/kernel/rcutree.c        |    2 +-
 b/kernel/rcutree_plugin.h |    3 ++-
 kernel/rcutree.c          |    2 +-
 kernel/rcutree_plugin.h   |    2 +-
 5 files changed, 21 insertions(+), 7 deletions(-)
#1 lore raw

PATCH 1/4 rcu: Eliminate softirq-mediated RCU_FAST_NO_HZ idle-entry loop

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

If a softirq is pending, the current CPU has RCU callbacks pending, and RCU does not immediately need anything from this CPU, then the current code resets the RCU_FAST_NO_HZ state machine. This means that upon exit from the subsequent softirq handler, RCU_FAST_NO_HZ will try really hard to force RCU into dyntick-idle mode. And if the same conditions hold after a few tries (determined by RCU_IDLE_OPT_FLUSHES), the same situation can repeat, possibly endlessly. This scenario is not particularly good for battery lifetime.

This commit therefore suppresses the early exit from the RCU_FAST_NO_HZ state machine in the case where there is a softirq pending. This change forces the state machine to retain its memory, and to enter holdoff if this condition persists.

---
 kernel/rcutree_plugin.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
kernel/rcutree_plugin.h +2 -1
index f7ceadf..392a651 100644
@@ -2133,7 +2133,8 @@ static void rcu_prepare_for_idle(int cpu)
 		/* First time through, initialize the counter. */
 		per_cpu(rcu_dyntick_drain, cpu) = RCU_IDLE_FLUSHES;
 	} else if (per_cpu(rcu_dyntick_drain, cpu) <= RCU_IDLE_OPT_FLUSHES &&
-		   !rcu_pending(cpu)) {
+		   !rcu_pending(cpu) &&
+		   !local_softirq_pending()) {
 		/* Can we go dyntick-idle despite still having callbacks? */
 		trace_rcu_prep_idle("Dyntick with callbacks");
 		per_cpu(rcu_dyntick_drain, cpu) = 0;
Signature · 1 line
1.7.8
#2 lore raw

PATCH 2/4 rcu: Hold off RCU_FAST_NO_HZ after timer posted

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

This commit handles workloads that transition quickly between idle and non-idle, and where the CPU's callbacks cannot be invoked, but where RCU does not have anything immediate for the CPU to do. Without this patch, the RCU_FAST_NO_HZ code can be invoked repeatedly on each entry to idle. The commit sets the per-CPU rcu_dyntick_holdoff variable to hold off further attempts for a tick.

---
 kernel/rcutree_plugin.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/rcutree_plugin.h +1 -1
index 392a651..c023464 100644
@@ -2138,7 +2138,7 @@ static void rcu_prepare_for_idle(int cpu)
 		/* Can we go dyntick-idle despite still having callbacks? */
 		trace_rcu_prep_idle("Dyntick with callbacks");
 		per_cpu(rcu_dyntick_drain, cpu) = 0;
-		per_cpu(rcu_dyntick_holdoff, cpu) = jiffies - 1;
+		per_cpu(rcu_dyntick_holdoff, cpu) = jiffies;
 		if (rcu_cpu_has_nonlazy_callbacks(cpu))
 			hrtimer_start(&per_cpu(rcu_idle_gp_timer, cpu),
 				      rcu_idle_gp_wait, HRTIMER_MODE_REL);
Signature · 1 line
1.7.8
#3 lore raw

PATCH 3/4 rcu: stop spurious warnings from synchronize_sched_expedited

In reply to Paul E. McKenney

From: Hugh Dickins <[email protected]>

synchronize_sched_expedited() is spamming CONFIG_DEBUG_PREEMPT=y users with an unintended warning from the cpu_is_offline() check:
use raw_smp_processor_id() instead of smp_processor_id() there.

---
 kernel/rcutree.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/rcutree.c +1 -1
index eacc10b..1050d6d 100644
@@ -2014,7 +2014,7 @@ void synchronize_sched_expedited(void)
 	/* Note that atomic_inc_return() implies full memory barrier. */
 	firstsnap = snap = atomic_inc_return(&sync_sched_expedited_started);
 	get_online_cpus();
-	WARN_ON_ONCE(cpu_is_offline(smp_processor_id()));
+	WARN_ON_ONCE(cpu_is_offline(raw_smp_processor_id()));
 
 	/*
 	 * Each pass through the following loop attempts to force a
Signature · 1 line
1.7.8
#4 lore raw

Re: [PATCH tip/core/rcu 3/4] rcu: stop spurious warnings from synchronize_sched_expedited

Quoted 5 lines · Paul E. McKenney

From: Hugh Dickins <[email protected]>

synchronize_sched_expedited() is spamming CONFIG_DEBUG_PREEMPT=y users with an unintended warning from the cpu_is_offline() check:
use raw_smp_processor_id() instead of smp_processor_id() there.

This fails to mention why it makes sense to test a random cpu for offline-ness..

Quoted 19 lines
---
 kernel/rcutree.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/rcutree.c +1 -1
index eacc10b..1050d6d 100644
@@ -2014,7 +2014,7 @@ void synchronize_sched_expedited(void)
 	/* Note that atomic_inc_return() implies full memory barrier. */
 	firstsnap = snap = atomic_inc_return(&sync_sched_expedited_started);
 	get_online_cpus();
-	WARN_ON_ONCE(cpu_is_offline(smp_processor_id()));
+	WARN_ON_ONCE(cpu_is_offline(raw_smp_processor_id()));
 
 	/*
 	 * Each pass through the following loop attempts to force a
#5 lore raw

Re: [PATCH tip/core/rcu 3/4] rcu: stop spurious warnings from synchronize_sched_expedited

Quoted 9 lines · Peter Zijlstra
Quoted 5 lines · Paul E. McKenney

From: Hugh Dickins <[email protected]>

synchronize_sched_expedited() is spamming CONFIG_DEBUG_PREEMPT=y users with an unintended warning from the cpu_is_offline() check:
use raw_smp_processor_id() instead of smp_processor_id() there.

This fails to mention why it makes sense to test a random cpu for offline-ness..

The check was already there, Hugh simply fixed it to use raw_. The check itself was added in c0d6d01bf (Check for illegal use of RCU from offlined CPUs). The purpose is to catch improper use of RCU from CPU_DYING notifiers and on the path from the CPU_DYING notifiers to the idle loop.

Thanx, Paul

Quoted 20 lines
Quoted 19 lines
---
 kernel/rcutree.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/rcutree.c +1 -1
index eacc10b..1050d6d 100644
@@ -2014,7 +2014,7 @@ void synchronize_sched_expedited(void)
 	/* Note that atomic_inc_return() implies full memory barrier. */
 	firstsnap = snap = atomic_inc_return(&sync_sched_expedited_started);
 	get_online_cpus();
-	WARN_ON_ONCE(cpu_is_offline(smp_processor_id()));
+	WARN_ON_ONCE(cpu_is_offline(raw_smp_processor_id()));
 
 	/*
 	 * Each pass through the following loop attempts to force a
#6 lore raw

Re: [PATCH tip/core/rcu 3/4] rcu: stop spurious warnings from synchronize_sched_expedited

Quoted 8 lines · Paul E. McKenney
Quoted 7 lines · Peter Zijlstra
Quoted 5 lines · Paul E. McKenney

From: Hugh Dickins <[email protected]>

synchronize_sched_expedited() is spamming CONFIG_DEBUG_PREEMPT=y users with an unintended warning from the cpu_is_offline() check:
use raw_smp_processor_id() instead of smp_processor_id() there.

Quoted 7 lines
Quoted 2 lines

This fails to mention why it makes sense to test a random cpu for offline-ness..

The check was already there, Hugh simply fixed it to use raw_. The check itself was added in c0d6d01bf (Check for illegal use of RCU from offlined CPUs). The purpose is to catch improper use of RCU from CPU_DYING notifiers and on the path from the CPU_DYING notifiers to the idle loop.

Then I think this patch wants to add a comment explaining this. Because cpu_offline(raw_smp_processor_id()) looks really rather suspicious.

#7 lore raw

Re: [PATCH tip/core/rcu 3/4] rcu: stop spurious warnings from synchronize_sched_expedited

Quoted 12 lines · Peter Zijlstra
Quoted 11 lines · Paul E. McKenney
Quoted 9 lines · Peter Zijlstra
Quoted 5 lines · Paul E. McKenney

From: Hugh Dickins <[email protected]>

synchronize_sched_expedited() is spamming CONFIG_DEBUG_PREEMPT=y users with an unintended warning from the cpu_is_offline() check:
use raw_smp_processor_id() instead of smp_processor_id() there.

This fails to mention why it makes sense to test a random cpu for offline-ness..

Quoted 7 lines
Quoted 4 lines

The check was already there, Hugh simply fixed it to use raw_. The check itself was added in c0d6d01bf (Check for illegal use of RCU from offlined CPUs). The purpose is to catch improper use of RCU from CPU_DYING notifiers and on the path from the CPU_DYING notifiers to the idle loop.

Then I think this patch wants to add a comment explaining this. Because cpu_offline(raw_smp_processor_id()) looks really rather suspicious.

Good point, will fix!

Thanx, Paul

#8 lore raw

PATCH 3/4 rcu: Stop spurious warnings from synchronize_sched_expedited

In reply to Paul E. McKenney

From: Hugh Dickins <[email protected]>

synchronize_sched_expedited() is spamming CONFIG_DEBUG_PREEMPT=y users with an unintended warning from the cpu_is_offline() check:
use raw_smp_processor_id() instead of smp_processor_id() there.

---
 kernel/rcutree.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/rcutree.c +1 -1
index eacc10b..1050d6d 100644
@@ -2014,7 +2014,7 @@ void synchronize_sched_expedited(void)
 	/* Note that atomic_inc_return() implies full memory barrier. */
 	firstsnap = snap = atomic_inc_return(&sync_sched_expedited_started);
 	get_online_cpus();
-	WARN_ON_ONCE(cpu_is_offline(smp_processor_id()));
+	WARN_ON_ONCE(cpu_is_offline(raw_smp_processor_id()));
 
 	/*
 	 * Each pass through the following loop attempts to force a
Signature · 1 line
1.7.8
#9 lore raw

PATCH 4/4 rcu: Fixes to rcutorture error handling and cleanup

In reply to Paul E. McKenney

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

The rcutorture initialization code ignored the error returns from rcu_torture_onoff_init() and rcu_torture_stall_init(). The rcutorture cleanup code failed to NULL out a number of pointers. These bugs will normally have no effect, but this commit fixes them nevertheless.

---
 kernel/rcutorture.c |   19 ++++++++++++++++---
 1 files changed, 16 insertions(+), 3 deletions(-)
kernel/rcutorture.c +16 -3
index a89b381..1463a06 100644
@@ -1337,6 +1337,7 @@ static void rcutorture_booster_cleanup(int cpu)
 
 	/* This must be outside of the mutex, otherwise deadlock! */
 	kthread_stop(t);
+	boost_tasks[cpu] = NULL;
 }
 
 static int rcutorture_booster_init(int cpu)
@@ -1484,13 +1485,15 @@ static void rcu_torture_onoff_cleanup(void)
 		return;
 	VERBOSE_PRINTK_STRING("Stopping rcu_torture_onoff task");
 	kthread_stop(onoff_task);
+	onoff_task = NULL;
 }
 
 #else /* #ifdef CONFIG_HOTPLUG_CPU */
 
-static void
+static int
 rcu_torture_onoff_init(void)
 {
+	return 0;
 }
 
 static void rcu_torture_onoff_cleanup(void)
@@ -1554,6 +1557,7 @@ static void rcu_torture_stall_cleanup(void)
 		return;
 	VERBOSE_PRINTK_STRING("Stopping rcu_torture_stall_task.");
 	kthread_stop(stall_task);
+	stall_task = NULL;
 }
 
 static int rcutorture_cpu_notify(struct notifier_block *self,
@@ -1665,6 +1669,7 @@ rcu_torture_cleanup(void)
 		VERBOSE_PRINTK_STRING("Stopping rcu_torture_shutdown task");
 		kthread_stop(shutdown_task);
 	}
+	shutdown_task = NULL;
 	rcu_torture_onoff_cleanup();
 
 	/* Wait for all RCU callbacks to fire.  */
@@ -1897,9 +1902,17 @@ rcu_torture_init(void)
 			goto unwind;
 		}
 	}
-	rcu_torture_onoff_init();
+	i = rcu_torture_onoff_init();
+	if (i != 0) {
+		firsterr = i;
+		goto unwind;
+	}
 	register_reboot_notifier(&rcutorture_shutdown_nb);
-	rcu_torture_stall_init();
+	i = rcu_torture_stall_init();
+	if (i != 0) {
+		firsterr = i;
+		goto unwind;
+	}
 	rcutorture_record_test_transition();
 	mutex_unlock(&fullstop_mutex);
 	return 0;
Signature · 1 line
1.7.8