Expedited grace-period improvements for 4.4
Subsystems (4)
cover Expedited grace-period improvements for 4.4
Hello!
This commit continues the effort to reduce the OS jitter from RCU's expedited grace-period primitives, while also loosening the coupling between CPU hotplug and RCU's expedited grace-period primitives:
1. Use rsp->expedited_wq instead of sync_rcu_preempt_exp_wq to
enable later code consolidation.
2. Move rcu_report_exp_rnp() to allow later code consolidation.
3. Consolidate combining-tree bitmaks setup for the initialization
portion of synchronize_rcu_expedited().
4. Use single-stage IPI algorithm for preemptible-RCU expedited
grace periods.
5. Make synchronize_sched_expedited() use the combining tree
to reduce memory contention when waiting for quiescent states.
6. Rename ->qs_pending to ->core_needs_qs to better match this
field's use.
7. Invert ->passed_quiesce and rename to ->cpu_no_qs in order to
enable later aggregate-OR for requests for normal and expedited grace periods.
8. Make ->cpu_no_qs be a union for aggregate OR.
9. Switch synchronize_sched_expedited() from stop-CPUs to IPI.
10. Stop silencing lockdep false positive for expedited grace periods,
given that synchronize_rcu_expedited() no longer invokes synchronize_sched_expedited(), eliminating the apparent deadlock. (Just for the record, there never was a real deadlock.)
11. Stop excluding CPU hotplug in synchronize_sched_expedited().
12. Remove try_get_online_cpus(), which is now no longer used.
13. Bring sync_sched_exp_select_cpus() into alignment with
sync_rcu_exp_select_cpus() as a first step towards consolidating them into one function.
14. Consolidate expedited CPU selection, now that #13 enabled it.
15. Add online/offline information to expedited stall warning message.
16. Dump blocking tasks in expedited stall-warning messages.
17. Enable stall warnings for synchronize_rcu_expedited().
18. Improve synchronize_sched_expedited() CPU-hotplug handling.
Thanx, Paul
------------------------------------------------------------------------
b/Documentation/RCU/trace.txt | 32 +- b/include/linux/cpu.h | 2 b/include/linux/sched.h | 10 b/kernel/cpu.c | 13 b/kernel/rcu/tree.c | 561 ++++++++++++++++++++++++++++++---------- b/kernel/rcu/tree.h | 50 ++- b/kernel/rcu/tree_plugin.h | 579 ++++++++++++++++++++++-------------------- b/kernel/rcu/tree_trace.c | 10 8 files changed, 782 insertions(+), 475 deletions(-)
PATCH 1/18 rcu: Use rsp->expedited_wq instead of sync_rcu_preempt_exp_wq
Now that there is an ->expedited_wq waitqueue in each rcu_state structure, there is no need for the sync_rcu_preempt_exp_wq global variable. This commit therefore substitutes ->expedited_wq for sync_rcu_preempt_exp_wq. It also initializes ->expedited_wq only once at boot instead of at the start of each expedited grace period.
--- kernel/rcu/tree.c | 2 +- kernel/rcu/tree_plugin.h | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-)
kernel/rcu/tree.c +1 -1
@@ -3556,7 +3556,6 @@ void synchronize_sched_expedited(void)rcu_exp_gp_seq_start (rsp);/* Stop each CPU that is online, non-idle, and not us. */ -init_waitqueue_head (&rsp->expedited_wq);atomic_set (&rsp->expedited_need_qs,1 );/* Extra count avoids race. */ for_each_online_cpu (cpu) {struct rcu_data *rdp =per_cpu_ptr (rsp->rda, cpu);
@@ -4179,6 +4178,7 @@ static void __init rcu_init_one(struct rcu_state *rsp, }init_waitqueue_head (&rsp->gp_wq); +init_waitqueue_head (&rsp->expedited_wq); rnp = rsp->level[rcu_num_lvls -1 ];for_each_possible_cpu (i) {while (i > rnp->grphi)
kernel/rcu/tree_plugin.h +2 -4
@@ -535,8 +535,6 @@ void synchronize_rcu(void) }EXPORT_SYMBOL_GPL (synchronize_rcu); -static DECLARE_WAIT_QUEUE_HEAD (sync_rcu_preempt_exp_wq); -/* * Return non-zero if there are any tasks in RCU read-side critical * sections blocking the current preemptible-RCU expedited grace period.
@@ -590,7 +588,7 @@ static void rcu_report_exp_rnp(struct rcu_state *rsp, struct rcu_node *rnp,raw_spin_unlock_irqrestore (&rnp->lock, flags);if (wake) {smp_mb ();/* EGP done before wake_up(). */ -wake_up (&sync_rcu_preempt_exp_wq); +wake_up (&rsp->expedited_wq); }break ; }
@@ -729,7 +727,7 @@ void synchronize_rcu_expedited(void)/* Wait for snapshotted ->blkd_tasks lists to drain. */ rnp =rcu_get_root (rsp); -wait_event (sync_rcu_preempt_exp_wq, +wait_event (rsp->expedited_wq,sync_rcu_preempt_exp_done (rnp));/* Clean up and exit. */
Signature · 1 line
2.5.2
PATCH 2/18 rcu: Move rcu_report_exp_rnp() to allow consolidation
This is a nearly pure code-movement commit, moving rcu_report_exp_rnp(), sync_rcu_preempt_exp_done(), and rcu_preempted_readers_exp() so that later commits can make synchronize_sched_expedited() use them. The non-code-movement portion of this commit tags rcu_report_exp_rnp() as __maybe_unused to avoid build errors when CONFIG_PREEMPT=n.
--- kernel/rcu/tree.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++ kernel/rcu/tree_plugin.h | 66 ------------------------------------------------ 2 files changed, 66 insertions(+), 66 deletions(-)
kernel/rcu/tree.c +66 -0
@@ -3379,6 +3379,72 @@ static bool rcu_exp_gp_seq_done(struct rcu_state *rsp, unsigned long s)return rcu_seq_done (&rsp->expedited_sequence, s); } +/* +* Return non-zero if there are any tasks in RCU read-side critical +* sections blocking the current preemptible-RCU expedited grace period. +* If there is no preemptible-RCU expedited grace period currently in +* progress, returns zero unconditionally. +*/ +static int rcu_preempted_readers_exp (struct rcu_node *rnp) +{ +return rnp->exp_tasks !=NULL ; +} + +/* +* return non-zero if there is no RCU expedited grace period in progress +* for the specified rcu_node structure, in other words, if all CPUs and +* tasks covered by the specified rcu_node structure have done their bit +* for the current expedited grace period. Works only for preemptible +* RCU -- other RCU implementation use other means. +* +* Caller must hold the root rcu_node's exp_funnel_mutex. +*/ +static int sync_rcu_preempt_exp_done (struct rcu_node *rnp) +{ +return !rcu_preempted_readers_exp (rnp) && +READ_ONCE (rnp->expmask) ==0 ; +} + +/* +* Report the exit from RCU read-side critical section for the last task +* that queued itself during or before the current expedited preemptible-RCU +* grace period. This event is reported either to the rcu_node structure on +* which the task was queued or to one of that rcu_node structure's ancestors, +* recursively up the tree. (Calm down, calm down, we do the recursion +* iteratively!) +* +* Caller must hold the root rcu_node's exp_funnel_mutex. +*/ +static void __maybe_unused rcu_report_exp_rnp (struct rcu_state *rsp, +struct rcu_node *rnp,bool wake) +{ +unsigned long flags; +unsigned long mask; + +raw_spin_lock_irqsave (&rnp->lock, flags); +smp_mb__after_unlock_lock (); +for (;;) { +if (!sync_rcu_preempt_exp_done (rnp)) { +raw_spin_unlock_irqrestore (&rnp->lock, flags); +break ; + } +if (rnp->parent ==NULL ) { +raw_spin_unlock_irqrestore (&rnp->lock, flags); +if (wake) { +smp_mb ();/* EGP done before wake_up(). */ +wake_up (&rsp->expedited_wq); + } +break ; + } + mask = rnp->grpmask; +raw_spin_unlock (&rnp->lock);/* irqs remain disabled */ + rnp = rnp->parent; +raw_spin_lock (&rnp->lock);/* irqs already disabled */ +smp_mb__after_unlock_lock (); + rnp->expmask &= ~mask; + } +} +/* Common code for synchronize_{rcu,sched}_expedited() work-done checking. */ static bool sync_exp_work_done(struct rcu_state *rsp, struct rcu_node *rnp, struct rcu_data *rdp,
kernel/rcu/tree_plugin.h +0 -66
72 lines not on this page · raw message
Signature · 1 line
2.5.2
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
kernel/rcu/tree.c static bool rcu_exp_gp_seq_done(struct rcu_state *rsp, unsigned long s) · line 3424
Quoted 7 lines · Paul E. McKenney
kernel/rcu/tree.c +7
-0+static void __maybe_unused rcu_report_exp_rnp (struct rcu_state *rsp, +struct rcu_node *rnp,bool wake) +{ +unsigned long flags; +unsigned long mask; + +raw_spin_lock_irqsave (&rnp->lock, flags);
Normally we require a comment with barriers, explaining the order and the pairing etc.. :-)
Quoted 22 lines
kernel/rcu/tree.c +22
-0+smp_mb__after_unlock_lock (); +for (;;) { +if (!sync_rcu_preempt_exp_done (rnp)) { +raw_spin_unlock_irqrestore (&rnp->lock, flags); +break ; + } +if (rnp->parent ==NULL ) { +raw_spin_unlock_irqrestore (&rnp->lock, flags); +if (wake) { +smp_mb ();/* EGP done before wake_up(). */ +wake_up (&rsp->expedited_wq); + } +break ; + } + mask = rnp->grpmask; +raw_spin_unlock (&rnp->lock);/* irqs remain disabled */ + rnp = rnp->parent; +raw_spin_lock (&rnp->lock);/* irqs already disabled */ +smp_mb__after_unlock_lock (); + rnp->expmask &= ~mask; + } +}
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
Quoted 12 lines · Peter Zijlstra
kernel/rcu/tree.c static bool rcu_exp_gp_seq_done(struct rcu_state *rsp, unsigned long s) · line 3424
Quoted 7 lines · Paul E. McKenney
kernel/rcu/tree.c +7
-0+static void __maybe_unused rcu_report_exp_rnp (struct rcu_state *rsp, +struct rcu_node *rnp,bool wake) +{ +unsigned long flags; +unsigned long mask; + +raw_spin_lock_irqsave (&rnp->lock, flags);Normally we require a comment with barriers, explaining the order and the pairing etc.. :-)
Quoted 1 line
Review
Quoted 1 line
(quoted diff) +1
-0+ smp_mb__after_unlock_lock();
Hmmmm... That is not good.
Worse yet, I am missing comments on most of the pre-existing barriers of this form.
The purpose is to enforce the heavy-weight grace-period memory-ordering guarantees documented in the synchronize_sched() header comment and elsewhere. They pair with anything you might use to check for violation of these guarantees, or, simiarly, any ordering that you might use when relying on these guarantees.
I could add something like "/* Enforce GP memory ordering. */"
Or perhaps "/* See synchronize_sched() header. */"
I do not propose reproducing the synchronize_sched() header on each of these. That would be verbose, even for me! ;-)
Other thoughts?
Thanx, Paul
Quoted 22 lines
Quoted 21 lines
kernel/rcu/tree.c +21
-0+for (;;) { +if (!sync_rcu_preempt_exp_done (rnp)) { +raw_spin_unlock_irqrestore (&rnp->lock, flags); +break ; + } +if (rnp->parent ==NULL ) { +raw_spin_unlock_irqrestore (&rnp->lock, flags); +if (wake) { +smp_mb ();/* EGP done before wake_up(). */ +wake_up (&rsp->expedited_wq); + } +break ; + } + mask = rnp->grpmask; +raw_spin_unlock (&rnp->lock);/* irqs remain disabled */ + rnp = rnp->parent; +raw_spin_lock (&rnp->lock);/* irqs already disabled */ +smp_mb__after_unlock_lock (); + rnp->expmask &= ~mask; + } +}
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
Quoted 13 lines · Paul E. McKenney
Quoted 12 lines · Peter Zijlstra
kernel/rcu/tree.c static bool rcu_exp_gp_seq_done(struct rcu_state *rsp, unsigned long s) · line 3424
Quoted 7 lines · Paul E. McKenney
kernel/rcu/tree.c +7
-0+static void __maybe_unused rcu_report_exp_rnp (struct rcu_state *rsp, +struct rcu_node *rnp,bool wake) +{ +unsigned long flags; +unsigned long mask; + +raw_spin_lock_irqsave (&rnp->lock, flags);Normally we require a comment with barriers, explaining the order and the pairing etc.. :-)
Quoted 6 lines
Quoted 1 line
Review
Quoted 1 line
(quoted diff) +1
-0+ smp_mb__after_unlock_lock();Hmmmm... That is not good.
Worse yet, I am missing comments on most of the pre-existing barriers of this form.
Yes I noticed.. :/
Quoted 7 lines
The purpose is to enforce the heavy-weight grace-period memory-ordering guarantees documented in the
synchronize_sched()header comment and elsewhere.They pair with anything you might use to check for violation of these guarantees, or, simiarly, any ordering that you might use when relying on these guarantees.
I'm sure you know what that means, but I've no clue ;-) That is, I wouldn't know where to start looking in the RCU implementation to verify the barrier is either needed or sufficient. Unless you mean _everywhere_ :-)
Quoted 8 lines
I could add something like "/* Enforce GP memory ordering. */"
Or perhaps "/* See
synchronize_sched()header. */"I do not propose reproducing the
synchronize_sched()header on each of these. That would be verbose, even for me! ;-)Other thoughts?
Well, this is an UNLOCK+LOCK on non-matching lock variables upgrade to full barrier thing, right?
To me its not clear which UNLOCK we even match here. I've just read the sync_sched() header, but that doesn't help me either, so referring to that isn't really helpful either.
In any case, I don't want to make too big a fuzz here, but I just stumbled over a lot of unannotated barriers and figured I ought to say something about it.
Mathieu Desnoyers
reviewerRe: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
Quoted 27 lines · ----- On Oct 7, 2015, at 3:51 AM, Peter Zijlstra [email protected] wrote:
Quoted 13 lines · Paul E. McKenney
Quoted 12 lines · Peter Zijlstra
kernel/rcu/tree.c static bool rcu_exp_gp_seq_done(struct rcu_state *rsp, unsigned long s) · line 3424
Quoted 7 lines · Paul E. McKenney
kernel/rcu/tree.c +7
-0+static void __maybe_unused rcu_report_exp_rnp (struct rcu_state *rsp, +struct rcu_node *rnp,bool wake) +{ +unsigned long flags; +unsigned long mask; + +raw_spin_lock_irqsave (&rnp->lock, flags);Normally we require a comment with barriers, explaining the order and the pairing etc.. :-)
Quoted 6 lines
Quoted 1 line
Review
Quoted 1 line
(quoted diff) +1
-0+ smp_mb__after_unlock_lock();Hmmmm... That is not good.
Worse yet, I am missing comments on most of the pre-existing barriers of this form.
Yes I noticed.. :/
Quoted 3 lines
The purpose is to enforce the heavy-weight grace-period memory-ordering guarantees documented in the
synchronize_sched()header comment and elsewhere.
Quoted 8 lines
Quoted 3 lines
They pair with anything you might use to check for violation of these guarantees, or, simiarly, any ordering that you might use when relying on these guarantees.
I'm sure you know what that means, but I've no clue ;-) That is, I wouldn't know where to start looking in the RCU implementation to verify the barrier is either needed or sufficient. Unless you mean _everywhere_ :-)
One example is the new membarrier system call. It relies on synchronize_sched() to enforce this:
from kernel/membarrier.c:
* All memory accesses performed in program order from each targeted thread
* is guaranteed to be ordered with respect to sys_membarrier(). If we use
* the semantic "barrier()" to represent a compiler barrier forcing memory
* accesses to be performed in program order across the barrier, and
* smp_mb() to represent explicit memory barriers forcing full memory
* ordering across the barrier, we have the following ordering table for
* each pair of barrier(), sys_membarrier() and smp_mb():
*
* The pair ordering is detailed as (O: ordered, X: not ordered):
*
* barrier() smp_mb() sys_membarrier()
* barrier() X X O * smp_mb() X O O * sys_membarrier() O O O
And include/uapi/linux/membarrier.h:
* @MEMBARRIER_CMD_SHARED: Execute a memory barrier on all running threads. * Upon return from system call, the caller thread * is ensured that all running threads have passed * through a state where all memory accesses to * user-space addresses match program order between * entry to and return from the system call * (non-running threads are de facto in such a * state). This covers threads from all processes * running on the system. This command returns 0.
I hope this sheds light on a userspace-facing interface to synchronize_sched() and clarifies its expected semantic a bit.
Thanks,
Mathieu
Quoted 20 lines
Quoted 9 lines
I could add something like "/* Enforce GP memory ordering. */"
Or perhaps "/* See
synchronize_sched()header. */"I do not propose reproducing the
synchronize_sched()header on each of these. That would be verbose, even for me! ;-)Other thoughts?
Well, this is an UNLOCK+LOCK on non-matching lock variables upgrade to full barrier thing, right?
To me its not clear which UNLOCK we even match here. I've just read the
sync_sched()header, but that doesn't help me either, so referring to that isn't really helpful either.In any case, I don't want to make too big a fuzz here, but I just stumbled over a lot of unannotated barriers and figured I ought to say something about it.
Signature · 3 lines
Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
Quoted 33 lines · Mathieu Desnoyers
Quoted 31 lines · ----- On Oct 7, 2015, at 3:51 AM, Peter Zijlstra [email protected] wrote:
Quoted 13 lines · Paul E. McKenney
Quoted 12 lines · Peter Zijlstra
kernel/rcu/tree.c static bool rcu_exp_gp_seq_done(struct rcu_state *rsp, unsigned long s) · line 3424
Quoted 7 lines · Paul E. McKenney
kernel/rcu/tree.c +7
-0+static void __maybe_unused rcu_report_exp_rnp (struct rcu_state *rsp, +struct rcu_node *rnp,bool wake) +{ +unsigned long flags; +unsigned long mask; + +raw_spin_lock_irqsave (&rnp->lock, flags);Normally we require a comment with barriers, explaining the order and the pairing etc.. :-)
Quoted 6 lines
Quoted 1 line
Review
Quoted 1 line
(quoted diff) +1
-0+ smp_mb__after_unlock_lock();Hmmmm... That is not good.
Worse yet, I am missing comments on most of the pre-existing barriers of this form.
Yes I noticed.. :/
Quoted 7 lines
The purpose is to enforce the heavy-weight grace-period memory-ordering guarantees documented in the
synchronize_sched()header comment and elsewhere.They pair with anything you might use to check for violation of these guarantees, or, simiarly, any ordering that you might use when relying on these guarantees.
Quoted 7 lines
Quoted 4 lines
I'm sure you know what that means, but I've no clue ;-) That is, I wouldn't know where to start looking in the RCU implementation to verify the barrier is either needed or sufficient. Unless you mean _everywhere_ :-)
One example is the new membarrier system call. It relies on
synchronize_sched()to enforce this:
That again doesn't explain which UNLOCKs with non-matching lock values it pairs with and what particular ordering is important here.
I'm fully well aware of what sync_sched() guarantees and how one can use it, that is not the issue, what I'm saying is that a generic description of sync_sched() doesn't help in figuring out WTH that barrier is for and which other code I should also inspect.
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
Quoted 2 lines · Peter Zijlstra
That again doesn't explain which UNLOCKs with non-matching lock values it pairs with and what particular ordering is important here.
So after staring at that stuff for a while I came up with the following. Does this make sense, or am I completely misunderstanding things?
Not been near a compiler.
--- kernel/rcu/tree.c | 99 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 61 insertions(+), 38 deletions(-)
kernel/rcu/tree.c +61 -38
@@ -1460,6 +1460,48 @@ static void trace_rcu_future_gp(struct rcu_node *rnp, struct rcu_data *rdp, }/* +* Wrappers for the rcu_node::lock acquire. +* +* Because the rcu_nodes form a tree, the tree taversal locking will observe +* different lock values, this in turn means that an UNLOCK of one level +* followed by a LOCK of another level does not imply a full memory barrier; +* and most importantly transitivity is lost. +* +* In order to restore full ordering between tree levels, augment the regular +* lock acquire functions with smp_mb__after_unlock_lock(). +*/ +static inline void raw_spin_lock_rcu_node (struct rcu_node *rnp) +{ +raw_spin_lock (&rnp->lock); +smp_mb__after_unlock_lock (); +} + +static inline void raw_spin_lock_irq_rcu_node (struct rcu_node *rnp) +{ +raw_spin_lock_irq (&rnp->lock); +smp_mb__after_unlock_lock (); +} + +static inline void +_raw_spin_lock_irqsave_rcu_node (struct rcu_node *rnp,unsigned long *flags) +{ +_raw_spin_lock_irqsave (&rnp->lock, flags); +smp_mb__after_unlock_lock (); +} + +#define raw_spin_lock_irqsave_rcu_node (rnp, flags) + _raw_spin_lock_irqsave_rcu_node((rnp), &(flags)) + +static inline bool raw_spin_trylock_rcu_node (struct rcu_node *rnp) +{ +bool locked =raw_spin_trylock (&rnp->lock); +if (locked) +smp_mb__after_unlock_lock (); +return locked; +} + + +/* * Start some future grace period, as needed to handle newly arrived * callbacks. The required future grace periods are recorded in each * rcu_node structure's ->need_future_gp field. Returns true if there
@@ -1512,10 +1554,8 @@ rcu_start_future_gp(struct rcu_node *rnp, struct rcu_data *rdp,* hold it, acquire the root rcu_node structure's lock in order to * start one (if needed). */ -if (rnp != rnp_root) { -raw_spin_lock (&rnp_root->lock); -smp_mb__after_unlock_lock (); - } +if (rnp != rnp_root) +raw_spin_lock_rcu_node (rnp);/* * Get a new grace-period number. If there really is no grace
@@ -1764,11 +1804,10 @@ static void note_gp_changes(struct rcu_state *rsp, struct rcu_data *rdp)if ((rdp->gpnum ==READ_ONCE (rnp->gpnum) && rdp->completed ==READ_ONCE (rnp->completed) && !unlikely (READ_ONCE (rdp->gpwrap))) ||/* w/out lock. */ - !raw_spin_trylock (&rnp->lock)) {/* irqs already off, so later. */ + !raw_spin_trylock_rcu_node (rnp)) {/* irqs already off, so later. */ local_irq_restore (flags);return ; } -smp_mb__after_unlock_lock (); needwake =__note_gp_changes (rsp, rnp, rdp);raw_spin_unlock_irqrestore (&rnp->lock, flags);if (needwake)
@@ -1792,8 +1831,7 @@ static int rcu_gp_init(struct rcu_state *rsp)struct rcu_node *rnp =rcu_get_root (rsp);WRITE_ONCE (rsp->gp_activity, jiffies); -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_irq_rcu_node (rnp);if (!READ_ONCE (rsp->gp_flags)) {/* Spurious wakeup, tell caller to go back to sleep. */ raw_spin_unlock_irq (&rnp->lock);
@@ -1825,8 +1863,7 @@ static int rcu_gp_init(struct rcu_state *rsp)*/ rcu_for_each_leaf_node (rsp, rnp) {rcu_gp_slow (rsp, gp_preinit_delay); -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_irq_rcu_node (rnp);if (rnp->qsmaskinit == rnp->qsmaskinitnext && !rnp->wait_blkd_tasks) {/* Nothing to do on this leaf rcu_node structure. */
@@ -1882,8 +1919,7 @@ static int rcu_gp_init(struct rcu_state *rsp)*/ rcu_for_each_node_breadth_first (rsp, rnp) {rcu_gp_slow (rsp, gp_init_delay); -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_irq_rcu_node (rnp); rdp =this_cpu_ptr (rsp->rda);rcu_preempt_check_blocked_tasks (rnp); rnp->qsmask = rnp->qsmaskinit;
@@ -1953,8 +1989,7 @@ static int rcu_gp_fqs(struct rcu_state *rsp, int fqs_state_in) }/* Clear flag to prevent immediate re-entry. */ if (READ_ONCE (rsp->gp_flags) &RCU_GP_FLAG_FQS ) { -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_irq_rcu_node (rnp);WRITE_ONCE (rsp->gp_flags,READ_ONCE (rsp->gp_flags) & ~RCU_GP_FLAG_FQS );raw_spin_unlock_irq (&rnp->lock);
@@ -1974,8 +2009,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)struct rcu_node *rnp =rcu_get_root (rsp);WRITE_ONCE (rsp->gp_activity, jiffies); -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_irq_rcu_node (rnp); gp_duration = jiffies - rsp->gp_start;if (gp_duration > rsp->gp_max) rsp->gp_max = gp_duration;
@@ -2000,8 +2034,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)* grace period is recorded in any of the rcu_node structures. */ rcu_for_each_node_breadth_first (rsp, rnp) { -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_irq_rcu_node (rnp);WARN_ON_ONCE (rcu_preempt_blocked_readers_cgp (rnp));WARN_ON_ONCE (rnp->qsmask);WRITE_ONCE (rnp->completed, rsp->gpnum);
@@ -2016,8 +2049,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)rcu_gp_slow (rsp, gp_cleanup_delay); } rnp =rcu_get_root (rsp); -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock ();/* Order GP before ->completed update. */ +raw_spin_lock_irq_rcu_node (rnp);/* Order GP before ->completed update. */ rcu_nocb_gp_set (rnp, nocb);/* Declare grace period done. */
@@ -2264,8 +2296,7 @@ rcu_report_qs_rnp(unsigned long mask, struct rcu_state *rsp,raw_spin_unlock_irqrestore (&rnp->lock, flags); rnp_c = rnp; rnp = rnp->parent; -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp, flags); oldmask = rnp_c->qsmask; }
@@ -2312,8 +2343,7 @@ static void rcu_report_unblock_qs_rnp(struct rcu_state *rsp, gps = rnp->gpnum; mask = rnp->grpmask;raw_spin_unlock (&rnp->lock);/* irqs remain disabled. */ -raw_spin_lock (&rnp_p->lock);/* irqs already disabled. */ -smp_mb__after_unlock_lock (); +raw_spin_lock_rcu_node (rnp);/* irqs already disabled. */ rcu_report_qs_rnp (mask, rsp, rnp_p, gps, flags); }
@@ -2335,8 +2365,7 @@ rcu_report_qs_rdp(int cpu, struct rcu_state *rsp, struct rcu_data *rdp)struct rcu_node *rnp; rnp = rdp->mynode; -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp, flags); if ((rdp->passed_quiesce == 0 && rdp->rcu_qs_ctr_snap == __this_cpu_read(rcu_qs_ctr)) || rdp->gpnum != rnp->gpnum || rnp->completed == rnp->gpnum ||
@@ -2562,8 +2591,7 @@ static void rcu_cleanup_dead_rnp(struct rcu_node *rnp_leaf) rnp = rnp->parent;if (!rnp)break ; -raw_spin_lock (&rnp->lock);/* irqs already disabled. */ -smp_mb__after_unlock_lock ();/* GP memory ordering. */ +raw_spin_lock_rcu_node (rnp);/* irqs already disabled. */ rnp->qsmaskinit &= ~mask; rnp->qsmask &= ~mask;if (rnp->qsmaskinit) {
@@ -2591,8 +2619,7 @@ static void rcu_cleanup_dying_idle_cpu(int cpu, struct rcu_state *rsp)/* Remove outgoing CPU from mask in the leaf rcu_node structure. */ mask = rdp->grpmask; -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock ();/* Enforce GP memory-order guarantee. */ +raw_spin_lock_irqsave_rcu_node (rnp, flags);/* Enforce GP memory-order guarantee. */ rnp->qsmaskinitnext &= ~mask;raw_spin_unlock_irqrestore (&rnp->lock, flags); }
@@ -2789,8 +2816,7 @@ static void force_qs_rnp(struct rcu_state *rsp,rcu_for_each_leaf_node (rsp, rnp) {cond_resched_rcu_qs (); mask =0 ; -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp, flags);if (rnp->qsmask ==0 ) { if (rcu_state_p == &rcu_sched_state || rsp != rcu_state_p ||
@@ -2861,8 +2887,7 @@ static void force_quiescent_state(struct rcu_state *rsp)/* rnp_old == rcu_get_root(rsp), rnp == NULL. */ /* Reached the root of the rcu_node tree, acquire lock. */ -raw_spin_lock_irqsave (&rnp_old->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp_old, flags);raw_spin_unlock (&rnp_old->fqslock);if (READ_ONCE (rsp->gp_flags) &RCU_GP_FLAG_FQS ) { rsp->n_force_qs_lh++;
@@ -2985,8 +3010,7 @@ static void __call_rcu_core(struct rcu_state *rsp, struct rcu_data *rdp,if (!rcu_gp_in_progress (rsp)) {struct rcu_node *rnp_root =rcu_get_root (rsp); -raw_spin_lock (&rnp_root->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_rcu_node (rnp_root); needwake =rcu_start_gp (rsp);raw_spin_unlock (&rnp_root->lock);if (needwake)
@@ -3925,8 +3949,7 @@ rcu_init_percpu_data(int cpu, struct rcu_state *rsp)*/ rnp = rdp->mynode; mask = rdp->grpmask; -raw_spin_lock (&rnp->lock);/* irqs already disabled. */ -smp_mb__after_unlock_lock (); +raw_spin_lock_rcu_node (rnp);/* irqs already disabled. */ rnp->qsmaskinitnext |= mask; rdp->gpnum = rnp->completed;/* Make CPU later note any new GP. */ rdp->completed = rnp->completed;
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
kernel/rcu/tree.c rcu_start_future_gp(struct rcu_node *rnp, struct rcu_data *rdp, · line 1558
Quoted 10 lines · Peter Zijlstra
kernel/rcu/tree.c +2
-4@@ -1512,10 +1554,8 @@ rcu_start_future_gp(struct rcu_node *rnp, struct rcu_data *rdp,* hold it, acquire the root rcu_node structure's lock in order to * start one (if needed). */ -if (rnp != rnp_root) { -raw_spin_lock (&rnp_root->lock); -smp_mb__after_unlock_lock (); - } +if (rnp != rnp_root) +raw_spin_lock_rcu_node (rnp);
rnp_root
kernel/rcu/tree.c static void rcu_report_unblock_qs_rnp(struct rcu_state *rsp, · line 2346
Quoted 7 lines
kernel/rcu/tree.c +1
-2@@ -2312,8 +2343,7 @@ static void rcu_report_unblock_qs_rnp(struct rcu_state *rsp, gps = rnp->gpnum; mask = rnp->grpmask;raw_spin_unlock (&rnp->lock);/* irqs remain disabled. */ -raw_spin_lock (&rnp_p->lock);/* irqs already disabled. */ -smp_mb__after_unlock_lock (); +raw_spin_lock_rcu_node (rnp);/* irqs already disabled. */
rnp_p
kbuild test robot
build botRe: Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
In reply to Peter Zijlstraconfig: i386-randconfig-x004-201540 (attached as .config) reproduce:
Hi Peter,
[auto build test ERROR on v4.3-rc4 -- if it's inappropriate base, please ignore]
config: i386-randconfig-x004-201540 (attached as .config) reproduce:
# save the attached .config to linux build tree
make ARCH=i386 All errors (new ones prefixed by >>):
kernel/rcu/tree.c: In function '_raw_spin_lock_irqsave_rcu_node':
>> kernel/rcu/tree.c:1488:2: error: too many arguments to function '_raw_spin_lock_irqsave'
_raw_spin_lock_irqsave(&rnp->lock, flags);
^
In file included from include/linux/spinlock.h:280:0,
from kernel/rcu/tree.c:33:
include/linux/spinlock_api_smp.h:34:26: note: declared here
unsigned long __lockfunc _raw_spin_lock_irqsave(raw_spinlock_t *lock)
^
kernel/rcu/tree.c: At top level:
>> kernel/rcu/tree.c:1493:34: error: expected declaration specifiers or '...' before '(' token
_raw_spin_lock_irqsave_rcu_node((rnp), &(flags))
^
>> kernel/rcu/tree.c:1493:41: error: expected declaration specifiers or '...' before '&' token
_raw_spin_lock_irqsave_rcu_node((rnp), &(flags))
^
kernel/rcu/tree.c: In function 'note_gp_changes':
>> kernel/rcu/tree.c:1807:7: error: implicit declaration of function 'raw_spin_trylock_rcu_node' [-Werror=implicit-function-declaration] !raw_spin_trylock_rcu_node(rnp)) { /* irqs already off, so later. */
^
cc1: some warnings being treated as errors
vim +/_raw_spin_lock_irqsave +1488 kernel/rcu/tree.c
1482 smp_mb__after_unlock_lock();
1483 }
1484
1485 static inline void
1486 _raw_spin_lock_irqsave_rcu_node(struct rcu_node *rnp, unsigned long *flags)
1487 {
> 1488 _raw_spin_lock_irqsave(&rnp->lock, flags); 1489 smp_mb__after_unlock_lock(); 1490 } 1491 1492 #define raw_spin_lock_irqsave_rcu_node(rnp, flags) > 1493 _raw_spin_lock_irqsave_rcu_node((rnp), &(flags))
1494
1495 static inline bool raw_spin_trylock_rcu_node(struct rcu_node *rnp)
1496 {
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
kbuild test robot
build botRe: Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
In reply to Peter Zijlstraconfig: x86_64-randconfig-x010-201540 (attached as .config) reproduce:
Hi Peter,
[auto build test WARNING on v4.3-rc4 -- if it's inappropriate base, please ignore]
config: x86_64-randconfig-x010-201540 (attached as .config) reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 All warnings (new ones prefixed by >>):
In file included from include/linux/kernel.h:12:0,
from kernel/rcu/tree.c:31:
kernel/rcu/tree.c: In function '_raw_spin_lock_irqsave_rcu_node':
include/linux/typecheck.h:11:18: warning: comparison of distinct pointer types lacks a cast
(void)(&__dummy == &__dummy2); \
^
>> include/linux/irqflags.h:63:3: note: in expansion of macro 'typecheck'
typecheck(unsigned long, flags); \
^
>> include/linux/irqflags.h:95:3: note: in expansion of macro 'raw_local_irq_save'
raw_local_irq_save(flags); \
^
>> include/linux/spinlock_api_up.h:40:8: note: in expansion of macro 'local_irq_save'
do { local_irq_save(flags); __LOCK(lock); } while (0)
^
>> include/linux/spinlock_api_up.h:69:45: note: in expansion of macro '__LOCK_IRQSAVE'
#define _raw_spin_lock_irqsave(lock, flags) __LOCK_IRQSAVE(lock, flags)
^
>> kernel/rcu/tree.c:1488:2: note: in expansion of macro '_raw_spin_lock_irqsave'
_raw_spin_lock_irqsave(&rnp->lock, flags);
^
In file included from arch/x86/include/asm/processor.h:32:0,
from arch/x86/include/asm/thread_info.h:52,
from include/linux/thread_info.h:54,
from arch/x86/include/asm/preempt.h:6,
from include/linux/preempt.h:64,
from include/linux/spinlock.h:50,
from kernel/rcu/tree.c:33:
>> include/linux/irqflags.h:64:9: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
flags = arch_local_irq_save(); \
^
>> include/linux/irqflags.h:95:3: note: in expansion of macro 'raw_local_irq_save'
raw_local_irq_save(flags); \
^
>> include/linux/spinlock_api_up.h:40:8: note: in expansion of macro 'local_irq_save'
do { local_irq_save(flags); __LOCK(lock); } while (0)
^
>> include/linux/spinlock_api_up.h:69:45: note: in expansion of macro '__LOCK_IRQSAVE'
#define _raw_spin_lock_irqsave(lock, flags) __LOCK_IRQSAVE(lock, flags)
^
>> kernel/rcu/tree.c:1488:2: note: in expansion of macro '_raw_spin_lock_irqsave'
_raw_spin_lock_irqsave(&rnp->lock, flags);
^
kernel/rcu/tree.c: At top level:
kernel/rcu/tree.c:1493:34: error: expected declaration specifiers or '...' before '(' token
_raw_spin_lock_irqsave_rcu_node((rnp), &(flags))
^
kernel/rcu/tree.c:1493:41: error: expected declaration specifiers or '...' before '&' token
_raw_spin_lock_irqsave_rcu_node((rnp), &(flags))
^ kernel/rcu/tree.c: In function 'note_gp_changes':
kernel/rcu/tree.c:1807:7: error: implicit declaration of function 'raw_spin_trylock_rcu_node' [-Werror=implicit-function-declaration]
!raw_spin_trylock_rcu_node(rnp)) { /* irqs already off, so later. */
^
cc1: some warnings being treated as errors
vim +/_raw_spin_lock_irqsave +1488 kernel/rcu/tree.c
1472 */
1473 static inline void raw_spin_lock_rcu_node(struct rcu_node *rnp)
1474 {
1475 raw_spin_lock(&rnp->lock);
1476 smp_mb__after_unlock_lock();
1477 }
1478
1479 static inline void raw_spin_lock_irq_rcu_node(struct rcu_node *rnp)
1480 {
1481 raw_spin_lock_irq(&rnp->lock);
1482 smp_mb__after_unlock_lock();
1483 }
1484
1485 static inline void
1486 _raw_spin_lock_irqsave_rcu_node(struct rcu_node *rnp, unsigned long *flags)
1487 {
> 1488 _raw_spin_lock_irqsave(&rnp->lock, flags);
1489 smp_mb__after_unlock_lock();
1490 }
1491
1492 #define raw_spin_lock_irqsave_rcu_node(rnp, flags)
1493 _raw_spin_lock_irqsave_rcu_node((rnp), &(flags))
1494
1495 static inline bool raw_spin_trylock_rcu_node(struct rcu_node *rnp)
1496 {
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
kbuild test robot
build botRe: Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
In reply to Peter Zijlstraconfig: i386-randconfig-x007-201540 (attached as .config) reproduce:
Hi Peter,
[auto build test WARNING on v4.3-rc4 -- if it's inappropriate base, please ignore]
config: i386-randconfig-x007-201540 (attached as .config) reproduce:
# save the attached .config to linux build tree
make ARCH=i386 All warnings (new ones prefixed by >>):
kernel/rcu/tree.c: In function '_raw_spin_lock_irqsave_rcu_node':
kernel/rcu/tree.c:1488:2: error: too many arguments to function '_raw_spin_lock_irqsave'
_raw_spin_lock_irqsave(&rnp->lock, flags);
^
In file included from include/linux/spinlock.h:280:0,
from kernel/rcu/tree.c:33:
include/linux/spinlock_api_smp.h:34:26: note: declared here
unsigned long __lockfunc _raw_spin_lock_irqsave(raw_spinlock_t *lock)
^
kernel/rcu/tree.c: At top level:
kernel/rcu/tree.c:1493:34: error: expected declaration specifiers or '...' before '(' token
_raw_spin_lock_irqsave_rcu_node((rnp), &(flags))
^
kernel/rcu/tree.c:1493:41: error: expected declaration specifiers or '...' before '&' token
_raw_spin_lock_irqsave_rcu_node((rnp), &(flags))
^
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from kernel/rcu/tree.c:30:
kernel/rcu/tree.c: In function 'note_gp_changes':
kernel/rcu/tree.c:1807:7: error: implicit declaration of function 'raw_spin_trylock_rcu_node' [-Werror=implicit-function-declaration]
!raw_spin_trylock_rcu_node(rnp)) { /* irqs already off, so later. */
^
include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
if (__builtin_constant_p((cond)) ? !!(cond) : \
^
>> kernel/rcu/tree.c:1804:2: note: in expansion of macro 'if'
if ((rdp->gpnum == READ_ONCE(rnp->gpnum) &&
^cc1: some warnings being treated as errors
vim +/if +1804 kernel/rcu/tree.c
5cd37193 kernel/rcu/tree.c Paul E. McKenney 2014-12-13 1788 rdp->rcu_qs_ctr_snap = __this_cpu_read(rcu_qs_ctr);
6eaef633 kernel/rcutree.c Paul E. McKenney 2013-03-19 1789 rdp->qs_pending = !!(rnp->qsmask & rdp->grpmask);
6eaef633 kernel/rcutree.c Paul E. McKenney 2013-03-19 1790 zero_cpu_stall_ticks(rdp);
7d0ae808 kernel/rcu/tree.c Paul E. McKenney 2015-03-03 1791 WRITE_ONCE(rdp->gpwrap, false);
6eaef633 kernel/rcutree.c Paul E. McKenney 2013-03-19 1792 }
48a7639c kernel/rcu/tree.c Paul E. McKenney 2014-03-11 1793 return ret;
6eaef633 kernel/rcutree.c Paul E. McKenney 2013-03-19 1794 }
6eaef633 kernel/rcutree.c Paul E. McKenney 2013-03-19 1795
d34ea322 kernel/rcutree.c Paul E. McKenney 2013-03-19 1796 static void note_gp_changes(struct rcu_state *rsp, struct rcu_data *rdp)
6eaef633 kernel/rcutree.c Paul E. McKenney 2013-03-19 1797 {
6eaef633 kernel/rcutree.c Paul E. McKenney 2013-03-19 1798 unsigned long flags;
48a7639c kernel/rcu/tree.c Paul E. McKenney 2014-03-11 1799 bool needwake;
6eaef633 kernel/rcutree.c Paul E. McKenney 2013-03-19 1800 struct rcu_node *rnp;
6eaef633 kernel/rcutree.c Paul E. McKenney 2013-03-19 1801
6eaef633 kernel/rcutree.c Paul E. McKenney 2013-03-19 1802 local_irq_save(flags);
6eaef633 kernel/rcutree.c Paul E. McKenney 2013-03-19 1803 rnp = rdp->mynode;
7d0ae808 kernel/rcu/tree.c Paul E. McKenney 2015-03-03 @1804 if ((rdp->gpnum == READ_ONCE(rnp->gpnum) &&
7d0ae808 kernel/rcu/tree.c Paul E. McKenney 2015-03-03 1805 rdp->completed == READ_ONCE(rnp->completed) &&
7d0ae808 kernel/rcu/tree.c Paul E. McKenney 2015-03-03 1806 !unlikely(READ_ONCE(rdp->gpwrap))) || /* w/out lock. */
3538015d kernel/rcu/tree.c Peter Zijlstra 2015-10-07 1807 !raw_spin_trylock_rcu_node(rnp)) { /* irqs already off, so later. */
6eaef633 kernel/rcutree.c Paul E. McKenney 2013-03-19 1808 local_irq_restore(flags);
6eaef633 kernel/rcutree.c Paul E. McKenney 2013-03-19 1809 return;
6eaef633 kernel/rcutree.c Paul E. McKenney 2013-03-19 1810 }
48a7639c kernel/rcu/tree.c Paul E. McKenney 2014-03-11 1811 needwake = __note_gp_changes(rsp, rnp, rdp);
6eaef633 kernel/rcutree.c Paul E. McKenney 2013-03-19 1812 raw_spin_unlock_irqrestore(&rnp->lock, flags);:::::: The code at line 1804 was first introduced by commit
:::::: 7d0ae8086b828311250c6afdf800b568ac9bd693 rcu: Convert ACCESS_ONCE() to READ_ONCE() and WRITE_ONCE()
:::::: TO: Paul E. McKenney <[email protected]> :::::: CC: Paul E. McKenney <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Peter Zijlstra
maintainerRe: Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
Quoted 3 lines · kbuild test robot
Hi Peter,
[auto build test WARNING on v4.3-rc4 -- if it's inappropriate base, please ignore]
So much punishment for not having compiled my proto patch :/
Wu, is there a tag one can include to ward off this patch sucking robot prematurely?
Fengguang Wu
build botRe: [kbuild-all] [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
Yes. The best way may be to push the patches to a git tree known to 0day robot:
On Wed, Oct 07, 2015 at 02:17:51PM +0200, Peter Zijlstra wrote:
> On Wed, Oct 07, 2015 at 08:11:01PM +0800, kbuild test robot wrote: > > Hi Peter, > > > > [auto build test WARNING on v4.3-rc4 -- if it's inappropriate base, please ignore] > > So much punishment for not having compiled my proto patch :/ > > Wu, is there a tag one can include to ward off this patch sucking robot > prematurely?
Yes. The best way may be to push the patches to a git tree known to 0day robot:
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/tree/repo/linux
So that it's tested first there. You'll then get private email reports if it's a private git branch.
The robot has logic to avoid duplicate testing an emailed patch (based on patch-id and author/title) if its git tree version has been tested.
We may also add a rule: only send private reports for patches with "RFC", "Not-yet-signed-off-by:", etc.
Thanks,
Fengguang
Peter Zijlstra
maintainerRe: [kbuild-all] [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
Quoted 10 lines · Fengguang Wu
Quoted 2 lines
Wu, is there a tag one can include to ward off this patch sucking robot prematurely?
Yes. The best way may be to push the patches to a git tree known to 0day robot:
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/tree/repo/linux
So that it's tested first there. You'll then get private email reports if it's a private git branch.
Right, but if I can't be bothered to compile test a patch, I also cannot be bothered to stuff it into git :-)
Quoted 2 lines
We may also add a rule: only send private reports for patches with "RFC", "Not-yet-signed-off-by:", etc.
How about not building when there's no "^Signed-off-by:" at all?
Even private build fails for patches like this -- esp. 3+ -- gets annoying real quick.
Also note that this 'patch' has: $subject ~ /^Re:/, nor did it have "^Subject:" like headers in the body.
Fengguang Wu
build botRe: [kbuild-all] [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
OK, that's understandable.
On Wed, Oct 07, 2015 at 03:55:29PM +0200, Peter Zijlstra wrote:
> On Wed, Oct 07, 2015 at 09:44:32PM +0800, Fengguang Wu wrote: > > > > Wu, is there a tag one can include to ward off this patch sucking robot > > > prematurely? > > > > Yes. The best way may be to push the patches to a git tree known to > > 0day robot: > > > > https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/tree/repo/linux > > > > So that it's tested first there. You'll then get private email reports > > if it's a private git branch. > > Right, but if I can't be bothered to compile test a patch, I also cannot > be bothered to stuff it into git :-)
OK, that's understandable.
> > We may also add a rule: only send private reports for patches with > > "RFC", "Not-yet-signed-off-by:", etc. > > How about not building when there's no "^Signed-off-by:" at all?
That's a good idea: no need to test quick demo-of-idea patches.
> Even private build fails for patches like this -- esp. 3+ -- gets > annoying real quick. > > Also note that this 'patch' has: $subject ~ /^Re:/, nor did it have > "^Subject:" like headers in the body.
That's good clues, too. So how about make the rule
Skip test if no "^Signed-off-by:" and Subject =~ /^Re:/
For a patch posted inside a discussion thread, as long as it have "^Signed-off-by:", I guess the author is serious and the patch could be tested seriously.
Thanks,
Fengguang
Peter Zijlstra
maintainerRe: [kbuild-all] [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
Quoted 11 lines · Fengguang Wu
Quoted 1 line · Peter Zijlstra
How about not building when there's no "^Signed-off-by:" at all?
That's a good idea: no need to test quick demo-of-idea patches.
Quoted 5 lines
Even private build fails for patches like this -- esp. 3+ -- gets annoying real quick.
Also note that this 'patch' has: $subject ~ /^Re:/, nor did it have "^Subject:" like headers in the body.
Quoted 7 lines
That's good clues, too. So how about make the rule
Skip test if no "^Signed-off-by:" and Subject =~ /^Re:/
For a patch posted inside a discussion thread, as long as it have "^Signed-off-by:", I guess the author is serious and the patch could be tested seriously.
Works for me; now hoping I will abide by my own suggested rules ;-)
Thanks!
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
In reply to Peter ZijlstraQuoted 9 lines · Peter Zijlstra
Quoted 2 lines · Peter Zijlstra
That again doesn't explain which UNLOCKs with non-matching lock values it pairs with and what particular ordering is important here.
So after staring at that stuff for a while I came up with the following. Does this make sense, or am I completely misunderstanding things?
Not been near a compiler.
Actually, this would be quite good. "Premature abstraction is the root of all evil" and all that, but this abstraction is anything but premature. My thought would be to have it against commit cd58087c9cee ("Merge branches 'doc.2015.10.06a', 'percpu-rwsem.2015.10.06a' and 'torture.2015.10.06a' into HEAD") in -rcu given the merge conflicts that would otherwise arise.
Thanx, Paul
Quoted 244 lines
--- kernel/rcu/tree.c | 99 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 61 insertions(+), 38 deletions(-)kernel/rcu/tree.c +61
-38@@ -1460,6 +1460,48 @@ static void trace_rcu_future_gp(struct rcu_node *rnp, struct rcu_data *rdp, }/* +* Wrappers for the rcu_node::lock acquire. +* +* Because the rcu_nodes form a tree, the tree taversal locking will observe +* different lock values, this in turn means that an UNLOCK of one level +* followed by a LOCK of another level does not imply a full memory barrier; +* and most importantly transitivity is lost. +* +* In order to restore full ordering between tree levels, augment the regular +* lock acquire functions with smp_mb__after_unlock_lock(). +*/ +static inline void raw_spin_lock_rcu_node (struct rcu_node *rnp) +{ +raw_spin_lock (&rnp->lock); +smp_mb__after_unlock_lock (); +} + +static inline void raw_spin_lock_irq_rcu_node (struct rcu_node *rnp) +{ +raw_spin_lock_irq (&rnp->lock); +smp_mb__after_unlock_lock (); +} + +static inline void +_raw_spin_lock_irqsave_rcu_node (struct rcu_node *rnp,unsigned long *flags) +{ +_raw_spin_lock_irqsave (&rnp->lock, flags); +smp_mb__after_unlock_lock (); +} + +#define raw_spin_lock_irqsave_rcu_node (rnp, flags) + _raw_spin_lock_irqsave_rcu_node((rnp), &(flags)) + +static inline bool raw_spin_trylock_rcu_node (struct rcu_node *rnp) +{ +bool locked =raw_spin_trylock (&rnp->lock); +if (locked) +smp_mb__after_unlock_lock (); +return locked; +} + + +/* * Start some future grace period, as needed to handle newly arrived * callbacks. The required future grace periods are recorded in each * rcu_node structure's ->need_future_gp field. Returns true if there @@ -1512,10 +1554,8 @@ rcu_start_future_gp(struct rcu_node *rnp, struct rcu_data *rdp,* hold it, acquire the root rcu_node structure's lock in order to * start one (if needed). */ -if (rnp != rnp_root) { -raw_spin_lock (&rnp_root->lock); -smp_mb__after_unlock_lock (); - } +if (rnp != rnp_root) +raw_spin_lock_rcu_node (rnp);/* * Get a new grace-period number. If there really is no grace @@ -1764,11 +1804,10 @@ static void note_gp_changes(struct rcu_state *rsp, struct rcu_data *rdp)if ((rdp->gpnum ==READ_ONCE (rnp->gpnum) && rdp->completed ==READ_ONCE (rnp->completed) && !unlikely (READ_ONCE (rdp->gpwrap))) ||/* w/out lock. */ - !raw_spin_trylock (&rnp->lock)) {/* irqs already off, so later. */ + !raw_spin_trylock_rcu_node (rnp)) {/* irqs already off, so later. */ local_irq_restore (flags);return ; } -smp_mb__after_unlock_lock (); needwake =__note_gp_changes (rsp, rnp, rdp);raw_spin_unlock_irqrestore (&rnp->lock, flags);if (needwake)@@ -1792,8 +1831,7 @@ static int rcu_gp_init(struct rcu_state *rsp)struct rcu_node *rnp =rcu_get_root (rsp);WRITE_ONCE (rsp->gp_activity, jiffies); -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_irq_rcu_node (rnp);if (!READ_ONCE (rsp->gp_flags)) {/* Spurious wakeup, tell caller to go back to sleep. */ raw_spin_unlock_irq (&rnp->lock);@@ -1825,8 +1863,7 @@ static int rcu_gp_init(struct rcu_state *rsp)*/ rcu_for_each_leaf_node (rsp, rnp) {rcu_gp_slow (rsp, gp_preinit_delay); -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_irq_rcu_node (rnp);if (rnp->qsmaskinit == rnp->qsmaskinitnext && !rnp->wait_blkd_tasks) {/* Nothing to do on this leaf rcu_node structure. */ @@ -1882,8 +1919,7 @@ static int rcu_gp_init(struct rcu_state *rsp)*/ rcu_for_each_node_breadth_first (rsp, rnp) {rcu_gp_slow (rsp, gp_init_delay); -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_irq_rcu_node (rnp); rdp =this_cpu_ptr (rsp->rda);rcu_preempt_check_blocked_tasks (rnp); rnp->qsmask = rnp->qsmaskinit;@@ -1953,8 +1989,7 @@ static int rcu_gp_fqs(struct rcu_state *rsp, int fqs_state_in) }/* Clear flag to prevent immediate re-entry. */ if (READ_ONCE (rsp->gp_flags) &RCU_GP_FLAG_FQS ) { -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_irq_rcu_node (rnp);WRITE_ONCE (rsp->gp_flags,READ_ONCE (rsp->gp_flags) & ~RCU_GP_FLAG_FQS );raw_spin_unlock_irq (&rnp->lock);@@ -1974,8 +2009,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)struct rcu_node *rnp =rcu_get_root (rsp);WRITE_ONCE (rsp->gp_activity, jiffies); -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_irq_rcu_node (rnp); gp_duration = jiffies - rsp->gp_start;if (gp_duration > rsp->gp_max) rsp->gp_max = gp_duration;@@ -2000,8 +2034,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)* grace period is recorded in any of the rcu_node structures. */ rcu_for_each_node_breadth_first (rsp, rnp) { -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_irq_rcu_node (rnp);WARN_ON_ONCE (rcu_preempt_blocked_readers_cgp (rnp));WARN_ON_ONCE (rnp->qsmask);WRITE_ONCE (rnp->completed, rsp->gpnum);@@ -2016,8 +2049,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)rcu_gp_slow (rsp, gp_cleanup_delay); } rnp =rcu_get_root (rsp); -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock ();/* Order GP before ->completed update. */ +raw_spin_lock_irq_rcu_node (rnp);/* Order GP before ->completed update. */ rcu_nocb_gp_set (rnp, nocb);/* Declare grace period done. */ @@ -2264,8 +2296,7 @@ rcu_report_qs_rnp(unsigned long mask, struct rcu_state *rsp,raw_spin_unlock_irqrestore (&rnp->lock, flags); rnp_c = rnp; rnp = rnp->parent; -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp, flags); oldmask = rnp_c->qsmask; }@@ -2312,8 +2343,7 @@ static void rcu_report_unblock_qs_rnp(struct rcu_state *rsp, gps = rnp->gpnum; mask = rnp->grpmask;raw_spin_unlock (&rnp->lock);/* irqs remain disabled. */ -raw_spin_lock (&rnp_p->lock);/* irqs already disabled. */ -smp_mb__after_unlock_lock (); +raw_spin_lock_rcu_node (rnp);/* irqs already disabled. */ rcu_report_qs_rnp (mask, rsp, rnp_p, gps, flags); }@@ -2335,8 +2365,7 @@ rcu_report_qs_rdp(int cpu, struct rcu_state *rsp, struct rcu_data *rdp)struct rcu_node *rnp; rnp = rdp->mynode; -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp, flags); if ((rdp->passed_quiesce == 0 && rdp->rcu_qs_ctr_snap == __this_cpu_read(rcu_qs_ctr)) || rdp->gpnum != rnp->gpnum || rnp->completed == rnp->gpnum ||@@ -2562,8 +2591,7 @@ static void rcu_cleanup_dead_rnp(struct rcu_node *rnp_leaf) rnp = rnp->parent;if (!rnp)break ; -raw_spin_lock (&rnp->lock);/* irqs already disabled. */ -smp_mb__after_unlock_lock ();/* GP memory ordering. */ +raw_spin_lock_rcu_node (rnp);/* irqs already disabled. */ rnp->qsmaskinit &= ~mask; rnp->qsmask &= ~mask;if (rnp->qsmaskinit) {@@ -2591,8 +2619,7 @@ static void rcu_cleanup_dying_idle_cpu(int cpu, struct rcu_state *rsp)/* Remove outgoing CPU from mask in the leaf rcu_node structure. */ mask = rdp->grpmask; -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock ();/* Enforce GP memory-order guarantee. */ +raw_spin_lock_irqsave_rcu_node (rnp, flags);/* Enforce GP memory-order guarantee. */ rnp->qsmaskinitnext &= ~mask;raw_spin_unlock_irqrestore (&rnp->lock, flags); }@@ -2789,8 +2816,7 @@ static void force_qs_rnp(struct rcu_state *rsp,rcu_for_each_leaf_node (rsp, rnp) {cond_resched_rcu_qs (); mask =0 ; -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp, flags);if (rnp->qsmask ==0 ) { if (rcu_state_p == &rcu_sched_state || rsp != rcu_state_p ||@@ -2861,8 +2887,7 @@ static void force_quiescent_state(struct rcu_state *rsp)/* rnp_old == rcu_get_root(rsp), rnp == NULL. */ /* Reached the root of the rcu_node tree, acquire lock. */ -raw_spin_lock_irqsave (&rnp_old->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp_old, flags);raw_spin_unlock (&rnp_old->fqslock);if (READ_ONCE (rsp->gp_flags) &RCU_GP_FLAG_FQS ) { rsp->n_force_qs_lh++;@@ -2985,8 +3010,7 @@ static void __call_rcu_core(struct rcu_state *rsp, struct rcu_data *rdp,if (!rcu_gp_in_progress (rsp)) {struct rcu_node *rnp_root =rcu_get_root (rsp); -raw_spin_lock (&rnp_root->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_rcu_node (rnp_root); needwake =rcu_start_gp (rsp);raw_spin_unlock (&rnp_root->lock);if (needwake)@@ -3925,8 +3949,7 @@ rcu_init_percpu_data(int cpu, struct rcu_state *rsp)*/ rnp = rdp->mynode; mask = rdp->grpmask; -raw_spin_lock (&rnp->lock);/* irqs already disabled. */ -smp_mb__after_unlock_lock (); +raw_spin_lock_rcu_node (rnp);/* irqs already disabled. */ rnp->qsmaskinitnext |= mask; rdp->gpnum = rnp->completed;/* Make CPU later note any new GP. */ rdp->completed = rnp->completed;
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
Quoted 6 lines · Paul E. McKenney
Actually, this would be quite good. "Premature abstraction is the root of all evil" and all that, but this abstraction is anything but premature. My thought would be to have it against commit
cd58087c9cee("Merge branches 'doc.2015.10.06a', 'percpu-rwsem.2015.10.06a' and 'torture.2015.10.06a' into HEAD") in -rcu given the merge conflicts that would otherwise arise.
OK here goes, compile tested this time ;-)
---
Subject: rcu: Clarify the smp_mb__after_unlock_lock usage
Because undocumented barriers are bad remove all the uncommented smp_mb__after_unlock_lock() usage and replace it with a documented set of wrappers.
The problem is that PPC has RCpc UNLOCK+LOCK where all other archs have RCsc, which means that on PPC UNLOCK x + LOCK y does not form a full barrier (which also implies transitivity) and needs help.
AFAICT the only case where this really matters is the rcu_node tree traversal, where we want to ensure the state a node is 'complete' before propagating its state up the tree, such that once we reach the top, all CPUs agree on the observed state.
--- kernel/rcu/tree.c | 128 ++++++++++++++++++++++++++++------------------- kernel/rcu/tree.h | 11 ---- kernel/rcu/tree_plugin.h | 18 +++---- 3 files changed, 82 insertions(+), 75 deletions(-)
kernel/rcu/tree.c +76 -52
@@ -1482,6 +1482,56 @@ static void trace_rcu_future_gp(struct rcu_node *rnp, struct rcu_data *rdp, }/* +* Place this after a lock-acquisition primitive to guarantee that +* an UNLOCK+LOCK pair act as a full barrier. This guarantee applies +* if the UNLOCK and LOCK are executed by the same CPU or if the +* UNLOCK and LOCK operate on the same lock variable. +*/ +#ifdef CONFIG_PPC +#define smp_mb__after_unlock_lock () smp_mb()/* Full ordering for lock. */ +#else /* #ifdef CONFIG_PPC */ +#define smp_mb__after_unlock_lock () do { } while (0) +#endif /* #else #ifdef CONFIG_PPC */ + +/* +* Wrappers for the rcu_node::lock acquire. +* +* Because the rcu_nodes form a tree, the tree traversal locking will observe +* different lock values, this in turn means that an UNLOCK of one level +* followed by a LOCK of another level does not imply a full memory barrier; +* and most importantly transitivity is lost. +* +* In order to restore full ordering between tree levels, augment the regular +* lock acquire functions with smp_mb__after_unlock_lock(). +*/ +static inline void raw_spin_lock_rcu_node (struct rcu_node *rnp) +{ +raw_spin_lock (&rnp->lock); +smp_mb__after_unlock_lock (); +} + +static inline void raw_spin_lock_irq_rcu_node (struct rcu_node *rnp) +{ +raw_spin_lock_irq (&rnp->lock); +smp_mb__after_unlock_lock (); +} + +#define raw_spin_lock_irqsave_rcu_node (rnp, flags) \ +do { \ + typecheck(unsigned long, flags); \ + flags = _raw_spin_lock_irqsave(&(rnp)->lock); \ + smp_mb__after_unlock_lock(); \ +} while (0) + +static inline bool raw_spin_trylock_rcu_node (struct rcu_node *rnp) +{ +bool locked =raw_spin_trylock (&rnp->lock); +if (locked) +smp_mb__after_unlock_lock (); +return locked; +} + +/* * Start some future grace period, as needed to handle newly arrived * callbacks. The required future grace periods are recorded in each * rcu_node structure's ->need_future_gp field. Returns true if there
@@ -1534,10 +1584,8 @@ rcu_start_future_gp(struct rcu_node *rnp, struct rcu_data *rdp,* hold it, acquire the root rcu_node structure's lock in order to * start one (if needed). */ -if (rnp != rnp_root) { -raw_spin_lock (&rnp_root->lock); -smp_mb__after_unlock_lock (); - } +if (rnp != rnp_root) +raw_spin_lock_rcu_node (rnp_root);/* * Get a new grace-period number. If there really is no grace
@@ -1786,11 +1834,10 @@ static void note_gp_changes(struct rcu_state *rsp, struct rcu_data *rdp)if ((rdp->gpnum ==READ_ONCE (rnp->gpnum) && rdp->completed ==READ_ONCE (rnp->completed) && !unlikely (READ_ONCE (rdp->gpwrap))) ||/* w/out lock. */ - !raw_spin_trylock (&rnp->lock)) {/* irqs already off, so later. */ + !raw_spin_trylock_rcu_node (rnp)) {/* irqs already off, so later. */ local_irq_restore (flags);return ; } -smp_mb__after_unlock_lock (); needwake =__note_gp_changes (rsp, rnp, rdp);raw_spin_unlock_irqrestore (&rnp->lock, flags);if (needwake)
@@ -1814,8 +1861,7 @@ static int rcu_gp_init(struct rcu_state *rsp)struct rcu_node *rnp =rcu_get_root (rsp);WRITE_ONCE (rsp->gp_activity, jiffies); -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_irq_rcu_node (rnp);if (!READ_ONCE (rsp->gp_flags)) {/* Spurious wakeup, tell caller to go back to sleep. */ raw_spin_unlock_irq (&rnp->lock);
@@ -1847,8 +1893,7 @@ static int rcu_gp_init(struct rcu_state *rsp)*/ rcu_for_each_leaf_node (rsp, rnp) {rcu_gp_slow (rsp, gp_preinit_delay); -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_irq_rcu_node (rnp);if (rnp->qsmaskinit == rnp->qsmaskinitnext && !rnp->wait_blkd_tasks) {/* Nothing to do on this leaf rcu_node structure. */
@@ -1904,8 +1949,7 @@ static int rcu_gp_init(struct rcu_state *rsp)*/ rcu_for_each_node_breadth_first (rsp, rnp) {rcu_gp_slow (rsp, gp_init_delay); -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_irq_rcu_node (rnp); rdp =this_cpu_ptr (rsp->rda);rcu_preempt_check_blocked_tasks (rnp); rnp->qsmask = rnp->qsmaskinit;
@@ -1973,8 +2017,7 @@ static void rcu_gp_fqs(struct rcu_state *rsp, bool first_time) }/* Clear flag to prevent immediate re-entry. */ if (READ_ONCE (rsp->gp_flags) &RCU_GP_FLAG_FQS ) { -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_irq_rcu_node (rnp);WRITE_ONCE (rsp->gp_flags,READ_ONCE (rsp->gp_flags) & ~RCU_GP_FLAG_FQS );raw_spin_unlock_irq (&rnp->lock);
@@ -1993,8 +2036,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)struct rcu_node *rnp =rcu_get_root (rsp);WRITE_ONCE (rsp->gp_activity, jiffies); -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_irq_rcu_node (rnp); gp_duration = jiffies - rsp->gp_start;if (gp_duration > rsp->gp_max) rsp->gp_max = gp_duration;
@@ -2019,8 +2061,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)* grace period is recorded in any of the rcu_node structures. */ rcu_for_each_node_breadth_first (rsp, rnp) { -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_irq_rcu_node (rnp);WARN_ON_ONCE (rcu_preempt_blocked_readers_cgp (rnp));WARN_ON_ONCE (rnp->qsmask);WRITE_ONCE (rnp->completed, rsp->gpnum);
@@ -2035,8 +2076,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)rcu_gp_slow (rsp, gp_cleanup_delay); } rnp =rcu_get_root (rsp); -raw_spin_lock_irq (&rnp->lock); -smp_mb__after_unlock_lock ();/* Order GP before ->completed update. */ +raw_spin_lock_irq_rcu_node (rnp);/* Order GP before ->completed update. */ rcu_nocb_gp_set (rnp, nocb);/* Declare grace period done. */
@@ -2284,8 +2324,7 @@ rcu_report_qs_rnp(unsigned long mask, struct rcu_state *rsp,raw_spin_unlock_irqrestore (&rnp->lock, flags); rnp_c = rnp; rnp = rnp->parent; -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp, flags); oldmask = rnp_c->qsmask; }
@@ -2332,8 +2371,7 @@ static void rcu_report_unblock_qs_rnp(struct rcu_state *rsp, gps = rnp->gpnum; mask = rnp->grpmask;raw_spin_unlock (&rnp->lock);/* irqs remain disabled. */ -raw_spin_lock (&rnp_p->lock);/* irqs already disabled. */ -smp_mb__after_unlock_lock (); +raw_spin_lock_rcu_node (rnp_p);/* irqs already disabled. */ rcu_report_qs_rnp (mask, rsp, rnp_p, gps, flags); }
@@ -2355,8 +2393,7 @@ rcu_report_qs_rdp(int cpu, struct rcu_state *rsp, struct rcu_data *rdp)struct rcu_node *rnp; rnp = rdp->mynode; -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp, flags); if ((rdp->cpu_no_qs.b.norm && rdp->rcu_qs_ctr_snap == __this_cpu_read(rcu_qs_ctr)) || rdp->gpnum != rnp->gpnum || rnp->completed == rnp->gpnum ||
@@ -2582,8 +2619,7 @@ static void rcu_cleanup_dead_rnp(struct rcu_node *rnp_leaf) rnp = rnp->parent;if (!rnp)break ; -raw_spin_lock (&rnp->lock);/* irqs already disabled. */ -smp_mb__after_unlock_lock ();/* GP memory ordering. */ +raw_spin_lock_rcu_node (rnp);/* irqs already disabled. */ rnp->qsmaskinit &= ~mask; rnp->qsmask &= ~mask;if (rnp->qsmaskinit) {
@@ -2611,8 +2647,7 @@ static void rcu_cleanup_dying_idle_cpu(int cpu, struct rcu_state *rsp)/* Remove outgoing CPU from mask in the leaf rcu_node structure. */ mask = rdp->grpmask; -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock ();/* Enforce GP memory-order guarantee. */ +raw_spin_lock_irqsave_rcu_node (rnp, flags);/* Enforce GP memory-order guarantee. */ rnp->qsmaskinitnext &= ~mask;raw_spin_unlock_irqrestore (&rnp->lock, flags); }
@@ -2809,8 +2844,7 @@ static void force_qs_rnp(struct rcu_state *rsp,rcu_for_each_leaf_node (rsp, rnp) {cond_resched_rcu_qs (); mask =0 ; -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp, flags);if (rnp->qsmask ==0 ) { if (rcu_state_p == &rcu_sched_state || rsp != rcu_state_p ||
@@ -2881,8 +2915,7 @@ static void force_quiescent_state(struct rcu_state *rsp)/* rnp_old == rcu_get_root(rsp), rnp == NULL. */ /* Reached the root of the rcu_node tree, acquire lock. */ -raw_spin_lock_irqsave (&rnp_old->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp_old, flags);raw_spin_unlock (&rnp_old->fqslock);if (READ_ONCE (rsp->gp_flags) &RCU_GP_FLAG_FQS ) { rsp->n_force_qs_lh++;
@@ -3005,8 +3038,7 @@ static void __call_rcu_core(struct rcu_state *rsp, struct rcu_data *rdp,if (!rcu_gp_in_progress (rsp)) {struct rcu_node *rnp_root =rcu_get_root (rsp); -raw_spin_lock (&rnp_root->lock); -smp_mb__after_unlock_lock (); +raw_spin_lock_rcu_node (rnp_root); needwake =rcu_start_gp (rsp);raw_spin_unlock (&rnp_root->lock);if (needwake)
@@ -3426,8 +3458,7 @@ static void sync_exp_reset_tree_hotplug(struct rcu_state *rsp)* CPUs for the current rcu_node structure up the rcu_node tree. */ rcu_for_each_leaf_node (rsp, rnp) { -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp, flags);if (rnp->expmaskinit == rnp->expmaskinitnext) {raw_spin_unlock_irqrestore (&rnp->lock, flags);continue ;/* No new CPUs, nothing to do. */
@@ -3447,8 +3478,7 @@ static void sync_exp_reset_tree_hotplug(struct rcu_state *rsp) rnp_up = rnp->parent; done = false;while (rnp_up) { -raw_spin_lock_irqsave (&rnp_up->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp_up, flags);if (rnp_up->expmaskinit) done = true; rnp_up->expmaskinit |= mask;
@@ -3472,8 +3502,7 @@ static void __maybe_unused sync_exp_reset_tree(struct rcu_state *rsp)sync_exp_reset_tree_hotplug (rsp);rcu_for_each_node_breadth_first (rsp, rnp) { -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp, flags);WARN_ON_ONCE (rnp->expmask); rnp->expmask = rnp->expmaskinit;raw_spin_unlock_irqrestore (&rnp->lock, flags);
@@ -3531,8 +3560,7 @@ static void __rcu_report_exp_rnp(struct rcu_state *rsp, struct rcu_node *rnp, mask = rnp->grpmask;raw_spin_unlock (&rnp->lock);/* irqs remain disabled */ rnp = rnp->parent; -raw_spin_lock (&rnp->lock);/* irqs already disabled */ -smp_mb__after_unlock_lock (); +raw_spin_lock_rcu_node (rnp);/* irqs already disabled */ WARN_ON_ONCE (!(rnp->expmask & mask)); rnp->expmask &= ~mask; }
@@ -3549,8 +3577,7 @@ static void __maybe_unused rcu_report_exp_rnp(struct rcu_state *rsp, {unsigned long flags; -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp, flags);__rcu_report_exp_rnp (rsp, rnp, wake, flags); }
@@ -3564,8 +3591,7 @@ static void rcu_report_exp_cpu_mult(struct rcu_state *rsp, struct rcu_node *rnp, {unsigned long flags; -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp, flags);if (!(rnp->expmask & mask)) {raw_spin_unlock_irqrestore (&rnp->lock, flags);return ;
@@ -3708,8 +3734,7 @@ static void sync_rcu_exp_select_cpus(struct rcu_state *rsp,sync_exp_reset_tree (rsp);rcu_for_each_leaf_node (rsp, rnp) { -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp, flags);/* Each pass checks a CPU for identity, offline, and idle. */ mask_ofl_test =0 ;
@@ -4198,8 +4223,7 @@ rcu_init_percpu_data(int cpu, struct rcu_state *rsp)*/ rnp = rdp->mynode; mask = rdp->grpmask; -raw_spin_lock (&rnp->lock);/* irqs already disabled. */ -smp_mb__after_unlock_lock (); +raw_spin_lock_rcu_node (rnp);/* irqs already disabled. */ rnp->qsmaskinitnext |= mask; rnp->expmaskinitnext |= mask;if (!rdp->beenonline)
kernel/rcu/tree.h +0 -11
@@ -653,14 +653,3 @@ static inline void rcu_nocb_q_lengths(struct rcu_data *rdp, long *ql, long *qll) }#endif /* #ifdef CONFIG_RCU_TRACE */ -/* -* Place this after a lock-acquisition primitive to guarantee that -* an UNLOCK+LOCK pair act as a full barrier. This guarantee applies -* if the UNLOCK and LOCK are executed by the same CPU or if the -* UNLOCK and LOCK operate on the same lock variable. -*/ -#ifdef CONFIG_PPC -#define smp_mb__after_unlock_lock () smp_mb()/* Full ordering for lock. */ -#else /* #ifdef CONFIG_PPC */ -#define smp_mb__after_unlock_lock () do { } while (0) -#endif /* #else #ifdef CONFIG_PPC */
kernel/rcu/tree_plugin.h +6 -12
@@ -301,8 +301,7 @@ static void rcu_preempt_note_context_switch(void)/* Possibly blocking in an RCU read-side critical section. */ rdp =this_cpu_ptr (rcu_state_p->rda); rnp = rdp->mynode; -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp, flags); t->rcu_read_unlock_special.b.blocked = true; t->rcu_blocked_node = rnp;
@@ -457,8 +456,7 @@ void rcu_read_unlock_special(struct task_struct *t)*/ for (;;) { rnp = t->rcu_blocked_node; -raw_spin_lock (&rnp->lock);/* irqs already disabled. */ -smp_mb__after_unlock_lock (); +raw_spin_lock_rcu_node (rnp);/* irqs already disabled. */ if (rnp == t->rcu_blocked_node)break ;WARN_ON_ONCE (1 );
@@ -989,8 +987,7 @@ static int rcu_boost(struct rcu_node *rnp) READ_ONCE(rnp->boost_tasks) == NULL)return 0 ;/* Nothing left to boost. */ -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp, flags);/* * Recheck under the lock: all tasks in need of boosting
@@ -1176,8 +1173,7 @@ static int rcu_spawn_one_boost_kthread(struct rcu_state *rsp, "rcub/%d", rnp_index);if (IS_ERR (t))return PTR_ERR (t); -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp, flags); rnp->boost_kthread_task = t;raw_spin_unlock_irqrestore (&rnp->lock, flags); sp.sched_priority = kthread_prio;
@@ -1567,8 +1563,7 @@ static void rcu_prepare_for_idle(void)if (!*rdp->nxttail[RCU_DONE_TAIL ])continue ; rnp = rdp->mynode; -raw_spin_lock (&rnp->lock);/* irqs already disabled. */ -smp_mb__after_unlock_lock (); +raw_spin_lock_rcu_node (rnp);/* irqs already disabled. */ needwake =rcu_accelerate_cbs (rsp, rnp, rdp);raw_spin_unlock (&rnp->lock);/* irqs remain disabled. */ if (needwake)
@@ -2068,8 +2063,7 @@ static void rcu_nocb_wait_gp(struct rcu_data *rdp)bool needwake;struct rcu_node *rnp = rdp->mynode; -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock (); +raw_spin_lock_irqsave_rcu_node (rnp, flags); needwake =rcu_start_future_gp (rnp, rdp, &c);raw_spin_unlock_irqrestore (&rnp->lock, flags);if (needwake)
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
In reply to Peter ZijlstraQuoted 42 lines · Peter Zijlstra
Quoted 33 lines · Mathieu Desnoyers
Quoted 31 lines · ----- On Oct 7, 2015, at 3:51 AM, Peter Zijlstra [email protected] wrote:
Quoted 13 lines · Paul E. McKenney
Quoted 12 lines · Peter Zijlstra
kernel/rcu/tree.c static bool rcu_exp_gp_seq_done(struct rcu_state *rsp, unsigned long s) · line 3424
Quoted 7 lines · Paul E. McKenney
kernel/rcu/tree.c +7
-0+static void __maybe_unused rcu_report_exp_rnp (struct rcu_state *rsp, +struct rcu_node *rnp,bool wake) +{ +unsigned long flags; +unsigned long mask; + +raw_spin_lock_irqsave (&rnp->lock, flags);Normally we require a comment with barriers, explaining the order and the pairing etc.. :-)
Quoted 6 lines
Quoted 1 line
Review
Quoted 1 line
(quoted diff) +1
-0+ smp_mb__after_unlock_lock();Hmmmm... That is not good.
Worse yet, I am missing comments on most of the pre-existing barriers of this form.
Yes I noticed.. :/
Quoted 7 lines
The purpose is to enforce the heavy-weight grace-period memory-ordering guarantees documented in the
synchronize_sched()header comment and elsewhere.They pair with anything you might use to check for violation of these guarantees, or, simiarly, any ordering that you might use when relying on these guarantees.
Quoted 7 lines
Quoted 4 lines
I'm sure you know what that means, but I've no clue ;-) That is, I wouldn't know where to start looking in the RCU implementation to verify the barrier is either needed or sufficient. Unless you mean _everywhere_ :-)
One example is the new membarrier system call. It relies on
synchronize_sched()to enforce this:
Quoted 7 lines
That again doesn't explain which UNLOCKs with non-matching lock values it pairs with and what particular ordering is important here.
I'm fully well aware of what
sync_sched()guarantees and how one can use it, that is not the issue, what I'm saying is that a generic description ofsync_sched()doesn't help in figuring out WTH that barrier is for and which other code I should also inspect.
Unfortunately, the answer is "pretty much all of it". :-(
The enforced ordering relies on pretty much every acquisition/release of an rcu_node structure's ->lock and all the dyntick-idle stuff, plus some explicit barriers and a few smp_load_acquire()s and smp_store_release()s.
Thanx, Paul
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
In reply to Peter ZijlstraQuoted 14 lines · Peter Zijlstra
Quoted 13 lines · Paul E. McKenney
Quoted 12 lines · Peter Zijlstra
kernel/rcu/tree.c static bool rcu_exp_gp_seq_done(struct rcu_state *rsp, unsigned long s) · line 3424
Quoted 7 lines · Paul E. McKenney
kernel/rcu/tree.c +7
-0+static void __maybe_unused rcu_report_exp_rnp (struct rcu_state *rsp, +struct rcu_node *rnp,bool wake) +{ +unsigned long flags; +unsigned long mask; + +raw_spin_lock_irqsave (&rnp->lock, flags);Normally we require a comment with barriers, explaining the order and the pairing etc.. :-)
Quoted 8 lines
Quoted 6 lines
Quoted 1 line
Review
Quoted 1 line
(quoted diff) +1
-0+ smp_mb__after_unlock_lock();Hmmmm... That is not good.
Worse yet, I am missing comments on most of the pre-existing barriers of this form.
Yes I noticed.. :/
Will fix, though probably as a follow-up patch. Once I figure out what comment makes sense...
Quoted 4 lines
Quoted 3 lines
The purpose is to enforce the heavy-weight grace-period memory-ordering guarantees documented in the
synchronize_sched()header comment and elsewhere.
Quoted 8 lines
Quoted 3 lines
They pair with anything you might use to check for violation of these guarantees, or, simiarly, any ordering that you might use when relying on these guarantees.
I'm sure you know what that means, but I've no clue ;-) That is, I wouldn't know where to start looking in the RCU implementation to verify the barrier is either needed or sufficient. Unless you mean _everywhere_ :-)
Pretty much everywhere.
Let's take the usual RCU removal pattern as an example:
void f1(struct foo *p)
{
list_del_rcu(p);
synchronize_rcu_expedited();
kfree(p);
}
void f2(void)
{
struct foo *p;
list_for_each_entry_rcu(p, &my_head, next)
do_something_with(p);
}So the synchronize_rcu_expedited() acts as an extremely heavyweight memory barrier that pairs with the rcu_dereference() inside of list_for_each_entry_rcu(). Easy enough, right?
But what exactly within synchronize_rcu_expedited() provides the ordering? The answer is a web of lock-based critical sections and explicit memory barriers, with the one you called out as needing a comment being one of them.
Quoted 11 lines
Quoted 8 lines
I could add something like "/* Enforce GP memory ordering. */"
Or perhaps "/* See
synchronize_sched()header. */"I do not propose reproducing the
synchronize_sched()header on each of these. That would be verbose, even for me! ;-)Other thoughts?
Well, this is an UNLOCK+LOCK on non-matching lock variables upgrade to full barrier thing, right?
Yep!
Quoted 3 lines
To me its not clear which UNLOCK we even match here. I've just read the
sync_sched()header, but that doesn't help me either, so referring to that isn't really helpful either.
Usually this pairs with an rcu_dereference() somewhere in the calling code. Some other task in the calling code, actually.
Quoted 3 lines
In any case, I don't want to make too big a fuzz here, but I just stumbled over a lot of unannotated barriers and figured I ought to say something about it.
I do need to better document how this works, no two ways about it.
Thanx, Paul
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
Quoted 24 lines · Paul E. McKenney
Quoted 4 lines
I'm sure you know what that means, but I've no clue ;-) That is, I wouldn't know where to start looking in the RCU implementation to verify the barrier is either needed or sufficient. Unless you mean _everywhere_ :-)
Pretty much everywhere.
Let's take the usual RCU removal pattern as an example:
void f1(struct foo *p) { list_del_rcu(p); synchronize_rcu_expedited(); kfree(p); } void f2(void) { struct foo *p; list_for_each_entry_rcu(p, &my_head, next) do_something_with(p); }
Quoted 8 lines
So the
synchronize_rcu_expedited()acts as an extremely heavyweight memory barrier that pairs with thercu_dereference()inside oflist_for_each_entry_rcu(). Easy enough, right?But what exactly within
synchronize_rcu_expedited()provides the ordering? The answer is a web of lock-based critical sections and explicit memory barriers, with the one you called out as needing a comment being one of them.
Right, but seeing there's possible implementations of sync_rcu(_exp)*() that do not have the whole rcu_node tree like thing, there's more to this particular barrier than the semantics of sync_rcu().
Some implementation choice requires this barrier upgrade -- and in another email I suggest its the whole tree thing, we need to firmly establish the state of one level before propagating the state up etc.
Now I'm not entirely sure this is fully correct, but its the best I could come up.
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
Quoted 38 lines · Peter Zijlstra
Quoted 24 lines · Paul E. McKenney
Quoted 4 lines
I'm sure you know what that means, but I've no clue ;-) That is, I wouldn't know where to start looking in the RCU implementation to verify the barrier is either needed or sufficient. Unless you mean _everywhere_ :-)
Pretty much everywhere.
Let's take the usual RCU removal pattern as an example:
void f1(struct foo *p) { list_del_rcu(p); synchronize_rcu_expedited(); kfree(p); } void f2(void) { struct foo *p; list_for_each_entry_rcu(p, &my_head, next) do_something_with(p); }Quoted 8 lines
So the
synchronize_rcu_expedited()acts as an extremely heavyweight memory barrier that pairs with thercu_dereference()inside oflist_for_each_entry_rcu(). Easy enough, right?But what exactly within
synchronize_rcu_expedited()provides the ordering? The answer is a web of lock-based critical sections and explicit memory barriers, with the one you called out as needing a comment being one of them.Right, but seeing there's possible implementations of sync_rcu(_exp)*() that do not have the whole
rcu_nodetree like thing, there's more to this particular barrier than the semantics ofsync_rcu().
Quoted 6 lines
Some implementation choice requires this barrier upgrade -- and in another email I suggest its the whole tree thing, we need to firmly establish the state of one level before propagating the state up etc.
Now I'm not entirely sure this is fully correct, but its the best I could come up.
It is pretty close. Ignoring dyntick idle for the moment, things go (very) roughly like this:
o The RCU grace-period kthread notices that a new grace period
is needed. It initializes the tree, which includes acquiring every rcu_node structure's ->lock.
o CPU A notices that there is a new grace period. It acquires
the ->lock of its leaf rcu_node structure, which forces full ordering against the grace-period kthread.
o Some time later, that CPU A realizes that it has passed
through a quiescent state, and again acquires its leaf rcu_node structure's ->lock, again enforcing full ordering, but this time against all CPUs corresponding to this same leaf rcu_node structure that previously noticed quiescent states for this same grace period. Also against all prior readers on this same CPU.
o Some time later, CPU B (corresponding to that same leaf
rcu_node structure) is the last of that leaf's group of CPUs to notice a quiescent state. It has also acquired that leaf's ->lock, again forcing ordering against its prior RCU read-side critical sections, but also against all the prior RCU read-side critical sections of all other CPUs corresponding to this same leaf.
o CPU B therefore moves up the tree, acquiring the parent
rcu_node structures' ->lock. In so doing, it forces full ordering against all prior RCU read-side critical sections of all CPUs corresponding to all leaf rcu_node structures subordinate to the current (non-leaf) rcu_node structure.
o And so on, up the tree.
o When CPU C reaches the root of the tree, and realizes that
it is the last CPU to report a quiescent state for the current grace period, its acquisition of the root rcu_node structure's ->lock has forced full ordering against all RCU read-side critical sections that started before this grace period -- on all CPUs.
CPU C therefore awakens the grace-period kthread.
o When the grace-period kthread wakes up, it does cleanup,
which (you guessed it!) requires acquiring the ->lock of each rcu_node structure. This not only forces full ordering against each pre-existing RCU read-side critical section, it also sets up things so that...
o When CPU D notices that the grace period ended, it does so
while holding its leaf rcu_node structure's ->lock. This forces full ordering against all relevant RCU read-side critical sections. This ordering prevails when CPU D later starts invoking RCU callbacks.
o Just for fun, suppose that one of those callbacks does an
"smp_store_release(&leak_gp, 1)". Suppose further that some CPU E that is not yet aware that the grace period is finished does an "r1 = smp_load_acquire(&lead_gp)" and gets 1. Even if CPU E was the very first CPU to report a quiescent state for the grace period, and even if CPU E has not executed any sort of ordering operations since, CPU E's subsequent code is -still- guaranteed to be fully ordered after each and every RCU read-side critical section that started before the grace period.
Hey, you asked!!! ;-)
Again, this is a cartoon-like view of the ordering that leaves out a lot of details, but it should get across the gist of the ordering.
Thanx, Paul
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
Quoted 10 lines · Paul E. McKenney
Quoted 6 lines
Some implementation choice requires this barrier upgrade -- and in another email I suggest its the whole tree thing, we need to firmly establish the state of one level before propagating the state up etc.
Now I'm not entirely sure this is fully correct, but its the best I could come up.
It is pretty close. Ignoring dyntick idle for the moment, things go (very) roughly like this:
Quoted 7 lines
o The RCU grace-period kthread notices that a new grace period
is needed. It initializes the tree, which includes acquiring every rcu_node structure's ->lock.o CPU A notices that there is a new grace period. It acquires
the ->lock of its leaf rcu_node structure, which forces full ordering against the grace-period kthread.
If the kthread took _all_ rcu_node locks, then this does not require the barrier upgrade because they will share a lock variable.
Quoted 7 lines
o Some time later, that CPU A realizes that it has passed
through a quiescent state, and again acquires its leafrcu_nodestructure's ->lock, again enforcing full ordering, but this time against all CPUs corresponding to this same leafrcu_nodestructure that previously noticed quiescent states for this same grace period. Also against all prior readers on this same CPU.
This again reads like the same lock variable is involved, and therefore the barrier upgrade is not required for this.
Quoted 7 lines
o Some time later, CPU B (corresponding to that same leaf
rcu_nodestructure) is the last of that leaf's group of CPUs to notice a quiescent state. It has also acquired that leaf's ->lock, again forcing ordering against its prior RCU read-side critical sections, but also against all the prior RCU read-side critical sections of all other CPUs corresponding to this same leaf.
same lock var again..
Quoted 5 lines
o CPU B therefore moves up the tree, acquiring the parent
rcu_nodestructures' ->lock. In so doing, it forces full ordering against all prior RCU read-side critical sections of all CPUs corresponding to all leafrcu_nodestructures subordinate to the current (non-leaf)rcu_nodestructure.
And here we iterate the tree and get another lock var involved, here the barrier upgrade will actually do something.
Quoted 1 line
o And so on, up the tree.
idem..
Quoted 6 lines
o When CPU C reaches the root of the tree, and realizes that
it is the last CPU to report a quiescent state for the current grace period, its acquisition of the rootrcu_nodestructure's ->lock has forced full ordering against all RCU read-side critical sections that started before this grace period -- on all CPUs.
Right, which makes the full barrier transitivity thing important
Quoted 7 lines
CPU C therefore awakens the grace-period kthread.
o When the grace-period kthread wakes up, it does cleanup,
which (you guessed it!) requires acquiring the ->lock of eachrcu_nodestructure. This not only forces full ordering against each pre-existing RCU read-side critical section, it also sets up things so that...
Again, if it takes _all_ rcu_nodes, it also shares a lock variable and hence the upgrade is not required.
Quoted 5 lines
o When CPU D notices that the grace period ended, it does so
while holding its leafrcu_nodestructure's ->lock. This forces full ordering against all relevant RCU read-side critical sections. This ordering prevails when CPU D later starts invoking RCU callbacks.
Does also not seem to require the upgrade..
Quoted 1 line
Hey, you asked!!! ;-)
No, I asked what all the barrier upgrade was for, most of the above does not seem to rely on that at all.
The only place this upgrade matters is the UNLOCK x + LOCK y scenario, as also per the comment above smp_mb__after_unlock_lock().
Any other ordering is not on this but on the other primitives and irrelevant to the barrier upgrade.
Quoted 2 lines
Again, this is a cartoon-like view of the ordering that leaves out a lot of details, but it should get across the gist of the ordering.
So the ordering I'm interested in, is the bit that is provided by the barrier upgrade, and that seems very limited and directly pertains to the tree iteration, ensuring its fully separated and transitive.
So I'll stick to explanation that the barrier upgrade is purely for the tree iteration, to separate and make transitive the tree level state.
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
Quoted 44 lines · Peter Zijlstra
Quoted 10 lines · Paul E. McKenney
Quoted 6 lines
Some implementation choice requires this barrier upgrade -- and in another email I suggest its the whole tree thing, we need to firmly establish the state of one level before propagating the state up etc.
Now I'm not entirely sure this is fully correct, but its the best I could come up.
It is pretty close. Ignoring dyntick idle for the moment, things go (very) roughly like this:
Quoted 7 lines
o The RCU grace-period kthread notices that a new grace period
is needed. It initializes the tree, which includes acquiring every rcu_node structure's ->lock.o CPU A notices that there is a new grace period. It acquires
the ->lock of its leaf rcu_node structure, which forces full ordering against the grace-period kthread.If the kthread took _all_
rcu_nodelocks, then this does not require the barrier upgrade because they will share a lock variable.Quoted 7 lines
o Some time later, that CPU A realizes that it has passed
through a quiescent state, and again acquires its leafrcu_nodestructure's ->lock, again enforcing full ordering, but this time against all CPUs corresponding to this same leafrcu_nodestructure that previously noticed quiescent states for this same grace period. Also against all prior readers on this same CPU.This again reads like the same lock variable is involved, and therefore the barrier upgrade is not required for this.
Quoted 7 lines
o Some time later, CPU B (corresponding to that same leaf
rcu_nodestructure) is the last of that leaf's group of CPUs to notice a quiescent state. It has also acquired that leaf's ->lock, again forcing ordering against its prior RCU read-side critical sections, but also against all the prior RCU read-side critical sections of all other CPUs corresponding to this same leaf.same lock var again..
Quoted 8 lines
Quoted 5 lines
o CPU B therefore moves up the tree, acquiring the parent
rcu_nodestructures' ->lock. In so doing, it forces full ordering against all prior RCU read-side critical sections of all CPUs corresponding to all leafrcu_nodestructures subordinate to the current (non-leaf)rcu_nodestructure.And here we iterate the tree and get another lock var involved, here the barrier upgrade will actually do something.
Yep. And I am way too lazy to sort out exactly which acquisitions really truly need smp_mb__after_unlock_lock() and which don't. Besides, if I tried to sort it out, I would occasionally get it wrong, and this would be a real pain to debug. Therefore, I simply do smp_mb__after_unlock_lock() on all acquisitions of the rcu_node structures' ->lock fields. I can actually validate that! ;-)
Quoted 34 lines
Quoted 1 line
o And so on, up the tree.
idem..
Quoted 6 lines
o When CPU C reaches the root of the tree, and realizes that
it is the last CPU to report a quiescent state for the current grace period, its acquisition of the rootrcu_nodestructure's ->lock has forced full ordering against all RCU read-side critical sections that started before this grace period -- on all CPUs.Right, which makes the full barrier transitivity thing important
Quoted 7 lines
CPU C therefore awakens the grace-period kthread.
o When the grace-period kthread wakes up, it does cleanup,
which (you guessed it!) requires acquiring the ->lock of eachrcu_nodestructure. This not only forces full ordering against each pre-existing RCU read-side critical section, it also sets up things so that...Again, if it takes _all_
rcu_nodes, it also shares a lock variable and hence the upgrade is not required.Quoted 5 lines
o When CPU D notices that the grace period ended, it does so
while holding its leafrcu_nodestructure's ->lock. This forces full ordering against all relevant RCU read-side critical sections. This ordering prevails when CPU D later starts invoking RCU callbacks.Does also not seem to require the upgrade..
Quoted 1 line
Hey, you asked!!! ;-)
Quoted 8 lines
No, I asked what all the barrier upgrade was for, most of the above does not seem to rely on that at all.
The only place this upgrade matters is the UNLOCK x + LOCK y scenario, as also per the comment above
smp_mb__after_unlock_lock().Any other ordering is not on this but on the other primitives and irrelevant to the barrier upgrade.
I am still keeping an smp_mb__after_unlock_lock() after every ->lock. Trying to track which needs it and which does not is asking for subtle bugs.
Quoted 9 lines
Quoted 2 lines
Again, this is a cartoon-like view of the ordering that leaves out a lot of details, but it should get across the gist of the ordering.
So the ordering I'm interested in, is the bit that is provided by the barrier upgrade, and that seems very limited and directly pertains to the tree iteration, ensuring its fully separated and transitive.
So I'll stick to explanation that the barrier upgrade is purely for the tree iteration, to separate and make transitive the tree level state.
Fair enough, but I will be sticking to the simple coding rule that keeps RCU out of trouble!
Thanx, Paul
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
Quoted 9 lines · Paul E. McKenney
Quoted 8 lines
Quoted 5 lines
o CPU B therefore moves up the tree, acquiring the parent
rcu_nodestructures' ->lock. In so doing, it forces full ordering against all prior RCU read-side critical sections of all CPUs corresponding to all leafrcu_nodestructures subordinate to the current (non-leaf)rcu_nodestructure.And here we iterate the tree and get another lock var involved, here the barrier upgrade will actually do something.
Quoted 6 lines
Yep. And I am way too lazy to sort out exactly which acquisitions really truly need
smp_mb__after_unlock_lock()and which don't. Besides, if I tried to sort it out, I would occasionally get it wrong, and this would be a real pain to debug. Therefore, I simply dosmp_mb__after_unlock_lock()on all acquisitions of thercu_nodestructures' ->lock fields. I can actually validate that! ;-)
This is a whole different line of reasoning once again.
The point remains, that the sole purpose of the barrier upgrade is for the tree iteration, having some extra (pointless but harmless) instances does not detract from that.
Quoted 2 lines
Fair enough, but I will be sticking to the simple coding rule that keeps RCU out of trouble!
Note that there are rnp->lock acquires without the extra barrier though, so you seem somewhat inconsistent with your own rule.
See for example:
rcu_dump_cpu_stacks()
print_other_cpu_stall()
print_cpu_stall()
(did not do an exhaustive scan, there might be more)
and yes, that is 'obvious' debug code and not critical to the correct behaviour of the code, but it is a deviation from 'the rule'.
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
Quoted 32 lines · Peter Zijlstra
Quoted 9 lines · Paul E. McKenney
Quoted 8 lines
Quoted 5 lines
o CPU B therefore moves up the tree, acquiring the parent
rcu_nodestructures' ->lock. In so doing, it forces full ordering against all prior RCU read-side critical sections of all CPUs corresponding to all leafrcu_nodestructures subordinate to the current (non-leaf)rcu_nodestructure.And here we iterate the tree and get another lock var involved, here the barrier upgrade will actually do something.
Quoted 6 lines
Yep. And I am way too lazy to sort out exactly which acquisitions really truly need
smp_mb__after_unlock_lock()and which don't. Besides, if I tried to sort it out, I would occasionally get it wrong, and this would be a real pain to debug. Therefore, I simply dosmp_mb__after_unlock_lock()on all acquisitions of thercu_nodestructures' ->lock fields. I can actually validate that! ;-)This is a whole different line of reasoning once again.
The point remains, that the sole purpose of the barrier upgrade is for the tree iteration, having some extra (pointless but harmless) instances does not detract from that.
Quoted 2 lines
Fair enough, but I will be sticking to the simple coding rule that keeps RCU out of trouble!
Note that there are
rnp->lockacquires without the extra barrier though, so you seem somewhat inconsistent with your own rule.See for example:
Quoted 8 lines
rcu_dump_cpu_stacks()
print_other_cpu_stall()
print_cpu_stall()(did not do an exhaustive scan, there might be more)
and yes, that is 'obvious' debug code and not critical to the correct behaviour of the code, but it is a deviation from 'the rule'.
Which I need to fix, thank you.
Thanx, Paul
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
In reply to Peter ZijlstraQuoted 32 lines · Peter Zijlstra
Quoted 9 lines · Paul E. McKenney
Quoted 8 lines
Quoted 5 lines
o CPU B therefore moves up the tree, acquiring the parent
rcu_nodestructures' ->lock. In so doing, it forces full ordering against all prior RCU read-side critical sections of all CPUs corresponding to all leafrcu_nodestructures subordinate to the current (non-leaf)rcu_nodestructure.And here we iterate the tree and get another lock var involved, here the barrier upgrade will actually do something.
Quoted 6 lines
Yep. And I am way too lazy to sort out exactly which acquisitions really truly need
smp_mb__after_unlock_lock()and which don't. Besides, if I tried to sort it out, I would occasionally get it wrong, and this would be a real pain to debug. Therefore, I simply dosmp_mb__after_unlock_lock()on all acquisitions of thercu_nodestructures' ->lock fields. I can actually validate that! ;-)This is a whole different line of reasoning once again.
The point remains, that the sole purpose of the barrier upgrade is for the tree iteration, having some extra (pointless but harmless) instances does not detract from that.
Quoted 2 lines
Fair enough, but I will be sticking to the simple coding rule that keeps RCU out of trouble!
Note that there are
rnp->lockacquires without the extra barrier though, so you seem somewhat inconsistent with your own rule.See for example:
Quoted 8 lines
rcu_dump_cpu_stacks()
print_other_cpu_stall()
print_cpu_stall()(did not do an exhaustive scan, there might be more)
and yes, that is 'obvious' debug code and not critical to the correct behaviour of the code, but it is a deviation from 'the rule'.
How about the following patch on top of yours?
Thanx, Paul
------------------------------------------------------------------------
commit 65764359aaec9513bc6aa94e79069469ec74b53e
Author: Paul E. McKenney <[email protected]> Date: Thu Oct 8 15:36:54 2015 -0700
rcu: Add transitivity to remaining rcu_node ->lock acquisitions
The rule is that all acquisitions of the rcu_node structure's ->lock must provide transitivity: The lock is not acquired that frequently, and sorting out exactly which required it and which did not would be a maintenance nightmare. This commit therefore supplies the needed transitivity to the remaining ->lock acquisitions.
kernel/rcu/tree.c +12 -12
@@ -1214,7 +1214,7 @@ static void rcu_dump_cpu_stacks(struct rcu_state *rsp)struct rcu_node *rnp;rcu_for_each_leaf_node (rsp, rnp) { -raw_spin_lock_irqsave (&rnp->lock, flags); +raw_spin_lock_irqsave_rcu_node (rnp, flags);if (rnp->qsmask !=0 ) {for (cpu =0 ; cpu <= rnp->grphi - rnp->grplo; cpu++)if (rnp->qsmask & (1UL << cpu))
@@ -1237,7 +1237,7 @@ static void print_other_cpu_stall(struct rcu_state *rsp, unsigned long gpnum)/* Only let one CPU complain about others per time interval. */ -raw_spin_lock_irqsave (&rnp->lock, flags); +raw_spin_lock_irqsave_rcu_node (rnp, flags); delta = jiffies -READ_ONCE (rsp->jiffies_stall);if (delta <RCU_STALL_RAT_DELAY || !rcu_gp_in_progress (rsp)) {raw_spin_unlock_irqrestore (&rnp->lock, flags);
@@ -1256,7 +1256,7 @@ static void print_other_cpu_stall(struct rcu_state *rsp, unsigned long gpnum) rsp->name);print_cpu_stall_info_begin ();rcu_for_each_leaf_node (rsp, rnp) { -raw_spin_lock_irqsave (&rnp->lock, flags); +raw_spin_lock_irqsave_rcu_node (rnp, flags); ndetected +=rcu_print_task_stall (rnp);if (rnp->qsmask !=0 ) {for (cpu =0 ; cpu <= rnp->grphi - rnp->grplo; cpu++)
@@ -1327,7 +1327,7 @@ static void print_cpu_stall(struct rcu_state *rsp)rcu_dump_cpu_stacks (rsp); -raw_spin_lock_irqsave (&rnp->lock, flags); +raw_spin_lock_irqsave_rcu_node (rnp, flags);if (ULONG_CMP_GE (jiffies,READ_ONCE (rsp->jiffies_stall)))WRITE_ONCE (rsp->jiffies_stall, jiffies +3 *rcu_jiffies_till_stall_check () +3 );
@@ -2897,7 +2897,7 @@ __rcu_process_callbacks(struct rcu_state *rsp)/* Does this CPU require a not-yet-started grace period? */ local_irq_save (flags);if (cpu_needs_another_gp (rsp, rdp)) { -raw_spin_lock (&rcu_get_root (rsp)->lock);/* irqs disabled. */ +raw_spin_lock_rcu_node (rcu_get_root (rsp));/* irqs disabled. */ needwake =rcu_start_gp (rsp);raw_spin_unlock_irqrestore (&rcu_get_root (rsp)->lock, flags);if (needwake)
@@ -3718,7 +3718,7 @@ retry_ipi: mask_ofl_ipi &= ~mask; }else {/* Failed, raced with offline. */ -raw_spin_lock_irqsave (&rnp->lock, flags); +raw_spin_lock_irqsave_rcu_node (rnp, flags);if (cpu_online (cpu) && (rnp->expmask & mask)) { raw_spin_unlock_irqrestore(&rnp->lock,
@@ -3727,8 +3727,8 @@ retry_ipi:if (cpu_online (cpu) && (rnp->expmask & mask)) goto retry_ipi; -raw_spin_lock_irqsave (&rnp->lock, - flags); +raw_spin_lock_irqsave_rcu_node (rnp, + flags); }if (!(rnp->expmask & mask)) mask_ofl_ipi &= ~mask;
@@ -4110,7 +4110,7 @@ static void rcu_init_new_rnp(struct rcu_node *rnp_leaf) rnp = rnp->parent;if (rnp ==NULL )return ; -raw_spin_lock (&rnp->lock);/* Interrupts already disabled. */ +raw_spin_lock_rcu_node (rnp);/* Interrupts already disabled. */ rnp->qsmaskinit |= mask;raw_spin_unlock (&rnp->lock);/* Interrupts remain disabled. */ }
@@ -4127,7 +4127,7 @@ rcu_boot_init_percpu_data(int cpu, struct rcu_state *rsp)struct rcu_node *rnp =rcu_get_root (rsp);/* Set up local state, ensuring consistent view of global state. */ -raw_spin_lock_irqsave (&rnp->lock, flags); +raw_spin_lock_irqsave_rcu_node (rnp, flags); rdp->grpmask =1UL << (cpu - rdp->mynode->grplo); rdp->dynticks = &per_cpu (rcu_dynticks, cpu);WARN_ON_ONCE (rdp->dynticks->dynticks_nesting !=DYNTICK_TASK_EXIT_IDLE );
@@ -4154,7 +4154,7 @@ rcu_init_percpu_data(int cpu, struct rcu_state *rsp)struct rcu_node *rnp =rcu_get_root (rsp);/* Set up local state, ensuring consistent view of global state. */ -raw_spin_lock_irqsave (&rnp->lock, flags); +raw_spin_lock_irqsave_rcu_node (rnp, flags); rdp->qlen_last_fqs_check =0 ; rdp->n_force_qs_snap = rsp->n_force_qs; rdp->blimit = blimit;
@@ -4301,7 +4301,7 @@ static int __init rcu_spawn_gp_kthread(void) t =kthread_create (rcu_gp_kthread, rsp,"%s" , rsp->name);BUG_ON (IS_ERR (t)); rnp =rcu_get_root (rsp); -raw_spin_lock_irqsave (&rnp->lock, flags); +raw_spin_lock_irqsave_rcu_node (rnp, flags); rsp->gp_kthread = t;if (kthread_prio) { sp.sched_priority = kthread_prio;
kernel/rcu/tree_plugin.h +1 -1
@@ -525,7 +525,7 @@ static void rcu_print_detail_task_stall_rnp(struct rcu_node *rnp)unsigned long flags;struct task_struct *t; -raw_spin_lock_irqsave (&rnp->lock, flags); +raw_spin_lock_irqsave_rcu_node (rnp, flags);if (!rcu_preempt_blocked_readers_cgp (rnp)) {raw_spin_unlock_irqrestore (&rnp->lock, flags);return ;
kernel/rcu/tree_trace.c +1 -1
@@ -319,7 +319,7 @@ static void show_one_rcugp(struct seq_file *m, struct rcu_state *rsp)unsigned long gpmax;struct rcu_node *rnp = &rsp->node[0 ]; -raw_spin_lock_irqsave (&rnp->lock, flags); +raw_spin_lock_irqsave_rcu_node (rnp, flags); completed =READ_ONCE (rsp->completed); gpnum =READ_ONCE (rsp->gpnum);if (completed == gpnum)
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation
Quoted 9 lines · Paul E. McKenney
Quoted 9 lines
Note that there are
rnp->lockacquires without the extra barrier though, so you seem somewhat inconsistent with your own rule.See for example:
rcu_dump_cpu_stacks()
print_other_cpu_stall()
print_cpu_stall()
Quoted 6 lines
Quoted 4 lines
(did not do an exhaustive scan, there might be more)
and yes, that is 'obvious' debug code and not critical to the correct behaviour of the code, but it is a deviation from 'the rule'.
How about the following patch on top of yours?
Works for me, Thanks!
PATCH 3/18 rcu: Consolidate tree setup for synchronize_rcu_expedited()
In reply to Paul E. McKenneyThis commit replaces sync_rcu_preempt_exp_init1(() and sync_rcu_preempt_exp_init2() with sync_exp_reset_tree_hotplug() and sync_exp_reset_tree(), which will also be used by synchronize_sched_expedited(), and sync_rcu_exp_select_nodes(), which contains code specific to synchronize_rcu_expedited().
--- kernel/rcu/tree.c | 86 ++++++++++++++++++++++++++++++++++++++- kernel/rcu/tree.h | 17 +++++--- kernel/rcu/tree_plugin.h | 102 +++++++++-------------------------------------- 3 files changed, 115 insertions(+), 90 deletions(-)
kernel/rcu/tree.c +85 -1
@@ -3380,6 +3380,87 @@ static bool rcu_exp_gp_seq_done(struct rcu_state *rsp, unsigned long s) }/* +* Reset the ->expmaskinit values in the rcu_node tree to reflect any +* recent CPU-online activity. Note that these masks are not cleared +* when CPUs go offline, so they reflect the union of all CPUs that have +* ever been online. This means that this function normally takes its +* no-work-to-do fastpath. +*/ +static void sync_exp_reset_tree_hotplug (struct rcu_state *rsp) +{ +bool done; +unsigned long flags; +unsigned long mask; +unsigned long oldmask; +int ncpus =READ_ONCE (rsp->ncpus); +struct rcu_node *rnp; +struct rcu_node *rnp_up; + +/* If no new CPUs onlined since last time, nothing to do. */ +if (likely (ncpus == rsp->ncpus_snap)) +return ; + rsp->ncpus_snap = ncpus; + +/* +* Each pass through the following loop propagates newly onlined +* CPUs for the current rcu_node structure up the rcu_node tree. +*/ +rcu_for_each_leaf_node (rsp, rnp) { +raw_spin_lock_irqsave (&rnp->lock, flags); +smp_mb__after_unlock_lock (); +if (rnp->expmaskinit == rnp->expmaskinitnext) { +raw_spin_unlock_irqrestore (&rnp->lock, flags); +continue ;/* No new CPUs, nothing to do. */ + } + +/* Update this node's mask, track old value for propagation. */ + oldmask = rnp->expmaskinit; + rnp->expmaskinit = rnp->expmaskinitnext; +raw_spin_unlock_irqrestore (&rnp->lock, flags); + +/* If was already nonzero, nothing to propagate. */ +if (oldmask) +continue ; + +/* Propagate the new CPU up the tree. */ + mask = rnp->grpmask; + rnp_up = rnp->parent; + done = false; +while (rnp_up) { +raw_spin_lock_irqsave (&rnp_up->lock, flags); +smp_mb__after_unlock_lock (); +if (rnp_up->expmaskinit) + done = true; + rnp_up->expmaskinit |= mask; +raw_spin_unlock_irqrestore (&rnp_up->lock, flags); +if (done) +break ; + mask = rnp_up->grpmask; + rnp_up = rnp_up->parent; + } + } +} + +/* +* Reset the ->expmask values in the rcu_node tree in preparation for +* a new expedited grace period. +*/ +static void __maybe_unused sync_exp_reset_tree (struct rcu_state *rsp) +{ +unsigned long flags; +struct rcu_node *rnp; + +sync_exp_reset_tree_hotplug (rsp); +rcu_for_each_node_breadth_first (rsp, rnp) { +raw_spin_lock_irqsave (&rnp->lock, flags); +smp_mb__after_unlock_lock (); +WARN_ON_ONCE (rnp->expmask); + rnp->expmask = rnp->expmaskinit; +raw_spin_unlock_irqrestore (&rnp->lock, flags); + } +} + +/* * Return non-zero if there are any tasks in RCU read-side critical * sections blocking the current preemptible-RCU expedited grace period. * If there is no preemptible-RCU expedited grace period currently in
@@ -3971,7 +4052,6 @@ rcu_init_percpu_data(int cpu, struct rcu_state *rsp)/* Set up local state, ensuring consistent view of global state. */ raw_spin_lock_irqsave (&rnp->lock, flags); - rdp->beenonline =1 ;/* We have now been online. */ rdp->qlen_last_fqs_check =0 ; rdp->n_force_qs_snap = rsp->n_force_qs; rdp->blimit = blimit;
@@ -3993,6 +4073,10 @@ rcu_init_percpu_data(int cpu, struct rcu_state *rsp)raw_spin_lock (&rnp->lock);/* irqs already disabled. */ smp_mb__after_unlock_lock (); rnp->qsmaskinitnext |= mask; + rnp->expmaskinitnext |= mask; +if (!rdp->beenonline) +WRITE_ONCE (rsp->ncpus,READ_ONCE (rsp->ncpus) +1 ); + rdp->beenonline = true;/* We have now been online. */ rdp->gpnum = rnp->completed;/* Make CPU later note any new GP. */ rdp->completed = rnp->completed; rdp->passed_quiesce = false;
kernel/rcu/tree.h +12 -5
@@ -171,16 +171,21 @@ struct rcu_node {/* an rcu_data structure, otherwise, each */ /* bit corresponds to a child rcu_node */ /* structure. */ -unsigned long expmask;/* Groups that have ->blkd_tasks */ -/* elements that need to drain to allow the */ -/* current expedited grace period to */ -/* complete (only for PREEMPT_RCU). */ unsigned long qsmaskinit; -/* Per-GP initial value for qsmask & expmask. */ +/* Per-GP initial value for qsmask. */ /* Initialized from ->qsmaskinitnext at the */ /* beginning of each grace period. */ unsigned long qsmaskinitnext;/* Online CPUs for next grace period. */ +unsigned long expmask;/* CPUs or groups that need to check in */ +/* to allow the current expedited GP */ +/* to complete. */ +unsigned long expmaskinit; +/* Per-GP initial values for expmask. */ +/* Initialized from ->expmaskinitnext at the */ +/* beginning of each expedited GP. */ +unsigned long expmaskinitnext; +/* Online CPUs for next expedited GP. */ unsigned long grpmask;/* Mask to apply to parent qsmask. */ /* Only one bit will be set in this mask. */ int grplo;/* lowest-numbered CPU or group here. */
@@ -466,6 +471,7 @@ struct rcu_state {struct rcu_data __percpu *rda;/* pointer of percu rcu_data. */ void (*call)(struct rcu_head *head,/* call_rcu() flavor. */ void (*func)(struct rcu_head *head)); +int ncpus;/* # CPUs seen so far. */ /* The following fields are guarded by the root rcu_node's lock. */
@@ -508,6 +514,7 @@ struct rcu_state {atomic_long_t expedited_normal;/* # fallbacks to normal. */ atomic_t expedited_need_qs;/* # CPUs left to check in. */ wait_queue_head_t expedited_wq;/* Wait for check-ins. */ +int ncpus_snap;/* # CPUs seen last time. */ unsigned long jiffies_force_qs;/* Time at which to invoke */ /* force_quiescent_state(). */
kernel/rcu/tree_plugin.h +18 -84
120 lines not on this page · raw message
Signature · 1 line
2.5.2
PATCH 4/18 rcu: Use single-stage IPI algorithm for RCU expedited grace period
In reply to Paul E. McKenneyThe current preemptible-RCU expedited grace-period algorithm invokes synchronize_sched_expedited() to enqueue all tasks currently running in a preemptible-RCU read-side critical section, then waits for all the ->blkd_tasks lists to drain. This works, but results in both an IPI and a double context switch even on CPUs that do not happen to be running in a preemptible RCU read-side critical section.
This commit implements a new algorithm that causes less OS jitter. This new algorithm IPIs all online CPUs that are not idle (from an RCU perspective), but refrains from self-IPIs. If a CPU receiving this IPI is not in a preemptible RCU read-side critical section (or is just now exiting one), it pushes quiescence up the rcu_node tree, otherwise, it sets a flag that will be handled by the upcoming outermost rcu_read_unlock(), which will then push quiescence up the tree.
The expedited grace period must of course wait on any pre-existing blocked readers, and newly blocked readers must be queued carefully based on the state of both the normal and the expedited grace periods. This new queueing approach also avoids the need to update boost state, courtesy of the fact that blocked tasks are no longer ever migrated to the root rcu_node structure.
--- include/linux/sched.h | 10 +- kernel/rcu/tree.c | 75 ++++++++---- kernel/rcu/tree_plugin.h | 296 +++++++++++++++++++++++++++++++++++++++-------- 3 files changed, 311 insertions(+), 70 deletions(-)
include/linux/sched.h +6 -4
@@ -1330,10 +1330,12 @@ struct sched_dl_entity {union rcu_special {struct { -bool blocked; -bool need_qs; - } b; -short s; +u8 blocked; +u8 need_qs; +u8 exp_need_qs; +u8 pad;/* Otherwise the compiler can store garbage here. */ + } b;/* Bits. */ +u32 s;/* Set of bits. */ };struct rcu_node ;
kernel/rcu/tree.c +55 -20
109 lines not on this page · raw message
kernel/rcu/tree_plugin.h +250 -46
384 lines not on this page · raw message
Signature · 1 line
2.5.2
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period
Quoted 9 lines · Paul E. McKenney
kernel/rcu/tree.c +2
-2@@ -3494,19 +3483,21 @@ static int sync_rcu_preempt_exp_done(struct rcu_node *rnp)* recursively up the tree. (Calm down, calm down, we do the recursion * iteratively!) * -* Caller must hold the root rcu_node's exp_funnel_mutex. +* Caller must hold the root rcu_node's exp_funnel_mutex and the +* specified rcu_node structure's ->lock. */ -static void __maybe_unused rcu_report_exp_rnp(struct rcu_state *rsp,
kernel/rcu/tree.c static int sync_rcu_preempt_exp_done(struct rcu_node *rnp) · line 3506
Quoted 10 lines
kernel/rcu/tree.c +3
-4- struct rcu_node *rnp, bool wake) +static void __rcu_report_exp_rnp (struct rcu_state *rsp,struct rcu_node *rnp, +bool wake,unsigned long flags) + __releases(rnp->lock) { -unsigned long flags;unsigned long mask; -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock ();
lockdep_assert_held(&rnp->lock);
kernel/rcu/tree.c static void __maybe_unused rcu_report_exp_rnp(struct rcu_state *rsp, · line 3530
Quoted 10 lines
kernel/rcu/tree.c +10
-0+/* +* Report expedited quiescent state for specified node. This is a +* lock-acquisition wrapper function for __rcu_report_exp_rnp(). +* +* Caller must hold the root rcu_node's exp_funnel_mutex. +*/ +static void __maybe_unused rcu_report_exp_rnp (struct rcu_state *rsp, +struct rcu_node *rnp,bool wake) +{ +unsigned long flags;
lockdep_assert_held(&rcu_get_root(rsp)->exp_funnel_mutex);
kernel/rcu/tree.c static void __maybe_unused rcu_report_exp_rnp(struct rcu_state *rsp, · line 3535
Quoted 5 lines
kernel/rcu/tree.c +5
-0+ +raw_spin_lock_irqsave (&rnp->lock, flags); +smp_mb__after_unlock_lock (); +__rcu_report_exp_rnp (rsp, rnp, wake, flags); +}
Etc.. these are much harder to ignore than comments.
Re: [PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period
Quoted 36 lines · Peter Zijlstra
Quoted 9 lines · Paul E. McKenney
kernel/rcu/tree.c +2
-2@@ -3494,19 +3483,21 @@ static int sync_rcu_preempt_exp_done(struct rcu_node *rnp)* recursively up the tree. (Calm down, calm down, we do the recursion * iteratively!) * -* Caller must hold the root rcu_node's exp_funnel_mutex. +* Caller must hold the root rcu_node's exp_funnel_mutex and the +* specified rcu_node structure's ->lock. */ -static void __maybe_unused rcu_report_exp_rnp(struct rcu_state *rsp,kernel/rcu/tree.c static int sync_rcu_preempt_exp_done(struct rcu_node *rnp) · line 3506
Quoted 10 lines
kernel/rcu/tree.c +3
-4- struct rcu_node *rnp, bool wake) +static void __rcu_report_exp_rnp (struct rcu_state *rsp,struct rcu_node *rnp, +bool wake,unsigned long flags) + __releases(rnp->lock) { -unsigned long flags;unsigned long mask; -raw_spin_lock_irqsave (&rnp->lock, flags); -smp_mb__after_unlock_lock ();lockdep_assert_held(&rnp->lock);kernel/rcu/tree.c static void __maybe_unused rcu_report_exp_rnp(struct rcu_state *rsp, · line 3530
Quoted 10 lines
kernel/rcu/tree.c +10
-0+/* +* Report expedited quiescent state for specified node. This is a +* lock-acquisition wrapper function for __rcu_report_exp_rnp(). +* +* Caller must hold the root rcu_node's exp_funnel_mutex. +*/ +static void __maybe_unused rcu_report_exp_rnp (struct rcu_state *rsp, +struct rcu_node *rnp,bool wake) +{ +unsigned long flags;lockdep_assert_held(&rcu_get_root(rsp)->exp_funnel_mutex);
Quoted 8 lines
kernel/rcu/tree.c static void __maybe_unused rcu_report_exp_rnp(struct rcu_state *rsp, · line 3535
Quoted 5 lines
kernel/rcu/tree.c +5
-0+ +raw_spin_lock_irqsave (&rnp->lock, flags); +smp_mb__after_unlock_lock (); +__rcu_report_exp_rnp (rsp, rnp, wake, flags); +}Etc.. these are much harder to ignore than comments.
Good point! I probably should do the same for interrupt disabling.
Thanx, Paul
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period
In reply to Paul E. McKenneykernel/rcu/tree_plugin.h static void __init rcu_bootup_announce(void) · line 120
Quoted 5 lines · Paul E. McKenney
kernel/rcu/tree_plugin.h +5
-0+/* Flags for rcu_preempt_ctxt_queue() decision table. */ +#define RCU_GP_TASKS 0x8 +#define RCU_EXP_TASKS 0x4 +#define RCU_GP_BLKD 0x2 +#define RCU_EXP_BLKD 0x1
Purely cosmetic, but that's backwards ;-) Most of our flags etc.. are in increasing order.
kernel/rcu/tree_plugin.h static void __init rcu_bootup_announce(void) · line 156
Quoted 7 lines
kernel/rcu/tree_plugin.h +7
-0+static void rcu_preempt_ctxt_queue (struct rcu_node *rnp,struct rcu_data *rdp, + unsigned long flags) __releases(rnp->lock) +{ +int blkd_state = (rnp->gp_tasks ?RCU_GP_TASKS :0 ) + + (rnp->exp_tasks ?RCU_EXP_TASKS :0 ) + + (rnp->qsmask & rdp->grpmask ?RCU_GP_BLKD :0 ) + + (rnp->expmask & rdp->grpmask ?RCU_EXP_BLKD :0 );
An alternative way is:
int blkd_state = RCU_GP_TASKS * !!rnp->gp_tasks + RCU_EXP_TASKS * !!rnp->exp_tasks + RCU_GP_BLKD * !!(rnp->qsmask & rdp->grpmask) + RCU_EXP_BLKD * !!(rnp->expmask & rdp->grpmask);
I suppose it depends on how your brain is wired which version reads easier :-)
Re: [PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period
Quoted 21 lines · Peter Zijlstra
kernel/rcu/tree_plugin.h static void __init rcu_bootup_announce(void) · line 120
Quoted 5 lines · Paul E. McKenney
kernel/rcu/tree_plugin.h +5
-0+/* Flags for rcu_preempt_ctxt_queue() decision table. */ +#define RCU_GP_TASKS 0x8 +#define RCU_EXP_TASKS 0x4 +#define RCU_GP_BLKD 0x2 +#define RCU_EXP_BLKD 0x1Purely cosmetic, but that's backwards ;-) Most of our flags etc.. are in increasing order.
kernel/rcu/tree_plugin.h static void __init rcu_bootup_announce(void) · line 156
Quoted 7 lines
kernel/rcu/tree_plugin.h +7
-0+static void rcu_preempt_ctxt_queue (struct rcu_node *rnp,struct rcu_data *rdp, + unsigned long flags) __releases(rnp->lock) +{ +int blkd_state = (rnp->gp_tasks ?RCU_GP_TASKS :0 ) + + (rnp->exp_tasks ?RCU_EXP_TASKS :0 ) + + (rnp->qsmask & rdp->grpmask ?RCU_GP_BLKD :0 ) + + (rnp->expmask & rdp->grpmask ?RCU_EXP_BLKD :0 );An alternative way is:
Quoted 7 lines
int blkd_state = RCU_GP_TASKS * !!rnp->gp_tasks + RCU_EXP_TASKS * !!rnp->exp_tasks + RCU_GP_BLKD * !!(rnp->qsmask & rdp->grpmask) + RCU_EXP_BLKD * !!(rnp->expmask & rdp->grpmask);I suppose it depends on how your brain is wired which version reads easier :-)
Indeed! ;-)
I will stick with the ?: for the moment, but your multiplied bang-up approach might well grow on me.
Thanx, Paul
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period
In reply to Paul E. McKenneyReview static void rcu_preempt_note_context_switch(void) · line 168
Quoted 3 lines · Paul E. McKenney
(quoted diff) +0
-1@@ -167,42 +307,18 @@ static void rcu_preempt_note_context_switch(void) - raw_spin_unlock_irqrestore(&rnp->lock, flags);
This again reminds me that we should move rcu_note_context_switch() under the IRQ disable section the scheduler already has or remove the IRQ disable from rcu_note_context_switch().
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period
Quoted 8 lines · Peter Zijlstra
Review static void rcu_preempt_note_context_switch(void) · line 168
Quoted 3 lines · Paul E. McKenney
(quoted diff) +0
-1@@ -167,42 +307,18 @@ static void rcu_preempt_note_context_switch(void) - raw_spin_unlock_irqrestore(&rnp->lock, flags);This again reminds me that we should move
rcu_note_context_switch()under the IRQ disable section the scheduler already has or remove the IRQ disable fromrcu_note_context_switch().
Ah, this is the unlikely path where we actually need to do work. The normal fast paths no longer require IRQs disabled, you already fixed that last time, good!
Re: [PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period
Quoted 10 lines · Peter Zijlstra
Quoted 8 lines · Peter Zijlstra
Review static void rcu_preempt_note_context_switch(void) · line 168
Quoted 3 lines · Paul E. McKenney
(quoted diff) +0
-1@@ -167,42 +307,18 @@ static void rcu_preempt_note_context_switch(void) - raw_spin_unlock_irqrestore(&rnp->lock, flags);This again reminds me that we should move
rcu_note_context_switch()under the IRQ disable section the scheduler already has or remove the IRQ disable fromrcu_note_context_switch().
Quoted 3 lines
Ah, this is the unlikely path where we actually need to do work. The normal fast paths no longer require IRQs disabled, you already fixed that last time, good!
But why not make the less-likely paths a bit cheaper while we are at it? ;-)
Thanx, Paul
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period
Quoted 11 lines · Paul E. McKenney
Quoted 10 lines · Peter Zijlstra
Quoted 8 lines · Peter Zijlstra
Review static void rcu_preempt_note_context_switch(void) · line 168
Quoted 3 lines · Paul E. McKenney
(quoted diff) +0
-1@@ -167,42 +307,18 @@ static void rcu_preempt_note_context_switch(void) - raw_spin_unlock_irqrestore(&rnp->lock, flags);This again reminds me that we should move
rcu_note_context_switch()under the IRQ disable section the scheduler already has or remove the IRQ disable fromrcu_note_context_switch().
Quoted 5 lines
Quoted 3 lines
Ah, this is the unlikely path where we actually need to do work. The normal fast paths no longer require IRQs disabled, you already fixed that last time, good!
But why not make the less-likely paths a bit cheaper while we are at it? ;-)
True, less critical, but if we're almost there we might as well fix that up too.
Re: [PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period
In reply to Peter ZijlstraQuoted 8 lines · Peter Zijlstra
Review static void rcu_preempt_note_context_switch(void) · line 168
Quoted 3 lines · Paul E. McKenney
(quoted diff) +0
-1@@ -167,42 +307,18 @@ static void rcu_preempt_note_context_switch(void) - raw_spin_unlock_irqrestore(&rnp->lock, flags);This again reminds me that we should move
rcu_note_context_switch()under the IRQ disable section the scheduler already has or remove the IRQ disable fromrcu_note_context_switch().
Like this? I verified that all callers of rcu_virt_note_context_switch() currently have interrupts disabled.
Thanx, Paul
------------------------------------------------------------------------
commit 945c702687f760872adf7ce6e030c81ba427bf34
Author: Paul E. McKenney <[email protected]> Date: Wed Oct 7 09:10:48 2015 -0700
rcu: Stop disabling interrupts in scheduler fastpaths
We need the scheduler's fastpaths to be, well, fast, and unnecessarily disabling and re-enabling interrupts is not necessarily consistent with this goal. Especially given that there are regions of the scheduler that already have interrupts disabled.
This commit therefore moves the call to rcu_note_context_switch() to one of the interrupts-disabled regions of the scheduler, and removes the now-redundant disabling and re-enabling of interrupts from rcu_note_context_switch() and the functions it calls.
include/linux/rcutree.h +1 -1
@@ -37,7 +37,7 @@ void rcu_cpu_stall_reset(void);/* * Note a virtualization-based context switch. This is simply a * wrapper around rcu_note_context_switch(), which allows TINY_RCU -* to save a few bytes. +* to save a few bytes. The caller must have disabled interrupts. */ static inline void rcu_virt_note_context_switch (int cpu) {
kernel/rcu/tree.c +9 -6
@@ -295,17 +295,16 @@ EXPORT_PER_CPU_SYMBOL_GPL(rcu_qs_ctr);* We inform the RCU core by emulating a zero-duration dyntick-idle * period, which we in turn do by incrementing the ->dynticks counter * by two. +* +* The caller must have disabled interrupts. */ static void rcu_momentary_dyntick_idle (void ) { -unsigned long flags;struct rcu_data *rdp;struct rcu_dynticks *rdtp;int resched_mask;struct rcu_state *rsp; -local_irq_save (flags); -/* * Yes, we can lose flag-setting operations. This is OK, because * the flag will be set again after some delay.
@@ -335,13 +334,12 @@ static void rcu_momentary_dyntick_idle(void)smp_mb__after_atomic ();/* Later stuff after QS. */ break ; } -local_irq_restore (flags); }/* * Note a context switch. This is a quiescent state for RCU-sched, * and requires special handling for preemptible RCU. -* The caller must have disabled preemption. +* The caller must have disabled interrupts. */ void rcu_note_context_switch (void ) {
@@ -371,9 +369,14 @@ EXPORT_SYMBOL_GPL(rcu_note_context_switch);*/ void rcu_all_qs (void ) { +unsigned long flags; +barrier ();/* Avoid RCU read-side critical sections leaking down. */ -if (unlikely (raw_cpu_read (rcu_sched_qs_mask))) +if (unlikely (raw_cpu_read (rcu_sched_qs_mask))) { +local_irq_save (flags);rcu_momentary_dyntick_idle (); +local_irq_restore (flags); + }this_cpu_inc (rcu_qs_ctr);barrier ();/* Avoid RCU read-side critical sections leaking up. */ }
kernel/rcu/tree_plugin.h +6 -8
@@ -146,8 +146,8 @@ static void __init rcu_bootup_announce(void)* the corresponding expedited grace period will also be the end of the * normal grace period. */ -static void rcu_preempt_ctxt_queue (struct rcu_node *rnp,struct rcu_data *rdp, - unsigned long flags) __releases(rnp->lock) +static void rcu_preempt_ctxt_queue (struct rcu_node *rnp,struct rcu_data *rdp) + __releases(rnp->lock) /* But leaves rrupts disabled. */ {int blkd_state = (rnp->gp_tasks ?RCU_GP_TASKS :0 ) + (rnp->exp_tasks ? RCU_EXP_TASKS : 0) +
@@ -235,7 +235,7 @@ static void rcu_preempt_ctxt_queue(struct rcu_node *rnp, struct rcu_data *rdp, rnp->gp_tasks = &t->rcu_node_entry;if (!rnp->exp_tasks && (blkd_state &RCU_EXP_BLKD )) rnp->exp_tasks = &t->rcu_node_entry; -raw_spin_unlock (&rnp->lock); +raw_spin_unlock (&rnp->lock);/* rrupts remain disabled. */ /* * Report the quiescent state for the expedited GP. This expedited
@@ -250,7 +250,6 @@ static void rcu_preempt_ctxt_queue(struct rcu_node *rnp, struct rcu_data *rdp, }else {WARN_ON_ONCE (t->rcu_read_unlock_special.b.exp_need_qs); } -local_irq_restore (flags); }/*
@@ -285,12 +284,11 @@ static void rcu_preempt_qs(void)* predating the current grace period drain, in other words, until * rnp->gp_tasks becomes NULL. * -* Caller must disable preemption. +* Caller must disable interrupts. */ static void rcu_preempt_note_context_switch (void ) {struct task_struct *t = current; -unsigned long flags;struct rcu_data *rdp;struct rcu_node *rnp;
@@ -300,7 +298,7 @@ static void rcu_preempt_note_context_switch(void)/* Possibly blocking in an RCU read-side critical section. */ rdp =this_cpu_ptr (rcu_state_p->rda); rnp = rdp->mynode; -raw_spin_lock_irqsave (&rnp->lock, flags); +raw_spin_lock (&rnp->lock);/* rrupts already disabled. */ smp_mb__after_unlock_lock (); t->rcu_read_unlock_special.b.blocked = true; t->rcu_blocked_node = rnp;
@@ -317,7 +315,7 @@ static void rcu_preempt_note_context_switch(void) (rnp->qsmask & rdp->grpmask) ? rnp->gpnum : rnp->gpnum + 1); -rcu_preempt_ctxt_queue (rnp, rdp, flags); +rcu_preempt_ctxt_queue (rnp, rdp); }else if (t->rcu_read_lock_nesting <0 && t->rcu_read_unlock_special.s) {
kernel/sched/core.c +1 -1
@@ -3056,7 +3056,6 @@ static void __sched __schedule(void) cpu =smp_processor_id (); rq =cpu_rq (cpu); -rcu_note_context_switch (); prev = rq->curr;schedule_debug (prev);
@@ -3072,6 +3071,7 @@ static void __sched __schedule(void)smp_mb__before_spinlock ();raw_spin_lock_irq (&rq->lock);lockdep_pin_lock (&rq->lock); +rcu_note_context_switch (); rq->clock_skip_update <<=1 ;/* promote REQ to ACT */
PATCH 5/18 rcu: Move synchronize_sched_expedited() to combining tree
In reply to Paul E. McKenneyCurrently, synchronize_sched_expedited() uses a single global counter to track the number of remaining context switches that the current expedited grace period must wait on. This is problematic on large systems, where the resulting memory contention can be pathological. This commit therefore makes synchronize_sched_expedited() instead use the combining tree in the same manner as synchronize_rcu_expedited(), keeping memory contention down to a dull roar.
This commit creates a temporary function sync_sched_exp_select_cpus() that is very similar to sync_rcu_exp_select_cpus(). A later commit will consolidate these two functions, which becomes possible when synchronize_sched_expedited() switches from stop_one_cpu_nowait() to smp_call_function_single().
--- kernel/rcu/tree.c | 123 ++++++++++++++++++++++++++++++++++++------------------ kernel/rcu/tree.h | 1 - 2 files changed, 82 insertions(+), 42 deletions(-)
kernel/rcu/tree.c +82 -41
@@ -3642,19 +3642,77 @@ static int synchronize_sched_expedited_cpu_stop(void *data)struct rcu_data *rdp = data;struct rcu_state *rsp = rdp->rsp; -/* We are here: If we are last, do the wakeup. */ - rdp->exp_done = true; -if (atomic_dec_and_test (&rsp->expedited_need_qs)) -wake_up (&rsp->expedited_wq); +/* Report the quiescent state. */ +rcu_report_exp_rdp (rsp, rdp, true);return 0 ; } +/* +* Select the nodes that the upcoming expedited grace period needs +* to wait for. +*/ +static void sync_sched_exp_select_cpus (struct rcu_state *rsp) +{ +int cpu; +unsigned long flags; +unsigned long mask; +unsigned long mask_ofl_test; +unsigned long mask_ofl_ipi; +struct rcu_data *rdp; +struct rcu_node *rnp; + +sync_exp_reset_tree (rsp); +rcu_for_each_leaf_node (rsp, rnp) { +raw_spin_lock_irqsave (&rnp->lock, flags); +smp_mb__after_unlock_lock (); + +/* Each pass checks a CPU for identity, offline, and idle. */ + mask_ofl_test =0 ; +for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++) { +struct rcu_data *rdp =per_cpu_ptr (rsp->rda, cpu); +struct rcu_dynticks *rdtp = &per_cpu (rcu_dynticks, cpu); + +if (raw_smp_processor_id () == cpu || +cpu_is_offline (cpu) || + !(atomic_add_return (0 , &rdtp->dynticks) &0x1 )) + mask_ofl_test |= rdp->grpmask; + } + mask_ofl_ipi = rnp->expmask & ~mask_ofl_test; + +/* +* Need to wait for any blocked tasks as well. Note that +* additional blocking tasks will also block the expedited +* GP until such time as the ->expmask bits are cleared. +*/ +if (rcu_preempt_has_tasks (rnp)) + rnp->exp_tasks = rnp->blkd_tasks.next; +raw_spin_unlock_irqrestore (&rnp->lock, flags); + +/* IPI the remaining CPUs for expedited quiescent state. */ + mask =1 ; +for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<=1 ) { +if (!(mask_ofl_ipi & mask)) +continue ; + rdp =per_cpu_ptr (rsp->rda, cpu); +stop_one_cpu_nowait (cpu, synchronize_sched_expedited_cpu_stop, + rdp, &rdp->exp_stop_work); + mask_ofl_ipi &= ~mask; + } +/* Report quiescent states for those that went offline. */ + mask_ofl_test |= mask_ofl_ipi; +if (mask_ofl_test) +rcu_report_exp_cpu_mult (rsp, rnp, mask_ofl_test, false); + } +} +static void synchronize_sched_expedited_wait (struct rcu_state *rsp) {int cpu;unsigned long jiffies_stall;unsigned long jiffies_start; -struct rcu_data *rdp; +unsigned long mask; +struct rcu_node *rnp; +struct rcu_node *rnp_root =rcu_get_root (rsp);int ret; jiffies_stall =rcu_jiffies_till_stall_check ();
@@ -3663,33 +3721,36 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)for (;;) { ret =wait_event_interruptible_timeout ( rsp->expedited_wq, - !atomic_read (&rsp->expedited_need_qs), +sync_rcu_preempt_exp_done (rnp_root), jiffies_stall);if (ret >0 )return ;if (ret <0 ) {/* Hit a signal, disable CPU stall warnings. */ wait_event (rsp->expedited_wq, - !atomic_read (&rsp->expedited_need_qs)); +sync_rcu_preempt_exp_done (rnp_root));return ; }pr_err ("INFO: %s detected expedited stalls on CPUs: {" , rsp->name); -for_each_online_cpu (cpu) { - rdp =per_cpu_ptr (rsp->rda, cpu); - -if (rdp->exp_done) -continue ; -pr_cont (" %d" , cpu); +rcu_for_each_leaf_node (rsp, rnp) { + mask =1 ; +for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<=1 ) { +if (!(rnp->expmask & mask)) +continue ; +pr_cont (" %d" , cpu); + } + mask <<=1 ; }pr_cont (" } %lu jiffies s: %lu\n" , jiffies - jiffies_start, rsp->expedited_sequence); -for_each_online_cpu (cpu) { - rdp =per_cpu_ptr (rsp->rda, cpu); - -if (rdp->exp_done) -continue ; -dump_cpu_task (cpu); +rcu_for_each_leaf_node (rsp, rnp) { + mask =1 ; +for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<=1 ) { +if (!(rnp->expmask & mask)) +continue ; +dump_cpu_task (cpu); + } } jiffies_stall =3 *rcu_jiffies_till_stall_check () +3 ; }
@@ -3713,7 +3774,6 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)*/ void synchronize_sched_expedited (void ) { -int cpu;unsigned long s;struct rcu_node *rnp;struct rcu_state *rsp = &rcu_sched_state;
@@ -3736,27 +3796,8 @@ void synchronize_sched_expedited(void) }rcu_exp_gp_seq_start (rsp); - -/* Stop each CPU that is online, non-idle, and not us. */ -atomic_set (&rsp->expedited_need_qs,1 );/* Extra count avoids race. */ -for_each_online_cpu (cpu) { -struct rcu_data *rdp =per_cpu_ptr (rsp->rda, cpu); -struct rcu_dynticks *rdtp = &per_cpu (rcu_dynticks, cpu); - - rdp->exp_done = false; - -/* Skip our CPU and any idle CPUs. */ -if (raw_smp_processor_id () == cpu || - !(atomic_add_return (0 , &rdtp->dynticks) &0x1 )) -continue ; -atomic_inc (&rsp->expedited_need_qs); -stop_one_cpu_nowait (cpu, synchronize_sched_expedited_cpu_stop, - rdp, &rdp->exp_stop_work); - } - -/* Remove extra count and, if necessary, wait for CPUs to stop. */ -if (!atomic_dec_and_test (&rsp->expedited_need_qs)) -synchronize_sched_expedited_wait (rsp); +sync_sched_exp_select_cpus (rsp); +synchronize_sched_expedited_wait (rsp);rcu_exp_gp_seq_end (rsp);mutex_unlock (&rnp->exp_funnel_mutex);
kernel/rcu/tree.h +0 -1
@@ -383,7 +383,6 @@ struct rcu_data {struct rcu_head oom_head;#endif /* #ifdef CONFIG_RCU_FAST_NO_HZ */ struct mutex exp_funnel_mutex; -bool exp_done;/* Expedited QS for this CPU? */ /* 7) Callback offloading. */ #ifdef CONFIG_RCU_NOCB_CPU
Signature · 1 line
2.5.2
PATCH 6/18 rcu: Rename qs_pending to core_needs_qs
In reply to Paul E. McKenneyAn upcoming commit needs to invert the sense of the ->passed_quiesce rcu_data structure field, so this commit is taking this opportunity to clarify things a bit by renaming ->qs_pending to ->core_needs_qs.
So if !rdp->core_needs_qs, then this CPU need not concern itself with quiescent states, in particular, it need not acquire its leaf rcu_node structure's ->lock to check. Otherwise, it needs to report the next quiescent state.
--- kernel/rcu/tree.c | 14 +++++++------- kernel/rcu/tree.h | 4 ++-- kernel/rcu/tree_plugin.h | 2 +- kernel/rcu/tree_trace.c | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-)
kernel/rcu/tree.c +7 -7
@@ -1746,7 +1746,7 @@ static bool __note_gp_changes(struct rcu_state *rsp, struct rcu_node *rnp,trace_rcu_grace_period (rsp->name, rdp->gpnum,TPS ("cpustart" )); rdp->passed_quiesce =0 ; rdp->rcu_qs_ctr_snap =__this_cpu_read (rcu_qs_ctr); - rdp->qs_pending = !!(rnp->qsmask & rdp->grpmask); + rdp->core_needs_qs = !!(rnp->qsmask & rdp->grpmask);zero_cpu_stall_ticks (rdp);WRITE_ONCE (rdp->gpwrap, false); }
@@ -2357,7 +2357,7 @@ rcu_report_qs_rdp(int cpu, struct rcu_state *rsp, struct rcu_data *rdp)if ((rnp->qsmask & mask) ==0 ) {raw_spin_unlock_irqrestore (&rnp->lock, flags); }else { - rdp->qs_pending =0 ; + rdp->core_needs_qs =0 ;/* * This GP can't end until cpu checks in, so all of our
@@ -2388,7 +2388,7 @@ rcu_check_quiescent_state(struct rcu_state *rsp, struct rcu_data *rdp)* Does this CPU still need to do its part for current grace period? * If no, return and let the other CPUs do their part as well. */ -if (!rdp->qs_pending) +if (!rdp->core_needs_qs)return ;/*
@@ -3828,10 +3828,10 @@ static int __rcu_pending(struct rcu_state *rsp, struct rcu_data *rdp)/* Is the RCU core waiting for a quiescent state from this CPU? */ if (rcu_scheduler_fully_active && - rdp->qs_pending && !rdp->passed_quiesce && + rdp->core_needs_qs && !rdp->passed_quiesce && rdp->rcu_qs_ctr_snap ==__this_cpu_read (rcu_qs_ctr)) { - rdp->n_rp_qs_pending++; - }else if (rdp->qs_pending && + rdp->n_rp_core_needs_qs++; + }else if (rdp->core_needs_qs && (rdp->passed_quiesce || rdp->rcu_qs_ctr_snap !=__this_cpu_read (rcu_qs_ctr))) { rdp->n_rp_report_qs++;
@@ -4157,7 +4157,7 @@ rcu_init_percpu_data(int cpu, struct rcu_state *rsp) rdp->completed = rnp->completed; rdp->passed_quiesce = false; rdp->rcu_qs_ctr_snap =per_cpu (rcu_qs_ctr, cpu); - rdp->qs_pending = false; + rdp->core_needs_qs = false;trace_rcu_grace_period (rsp->name, rdp->gpnum,TPS ("cpuonl" ));raw_spin_unlock_irqrestore (&rnp->lock, flags); }
kernel/rcu/tree.h +2 -2
@@ -303,7 +303,7 @@ struct rcu_data {unsigned long rcu_qs_ctr_snap;/* Snapshot of rcu_qs_ctr to check */ /* for rcu_all_qs() invocations. */ bool passed_quiesce;/* User-mode/idle loop etc. */ -bool qs_pending;/* Core waits for quiesc state. */ +bool core_needs_qs;/* Core waits for quiesc state. */ bool beenonline;/* CPU online at least once. */ bool gpwrap;/* Possible gpnum/completed wrap. */ struct rcu_node *mynode;/* This CPU's leaf of hierarchy */
@@ -368,7 +368,7 @@ struct rcu_data {/* 5) __rcu_pending() statistics. */ unsigned long n_rcu_pending;/* rcu_pending() calls since boot. */ -unsigned long n_rp_qs_pending; +unsigned long n_rp_core_needs_qs;unsigned long n_rp_report_qs;unsigned long n_rp_cb_ready;unsigned long n_rp_cpu_needs_gp;
kernel/rcu/tree_plugin.h +1 -1
@@ -619,7 +619,7 @@ static void rcu_preempt_check_callbacks(void)return ; }if (t->rcu_read_lock_nesting >0 && -__this_cpu_read (rcu_data_p->qs_pending) && +__this_cpu_read (rcu_data_p->core_needs_qs) && !__this_cpu_read (rcu_data_p->passed_quiesce)) t->rcu_read_unlock_special.b.need_qs = true; }
kernel/rcu/tree_trace.c +2 -2
@@ -123,7 +123,7 @@ static void print_one_rcu_data(struct seq_file *m, struct rcu_data *rdp)ulong2long (rdp->completed),ulong2long (rdp->gpnum), rdp->passed_quiesce, rdp->rcu_qs_ctr_snap ==per_cpu (rcu_qs_ctr, rdp->cpu), - rdp->qs_pending); + rdp->core_needs_qs); seq_printf(m, " dt=%d/%llx/%d df=%lu", atomic_read(&rdp->dynticks->dynticks), rdp->dynticks->dynticks_nesting,
@@ -361,7 +361,7 @@ static void print_one_rcu_pending(struct seq_file *m, struct rcu_data *rdp)cpu_is_offline (rdp->cpu) ?'!' :' ' , rdp->n_rcu_pending);seq_printf (m,"qsp=%ld rpq=%ld cbr=%ld cng=%ld " , - rdp->n_rp_qs_pending, + rdp->n_rp_core_needs_qs, rdp->n_rp_report_qs, rdp->n_rp_cb_ready, rdp->n_rp_cpu_needs_gp);
Signature · 1 line
2.5.2
PATCH 7/18 rcu: Invert passed_quiesce and rename to cpu_no_qs
In reply to Paul E. McKenneyThis commit inverts the sense of the rcu_data structure's ->passed_quiesce field and renames it to ->cpu_no_qs. This will allow a later commit to use an "aggregate OR" operation to test expedited as well as normal grace periods without added overhead.
--- Documentation/RCU/trace.txt | 32 ++++++++++++++++---------------- kernel/rcu/tree.c | 22 +++++++++++----------- kernel/rcu/tree.h | 2 +- kernel/rcu/tree_plugin.h | 6 +++--- kernel/rcu/tree_trace.c | 4 ++-- 5 files changed, 33 insertions(+), 33 deletions(-)
Documentation/RCU/trace.txt +16 -16
@@ -56,14 +56,14 @@ rcuboost: The output of "cat rcu/rcu_preempt/rcudata" looks as follows: - 0!c=30455 g=30456 pq=1/0 qp=1 dt=126535/140000000000000/0 df=2002 of=4 ql=0/0 qs=N... b=10 ci=74572 nci=0 co=1131 ca=716 - 1!c=30719 g=30720 pq=1/0 qp=0 dt=132007/140000000000000/0 df=1874 of=10 ql=0/0 qs=N... b=10 ci=123209 nci=0 co=685 ca=982 - 2!c=30150 g=30151 pq=1/1 qp=1 dt=138537/140000000000000/0 df=1707 of=8 ql=0/0 qs=N... b=10 ci=80132 nci=0 co=1328 ca=1458 - 3 c=31249 g=31250 pq=1/1 qp=0 dt=107255/140000000000000/0 df=1749 of=6 ql=0/450 qs=NRW. b=10 ci=151700 nci=0 co=509 ca=622 - 4!c=29502 g=29503 pq=1/0 qp=1 dt=83647/140000000000000/0 df=965 of=5 ql=0/0 qs=N... b=10 ci=65643 nci=0 co=1373 ca=1521 - 5 c=31201 g=31202 pq=1/0 qp=1 dt=70422/0/0 df=535 of=7 ql=0/0 qs=.... b=10 ci=58500 nci=0 co=764 ca=698 - 6!c=30253 g=30254 pq=1/0 qp=1 dt=95363/140000000000000/0 df=780 of=5 ql=0/0 qs=N... b=10 ci=100607 nci=0 co=1414 ca=1353 - 7 c=31178 g=31178 pq=1/0 qp=0 dt=91536/0/0 df=547 of=4 ql=0/0 qs=.... b=10 ci=109819 nci=0 co=1115 ca=969 + 0!c=30455 g=30456 cnq=1/0:1 dt=126535/140000000000000/0 df=2002 of=4 ql=0/0 qs=N... b=10 ci=74572 nci=0 co=1131 ca=716 + 1!c=30719 g=30720 cnq=1/0:0 dt=132007/140000000000000/0 df=1874 of=10 ql=0/0 qs=N... b=10 ci=123209 nci=0 co=685 ca=982 + 2!c=30150 g=30151 cnq=1/1:1 dt=138537/140000000000000/0 df=1707 of=8 ql=0/0 qs=N... b=10 ci=80132 nci=0 co=1328 ca=1458 + 3 c=31249 g=31250 cnq=1/1:0 dt=107255/140000000000000/0 df=1749 of=6 ql=0/450 qs=NRW. b=10 ci=151700 nci=0 co=509 ca=622 + 4!c=29502 g=29503 cnq=1/0:1 dt=83647/140000000000000/0 df=965 of=5 ql=0/0 qs=N... b=10 ci=65643 nci=0 co=1373 ca=1521 + 5 c=31201 g=31202 cnq=1/0:1 dt=70422/0/0 df=535 of=7 ql=0/0 qs=.... b=10 ci=58500 nci=0 co=764 ca=698 + 6!c=30253 g=30254 cnq=1/0:1 dt=95363/140000000000000/0 df=780 of=5 ql=0/0 qs=N... b=10 ci=100607 nci=0 co=1414 ca=1353 + 7 c=31178 g=31178 cnq=1/0:0 dt=91536/0/0 df=547 of=4 ql=0/0 qs=.... b=10 ci=109819 nci=0 co=1115 ca=969 This file has one line per CPU, or eight for this 8-CPU system. The fields are as follows:
@@ -188,14 +188,14 @@ o "ca" is the number of RCU callbacks that have been adopted by this Kernels compiled with CONFIG_RCU_BOOST=y display the following from /debug/rcu/rcu_preempt/rcudata: - 0!c=12865 g=12866 pq=1/0 qp=1 dt=83113/140000000000000/0 df=288 of=11 ql=0/0 qs=N... kt=0/O ktl=944 b=10 ci=60709 nci=0 co=748 ca=871 - 1 c=14407 g=14408 pq=1/0 qp=0 dt=100679/140000000000000/0 df=378 of=7 ql=0/119 qs=NRW. kt=0/W ktl=9b6 b=10 ci=109740 nci=0 co=589 ca=485 - 2 c=14407 g=14408 pq=1/0 qp=0 dt=105486/0/0 df=90 of=9 ql=0/89 qs=NRW. kt=0/W ktl=c0c b=10 ci=83113 nci=0 co=533 ca=490 - 3 c=14407 g=14408 pq=1/0 qp=0 dt=107138/0/0 df=142 of=8 ql=0/188 qs=NRW. kt=0/W ktl=b96 b=10 ci=121114 nci=0 co=426 ca=290 - 4 c=14405 g=14406 pq=1/0 qp=1 dt=50238/0/0 df=706 of=7 ql=0/0 qs=.... kt=0/W ktl=812 b=10 ci=34929 nci=0 co=643 ca=114 - 5!c=14168 g=14169 pq=1/0 qp=0 dt=45465/140000000000000/0 df=161 of=11 ql=0/0 qs=N... kt=0/O ktl=b4d b=10 ci=47712 nci=0 co=677 ca=722 - 6 c=14404 g=14405 pq=1/0 qp=0 dt=59454/0/0 df=94 of=6 ql=0/0 qs=.... kt=0/W ktl=e57 b=10 ci=55597 nci=0 co=701 ca=811 - 7 c=14407 g=14408 pq=1/0 qp=1 dt=68850/0/0 df=31 of=8 ql=0/0 qs=.... kt=0/W ktl=14bd b=10 ci=77475 nci=0 co=508 ca=1042 + 0!c=12865 g=12866 cnq=1/0:1 dt=83113/140000000000000/0 df=288 of=11 ql=0/0 qs=N... kt=0/O ktl=944 b=10 ci=60709 nci=0 co=748 ca=871 + 1 c=14407 g=14408 cnq=1/0:0 dt=100679/140000000000000/0 df=378 of=7 ql=0/119 qs=NRW. kt=0/W ktl=9b6 b=10 ci=109740 nci=0 co=589 ca=485 + 2 c=14407 g=14408 cnq=1/0:0 dt=105486/0/0 df=90 of=9 ql=0/89 qs=NRW. kt=0/W ktl=c0c b=10 ci=83113 nci=0 co=533 ca=490 + 3 c=14407 g=14408 cnq=1/0:0 dt=107138/0/0 df=142 of=8 ql=0/188 qs=NRW. kt=0/W ktl=b96 b=10 ci=121114 nci=0 co=426 ca=290 + 4 c=14405 g=14406 cnq=1/0:1 dt=50238/0/0 df=706 of=7 ql=0/0 qs=.... kt=0/W ktl=812 b=10 ci=34929 nci=0 co=643 ca=114 + 5!c=14168 g=14169 cnq=1/0:0 dt=45465/140000000000000/0 df=161 of=11 ql=0/0 qs=N... kt=0/O ktl=b4d b=10 ci=47712 nci=0 co=677 ca=722 + 6 c=14404 g=14405 cnq=1/0:0 dt=59454/0/0 df=94 of=6 ql=0/0 qs=.... kt=0/W ktl=e57 b=10 ci=55597 nci=0 co=701 ca=811 + 7 c=14407 g=14408 cnq=1/0:1 dt=68850/0/0 df=31 of=8 ql=0/0 qs=.... kt=0/W ktl=14bd b=10 ci=77475 nci=0 co=508 ca=1042 This is similar to the output discussed above, but contains the following additional fields:
kernel/rcu/tree.c +11 -11
@@ -245,21 +245,21 @@ static int rcu_gp_in_progress(struct rcu_state *rsp)*/ void rcu_sched_qs (void ) { -if (!__this_cpu_read (rcu_sched_data.passed_quiesce)) { +if (__this_cpu_read (rcu_sched_data.cpu_no_qs)) {trace_rcu_grace_period (TPS ("rcu_sched" ),__this_cpu_read (rcu_sched_data.gpnum),TPS ("cpuqs" )); -__this_cpu_write (rcu_sched_data.passed_quiesce,1 ); +__this_cpu_write (rcu_sched_data.cpu_no_qs, false); } }void rcu_bh_qs (void ) { -if (!__this_cpu_read (rcu_bh_data.passed_quiesce)) { +if (__this_cpu_read (rcu_bh_data.cpu_no_qs)) {trace_rcu_grace_period (TPS ("rcu_bh" ),__this_cpu_read (rcu_bh_data.gpnum),TPS ("cpuqs" )); -__this_cpu_write (rcu_bh_data.passed_quiesce,1 ); +__this_cpu_write (rcu_bh_data.cpu_no_qs, false); } }
@@ -1744,7 +1744,7 @@ static bool __note_gp_changes(struct rcu_state *rsp, struct rcu_node *rnp,*/ rdp->gpnum = rnp->gpnum;trace_rcu_grace_period (rsp->name, rdp->gpnum,TPS ("cpustart" )); - rdp->passed_quiesce =0 ; + rdp->cpu_no_qs = true; rdp->rcu_qs_ctr_snap =__this_cpu_read (rcu_qs_ctr); rdp->core_needs_qs = !!(rnp->qsmask & rdp->grpmask);zero_cpu_stall_ticks (rdp);
@@ -2337,7 +2337,7 @@ rcu_report_qs_rdp(int cpu, struct rcu_state *rsp, struct rcu_data *rdp) rnp = rdp->mynode;raw_spin_lock_irqsave (&rnp->lock, flags);smp_mb__after_unlock_lock (); -if ((rdp->passed_quiesce ==0 && +if ((rdp->cpu_no_qs && rdp->rcu_qs_ctr_snap ==__this_cpu_read (rcu_qs_ctr)) || rdp->gpnum != rnp->gpnum || rnp->completed == rnp->gpnum || rdp->gpwrap) {
@@ -2348,7 +2348,7 @@ rcu_report_qs_rdp(int cpu, struct rcu_state *rsp, struct rcu_data *rdp)* We will instead need a new quiescent state that lies * within the current grace period. */ - rdp->passed_quiesce =0 ;/* need qs for new gp. */ + rdp->cpu_no_qs = true;/* need qs for new gp. */ rdp->rcu_qs_ctr_snap =__this_cpu_read (rcu_qs_ctr);raw_spin_unlock_irqrestore (&rnp->lock, flags);return ;
@@ -2395,7 +2395,7 @@ rcu_check_quiescent_state(struct rcu_state *rsp, struct rcu_data *rdp)* Was there a quiescent state since the beginning of the grace * period? If no, then exit and wait for the next call. */ -if (!rdp->passed_quiesce && +if (rdp->cpu_no_qs && rdp->rcu_qs_ctr_snap ==__this_cpu_read (rcu_qs_ctr))return ;
@@ -3828,11 +3828,11 @@ static int __rcu_pending(struct rcu_state *rsp, struct rcu_data *rdp)/* Is the RCU core waiting for a quiescent state from this CPU? */ if (rcu_scheduler_fully_active && - rdp->core_needs_qs && !rdp->passed_quiesce && + rdp->core_needs_qs && rdp->cpu_no_qs && rdp->rcu_qs_ctr_snap ==__this_cpu_read (rcu_qs_ctr)) { rdp->n_rp_core_needs_qs++; }else if (rdp->core_needs_qs && - (rdp->passed_quiesce || + (!rdp->cpu_no_qs || rdp->rcu_qs_ctr_snap !=__this_cpu_read (rcu_qs_ctr))) { rdp->n_rp_report_qs++;return 1 ;
@@ -4155,7 +4155,7 @@ rcu_init_percpu_data(int cpu, struct rcu_state *rsp) rdp->beenonline = true;/* We have now been online. */ rdp->gpnum = rnp->completed;/* Make CPU later note any new GP. */ rdp->completed = rnp->completed; - rdp->passed_quiesce = false; + rdp->cpu_no_qs = true; rdp->rcu_qs_ctr_snap =per_cpu (rcu_qs_ctr, cpu); rdp->core_needs_qs = false;trace_rcu_grace_period (rsp->name, rdp->gpnum,TPS ("cpuonl" ));
kernel/rcu/tree.h +1 -1
@@ -302,7 +302,7 @@ struct rcu_data {/* is aware of having started. */ unsigned long rcu_qs_ctr_snap;/* Snapshot of rcu_qs_ctr to check */ /* for rcu_all_qs() invocations. */ -bool passed_quiesce;/* User-mode/idle loop etc. */ +bool cpu_no_qs;/* No QS yet for this CPU. */ bool core_needs_qs;/* Core waits for quiesc state. */ bool beenonline;/* CPU online at least once. */ bool gpwrap;/* Possible gpnum/completed wrap. */
kernel/rcu/tree_plugin.h +3 -3
@@ -265,11 +265,11 @@ static void rcu_preempt_ctxt_queue(struct rcu_node *rnp, struct rcu_data *rdp,*/ static void rcu_preempt_qs (void ) { -if (!__this_cpu_read (rcu_data_p->passed_quiesce)) { +if (__this_cpu_read (rcu_data_p->cpu_no_qs)) {trace_rcu_grace_period (TPS ("rcu_preempt" ),__this_cpu_read (rcu_data_p->gpnum),TPS ("cpuqs" )); -__this_cpu_write (rcu_data_p->passed_quiesce,1 ); +__this_cpu_write (rcu_data_p->cpu_no_qs, false);barrier ();/* Coordinate with rcu_preempt_check_callbacks(). */ current->rcu_read_unlock_special.b.need_qs = false; }
@@ -620,7 +620,7 @@ static void rcu_preempt_check_callbacks(void) }if (t->rcu_read_lock_nesting >0 &&__this_cpu_read (rcu_data_p->core_needs_qs) && - !__this_cpu_read (rcu_data_p->passed_quiesce)) +__this_cpu_read (rcu_data_p->cpu_no_qs)) t->rcu_read_unlock_special.b.need_qs = true; }
kernel/rcu/tree_trace.c +2 -2
@@ -117,11 +117,11 @@ static void print_one_rcu_data(struct seq_file *m, struct rcu_data *rdp)if (!rdp->beenonline)return ; -seq_printf (m,"%3d%cc=%ld g=%ld pq=%d/%d qp=%d" , +seq_printf (m,"%3d%cc=%ld g=%ld cnq=%d/%d:%d" , rdp->cpu,cpu_is_offline (rdp->cpu) ?'!' :' ' ,ulong2long (rdp->completed),ulong2long (rdp->gpnum), - rdp->passed_quiesce, + rdp->cpu_no_qs, rdp->rcu_qs_ctr_snap ==per_cpu (rcu_qs_ctr, rdp->cpu), rdp->core_needs_qs); seq_printf(m, " dt=%d/%llx/%d df=%lu",
Signature · 1 line
2.5.2
PATCH 8/18 rcu: Make ->cpu_no_qs be a union for aggregate OR
In reply to Paul E. McKenneyThis commit converts the rcu_data structure's ->cpu_no_qs field to a union. The bytewise side of this union allows individual access to indications as to whether this CPU needs to find a quiescent state for a normal (.norm) and/or expedited (.exp) grace period. The setwise side of the union allows testing whether or not a quiescent state is needed at all, for either type of grace period.
For now, only .norm is used. A later commit will introduce the expedited usage.
--- kernel/rcu/tree.c | 22 +++++++++++----------- kernel/rcu/tree.h | 14 +++++++++++++- kernel/rcu/tree_plugin.h | 6 +++--- kernel/rcu/tree_trace.c | 2 +- 4 files changed, 28 insertions(+), 16 deletions(-)
kernel/rcu/tree.c +11 -11
@@ -245,21 +245,21 @@ static int rcu_gp_in_progress(struct rcu_state *rsp)*/ void rcu_sched_qs (void ) { -if (__this_cpu_read (rcu_sched_data.cpu_no_qs)) { +if (__this_cpu_read (rcu_sched_data.cpu_no_qs.s)) {trace_rcu_grace_period (TPS ("rcu_sched" ),__this_cpu_read (rcu_sched_data.gpnum),TPS ("cpuqs" )); -__this_cpu_write (rcu_sched_data.cpu_no_qs, false); +__this_cpu_write (rcu_sched_data.cpu_no_qs.b.norm, false); } }void rcu_bh_qs (void ) { -if (__this_cpu_read (rcu_bh_data.cpu_no_qs)) { +if (__this_cpu_read (rcu_bh_data.cpu_no_qs.s)) {trace_rcu_grace_period (TPS ("rcu_bh" ),__this_cpu_read (rcu_bh_data.gpnum),TPS ("cpuqs" )); -__this_cpu_write (rcu_bh_data.cpu_no_qs, false); +__this_cpu_write (rcu_bh_data.cpu_no_qs.b.norm, false); } }
@@ -1744,7 +1744,7 @@ static bool __note_gp_changes(struct rcu_state *rsp, struct rcu_node *rnp,*/ rdp->gpnum = rnp->gpnum;trace_rcu_grace_period (rsp->name, rdp->gpnum,TPS ("cpustart" )); - rdp->cpu_no_qs = true; + rdp->cpu_no_qs.b.norm = true; rdp->rcu_qs_ctr_snap =__this_cpu_read (rcu_qs_ctr); rdp->core_needs_qs = !!(rnp->qsmask & rdp->grpmask);zero_cpu_stall_ticks (rdp);
@@ -2337,7 +2337,7 @@ rcu_report_qs_rdp(int cpu, struct rcu_state *rsp, struct rcu_data *rdp) rnp = rdp->mynode;raw_spin_lock_irqsave (&rnp->lock, flags);smp_mb__after_unlock_lock (); -if ((rdp->cpu_no_qs && +if ((rdp->cpu_no_qs.b.norm && rdp->rcu_qs_ctr_snap ==__this_cpu_read (rcu_qs_ctr)) || rdp->gpnum != rnp->gpnum || rnp->completed == rnp->gpnum || rdp->gpwrap) {
@@ -2348,7 +2348,7 @@ rcu_report_qs_rdp(int cpu, struct rcu_state *rsp, struct rcu_data *rdp)* We will instead need a new quiescent state that lies * within the current grace period. */ - rdp->cpu_no_qs = true;/* need qs for new gp. */ + rdp->cpu_no_qs.b.norm = true;/* need qs for new gp. */ rdp->rcu_qs_ctr_snap =__this_cpu_read (rcu_qs_ctr);raw_spin_unlock_irqrestore (&rnp->lock, flags);return ;
@@ -2395,7 +2395,7 @@ rcu_check_quiescent_state(struct rcu_state *rsp, struct rcu_data *rdp)* Was there a quiescent state since the beginning of the grace * period? If no, then exit and wait for the next call. */ -if (rdp->cpu_no_qs && +if (rdp->cpu_no_qs.b.norm && rdp->rcu_qs_ctr_snap ==__this_cpu_read (rcu_qs_ctr))return ;
@@ -3828,11 +3828,11 @@ static int __rcu_pending(struct rcu_state *rsp, struct rcu_data *rdp)/* Is the RCU core waiting for a quiescent state from this CPU? */ if (rcu_scheduler_fully_active && - rdp->core_needs_qs && rdp->cpu_no_qs && + rdp->core_needs_qs && rdp->cpu_no_qs.b.norm && rdp->rcu_qs_ctr_snap ==__this_cpu_read (rcu_qs_ctr)) { rdp->n_rp_core_needs_qs++; }else if (rdp->core_needs_qs && - (!rdp->cpu_no_qs || + (!rdp->cpu_no_qs.b.norm || rdp->rcu_qs_ctr_snap !=__this_cpu_read (rcu_qs_ctr))) { rdp->n_rp_report_qs++;return 1 ;
@@ -4155,7 +4155,7 @@ rcu_init_percpu_data(int cpu, struct rcu_state *rsp) rdp->beenonline = true;/* We have now been online. */ rdp->gpnum = rnp->completed;/* Make CPU later note any new GP. */ rdp->completed = rnp->completed; - rdp->cpu_no_qs = true; + rdp->cpu_no_qs.b.norm = true; rdp->rcu_qs_ctr_snap =per_cpu (rcu_qs_ctr, cpu); rdp->core_needs_qs = false;trace_rcu_grace_period (rsp->name, rdp->gpnum,TPS ("cpuonl" ));
kernel/rcu/tree.h +13 -1
@@ -286,6 +286,18 @@ struct rcu_node {for ((rnp) = (rsp)->level[rcu_num_lvls -1 ]; \ (rnp) < &(rsp)->node[rcu_num_nodes]; (rnp)++) +/* +* Union to allow "aggregate OR" operation on the need for a quiescent +* state by the normal and expedited grace periods. +*/ +union rcu_noqs { +struct { +u8 norm; +u8 exp; + } b;/* Bits. */ +u16 s;/* Set of bits, aggregate OR here. */ +}; +/* Index values for nxttail array in struct rcu_data. */ #define RCU_DONE_TAIL 0/* Also RCU_WAIT head. */ #define RCU_WAIT_TAIL 1/* Also RCU_NEXT_READY head. */
@@ -302,7 +314,7 @@ struct rcu_data {/* is aware of having started. */ unsigned long rcu_qs_ctr_snap;/* Snapshot of rcu_qs_ctr to check */ /* for rcu_all_qs() invocations. */ -bool cpu_no_qs;/* No QS yet for this CPU. */ +union rcu_noqs cpu_no_qs;/* No QSes yet for this CPU. */ bool core_needs_qs;/* Core waits for quiesc state. */ bool beenonline;/* CPU online at least once. */ bool gpwrap;/* Possible gpnum/completed wrap. */
kernel/rcu/tree_plugin.h +3 -3
@@ -265,11 +265,11 @@ static void rcu_preempt_ctxt_queue(struct rcu_node *rnp, struct rcu_data *rdp,*/ static void rcu_preempt_qs (void ) { -if (__this_cpu_read (rcu_data_p->cpu_no_qs)) { +if (__this_cpu_read (rcu_data_p->cpu_no_qs.s)) {trace_rcu_grace_period (TPS ("rcu_preempt" ),__this_cpu_read (rcu_data_p->gpnum),TPS ("cpuqs" )); -__this_cpu_write (rcu_data_p->cpu_no_qs, false); +__this_cpu_write (rcu_data_p->cpu_no_qs.b.norm, false);barrier ();/* Coordinate with rcu_preempt_check_callbacks(). */ current->rcu_read_unlock_special.b.need_qs = false; }
@@ -620,7 +620,7 @@ static void rcu_preempt_check_callbacks(void) }if (t->rcu_read_lock_nesting >0 &&__this_cpu_read (rcu_data_p->core_needs_qs) && -__this_cpu_read (rcu_data_p->cpu_no_qs)) +__this_cpu_read (rcu_data_p->cpu_no_qs.b.norm)) t->rcu_read_unlock_special.b.need_qs = true; }
kernel/rcu/tree_trace.c +1 -1
@@ -121,7 +121,7 @@ static void print_one_rcu_data(struct seq_file *m, struct rcu_data *rdp) rdp->cpu,cpu_is_offline (rdp->cpu) ?'!' :' ' ,ulong2long (rdp->completed),ulong2long (rdp->gpnum), - rdp->cpu_no_qs, + rdp->cpu_no_qs.b.norm, rdp->rcu_qs_ctr_snap ==per_cpu (rcu_qs_ctr, rdp->cpu), rdp->core_needs_qs); seq_printf(m, " dt=%d/%llx/%d df=%lu",
Signature · 1 line
2.5.2
PATCH 9/18 rcu: Switch synchronize_sched_expedited() to IPI
In reply to Paul E. McKenneyThis commit switches synchronize_sched_expedited() from stop_one_cpu_nowait() to smp_call_function_single(), thus moving from an IPI and a pair of context switches to an IPI and a single pass through the scheduler. Of course, if the scheduler actually does decide to switch to a different task, there will still be a pair of context switches, but there would likely have been a pair of context switches anyway, just a bit later.
--- kernel/rcu/tree.c | 32 ++++++++++++++++++++------------ kernel/rcu/tree.h | 3 --- 2 files changed, 20 insertions(+), 15 deletions(-)
kernel/rcu/tree.c +20 -12
@@ -161,6 +161,8 @@ static void rcu_cleanup_dead_rnp(struct rcu_node *rnp_leaf);static void rcu_boost_kthread_setaffinity (struct rcu_node *rnp,int outgoingcpu);static void invoke_rcu_core (void );static void invoke_rcu_callbacks (struct rcu_state *rsp,struct rcu_data *rdp); +static void __maybe_unused rcu_report_exp_rdp (struct rcu_state *rsp, +struct rcu_data *rdp,bool wake);/* rcuc/rcub kthread realtime priority */ #ifdef CONFIG_RCU_KTHREAD_PRIO
@@ -250,6 +252,12 @@ void rcu_sched_qs(void)__this_cpu_read (rcu_sched_data.gpnum), TPS("cpuqs"));__this_cpu_write (rcu_sched_data.cpu_no_qs.b.norm, false); +if (__this_cpu_read (rcu_sched_data.cpu_no_qs.b.exp)) { +__this_cpu_write (rcu_sched_data.cpu_no_qs.b.exp, false); +rcu_report_exp_rdp (&rcu_sched_state, +this_cpu_ptr (&rcu_sched_data), + true); + } } }
@@ -3555,8 +3563,8 @@ static void rcu_report_exp_cpu_mult(struct rcu_state *rsp, struct rcu_node *rnp,* Report expedited quiescent state for specified rcu_data (CPU). * Caller must hold the root rcu_node's exp_funnel_mutex. */ -static void __maybe_unused rcu_report_exp_rdp (struct rcu_state *rsp, -struct rcu_data *rdp,bool wake) +static void rcu_report_exp_rdp (struct rcu_state *rsp,struct rcu_data *rdp, +bool wake) {rcu_report_exp_cpu_mult (rsp, rdp->mynode, rdp->grpmask, wake); }
@@ -3637,14 +3645,10 @@ static struct rcu_node *exp_funnel_lock(struct rcu_state *rsp, unsigned long s) }/* Invoked on each online non-idle CPU for expedited quiescent state. */ -static int synchronize_sched_expedited_cpu_stop (void *data) +static void synchronize_sched_expedited_cpu_stop (void *data) { -struct rcu_data *rdp = data; -struct rcu_state *rsp = rdp->rsp; - -/* Report the quiescent state. */ -rcu_report_exp_rdp (rsp, rdp, true); -return 0 ; +__this_cpu_write (rcu_sched_data.cpu_no_qs.b.exp, true); +resched_cpu (smp_processor_id ()); }/*
@@ -3659,6 +3663,7 @@ static void sync_sched_exp_select_cpus(struct rcu_state *rsp)unsigned long mask_ofl_test;unsigned long mask_ofl_ipi;struct rcu_data *rdp; +int ret;struct rcu_node *rnp;sync_exp_reset_tree (rsp);
@@ -3694,9 +3699,9 @@ static void sync_sched_exp_select_cpus(struct rcu_state *rsp)if (!(mask_ofl_ipi & mask))continue ; rdp =per_cpu_ptr (rsp->rda, cpu); -stop_one_cpu_nowait (cpu, synchronize_sched_expedited_cpu_stop, - rdp, &rdp->exp_stop_work); - mask_ofl_ipi &= ~mask; + ret =smp_call_function_single (cpu, synchronize_sched_expedited_cpu_stop,NULL ,0 ); +if (!ret) + mask_ofl_ipi &= ~mask; }/* Report quiescent states for those that went offline. */ mask_ofl_test |= mask_ofl_ipi;
@@ -4201,6 +4206,9 @@ int rcu_cpu_notify(struct notifier_block *self,rcu_cleanup_dying_cpu (rsp);break ;case CPU_DYING_IDLE : +/* QS for any half-done expedited RCU-sched GP. */ +rcu_sched_qs (); +for_each_rcu_flavor (rsp) {rcu_cleanup_dying_idle_cpu (cpu, rsp); }
kernel/rcu/tree.h +0 -3
@@ -324,9 +324,6 @@ struct rcu_data {/* ticks this CPU has handled */ /* during and after the last grace */ /* period it is aware of. */ -struct cpu_stop_work exp_stop_work; -/* Expedited grace-period control */ -/* for CPU stopping. */ /* 2) batch handling */ /*
Signature · 1 line
2.5.2
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 09/18] rcu: Switch synchronize_sched_expedited() to IPI
kernel/rcu/tree.c static void rcu_cleanup_dead_rnp(struct rcu_node *rnp_leaf); · line 166
Quoted 7 lines · Paul E. McKenney
kernel/rcu/tree.c +2
-0@@ -161,6 +161,8 @@ static void rcu_cleanup_dead_rnp(struct rcu_node *rnp_leaf);static void rcu_boost_kthread_setaffinity (struct rcu_node *rnp,int outgoingcpu);static void invoke_rcu_core (void );static void invoke_rcu_callbacks (struct rcu_state *rsp,struct rcu_data *rdp); +static void __maybe_unused rcu_report_exp_rdp (struct rcu_state *rsp, +struct rcu_data *rdp,bool wake);
Do we really need that on a declaration? I though only unused definitions required it.
Re: [PATCH tip/core/rcu 09/18] rcu: Switch synchronize_sched_expedited() to IPI
Quoted 12 lines · Peter Zijlstra
kernel/rcu/tree.c static void rcu_cleanup_dead_rnp(struct rcu_node *rnp_leaf); · line 166
Quoted 7 lines · Paul E. McKenney
kernel/rcu/tree.c +2
-0@@ -161,6 +161,8 @@ static void rcu_cleanup_dead_rnp(struct rcu_node *rnp_leaf);static void rcu_boost_kthread_setaffinity (struct rcu_node *rnp,int outgoingcpu);static void invoke_rcu_core (void );static void invoke_rcu_callbacks (struct rcu_state *rsp,struct rcu_data *rdp); +static void __maybe_unused rcu_report_exp_rdp (struct rcu_state *rsp, +struct rcu_data *rdp,bool wake);Do we really need that on a declaration? I though only unused definitions required it.
You are right. I should have removed it in commit 4684e21b19e9 ("rcu:
Switch synchronize_sched_expedited() to IPI"), fixed.
Thanx, Paul
PATCH 10/18 rcu: Stop silencing lockdep false positive for expedited grace periods
In reply to Paul E. McKenneyThis reverts commit af859beaaba4 (rcu: Silence lockdep false positive for expedited grace periods). Because synchronize_rcu_expedited() no longer invokes synchronize_sched_expedited(), ->exp_funnel_mutex acquisition is no longer nested, so the false positive no longer happens. This commit therefore removes the extra lockdep data structures, as they are no longer needed.
--- kernel/rcu/tree.c | 17 ++--------------- kernel/rcu/tree.h | 8 -------- 2 files changed, 2 insertions(+), 23 deletions(-)
kernel/rcu/tree.c +2 -15
@@ -71,7 +71,6 @@ MODULE_ALIAS("rcutree");static struct lock_class_key rcu_node_class[RCU_NUM_LVLS ];static struct lock_class_key rcu_fqs_class[RCU_NUM_LVLS ];static struct lock_class_key rcu_exp_class[RCU_NUM_LVLS ]; -static struct lock_class_key rcu_exp_sched_class[RCU_NUM_LVLS ];/* * In order to export the rcu_state name to the tracing tools, it
@@ -4095,7 +4094,6 @@ static void rcu_init_new_rnp(struct rcu_node *rnp_leaf)static void __init rcu_boot_init_percpu_data (int cpu,struct rcu_state *rsp) { -static struct lock_class_key rcu_exp_sched_rdp_class;unsigned long flags;struct rcu_data *rdp =per_cpu_ptr (rsp->rda, cpu);struct rcu_node *rnp =rcu_get_root (rsp);
@@ -4111,10 +4109,6 @@ rcu_boot_init_percpu_data(int cpu, struct rcu_state *rsp)mutex_init (&rdp->exp_funnel_mutex);rcu_boot_init_nocb_percpu_data (rdp);raw_spin_unlock_irqrestore (&rnp->lock, flags); -if (rsp == &rcu_sched_state) -lockdep_set_class_and_name (&rdp->exp_funnel_mutex, - &rcu_exp_sched_rdp_class, -"rcu_data_exp_sched" ); }/*
@@ -4340,7 +4334,6 @@ static void __init rcu_init_one(struct rcu_state *rsp,static const char *const buf[] =RCU_NODE_NAME_INIT ;static const char *const fqs[] =RCU_FQS_NAME_INIT ;static const char *const exp[] =RCU_EXP_NAME_INIT ; -static const char *const exp_sched[] =RCU_EXP_SCHED_NAME_INIT ;static u8 fl_mask =0x1 ;int levelcnt[RCU_NUM_LVLS ];/* # nodes in each level. */
@@ -4400,14 +4393,8 @@ static void __init rcu_init_one(struct rcu_state *rsp,INIT_LIST_HEAD (&rnp->blkd_tasks);rcu_init_one_nocb (rnp);mutex_init (&rnp->exp_funnel_mutex); -if (rsp == &rcu_sched_state) -lockdep_set_class_and_name ( - &rnp->exp_funnel_mutex, - &rcu_exp_sched_class[i], exp_sched[i]); -else -lockdep_set_class_and_name ( - &rnp->exp_funnel_mutex, - &rcu_exp_class[i], exp[i]); +lockdep_set_class_and_name (&rnp->exp_funnel_mutex, + &rcu_exp_class[i], exp[i]); } }
kernel/rcu/tree.h +0 -8
@@ -70,8 +70,6 @@# define RCU_NODE_NAME_INIT { "rcu_node_0" }# define RCU_FQS_NAME_INIT { "rcu_node_fqs_0" }# define RCU_EXP_NAME_INIT { "rcu_node_exp_0" } -# define RCU_EXP_SCHED_NAME_INIT \ - { "rcu_node_exp_sched_0" }#elif NR_CPUS <=RCU_FANOUT_2 # define RCU_NUM_LVLS 2# define NUM_RCU_LVL_0 1
@@ -81,8 +79,6 @@# define RCU_NODE_NAME_INIT { "rcu_node_0", "rcu_node_1" }# define RCU_FQS_NAME_INIT { "rcu_node_fqs_0", "rcu_node_fqs_1" }# define RCU_EXP_NAME_INIT { "rcu_node_exp_0", "rcu_node_exp_1" } -# define RCU_EXP_SCHED_NAME_INIT \ - { "rcu_node_exp_sched_0", "rcu_node_exp_sched_1" }#elif NR_CPUS <=RCU_FANOUT_3 # define RCU_NUM_LVLS 3# define NUM_RCU_LVL_0 1
@@ -93,8 +89,6 @@# define RCU_NODE_NAME_INIT { "rcu_node_0", "rcu_node_1", "rcu_node_2" }# define RCU_FQS_NAME_INIT { "rcu_node_fqs_0", "rcu_node_fqs_1", "rcu_node_fqs_2" }# define RCU_EXP_NAME_INIT { "rcu_node_exp_0", "rcu_node_exp_1", "rcu_node_exp_2" } -# define RCU_EXP_SCHED_NAME_INIT \ - { "rcu_node_exp_sched_0", "rcu_node_exp_sched_1", "rcu_node_exp_sched_2" }#elif NR_CPUS <=RCU_FANOUT_4 # define RCU_NUM_LVLS 4# define NUM_RCU_LVL_0 1
@@ -106,8 +100,6 @@# define RCU_NODE_NAME_INIT { "rcu_node_0", "rcu_node_1", "rcu_node_2", "rcu_node_3" }# define RCU_FQS_NAME_INIT { "rcu_node_fqs_0", "rcu_node_fqs_1", "rcu_node_fqs_2", "rcu_node_fqs_3" }# define RCU_EXP_NAME_INIT { "rcu_node_exp_0", "rcu_node_exp_1", "rcu_node_exp_2", "rcu_node_exp_3" } -# define RCU_EXP_SCHED_NAME_INIT \ - { "rcu_node_exp_sched_0", "rcu_node_exp_sched_1", "rcu_node_exp_sched_2", "rcu_node_exp_sched_3" }#else # error "CONFIG_RCU_FANOUT insufficient for NR_CPUS"#endif /* #if (NR_CPUS) <= RCU_FANOUT_1 */
Signature · 1 line
2.5.2
PATCH 11/18 rcu: Stop excluding CPU hotplug in synchronize_sched_expedited()
In reply to Paul E. McKenneyNow that synchronize_sched_expedited() uses IPIs, a hook in rcu_sched_qs(), and the ->expmask field in the rcu_node combining tree, it is no longer necessary to exclude CPU hotplug. Any races with CPU hotplug will be detected when attempting to send the IPI. This commit therefore removes the code excluding CPU hotplug operations.
--- kernel/rcu/tree.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-)
kernel/rcu/tree.c +1 -13
@@ -3785,19 +3785,9 @@ void synchronize_sched_expedited(void)/* Take a snapshot of the sequence number. */ s =rcu_exp_gp_seq_snap (rsp); -if (!try_get_online_cpus ()) { -/* CPU hotplug operation in flight, fall back to normal GP. */ -wait_rcu_gp (call_rcu_sched); -atomic_long_inc (&rsp->expedited_normal); -return ; - } -WARN_ON_ONCE (cpu_is_offline (raw_smp_processor_id ())); - rnp =exp_funnel_lock (rsp, s); -if (rnp ==NULL ) { -put_online_cpus (); +if (rnp ==NULL )return ;/* Someone else did our work for us. */ - }rcu_exp_gp_seq_start (rsp);sync_sched_exp_select_cpus (rsp);
@@ -3805,8 +3795,6 @@ void synchronize_sched_expedited(void)rcu_exp_gp_seq_end (rsp);mutex_unlock (&rnp->exp_funnel_mutex); - -put_online_cpus (); }EXPORT_SYMBOL_GPL (synchronize_sched_expedited);
Signature · 1 line
2.5.2
PATCH 12/18 cpu: Remove try_get_online_cpus()
In reply to Paul E. McKenneyNow that synchronize_sched_expedited() no longer uses it, there are no users of try_get_online_cpus() in mainline. This commit therefore removes it.
--- include/linux/cpu.h | 2 -- kernel/cpu.c | 13 ------------- 2 files changed, 15 deletions(-)
include/linux/cpu.h +0 -2
@@ -228,7 +228,6 @@ extern struct bus_type cpu_subsys;extern void cpu_hotplug_begin (void );extern void cpu_hotplug_done (void );extern void get_online_cpus (void ); -extern bool try_get_online_cpus (void );extern void put_online_cpus (void );extern void cpu_hotplug_disable (void );extern void cpu_hotplug_enable (void );
@@ -246,7 +245,6 @@ int cpu_down(unsigned int cpu);static inline void cpu_hotplug_begin (void ) {}static inline void cpu_hotplug_done (void ) {}#define get_online_cpus () do { } while (0) -#define try_get_online_cpus () true#define put_online_cpus () do { } while (0)#define cpu_hotplug_disable () do { } while (0)#define cpu_hotplug_enable () do { } while (0)
kernel/cpu.c +0 -13
@@ -102,19 +102,6 @@ void get_online_cpus(void) }EXPORT_SYMBOL_GPL (get_online_cpus); -bool try_get_online_cpus (void ) -{ -if (cpu_hotplug.active_writer == current) -return true; -if (!mutex_trylock (&cpu_hotplug.lock)) -return false; -cpuhp_lock_acquire_tryread (); -atomic_inc (&cpu_hotplug.refcount); -mutex_unlock (&cpu_hotplug.lock); -return true; -} -EXPORT_SYMBOL_GPL (try_get_online_cpus); -void put_online_cpus (void ) {int refcount;
Signature · 1 line
2.5.2
PATCH 13/18 rcu: Prepare for consolidating expedited CPU selection
In reply to Paul E. McKenneyThis commit brings sync_sched_exp_select_cpus() into alignment with sync_rcu_exp_select_cpus(), as a first step towards consolidating them into one function.
--- kernel/rcu/tree.c | 2 -- 1 file changed, 2 deletions(-)
kernel/rcu/tree.c +0 -2
@@ -3661,7 +3661,6 @@ static void sync_sched_exp_select_cpus(struct rcu_state *rsp)unsigned long mask;unsigned long mask_ofl_test;unsigned long mask_ofl_ipi; -struct rcu_data *rdp;int ret;struct rcu_node *rnp;
@@ -3697,7 +3696,6 @@ static void sync_sched_exp_select_cpus(struct rcu_state *rsp)for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<=1 ) {if (!(mask_ofl_ipi & mask))continue ; - rdp =per_cpu_ptr (rsp->rda, cpu); ret =smp_call_function_single (cpu, synchronize_sched_expedited_cpu_stop,NULL ,0 );if (!ret) mask_ofl_ipi &= ~mask;
Signature · 1 line
2.5.2
PATCH 14/18 rcu: Consolidate expedited CPU selection
In reply to Paul E. McKenneyNow that sync_sched_exp_select_cpus() and sync_rcu_exp_select_cpus() are identical aside from the the argument to smp_call_function_single(), this commit consolidates them with a functional argument.
--- kernel/rcu/tree.c | 7 +++--- kernel/rcu/tree_plugin.h | 61 +----------------------------------------------- 2 files changed, 5 insertions(+), 63 deletions(-)
kernel/rcu/tree.c +4 -3
@@ -3654,7 +3654,8 @@ static void synchronize_sched_expedited_cpu_stop(void *data)* Select the nodes that the upcoming expedited grace period needs * to wait for. */ -static void sync_sched_exp_select_cpus (struct rcu_state *rsp) +static void sync_rcu_exp_select_cpus (struct rcu_state *rsp, +smp_call_func_t func) {int cpu;unsigned long flags;
@@ -3696,7 +3697,7 @@ static void sync_sched_exp_select_cpus(struct rcu_state *rsp)for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<=1 ) {if (!(mask_ofl_ipi & mask))continue ; - ret =smp_call_function_single (cpu, synchronize_sched_expedited_cpu_stop,NULL ,0 ); + ret =smp_call_function_single (cpu, func, rsp,0 );if (!ret) mask_ofl_ipi &= ~mask; }
@@ -3788,7 +3789,7 @@ void synchronize_sched_expedited(void)return ;/* Someone else did our work for us. */ rcu_exp_gp_seq_start (rsp); -sync_sched_exp_select_cpus (rsp); +sync_rcu_exp_select_cpus (rsp, synchronize_sched_expedited_cpu_stop);synchronize_sched_expedited_wait (rsp);rcu_exp_gp_seq_end (rsp);
kernel/rcu/tree_plugin.h +1 -60
73 lines not on this page · raw message
Signature · 1 line
2.5.2
PATCH 15/18 rcu: Add online/offline info to expedited stall warning message
In reply to Paul E. McKenneyThis commit makes the RCU CPU stall warning message print online/offline indications immediately after the CPU number. A "O" indicates global offline, a "." global online, and a "o" indicates RCU believes that the CPU is offline for the current grace period and "." otherwise, and an "N" indicates that RCU believes that the CPU will be offline for the next grace period, and "." otherwise, all right after the CPU number. So for CPU 10, you would normally see "10-...:" indicating that everything believes that the CPU is online.
--- kernel/rcu/tree.c | 9 ++++++++- kernel/rcu/tree.h | 1 + kernel/rcu/tree_plugin.h | 31 +++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletion(-)
kernel/rcu/tree.c +8 -1
@@ -3737,11 +3737,18 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)pr_err ("INFO: %s detected expedited stalls on CPUs: {" , rsp->name);rcu_for_each_leaf_node (rsp, rnp) { + (void )rcu_print_task_exp_stall (rnp); mask =1 ;for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<=1 ) { +struct rcu_data *rdp; +if (!(rnp->expmask & mask))continue ; -pr_cont (" %d" , cpu); + rdp =per_cpu_ptr (rsp->rda, cpu); +pr_cont (" %d-%c%c%c" , cpu, +"O." [cpu_online (cpu)], +"o." [!!(rdp->grpmask & rnp->expmaskinit)], +"N." [!!(rdp->grpmask & rnp->expmaskinitnext)]); } mask <<=1 ; }
kernel/rcu/tree.h +1 -0
@@ -589,6 +589,7 @@ static bool rcu_preempt_has_tasks(struct rcu_node *rnp);#endif /* #ifdef CONFIG_HOTPLUG_CPU */ static void rcu_print_detail_task_stall (struct rcu_state *rsp);static int rcu_print_task_stall (struct rcu_node *rnp); +static int rcu_print_task_exp_stall (struct rcu_node *rnp);static void rcu_preempt_check_blocked_tasks (struct rcu_node *rnp);static void rcu_preempt_check_callbacks (void );void call_rcu (struct rcu_head *head,void (*func)(struct rcu_head *rcu));
kernel/rcu/tree_plugin.h +31 -0
@@ -586,6 +586,27 @@ static int rcu_print_task_stall(struct rcu_node *rnp) }/* +* Scan the current list of tasks blocked within RCU read-side critical +* sections, printing out the tid of each that is blocking the current +* expedited grace period. +*/ +static int rcu_print_task_exp_stall (struct rcu_node *rnp) +{ +struct task_struct *t; +int ndetected =0 ; + +if (!rnp->exp_tasks) +return 0 ; + t =list_entry (rnp->exp_tasks->prev, + struct task_struct, rcu_node_entry); +list_for_each_entry_continue (t, &rnp->blkd_tasks, rcu_node_entry) { +pr_cont (" P%d" , t->pid); + ndetected++; + } +return ndetected; +} + +/* * Check that the list of blocked tasks for the newly completed grace * period is in fact empty. It is a serious bug to complete a grace * period that still has RCU readers blocked! This function must be
@@ -846,6 +867,16 @@ static int rcu_print_task_stall(struct rcu_node *rnp) }/* +* Because preemptible RCU does not exist, we never have to check for +* tasks blocked within RCU read-side critical sections that are +* blocking the current expedited grace period. +*/ +static int rcu_print_task_exp_stall (struct rcu_node *rnp) +{ +return 0 ; +} + +/* * Because there is no preemptible RCU, there can be no readers blocked, * so there is no need to check for blocked tasks. So check only for * bogus qsmask values.
Signature · 1 line
2.5.2
PATCH 16/18 rcu: Add tasks to expedited stall-warning messages
In reply to Paul E. McKenneyThis commit adds task-print ability to the expedited RCU CPU stall warning messages in preparation for adding stall warnings to synchornize_rcu_expedited().
--- kernel/rcu/tree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/rcu/tree.c +1 -1
@@ -3734,7 +3734,7 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp) sync_rcu_preempt_exp_done(rnp_root));return ; } -pr_err ("INFO: %s detected expedited stalls on CPUs: {" , +pr_err ("INFO: %s detected expedited stalls on CPUs/tasks: {" , rsp->name);rcu_for_each_leaf_node (rsp, rnp) { (void )rcu_print_task_exp_stall (rnp);
Signature · 1 line
2.5.2
PATCH 17/18 rcu: Enable stall warnings for synchronize_rcu_expedited()
In reply to Paul E. McKenneyThis commit redirects synchronize_rcu_expedited()'s wait to synchronize_sched_expedited_wait(), thus enabling RCU CPU stall warnings.
--- kernel/rcu/tree_plugin.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
kernel/rcu/tree_plugin.h +1 -2
@@ -761,8 +761,7 @@ void synchronize_rcu_expedited(void)/* Wait for snapshotted ->blkd_tasks lists to drain. */ rnp =rcu_get_root (rsp); -wait_event (rsp->expedited_wq, -sync_rcu_preempt_exp_done (rnp)); +synchronize_sched_expedited_wait (rsp);/* Clean up and exit. */ rcu_exp_gp_seq_end (rsp);
Signature · 1 line
2.5.2
PATCH 18/18 rcu: Better hotplug handling for synchronize_sched_expedited()
In reply to Paul E. McKenneyEarlier versions of synchronize_sched_expedited() can prematurely end grace periods due to the fact that a CPU marked as cpu_is_offline() can still be using RCU read-side critical sections during the time that CPU makes its last pass through the scheduler and into the idle loop and during the time that a given CPU is in the process of coming online. This commit therefore eliminates this window by adding additional interaction with the CPU-hotplug operations.
--- kernel/rcu/tree.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 62 insertions(+), 6 deletions(-)
kernel/rcu/tree.c +62 -6
@@ -246,17 +246,23 @@ static int rcu_gp_in_progress(struct rcu_state *rsp)*/ void rcu_sched_qs (void ) { +unsigned long flags; +if (__this_cpu_read (rcu_sched_data.cpu_no_qs.s)) {trace_rcu_grace_period (TPS ("rcu_sched" ),__this_cpu_read (rcu_sched_data.gpnum),TPS ("cpuqs" ));__this_cpu_write (rcu_sched_data.cpu_no_qs.b.norm, false); +if (!__this_cpu_read (rcu_sched_data.cpu_no_qs.b.exp)) +return ; +local_irq_save (flags);if (__this_cpu_read (rcu_sched_data.cpu_no_qs.b.exp)) {__this_cpu_write (rcu_sched_data.cpu_no_qs.b.exp, false);rcu_report_exp_rdp (&rcu_sched_state,this_cpu_ptr (&rcu_sched_data), true); } +local_irq_restore (flags); } }
@@ -3553,7 +3559,10 @@ static void rcu_report_exp_cpu_mult(struct rcu_state *rsp, struct rcu_node *rnp,raw_spin_lock_irqsave (&rnp->lock, flags);smp_mb__after_unlock_lock (); -WARN_ON_ONCE ((rnp->expmask & mask) != mask); +if (!(rnp->expmask & mask)) { +raw_spin_unlock_irqrestore (&rnp->lock, flags); +return ; + } rnp->expmask &= ~mask;__rcu_report_exp_rnp (rsp, rnp, wake, flags);/* Releases rnp->lock. */ }
@@ -3644,12 +3653,37 @@ static struct rcu_node *exp_funnel_lock(struct rcu_state *rsp, unsigned long s) }/* Invoked on each online non-idle CPU for expedited quiescent state. */ -static void synchronize_sched_expedited_cpu_stop (void *data) +static void sync_sched_exp_handler (void *data) { +struct rcu_data *rdp; +struct rcu_node *rnp; +struct rcu_state *rsp = data; + + rdp =this_cpu_ptr (rsp->rda); + rnp = rdp->mynode; +if (!(READ_ONCE (rnp->expmask) & rdp->grpmask) || +__this_cpu_read (rcu_sched_data.cpu_no_qs.b.exp)) +return ;__this_cpu_write (rcu_sched_data.cpu_no_qs.b.exp, true);resched_cpu (smp_processor_id ()); } +/* Send IPI for expedited cleanup if needed at end of CPU-hotplug operation. */ +static void sync_sched_exp_online_cleanup (int cpu) +{ +struct rcu_data *rdp; +int ret; +struct rcu_node *rnp; +struct rcu_state *rsp = &rcu_sched_state; + + rdp =per_cpu_ptr (rsp->rda, cpu); + rnp = rdp->mynode; +if (!(READ_ONCE (rnp->expmask) & rdp->grpmask)) +return ; + ret =smp_call_function_single (cpu, sync_sched_exp_handler, rsp,0 ); +WARN_ON_ONCE (ret); +} +/* * Select the nodes that the upcoming expedited grace period needs * to wait for.
@@ -3677,7 +3711,6 @@ static void sync_rcu_exp_select_cpus(struct rcu_state *rsp,struct rcu_dynticks *rdtp = &per_cpu (rcu_dynticks, cpu);if (raw_smp_processor_id () == cpu || -cpu_is_offline (cpu) || !(atomic_add_return (0 , &rdtp->dynticks) &0x1 )) mask_ofl_test |= rdp->grpmask; }
@@ -3697,9 +3730,28 @@ static void sync_rcu_exp_select_cpus(struct rcu_state *rsp,for (cpu = rnp->grplo; cpu <= rnp->grphi; cpu++, mask <<=1 ) {if (!(mask_ofl_ipi & mask))continue ; +retry_ipi: ret =smp_call_function_single (cpu, func, rsp,0 ); -if (!ret) +if (!ret) { mask_ofl_ipi &= ~mask; + }else { +/* Failed, raced with offline. */ +raw_spin_lock_irqsave (&rnp->lock, flags); +if (cpu_online (cpu) && + (rnp->expmask & mask)) { +raw_spin_unlock_irqrestore (&rnp->lock, + flags); +schedule_timeout_uninterruptible (1 ); +if (cpu_online (cpu) && + (rnp->expmask & mask)) + goto retry_ipi; +raw_spin_lock_irqsave (&rnp->lock, + flags); + } +if (!(rnp->expmask & mask)) + mask_ofl_ipi &= ~mask; +raw_spin_unlock_irqrestore (&rnp->lock, flags); + } }/* Report quiescent states for those that went offline. */ mask_ofl_test |= mask_ofl_ipi;
@@ -3796,7 +3848,7 @@ void synchronize_sched_expedited(void)return ;/* Someone else did our work for us. */ rcu_exp_gp_seq_start (rsp); -sync_rcu_exp_select_cpus (rsp, synchronize_sched_expedited_cpu_stop); +sync_rcu_exp_select_cpus (rsp, sync_sched_exp_handler);synchronize_sched_expedited_wait (rsp);rcu_exp_gp_seq_end (rsp);
@@ -4183,6 +4235,7 @@ int rcu_cpu_notify(struct notifier_block *self,break ;case CPU_ONLINE :case CPU_DOWN_FAILED : +sync_sched_exp_online_cleanup (cpu);rcu_boost_kthread_setaffinity (rnp,-1 );break ;case CPU_DOWN_PREPARE :
@@ -4195,7 +4248,10 @@ int rcu_cpu_notify(struct notifier_block *self,break ;case CPU_DYING_IDLE :/* QS for any half-done expedited RCU-sched GP. */ -rcu_sched_qs (); +preempt_disable (); +rcu_report_exp_rdp (&rcu_sched_state, +this_cpu_ptr (rcu_sched_state.rda), true); +preempt_enable ();for_each_rcu_flavor (rsp) {rcu_cleanup_dying_idle_cpu (cpu, rsp);
Signature · 1 line
2.5.2
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited()
Quoted 11 lines · Paul E. McKenney
void
rcu_sched_qs(void)
{(quoted diff) +4
-0+ unsigned long flags; + if (__this_cpu_read(rcu_sched_data.cpu_no_qs.s)) { trace_rcu_grace_period(TPS("rcu_sched"), __this_cpu_read(rcu_sched_data.gpnum), TPS("cpuqs")); __this_cpu_write(rcu_sched_data.cpu_no_qs.b.norm, false); + if (!__this_cpu_read(rcu_sched_data.cpu_no_qs.b.exp)) + return;
Review
Quoted 10 lines
(quoted diff) +2
-0+ local_irq_save(flags); if (__this_cpu_read(rcu_sched_data.cpu_no_qs.b.exp)) { __this_cpu_write(rcu_sched_data.cpu_no_qs.b.exp, false); rcu_report_exp_rdp(&rcu_sched_state, this_cpu_ptr(&rcu_sched_data), true); } + local_irq_restore(flags); } }
*sigh*.. still rare I suppose, but should we look at doing something like this?
--- kernel/sched/core.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
kernel/sched/core.c +4 -2
@@ -3050,7 +3050,6 @@ static void __sched __schedule(void) cpu =smp_processor_id (); rq =cpu_rq (cpu); -rcu_note_context_switch (); prev = rq->curr;schedule_debug (prev);
@@ -3058,13 +3057,16 @@ static void __sched __schedule(void)if (sched_feat (HRTICK ))hrtick_clear (rq); +local_irq_disable (); +rcu_note_context_switch (); +/* * Make sure that signal_pending_state()->signal_pending() below * can't be reordered with __set_current_state(TASK_INTERRUPTIBLE) * done by the caller to avoid the race with signal_wake_up(). */ smp_mb__before_spinlock (); -raw_spin_lock_irq (&rq->lock); +raw_spin_lock (&rq->lock);lockdep_pin_lock (&rq->lock); rq->clock_skip_update <<=1 ;/* promote REQ to ACT */
Re: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited()
Quoted 17 lines · Peter Zijlstra
Quoted 16 lines · Paul E. McKenney
void
rcu_sched_qs(void)
{(quoted diff) +5
-0+ unsigned long flags; + if (__this_cpu_read(rcu_sched_data.cpu_no_qs.s)) { trace_rcu_grace_period(TPS("rcu_sched"), __this_cpu_read(rcu_sched_data.gpnum), TPS("cpuqs")); __this_cpu_write(rcu_sched_data.cpu_no_qs.b.norm, false); + if (!__this_cpu_read(rcu_sched_data.cpu_no_qs.b.exp)) + return; + local_irq_save(flags); if (__this_cpu_read(rcu_sched_data.cpu_no_qs.b.exp)) { __this_cpu_write(rcu_sched_data.cpu_no_qs.b.exp, false); rcu_report_exp_rdp(&rcu_sched_state, this_cpu_ptr(&rcu_sched_data),
Quoted 8 lines
Review
Quoted 5 lines
(quoted diff) +1
-0true); } + local_irq_restore(flags); } }*sigh*.. still rare I suppose, but should we look at doing something like this?
Indeed, that approach looks better than moving rcu_note_context_switch(), which probably results in deadlocks. I will update my patch accordingly.
Thanx, Paul
Quoted 35 lines
--- kernel/sched/core.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)kernel/sched/core.c +4
-2@@ -3050,7 +3050,6 @@ static void __sched __schedule(void) cpu =smp_processor_id (); rq =cpu_rq (cpu); -rcu_note_context_switch (); prev = rq->curr;schedule_debug (prev);@@ -3058,13 +3057,16 @@ static void __sched __schedule(void)if (sched_feat (HRTICK ))hrtick_clear (rq); +local_irq_disable (); +rcu_note_context_switch (); +/* * Make sure that signal_pending_state()->signal_pending() below * can't be reordered with __set_current_state(TASK_INTERRUPTIBLE) * done by the caller to avoid the race with signal_wake_up(). */ smp_mb__before_spinlock (); -raw_spin_lock_irq (&rq->lock); +raw_spin_lock (&rq->lock);lockdep_pin_lock (&rq->lock); rq->clock_skip_update <<=1 ;/* promote REQ to ACT */
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited()
Quoted 4 lines · Paul E. McKenney
On Wed, Oct 07, 2015 at 04:26:27PM +0200, Peter Zijlstra wrote:
Indeed, that approach looks better than moving
rcu_note_context_switch(), which probably results in deadlocks. I will update my patch accordingly.
Yeah, calling rcu_note_context_switch() under the rq->lock is asking for trouble we don't need.
Thanks!
Re: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited()
Quoted 8 lines · Peter Zijlstra
Quoted 4 lines · Paul E. McKenney
On Wed, Oct 07, 2015 at 04:26:27PM +0200, Peter Zijlstra wrote:
Indeed, that approach looks better than moving
rcu_note_context_switch(), which probably results in deadlocks. I will update my patch accordingly.Yeah, calling
rcu_note_context_switch()under therq->lockis asking for trouble we don't need.
Please see below for the fixed version. Thoughts?
(Queued for 4.5, want some serious testing on this.)
Thanx, Paul
------------------------------------------------------------------------
commit 3ab3edf72a59a800e6e59ad3128f5b3b251b8962
Author: Paul E. McKenney <[email protected]> Date: Wed Oct 7 09:10:48 2015 -0700
rcu: Stop disabling interrupts in scheduler fastpaths
We need the scheduler's fastpaths to be, well, fast, and unnecessarily disabling and re-enabling interrupts is not necessarily consistent with this goal. Especially given that there are regions of the scheduler that already have interrupts disabled.
This commit therefore moves the call to rcu_note_context_switch() to one of the interrupts-disabled regions of the scheduler, and removes the now-redundant disabling and re-enabling of interrupts from rcu_note_context_switch() and the functions it calls.
[ paulmck: Shift rcu_note_context_switch() to avoid deadlock, as suggested
by Peter Zijlstra. ]
include/linux/rcutree.h +1 -1
@@ -37,7 +37,7 @@ void rcu_cpu_stall_reset(void);/* * Note a virtualization-based context switch. This is simply a * wrapper around rcu_note_context_switch(), which allows TINY_RCU -* to save a few bytes. +* to save a few bytes. The caller must have disabled interrupts. */ static inline void rcu_virt_note_context_switch (int cpu) {
kernel/rcu/tree.c +9 -6
@@ -295,17 +295,16 @@ EXPORT_PER_CPU_SYMBOL_GPL(rcu_qs_ctr);* We inform the RCU core by emulating a zero-duration dyntick-idle * period, which we in turn do by incrementing the ->dynticks counter * by two. +* +* The caller must have disabled interrupts. */ static void rcu_momentary_dyntick_idle (void ) { -unsigned long flags;struct rcu_data *rdp;struct rcu_dynticks *rdtp;int resched_mask;struct rcu_state *rsp; -local_irq_save (flags); -/* * Yes, we can lose flag-setting operations. This is OK, because * the flag will be set again after some delay.
@@ -335,13 +334,12 @@ static void rcu_momentary_dyntick_idle(void)smp_mb__after_atomic ();/* Later stuff after QS. */ break ; } -local_irq_restore (flags); }/* * Note a context switch. This is a quiescent state for RCU-sched, * and requires special handling for preemptible RCU. -* The caller must have disabled preemption. +* The caller must have disabled interrupts. */ void rcu_note_context_switch (void ) {
@@ -371,9 +369,14 @@ EXPORT_SYMBOL_GPL(rcu_note_context_switch);*/ void rcu_all_qs (void ) { +unsigned long flags; +barrier ();/* Avoid RCU read-side critical sections leaking down. */ -if (unlikely (raw_cpu_read (rcu_sched_qs_mask))) +if (unlikely (raw_cpu_read (rcu_sched_qs_mask))) { +local_irq_save (flags);rcu_momentary_dyntick_idle (); +local_irq_restore (flags); + }this_cpu_inc (rcu_qs_ctr);barrier ();/* Avoid RCU read-side critical sections leaking up. */ }
kernel/rcu/tree_plugin.h +6 -8
@@ -146,8 +146,8 @@ static void __init rcu_bootup_announce(void)* the corresponding expedited grace period will also be the end of the * normal grace period. */ -static void rcu_preempt_ctxt_queue (struct rcu_node *rnp,struct rcu_data *rdp, - unsigned long flags) __releases(rnp->lock) +static void rcu_preempt_ctxt_queue (struct rcu_node *rnp,struct rcu_data *rdp) + __releases(rnp->lock) /* But leaves rrupts disabled. */ {int blkd_state = (rnp->gp_tasks ?RCU_GP_TASKS :0 ) + (rnp->exp_tasks ? RCU_EXP_TASKS : 0) +
@@ -235,7 +235,7 @@ static void rcu_preempt_ctxt_queue(struct rcu_node *rnp, struct rcu_data *rdp, rnp->gp_tasks = &t->rcu_node_entry;if (!rnp->exp_tasks && (blkd_state &RCU_EXP_BLKD )) rnp->exp_tasks = &t->rcu_node_entry; -raw_spin_unlock (&rnp->lock); +raw_spin_unlock (&rnp->lock);/* rrupts remain disabled. */ /* * Report the quiescent state for the expedited GP. This expedited
@@ -250,7 +250,6 @@ static void rcu_preempt_ctxt_queue(struct rcu_node *rnp, struct rcu_data *rdp, }else {WARN_ON_ONCE (t->rcu_read_unlock_special.b.exp_need_qs); } -local_irq_restore (flags); }/*
@@ -285,12 +284,11 @@ static void rcu_preempt_qs(void)* predating the current grace period drain, in other words, until * rnp->gp_tasks becomes NULL. * -* Caller must disable preemption. +* Caller must disable interrupts. */ static void rcu_preempt_note_context_switch (void ) {struct task_struct *t = current; -unsigned long flags;struct rcu_data *rdp;struct rcu_node *rnp;
@@ -300,7 +298,7 @@ static void rcu_preempt_note_context_switch(void)/* Possibly blocking in an RCU read-side critical section. */ rdp =this_cpu_ptr (rcu_state_p->rda); rnp = rdp->mynode; -raw_spin_lock_irqsave (&rnp->lock, flags); +raw_spin_lock (&rnp->lock);/* rrupts already disabled. */ smp_mb__after_unlock_lock (); t->rcu_read_unlock_special.b.blocked = true; t->rcu_blocked_node = rnp;
@@ -317,7 +315,7 @@ static void rcu_preempt_note_context_switch(void) (rnp->qsmask & rdp->grpmask) ? rnp->gpnum : rnp->gpnum + 1); -rcu_preempt_ctxt_queue (rnp, rdp, flags); +rcu_preempt_ctxt_queue (rnp, rdp); }else if (t->rcu_read_lock_nesting <0 && t->rcu_read_unlock_special.s) {
kernel/sched/core.c +4 -2
@@ -3056,7 +3056,6 @@ static void __sched __schedule(void) cpu =smp_processor_id (); rq =cpu_rq (cpu); -rcu_note_context_switch (); prev = rq->curr;schedule_debug (prev);
@@ -3064,13 +3063,16 @@ static void __sched __schedule(void)if (sched_feat (HRTICK ))hrtick_clear (rq); +local_irq_disable (); +rcu_note_context_switch (); +/* * Make sure that signal_pending_state()->signal_pending() below * can't be reordered with __set_current_state(TASK_INTERRUPTIBLE) * done by the caller to avoid the race with signal_wake_up(). */ smp_mb__before_spinlock (); -raw_spin_lock_irq (&rq->lock); +raw_spin_lock (&rq->lock);lockdep_pin_lock (&rq->lock); rq->clock_skip_update <<=1 ;/* promote REQ to ACT */
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited()
Review
Quoted 5 lines · Paul E. McKenney
Please see below for the fixed version. Thoughts?
(quoted diff) +3
-0+ __releases(rnp->lock) /* But leaves rrupts disabled. */ + raw_spin_unlock(&rnp->lock); /* rrupts remain disabled. */ + raw_spin_lock(&rnp->lock); /* rrupts already disabled. */
What them 'rrupts' about? ;-)
Re: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited()
Quoted 8 lines · Peter Zijlstra
Review
Quoted 5 lines · Paul E. McKenney
Please see below for the fixed version. Thoughts?
(quoted diff) +3
-0+ __releases(rnp->lock) /* But leaves rrupts disabled. */ + raw_spin_unlock(&rnp->lock); /* rrupts remain disabled. */ + raw_spin_lock(&rnp->lock); /* rrupts already disabled. */What them 'rrupts' about? ;-)
Interrupts when it won't fit. I suppose I could use IRQs instead. ;-)
Thanx, Paul
Josh Triplett
maintainerRe: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited()
Quoted 11 lines · Paul E. McKenney
Quoted 8 lines · Peter Zijlstra
Review
Quoted 5 lines · Paul E. McKenney
Please see below for the fixed version. Thoughts?
(quoted diff) +3
-0+ __releases(rnp->lock) /* But leaves rrupts disabled. */ + raw_spin_unlock(&rnp->lock); /* rrupts remain disabled. */ + raw_spin_lock(&rnp->lock); /* rrupts already disabled. */What them 'rrupts' about? ;-)
Interrupts when it won't fit. I suppose I could use IRQs instead. ;-)
In this particular case, "IRQs" works just as well; however, in general, this seems like an excellent example of when to ignore the 80-column guideline. :)
- Josh Triplett
Re: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited()
Quoted 11 lines · Josh Triplett
Quoted 10 lines · Paul E. McKenney
Quoted 8 lines · Peter Zijlstra
Review
Quoted 5 lines · Paul E. McKenney
Please see below for the fixed version. Thoughts?
(quoted diff) +3
-0+ __releases(rnp->lock) /* But leaves rrupts disabled. */ + raw_spin_unlock(&rnp->lock); /* rrupts remain disabled. */ + raw_spin_lock(&rnp->lock); /* rrupts already disabled. */What them 'rrupts' about? ;-)
Quoted 5 lines
Quoted 1 line
Interrupts when it won't fit. I suppose I could use IRQs instead. ;-)
In this particular case, "IRQs" works just as well; however, in general, this seems like an excellent example of when to ignore the 80-column guideline. :)
But but but... You are talking to someone who used actual PUNCHED CARDS
in real life in a paying job!!! ;-)
Thanx, Paul
Josh Triplett
maintainerRe: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited()
Quoted 12 lines · Paul E. McKenney
Quoted 11 lines · Josh Triplett
Quoted 10 lines · Paul E. McKenney
Quoted 8 lines · Peter Zijlstra
Review
Quoted 5 lines · Paul E. McKenney
Please see below for the fixed version. Thoughts?
(quoted diff) +3
-0+ __releases(rnp->lock) /* But leaves rrupts disabled. */ + raw_spin_unlock(&rnp->lock); /* rrupts remain disabled. */ + raw_spin_lock(&rnp->lock); /* rrupts already disabled. */What them 'rrupts' about? ;-)
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Interrupts when it won't fit. I suppose I could use IRQs instead. ;-)
In this particular case, "IRQs" works just as well; however, in general, this seems like an excellent example of when to ignore the 80-column guideline. :)
But but but... You are talking to someone who used actual PUNCHED CARDS
in real life in a paying job!!! ;-)
And I learned on a DOS system with 80x25 text mode. Let us revel in wonderment at the capabilities of modern systems. :)
- Josh Triplett
Re: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited()
Quoted 19 lines · Josh Triplett
Quoted 18 lines · Paul E. McKenney
Quoted 16 lines · Josh Triplett
Quoted 11 lines · Paul E. McKenney
Quoted 8 lines · Peter Zijlstra
Review
Quoted 5 lines · Paul E. McKenney
Please see below for the fixed version. Thoughts?
(quoted diff) +3
-0+ __releases(rnp->lock) /* But leaves rrupts disabled. */ + raw_spin_unlock(&rnp->lock); /* rrupts remain disabled. */ + raw_spin_lock(&rnp->lock); /* rrupts already disabled. */What them 'rrupts' about? ;-)
Interrupts when it won't fit. I suppose I could use IRQs instead. ;-)
In this particular case, "IRQs" works just as well; however, in general, this seems like an excellent example of when to ignore the 80-column guideline. :)
Quoted 5 lines
Quoted 2 lines
But but but... You are talking to someone who used actual PUNCHED CARDS
in real life in a paying job!!! ;-)And I learned on a DOS system with 80x25 text mode. Let us revel in wonderment at the capabilities of modern systems. :)
But of course! On my new 2880x1620 screen, I can put four 80x24 xterms on each row!
Too bad that I cannot actually read them without an external monitor, and my current external monitors are only 1920x1600. ;-)
Thanx, Paul