lkml.org 
[lkml]   [2015]   [Apr]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 01/10] module: Sanitize RCU usage and locking

* Peter Zijlstra <peterz@infradead.org> wrote:

> Currently the RCU usage in module is an inconsistent mess of RCU and
> RCU-sched, this is broken for CONFIG_PREEMPT where synchronize_rcu()
> does not imply synchronize_sched().
>
> Most usage sites use preempt_{dis,en}able() which is RCU-sched, but
> (most of) the modification sites use synchronize_rcu(). With the
> exception of the module bug list, which actually uses RCU.
>
> Convert everything over to RCU-sched.
>
> Furthermore add lockdep asserts to all sites, because its not at all
> clear to me the required locking is observed, esp. on exported
> functions.

nit:

s/its/it's

> +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?

> @@ -3106,11 +3128,11 @@ static noinline int do_init_module(struc
> mod->init_text_size = 0;
> /*
> * We want to free module_init, but be aware that kallsyms may be
> + * walking this with preempt disabled. In all the failure paths, we
> + * call synchronize_sched, but we don't want to slow down the success
> + * path, so use actual RCU here.

nit:

s/synchronize_sched
/synchronize_sched()

Thanks,

Ingo


\
 
 \ /
  Last update: 2015-04-13 18:01    [W:0.269 / U:0.420 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site