public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking
@ 2009-12-15 23:02 Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 01/18] rcu: adjust force_quiescent_state() locking, step 1 Paul E. McKenney
                   ` (17 more replies)
  0 siblings, 18 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells

Hello!

This is just a sneak preview of RCU patches that I have in flight, not
yet for inclusion.

Patches 1-10 simplify TREE_RCU's (and TREE_PREEMPT_RCU's) race conditions
by prohibiting grace periods from starting while force_quiescent_state()
is active.  Patch 10 maintains grace-period latency by the simple
expedient of having force_quiescent_state() check to see if someone
wanted to start a grace period, and, if so, starting one on their behalf.

Patch 11 adds parameters to rcutorture to allow testing with extremely
rapid re-invocations of force_quiescent_state() -- microseconds rather
than the previous milliseconds.

Patch 12 makes the RCU and rcutorture entries of the MAINTAINERS file
call out the correct source files, which have changed due to the recent
re-implementation.

Patches 13-18 add debugging checks, so that an rcu_dereference(p)
can now be rcu_dereference(p, rcu_read_lock_held()), which
will complain if invoked outside of an RCU read-side critical
section when CONFIG_PROVE_LOCKING is specified.  There are
also rcu_read_lock_bh_held(), rcu_read_lock_sched_held(), and
srcu_read_lock_held() for the other flavors of RCU.

One can also do something like:

	p = rcu_dereference_check(gp, rcu_read_lock_bh_held() ||
				  lockdep_is_held(my_lock));

to handle the case where either the access must either be protected
by RCU-bh or by my_lock.

Again, strictly FYI, not yet for inclusion.

							Thanx, Paul

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH RFC tip/core/rcu 01/18] rcu: adjust force_quiescent_state() locking, step 1
  2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
@ 2009-12-15 23:02 ` Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 02/18] rcu: adjust force_quiescent_state() locking, step 2 Paul E. McKenney
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	Paul E. McKenney

Not yet for inclusion, proposing for 2.6.34.

This causes rnp->lock to be held on entry to force_quiescent_state()'s
switch statement.  This is a first step towards prohibiting starting
grace periods while force_quiescent_state() is executing, which will
reduce the number and complexity of races that force_quiescent_state()
is involved in.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |   27 ++++++++++++++++++---------
 1 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index 53ae959..eae331d 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -1204,7 +1204,7 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 	}
 	if (relaxed &&
 	    (long)(rsp->jiffies_force_qs - jiffies) >= 0)
-		goto unlock_ret; /* no emergency and done recently. */
+		goto unlock_fqs_ret; /* no emergency and done recently. */
 	rsp->n_force_qs++;
 	spin_lock(&rnp->lock);
 	lastcomp = rsp->gpnum - 1;
@@ -1213,31 +1213,32 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 	if(!rcu_gp_in_progress(rsp)) {
 		rsp->n_force_qs_ngp++;
 		spin_unlock(&rnp->lock);
-		goto unlock_ret;  /* no GP in progress, time updated. */
+		goto unlock_fqs_ret;  /* no GP in progress, time updated. */
 	}
-	spin_unlock(&rnp->lock);
 	switch (signaled) {
 	case RCU_GP_IDLE:
 	case RCU_GP_INIT:
 
+		spin_unlock(&rnp->lock);
 		break; /* grace period idle or initializing, ignore. */
 
 	case RCU_SAVE_DYNTICK:
 
+		spin_unlock(&rnp->lock);
 		if (RCU_SIGNAL_INIT != RCU_SAVE_DYNTICK)
 			break; /* So gcc recognizes the dead code. */
 
 		/* Record dyntick-idle state. */
 		if (rcu_process_dyntick(rsp, lastcomp,
 					dyntick_save_progress_counter))
-			goto unlock_ret;
+			goto unlock_fqs_ret;
+		spin_lock(&rnp->lock);
 		/* fall into next case. */
 
 	case RCU_SAVE_COMPLETED:
 
 		/* Update state, record completion counter. */
 		forcenow = 0;
-		spin_lock(&rnp->lock);
 		if (lastcomp + 1 == rsp->gpnum &&
 		    lastcomp == rsp->completed &&
 		    rsp->signaled == signaled) {
@@ -1245,23 +1246,31 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 			rsp->completed_fqs = lastcomp;
 			forcenow = signaled == RCU_SAVE_COMPLETED;
 		}
-		spin_unlock(&rnp->lock);
-		if (!forcenow)
+		if (!forcenow) {
+			spin_unlock(&rnp->lock);
 			break;
+		}
 		/* fall into next case. */
 
 	case RCU_FORCE_QS:
 
 		/* Check dyntick-idle state, send IPI to laggarts. */
+		spin_unlock(&rnp->lock);
 		if (rcu_process_dyntick(rsp, rsp->completed_fqs,
 					rcu_implicit_dynticks_qs))
-			goto unlock_ret;
+			goto unlock_fqs_ret;
 
 		/* Leave state in case more forcing is required. */
 
 		break;
+
+	default:
+
+		spin_unlock(&rnp->lock);
+		WARN_ON_ONCE(1);
+		break;
 	}
-unlock_ret:
+unlock_fqs_ret:
 	spin_unlock_irqrestore(&rsp->fqslock, flags);
 }
 
-- 
1.5.2.5


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC tip/core/rcu 02/18] rcu: adjust force_quiescent_state() locking, step 2
  2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 01/18] rcu: adjust force_quiescent_state() locking, step 1 Paul E. McKenney
@ 2009-12-15 23:02 ` Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 03/18] rcu: prohibit starting new grace periods while forcing quiescent states Paul E. McKenney
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	Paul E. McKenney

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Proposed for 2.6.34, not for inclusion.

This patch releases rnp->lock after the end of force_quiescent_state()'s
switch statement.  This is a second step towards prohibiting starting
grace periods while force_quiescent_state() is executing, which will
reduce the number and complexity of races that force_quiescent_state()
is involved in.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |   13 +++----------
 1 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index eae331d..d42ad30 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -1219,7 +1219,6 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 	case RCU_GP_IDLE:
 	case RCU_GP_INIT:
 
-		spin_unlock(&rnp->lock);
 		break; /* grace period idle or initializing, ignore. */
 
 	case RCU_SAVE_DYNTICK:
@@ -1246,10 +1245,8 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 			rsp->completed_fqs = lastcomp;
 			forcenow = signaled == RCU_SAVE_COMPLETED;
 		}
-		if (!forcenow) {
-			spin_unlock(&rnp->lock);
+		if (!forcenow)
 			break;
-		}
 		/* fall into next case. */
 
 	case RCU_FORCE_QS:
@@ -1262,14 +1259,10 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 
 		/* Leave state in case more forcing is required. */
 
-		break;
-
-	default:
-
-		spin_unlock(&rnp->lock);
-		WARN_ON_ONCE(1);
+		spin_lock(&rnp->lock);
 		break;
 	}
+	spin_unlock(&rnp->lock);
 unlock_fqs_ret:
 	spin_unlock_irqrestore(&rsp->fqslock, flags);
 }
-- 
1.5.2.5


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC tip/core/rcu 03/18] rcu: prohibit starting new grace periods while forcing quiescent states
  2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 01/18] rcu: adjust force_quiescent_state() locking, step 1 Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 02/18] rcu: adjust force_quiescent_state() locking, step 2 Paul E. McKenney
@ 2009-12-15 23:02 ` Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 04/18] rcu: eliminate local variable signaled from force_quiescent_state() Paul E. McKenney
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	Paul E. McKenney

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Proposed for 2.6.34, not for inclusion.

Reduce the number and variety of race conditions by prohibiting the
start of a new grace period while force_quiescent_state() is active.
A new fqs_active flag in the rcu_state structure is used to trace
whether or not force_quiescent_state() is active, and this new flag
is tested by rcu_start_gp().  If the CPU that closed out the last
grace period needs another grace period, this new grace period may
be delayed up to one scheduling-clock tick, but it will eventually
get started.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |   31 +++++++++++++++++--------------
 kernel/rcutree.h |    2 ++
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index d42ad30..41688ff 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -659,7 +659,7 @@ rcu_start_gp(struct rcu_state *rsp, unsigned long flags)
 	struct rcu_data *rdp = rsp->rda[smp_processor_id()];
 	struct rcu_node *rnp = rcu_get_root(rsp);
 
-	if (!cpu_needs_another_gp(rsp, rdp)) {
+	if (!cpu_needs_another_gp(rsp, rdp) || rsp->fqs_active) {
 		if (rnp->completed == rsp->completed) {
 			spin_unlock_irqrestore(&rnp->lock, flags);
 			return;
@@ -1195,6 +1195,7 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 	struct rcu_node *rnp = rcu_get_root(rsp);
 	u8 signaled;
 	u8 forcenow;
+	u8 gpdone;
 
 	if (!rcu_gp_in_progress(rsp))
 		return;  /* No grace period in progress, nothing to force. */
@@ -1206,15 +1207,16 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 	    (long)(rsp->jiffies_force_qs - jiffies) >= 0)
 		goto unlock_fqs_ret; /* no emergency and done recently. */
 	rsp->n_force_qs++;
-	spin_lock(&rnp->lock);
+	spin_lock(&rnp->lock);  /* irqs already disabled */
 	lastcomp = rsp->gpnum - 1;
 	signaled = rsp->signaled;
 	rsp->jiffies_force_qs = jiffies + RCU_JIFFIES_TILL_FORCE_QS;
 	if(!rcu_gp_in_progress(rsp)) {
 		rsp->n_force_qs_ngp++;
-		spin_unlock(&rnp->lock);
+		spin_unlock(&rnp->lock);  /* irqs remain disabled */
 		goto unlock_fqs_ret;  /* no GP in progress, time updated. */
 	}
+	rsp->fqs_active = 1;
 	switch (signaled) {
 	case RCU_GP_IDLE:
 	case RCU_GP_INIT:
@@ -1223,15 +1225,16 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 
 	case RCU_SAVE_DYNTICK:
 
-		spin_unlock(&rnp->lock);
+		spin_unlock(&rnp->lock);  /* irqs remain disabled */
 		if (RCU_SIGNAL_INIT != RCU_SAVE_DYNTICK)
 			break; /* So gcc recognizes the dead code. */
 
 		/* Record dyntick-idle state. */
-		if (rcu_process_dyntick(rsp, lastcomp,
-					dyntick_save_progress_counter))
-			goto unlock_fqs_ret;
-		spin_lock(&rnp->lock);
+		gpdone = rcu_process_dyntick(rsp, lastcomp,
+					     dyntick_save_progress_counter);
+		spin_lock(&rnp->lock);  /* irqs already disabled */
+		if (gpdone)
+			break;
 		/* fall into next case. */
 
 	case RCU_SAVE_COMPLETED:
@@ -1252,17 +1255,17 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 	case RCU_FORCE_QS:
 
 		/* Check dyntick-idle state, send IPI to laggarts. */
-		spin_unlock(&rnp->lock);
-		if (rcu_process_dyntick(rsp, rsp->completed_fqs,
-					rcu_implicit_dynticks_qs))
-			goto unlock_fqs_ret;
+		spin_unlock(&rnp->lock);  /* irqs remain disabled */
+		gpdone = rcu_process_dyntick(rsp, rsp->completed_fqs,
+					     rcu_implicit_dynticks_qs);
 
 		/* Leave state in case more forcing is required. */
 
-		spin_lock(&rnp->lock);
+		spin_lock(&rnp->lock);  /* irqs already disabled */
 		break;
 	}
-	spin_unlock(&rnp->lock);
+	rsp->fqs_active = 0;
+	spin_unlock(&rnp->lock);  /* irqs remain disabled */
 unlock_fqs_ret:
 	spin_unlock_irqrestore(&rsp->fqslock, flags);
 }
diff --git a/kernel/rcutree.h b/kernel/rcutree.h
index d2a0046..dc386a7 100644
--- a/kernel/rcutree.h
+++ b/kernel/rcutree.h
@@ -277,6 +277,8 @@ struct rcu_state {
 
 	u8	signaled ____cacheline_internodealigned_in_smp;
 						/* Force QS state. */
+	u8	fqs_active;			/* force_quiescent_state() */
+						/*  is running. */
 	long	gpnum;				/* Current gp number. */
 	long	completed;			/* # of last completed gp. */
 
-- 
1.5.2.5


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC tip/core/rcu 04/18] rcu: eliminate local variable signaled from force_quiescent_state()
  2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
                   ` (2 preceding siblings ...)
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 03/18] rcu: prohibit starting new grace periods while forcing quiescent states Paul E. McKenney
@ 2009-12-15 23:02 ` Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 05/18] rcu: eliminate local variable lastcomp " Paul E. McKenney
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	Paul E. McKenney

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Proposed for 2.6.34, not for inclusion.

Because the root rcu_node lock is held across entry to the switch
statement in force_quiescent_state(), it is no longer necessary
to snapshot rsp->signaled to a local variable.  Eliminate both the
snapshotting and the local variable.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index 41688ff..1d8cfb1 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -1193,7 +1193,6 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 	unsigned long flags;
 	long lastcomp;
 	struct rcu_node *rnp = rcu_get_root(rsp);
-	u8 signaled;
 	u8 forcenow;
 	u8 gpdone;
 
@@ -1209,7 +1208,6 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 	rsp->n_force_qs++;
 	spin_lock(&rnp->lock);  /* irqs already disabled */
 	lastcomp = rsp->gpnum - 1;
-	signaled = rsp->signaled;
 	rsp->jiffies_force_qs = jiffies + RCU_JIFFIES_TILL_FORCE_QS;
 	if(!rcu_gp_in_progress(rsp)) {
 		rsp->n_force_qs_ngp++;
@@ -1217,7 +1215,7 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 		goto unlock_fqs_ret;  /* no GP in progress, time updated. */
 	}
 	rsp->fqs_active = 1;
-	switch (signaled) {
+	switch (rsp->signaled) {
 	case RCU_GP_IDLE:
 	case RCU_GP_INIT:
 
@@ -1242,11 +1240,10 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 		/* Update state, record completion counter. */
 		forcenow = 0;
 		if (lastcomp + 1 == rsp->gpnum &&
-		    lastcomp == rsp->completed &&
-		    rsp->signaled == signaled) {
+		    lastcomp == rsp->completed) {
+			forcenow = rsp->signaled == RCU_SAVE_COMPLETED;
 			rsp->signaled = RCU_FORCE_QS;
 			rsp->completed_fqs = lastcomp;
-			forcenow = signaled == RCU_SAVE_COMPLETED;
 		}
 		if (!forcenow)
 			break;
-- 
1.5.2.5


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC tip/core/rcu 05/18] rcu: eliminate local variable lastcomp from force_quiescent_state()
  2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
                   ` (3 preceding siblings ...)
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 04/18] rcu: eliminate local variable signaled from force_quiescent_state() Paul E. McKenney
@ 2009-12-15 23:02 ` Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 06/18] rcu: eliminate second argument of rcu_process_dyntick() Paul E. McKenney
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	Paul E. McKenney

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Proposed for 2.6.34, not for inclusion.

Because rsp->fqs_active is set to 1 across force_quiescent_state()'s
switch statement, rcu_start_gp() will refrain from starting a new
grace period during this time.  Therefore, rsp->gpnum is constant,
and can be propagated to all uses of lastcomp, eliminating this local
variable.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |   10 +++-------
 kernel/rcutree.h |    2 --
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index 1d8cfb1..62b6433 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -1191,7 +1191,6 @@ static int rcu_process_dyntick(struct rcu_state *rsp, long lastcomp,
 static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 {
 	unsigned long flags;
-	long lastcomp;
 	struct rcu_node *rnp = rcu_get_root(rsp);
 	u8 forcenow;
 	u8 gpdone;
@@ -1207,7 +1206,6 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 		goto unlock_fqs_ret; /* no emergency and done recently. */
 	rsp->n_force_qs++;
 	spin_lock(&rnp->lock);  /* irqs already disabled */
-	lastcomp = rsp->gpnum - 1;
 	rsp->jiffies_force_qs = jiffies + RCU_JIFFIES_TILL_FORCE_QS;
 	if(!rcu_gp_in_progress(rsp)) {
 		rsp->n_force_qs_ngp++;
@@ -1228,7 +1226,7 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 			break; /* So gcc recognizes the dead code. */
 
 		/* Record dyntick-idle state. */
-		gpdone = rcu_process_dyntick(rsp, lastcomp,
+		gpdone = rcu_process_dyntick(rsp, rsp->gpnum - 1,
 					     dyntick_save_progress_counter);
 		spin_lock(&rnp->lock);  /* irqs already disabled */
 		if (gpdone)
@@ -1239,11 +1237,9 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 
 		/* Update state, record completion counter. */
 		forcenow = 0;
-		if (lastcomp + 1 == rsp->gpnum &&
-		    lastcomp == rsp->completed) {
+		if (rsp->gpnum - 1 == rsp->completed) {
 			forcenow = rsp->signaled == RCU_SAVE_COMPLETED;
 			rsp->signaled = RCU_FORCE_QS;
-			rsp->completed_fqs = lastcomp;
 		}
 		if (!forcenow)
 			break;
@@ -1253,7 +1249,7 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 
 		/* Check dyntick-idle state, send IPI to laggarts. */
 		spin_unlock(&rnp->lock);  /* irqs remain disabled */
-		gpdone = rcu_process_dyntick(rsp, rsp->completed_fqs,
+		gpdone = rcu_process_dyntick(rsp, rsp->gpnum - 1,
 					     rcu_implicit_dynticks_qs);
 
 		/* Leave state in case more forcing is required. */
diff --git a/kernel/rcutree.h b/kernel/rcutree.h
index dc386a7..5348561 100644
--- a/kernel/rcutree.h
+++ b/kernel/rcutree.h
@@ -296,8 +296,6 @@ struct rcu_state {
 	long orphan_qlen;			/* Number of orphaned cbs. */
 	spinlock_t fqslock;			/* Only one task forcing */
 						/*  quiescent states. */
-	long	completed_fqs;			/* Value of completed @ snap. */
-						/*  Protected by fqslock. */
 	unsigned long jiffies_force_qs;		/* Time at which to invoke */
 						/*  force_quiescent_state(). */
 	unsigned long n_force_qs;		/* Number of calls to */
-- 
1.5.2.5


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC tip/core/rcu 06/18] rcu: eliminate second argument of rcu_process_dyntick()
  2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
                   ` (4 preceding siblings ...)
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 05/18] rcu: eliminate local variable lastcomp " Paul E. McKenney
@ 2009-12-15 23:02 ` Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 07/18] rcu: eliminate rcu_process_dyntick() return value Paul E. McKenney
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	Paul E. McKenney

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Proposed for 2.6.34, not for inclusion.

At this point, the second argument to all calls to rcu_process_dyntick()
is a function of the same field of the structure passed in as the first
argument, namely, rsp->gpnum-1.  So propagate rsp->gpnum-1 to all uses
of the second argument within rcu_process_dyntick() and then eliminate
the second argument.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index 62b6433..c7d0070 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -1147,7 +1147,7 @@ void rcu_check_callbacks(int cpu, int user)
  * Returns 1 if the current grace period ends while scanning (possibly
  * because we made it end).
  */
-static int rcu_process_dyntick(struct rcu_state *rsp, long lastcomp,
+static int rcu_process_dyntick(struct rcu_state *rsp,
 			       int (*f)(struct rcu_data *))
 {
 	unsigned long bit;
@@ -1159,7 +1159,7 @@ static int rcu_process_dyntick(struct rcu_state *rsp, long lastcomp,
 	rcu_for_each_leaf_node(rsp, rnp) {
 		mask = 0;
 		spin_lock_irqsave(&rnp->lock, flags);
-		if (rnp->completed != lastcomp) {
+		if (rnp->completed != rsp->gpnum - 1) {
 			spin_unlock_irqrestore(&rnp->lock, flags);
 			return 1;
 		}
@@ -1173,7 +1173,7 @@ static int rcu_process_dyntick(struct rcu_state *rsp, long lastcomp,
 			if ((rnp->qsmask & bit) != 0 && f(rsp->rda[cpu]))
 				mask |= bit;
 		}
-		if (mask != 0 && rnp->completed == lastcomp) {
+		if (mask != 0 && rnp->completed == rsp->gpnum - 1) {
 
 			/* rcu_report_qs_rnp() releases rnp->lock. */
 			rcu_report_qs_rnp(mask, rsp, rnp, flags);
@@ -1226,7 +1226,7 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 			break; /* So gcc recognizes the dead code. */
 
 		/* Record dyntick-idle state. */
-		gpdone = rcu_process_dyntick(rsp, rsp->gpnum - 1,
+		gpdone = rcu_process_dyntick(rsp,
 					     dyntick_save_progress_counter);
 		spin_lock(&rnp->lock);  /* irqs already disabled */
 		if (gpdone)
@@ -1249,8 +1249,7 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 
 		/* Check dyntick-idle state, send IPI to laggarts. */
 		spin_unlock(&rnp->lock);  /* irqs remain disabled */
-		gpdone = rcu_process_dyntick(rsp, rsp->gpnum - 1,
-					     rcu_implicit_dynticks_qs);
+		gpdone = rcu_process_dyntick(rsp, rcu_implicit_dynticks_qs);
 
 		/* Leave state in case more forcing is required. */
 
-- 
1.5.2.5


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC tip/core/rcu 07/18] rcu: eliminate rcu_process_dyntick() return value
  2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
                   ` (5 preceding siblings ...)
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 06/18] rcu: eliminate second argument of rcu_process_dyntick() Paul E. McKenney
@ 2009-12-15 23:02 ` Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 08/18] rcu: remove leg of force_quiescent_state() switch statement Paul E. McKenney
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	Paul E. McKenney

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Proposed for 2.6.34, not for inclusion.

Because a new grace period cannot start while we are executing within the
force_quiescent_state() function's switch statement, if any test within
that switch statement or within any function called from that switch
statement shows that the current grace period has ended, we can safely
re-do that test any time before we leave the switch statement.  This
means that we no longer need a return value from rcu_process_dyntick(),
as we can simply invoke rcu_gp_in_progress() to check whether the old
grace period has finished -- there is no longer any need to worry about
whether or not a new grace period has been started.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |   17 ++++++-----------
 1 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index c7d0070..e497119 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -1144,11 +1144,9 @@ void rcu_check_callbacks(int cpu, int user)
 /*
  * Scan the leaf rcu_node structures, processing dyntick state for any that
  * have not yet encountered a quiescent state, using the function specified.
- * Returns 1 if the current grace period ends while scanning (possibly
- * because we made it end).
  */
-static int rcu_process_dyntick(struct rcu_state *rsp,
-			       int (*f)(struct rcu_data *))
+static void rcu_process_dyntick(struct rcu_state *rsp,
+				int (*f)(struct rcu_data *))
 {
 	unsigned long bit;
 	int cpu;
@@ -1161,7 +1159,7 @@ static int rcu_process_dyntick(struct rcu_state *rsp,
 		spin_lock_irqsave(&rnp->lock, flags);
 		if (rnp->completed != rsp->gpnum - 1) {
 			spin_unlock_irqrestore(&rnp->lock, flags);
-			return 1;
+			return;
 		}
 		if (rnp->qsmask == 0) {
 			spin_unlock_irqrestore(&rnp->lock, flags);
@@ -1181,7 +1179,6 @@ static int rcu_process_dyntick(struct rcu_state *rsp,
 		}
 		spin_unlock_irqrestore(&rnp->lock, flags);
 	}
-	return 0;
 }
 
 /*
@@ -1193,7 +1190,6 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 	unsigned long flags;
 	struct rcu_node *rnp = rcu_get_root(rsp);
 	u8 forcenow;
-	u8 gpdone;
 
 	if (!rcu_gp_in_progress(rsp))
 		return;  /* No grace period in progress, nothing to force. */
@@ -1226,10 +1222,9 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 			break; /* So gcc recognizes the dead code. */
 
 		/* Record dyntick-idle state. */
-		gpdone = rcu_process_dyntick(rsp,
-					     dyntick_save_progress_counter);
+		rcu_process_dyntick(rsp, dyntick_save_progress_counter);
 		spin_lock(&rnp->lock);  /* irqs already disabled */
-		if (gpdone)
+		if (!rcu_gp_in_progress(rsp))
 			break;
 		/* fall into next case. */
 
@@ -1249,7 +1244,7 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 
 		/* Check dyntick-idle state, send IPI to laggarts. */
 		spin_unlock(&rnp->lock);  /* irqs remain disabled */
-		gpdone = rcu_process_dyntick(rsp, rcu_implicit_dynticks_qs);
+		rcu_process_dyntick(rsp, rcu_implicit_dynticks_qs);
 
 		/* Leave state in case more forcing is required. */
 
-- 
1.5.2.5


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC tip/core/rcu 08/18] rcu: remove leg of force_quiescent_state() switch statement
  2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
                   ` (6 preceding siblings ...)
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 07/18] rcu: eliminate rcu_process_dyntick() return value Paul E. McKenney
@ 2009-12-15 23:02 ` Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 09/18] rcu: remove redundant grace-period check Paul E. McKenney
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	Paul E. McKenney

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Proposed for 2.6.34, not for inclusion.

The comparisons of rsp->gpnum nad rsp->completed in rcu_process_dyntick()
and force_quiescent_state() can be replaced by the much more clear
rcu_gp_in_progress() predicate function.  After doing this, it becomes
clear that the RCU_SAVE_COMPLETED leg of the force_quiescent_state()
function's switch statement is almost completely a no-op.  A small change
to the RCU_SAVE_DYNTICK leg renders it a complete no-op, after which it
can be removed.  Doing so also eliminates the forcenow local variable
from force_quiescent_state().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |   22 +++++-----------------
 kernel/rcutree.h |    5 ++---
 2 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index e497119..6268f37 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -1144,6 +1144,7 @@ void rcu_check_callbacks(int cpu, int user)
 /*
  * Scan the leaf rcu_node structures, processing dyntick state for any that
  * have not yet encountered a quiescent state, using the function specified.
+ * The caller must have suppressed start of new grace periods.
  */
 static void rcu_process_dyntick(struct rcu_state *rsp,
 				int (*f)(struct rcu_data *))
@@ -1157,7 +1158,7 @@ static void rcu_process_dyntick(struct rcu_state *rsp,
 	rcu_for_each_leaf_node(rsp, rnp) {
 		mask = 0;
 		spin_lock_irqsave(&rnp->lock, flags);
-		if (rnp->completed != rsp->gpnum - 1) {
+		if (!rcu_gp_in_progress(rsp)) {
 			spin_unlock_irqrestore(&rnp->lock, flags);
 			return;
 		}
@@ -1171,7 +1172,7 @@ static void rcu_process_dyntick(struct rcu_state *rsp,
 			if ((rnp->qsmask & bit) != 0 && f(rsp->rda[cpu]))
 				mask |= bit;
 		}
-		if (mask != 0 && rnp->completed == rsp->gpnum - 1) {
+		if (mask != 0 && rcu_gp_in_progress(rsp)) {
 
 			/* rcu_report_qs_rnp() releases rnp->lock. */
 			rcu_report_qs_rnp(mask, rsp, rnp, flags);
@@ -1189,7 +1190,6 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 {
 	unsigned long flags;
 	struct rcu_node *rnp = rcu_get_root(rsp);
-	u8 forcenow;
 
 	if (!rcu_gp_in_progress(rsp))
 		return;  /* No grace period in progress, nothing to force. */
@@ -1224,21 +1224,9 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 		/* Record dyntick-idle state. */
 		rcu_process_dyntick(rsp, dyntick_save_progress_counter);
 		spin_lock(&rnp->lock);  /* irqs already disabled */
-		if (!rcu_gp_in_progress(rsp))
-			break;
-		/* fall into next case. */
-
-	case RCU_SAVE_COMPLETED:
-
-		/* Update state, record completion counter. */
-		forcenow = 0;
-		if (rsp->gpnum - 1 == rsp->completed) {
-			forcenow = rsp->signaled == RCU_SAVE_COMPLETED;
+		if (rcu_gp_in_progress(rsp))
 			rsp->signaled = RCU_FORCE_QS;
-		}
-		if (!forcenow)
-			break;
-		/* fall into next case. */
+		break;
 
 	case RCU_FORCE_QS:
 
diff --git a/kernel/rcutree.h b/kernel/rcutree.h
index 5348561..edb6fae 100644
--- a/kernel/rcutree.h
+++ b/kernel/rcutree.h
@@ -237,12 +237,11 @@ struct rcu_data {
 #define RCU_GP_IDLE		0	/* No grace period in progress. */
 #define RCU_GP_INIT		1	/* Grace period being initialized. */
 #define RCU_SAVE_DYNTICK	2	/* Need to scan dyntick state. */
-#define RCU_SAVE_COMPLETED	3	/* Need to save rsp->completed. */
-#define RCU_FORCE_QS		4	/* Need to force quiescent state. */
+#define RCU_FORCE_QS		3	/* Need to force quiescent state. */
 #ifdef CONFIG_NO_HZ
 #define RCU_SIGNAL_INIT		RCU_SAVE_DYNTICK
 #else /* #ifdef CONFIG_NO_HZ */
-#define RCU_SIGNAL_INIT		RCU_SAVE_COMPLETED
+#define RCU_SIGNAL_INIT		RCU_FORCE_QS
 #endif /* #else #ifdef CONFIG_NO_HZ */
 
 #define RCU_JIFFIES_TILL_FORCE_QS	 3	/* for rsp->jiffies_force_qs */
-- 
1.5.2.5


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC tip/core/rcu 09/18] rcu: remove redundant grace-period check
  2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
                   ` (7 preceding siblings ...)
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 08/18] rcu: remove leg of force_quiescent_state() switch statement Paul E. McKenney
@ 2009-12-15 23:02 ` Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 10/18] rcu: make force_quiescent_state() start grace period if needed Paul E. McKenney
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	Paul E. McKenney

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Proposed for 2.6.34, not for inclusion.

The rcu_process_dyntick() function checks twice for the end of
the current grace period.  However, it holds the current rcu_node
structure's ->lock field throughout, and doesn't get to the second call
to rcu_gp_in_progress() unless there is at least one CPU corresponding
to this rcu_node structure that has not yet checked in for the current
grace period, which would prevent the current grace period from ending.
So the current grace period cannot have ended, and the second check is
redundant, so remove it.

Also, given that this function is used even with !CONFIG_NO_HZ, its name
is quite misleading.  Change from rcu_process_dyntick() to force_qs_rnp().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index 6268f37..d920285 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -1146,8 +1146,7 @@ void rcu_check_callbacks(int cpu, int user)
  * have not yet encountered a quiescent state, using the function specified.
  * The caller must have suppressed start of new grace periods.
  */
-static void rcu_process_dyntick(struct rcu_state *rsp,
-				int (*f)(struct rcu_data *))
+static void force_qs_rnp(struct rcu_state *rsp, int (*f)(struct rcu_data *))
 {
 	unsigned long bit;
 	int cpu;
@@ -1172,7 +1171,7 @@ static void rcu_process_dyntick(struct rcu_state *rsp,
 			if ((rnp->qsmask & bit) != 0 && f(rsp->rda[cpu]))
 				mask |= bit;
 		}
-		if (mask != 0 && rcu_gp_in_progress(rsp)) {
+		if (mask != 0) {
 
 			/* rcu_report_qs_rnp() releases rnp->lock. */
 			rcu_report_qs_rnp(mask, rsp, rnp, flags);
@@ -1222,7 +1221,7 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 			break; /* So gcc recognizes the dead code. */
 
 		/* Record dyntick-idle state. */
-		rcu_process_dyntick(rsp, dyntick_save_progress_counter);
+		force_qs_rnp(rsp, dyntick_save_progress_counter);
 		spin_lock(&rnp->lock);  /* irqs already disabled */
 		if (rcu_gp_in_progress(rsp))
 			rsp->signaled = RCU_FORCE_QS;
@@ -1232,7 +1231,7 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 
 		/* Check dyntick-idle state, send IPI to laggarts. */
 		spin_unlock(&rnp->lock);  /* irqs remain disabled */
-		rcu_process_dyntick(rsp, rcu_implicit_dynticks_qs);
+		force_qs_rnp(rsp, rcu_implicit_dynticks_qs);
 
 		/* Leave state in case more forcing is required. */
 
-- 
1.5.2.5


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC tip/core/rcu 10/18] rcu: make force_quiescent_state() start grace period if needed
  2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
                   ` (8 preceding siblings ...)
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 09/18] rcu: remove redundant grace-period check Paul E. McKenney
@ 2009-12-15 23:02 ` Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 11/18] rcu: add force_quiescent_state() testing to rcutorture Paul E. McKenney
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	Paul E. McKenney

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Proposed for 2.6.34, not for inclusion.

Grace periods cannot be started while force_quiescent_state() is
active.  This is OK in that the affected CPUs will try again later,
but it does induce needless grace-period delays.  This patch causes
rcu_start_gp() to record a failed attempt to start a grace period.
When force_quiescent_state() prepares to return, it then starts the
grace period if there was such a failed attempt.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |    8 ++++++++
 kernel/rcutree.h |    5 +++++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index d920285..55e8f6e 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -660,6 +660,8 @@ rcu_start_gp(struct rcu_state *rsp, unsigned long flags)
 	struct rcu_node *rnp = rcu_get_root(rsp);
 
 	if (!cpu_needs_another_gp(rsp, rdp) || rsp->fqs_active) {
+		if (cpu_needs_another_gp(rsp, rdp))
+			rsp->fqs_need_gp = 1;
 		if (rnp->completed == rsp->completed) {
 			spin_unlock_irqrestore(&rnp->lock, flags);
 			return;
@@ -1239,6 +1241,12 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
 		break;
 	}
 	rsp->fqs_active = 0;
+	if (rsp->fqs_need_gp) {
+		spin_unlock(&rsp->fqslock); /* irqs remain disabled */
+		rsp->fqs_need_gp = 0;
+		rcu_start_gp(rsp, flags); /* releases rnp->lock */
+		return;
+	}
 	spin_unlock(&rnp->lock);  /* irqs remain disabled */
 unlock_fqs_ret:
 	spin_unlock_irqrestore(&rsp->fqslock, flags);
diff --git a/kernel/rcutree.h b/kernel/rcutree.h
index edb6fae..bd5d78a 100644
--- a/kernel/rcutree.h
+++ b/kernel/rcutree.h
@@ -278,6 +278,11 @@ struct rcu_state {
 						/* Force QS state. */
 	u8	fqs_active;			/* force_quiescent_state() */
 						/*  is running. */
+	u8	fqs_need_gp;			/* A CPU was prevented from */
+						/*  starting a new grace */
+						/*  period because */
+						/*  force_quiescent_state() */
+						/*  was running. */
 	long	gpnum;				/* Current gp number. */
 	long	completed;			/* # of last completed gp. */
 
-- 
1.5.2.5


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC tip/core/rcu 11/18] rcu: add force_quiescent_state() testing to rcutorture
  2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
                   ` (9 preceding siblings ...)
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 10/18] rcu: make force_quiescent_state() start grace period if needed Paul E. McKenney
@ 2009-12-15 23:02 ` Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 12/18] rcu: make MAINTAINERS file match new RCU reality Paul E. McKenney
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	Paul E. McKenney

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Proposed for 2.6.34, not for inclusion.

Add force_quiescent_state() testing to rcutorture, with a separate
thread that repeatedly invokes force_quiescent_state() in bursts.
This can greatly increase the probability of encountering certain types
of race conditions.

Suggested-by: Josh Triplett <josh@joshtriplett.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 include/linux/rcutiny.h |   12 +++++++
 include/linux/rcutree.h |    3 ++
 kernel/rcutorture.c     |   80 +++++++++++++++++++++++++++++++++++++++++++++-
 kernel/rcutree.c        |   18 ++++++++++
 kernel/rcutree_plugin.h |   19 +++++++++++
 5 files changed, 130 insertions(+), 2 deletions(-)

diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
index c4ba9a7..b524590 100644
--- a/include/linux/rcutiny.h
+++ b/include/linux/rcutiny.h
@@ -62,6 +62,18 @@ static inline long rcu_batches_completed_bh(void)
 
 extern int rcu_expedited_torture_stats(char *page);
 
+static inline void rcu_force_quiescent_state(void)
+{
+}
+
+static inline void rcu_bh_force_quiescent_state(void)
+{
+}
+
+static inline void rcu_sched_force_quiescent_state(void)
+{
+}
+
 #define synchronize_rcu synchronize_sched
 
 static inline void synchronize_rcu_expedited(void)
diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
index c93eee5..564a025 100644
--- a/include/linux/rcutree.h
+++ b/include/linux/rcutree.h
@@ -88,6 +88,9 @@ extern void rcu_check_callbacks(int cpu, int user);
 extern long rcu_batches_completed(void);
 extern long rcu_batches_completed_bh(void);
 extern long rcu_batches_completed_sched(void);
+extern void rcu_force_quiescent_state(void);
+extern void rcu_bh_force_quiescent_state(void);
+extern void rcu_sched_force_quiescent_state(void);
 
 #ifdef CONFIG_NO_HZ
 void rcu_enter_nohz(void);
diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index a621a67..b4096d3 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -61,6 +61,9 @@ static int test_no_idle_hz;	/* Test RCU's support for tickless idle CPUs. */
 static int shuffle_interval = 3; /* Interval between shuffles (in sec)*/
 static int stutter = 5;		/* Start/stop testing interval (in sec) */
 static int irqreader = 1;	/* RCU readers from irq (timers). */
+static int fqs_duration = 0;	/* Duration of bursts (us), 0 to disable. */
+static int fqs_holdoff = 0;	/* Hold time within burst (us). */
+static int fqs_stutter = 3;	/* Wait time between bursts (s). */
 static char *torture_type = "rcu"; /* What RCU implementation to torture. */
 
 module_param(nreaders, int, 0444);
@@ -79,6 +82,12 @@ module_param(stutter, int, 0444);
 MODULE_PARM_DESC(stutter, "Number of seconds to run/halt test");
 module_param(irqreader, int, 0444);
 MODULE_PARM_DESC(irqreader, "Allow RCU readers from irq handlers");
+module_param(fqs_duration, int, 0444);
+MODULE_PARM_DESC(fqs_duration, "Duration of fqs bursts (us)");
+module_param(fqs_holdoff, int, 0444);
+MODULE_PARM_DESC(fqs_holdoff, "Holdoff time within fqs bursts (us)");
+module_param(fqs_stutter, int, 0444);
+MODULE_PARM_DESC(fqs_stutter, "Wait time between fqs bursts (s)");
 module_param(torture_type, charp, 0444);
 MODULE_PARM_DESC(torture_type, "Type of RCU to torture (rcu, rcu_bh, srcu)");
 
@@ -99,6 +108,7 @@ static struct task_struct **reader_tasks;
 static struct task_struct *stats_task;
 static struct task_struct *shuffler_task;
 static struct task_struct *stutter_task;
+static struct task_struct *fqs_task;
 
 #define RCU_TORTURE_PIPE_LEN 10
 
@@ -263,6 +273,7 @@ struct rcu_torture_ops {
 	void (*deferred_free)(struct rcu_torture *p);
 	void (*sync)(void);
 	void (*cb_barrier)(void);
+	void (*fqs)(void);
 	int (*stats)(char *page);
 	int irq_capable;
 	char *name;
@@ -347,6 +358,7 @@ static struct rcu_torture_ops rcu_ops = {
 	.deferred_free	= rcu_torture_deferred_free,
 	.sync		= synchronize_rcu,
 	.cb_barrier	= rcu_barrier,
+	.fqs		= rcu_force_quiescent_state,
 	.stats		= NULL,
 	.irq_capable	= 1,
 	.name		= "rcu"
@@ -388,6 +400,7 @@ static struct rcu_torture_ops rcu_sync_ops = {
 	.deferred_free	= rcu_sync_torture_deferred_free,
 	.sync		= synchronize_rcu,
 	.cb_barrier	= NULL,
+	.fqs		= rcu_force_quiescent_state,
 	.stats		= NULL,
 	.irq_capable	= 1,
 	.name		= "rcu_sync"
@@ -403,6 +416,7 @@ static struct rcu_torture_ops rcu_expedited_ops = {
 	.deferred_free	= rcu_sync_torture_deferred_free,
 	.sync		= synchronize_rcu_expedited,
 	.cb_barrier	= NULL,
+	.fqs		= rcu_force_quiescent_state,
 	.stats		= NULL,
 	.irq_capable	= 1,
 	.name		= "rcu_expedited"
@@ -465,6 +479,7 @@ static struct rcu_torture_ops rcu_bh_ops = {
 	.deferred_free	= rcu_bh_torture_deferred_free,
 	.sync		= rcu_bh_torture_synchronize,
 	.cb_barrier	= rcu_barrier_bh,
+	.fqs		= rcu_bh_force_quiescent_state,
 	.stats		= NULL,
 	.irq_capable	= 1,
 	.name		= "rcu_bh"
@@ -480,6 +495,7 @@ static struct rcu_torture_ops rcu_bh_sync_ops = {
 	.deferred_free	= rcu_sync_torture_deferred_free,
 	.sync		= rcu_bh_torture_synchronize,
 	.cb_barrier	= NULL,
+	.fqs		= rcu_bh_force_quiescent_state,
 	.stats		= NULL,
 	.irq_capable	= 1,
 	.name		= "rcu_bh_sync"
@@ -621,6 +637,7 @@ static struct rcu_torture_ops sched_ops = {
 	.deferred_free	= rcu_sched_torture_deferred_free,
 	.sync		= sched_torture_synchronize,
 	.cb_barrier	= rcu_barrier_sched,
+	.fqs		= rcu_sched_force_quiescent_state,
 	.stats		= NULL,
 	.irq_capable	= 1,
 	.name		= "sched"
@@ -636,6 +653,7 @@ static struct rcu_torture_ops sched_sync_ops = {
 	.deferred_free	= rcu_sync_torture_deferred_free,
 	.sync		= sched_torture_synchronize,
 	.cb_barrier	= NULL,
+	.fqs		= rcu_sched_force_quiescent_state,
 	.stats		= NULL,
 	.name		= "sched_sync"
 };
@@ -650,12 +668,45 @@ static struct rcu_torture_ops sched_expedited_ops = {
 	.deferred_free	= rcu_sync_torture_deferred_free,
 	.sync		= synchronize_sched_expedited,
 	.cb_barrier	= NULL,
+	.fqs		= rcu_sched_force_quiescent_state,
 	.stats		= rcu_expedited_torture_stats,
 	.irq_capable	= 1,
 	.name		= "sched_expedited"
 };
 
 /*
+ * RCU torture force-quiescent-state kthread.  Repeatedly induces
+ * bursts of calls to force_quiescent_state(), increasing the probability
+ * of occurrence of some important types of race conditions.
+ */
+static int
+rcu_torture_fqs(void *arg)
+{
+	unsigned long fqs_resume_time;
+	int fqs_burst_remaining;
+
+	VERBOSE_PRINTK_STRING("rcu_torture_fqs task started");
+	do {
+		fqs_resume_time = jiffies + fqs_stutter * HZ;
+		while (jiffies - fqs_resume_time > LONG_MAX) {
+			schedule_timeout_interruptible(1);
+		}
+		fqs_burst_remaining = fqs_duration;
+		while (fqs_burst_remaining > 0) {
+			cur_ops->fqs();
+			udelay(fqs_holdoff);
+			fqs_burst_remaining -= fqs_holdoff;
+		}
+		rcu_stutter_wait("rcu_torture_fqs");
+	} while (!kthread_should_stop() && fullstop == FULLSTOP_DONTSTOP);
+	VERBOSE_PRINTK_STRING("rcu_torture_fqs task stopping");
+	rcutorture_shutdown_absorb("rcu_torture_fqs");
+	while (!kthread_should_stop())
+		schedule_timeout_uninterruptible(1);
+	return 0;
+}
+
+/*
  * RCU torture writer kthread.  Repeatedly substitutes a new structure
  * for that pointed to by rcu_torture_current, freeing the old structure
  * after a series of grace periods (the "pipeline").
@@ -1030,10 +1081,11 @@ rcu_torture_print_module_parms(char *tag)
 	printk(KERN_ALERT "%s" TORTURE_FLAG
 		"--- %s: nreaders=%d nfakewriters=%d "
 		"stat_interval=%d verbose=%d test_no_idle_hz=%d "
-		"shuffle_interval=%d stutter=%d irqreader=%d\n",
+		"shuffle_interval=%d stutter=%d irqreader=%d "
+		"fqs_duration=%d fqs_holdoff=%d fqs_stutter=%d\n",
 		torture_type, tag, nrealreaders, nfakewriters,
 		stat_interval, verbose, test_no_idle_hz, shuffle_interval,
-		stutter, irqreader);
+		stutter, irqreader, fqs_duration, fqs_holdoff, fqs_stutter);
 }
 
 static struct notifier_block rcutorture_nb = {
@@ -1109,6 +1161,12 @@ rcu_torture_cleanup(void)
 	}
 	stats_task = NULL;
 
+	if (fqs_task) {
+		VERBOSE_PRINTK_STRING("Stopping rcu_torture_fqs task");
+		kthread_stop(fqs_task);
+	}
+	fqs_task = NULL;
+
 	/* Wait for all RCU callbacks to fire.  */
 
 	if (cur_ops->cb_barrier != NULL)
@@ -1154,6 +1212,11 @@ rcu_torture_init(void)
 		mutex_unlock(&fullstop_mutex);
 		return -EINVAL;
 	}
+	if (cur_ops->fqs == NULL && fqs_duration != 0) {
+		printk(KERN_ALERT "rcu-torture: ->fqs NULL and non-zero "
+				  "fqs_duration, fqs disabled.\n");
+		fqs_duration = 0;
+	}
 	if (cur_ops->init)
 		cur_ops->init(); /* no "goto unwind" prior to this point!!! */
 
@@ -1282,6 +1345,19 @@ rcu_torture_init(void)
 			goto unwind;
 		}
 	}
+	if (fqs_duration < 0)
+		fqs_duration = 0;
+	if (fqs_duration) {
+		/* Create the stutter thread */
+		fqs_task = kthread_run(rcu_torture_fqs, NULL,
+				       "rcu_torture_fqs");
+		if (IS_ERR(fqs_task)) {
+			firsterr = PTR_ERR(fqs_task);
+			VERBOSE_PRINTK_ERRSTRING("Failed to create fqs");
+			fqs_task = NULL;
+			goto unwind;
+		}
+	}
 	register_reboot_notifier(&rcutorture_nb);
 	mutex_unlock(&fullstop_mutex);
 	return 0;
diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index 55e8f6e..0a4c328 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -157,6 +157,24 @@ long rcu_batches_completed_bh(void)
 EXPORT_SYMBOL_GPL(rcu_batches_completed_bh);
 
 /*
+ * Force a quiescent state for RCU BH.
+ */
+void rcu_bh_force_quiescent_state(void)
+{
+	force_quiescent_state(&rcu_bh_state, 0);
+}
+EXPORT_SYMBOL_GPL(rcu_bh_force_quiescent_state);
+
+/*
+ * Force a quiescent state for RCU-sched.
+ */
+void rcu_sched_force_quiescent_state(void)
+{
+	force_quiescent_state(&rcu_sched_state, 0);
+}
+EXPORT_SYMBOL_GPL(rcu_sched_force_quiescent_state);
+
+/*
  * Does the CPU have callbacks ready to be invoked?
  */
 static int
diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
index 37fbccd..f11ebd4 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.h
@@ -62,6 +62,15 @@ long rcu_batches_completed(void)
 EXPORT_SYMBOL_GPL(rcu_batches_completed);
 
 /*
+ * Force a quiescent state for preemptible RCU.
+ */
+void rcu_force_quiescent_state(void)
+{
+	force_quiescent_state(&rcu_preempt_state, 0);
+}
+EXPORT_SYMBOL_GPL(rcu_force_quiescent_state);
+
+/*
  * Record a preemptable-RCU quiescent state for the specified CPU.  Note
  * that this just means that the task currently running on the CPU is
  * not in a quiescent state.  There might be any number of tasks blocked
@@ -713,6 +722,16 @@ long rcu_batches_completed(void)
 EXPORT_SYMBOL_GPL(rcu_batches_completed);
 
 /*
+ * Force a quiescent state for RCU, which, because there is no preemptible
+ * RCU, becomes the same as rcu-sched.
+ */
+void rcu_force_quiescent_state(void)
+{
+	rcu_sched_force_quiescent_state();
+}
+EXPORT_SYMBOL_GPL(rcu_force_quiescent_state);
+
+/*
  * Because preemptable RCU does not exist, we never have to check for
  * CPUs being in quiescent states.
  */
-- 
1.5.2.5


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC tip/core/rcu 12/18] rcu: make MAINTAINERS file match new RCU reality
  2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
                   ` (10 preceding siblings ...)
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 11/18] rcu: add force_quiescent_state() testing to rcutorture Paul E. McKenney
@ 2009-12-15 23:02 ` Paul E. McKenney
  2009-12-16  0:53   ` Josh Triplett
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 13/18] rcu: add debug check for too many rcu_read_unlock() Paul E. McKenney
                   ` (5 subsequent siblings)
  17 siblings, 1 reply; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	Paul E. McKenney

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Proposed for 2.6.34, not for inclusion.

Both rcutorture and RCU are supported.  Also, update the files
to match the new layout.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 MAINTAINERS |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index e1da925..21c9d3b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4335,7 +4335,7 @@ F:	drivers/net/wireless/ray*
 RCUTORTURE MODULE
 M:	Josh Triplett <josh@freedesktop.org>
 M:	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
-S:	Maintained
+S:	Supported
 F:	Documentation/RCU/torture.txt
 F:	kernel/rcutorture.c
 
@@ -4360,11 +4360,12 @@ M:	Dipankar Sarma <dipankar@in.ibm.com>
 M:	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
 W:	http://www.rdrop.com/users/paulmck/rclock/
 S:	Supported
-F:	Documentation/RCU/rcu.txt
-F:	Documentation/RCU/rcuref.txt
-F:	include/linux/rcupdate.h
-F:	include/linux/srcu.h
-F:	kernel/rcupdate.c
+F:	Documentation/RCU/
+F:	include/linux/rcu*
+F:	include/linux/srcu*
+F:	kernel/rcu*
+F:	kernel/srcu*
+X:	kernel/rcutorture.c
 
 REAL TIME CLOCK DRIVER
 M:	Paul Gortmaker <p_gortmaker@yahoo.com>
-- 
1.5.2.5


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC tip/core/rcu 13/18] rcu: add debug check for too many rcu_read_unlock()
  2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
                   ` (11 preceding siblings ...)
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 12/18] rcu: make MAINTAINERS file match new RCU reality Paul E. McKenney
@ 2009-12-15 23:02 ` Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 14/18] rcu: lockdep check for exiting to user space as RCU reader Paul E. McKenney
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	Paul E. McKenney

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Proposed for 2.6.34, not for inclusion.

TREE_PREEMPT_RCU maintains an rcu_read_lock_nesting counter in the
task structure, which happens to be a signed int.  So this patch adds a
check for this counter being negative at the end of __rcu_read_unlock().
This check is under CONFIG_PROVE_LOCKING, so can be thought of as being
part of lockdep.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree_plugin.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
index f11ebd4..e77cdf3 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.h
@@ -304,6 +304,9 @@ void __rcu_read_unlock(void)
 	if (--ACCESS_ONCE(t->rcu_read_lock_nesting) == 0 &&
 	    unlikely(ACCESS_ONCE(t->rcu_read_unlock_special)))
 		rcu_read_unlock_special(t);
+#ifdef CONFIG_PROVE_LOCKING
+	WARN_ON_ONCE(ACCESS_ONCE(t->rcu_read_lock_nesting) < 0);
+#endif /* #ifdef CONFIG_PROVE_LOCKING */
 }
 EXPORT_SYMBOL_GPL(__rcu_read_unlock);
 
-- 
1.5.2.5


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC tip/core/rcu 14/18] rcu: lockdep check for exiting to user space as RCU reader
  2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
                   ` (12 preceding siblings ...)
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 13/18] rcu: add debug check for too many rcu_read_unlock() Paul E. McKenney
@ 2009-12-15 23:02 ` Paul E. McKenney
  2009-12-16 10:24   ` Peter Zijlstra
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 15/18] rcu: give different levels of the rcu_node hierarchy distinct lockdep names Paul E. McKenney
                   ` (3 subsequent siblings)
  17 siblings, 1 reply; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	Paul E. McKenney

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Proposed for 2.6.34, not for inclusion.

It is illegal to return to user-space execution while running within an
RCU read-side critical section.  It turns out that CONFIG_TREE_PREEMPT_RCU
has enough information lying around to detect this, so add the checks
to lockdep (CONFIG_PROVE_LOCKING).

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 include/linux/rcutiny.h |    4 ++++
 include/linux/rcutree.h |    1 +
 kernel/lockdep.c        |   10 ++++++++++
 kernel/rcutree_plugin.h |   22 ++++++++++++++++++++++
 4 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
index b524590..c32b16d 100644
--- a/include/linux/rcutiny.h
+++ b/include/linux/rcutiny.h
@@ -29,6 +29,10 @@
 
 void rcu_sched_qs(int cpu);
 void rcu_bh_qs(int cpu);
+static inline int rcu_read_lock_held(void)
+{
+	return 0;
+}
 
 #define __rcu_read_lock()	preempt_disable()
 #define __rcu_read_unlock()	preempt_enable()
diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
index 564a025..8cd4ac1 100644
--- a/include/linux/rcutree.h
+++ b/include/linux/rcutree.h
@@ -37,6 +37,7 @@ extern void rcu_bh_qs(int cpu);
 extern int rcu_needs_cpu(int cpu);
 extern void rcu_scheduler_starting(void);
 extern int rcu_expedited_torture_stats(char *page);
+extern int rcu_read_lock_held(void);
 
 #ifdef CONFIG_TREE_PREEMPT_RCU
 
diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index 9af5672..a912634 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -3799,4 +3799,14 @@ void lockdep_sys_exit(void)
 				curr->comm, curr->pid);
 		lockdep_print_held_locks(curr);
 	}
+	if (unlikely(rcu_read_lock_held())) {
+		if (!debug_locks_off())
+			return;
+		printk("\n================================================\n");
+		printk(  "[ BUG: returning to user space as RCU reader!  ]\n");
+		printk(  "------------------------------------------------\n");
+		printk("%s/%d is leaving the kernel as RCU reader!\n",
+				curr->comm, curr->pid);
+		lockdep_print_held_locks(curr);
+	}
 }
diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
index e77cdf3..f6258ae 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.h
@@ -310,6 +310,18 @@ void __rcu_read_unlock(void)
 }
 EXPORT_SYMBOL_GPL(__rcu_read_unlock);
 
+/*
+ * Return 1 if the current task is provably within an RCU read-side
+ * critical section.  The bit about checking a running task to see if
+ * it is blocked is a bit strange, but keep in mind that sleep and
+ * wakeup are not atomic operations.
+ */
+int rcu_read_lock_held(void)
+{
+	return ACCESS_ONCE(current->rcu_read_lock_nesting) != 0 ||
+	       (current->rcu_read_unlock_special & RCU_READ_UNLOCK_BLOCKED);
+}
+
 #ifdef CONFIG_RCU_CPU_STALL_DETECTOR
 
 /*
@@ -761,6 +773,16 @@ static void rcu_report_unblock_qs_rnp(struct rcu_node *rnp, unsigned long flags)
 
 #endif /* #ifdef CONFIG_HOTPLUG_CPU */
 
+/*
+ * Return 1 if the current task is provably within an RCU read-side
+ * critical section.  But without preemptible RCU, we never can be
+ * sure, so always return 0.
+ */
+int rcu_read_lock_held(void)
+{
+	return 0;
+}
+
 #ifdef CONFIG_RCU_CPU_STALL_DETECTOR
 
 /*
-- 
1.5.2.5


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC tip/core/rcu 15/18] rcu: give different levels of the rcu_node hierarchy distinct lockdep names
  2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
                   ` (13 preceding siblings ...)
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 14/18] rcu: lockdep check for exiting to user space as RCU reader Paul E. McKenney
@ 2009-12-15 23:02 ` Paul E. McKenney
  2009-12-16  0:59   ` Josh Triplett
  2009-12-16 10:26   ` Peter Zijlstra
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 16/18] rcu: make lockdep aware of SRCU read-side critical sections Paul E. McKenney
                   ` (2 subsequent siblings)
  17 siblings, 2 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	Paul E. McKenney

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Proposed for 2.6.34, not for inclusion.

Previously, each level of the rcu_node hierarchy had the same rather
unimaginative name: "&rcu_node_class[i]".  This makes lockdep diagnostics
involving these lockdep classes less helpful than would be nice.  This
patch fixes this by giving each level of the rcu_node hierarchy a distinct
name: "rcu_node_level_0", "rcu_node_level_1", and so on.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index 0a4c328..a6e45f6 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -1811,11 +1811,17 @@ static void __init rcu_init_levelspread(struct rcu_state *rsp)
  */
 static void __init rcu_init_one(struct rcu_state *rsp)
 {
+	static char *buf[] = { "rcu_node_level_0",
+			       "rcu_node_level_1",
+			       "rcu_node_level_2",
+			       "rcu_node_level_3" };  /* Match MAX_RCU_LVLS */
 	int cpustride = 1;
 	int i;
 	int j;
 	struct rcu_node *rnp;
 
+	WARN_ON_ONCE(MAX_RCU_LVLS > 4);  /* Fix buf[] initialization! */
+
 	/* Initialize the level-tracking arrays. */
 
 	for (i = 1; i < NUM_RCU_LVLS; i++)
@@ -1829,7 +1835,8 @@ static void __init rcu_init_one(struct rcu_state *rsp)
 		rnp = rsp->level[i];
 		for (j = 0; j < rsp->levelcnt[i]; j++, rnp++) {
 			spin_lock_init(&rnp->lock);
-			lockdep_set_class(&rnp->lock, &rcu_node_class[i]);
+			lockdep_set_class_and_name(&rnp->lock,
+						   &rcu_node_class[i], buf[i]);
 			rnp->gpnum = 0;
 			rnp->qsmask = 0;
 			rnp->qsmaskinit = 0;
-- 
1.5.2.5


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC tip/core/rcu 16/18] rcu: make lockdep aware of SRCU read-side critical sections.
  2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
                   ` (14 preceding siblings ...)
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 15/18] rcu: give different levels of the rcu_node hierarchy distinct lockdep names Paul E. McKenney
@ 2009-12-15 23:02 ` Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 17/18] rcu: Provide different lockdep classes for each flavor of RCU Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 18/18] rcu: add primitives to check for RCU read-side critical sections Paul E. McKenney
  17 siblings, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	Paul E. McKenney

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Proposed for 2.6.34, not for inclusion.

This patch adds lockdep classes and code to make lockdep aware of
SRCU read-side critical sections.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 include/linux/srcu.h |   44 ++++++++++++++++++++++++++++++++++++++++++--
 kernel/srcu.c        |   26 +++++++++++++-------------
 2 files changed, 55 insertions(+), 15 deletions(-)

diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 4765d97..5a07b90 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -45,10 +45,50 @@ struct srcu_struct {
 
 int init_srcu_struct(struct srcu_struct *sp);
 void cleanup_srcu_struct(struct srcu_struct *sp);
-int srcu_read_lock(struct srcu_struct *sp) __acquires(sp);
-void srcu_read_unlock(struct srcu_struct *sp, int idx) __releases(sp);
+int __srcu_read_lock(struct srcu_struct *sp) __acquires(sp);
+void __srcu_read_unlock(struct srcu_struct *sp, int idx) __releases(sp);
 void synchronize_srcu(struct srcu_struct *sp);
 void synchronize_srcu_expedited(struct srcu_struct *sp);
 long srcu_batches_completed(struct srcu_struct *sp);
 
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+extern struct lockdep_map srcu_lock_map;
+# define srcu_read_acquire() \
+		lock_acquire(&srcu_lock_map, 0, 0, 2, 1, NULL, _THIS_IP_)
+# define srcu_read_release() \
+		lock_release(&srcu_lock_map, 1, _THIS_IP_)
+#else /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
+# define srcu_read_acquire()	do { } while (0)
+# define srcu_read_release()	do { } while (0)
+#endif /* #else #ifdef CONFIG_DEBUG_LOCK_ALLOC */
+
+/**
+ * srcu_read_lock - register a new reader for an SRCU-protected structure.
+ * @sp: srcu_struct in which to register the new reader.
+ *
+ * Enter an SRCU read-side critical section.  Note that SRCU read-side
+ * critical sections may be nested.
+ */
+static inline int srcu_read_lock(struct srcu_struct *sp) __acquires(sp)
+{
+	int retval = __srcu_read_lock(sp);
+
+	srcu_read_acquire();
+	return retval;
+}
+
+/**
+ * srcu_read_unlock - unregister a old reader from an SRCU-protected structure.
+ * @sp: srcu_struct in which to unregister the old reader.
+ * @idx: return value from corresponding srcu_read_lock().
+ *
+ * Exit an SRCU read-side critical section.
+ */
+static inline void srcu_read_unlock(struct srcu_struct *sp, int idx)
+	__releases(sp)
+{
+	srcu_read_release();
+	__srcu_read_unlock(sp, idx);
+}
+
 #endif
diff --git a/kernel/srcu.c b/kernel/srcu.c
index 818d7d9..4575f63 100644
--- a/kernel/srcu.c
+++ b/kernel/srcu.c
@@ -34,6 +34,13 @@
 #include <linux/smp.h>
 #include <linux/srcu.h>
 
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+static struct lock_class_key srcu_lock_key;
+struct lockdep_map srcu_lock_map =
+	STATIC_LOCKDEP_MAP_INIT("srcu_read_lock", &srcu_lock_key);
+EXPORT_SYMBOL_GPL(srcu_lock_map);
+#endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
+
 /**
  * init_srcu_struct - initialize a sleep-RCU structure
  * @sp: structure to initialize.
@@ -100,15 +107,12 @@ void cleanup_srcu_struct(struct srcu_struct *sp)
 }
 EXPORT_SYMBOL_GPL(cleanup_srcu_struct);
 
-/**
- * srcu_read_lock - register a new reader for an SRCU-protected structure.
- * @sp: srcu_struct in which to register the new reader.
- *
+/*
  * Counts the new reader in the appropriate per-CPU element of the
  * srcu_struct.  Must be called from process context.
  * Returns an index that must be passed to the matching srcu_read_unlock().
  */
-int srcu_read_lock(struct srcu_struct *sp)
+int __srcu_read_lock(struct srcu_struct *sp)
 {
 	int idx;
 
@@ -120,26 +124,22 @@ int srcu_read_lock(struct srcu_struct *sp)
 	preempt_enable();
 	return idx;
 }
-EXPORT_SYMBOL_GPL(srcu_read_lock);
+EXPORT_SYMBOL_GPL(__srcu_read_lock);
 
-/**
- * srcu_read_unlock - unregister a old reader from an SRCU-protected structure.
- * @sp: srcu_struct in which to unregister the old reader.
- * @idx: return value from corresponding srcu_read_lock().
- *
+/*
  * Removes the count for the old reader from the appropriate per-CPU
  * element of the srcu_struct.  Note that this may well be a different
  * CPU than that which was incremented by the corresponding srcu_read_lock().
  * Must be called from process context.
  */
-void srcu_read_unlock(struct srcu_struct *sp, int idx)
+void __srcu_read_unlock(struct srcu_struct *sp, int idx)
 {
 	preempt_disable();
 	srcu_barrier();  /* ensure compiler won't misorder critical section. */
 	per_cpu_ptr(sp->per_cpu_ref, smp_processor_id())->c[idx]--;
 	preempt_enable();
 }
-EXPORT_SYMBOL_GPL(srcu_read_unlock);
+EXPORT_SYMBOL_GPL(__srcu_read_unlock);
 
 /*
  * Helper function for synchronize_srcu() and synchronize_srcu_expedited().
-- 
1.5.2.5


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC tip/core/rcu 17/18] rcu: Provide different lockdep classes for each flavor of RCU
  2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
                   ` (15 preceding siblings ...)
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 16/18] rcu: make lockdep aware of SRCU read-side critical sections Paul E. McKenney
@ 2009-12-15 23:02 ` Paul E. McKenney
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 18/18] rcu: add primitives to check for RCU read-side critical sections Paul E. McKenney
  17 siblings, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	Paul E. McKenney

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Proposed for 2.6.34, not for inclusion.

In the past, all three flavors of RCU shared a common lockdep class.
This patch gives each of rcu_read_lock(), rcu_read_lock_bh(), and
rcu_read_lock_sched() their own lockdep classes to permit more precise
checking for read-holding the proper flavor of RCU.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 include/linux/rcupdate.h |   39 +++++++++++++++++++++++++++++----------
 kernel/rcupdate.c        |   10 ++++++++++
 2 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 24440f4..723c564 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -78,14 +78,33 @@ extern void rcu_init(void);
 } while (0)
 
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
+
 extern struct lockdep_map rcu_lock_map;
-# define rcu_read_acquire()	\
-			lock_acquire(&rcu_lock_map, 0, 0, 2, 1, NULL, _THIS_IP_)
+# define rcu_read_acquire() \
+		lock_acquire(&rcu_lock_map, 0, 0, 2, 1, NULL, _THIS_IP_)
 # define rcu_read_release()	lock_release(&rcu_lock_map, 1, _THIS_IP_)
-#else
-# define rcu_read_acquire()	do { } while (0)
-# define rcu_read_release()	do { } while (0)
-#endif
+
+extern struct lockdep_map rcu_bh_lock_map;
+# define rcu_read_acquire_bh() \
+		lock_acquire(&rcu_bh_lock_map, 0, 0, 2, 1, NULL, _THIS_IP_)
+# define rcu_read_release_bh()	lock_release(&rcu_bh_lock_map, 1, _THIS_IP_)
+
+extern struct lockdep_map rcu_sched_lock_map;
+# define rcu_read_acquire_sched() \
+		lock_acquire(&rcu_sched_lock_map, 0, 0, 2, 1, NULL, _THIS_IP_)
+# define rcu_read_release_sched() \
+		lock_release(&rcu_sched_lock_map, 1, _THIS_IP_)
+
+#else /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
+
+# define rcu_read_acquire()		do { } while (0)
+# define rcu_read_release()		do { } while (0)
+# define rcu_read_acquire_bh()		do { } while (0)
+# define rcu_read_release_bh()		do { } while (0)
+# define rcu_read_acquire_sched()	do { } while (0)
+# define rcu_read_release_sched()	do { } while (0)
+
+#endif /* #else #ifdef CONFIG_DEBUG_LOCK_ALLOC */
 
 /**
  * rcu_read_lock - mark the beginning of an RCU read-side critical section.
@@ -160,7 +179,7 @@ static inline void rcu_read_lock_bh(void)
 {
 	__rcu_read_lock_bh();
 	__acquire(RCU_BH);
-	rcu_read_acquire();
+	rcu_read_acquire_bh();
 }
 
 /*
@@ -170,7 +189,7 @@ static inline void rcu_read_lock_bh(void)
  */
 static inline void rcu_read_unlock_bh(void)
 {
-	rcu_read_release();
+	rcu_read_release_bh();
 	__release(RCU_BH);
 	__rcu_read_unlock_bh();
 }
@@ -188,7 +207,7 @@ static inline void rcu_read_lock_sched(void)
 {
 	preempt_disable();
 	__acquire(RCU_SCHED);
-	rcu_read_acquire();
+	rcu_read_acquire_sched();
 }
 
 /* Used by lockdep and tracing: cannot be traced, cannot call lockdep. */
@@ -205,7 +224,7 @@ static inline notrace void rcu_read_lock_sched_notrace(void)
  */
 static inline void rcu_read_unlock_sched(void)
 {
-	rcu_read_release();
+	rcu_read_release_sched();
 	__release(RCU_SCHED);
 	preempt_enable();
 }
diff --git a/kernel/rcupdate.c b/kernel/rcupdate.c
index 9b7fd47..033cb55 100644
--- a/kernel/rcupdate.c
+++ b/kernel/rcupdate.c
@@ -50,6 +50,16 @@ static struct lock_class_key rcu_lock_key;
 struct lockdep_map rcu_lock_map =
 	STATIC_LOCKDEP_MAP_INIT("rcu_read_lock", &rcu_lock_key);
 EXPORT_SYMBOL_GPL(rcu_lock_map);
+
+static struct lock_class_key rcu_bh_lock_key;
+struct lockdep_map rcu_bh_lock_map =
+	STATIC_LOCKDEP_MAP_INIT("rcu_read_lock_bh", &rcu_bh_lock_key);
+EXPORT_SYMBOL_GPL(rcu_bh_lock_map);
+
+static struct lock_class_key rcu_sched_lock_key;
+struct lockdep_map rcu_sched_lock_map =
+	STATIC_LOCKDEP_MAP_INIT("rcu_read_lock_sched", &rcu_sched_lock_key);
+EXPORT_SYMBOL_GPL(rcu_sched_lock_map);
 #endif
 
 /*
-- 
1.5.2.5


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC tip/core/rcu 18/18] rcu: add primitives to check for RCU read-side critical sections
  2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
                   ` (16 preceding siblings ...)
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 17/18] rcu: Provide different lockdep classes for each flavor of RCU Paul E. McKenney
@ 2009-12-15 23:02 ` Paul E. McKenney
  2009-12-16  1:04   ` Josh Triplett
  2009-12-16 10:31   ` Peter Zijlstra
  17 siblings, 2 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-15 23:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
	niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells,
	Paul E. McKenney

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Proposed for 2.6.34, not for inclusion.

Create rcu_read_lock_held(), rcu_read_lock_bh_held(),
rcu_read_lock_sched_held(), and srcu_read_lock_held() primitives that
return non-zero if there might be the corresponding type of RCU read-side
critical section in effect at the time that they are invoked.  If there is
doubt, they report being in the critical section.  They give exact
answers if CONFIG_PROVE_LOCKING.

Also create rcu_dereference_check(), which takes a second boolean argument
into which one puts rcu_read_lock_held() or similar.  For example:

	rcu_dereference_check(gp, rcu_read_lock_held() ||
				  lockdep_is_held(my_lock));

There will likely need to be a lockdep_might_be_held() to handle the
case where debug_locks==0.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 include/linux/rcupdate.h |   85 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/rcutiny.h  |    4 --
 include/linux/rcutree.h  |    1 -
 include/linux/srcu.h     |   24 +++++++++++++
 kernel/rcutorture.c      |   12 +++++-
 kernel/rcutree_plugin.h  |   22 ------------
 lib/debug_locks.c        |    1 +
 7 files changed, 120 insertions(+), 29 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 723c564..84b891d 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -95,6 +95,70 @@ extern struct lockdep_map rcu_sched_lock_map;
 # define rcu_read_release_sched() \
 		lock_release(&rcu_sched_lock_map, 1, _THIS_IP_)
 
+/**
+ * rcu_read_lock_held - might we be in RCU read-side critical section?
+ *
+ * If CONFIG_PROVE_LOCKING is selected and enabled, returns nonzero iff in
+ * an RCU read-side critical section.  In absence of CONFIG_PROVE_LOCKING,
+ * this assumes we are in an RCU read-side critical section unless it can
+ * prove otherwise.
+ */
+static inline int rcu_read_lock_held(void)
+{
+	if (debug_locks)
+		return lock_is_held(&rcu_lock_map);
+	return 1;
+}
+
+/**
+ * rcu_read_lock_bh_held - might we be in RCU-bh read-side critical section?
+ *
+ * If CONFIG_PROVE_LOCKING is selected and enabled, returns nonzero iff in
+ * an RCU-bh read-side critical section.  In absence of CONFIG_PROVE_LOCKING,
+ * this assumes we are in an RCU-bh read-side critical section unless it can
+ * prove otherwise.
+ */
+static inline int rcu_read_lock_bh_held(void)
+{
+	if (debug_locks)
+		return lock_is_held(&rcu_bh_lock_map);
+	return 1;
+}
+
+/**
+ * rcu_read_lock_sched_held - might we be in RCU-sched read-side critical section?
+ *
+ * If CONFIG_PROVE_LOCKING is selected and enabled, returns nonzero iff in an
+ * RCU-sched read-side critical section.  In absence of CONFIG_PROVE_LOCKING,
+ * this assumes we are in an RCU-sched read-side critical section unless it
+ * can prove otherwise.  Note that disabling of preemption (including
+ * disabling irqs) counts as an RCU-sched read-side critical section.
+ */
+static inline int rcu_read_lock_sched_held(void)
+{
+	int lockdep_opinion = 0;
+
+	if (debug_locks)
+		lockdep_opinion = lock_is_held(&rcu_sched_lock_map);
+	return lockdep_opinion || preempt_count() != 0;
+}
+
+/**
+ * rcu_dereference_check - rcu_dereference with debug checking
+ *
+ * Do an rcu_dereference(), but check that the context is correct.
+ * For example, rcu_dereference_check(gp, rcu_read_lock_held()) to
+ * ensure that the rcu_dereference_check() executes within an RCU
+ * read-side critical section.  It is also possible to check for
+ * locks being held, for example, by using lockdep_is_held().
+ */
+#define rcu_dereference_check(p, c) \
+	({ \
+		if (debug_locks) \
+			WARN_ON_ONCE(!c); \
+		rcu_dereference(p); \
+	})
+
 #else /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
 
 # define rcu_read_acquire()		do { } while (0)
@@ -104,6 +168,27 @@ extern struct lockdep_map rcu_sched_lock_map;
 # define rcu_read_acquire_sched()	do { } while (0)
 # define rcu_read_release_sched()	do { } while (0)
 
+static inline int rcu_read_lock_held(void)
+{
+	return 1;
+}
+
+static inline int rcu_read_lock_bh_held(void)
+{
+	return 1;
+}
+
+static inline int rcu_read_lock_sched_held(void)
+{
+	return preempt_count() != 0;
+}
+
+#define rcu_dereference_check(p, c) \
+	({ \
+		(void)(c); \
+		rcu_dereference(p); \
+	})
+
 #endif /* #else #ifdef CONFIG_DEBUG_LOCK_ALLOC */
 
 /**
diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
index c32b16d..b524590 100644
--- a/include/linux/rcutiny.h
+++ b/include/linux/rcutiny.h
@@ -29,10 +29,6 @@
 
 void rcu_sched_qs(int cpu);
 void rcu_bh_qs(int cpu);
-static inline int rcu_read_lock_held(void)
-{
-	return 0;
-}
 
 #define __rcu_read_lock()	preempt_disable()
 #define __rcu_read_unlock()	preempt_enable()
diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
index 8cd4ac1..564a025 100644
--- a/include/linux/rcutree.h
+++ b/include/linux/rcutree.h
@@ -37,7 +37,6 @@ extern void rcu_bh_qs(int cpu);
 extern int rcu_needs_cpu(int cpu);
 extern void rcu_scheduler_starting(void);
 extern int rcu_expedited_torture_stats(char *page);
-extern int rcu_read_lock_held(void);
 
 #ifdef CONFIG_TREE_PREEMPT_RCU
 
diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 5a07b90..9404d35 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -52,14 +52,38 @@ void synchronize_srcu_expedited(struct srcu_struct *sp);
 long srcu_batches_completed(struct srcu_struct *sp);
 
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
+
 extern struct lockdep_map srcu_lock_map;
 # define srcu_read_acquire() \
 		lock_acquire(&srcu_lock_map, 0, 0, 2, 1, NULL, _THIS_IP_)
 # define srcu_read_release() \
 		lock_release(&srcu_lock_map, 1, _THIS_IP_)
+
+/**
+ * srcu_read_lock_held - might we be in SRCU read-side critical section?
+ *
+ * If CONFIG_PROVE_LOCKING is selected and enabled, returns nonzero iff in
+ * an SRCU read-side critical section.  In absence of CONFIG_PROVE_LOCKING,
+ * this assumes we are in an SRCU read-side critical section unless it can
+ * prove otherwise.
+ */
+static inline int srcu_read_lock_held(void)
+{
+	if (debug_locks)
+		return lock_is_held(&srcu_lock_map);
+	return 1;
+}
+
 #else /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
+
 # define srcu_read_acquire()	do { } while (0)
 # define srcu_read_release()	do { } while (0)
+
+static inline int srcu_read_lock_held(void)
+{
+	return 1;
+}
+
 #endif /* #else #ifdef CONFIG_DEBUG_LOCK_ALLOC */
 
 /**
diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index b4096d3..dc986f0 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -796,7 +796,11 @@ static void rcu_torture_timer(unsigned long unused)
 
 	idx = cur_ops->readlock();
 	completed = cur_ops->completed();
-	p = rcu_dereference(rcu_torture_current);
+	p = rcu_dereference_check(rcu_torture_current,
+				  rcu_read_lock_held() ||
+				  rcu_read_lock_bh_held() ||
+				  rcu_read_lock_sched_held() ||
+				  srcu_read_lock_held());
 	if (p == NULL) {
 		/* Leave because rcu_torture_writer is not yet underway */
 		cur_ops->readunlock(idx);
@@ -853,7 +857,11 @@ rcu_torture_reader(void *arg)
 		}
 		idx = cur_ops->readlock();
 		completed = cur_ops->completed();
-		p = rcu_dereference(rcu_torture_current);
+		p = rcu_dereference_check(rcu_torture_current,
+					  rcu_read_lock_held() ||
+					  rcu_read_lock_bh_held() ||
+					  rcu_read_lock_sched_held() ||
+					  srcu_read_lock_held());
 		if (p == NULL) {
 			/* Wait for rcu_torture_writer to get underway */
 			cur_ops->readunlock(idx);
diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
index f6258ae..e77cdf3 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.h
@@ -310,18 +310,6 @@ void __rcu_read_unlock(void)
 }
 EXPORT_SYMBOL_GPL(__rcu_read_unlock);
 
-/*
- * Return 1 if the current task is provably within an RCU read-side
- * critical section.  The bit about checking a running task to see if
- * it is blocked is a bit strange, but keep in mind that sleep and
- * wakeup are not atomic operations.
- */
-int rcu_read_lock_held(void)
-{
-	return ACCESS_ONCE(current->rcu_read_lock_nesting) != 0 ||
-	       (current->rcu_read_unlock_special & RCU_READ_UNLOCK_BLOCKED);
-}
-
 #ifdef CONFIG_RCU_CPU_STALL_DETECTOR
 
 /*
@@ -773,16 +761,6 @@ static void rcu_report_unblock_qs_rnp(struct rcu_node *rnp, unsigned long flags)
 
 #endif /* #ifdef CONFIG_HOTPLUG_CPU */
 
-/*
- * Return 1 if the current task is provably within an RCU read-side
- * critical section.  But without preemptible RCU, we never can be
- * sure, so always return 0.
- */
-int rcu_read_lock_held(void)
-{
-	return 0;
-}
-
 #ifdef CONFIG_RCU_CPU_STALL_DETECTOR
 
 /*
diff --git a/lib/debug_locks.c b/lib/debug_locks.c
index bc3b117..5bf0020 100644
--- a/lib/debug_locks.c
+++ b/lib/debug_locks.c
@@ -23,6 +23,7 @@
  * shut up after that.
  */
 int debug_locks = 1;
+EXPORT_SYMBOL_GPL(debug_locks);
 
 /*
  * The locking-testsuite uses <debug_locks_silent> to get a
-- 
1.5.2.5


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC tip/core/rcu 12/18] rcu: make MAINTAINERS file match new RCU reality
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 12/18] rcu: make MAINTAINERS file match new RCU reality Paul E. McKenney
@ 2009-12-16  0:53   ` Josh Triplett
  0 siblings, 0 replies; 31+ messages in thread
From: Josh Triplett @ 2009-12-16  0:53 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	dvhltc, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells

On Tue, Dec 15, 2009 at 03:02:35PM -0800, Paul E. McKenney wrote:
> Proposed for 2.6.34, not for inclusion.
> 
> Both rcutorture and RCU are supported.  Also, update the files
> to match the new layout.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

At this point, I think it would make sense to just merge the rcutorture
entry into the main RCU entry; no real point keeping them separate.

- Josh Triplett

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC tip/core/rcu 15/18] rcu: give different levels of the rcu_node hierarchy distinct lockdep names
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 15/18] rcu: give different levels of the rcu_node hierarchy distinct lockdep names Paul E. McKenney
@ 2009-12-16  0:59   ` Josh Triplett
  2009-12-16  1:59     ` Paul E. McKenney
  2009-12-16 10:26   ` Peter Zijlstra
  1 sibling, 1 reply; 31+ messages in thread
From: Josh Triplett @ 2009-12-16  0:59 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	dvhltc, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells

On Tue, Dec 15, 2009 at 03:02:38PM -0800, Paul E. McKenney wrote:
> From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> Proposed for 2.6.34, not for inclusion.
> 
> Previously, each level of the rcu_node hierarchy had the same rather
> unimaginative name: "&rcu_node_class[i]".  This makes lockdep diagnostics
> involving these lockdep classes less helpful than would be nice.  This
> patch fixes this by giving each level of the rcu_node hierarchy a distinct
> name: "rcu_node_level_0", "rcu_node_level_1", and so on.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
>  kernel/rcutree.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> index 0a4c328..a6e45f6 100644
> --- a/kernel/rcutree.c
> +++ b/kernel/rcutree.c
> @@ -1811,11 +1811,17 @@ static void __init rcu_init_levelspread(struct rcu_state *rsp)
>   */
>  static void __init rcu_init_one(struct rcu_state *rsp)
>  {
> +	static char *buf[] = { "rcu_node_level_0",
> +			       "rcu_node_level_1",
> +			       "rcu_node_level_2",
> +			       "rcu_node_level_3" };  /* Match MAX_RCU_LVLS */
>  	int cpustride = 1;
>  	int i;
>  	int j;
>  	struct rcu_node *rnp;
>  
> +	WARN_ON_ONCE(MAX_RCU_LVLS > 4);  /* Fix buf[] initialization! */
> +

BUILD_BUG_ON seems better.  For that matter, please consider moving
these near the rest of the level-specific defines, making them const,
and ideally not emitting the strings for levels you don't have.

- Josh Triplett

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC tip/core/rcu 18/18] rcu: add primitives to check for RCU read-side critical sections
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 18/18] rcu: add primitives to check for RCU read-side critical sections Paul E. McKenney
@ 2009-12-16  1:04   ` Josh Triplett
  2009-12-16  2:08     ` Paul E. McKenney
  2009-12-16 10:31   ` Peter Zijlstra
  1 sibling, 1 reply; 31+ messages in thread
From: Josh Triplett @ 2009-12-16  1:04 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	dvhltc, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells

On Tue, Dec 15, 2009 at 03:02:41PM -0800, Paul E. McKenney wrote:
> From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> Proposed for 2.6.34, not for inclusion.
> 
> Create rcu_read_lock_held(), rcu_read_lock_bh_held(),
> rcu_read_lock_sched_held(), and srcu_read_lock_held() primitives that
> return non-zero if there might be the corresponding type of RCU read-side
> critical section in effect at the time that they are invoked.  If there is
> doubt, they report being in the critical section.  They give exact
> answers if CONFIG_PROVE_LOCKING.
> 
> Also create rcu_dereference_check(), which takes a second boolean argument
> into which one puts rcu_read_lock_held() or similar.  For example:
> 
> 	rcu_dereference_check(gp, rcu_read_lock_held() ||
> 				  lockdep_is_held(my_lock));

Useful for the case where you do have an additional lock, but it seems
like it would help to have variants for the most common cases;
specifically:
rcu_dereference_check(thing, rcu_read_lock_held())
rcu_dereference_check(thing, rcu_read_lock_bh_held())
and so on.

Even then, it seems painful to have to annotate each rcu_dereference.
Ideally, I'd propose the reverse: annotate any rcu_dereference which
*can* occur outside an RCU read-side critical section.  (Variants of RCU
notwithstanding...)

- Josh Triplett

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC tip/core/rcu 15/18] rcu: give different levels of the rcu_node hierarchy distinct lockdep names
  2009-12-16  0:59   ` Josh Triplett
@ 2009-12-16  1:59     ` Paul E. McKenney
  0 siblings, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-16  1:59 UTC (permalink / raw)
  To: Josh Triplett
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	dvhltc, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells

On Tue, Dec 15, 2009 at 04:59:59PM -0800, Josh Triplett wrote:
> On Tue, Dec 15, 2009 at 03:02:38PM -0800, Paul E. McKenney wrote:
> > From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > 
> > Proposed for 2.6.34, not for inclusion.
> > 
> > Previously, each level of the rcu_node hierarchy had the same rather
> > unimaginative name: "&rcu_node_class[i]".  This makes lockdep diagnostics
> > involving these lockdep classes less helpful than would be nice.  This
> > patch fixes this by giving each level of the rcu_node hierarchy a distinct
> > name: "rcu_node_level_0", "rcu_node_level_1", and so on.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > ---
> >  kernel/rcutree.c |    9 ++++++++-
> >  1 files changed, 8 insertions(+), 1 deletions(-)
> > 
> > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > index 0a4c328..a6e45f6 100644
> > --- a/kernel/rcutree.c
> > +++ b/kernel/rcutree.c
> > @@ -1811,11 +1811,17 @@ static void __init rcu_init_levelspread(struct rcu_state *rsp)
> >   */
> >  static void __init rcu_init_one(struct rcu_state *rsp)
> >  {
> > +	static char *buf[] = { "rcu_node_level_0",
> > +			       "rcu_node_level_1",
> > +			       "rcu_node_level_2",
> > +			       "rcu_node_level_3" };  /* Match MAX_RCU_LVLS */
> >  	int cpustride = 1;
> >  	int i;
> >  	int j;
> >  	struct rcu_node *rnp;
> >  
> > +	WARN_ON_ONCE(MAX_RCU_LVLS > 4);  /* Fix buf[] initialization! */
> > +
> 
> BUILD_BUG_ON seems better.  For that matter, please consider moving
> these near the rest of the level-specific defines, making them const,
> and ideally not emitting the strings for levels you don't have.

Good point, I had forgotten about BUILD_BUG_ON().  And reviewing its
definition, I can see why one might be motivated to forget.  ;-)

The reason I put the definition of buf[] here and the reason that I am not
worried about the memory it consumes is that this is an __init function,
so its memory should be reused once the system boots.  And if __init
does not apply to the static variables in a function, it should!  ;-)

							Thanx, Paul

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC tip/core/rcu 18/18] rcu: add primitives to check for RCU read-side critical sections
  2009-12-16  1:04   ` Josh Triplett
@ 2009-12-16  2:08     ` Paul E. McKenney
  0 siblings, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-16  2:08 UTC (permalink / raw)
  To: Josh Triplett
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	dvhltc, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells

On Tue, Dec 15, 2009 at 05:04:39PM -0800, Josh Triplett wrote:
> On Tue, Dec 15, 2009 at 03:02:41PM -0800, Paul E. McKenney wrote:
> > From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > 
> > Proposed for 2.6.34, not for inclusion.
> > 
> > Create rcu_read_lock_held(), rcu_read_lock_bh_held(),
> > rcu_read_lock_sched_held(), and srcu_read_lock_held() primitives that
> > return non-zero if there might be the corresponding type of RCU read-side
> > critical section in effect at the time that they are invoked.  If there is
> > doubt, they report being in the critical section.  They give exact
> > answers if CONFIG_PROVE_LOCKING.
> > 
> > Also create rcu_dereference_check(), which takes a second boolean argument
> > into which one puts rcu_read_lock_held() or similar.  For example:
> > 
> > 	rcu_dereference_check(gp, rcu_read_lock_held() ||
> > 				  lockdep_is_held(my_lock));
> 
> Useful for the case where you do have an additional lock, but it seems
> like it would help to have variants for the most common cases;
> specifically:
> rcu_dereference_check(thing, rcu_read_lock_held())
> rcu_dereference_check(thing, rcu_read_lock_bh_held())
> and so on.

I figured that I would try applying rcu_dereference_check() to a few
places and see what the most common variants really are.  I briefly
considered defining the set, but choked on the possibility of code
that might be executed from within several different variants of RCU,
so decided to start with the single general-purpose variant.

> Even then, it seems painful to have to annotate each rcu_dereference.
> Ideally, I'd propose the reverse: annotate any rcu_dereference which
> *can* occur outside an RCU read-side critical section.  (Variants of RCU
> notwithstanding...)

I was completely with you to start with, but...

Peter Zijlstra reminded me that his earlier foray into this space turned
up a number of situations where the code containing rcu_dereference()
simply cannot know which combinations of locks and/or RCU flavors should
be in effect.  One example of this is the RCU-protected trie, which -can-
be RCU-protected on read, but -also- can be used with locks.  And each
caller of course would supply a different lock.

I would not be surprised to find that we end up wanting a shorthand
for rcu_dereference_check(thing, rcu_read_lock_held()), but am less sure
about the others.

							Thanx, Paul

PS. And thank you for looking these over!

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC tip/core/rcu 14/18] rcu: lockdep check for exiting to user space as RCU reader
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 14/18] rcu: lockdep check for exiting to user space as RCU reader Paul E. McKenney
@ 2009-12-16 10:24   ` Peter Zijlstra
  2009-12-16 15:11     ` Paul E. McKenney
  0 siblings, 1 reply; 31+ messages in thread
From: Peter Zijlstra @ 2009-12-16 10:24 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, dvhltc, niv, tglx, rostedt, Valdis.Kletnieks, dhowells

On Tue, 2009-12-15 at 15:02 -0800, Paul E. McKenney wrote:
> From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> Proposed for 2.6.34, not for inclusion.
> 
> It is illegal to return to user-space execution while running within an
> RCU read-side critical section.  It turns out that CONFIG_TREE_PREEMPT_RCU
> has enough information lying around to detect this, so add the checks
> to lockdep (CONFIG_PROVE_LOCKING).

But uhm, we already track the rcu read lock as a regular lockdep lock,
so it should already check this, no?



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC tip/core/rcu 15/18] rcu: give different levels of the rcu_node hierarchy distinct lockdep names
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 15/18] rcu: give different levels of the rcu_node hierarchy distinct lockdep names Paul E. McKenney
  2009-12-16  0:59   ` Josh Triplett
@ 2009-12-16 10:26   ` Peter Zijlstra
  2009-12-16 10:33     ` Peter Zijlstra
  2009-12-16 15:13     ` Paul E. McKenney
  1 sibling, 2 replies; 31+ messages in thread
From: Peter Zijlstra @ 2009-12-16 10:26 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, dvhltc, niv, tglx, rostedt, Valdis.Kletnieks, dhowells

On Tue, 2009-12-15 at 15:02 -0800, Paul E. McKenney wrote:
> From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> Proposed for 2.6.34, not for inclusion.
> 
> Previously, each level of the rcu_node hierarchy had the same rather
> unimaginative name: "&rcu_node_class[i]".  This makes lockdep diagnostics
> involving these lockdep classes less helpful than would be nice.  This
> patch fixes this by giving each level of the rcu_node hierarchy a distinct
> name: "rcu_node_level_0", "rcu_node_level_1", and so on.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
>  kernel/rcutree.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> index 0a4c328..a6e45f6 100644
> --- a/kernel/rcutree.c
> +++ b/kernel/rcutree.c
> @@ -1811,11 +1811,17 @@ static void __init rcu_init_levelspread(struct rcu_state *rsp)
>   */
>  static void __init rcu_init_one(struct rcu_state *rsp)
>  {
> +	static char *buf[] = { "rcu_node_level_0",
> +			       "rcu_node_level_1",
> +			       "rcu_node_level_2",
> +			       "rcu_node_level_3" };  /* Match MAX_RCU_LVLS */
>  	int cpustride = 1;
>  	int i;
>  	int j;
>  	struct rcu_node *rnp;
>  
> +	WARN_ON_ONCE(MAX_RCU_LVLS > 4);  /* Fix buf[] initialization! */

So you're going to WARN here,

>  	/* Initialize the level-tracking arrays. */
>  
>  	for (i = 1; i < NUM_RCU_LVLS; i++)
> @@ -1829,7 +1835,8 @@ static void __init rcu_init_one(struct rcu_state *rsp)
>  		rnp = rsp->level[i];
>  		for (j = 0; j < rsp->levelcnt[i]; j++, rnp++) {
>  			spin_lock_init(&rnp->lock);
> -			lockdep_set_class(&rnp->lock, &rcu_node_class[i]);
> +			lockdep_set_class_and_name(&rnp->lock,
> +						   &rcu_node_class[i], buf[i]);

and segfault here because i overruns its bounds?

Might as well BUG_ON then.


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC tip/core/rcu 18/18] rcu: add primitives to check for RCU read-side critical sections
  2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 18/18] rcu: add primitives to check for RCU read-side critical sections Paul E. McKenney
  2009-12-16  1:04   ` Josh Triplett
@ 2009-12-16 10:31   ` Peter Zijlstra
  2009-12-16 15:18     ` Paul E. McKenney
  1 sibling, 1 reply; 31+ messages in thread
From: Peter Zijlstra @ 2009-12-16 10:31 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, dvhltc, niv, tglx, rostedt, Valdis.Kletnieks, dhowells

On Tue, 2009-12-15 at 15:02 -0800, Paul E. McKenney wrote:
> 
> Also create rcu_dereference_check(), which takes a second boolean argument
> into which one puts rcu_read_lock_held() or similar.  For example:
> 
>         rcu_dereference_check(gp, rcu_read_lock_held() ||
>                                   lockdep_is_held(my_lock));

Ah, so you're going to tackle this the other way around, interesting :-)


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC tip/core/rcu 15/18] rcu: give different levels of the rcu_node hierarchy distinct lockdep names
  2009-12-16 10:26   ` Peter Zijlstra
@ 2009-12-16 10:33     ` Peter Zijlstra
  2009-12-16 15:13     ` Paul E. McKenney
  1 sibling, 0 replies; 31+ messages in thread
From: Peter Zijlstra @ 2009-12-16 10:33 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, dvhltc, niv, tglx, rostedt, Valdis.Kletnieks, dhowells

On Wed, 2009-12-16 at 11:26 +0100, Peter Zijlstra wrote:
> On Tue, 2009-12-15 at 15:02 -0800, Paul E. McKenney wrote:
> > From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > 
> > Proposed for 2.6.34, not for inclusion.
> > 
> > Previously, each level of the rcu_node hierarchy had the same rather
> > unimaginative name: "&rcu_node_class[i]".  This makes lockdep diagnostics
> > involving these lockdep classes less helpful than would be nice.  This
> > patch fixes this by giving each level of the rcu_node hierarchy a distinct
> > name: "rcu_node_level_0", "rcu_node_level_1", and so on.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > ---
> >  kernel/rcutree.c |    9 ++++++++-
> >  1 files changed, 8 insertions(+), 1 deletions(-)
> > 
> > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > index 0a4c328..a6e45f6 100644
> > --- a/kernel/rcutree.c
> > +++ b/kernel/rcutree.c
> > @@ -1811,11 +1811,17 @@ static void __init rcu_init_levelspread(struct rcu_state *rsp)
> >   */
> >  static void __init rcu_init_one(struct rcu_state *rsp)
> >  {
> > +	static char *buf[] = { "rcu_node_level_0",
> > +			       "rcu_node_level_1",
> > +			       "rcu_node_level_2",
> > +			       "rcu_node_level_3" };  /* Match MAX_RCU_LVLS */
> >  	int cpustride = 1;
> >  	int i;
> >  	int j;
> >  	struct rcu_node *rnp;
> >  
> > +	WARN_ON_ONCE(MAX_RCU_LVLS > 4);  /* Fix buf[] initialization! */

An even better option:

 BUILD_BUG_ON(MAX_RCU_LVLS > array_size(buf));

That way it'll fail to even build when these two get out of whack.


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC tip/core/rcu 14/18] rcu: lockdep check for exiting to user space as RCU reader
  2009-12-16 10:24   ` Peter Zijlstra
@ 2009-12-16 15:11     ` Paul E. McKenney
  0 siblings, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-16 15:11 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, dvhltc, niv, tglx, rostedt, Valdis.Kletnieks, dhowells

On Wed, Dec 16, 2009 at 11:24:42AM +0100, Peter Zijlstra wrote:
> On Tue, 2009-12-15 at 15:02 -0800, Paul E. McKenney wrote:
> > From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > 
> > Proposed for 2.6.34, not for inclusion.
> > 
> > It is illegal to return to user-space execution while running within an
> > RCU read-side critical section.  It turns out that CONFIG_TREE_PREEMPT_RCU
> > has enough information lying around to detect this, so add the checks
> > to lockdep (CONFIG_PROVE_LOCKING).
> 
> But uhm, we already track the rcu read lock as a regular lockdep lock,
> so it should already check this, no?

I guess I can drop that patch, then.  ;-)

Thank you for looking this over!

							Thanx, Paul

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC tip/core/rcu 15/18] rcu: give different levels of the rcu_node hierarchy distinct lockdep names
  2009-12-16 10:26   ` Peter Zijlstra
  2009-12-16 10:33     ` Peter Zijlstra
@ 2009-12-16 15:13     ` Paul E. McKenney
  1 sibling, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-16 15:13 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, dvhltc, niv, tglx, rostedt, Valdis.Kletnieks, dhowells

On Wed, Dec 16, 2009 at 11:26:00AM +0100, Peter Zijlstra wrote:
> On Tue, 2009-12-15 at 15:02 -0800, Paul E. McKenney wrote:
> > From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > 
> > Proposed for 2.6.34, not for inclusion.
> > 
> > Previously, each level of the rcu_node hierarchy had the same rather
> > unimaginative name: "&rcu_node_class[i]".  This makes lockdep diagnostics
> > involving these lockdep classes less helpful than would be nice.  This
> > patch fixes this by giving each level of the rcu_node hierarchy a distinct
> > name: "rcu_node_level_0", "rcu_node_level_1", and so on.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > ---
> >  kernel/rcutree.c |    9 ++++++++-
> >  1 files changed, 8 insertions(+), 1 deletions(-)
> > 
> > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > index 0a4c328..a6e45f6 100644
> > --- a/kernel/rcutree.c
> > +++ b/kernel/rcutree.c
> > @@ -1811,11 +1811,17 @@ static void __init rcu_init_levelspread(struct rcu_state *rsp)
> >   */
> >  static void __init rcu_init_one(struct rcu_state *rsp)
> >  {
> > +	static char *buf[] = { "rcu_node_level_0",
> > +			       "rcu_node_level_1",
> > +			       "rcu_node_level_2",
> > +			       "rcu_node_level_3" };  /* Match MAX_RCU_LVLS */
> >  	int cpustride = 1;
> >  	int i;
> >  	int j;
> >  	struct rcu_node *rnp;
> >  
> > +	WARN_ON_ONCE(MAX_RCU_LVLS > 4);  /* Fix buf[] initialization! */
> 
> So you're going to WARN here,
> 
> >  	/* Initialize the level-tracking arrays. */
> >  
> >  	for (i = 1; i < NUM_RCU_LVLS; i++)
> > @@ -1829,7 +1835,8 @@ static void __init rcu_init_one(struct rcu_state *rsp)
> >  		rnp = rsp->level[i];
> >  		for (j = 0; j < rsp->levelcnt[i]; j++, rnp++) {
> >  			spin_lock_init(&rnp->lock);
> > -			lockdep_set_class(&rnp->lock, &rcu_node_class[i]);
> > +			lockdep_set_class_and_name(&rnp->lock,
> > +						   &rcu_node_class[i], buf[i]);
> 
> and segfault here because i overruns its bounds?

Might or might not, depending on memory layout.

> Might as well BUG_ON then.

I will give BUILD_BUG_ON() a try, but with the array size computed at
compile time as you suggest elsewhere.

							Thanx, Paul

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC tip/core/rcu 18/18] rcu: add primitives to check for RCU read-side critical sections
  2009-12-16 10:31   ` Peter Zijlstra
@ 2009-12-16 15:18     ` Paul E. McKenney
  0 siblings, 0 replies; 31+ messages in thread
From: Paul E. McKenney @ 2009-12-16 15:18 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, dvhltc, niv, tglx, rostedt, Valdis.Kletnieks, dhowells

On Wed, Dec 16, 2009 at 11:31:34AM +0100, Peter Zijlstra wrote:
> On Tue, 2009-12-15 at 15:02 -0800, Paul E. McKenney wrote:
> > 
> > Also create rcu_dereference_check(), which takes a second boolean argument
> > into which one puts rcu_read_lock_held() or similar.  For example:
> > 
> >         rcu_dereference_check(gp, rcu_read_lock_held() ||
> >                                   lockdep_is_held(my_lock));
> 
> Ah, so you're going to tackle this the other way around, interesting :-)

Still feeling my way around this one, but for the moment, yes.  ;-)

One potential issue is that for lockdep, avoiding false positives means
erring on the side of the lock -not- being held, while for this approach
to rcu_dereference() checking, avoiding false positives means erring on
the side of the lock being held.

I might need to create a CONFIG_PROVE_LOCKING_RCU to allow shutting off
the more-detailed RCU checking when people want to do partial deadlock
checking, but will see how it goes.

							Thanx, Paul

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2009-12-16 15:18 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-15 23:02 [PATCH RFC tip/core/rcu 0/18] rcu: simplify race conditions, add checking Paul E. McKenney
2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 01/18] rcu: adjust force_quiescent_state() locking, step 1 Paul E. McKenney
2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 02/18] rcu: adjust force_quiescent_state() locking, step 2 Paul E. McKenney
2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 03/18] rcu: prohibit starting new grace periods while forcing quiescent states Paul E. McKenney
2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 04/18] rcu: eliminate local variable signaled from force_quiescent_state() Paul E. McKenney
2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 05/18] rcu: eliminate local variable lastcomp " Paul E. McKenney
2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 06/18] rcu: eliminate second argument of rcu_process_dyntick() Paul E. McKenney
2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 07/18] rcu: eliminate rcu_process_dyntick() return value Paul E. McKenney
2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 08/18] rcu: remove leg of force_quiescent_state() switch statement Paul E. McKenney
2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 09/18] rcu: remove redundant grace-period check Paul E. McKenney
2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 10/18] rcu: make force_quiescent_state() start grace period if needed Paul E. McKenney
2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 11/18] rcu: add force_quiescent_state() testing to rcutorture Paul E. McKenney
2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 12/18] rcu: make MAINTAINERS file match new RCU reality Paul E. McKenney
2009-12-16  0:53   ` Josh Triplett
2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 13/18] rcu: add debug check for too many rcu_read_unlock() Paul E. McKenney
2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 14/18] rcu: lockdep check for exiting to user space as RCU reader Paul E. McKenney
2009-12-16 10:24   ` Peter Zijlstra
2009-12-16 15:11     ` Paul E. McKenney
2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 15/18] rcu: give different levels of the rcu_node hierarchy distinct lockdep names Paul E. McKenney
2009-12-16  0:59   ` Josh Triplett
2009-12-16  1:59     ` Paul E. McKenney
2009-12-16 10:26   ` Peter Zijlstra
2009-12-16 10:33     ` Peter Zijlstra
2009-12-16 15:13     ` Paul E. McKenney
2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 16/18] rcu: make lockdep aware of SRCU read-side critical sections Paul E. McKenney
2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 17/18] rcu: Provide different lockdep classes for each flavor of RCU Paul E. McKenney
2009-12-15 23:02 ` [PATCH RFC tip/core/rcu 18/18] rcu: add primitives to check for RCU read-side critical sections Paul E. McKenney
2009-12-16  1:04   ` Josh Triplett
2009-12-16  2:08     ` Paul E. McKenney
2009-12-16 10:31   ` Peter Zijlstra
2009-12-16 15:18     ` Paul E. McKenney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox