Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1240676 > unrolled thread
| Started by | "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> |
|---|---|
| First post | 2015-10-06 18:30 +0200 |
| Last post | 2015-10-06 18:40 +0200 |
| Articles | 20 on this page of 66 — 6 participants |
Back to article view | Back to linux.kernel
[PATCH tip/core/rcu 0/18] Expedited grace-period improvements for 4.4 "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:30 +0200
[PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:40 +0200
Re: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited() Peter Zijlstra <peterz@infradead.org> - 2015-10-07 16:30 +0200
Re: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-07 18:30 +0200
Re: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited() Peter Zijlstra <peterz@infradead.org> - 2015-10-08 11:10 +0200
Re: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-08 17:10 +0200
Re: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited() Peter Zijlstra <peterz@infradead.org> - 2015-10-08 17:20 +0200
Re: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-08 17:20 +0200
Re: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited() Josh Triplett <josh@joshtriplett.org> - 2015-10-08 20:10 +0200
Re: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-09 02:20 +0200
Re: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited() Josh Triplett <josh@joshtriplett.org> - 2015-10-09 02:50 +0200
Re: [PATCH tip/core/rcu 18/18] rcu: Better hotplug handling for synchronize_sched_expedited() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-09 06:00 +0200
[PATCH tip/core/rcu 10/18] rcu: Stop silencing lockdep false positive for expedited grace periods "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:40 +0200
[PATCH tip/core/rcu 08/18] rcu: Make ->cpu_no_qs be a union for aggregate OR "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:40 +0200
[PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:40 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation Peter Zijlstra <peterz@infradead.org> - 2015-10-06 22:30 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 23:00 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation Peter Zijlstra <peterz@infradead.org> - 2015-10-07 10:00 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2015-10-07 10:50 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation Peter Zijlstra <peterz@infradead.org> - 2015-10-07 13:10 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation Peter Zijlstra <peterz@infradead.org> - 2015-10-07 14:00 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation Peter Zijlstra <peterz@infradead.org> - 2015-10-07 14:10 +0200
Re: Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation kbuild test robot <lkp@intel.com> - 2015-10-07 14:10 +0200
Re: Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation kbuild test robot <lkp@intel.com> - 2015-10-07 14:10 +0200
Re: Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation Peter Zijlstra <peterz@infradead.org> - 2015-10-07 14:20 +0200
Re: [kbuild-all] [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation Fengguang Wu <lkp@intel.com> - 2015-10-07 15:50 +0200
Re: [kbuild-all] [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation Peter Zijlstra <peterz@infradead.org> - 2015-10-07 16:00 +0200
Re: [kbuild-all] [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation Peter Zijlstra <peterz@infradead.org> - 2015-10-07 16:30 +0200
Re: [kbuild-all] [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation Fengguang Wu <lkp@intel.com> - 2015-10-07 16:30 +0200
Re: Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation kbuild test robot <lkp@intel.com> - 2015-10-07 14:20 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-07 17:20 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation Peter Zijlstra <peterz@infradead.org> - 2015-10-08 12:30 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-07 17:20 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-07 16:40 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation Peter Zijlstra <peterz@infradead.org> - 2015-10-07 16:50 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-07 18:50 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation Peter Zijlstra <peterz@infradead.org> - 2015-10-08 11:50 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-08 17:40 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation Peter Zijlstra <peterz@infradead.org> - 2015-10-08 19:20 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-08 19:50 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-09 02:20 +0200
Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation Peter Zijlstra <peterz@infradead.org> - 2015-10-09 10:50 +0200
[PATCH tip/core/rcu 05/18] rcu: Move synchronize_sched_expedited() to combining tree "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:40 +0200
[PATCH tip/core/rcu 16/18] rcu: Add tasks to expedited stall-warning messages "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:40 +0200
[PATCH tip/core/rcu 12/18] cpu: Remove try_get_online_cpus() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:40 +0200
[PATCH tip/core/rcu 03/18] rcu: Consolidate tree setup for synchronize_rcu_expedited() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:40 +0200
[PATCH tip/core/rcu 13/18] rcu: Prepare for consolidating expedited CPU selection "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:40 +0200
[PATCH tip/core/rcu 07/18] rcu: Invert passed_quiesce and rename to cpu_no_qs "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:40 +0200
[PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:40 +0200
Re: [PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period Peter Zijlstra <peterz@infradead.org> - 2015-10-07 15:30 +0200
Re: [PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-07 20:20 +0200
Re: [PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period Peter Zijlstra <peterz@infradead.org> - 2015-10-07 15:40 +0200
Re: [PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-07 17:50 +0200
Re: [PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period Peter Zijlstra <peterz@infradead.org> - 2015-10-07 15:50 +0200
Re: [PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-07 18:20 +0200
Re: [PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period Peter Zijlstra <peterz@infradead.org> - 2015-10-08 11:10 +0200
Re: [PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period Peter Zijlstra <peterz@infradead.org> - 2015-10-07 15:50 +0200
Re: [PATCH tip/core/rcu 04/18] rcu: Use single-stage IPI algorithm for RCU expedited grace period "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-07 18:20 +0200
[PATCH tip/core/rcu 09/18] rcu: Switch synchronize_sched_expedited() to IPI "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:40 +0200
Re: [PATCH tip/core/rcu 09/18] rcu: Switch synchronize_sched_expedited() to IPI Peter Zijlstra <peterz@infradead.org> - 2015-10-07 16:20 +0200
Re: [PATCH tip/core/rcu 09/18] rcu: Switch synchronize_sched_expedited() to IPI "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-07 18:30 +0200
[PATCH tip/core/rcu 15/18] rcu: Add online/offline info to expedited stall warning message "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:40 +0200
[PATCH tip/core/rcu 14/18] rcu: Consolidate expedited CPU selection "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:40 +0200
[PATCH tip/core/rcu 01/18] rcu: Use rsp->expedited_wq instead of sync_rcu_preempt_exp_wq "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:40 +0200
[PATCH tip/core/rcu 06/18] rcu: Rename qs_pending to core_needs_qs "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:40 +0200
[PATCH tip/core/rcu 17/18] rcu: Enable stall warnings for synchronize_rcu_expedited() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-06 18:40 +0200
Page 2 of 4 — ← Prev page 1 [2] 3 4 Next page →
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-10-07 14:00 +0200 |
| Subject | Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qgVy2-3SB-17@gated-at.bofh.it> |
| In reply to | #1241372 |
On Wed, Oct 07, 2015 at 01:01:20PM +0200, Peter Zijlstra wrote:
> 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(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 775d36cc0050..46e1e23ff762 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -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;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-10-07 14:10 +0200 |
| Subject | Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qgVHI-4l3-13@gated-at.bofh.it> |
| In reply to | #1241410 |
On Wed, Oct 07, 2015 at 01:50:46PM +0200, Peter Zijlstra wrote:
> @@ -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
> @@ -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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | kbuild test robot <lkp@intel.com> |
|---|---|
| Date | 2015-10-07 14:10 +0200 |
| Subject | Re: Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qgVHI-4l3-5@gated-at.bofh.it> |
| In reply to | #1241410 |
[Multipart message — attachments visible in raw view] — view raw
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
[toc] | [prev] | [next] | [standalone]
| From | kbuild test robot <lkp@intel.com> |
|---|---|
| Date | 2015-10-07 14:10 +0200 |
| Subject | Re: Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qgVHJ-4l3-21@gated-at.bofh.it> |
| In reply to | #1241410 |
[Multipart message — attachments visible in raw view] — view raw
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
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-10-07 14:20 +0200 |
| Subject | Re: Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qgVRn-4wo-7@gated-at.bofh.it> |
| In reply to | #1241410 |
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? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Fengguang Wu <lkp@intel.com> |
|---|---|
| Date | 2015-10-07 15:50 +0200 |
| Subject | Re: [kbuild-all] [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qgXgu-6rH-23@gated-at.bofh.it> |
| In reply to | #1241420 |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-10-07 16:00 +0200 |
| Subject | Re: [kbuild-all] [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qgXqb-6Da-39@gated-at.bofh.it> |
| In reply to | #1241512 |
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 :-) > 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-10-07 16:30 +0200 |
| Subject | Re: [kbuild-all] [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qgXTc-7qA-19@gated-at.bofh.it> |
| In reply to | #1241528 |
On Wed, Oct 07, 2015 at 10:21:33PM +0800, Fengguang Wu wrote: > On Wed, Oct 07, 2015 at 03:55:29PM +0200, Peter Zijlstra wrote: > > 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. Works for me; now hoping I will abide by my own suggested rules ;-) Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Fengguang Wu <lkp@intel.com> |
|---|---|
| Date | 2015-10-07 16:30 +0200 |
| Subject | Re: [kbuild-all] [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qgXTc-7qA-21@gated-at.bofh.it> |
| In reply to | #1241528 |
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | kbuild test robot <lkp@intel.com> |
|---|---|
| Date | 2015-10-07 14:20 +0200 |
| Subject | Re: Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qgVRn-4wo-9@gated-at.bofh.it> |
| In reply to | #1241410 |
[Multipart message — attachments visible in raw view] — view raw
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 <paulmck@linux.vnet.ibm.com>
:::::: CC: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[toc] | [prev] | [next] | [standalone]
| From | "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> |
|---|---|
| Date | 2015-10-07 17:20 +0200 |
| Subject | Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qgYFz-9b-1@gated-at.bofh.it> |
| In reply to | #1241410 |
On Wed, Oct 07, 2015 at 01:50:46PM +0200, Peter Zijlstra wrote:
> On Wed, Oct 07, 2015 at 01:01:20PM +0200, Peter Zijlstra wrote:
>
> > 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
> ---
> kernel/rcu/tree.c | 99 ++++++++++++++++++++++++++++++++++---------------------
> 1 file changed, 61 insertions(+), 38 deletions(-)
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 775d36cc0050..46e1e23ff762 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -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;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-10-08 12:30 +0200 |
| Subject | Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qhgCv-FB-25@gated-at.bofh.it> |
| In reply to | #1241586 |
On Wed, Oct 07, 2015 at 08:18:29AM -0700, Paul E. McKenney wrote:
> 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.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
kernel/rcu/tree.c | 128 ++++++++++++++++++++++++++++-------------------
kernel/rcu/tree.h | 11 ----
kernel/rcu/tree_plugin.h | 18 +++----
3 files changed, 82 insertions(+), 75 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index b7cd210f3b1e..6ee3a6ffcc27 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -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)
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index 9fb4e238d4dc..1d2eb0859f70 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -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 */
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 630c19772630..fa0e3b96a9ed 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -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)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> |
|---|---|
| Date | 2015-10-07 17:20 +0200 |
| Subject | Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qgYFA-9b-3@gated-at.bofh.it> |
| In reply to | #1241372 |
On Wed, Oct 07, 2015 at 01:01:20PM +0200, Peter Zijlstra wrote:
> On Wed, Oct 07, 2015 at 08:42:05AM +0000, Mathieu Desnoyers wrote:
> > ----- On Oct 7, 2015, at 3:51 AM, Peter Zijlstra peterz@infradead.org wrote:
> >
> > > On Tue, Oct 06, 2015 at 01:58:50PM -0700, Paul E. McKenney wrote:
> > >> On Tue, Oct 06, 2015 at 10:29:37PM +0200, Peter Zijlstra wrote:
> > >> > On Tue, Oct 06, 2015 at 09:29:21AM -0700, Paul E. McKenney wrote:
> > >> > > +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.. :-)
> > >> >
> > >> > > + 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.. :/
> > >
> > >> 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_
> > > :-)
> >
> > 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.
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> |
|---|---|
| Date | 2015-10-07 16:40 +0200 |
| Subject | Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qgY2R-7BV-7@gated-at.bofh.it> |
| In reply to | #1241208 |
On Wed, Oct 07, 2015 at 09:51:14AM +0200, Peter Zijlstra wrote:
> On Tue, Oct 06, 2015 at 01:58:50PM -0700, Paul E. McKenney wrote:
> > On Tue, Oct 06, 2015 at 10:29:37PM +0200, Peter Zijlstra wrote:
> > > On Tue, Oct 06, 2015 at 09:29:21AM -0700, Paul E. McKenney wrote:
> > > > +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.. :-)
> > >
> > > > + 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...
> > 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_
> :-)
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.
> > 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!
> 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.
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-10-07 16:50 +0200 |
| Subject | Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qgYcy-7Ng-7@gated-at.bofh.it> |
| In reply to | #1241556 |
On Wed, Oct 07, 2015 at 07:33:25AM -0700, Paul E. McKenney wrote:
> > 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.
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> |
|---|---|
| Date | 2015-10-07 18:50 +0200 |
| Subject | Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qh04G-25n-5@gated-at.bofh.it> |
| In reply to | #1241569 |
On Wed, Oct 07, 2015 at 04:40:24PM +0200, Peter Zijlstra wrote:
> On Wed, Oct 07, 2015 at 07:33:25AM -0700, Paul E. McKenney wrote:
> > > 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.
>
> 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.
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-10-08 11:50 +0200 |
| Subject | Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qhfZL-88e-1@gated-at.bofh.it> |
| In reply to | #1241679 |
On Wed, Oct 07, 2015 at 09:48:58AM -0700, Paul E. McKenney wrote: > > 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. If the kthread took _all_ rcu_node locks, then this does not require the barrier upgrade because they will share a lock variable. > 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. This again reads like the same lock variable is involved, and therefore the barrier upgrade is not required for this. > 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. same lock var again.. > 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. And here we iterate the tree and get another lock var involved, here the barrier upgrade will actually do something. > o And so on, up the tree. idem.. > 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. Right, which makes the full barrier transitivity thing important > 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... Again, if it takes _all_ rcu_nodes, it also shares a lock variable and hence the upgrade is not required. > 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. Does also not seem to require the upgrade.. > 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. > 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> |
|---|---|
| Date | 2015-10-08 17:40 +0200 |
| Subject | Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qhlsu-7EU-13@gated-at.bofh.it> |
| In reply to | #1242122 |
On Thu, Oct 08, 2015 at 11:49:33AM +0200, Peter Zijlstra wrote: > On Wed, Oct 07, 2015 at 09:48:58AM -0700, Paul E. McKenney wrote: > > > > 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. > > If the kthread took _all_ rcu_node locks, then this does not require the > barrier upgrade because they will share a lock variable. > > > 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. > > This again reads like the same lock variable is involved, and therefore > the barrier upgrade is not required for this. > > > 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. > > same lock var again.. > > > 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. > > 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! ;-) > > o And so on, up the tree. > > idem.. > > > 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. > > Right, which makes the full barrier transitivity thing important > > > 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... > > Again, if it takes _all_ rcu_nodes, it also shares a lock variable and > hence the upgrade is not required. > > > 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. > > Does also not seem to require the upgrade.. > > > 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. 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. > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-10-08 19:20 +0200 |
| Subject | Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qhn1f-1wq-3@gated-at.bofh.it> |
| In reply to | #1242536 |
On Thu, Oct 08, 2015 at 08:33:51AM -0700, Paul E. McKenney wrote: > > > 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. > > > > 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! ;-) 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. > 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'. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> |
|---|---|
| Date | 2015-10-08 19:50 +0200 |
| Subject | Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation |
| Message-ID | <qhnui-24d-13@gated-at.bofh.it> |
| In reply to | #1242620 |
On Thu, Oct 08, 2015 at 07:12:03PM +0200, Peter Zijlstra wrote: > On Thu, Oct 08, 2015 at 08:33:51AM -0700, Paul E. McKenney wrote: > > > > > 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. > > > > > > 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! ;-) > > 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. > > > 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'. Which I need to fix, thank you. Thanx, Paul -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
Page 2 of 4 — ← Prev page 1 [2] 3 4 Next page →
Back to top | Article view | linux.kernel
csiph-web