lkml.org 
[lkml]   [2015]   [Jun]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC/INCOMPLETE 01/13] context_tracking: Add context_tracking_assert_state

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

> > Yeah, and inverting the condition. Assuming the condition was assert()-style
> > inverted to begin with! :-)
>
> It appears to have been. ;-)
>
> Please see below for an untested patch. I made this be one big patch, but could
> have one patch add RCU_LOCKDEP_WARN(), a series convert uses from
> rcu_lockdep_assert() to RCU_LOCKDEP_WARN(), and a final patch remove
> rcu_lockdep_assert(). Let me know!

One big patch is perfect I think - it's a rename and a relatively mechanic
inversion of conditions, no point in splitting it up any more IMHO. (But it's your
call really.)

So I had a quick look at this patch, and IMHO the RCU_LOCKDEP_WARN() lines read a
lot more 'naturally', because the new, inverted conditions now highlight buggy
scenarios - which has the same logic parity as the kernel's historic
WARN_ON()/BUG_ON() patterns:

Reviewed-by: Ingo Molnar <mingo@kernel.org>

This one looked a bit weird:

> index a0a0dd03c73a..47268fb1d27b 100644
> --- a/kernel/rcu/update.c
> +++ b/kernel/rcu/update.c
> @@ -589,8 +589,8 @@ EXPORT_SYMBOL_GPL(call_rcu_tasks);
> void synchronize_rcu_tasks(void)
> {
> /* Complain if the scheduler has not started. */
> - rcu_lockdep_assert(!rcu_scheduler_active,
> - "synchronize_rcu_tasks called too soon");
> + RCU_LOCKDEP_WARN(rcu_scheduler_active,
> + "synchronize_rcu_tasks called too soon");
>

So I'd assume that a flag called 'rcu_scheduler_active' would be 1 if the RCU
scheduler is active.

So why do we warn on it being active? Shouldn't the condition be:

RCU_LOCKDEP_WARN(!rcu_scheduler_active,
"synchronize_rcu_tasks called too soon");

I.e. we warn when the RCU scheduler is not yet active and we called
synchronize_rcu_tasks() too soon?

So either the original assert() was wrong, or I'm missing something obvious?

Thanks,

Ingo


\
 
 \ /
  Last update: 2015-06-30 13:21    [W:1.254 / U:0.592 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site