Messages in this thread |  | | Date | Mon, 13 Apr 2015 18:32:57 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH v5 01/10] module: Sanitize RCU usage and locking |
| |
* Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Apr 13, 2015 at 05:32:29PM +0200, Ingo Molnar wrote: > > > +static void module_assert_mutex_or_preempt(void) > > > +{ > > > +#ifdef CONFIG_LOCKDEP > > > + int rcu_held = rcu_read_lock_sched_held(); > > > + int mutex_held = 1; > > > + > > > + if (debug_locks) > > > + mutex_held = lockdep_is_held(&module_mutex); > > > + > > > + WARN_ON(!rcu_held && !mutex_held); > > > > So because rcu_read_lock_sched_held() also depends on debug_locks > > being on to be fully correct, shouldn't the warning also be within the > > debug_locks condition? > > Ah, see how mutex_held will be true for !debug_locks and therefore we'll > not trigger the warn. > > Maybe not the best way to code that though. > > Something like so perhaps: > > static void module_assert_mutex_or_preempt(void) > { > #ifdef CONFIG_LOCKDEP > if (!debug_locks) > return; > > WARN_ON(!rcu_held_lock_sched_held() && > !lockdep_is_held(&module_mutex)); > #endif
Yeah. I'd also make it:
if (unlikely(!debug_locks)) return;
or such.
Thanks,
Ingo
|  |