On 2/6/25 16:27, Steven Rostedt wrote: > On Thu, 6 Feb 2025 14:28:17 +0900 > Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx> wrote: > >>> --- a/kernel/module/main.c >>> +++ b/kernel/module/main.c >>> @@ -3809,6 +3809,20 @@ bool is_module_text_address(unsigned long addr) >>> return ret; >>> } >>> >> >> It is better to add a kerneldoc for this API. > > Agreed, but I was planning on this changing. Waiting to hear from the > module maintainers. > >> >> /** >> * module_for_each_mod() - iterate all modules >> * @func: Callback function >> * @data: User data >> * >> * Call the @func with each module in the system. If @func returns !0, this >> * stops itrating. Note that @func must not sleep since it is called under >> * the preemption disabled. >> */ >> >> BTW, do we really need to disable preempt or is it enough to call >> rcu_read_lock()? > > Bah, as I expected this function to be changed, I didn't spend too much > time on looking at its implementation. I just cut and pasted how the other > loops worked. But yes, it should not be disabling preemption. In fact, I > think the module code itself should not be disabling preemption! > > I'll have to go and look into that. The series "module: Use RCU instead of RCU-sched" from Sebastian Andrzej Siewior cleans this up [1]. It is currently queued on modules-next (for 6.15-rc1). The new function module_for_each_mod() should then use "guard(rcu)();". [1] https://lore.kernel.org/linux-modules/20250108090457.512198-1-bigeasy@xxxxxxxxxxxxx/ -- Thanks, Petr >>> +void module_for_each_mod(int(*func)(struct module *mod, void *data), void *data) >>> +{ >>> + struct module *mod; >>> + >>> + preempt_disable(); >>> + list_for_each_entry_rcu(mod, &modules, list) { >>> + if (mod->state == MODULE_STATE_UNFORMED) >>> + continue; >>> + if (func(mod, data)) >>> + break; >>> + } >>> + preempt_enable(); >>> +} >>> + >>> /** >>> * __module_text_address() - get the module whose code contains an address. >>> * @addr: the address. >>> -- >
