mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Daniel Gomez <da.gomez@samsung.com>,
	Luis Chamberlain <mcgrof@kernel.org>,
	"Paul E . McKenney" <paulmck@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Petr Pavlu <petr.pavlu@suse.com>,
	Sami Tolvanen <samitolvanen@google.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Subject: [PATCH v2 02/28] module: Begin to move from RCU-sched to RCU.
Date: Fri, 20 Dec 2024 18:41:16 +0100	[thread overview]
Message-ID: <20241220174731.514432-3-bigeasy@linutronix.de> (raw)
In-Reply-To: <20241220174731.514432-1-bigeasy@linutronix.de>

The RCU usage in module was introduced in commit d72b37513cdfb ("Remove
stop_machine during module load v2") and it claimed not to be RCU but
similar. Then there was another improvement in commit e91defa26c527
("module: don't use stop_machine on module load"). It become a mix of
RCU and RCU-sched and was eventually fixed 0be964be0d450 ("module:
Sanitize RCU usage and locking"). Later RCU & RCU-sched was merged in
commit cb2f55369d3a9 ("modules: Replace synchronize_sched() and
call_rcu_sched()") so that was aligned.

Looking at it today, there is still leftovers. The preempt_disable() was
used instead rcu_read_lock_sched(). The RCU & RCU-sched merge was not
complete as there is still rcu_dereference_sched() for module::kallsyms.

The RCU-list modules and unloaded_tainted_modules are always accessed
under RCU protection or the module_mutex. The modules list iteration can
always happen safely because the module will not disappear.
Once the module is removed (free_module()) then after removing the
module from the list, there is a synchronize_rcu() which waits until
every RCU reader left the section. That means iterating over the list
within a RCU-read section is enough, there is no need to disable
preemption. module::kallsyms is first assigned in add_kallsyms() before
the module is added to the list. At this point, it points to init data.
This pointer is later updated and before the init code is removed there
is also synchronize_rcu() in do_free_init(). That means A RCU read lock
is enough for protection and rcu_dereference() can be safely used.

Convert module code and its users step by step. Update comments and
convert print_modules() to use RCU.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/module/main.c        | 9 ++++-----
 kernel/module/tree_lookup.c | 8 ++++----
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 5399c182b3cbe..5cce4a92d7ba3 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -67,7 +67,7 @@
 
 /*
  * Mutex protects:
- * 1) List of modules (also safely readable with preempt_disable),
+ * 1) List of modules (also safely readable within RCU read section),
  * 2) module_use links,
  * 3) mod_tree.addr_min/mod_tree.addr_max.
  * (delete and add uses RCU list operations).
@@ -1348,7 +1348,7 @@ static void free_module(struct module *mod)
 	mod_tree_remove(mod);
 	/* Remove this module from bug list, this uses list_del_rcu */
 	module_bug_cleanup(mod);
-	/* Wait for RCU-sched synchronizing before releasing mod->list and buglist. */
+	/* Wait for RCU synchronizing before releasing mod->list and buglist. */
 	synchronize_rcu();
 	if (try_add_tainted_module(mod))
 		pr_err("%s: adding tainted module to the unloaded tainted modules list failed.\n",
@@ -2965,7 +2965,7 @@ static noinline int do_init_module(struct module *mod)
 #endif
 	/*
 	 * We want to free module_init, but be aware that kallsyms may be
-	 * walking this with preempt disabled.  In all the failure paths, we
+	 * walking this within an RCU read section. In all the failure paths, we
 	 * call synchronize_rcu(), but we don't want to slow down the success
 	 * path. execmem_free() cannot be called in an interrupt, so do the
 	 * work and call synchronize_rcu() in a work queue.
@@ -3754,7 +3754,7 @@ void print_modules(void)
 
 	printk(KERN_DEFAULT "Modules linked in:");
 	/* Most callers should already have preempt disabled, but make sure */
-	preempt_disable();
+	guard(rcu)();
 	list_for_each_entry_rcu(mod, &modules, list) {
 		if (mod->state == MODULE_STATE_UNFORMED)
 			continue;
@@ -3762,7 +3762,6 @@ void print_modules(void)
 	}
 
 	print_unloaded_tainted_modules();
-	preempt_enable();
 	if (last_unloaded_module.name[0])
 		pr_cont(" [last unloaded: %s%s]", last_unloaded_module.name,
 			last_unloaded_module.taints);
diff --git a/kernel/module/tree_lookup.c b/kernel/module/tree_lookup.c
index 277197977d438..d3204c5c74eb7 100644
--- a/kernel/module/tree_lookup.c
+++ b/kernel/module/tree_lookup.c
@@ -12,11 +12,11 @@
 
 /*
  * Use a latched RB-tree for __module_address(); this allows us to use
- * RCU-sched lookups of the address from any context.
+ * RCU lookups of the address from any context.
  *
- * This is conditional on PERF_EVENTS || TRACING because those can really hit
- * __module_address() hard by doing a lot of stack unwinding; potentially from
- * NMI context.
+ * This is conditional on PERF_EVENTS || TRACING || CFI_CLANG because those can
+ * really hit __module_address() hard by doing a lot of stack unwinding;
+ * potentially from NMI context.
  */
 
 static __always_inline unsigned long __mod_tree_val(struct latch_tree_node *n)
-- 
2.45.2


  parent reply	other threads:[~2024-12-20 17:47 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-20 17:41 [PATCH v2 00/28] module: Use RCU instead of RCU-sched Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 01/28] module: Extend the preempt disabled section in dereference_symbol_descriptor() Sebastian Andrzej Siewior
2025-01-07 10:58   ` Sergey Senozhatsky
2024-12-20 17:41 ` Sebastian Andrzej Siewior [this message]
2024-12-20 17:41 ` [PATCH v2 03/28] module: Use proper RCU assignment in add_kallsyms() Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 04/28] module: Use RCU in find_kallsyms_symbol() Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 05/28] module: Use RCU in module_get_kallsym() Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 06/28] module: Use RCU in find_module_all() Sebastian Andrzej Siewior
2025-01-02 14:16   ` Petr Mladek
2024-12-20 17:41 ` [PATCH v2 07/28] module: Use RCU in __find_kallsyms_symbol_value() Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 08/28] module: Use RCU in module_kallsyms_on_each_symbol() Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 09/28] module: Remove module_assert_mutex_or_preempt() from try_add_tainted_module() Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 10/28] module: Use RCU in find_symbol() Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 11/28] module: Use RCU in __is_module_percpu_address() Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 12/28] module: Allow __module_address() to be called from RCU section Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 13/28] module: Use RCU in search_module_extables() Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 14/28] module: Use RCU in all users of __module_address() Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 15/28] module: Use RCU in all users of __module_text_address() Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 16/28] ARM: " Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 17/28] arm64: " Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 18/28] LoongArch/orc: Use RCU in all users of __module_address() Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 19/28] LoongArch: ftrace: Use RCU in all users of __module_text_address() Sebastian Andrzej Siewior
2024-12-27 17:19   ` Steven Rostedt
2025-01-07 17:12     ` Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 20/28] powerpc/ftrace: " Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 21/28] x86: Use RCU in all users of __module_address() Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 22/28] jump_label: " Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 23/28] jump_label: Use RCU in all users of __module_text_address() Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 24/28] bpf: " Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 25/28] kprobes: " Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 26/28] static_call: " Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 27/28] bug: Use RCU instead RCU-sched to protect module_bug_list Sebastian Andrzej Siewior
2024-12-20 17:41 ` [PATCH v2 28/28] cfi: Use RCU while invoking __module_address() Sebastian Andrzej Siewior
2024-12-30 21:13   ` Petr Pavlu
2025-01-02 23:59     ` Sami Tolvanen
2024-12-31  3:33   ` Elliot Berman
2025-01-03  0:24     ` Sami Tolvanen
2025-01-06 18:00       ` Elliot Berman
2025-01-06 21:24         ` Sami Tolvanen
2025-01-07 15:44           ` Sebastian Andrzej Siewior
2025-01-02 12:47 ` [PATCH v2 00/28] module: Use RCU instead of RCU-sched Christophe Leroy
2025-01-07 15:45   ` Sebastian Andrzej Siewior
2025-01-07  8:48 ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241220174731.514432-3-bigeasy@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=da.gomez@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=petr.pavlu@suse.com \
    --cc=samitolvanen@google.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®