CPU hotplug changes for 3.8
cover CPU hotplug changes for 3.8
Hello!
This patch series provides further updates to RCU's interaction with CPU hotplug. The individual patches are as follows:
1. Fix an obsolete comment describing ->onofflock.
2. Make rcutorture print the errno from cpu_down() failure if
the "verbose" module parameter is specified.
3. Make _cpu_down() use stop_cpus() instead of __stop_machine().
4. Rename ->onofflock to ->orphan_lock to correctly indicate this
lock's current scope.
5. Avoid counter wrap in synchronize_sched_expedited().
6. Move synchronize_sched_expedited() from global variables to
the rcu_state structure.
7. Instrument synchronize_rcu_expedited() for debugfs tracing.
Thanx, Paul
b/kernel/cpu.c | 9 ++- b/kernel/rcutorture.c | 9 +++ b/kernel/rcutree.c | 4 - b/kernel/rcutree.h | 7 +- b/kernel/rcutree_plugin.h | 3 - kernel/rcutree.c | 112 ++++++++++++++++++++++++++++++---------------- kernel/rcutree.h | 12 ++++ 7 files changed, 108 insertions(+), 48 deletions(-)
PATCH 1/7 rcu: Fix comment about _rcu_barrier()/orphanage exclusion
From: "Paul E. McKenney" <[email protected]>
In the old days, _rcu_barrier() acquired ->onofflock to exclude rcu_send_cbs_to_orphanage(), which allowed the latter to avoid memory barriers in callback handling. However, _rcu_barrier() recently started doing get_online_cpus() to lock out CPU-hotplug operations entirely, which means that the comment in rcu_send_cbs_to_orphanage() that talks about ->onofflock is now obsolete. This commit therefore fixes the comment.
--- kernel/rcutree.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
kernel/rcutree.c +2 -2
@@ -1581,8 +1581,8 @@ rcu_send_cbs_to_orphanage(int cpu, struct rcu_state *rsp, {/* * Orphan the callbacks. First adjust the counts. This is safe -* because ->onofflock excludes _rcu_barrier()'s adoption of -* the callbacks, thus no memory barrier is required. +* because _rcu_barrier() excludes CPU-hotplug operations, so it +* cannot be running now. Thus no memory barrier is required. */ if (rdp->nxtlist !=NULL ) { rsp->qlen_lazy += rdp->qlen_lazy;
Signature · 1 line
1.7.8
PATCH 2/7 rcu: Make rcutorture give diagnostics if CPU offline fails
From: "Paul E. McKenney" <[email protected]>
This commit causes rcutorture to print the errno if cpu_down() fails when the rcutorture "verbose" module parameter is specified.
--- kernel/rcutorture.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-)
kernel/rcutorture.c +8 -1
@@ -1502,6 +1502,7 @@ rcu_torture_onoff(void *arg)unsigned long delta;int maxcpu =-1 ;DEFINE_RCU_RANDOM (rand); +int ret;unsigned long starttime;VERBOSE_PRINTK_STRING ("rcu_torture_onoff task started" );
@@ -1522,7 +1523,13 @@ rcu_torture_onoff(void *arg) torture_type, cpu); starttime = jiffies; n_offline_attempts++; -if (cpu_down (cpu) ==0 ) { + ret =cpu_down (cpu); +if (ret) { +if (verbose) +pr_alert ("%s" TORTURE_FLAG +"rcu_torture_onoff task: offline %d failed: errno %d\n" , + torture_type, cpu, ret); + }else {if (verbose) pr_alert("%s" TORTURE_FLAG "rcu_torture_onoff task: offlined %d\n",
Signature · 1 line
1.7.8
PATCH 3/7 cpu: No more __stop_machine() in _cpu_down()
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
The _cpu_down() function invoked as part of the CPU-hotplug offlining process currently invokes __stop_machine(), which is slow and inflicts substantial real-time latencies on the entire system. This patch substitutes stop_cpus() for __stop_machine() in order to improve both performance and real-time latency.
This is currently unsafe, because there are a number of uses of preempt_disable() that are intended to block CPU-hotplug offlining. These will be fixed, but in the meantime, this commit is one way to help locate them.
--- kernel/cpu.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
kernel/cpu.c +7 -2
@@ -247,13 +247,18 @@ static int __ref take_cpu_down(void *_param) {struct take_cpu_down_param *param = _param;int err; +unsigned long flags;/* Ensure this CPU doesn't handle any more interrupts. */ +local_irq_save (flags); err =__cpu_disable (); -if (err <0 ) +if (err <0 ) { +local_irq_restore (flags);return err; + }cpu_notify (CPU_DYING | param->mod, param->hcpu); +local_irq_restore (flags);return 0 ; }
@@ -286,7 +291,7 @@ static int __ref _cpu_down(unsigned int cpu, int tasks_frozen) }smpboot_park_threads (cpu); - err =__stop_machine (take_cpu_down, &tcd_param,cpumask_of (cpu)); + err =stop_cpus (cpumask_of (cpu), take_cpu_down, &tcd_param);if (err) {/* CPU didn't die: tell everyone. Can't complain. */ smpboot_unpark_threads (cpu);
Signature · 1 line
1.7.8
PATCH 4/7 rcu: Rename ->onofflock to ->orphan_lock
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
The ->onofflock field in the rcu_state structure at one time synchronized CPU-hotplug operations for RCU. However, its scope has decreased over time so that it now only protects the lists of orphaned RCU callbacks. This commit therefore renames it to ->orphan_lock to reflect its current use.
--- kernel/rcutree.c | 12 ++++++------ kernel/rcutree.h | 7 +++---- kernel/rcutree_plugin.h | 3 ++- 3 files changed, 11 insertions(+), 11 deletions(-)
kernel/rcutree.c +6 -6
@@ -70,7 +70,7 @@ static struct lock_class_key rcu_fqs_class[RCU_NUM_LVLS]; .fqs_state = RCU_GP_IDLE, \ .gpnum = -300, \ .completed = -300, \ - .onofflock = __RAW_SPIN_LOCK_UNLOCKED(&sname##_state.onofflock), \ + .orphan_lock = __RAW_SPIN_LOCK_UNLOCKED(&sname##_state.orphan_lock), \ .orphan_nxttail = &sname##_state.orphan_nxtlist, \ .orphan_donetail = &sname##_state.orphan_donelist, \ .barrier_mutex = __MUTEX_INITIALIZER(sname##_state.barrier_mutex), \
@@ -1573,7 +1573,7 @@ rcu_check_quiescent_state(struct rcu_state *rsp, struct rcu_data *rdp)/* * Send the specified CPU's RCU callbacks to the orphanage. The * specified CPU must be offline, and the caller must hold the -* ->onofflock. +* ->orphan_lock. */ static void rcu_send_cbs_to_orphanage(int cpu, struct rcu_state *rsp,
@@ -1623,7 +1623,7 @@ rcu_send_cbs_to_orphanage(int cpu, struct rcu_state *rsp,/* * Adopt the RCU callbacks from the specified rcu_state structure's -* orphanage. The caller must hold the ->onofflock. +* orphanage. The caller must hold the ->orphan_lock. */ static void rcu_adopt_orphan_cbs (struct rcu_state *rsp) {
@@ -1702,7 +1702,7 @@ static void rcu_cleanup_dead_cpu(int cpu, struct rcu_state *rsp)/* Exclude any attempts to start a new grace period. */ mutex_lock (&rsp->onoff_mutex); -raw_spin_lock_irqsave (&rsp->onofflock, flags); +raw_spin_lock_irqsave (&rsp->orphan_lock, flags);/* Orphan the dead CPU's callbacks, and adopt them if appropriate. */ rcu_send_cbs_to_orphanage (cpu, rsp, rnp, rdp);
@@ -1729,10 +1729,10 @@ static void rcu_cleanup_dead_cpu(int cpu, struct rcu_state *rsp)/* * We still hold the leaf rcu_node structure lock here, and * irqs are still disabled. The reason for this subterfuge is -* because invoking rcu_report_unblock_qs_rnp() with ->onofflock +* because invoking rcu_report_unblock_qs_rnp() with ->orphan_lock * held leads to deadlock. */ -raw_spin_unlock (&rsp->onofflock);/* irqs remain disabled. */ +raw_spin_unlock (&rsp->orphan_lock);/* irqs remain disabled. */ rnp = rdp->mynode;if (need_report &RCU_OFL_TASKS_NORM_GP )rcu_report_unblock_qs_rnp (rnp, flags);
kernel/rcutree.h +3 -4
@@ -383,9 +383,8 @@ struct rcu_state {/* End of fields guarded by root rcu_node's lock. */ -raw_spinlock_t onofflock____cacheline_internodealigned_in_smp ; -/* exclude on/offline and */ -/* starting new GP. */ +raw_spinlock_t orphan_lock____cacheline_internodealigned_in_smp ; +/* Protect following fields. */ struct rcu_head *orphan_nxtlist;/* Orphaned callbacks that */ /* need a grace period. */ struct rcu_head **orphan_nxttail;/* Tail of above. */
@@ -394,7 +393,7 @@ struct rcu_state {struct rcu_head **orphan_donetail;/* Tail of above. */ long qlen_lazy;/* Number of lazy callbacks. */ long qlen;/* Total number of callbacks. */ -/* End of fields guarded by onofflock. */ +/* End of fields guarded by orphan_lock. */ struct mutex onoff_mutex;/* Coordinate hotplug & GPs. */
kernel/rcutree_plugin.h +2 -1
@@ -757,7 +757,8 @@ static void rcu_report_exp_rnp(struct rcu_state *rsp, struct rcu_node *rnp,* grace period for the specified rcu_node structure. If there are no such * tasks, report it up the rcu_node hierarchy. * -* Caller must hold sync_rcu_preempt_exp_mutex and rsp->onofflock. +* Caller must hold sync_rcu_preempt_exp_mutex and must exclude +* CPU hotplug operations. */ static void sync_rcu_preempt_exp_init (struct rcu_state *rsp,struct rcu_node *rnp)
Signature · 1 line
1.7.8
PATCH 5/7 rcu: Avoid counter wrap in synchronize_sched_expedited()
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
There is a counter scheme similar to ticket locking that synchronize_sched_expedited() uses to service multiple concurrent callers with the same expedited grace period. Upon entry, a sync_sched_expedited_started variable is atomically incremented, and upon completion of a expedited grace period a separate sync_sched_expedited_done variable is atomically incremented.
However, if a synchronize_sched_expedited() is delayed while in try_stop_cpus(), concurrent invocations will increment the sync_sched_expedited_started counter, which will eventually overflow. If the original synchronize_sched_expedited() resumes execution just as the counter overflows, a concurrent invocation could incorrectly conclude that an expedited grace period elapsed in zero time, which would be bad. One could rely on counter size to prevent this from happening in practice, but the goal is to formally validate this code, so it needs to be fixed anyway.
This commit therefore checks the gap between the two counters before incrementing sync_sched_expedited_started, and if the gap is too large, does a normal grace period instead. Overflow is thus only possible if there are more than about 3.5 billion threads on 32-bit systems, which can be excluded until such time as task_struct fits into a single byte and 4G/4G patches are accepted into mainline. It is also easy to encode this limitation into mechanical theorem provers.
--- kernel/rcutree.c | 62 ++++++++++++++++++++++++++++++++++++++--------------- 1 files changed, 44 insertions(+), 18 deletions(-)
kernel/rcutree.c +44 -18
@@ -2249,8 +2249,8 @@ void synchronize_rcu_bh(void) }EXPORT_SYMBOL_GPL (synchronize_rcu_bh); -static atomic_t sync_sched_expedited_started =ATOMIC_INIT (0 ); -static atomic_t sync_sched_expedited_done =ATOMIC_INIT (0 ); +static atomic_long_t sync_sched_expedited_started =ATOMIC_LONG_INIT (0 ); +static atomic_long_t sync_sched_expedited_done =ATOMIC_LONG_INIT (0 );static int synchronize_sched_expedited_cpu_stop (void *data) {
@@ -2308,10 +2308,30 @@ static int synchronize_sched_expedited_cpu_stop(void *data)*/ void synchronize_sched_expedited (void ) { -int firstsnap, s, snap, trycount =0 ; +long firstsnap, s, snap; +int trycount =0 ; -/* Note that atomic_inc_return() implies full memory barrier. */ - firstsnap = snap =atomic_inc_return (&sync_sched_expedited_started); +/* +* If we are in danger of counter wrap, just do synchronize_sched(). +* By allowing sync_sched_expedited_started to advance no more than +* ULONG_MAX/8 ahead of sync_sched_expedited_done, we are ensuring +* that more than 3.5 billion CPUs would be required to force a +* counter wrap on a 32-bit system. Quite a few more CPUs would of +* course be required on a 64-bit system. +*/ +if (ULONG_CMP_GE ((ulong )atomic_read (&sync_sched_expedited_started), + (ulong )atomic_read (&sync_sched_expedited_done) + +ULONG_MAX /8 )) { +synchronize_sched (); +return ; + } + +/* +* Take a ticket. Note that atomic_inc_return() implies a +* full memory barrier. +*/ + snap =atomic_long_inc_return (&sync_sched_expedited_started); + firstsnap = snap;get_online_cpus ();WARN_ON_ONCE (cpu_is_offline (raw_smp_processor_id ()));
@@ -2324,6 +2344,13 @@ void synchronize_sched_expedited(void) NULL) == -EAGAIN) {put_online_cpus (); +/* Check to see if someone else did our work for us. */ + s =atomic_long_read (&sync_sched_expedited_done); +if (ULONG_CMP_GE ((ulong )s, (ulong )firstsnap)) { +smp_mb ();/* ensure test happens before caller kfree */ +return ; + } +/* No joy, try again later. Or just synchronize_sched(). */ if (trycount++ <10 ) {udelay (trycount *num_online_cpus ());
@@ -2332,23 +2359,22 @@ void synchronize_sched_expedited(void)return ; } -/* Check to see if someone else did our work for us. */ - s =atomic_read (&sync_sched_expedited_done); -if (UINT_CMP_GE ((unsigned )s, (unsigned )firstsnap)) { +/* Recheck to see if someone else did our work for us. */ + s =atomic_long_read (&sync_sched_expedited_done); +if (ULONG_CMP_GE ((ulong )s, (ulong )firstsnap)) {smp_mb ();/* ensure test happens before caller kfree */ return ; }/* * Refetching sync_sched_expedited_started allows later -* callers to piggyback on our grace period. We subtract -* 1 to get the same token that the last incrementer got. -* We retry after they started, so our grace period works -* for them, and they started after our first try, so their -* grace period works for us. +* callers to piggyback on our grace period. We retry +* after they started, so our grace period works for them, +* and they started after our first try, so their grace +* period works for us. */ get_online_cpus (); - snap =atomic_read (&sync_sched_expedited_started); + snap =atomic_long_read (&sync_sched_expedited_started);smp_mb ();/* ensure read is before try_stop_cpus(). */ }
@@ -2356,15 +2382,15 @@ void synchronize_sched_expedited(void)* Everyone up to our most recent fetch is covered by our grace * period. Update the counter, but only if our work is still * relevant -- which it won't be if someone who started later -* than we did beat us to the punch. +* than we did already did their update. */ do { - s =atomic_read (&sync_sched_expedited_done); -if (UINT_CMP_GE ((unsigned )s, (unsigned )snap)) { + s =atomic_long_read (&sync_sched_expedited_done); +if (ULONG_CMP_GE ((ulong )s, (ulong )snap)) {smp_mb ();/* ensure test happens before caller kfree */ break ; } - }while (atomic_cmpxchg (&sync_sched_expedited_done, s, snap) != s); + }while (atomic_long_cmpxchg (&sync_sched_expedited_done, s, snap) != s);put_online_cpus (); }
Signature · 1 line
1.7.8
PATCH 6/7 rcu: Move synchronize_sched_expedited() state to rcu_state
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
Tracing (debugfs) of expedited RCU primitives is required, which in turn requires that the relevant data be located where the tracing code can find it, not in its current static global variables in kernel/rcutree.c. This commit therefore moves sync_sched_expedited_started and sync_sched_expedited_done to the rcu_state structure, as fields ->expedited_start and ->expedited_done, respectively.
--- kernel/rcutree.c | 20 +++++++++----------- kernel/rcutree.h | 3 +++ 2 files changed, 12 insertions(+), 11 deletions(-)
kernel/rcutree.c +9 -11
@@ -2249,9 +2249,6 @@ void synchronize_rcu_bh(void) }EXPORT_SYMBOL_GPL (synchronize_rcu_bh); -static atomic_long_t sync_sched_expedited_started =ATOMIC_LONG_INIT (0 ); -static atomic_long_t sync_sched_expedited_done =ATOMIC_LONG_INIT (0 ); -static int synchronize_sched_expedited_cpu_stop (void *data) {/*
@@ -2310,6 +2307,7 @@ void synchronize_sched_expedited(void) {long firstsnap, s, snap;int trycount =0 ; +struct rcu_state *rsp = &rcu_sched_state;/* * If we are in danger of counter wrap, just do synchronize_sched().
@@ -2319,8 +2317,8 @@ void synchronize_sched_expedited(void)* counter wrap on a 32-bit system. Quite a few more CPUs would of * course be required on a 64-bit system. */ -if (ULONG_CMP_GE ((ulong )atomic_read (&sync_sched_expedited_started), - (ulong )atomic_read (&sync_sched_expedited_done) + +if (ULONG_CMP_GE ((ulong )atomic_long_read (&rsp->expedited_start), + (ulong )atomic_long_read (&rsp->expedited_done) +ULONG_MAX /8 )) {synchronize_sched ();return ;
@@ -2330,7 +2328,7 @@ void synchronize_sched_expedited(void)* Take a ticket. Note that atomic_inc_return() implies a * full memory barrier. */ - snap =atomic_long_inc_return (&sync_sched_expedited_started); + snap =atomic_long_inc_return (&rsp->expedited_start); firstsnap = snap;get_online_cpus ();WARN_ON_ONCE (cpu_is_offline (raw_smp_processor_id ()));
@@ -2345,7 +2343,7 @@ void synchronize_sched_expedited(void)put_online_cpus ();/* Check to see if someone else did our work for us. */ - s =atomic_long_read (&sync_sched_expedited_done); + s =atomic_long_read (&rsp->expedited_done);if (ULONG_CMP_GE ((ulong )s, (ulong )firstsnap)) {smp_mb ();/* ensure test happens before caller kfree */ return ;
@@ -2360,7 +2358,7 @@ void synchronize_sched_expedited(void) }/* Recheck to see if someone else did our work for us. */ - s =atomic_long_read (&sync_sched_expedited_done); + s =atomic_long_read (&rsp->expedited_done);if (ULONG_CMP_GE ((ulong )s, (ulong )firstsnap)) {smp_mb ();/* ensure test happens before caller kfree */ return ;
@@ -2374,7 +2372,7 @@ void synchronize_sched_expedited(void)* period works for us. */ get_online_cpus (); - snap =atomic_long_read (&sync_sched_expedited_started); + snap =atomic_long_read (&rsp->expedited_start);smp_mb ();/* ensure read is before try_stop_cpus(). */ }
@@ -2385,12 +2383,12 @@ void synchronize_sched_expedited(void)* than we did already did their update. */ do { - s =atomic_long_read (&sync_sched_expedited_done); + s =atomic_long_read (&rsp->expedited_done);if (ULONG_CMP_GE ((ulong )s, (ulong )snap)) {smp_mb ();/* ensure test happens before caller kfree */ break ; } - }while (atomic_long_cmpxchg (&sync_sched_expedited_done, s, snap) != s); + }while (atomic_long_cmpxchg (&rsp->expedited_done, s, snap) != s);put_online_cpus (); }
kernel/rcutree.h +3 -0
@@ -404,6 +404,9 @@ struct rcu_state {/* _rcu_barrier(). */ /* End of fields guarded by barrier_mutex. */ +atomic_long_t expedited_start;/* Starting ticket. */ +atomic_long_t expedited_done;/* Done ticket. */ +unsigned long jiffies_force_qs;/* Time at which to invoke */ /* force_quiescent_state(). */ unsigned long n_force_qs;/* Number of calls to */
Signature · 1 line
1.7.8
PATCH 7/7 rcu: Instrument synchronize_rcu_expedited() for debugfs tracing
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
This commit adds the counters to rcu_state and updates them in synchronize_rcu_expedited() to provide the data needed for debugfs tracing.
--- kernel/rcutree.c | 18 +++++++++++++++--- kernel/rcutree.h | 9 +++++++++ 2 files changed, 24 insertions(+), 3 deletions(-)
kernel/rcutree.c +15 -3
@@ -2321,6 +2321,7 @@ void synchronize_sched_expedited(void) (ulong)atomic_long_read(&rsp->expedited_done) + ULONG_MAX / 8)) {synchronize_sched (); +atomic_long_inc (&rsp->expedited_wrap);return ; }
@@ -2341,11 +2342,14 @@ void synchronize_sched_expedited(void) synchronize_sched_expedited_cpu_stop, NULL) == -EAGAIN) {put_online_cpus (); +atomic_long_inc (&rsp->expedited_tryfail);/* Check to see if someone else did our work for us. */ s =atomic_long_read (&rsp->expedited_done);if (ULONG_CMP_GE ((ulong )s, (ulong )firstsnap)) { -smp_mb ();/* ensure test happens before caller kfree */ +/* ensure test happens before caller kfree */ +smp_mb__before_atomic_inc ();/* ^^^ */ +atomic_long_inc (&rsp->expedited_workdone1);return ; }
@@ -2354,13 +2358,16 @@ void synchronize_sched_expedited(void)udelay (trycount *num_online_cpus ()); }else {synchronize_sched (); +atomic_long_inc (&rsp->expedited_normal);return ; }/* Recheck to see if someone else did our work for us. */ s =atomic_long_read (&rsp->expedited_done);if (ULONG_CMP_GE ((ulong )s, (ulong )firstsnap)) { -smp_mb ();/* ensure test happens before caller kfree */ +/* ensure test happens before caller kfree */ +smp_mb__before_atomic_inc ();/* ^^^ */ +atomic_long_inc (&rsp->expedited_workdone2);return ; }
@@ -2375,6 +2382,7 @@ void synchronize_sched_expedited(void) snap =atomic_long_read (&rsp->expedited_start);smp_mb ();/* ensure read is before try_stop_cpus(). */ } +atomic_long_inc (&rsp->expedited_stoppedcpus);/* * Everyone up to our most recent fetch is covered by our grace
@@ -2383,12 +2391,16 @@ void synchronize_sched_expedited(void)* than we did already did their update. */ do { +atomic_long_inc (&rsp->expedited_done_tries); s =atomic_long_read (&rsp->expedited_done);if (ULONG_CMP_GE ((ulong )s, (ulong )snap)) { -smp_mb ();/* ensure test happens before caller kfree */ +/* ensure test happens before caller kfree */ +smp_mb__before_atomic_inc ();/* ^^^ */ +atomic_long_inc (&rsp->expedited_done_lost);break ; } }while (atomic_long_cmpxchg (&rsp->expedited_done, s, snap) != s); +atomic_long_inc (&rsp->expedited_done_exit);put_online_cpus (); }
kernel/rcutree.h +9 -0
@@ -406,6 +406,15 @@ struct rcu_state {atomic_long_t expedited_start;/* Starting ticket. */ atomic_long_t expedited_done;/* Done ticket. */ +atomic_long_t expedited_wrap;/* # near-wrap incidents. */ +atomic_long_t expedited_tryfail;/* # acquisition failures. */ +atomic_long_t expedited_workdone1;/* # done by others #1. */ +atomic_long_t expedited_workdone2;/* # done by others #2. */ +atomic_long_t expedited_normal;/* # fallbacks to normal. */ +atomic_long_t expedited_stoppedcpus;/* # successful stop_cpus. */ +atomic_long_t expedited_done_tries;/* # tries to update _done. */ +atomic_long_t expedited_done_lost;/* # times beaten to _done. */ +atomic_long_t expedited_done_exit;/* # times exited _done loop. */ unsigned long jiffies_force_qs;/* Time at which to invoke */ /* force_quiescent_state(). */
Signature · 1 line
1.7.8