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. Thanks! -- Steve > > Thank you, > > > +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. > > --