* Peter Zijlstra <pet...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
