On Fri 2022-02-25 10:27 +0000, Aaron Tomlin wrote: > On Fri 2022-02-25 11:15 +0100, Petr Mladek wrote: > > rcu_dereference_sched() makes sparse happy. But lockdep complains > > because the _rcu pointer is not accessed under: > > > > rcu_read_lock_sched(); > > rcu_read_unlock_sched(); > > Hi Petr, > > > > > This is not the case here. Note that module_mutex does not > > disable preemtion. > > > > Now, the code is safe. The RCU access makes sure that "mod" > > can't be freed in the meantime: > > > > + add_kallsyms() is called by the module loaded when the module > > is being loaded. It could not get removed in parallel > > by definition. > > > > + module_kallsyms_on_each_symbol() takes module_mutex. > > It means that the module could not get removed. > > Indeed, which is why I did not use rcu_read_lock_sched() and > rcu_read_unlock_sched() with rcu_dereference_sched(). That being said, I > should have mentioned this in the commit message. > > > IMHO, we have two possibilities here: > > > > + Make sparse and lockdep happy by using rcu_dereference_sched() > > and calling the code under rcu_read_lock_sched(). > > > > + Cast (struct mod_kallsyms *)mod->kallsyms when accessing > > the value. > > I prefer the first option. > > > I do not have strong preference. I am fine with both. > > > > Anyway, such a fix should be done in a separate patch! > > Agreed. Luis, If I understand correctly, it might be cleaner to resolve the above in two separate patches for a v9 i.e. a) address the sparse and lockdep feedback and b) refactor the code, before the latest version [1] is merged into module-next. I assume the previous iteration will be reverted first? Please let me know your thoughts [1]: https://lore.kernel.org/all/20220222141303.1392190-1-atomlin@xxxxxxxxxx/ Kind regards, -- Aaron Tomlin
