* [PATCH tip/core/rcu 0/3] rcu: improve diagnostics and documentation of source files
@ 2010-01-05 0:03 Paul E. McKenney
2010-01-05 0:04 ` [PATCH tip/core/rcu 1/3] rcu: make MAINTAINERS file match new RCU reality Paul E. McKenney
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Paul E. McKenney @ 2010-01-05 0:03 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, dvhltc,
niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells
Hello!
This patch set corrects the file specifiers for the RCU sources in the
MAINTAINERS file and also makes a couple of minor improvements to RCU's
diagnostics.
Thanx, Paul
MAINTAINERS | 14 +++++++-------
kernel/rcutree.c | 10 ++++++++--
kernel/rcutree_plugin.h | 4 +++-
3 files changed, 18 insertions(+), 10 deletions(-)
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH tip/core/rcu 1/3] rcu: make MAINTAINERS file match new RCU reality 2010-01-05 0:03 [PATCH tip/core/rcu 0/3] rcu: improve diagnostics and documentation of source files Paul E. McKenney @ 2010-01-05 0:04 ` Paul E. McKenney 2010-01-13 10:27 ` [tip:core/rcu] rcu: Make " tip-bot for Paul E. McKenney 2010-01-05 0:04 ` [PATCH tip/core/rcu 2/3] rcu: add debug check for too many rcu_read_unlock() Paul E. McKenney 2010-01-05 0:04 ` [PATCH tip/core/rcu 3/3] rcu: give different levels of the rcu_node hierarchy distinct lockdep names Paul E. McKenney 2 siblings, 1 reply; 12+ messages in thread From: Paul E. McKenney @ 2010-01-05 0:04 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 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] 12+ messages in thread
* [tip:core/rcu] rcu: Make MAINTAINERS file match new RCU reality 2010-01-05 0:04 ` [PATCH tip/core/rcu 1/3] rcu: make MAINTAINERS file match new RCU reality Paul E. McKenney @ 2010-01-13 10:27 ` tip-bot for Paul E. McKenney 0 siblings, 0 replies; 12+ messages in thread From: tip-bot for Paul E. McKenney @ 2010-01-13 10:27 UTC (permalink / raw) To: linux-tip-commits; +Cc: linux-kernel, paulmck, hpa, mingo, tglx, mingo Commit-ID: f9094d8e5587cf21091a9516628147c0b55e4264 Gitweb: http://git.kernel.org/tip/f9094d8e5587cf21091a9516628147c0b55e4264 Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com> AuthorDate: Mon, 4 Jan 2010 16:04:00 -0800 Committer: Ingo Molnar <mingo@elte.hu> CommitDate: Wed, 13 Jan 2010 09:06:06 +0100 rcu: Make MAINTAINERS file match new RCU reality 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> Cc: laijs@cn.fujitsu.com Cc: dipankar@in.ibm.com Cc: mathieu.desnoyers@polymtl.ca Cc: josh@joshtriplett.org Cc: dvhltc@us.ibm.com Cc: niv@us.ibm.com Cc: peterz@infradead.org Cc: rostedt@goodmis.org Cc: Valdis.Kletnieks@vt.edu Cc: dhowells@redhat.com LKML-Reference: <12626498422334-git-send-email-> Signed-off-by: Ingo Molnar <mingo@elte.hu> --- MAINTAINERS | 13 +++++++------ 1 files changed, 7 insertions(+), 6 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index c8f47bf..175da7c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4509,7 +4509,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 @@ -4534,11 +4534,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> ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH tip/core/rcu 2/3] rcu: add debug check for too many rcu_read_unlock() 2010-01-05 0:03 [PATCH tip/core/rcu 0/3] rcu: improve diagnostics and documentation of source files Paul E. McKenney 2010-01-05 0:04 ` [PATCH tip/core/rcu 1/3] rcu: make MAINTAINERS file match new RCU reality Paul E. McKenney @ 2010-01-05 0:04 ` Paul E. McKenney 2010-01-05 2:03 ` Josh Triplett 2010-01-13 10:27 ` [tip:core/rcu] rcu: Add " tip-bot for Paul E. McKenney 2010-01-05 0:04 ` [PATCH tip/core/rcu 3/3] rcu: give different levels of the rcu_node hierarchy distinct lockdep names Paul E. McKenney 2 siblings, 2 replies; 12+ messages in thread From: Paul E. McKenney @ 2010-01-05 0:04 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> 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] 12+ messages in thread
* Re: [PATCH tip/core/rcu 2/3] rcu: add debug check for too many rcu_read_unlock() 2010-01-05 0:04 ` [PATCH tip/core/rcu 2/3] rcu: add debug check for too many rcu_read_unlock() Paul E. McKenney @ 2010-01-05 2:03 ` Josh Triplett 2010-01-05 2:19 ` Paul E. McKenney 2010-01-13 10:27 ` [tip:core/rcu] rcu: Add " tip-bot for Paul E. McKenney 1 sibling, 1 reply; 12+ messages in thread From: Josh Triplett @ 2010-01-05 2:03 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 Mon, Jan 04, 2010 at 04:04:01PM -0800, Paul E. McKenney wrote: > From: Paul E. McKenney <paulmck@linux.vnet.ibm.com> > > 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); Given that you *already* need to access t->rcu_read_lock_nesting here, why not just do the test all the time? Ideally you could access t->rcu_read_lock_nesting once, decrement it, and test for both 0 and negative. - Josh Triplett ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH tip/core/rcu 2/3] rcu: add debug check for too many rcu_read_unlock() 2010-01-05 2:03 ` Josh Triplett @ 2010-01-05 2:19 ` Paul E. McKenney 2010-01-05 2:28 ` Josh Triplett 0 siblings, 1 reply; 12+ messages in thread From: Paul E. McKenney @ 2010-01-05 2:19 UTC (permalink / raw) To: Josh Triplett Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers, dvhltc, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells On Mon, Jan 04, 2010 at 06:03:08PM -0800, Josh Triplett wrote: > On Mon, Jan 04, 2010 at 04:04:01PM -0800, Paul E. McKenney wrote: > > From: Paul E. McKenney <paulmck@linux.vnet.ibm.com> > > > > 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); > > Given that you *already* need to access t->rcu_read_lock_nesting here, > why not just do the test all the time? Ideally you could access > t->rcu_read_lock_nesting once, decrement it, and test for both 0 and > negative. Because I was paranoid about the extra branch. Perhaps needlessly paranoid, but this is rcu_read_unlock() we are talking about here. ;-) You seem to be suggesting making the first test be "<=", then sorting things out later, but given that both the equals-zero and the greater-than-zero cases are quite common, I couldn't figure out how to avoid the extra test and branch in the common case. Hence the #ifdef. Thanx, Paul ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH tip/core/rcu 2/3] rcu: add debug check for too many rcu_read_unlock() 2010-01-05 2:19 ` Paul E. McKenney @ 2010-01-05 2:28 ` Josh Triplett 2010-01-05 16:21 ` Paul E. McKenney 0 siblings, 1 reply; 12+ messages in thread From: Josh Triplett @ 2010-01-05 2:28 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 Mon, Jan 04, 2010 at 06:19:19PM -0800, Paul E. McKenney wrote: > On Mon, Jan 04, 2010 at 06:03:08PM -0800, Josh Triplett wrote: > > On Mon, Jan 04, 2010 at 04:04:01PM -0800, Paul E. McKenney wrote: > > > From: Paul E. McKenney <paulmck@linux.vnet.ibm.com> > > > > > > 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); > > > > Given that you *already* need to access t->rcu_read_lock_nesting here, > > why not just do the test all the time? Ideally you could access > > t->rcu_read_lock_nesting once, decrement it, and test for both 0 and > > negative. > > Because I was paranoid about the extra branch. Perhaps needlessly > paranoid, but this is rcu_read_unlock() we are talking about here. ;-) > > You seem to be suggesting making the first test be "<=", then > sorting things out later, but given that both the equals-zero and the > greater-than-zero cases are quite common, I couldn't figure out how to > avoid the extra test and branch in the common case. Hence the #ifdef. No, I think you could simply read the predecremented value into a local variable, test it once with == 0, then have the WARN_ON_ONCE, and hope that the compiler figures out it can just test the register once and then do multiple jumps on the same flags. You could try it and see what code it generates. - Josh Triplett ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH tip/core/rcu 2/3] rcu: add debug check for too many rcu_read_unlock() 2010-01-05 2:28 ` Josh Triplett @ 2010-01-05 16:21 ` Paul E. McKenney 2010-01-05 17:08 ` Josh Triplett 0 siblings, 1 reply; 12+ messages in thread From: Paul E. McKenney @ 2010-01-05 16:21 UTC (permalink / raw) To: Josh Triplett Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers, dvhltc, niv, tglx, peterz, rostedt, Valdis.Kletnieks, dhowells On Mon, Jan 04, 2010 at 06:28:15PM -0800, Josh Triplett wrote: > On Mon, Jan 04, 2010 at 06:19:19PM -0800, Paul E. McKenney wrote: > > On Mon, Jan 04, 2010 at 06:03:08PM -0800, Josh Triplett wrote: > > > On Mon, Jan 04, 2010 at 04:04:01PM -0800, Paul E. McKenney wrote: > > > > From: Paul E. McKenney <paulmck@linux.vnet.ibm.com> > > > > > > > > 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); > > > > > > Given that you *already* need to access t->rcu_read_lock_nesting here, > > > why not just do the test all the time? Ideally you could access > > > t->rcu_read_lock_nesting once, decrement it, and test for both 0 and > > > negative. > > > > Because I was paranoid about the extra branch. Perhaps needlessly > > paranoid, but this is rcu_read_unlock() we are talking about here. ;-) > > > > You seem to be suggesting making the first test be "<=", then > > sorting things out later, but given that both the equals-zero and the > > greater-than-zero cases are quite common, I couldn't figure out how to > > avoid the extra test and branch in the common case. Hence the #ifdef. > > No, I think you could simply read the predecremented value into a local > variable, test it once with == 0, then have the WARN_ON_ONCE, and hope > that the compiler figures out it can just test the register once and > then do multiple jumps on the same flags. > > You could try it and see what code it generates. I agree that a smart compiler could share condition-code state, but there still will be the extra branch. (Keep in mind that this is a .h file, so #ifdef is permitted -- though I might nevertheless make a one-line function/macro.) Thanx, Paul ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH tip/core/rcu 2/3] rcu: add debug check for too many rcu_read_unlock() 2010-01-05 16:21 ` Paul E. McKenney @ 2010-01-05 17:08 ` Josh Triplett 0 siblings, 0 replies; 12+ messages in thread From: Josh Triplett @ 2010-01-05 17:08 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, Jan 05, 2010 at 08:21:37AM -0800, Paul E. McKenney wrote: > On Mon, Jan 04, 2010 at 06:28:15PM -0800, Josh Triplett wrote: > > On Mon, Jan 04, 2010 at 06:19:19PM -0800, Paul E. McKenney wrote: > > > On Mon, Jan 04, 2010 at 06:03:08PM -0800, Josh Triplett wrote: > > > > On Mon, Jan 04, 2010 at 04:04:01PM -0800, Paul E. McKenney wrote: > > > > > From: Paul E. McKenney <paulmck@linux.vnet.ibm.com> > > > > > > > > > > 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); > > > > > > > > Given that you *already* need to access t->rcu_read_lock_nesting here, > > > > why not just do the test all the time? Ideally you could access > > > > t->rcu_read_lock_nesting once, decrement it, and test for both 0 and > > > > negative. > > > > > > Because I was paranoid about the extra branch. Perhaps needlessly > > > paranoid, but this is rcu_read_unlock() we are talking about here. ;-) > > > > > > You seem to be suggesting making the first test be "<=", then > > > sorting things out later, but given that both the equals-zero and the > > > greater-than-zero cases are quite common, I couldn't figure out how to > > > avoid the extra test and branch in the common case. Hence the #ifdef. > > > > No, I think you could simply read the predecremented value into a local > > variable, test it once with == 0, then have the WARN_ON_ONCE, and hope > > that the compiler figures out it can just test the register once and > > then do multiple jumps on the same flags. > > > > You could try it and see what code it generates. > > I agree that a smart compiler could share condition-code state, but > there still will be the extra branch. (Keep in mind that this is a > .h file, so #ifdef is permitted -- though I might nevertheless make > a one-line function/macro.) Hmmm. Seems like one untaken branch, with unlikely() even, should prove sufficiently cheap. But, as you said, this is rcu_read_unlock() we are talking about here. ;) Fair enough; might as well hide it under CONFIG_PROVE_LOCKING, or perhaps some RCU-specific debugging-related CONFIG symbol. - Josh Triplett ^ permalink raw reply [flat|nested] 12+ messages in thread
* [tip:core/rcu] rcu: Add debug check for too many rcu_read_unlock() 2010-01-05 0:04 ` [PATCH tip/core/rcu 2/3] rcu: add debug check for too many rcu_read_unlock() Paul E. McKenney 2010-01-05 2:03 ` Josh Triplett @ 2010-01-13 10:27 ` tip-bot for Paul E. McKenney 1 sibling, 0 replies; 12+ messages in thread From: tip-bot for Paul E. McKenney @ 2010-01-13 10:27 UTC (permalink / raw) To: linux-tip-commits; +Cc: linux-kernel, paulmck, hpa, mingo, tglx, mingo Commit-ID: cba8244a0f1c277b6b1e48ed6504fa434119e24d Gitweb: http://git.kernel.org/tip/cba8244a0f1c277b6b1e48ed6504fa434119e24d Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com> AuthorDate: Mon, 4 Jan 2010 16:04:01 -0800 Committer: Ingo Molnar <mingo@elte.hu> CommitDate: Wed, 13 Jan 2010 09:06:06 +0100 rcu: Add debug check for too many rcu_read_unlock() 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> Cc: laijs@cn.fujitsu.com Cc: dipankar@in.ibm.com Cc: mathieu.desnoyers@polymtl.ca Cc: josh@joshtriplett.org Cc: dvhltc@us.ibm.com Cc: niv@us.ibm.com Cc: peterz@infradead.org Cc: rostedt@goodmis.org Cc: Valdis.Kletnieks@vt.edu Cc: dhowells@redhat.com LKML-Reference: <12626498423064-git-send-email-> Signed-off-by: Ingo Molnar <mingo@elte.hu> --- 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); ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH tip/core/rcu 3/3] rcu: give different levels of the rcu_node hierarchy distinct lockdep names 2010-01-05 0:03 [PATCH tip/core/rcu 0/3] rcu: improve diagnostics and documentation of source files Paul E. McKenney 2010-01-05 0:04 ` [PATCH tip/core/rcu 1/3] rcu: make MAINTAINERS file match new RCU reality Paul E. McKenney 2010-01-05 0:04 ` [PATCH tip/core/rcu 2/3] rcu: add debug check for too many rcu_read_unlock() Paul E. McKenney @ 2010-01-05 0:04 ` Paul E. McKenney 2010-01-13 10:28 ` [tip:core/rcu] rcu: Give " tip-bot for Paul E. McKenney 2 siblings, 1 reply; 12+ messages in thread From: Paul E. McKenney @ 2010-01-05 0:04 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> 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. This version of the patch includes improved diagnostics suggested by Josh Triplett and Peter Zijlstra. 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..3b13d64 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; + BUILD_BUG_ON(MAX_RCU_LVLS > ARRAY_SIZE(buf)); /* Fix buf[] init! */ + /* 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] 12+ messages in thread
* [tip:core/rcu] rcu: Give different levels of the rcu_node hierarchy distinct lockdep names 2010-01-05 0:04 ` [PATCH tip/core/rcu 3/3] rcu: give different levels of the rcu_node hierarchy distinct lockdep names Paul E. McKenney @ 2010-01-13 10:28 ` tip-bot for Paul E. McKenney 0 siblings, 0 replies; 12+ messages in thread From: tip-bot for Paul E. McKenney @ 2010-01-13 10:28 UTC (permalink / raw) To: linux-tip-commits; +Cc: linux-kernel, paulmck, hpa, mingo, tglx, mingo Commit-ID: b6407e863934965cdc66cbc244d811ceeb6f4d77 Gitweb: http://git.kernel.org/tip/b6407e863934965cdc66cbc244d811ceeb6f4d77 Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com> AuthorDate: Mon, 4 Jan 2010 16:04:02 -0800 Committer: Ingo Molnar <mingo@elte.hu> CommitDate: Wed, 13 Jan 2010 09:06:07 +0100 rcu: Give different levels of the rcu_node hierarchy distinct lockdep names 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. This version of the patch includes improved diagnostics suggested by Josh Triplett and Peter Zijlstra. Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> Cc: laijs@cn.fujitsu.com Cc: dipankar@in.ibm.com Cc: mathieu.desnoyers@polymtl.ca Cc: josh@joshtriplett.org Cc: dvhltc@us.ibm.com Cc: niv@us.ibm.com Cc: peterz@infradead.org Cc: rostedt@goodmis.org Cc: Valdis.Kletnieks@vt.edu Cc: dhowells@redhat.com LKML-Reference: <12626498421830-git-send-email-> Signed-off-by: Ingo Molnar <mingo@elte.hu> --- kernel/rcutree.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/kernel/rcutree.c b/kernel/rcutree.c index 0a4c328..3b13d64 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; + BUILD_BUG_ON(MAX_RCU_LVLS > ARRAY_SIZE(buf)); /* Fix buf[] init! */ + /* 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; ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2010-01-13 10:28 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-01-05 0:03 [PATCH tip/core/rcu 0/3] rcu: improve diagnostics and documentation of source files Paul E. McKenney 2010-01-05 0:04 ` [PATCH tip/core/rcu 1/3] rcu: make MAINTAINERS file match new RCU reality Paul E. McKenney 2010-01-13 10:27 ` [tip:core/rcu] rcu: Make " tip-bot for Paul E. McKenney 2010-01-05 0:04 ` [PATCH tip/core/rcu 2/3] rcu: add debug check for too many rcu_read_unlock() Paul E. McKenney 2010-01-05 2:03 ` Josh Triplett 2010-01-05 2:19 ` Paul E. McKenney 2010-01-05 2:28 ` Josh Triplett 2010-01-05 16:21 ` Paul E. McKenney 2010-01-05 17:08 ` Josh Triplett 2010-01-13 10:27 ` [tip:core/rcu] rcu: Add " tip-bot for Paul E. McKenney 2010-01-05 0:04 ` [PATCH tip/core/rcu 3/3] rcu: give different levels of the rcu_node hierarchy distinct lockdep names Paul E. McKenney 2010-01-13 10:28 ` [tip:core/rcu] rcu: Give " tip-bot for 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