On 2/6/25 16:27, Steven Rostedt wrote:

On Thu, 6 Feb 2025 14:28:17 +0900
Masami Hiramatsu (Google) [off-list ref] wrote:

quoted

quoted

--- 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.

quoted

/** 
 * 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@linutronix.de/ (local)

-- 
Thanks,
Petr

quoted

quoted

+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.
--