On 2026/10/19 23:25, Masami Hiramatsu (Google) wrote: > Hi, > > On Wed, 16 Sep 2026 16:27:15 +0800, Yaxiong Tian <[email protected]> > wrote: > >> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c >> --- a/kernel/trace/ftrace.c >> +++ b/kernel/trace/ftrace.c >> @@ -8010,11 +8011,137 @@ void ftrace_release_mod(struct module *mod) >> + >> + /* mod->kallsyms is not replaced until do_init_module() returns. */ >> + rcu_read_lock(); >> + kallsyms = rcu_dereference(mod->kallsyms); > Consider using rcu_access_pointer(mod->kallsyms) or > rcu_dereference_raw(mod->kallsyms) rather than a transient > rcu_read_lock()/rcu_read_unlock() pair. Agreed, I will use rcu_dereference_raw() and drop the lock pair.
>> + rcu_read_unlock(); >> + >> + if (!kallsyms || kallsyms->num_symtab < 2) >> [...] >> + * ftrace never came up, where the loop below would not run anyway; that >> + * also makes reading ftrace_disabled without the lock safe. >> + */ >> + syms = ftrace_disabled ? NULL : ftrace_mod_syms_alloc(mod); > Because ftrace_disabled is one-way flag, if you find ftrace_disabled here, > you can return from this function soon. > > Thanks, Right. I will check it once before the allocation and jump to a common out_cached_mods label, so ftrace_lock is not taken at all in that case and process_cached_mods() still runs exactly once. I keep the second check under the lock because ftrace_kill() can set the flag after the unlocked read. if (ftrace_disabled) goto out_cached_mods; syms = ftrace_mod_syms_alloc(mod); mutex_lock(&ftrace_lock); if (ftrace_disabled) goto out_unlock; ... out_unlock: mutex_unlock(&ftrace_lock); kvfree(syms); out_cached_mods: process_cached_mods(mod->name); >> + >> mutex_lock(&ftrace_lock); >>
