Re: [PATCH] module: Use complete RCU protection instead a mix of RCU and RCU-sched.

[Peter Zijlstra <peterz@infradead.org>]

On Thu, Dec 05, 2024 at 10:51:02PM +0100, Sebastian Andrzej Siewior wrote:
> The RCU usage in module was introduced in commit d72b37513cdfb ("Remove
> stop_machine during module load v2") and it claimed not to be RCU but
> similar. Then there was another improvement in commit e91defa26c527
> ("module: don't use stop_machine on module load"). It become a mix of
> RCU and RCU-sched and was eventually fixed 0be964be0d450 ("module:
> Sanitize RCU usage and locking"). Later RCU & RCU-sched was merged in
> commit cb2f55369d3a9 ("modules: Replace synchronize_sched() and
> call_rcu_sched()") so that was aligned.
> 
> Looking at it today, there is still leftovers. The preempt_disable() was
> used instead rcu_read_lock_sched(). The RCU & RCU-sched merge was not
> complete as there is still rcu_dereference_sched() for module::kallsyms.
> 
> The RCU-list modules and unloaded_tainted_modules are always accessed
> under RCU protection or the module_mutex. The modules list iteration can
> always happen safely because the module will not disappear.
> Once the module is removed (free_module()) then after removing the
> module from the list, there is a synchronize_rcu() which waits until
> every RCU reader left the section. That means iterating over the list
> within a RCU-read section is enough, there is no need to disable
> preemption. module::kallsyms is first assigned in add_kallsyms() before
> the module is added to the list. At this point, it points to init data.
> This pointer is later updated and before the init code is removed there
> is also synchronize_rcu() in do_free_init(). That means A RCU read lock
> is enough for protection and rcu_dereference() can be safely used.
> 
> Replace preempt-disable sections with RCU-read sections. Replace
> rcu_dereference_sched() with rcu_dereference(). Remove
> module_assert_mutex_or_preempt(), its goal is covered by the following
> RCU usage.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  kernel/module/internal.h | 11 -------
>  kernel/module/kallsyms.c | 45 +++++++++-------------------
>  kernel/module/main.c     | 64 ++++++++++++----------------------------
>  kernel/module/tracking.c |  2 --
>  kernel/module/version.c  | 11 ++++---
>  5 files changed, 38 insertions(+), 95 deletions(-)

There's more in kernel/jump_label.c and possibly other sites, grep for
__module_address.