diff mbox

[RFC,tip/core/rcu,37/41] lockdep: Add CPU-idle/offline warning to lockdep-RCU splat

Message ID 1328125319-5205-37-git-send-email-paulmck@linux.vnet.ibm.com
State Superseded
Headers show

Commit Message

Paul E. McKenney Feb. 1, 2012, 7:41 p.m. UTC
From: "Paul E. McKenney" <paul.mckenney@linaro.org>

It is illegal to use RCU from a CPU that has reported idleness or
offlinedness to RCU.  However, it can be quite difficult to determine
from a stack trace whether or not a given CPU is idle or offline.
Therefore, this commit adds idle/offline diagnostics to the lockdep-RCU
error message.

Signed-off-by: Paul E. McKenney <paul.mckenney@linaro.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/lockdep.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

Comments

Josh Triplett Feb. 2, 2012, 6:07 a.m. UTC | #1
On Wed, Feb 01, 2012 at 11:41:55AM -0800, Paul E. McKenney wrote:
> --- a/kernel/lockdep.c
> +++ b/kernel/lockdep.c
> @@ -4176,7 +4176,13 @@ void lockdep_rcu_suspicious(const char *file, const int line, const char *s)
>  	printk("-------------------------------\n");
>  	printk("%s:%d %s!\n", file, line, s);
>  	printk("\nother info that might help us debug this:\n\n");
> -	printk("\nrcu_scheduler_active = %d, debug_locks = %d\n", rcu_scheduler_active, debug_locks);
> +	printk("\n%srcu_scheduler_active = %d, debug_locks = %d\n",
> +	       !rcu_lockdep_current_cpu_online()
> +			? "RCU used illegally from offline CPU!\n"
> +			: rcu_is_cpu_idle()
> +				? "RCU used illegally from idle CPU!\n"
> +				: "",

Not the usual way I've seen chained ?: indented in kernel code:

 cond1 ? value1 :
 cond2 ? value2 :
 value3

That avoids repeated indentation over to the right, much like "else if".

- Josh Triplett
Paul E. McKenney Feb. 2, 2012, 6:30 p.m. UTC | #2
On Wed, Feb 01, 2012 at 10:07:52PM -0800, Josh Triplett wrote:
> On Wed, Feb 01, 2012 at 11:41:55AM -0800, Paul E. McKenney wrote:
> > --- a/kernel/lockdep.c
> > +++ b/kernel/lockdep.c
> > @@ -4176,7 +4176,13 @@ void lockdep_rcu_suspicious(const char *file, const int line, const char *s)
> >  	printk("-------------------------------\n");
> >  	printk("%s:%d %s!\n", file, line, s);
> >  	printk("\nother info that might help us debug this:\n\n");
> > -	printk("\nrcu_scheduler_active = %d, debug_locks = %d\n", rcu_scheduler_active, debug_locks);
> > +	printk("\n%srcu_scheduler_active = %d, debug_locks = %d\n",
> > +	       !rcu_lockdep_current_cpu_online()
> > +			? "RCU used illegally from offline CPU!\n"
> > +			: rcu_is_cpu_idle()
> > +				? "RCU used illegally from idle CPU!\n"
> > +				: "",
> 
> Not the usual way I've seen chained ?: indented in kernel code:
> 
>  cond1 ? value1 :
>  cond2 ? value2 :
>  value3
> 
> That avoids repeated indentation over to the right, much like "else if".

I tried the following, but didn't like it:

	       !rcu_lockdep_current_cpu_online() ? "RCU used illegally from offline CPU!\n" :
	       rcu_is_cpu_idle() ? "RCU used illegally from idle CPU!\n" :
	       "",

							Thanx, Paul
Josh Triplett Feb. 3, 2012, 6:12 a.m. UTC | #3
On Thu, Feb 02, 2012 at 10:30:07AM -0800, Paul E. McKenney wrote:
> On Wed, Feb 01, 2012 at 10:07:52PM -0800, Josh Triplett wrote:
> > On Wed, Feb 01, 2012 at 11:41:55AM -0800, Paul E. McKenney wrote:
> > > --- a/kernel/lockdep.c
> > > +++ b/kernel/lockdep.c
> > > @@ -4176,7 +4176,13 @@ void lockdep_rcu_suspicious(const char *file, const int line, const char *s)
> > >  	printk("-------------------------------\n");
> > >  	printk("%s:%d %s!\n", file, line, s);
> > >  	printk("\nother info that might help us debug this:\n\n");
> > > -	printk("\nrcu_scheduler_active = %d, debug_locks = %d\n", rcu_scheduler_active, debug_locks);
> > > +	printk("\n%srcu_scheduler_active = %d, debug_locks = %d\n",
> > > +	       !rcu_lockdep_current_cpu_online()
> > > +			? "RCU used illegally from offline CPU!\n"
> > > +			: rcu_is_cpu_idle()
> > > +				? "RCU used illegally from idle CPU!\n"
> > > +				: "",
> > 
> > Not the usual way I've seen chained ?: indented in kernel code:
> > 
> >  cond1 ? value1 :
> >  cond2 ? value2 :
> >  value3
> > 
> > That avoids repeated indentation over to the right, much like "else if".
> 
> I tried the following, but didn't like it:
> 
> 	       !rcu_lockdep_current_cpu_online() ? "RCU used illegally from offline CPU!\n" :
> 	       rcu_is_cpu_idle() ? "RCU used illegally from idle CPU!\n" :
> 	       "",

Seems like an improvement to me, but it also doesn't matter enough to
bikeshed further about. :)

- Josh Triplett
diff mbox

Patch

diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index 8889f7d..ea9ee45 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -4176,7 +4176,13 @@  void lockdep_rcu_suspicious(const char *file, const int line, const char *s)
 	printk("-------------------------------\n");
 	printk("%s:%d %s!\n", file, line, s);
 	printk("\nother info that might help us debug this:\n\n");
-	printk("\nrcu_scheduler_active = %d, debug_locks = %d\n", rcu_scheduler_active, debug_locks);
+	printk("\n%srcu_scheduler_active = %d, debug_locks = %d\n",
+	       !rcu_lockdep_current_cpu_online()
+			? "RCU used illegally from offline CPU!\n"
+			: rcu_is_cpu_idle()
+				? "RCU used illegally from idle CPU!\n"
+				: "",
+	       rcu_scheduler_active, debug_locks);
 
 	/*
 	 * If a CPU is in the RCU-free window in idle (ie: in the section