Le 25/02/2022 à 13:21, Aaron Tomlin a écrit : > 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/ > I would do it the other way: first move the code into a separate file, and then handle the sparse __rcu feedback as a followup patch to the series. Regarding module-next, AFAICS at the moment we still have only the 10 first patches of v6 in the tree. I guess the way forward will be to rebase module-next and drop those patches and commit v9 instead. Christophe
