[PATCH v3 00/28] module: Use RCU instead of RCU-sched.

[PATCH v3 00/28] module: Use RCU instead of RCU-sched.

[Sebastian Andrzej Siewior]

Hi,

This is an updated version of the initial post after PeterZ made me
aware that there are users outside of the module directory.
The goal is replace the mix auf rcu_read_lock(), rcu_read_lock_sched()
and preempt_disable() with just rcu_read_lock().

I've split it into smaller chunks which can be applied/ reviewed
independently.

v2…v3: https://lore.kernel.org/all/20241220174731.514432-1-bigeasy@linutronix.de/
  - Converted cfi to use RCU.
  - Use scoped_guard() in LoongArch's ftrace code after Steven suggested
    it.

v1…v2: https://lore.kernel.org/all/20241205215102.hRywUW2A@linutronix.de/
  - Split into smaller patches.
  - Converted all users.

Sebastian Andrzej Siewior (28):
  module: Extend the preempt disabled section in
    dereference_symbol_descriptor().
  module: Begin to move from RCU-sched to RCU.
  module: Use proper RCU assignment in add_kallsyms().
  module: Use RCU in find_kallsyms_symbol().
  module: Use RCU in module_get_kallsym().
  module: Use RCU in find_module_all().
  module: Use RCU in __find_kallsyms_symbol_value().
  module: Use RCU in module_kallsyms_on_each_symbol().
  module: Remove module_assert_mutex_or_preempt() from
    try_add_tainted_module().
  module: Use RCU in find_symbol().
  module: Use RCU in __is_module_percpu_address().
  module: Allow __module_address() to be called from RCU section.
  module: Use RCU in search_module_extables().
  module: Use RCU in all users of __module_address().
  module: Use RCU in all users of __module_text_address().
  ARM: module: Use RCU in all users of __module_text_address().
  arm64: module: Use RCU in all users of __module_text_address().
  LoongArch/orc: Use RCU in all users of __module_address().
  LoongArch: ftrace: Use RCU in all users of __module_text_address().
  powerpc/ftrace: Use RCU in all users of __module_text_address().
  cfi: Use RCU while invoking __module_address().
  x86: Use RCU in all users of __module_address().
  jump_label: Use RCU in all users of __module_address().
  jump_label: Use RCU in all users of __module_text_address().
  bpf: Use RCU in all users of __module_text_address().
  kprobes: Use RCU in all users of __module_text_address().
  static_call: Use RCU in all users of __module_text_address().
  bug: Use RCU instead RCU-sched to protect module_bug_list.

 arch/arm/kernel/module-plts.c            |   4 +-
 arch/arm64/kernel/ftrace.c               |   7 +-
 arch/loongarch/kernel/ftrace_dyn.c       |   9 +-
 arch/loongarch/kernel/unwind_orc.c       |   4 +-
 arch/powerpc/kernel/trace/ftrace.c       |   6 +-
 arch/powerpc/kernel/trace/ftrace_64_pg.c |   6 +-
 arch/x86/kernel/callthunks.c             |   3 +-
 arch/x86/kernel/unwind_orc.c             |   4 +-
 include/linux/kallsyms.h                 |   3 +-
 include/linux/module.h                   |   2 +-
 kernel/cfi.c                             |   5 +-
 kernel/jump_label.c                      |  31 ++++---
 kernel/kprobes.c                         |   4 +-
 kernel/livepatch/core.c                  |   4 +-
 kernel/module/internal.h                 |  11 ---
 kernel/module/kallsyms.c                 |  73 ++++++----------
 kernel/module/main.c                     | 103 ++++++++---------------
 kernel/module/tracking.c                 |   2 -
 kernel/module/tree_lookup.c              |   8 +-
 kernel/module/version.c                  |  14 +--
 kernel/static_call_inline.c              |  13 ++-
 kernel/trace/bpf_trace.c                 |  19 ++---
 kernel/trace/trace_kprobe.c              |   9 +-
 lib/bug.c                                |  22 ++---
 24 files changed, 136 insertions(+), 230 deletions(-)

Sebastian

[PATCH v3 01/28] module: Extend the preempt disabled section in dereference_symbol_descriptor().

[Sebastian Andrzej Siewior]

dereference_symbol_descriptor() needs to obtain the module pointer
belonging to pointer in order to resolve that pointer.
The returned mod pointer is obtained under RCU-sched/ preempt_disable()
guarantees and needs to be used within this section to ensure that the
module is not removed in the meantime.

Extend the preempt_disable() section to also cover
dereference_module_function_descriptor().

Fixes: 04b8eb7a4ccd9 ("symbol lookup: introduce dereference_symbol_descriptor()")
Cc: James E.J. Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Helge Deller <deller@gmx.de>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Naveen N Rao <naveen@kernel.org>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/kallsyms.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index c3f075e8f60cb..1c6a6c1704d8d 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -57,10 +57,10 @@ static inline void *dereference_symbol_descriptor(void *ptr)
 
 	preempt_disable();
 	mod = __module_address((unsigned long)ptr);
-	preempt_enable();
 
 	if (mod)
 		ptr = dereference_module_function_descriptor(mod, ptr);
+	preempt_enable();
 #endif
 	return ptr;
 }
-- 
2.47.1

Re: [PATCH v3 01/28] module: Extend the preempt disabled section in dereference_symbol_descriptor().

[Helge Deller]

On 1/8/25 10:04, Sebastian Andrzej Siewior wrote:
> dereference_symbol_descriptor() needs to obtain the module pointer
> belonging to pointer in order to resolve that pointer.
> The returned mod pointer is obtained under RCU-sched/ preempt_disable()
> guarantees and needs to be used within this section to ensure that the
> module is not removed in the meantime.
>
> Extend the preempt_disable() section to also cover
> dereference_module_function_descriptor().
>
> Fixes: 04b8eb7a4ccd9 ("symbol lookup: introduce dereference_symbol_descriptor()")
> Cc: James E.J. Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Helge Deller <deller@gmx.de>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Naveen N Rao <naveen@kernel.org>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
> Cc: linux-parisc@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Nice catch.

Acked-by: Helge Deller <deller@gmx.de>

This patch really should be backported.
Can you add a Cc: stable tag?

Helge


> ---
>   include/linux/kallsyms.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
> index c3f075e8f60cb..1c6a6c1704d8d 100644
> --- a/include/linux/kallsyms.h
> +++ b/include/linux/kallsyms.h
> @@ -57,10 +57,10 @@ static inline void *dereference_symbol_descriptor(void *ptr)
>
>   	preempt_disable();
>   	mod = __module_address((unsigned long)ptr);
> -	preempt_enable();
>
>   	if (mod)
>   		ptr = dereference_module_function_descriptor(mod, ptr);
> +	preempt_enable();
>   #endif
>   	return ptr;
>   }

Re: [PATCH v3 01/28] module: Extend the preempt disabled section in dereference_symbol_descriptor().

[Sebastian Andrzej Siewior]

On 2025-01-08 10:55:04 [+0100], Helge Deller wrote:
> Nice catch.
> 
> Acked-by: Helge Deller <deller@gmx.de>
> 
> This patch really should be backported.
> Can you add a Cc: stable tag?

It should be picked due to the fixes tag. I add it if I am going to
repost it.

> Helge

Sebastian

[PATCH v3 02/28] module: Begin to move from RCU-sched to RCU.

[Sebastian Andrzej Siewior]

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.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
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.47.1

[PATCH v3 03/28] module: Use proper RCU assignment in add_kallsyms().

[Sebastian Andrzej Siewior]

add_kallsyms() assigns the RCU pointer module::kallsyms and setups the
structures behind it which point to init-data. The module was not
published yet, nothing can see the kallsyms pointer and the data behind
it. Also module's init function was not yet invoked.
There is no need to use rcu_dereference() here, it is just to keep
checkers quiet. The whole RCU read section is also not needed.

Use a local kallsyms pointer and setup the data structures. Assign that
pointer to the data structure at the end via rcu_assign_pointer().

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/module/kallsyms.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index bf65e0c3c86fc..45846ae4042d1 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -177,19 +177,15 @@ void add_kallsyms(struct module *mod, const struct load_info *info)
 	unsigned long strtab_size;
 	void *data_base = mod->mem[MOD_DATA].base;
 	void *init_data_base = mod->mem[MOD_INIT_DATA].base;
+	struct mod_kallsyms *kallsyms;
 
-	/* Set up to point into init section. */
-	mod->kallsyms = (void __rcu *)init_data_base +
-		info->mod_kallsyms_init_off;
+	kallsyms = init_data_base + info->mod_kallsyms_init_off;
 
-	rcu_read_lock();
-	/* The following is safe since this pointer cannot change */
-	rcu_dereference(mod->kallsyms)->symtab = (void *)symsec->sh_addr;
-	rcu_dereference(mod->kallsyms)->num_symtab = symsec->sh_size / sizeof(Elf_Sym);
+	kallsyms->symtab = (void *)symsec->sh_addr;
+	kallsyms->num_symtab = symsec->sh_size / sizeof(Elf_Sym);
 	/* Make sure we get permanent strtab: don't use info->strtab. */
-	rcu_dereference(mod->kallsyms)->strtab =
-		(void *)info->sechdrs[info->index.str].sh_addr;
-	rcu_dereference(mod->kallsyms)->typetab = init_data_base + info->init_typeoffs;
+	kallsyms->strtab = (void *)info->sechdrs[info->index.str].sh_addr;
+	kallsyms->typetab = init_data_base + info->init_typeoffs;
 
 	/*
 	 * Now populate the cut down core kallsyms for after init
@@ -199,20 +195,19 @@ void add_kallsyms(struct module *mod, const struct load_info *info)
 	mod->core_kallsyms.strtab = s = data_base + info->stroffs;
 	mod->core_kallsyms.typetab = data_base + info->core_typeoffs;
 	strtab_size = info->core_typeoffs - info->stroffs;
-	src = rcu_dereference(mod->kallsyms)->symtab;
-	for (ndst = i = 0; i < rcu_dereference(mod->kallsyms)->num_symtab; i++) {
-		rcu_dereference(mod->kallsyms)->typetab[i] = elf_type(src + i, info);
+	src = kallsyms->symtab;
+	for (ndst = i = 0; i < kallsyms->num_symtab; i++) {
+		kallsyms->typetab[i] = elf_type(src + i, info);
 		if (i == 0 || is_livepatch_module(mod) ||
 		    is_core_symbol(src + i, info->sechdrs, info->hdr->e_shnum,
 				   info->index.pcpu)) {
 			ssize_t ret;
 
 			mod->core_kallsyms.typetab[ndst] =
-			    rcu_dereference(mod->kallsyms)->typetab[i];
+				kallsyms->typetab[i];
 			dst[ndst] = src[i];
 			dst[ndst++].st_name = s - mod->core_kallsyms.strtab;
-			ret = strscpy(s,
-				      &rcu_dereference(mod->kallsyms)->strtab[src[i].st_name],
+			ret = strscpy(s, &kallsyms->strtab[src[i].st_name],
 				      strtab_size);
 			if (ret < 0)
 				break;
@@ -220,7 +215,9 @@ void add_kallsyms(struct module *mod, const struct load_info *info)
 			strtab_size -= ret + 1;
 		}
 	}
-	rcu_read_unlock();
+
+	/* Set up to point into init section. */
+	rcu_assign_pointer(mod->kallsyms, kallsyms);
 	mod->core_kallsyms.num_symtab = ndst;
 }
 
-- 
2.47.1

[PATCH v3 04/28] module: Use RCU in find_kallsyms_symbol().

[Sebastian Andrzej Siewior]

The modules list and module::kallsyms can be accessed under RCU
assumption.

Use rcu_dereference() to reference the kallsyms pointer in
find_kallsyms_symbol().  Use a RCU section instead of preempt_disable in
callers of find_kallsyms_symbol(). Keep the preempt-disable in
module_address_lookup() due to __module_address().

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/module/kallsyms.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index 45846ae4042d1..3f59d04795572 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -257,7 +257,7 @@ static const char *find_kallsyms_symbol(struct module *mod,
 {
 	unsigned int i, best = 0;
 	unsigned long nextval, bestval;
-	struct mod_kallsyms *kallsyms = rcu_dereference_sched(mod->kallsyms);
+	struct mod_kallsyms *kallsyms = rcu_dereference(mod->kallsyms);
 	struct module_memory *mod_mem;
 
 	/* At worse, next value is at end of module */
@@ -329,6 +329,7 @@ int module_address_lookup(unsigned long addr,
 	int ret = 0;
 	struct module *mod;
 
+	guard(rcu)();
 	preempt_disable();
 	mod = __module_address(addr);
 	if (mod) {
@@ -356,7 +357,7 @@ int lookup_module_symbol_name(unsigned long addr, char *symname)
 {
 	struct module *mod;
 
-	preempt_disable();
+	guard(rcu)();
 	list_for_each_entry_rcu(mod, &modules, list) {
 		if (mod->state == MODULE_STATE_UNFORMED)
 			continue;
@@ -368,12 +369,10 @@ int lookup_module_symbol_name(unsigned long addr, char *symname)
 				goto out;
 
 			strscpy(symname, sym, KSYM_NAME_LEN);
-			preempt_enable();
 			return 0;
 		}
 	}
 out:
-	preempt_enable();
 	return -ERANGE;
 }
 
-- 
2.47.1

[PATCH v3 05/28] module: Use RCU in module_get_kallsym().

[Sebastian Andrzej Siewior]

The modules list and module::kallsyms can be accessed under RCU
assumption.

Iterate the modules with RCU protection, use rcu_dereference() to access
the kallsyms pointer.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/module/kallsyms.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index 3f59d04795572..4eef518204eb5 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -381,13 +381,13 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
 {
 	struct module *mod;
 
-	preempt_disable();
+	guard(rcu)();
 	list_for_each_entry_rcu(mod, &modules, list) {
 		struct mod_kallsyms *kallsyms;
 
 		if (mod->state == MODULE_STATE_UNFORMED)
 			continue;
-		kallsyms = rcu_dereference_sched(mod->kallsyms);
+		kallsyms = rcu_dereference(mod->kallsyms);
 		if (symnum < kallsyms->num_symtab) {
 			const Elf_Sym *sym = &kallsyms->symtab[symnum];
 
@@ -396,12 +396,10 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
 			strscpy(name, kallsyms_symbol_name(kallsyms, symnum), KSYM_NAME_LEN);
 			strscpy(module_name, mod->name, MODULE_NAME_LEN);
 			*exported = is_exported(name, *value, mod);
-			preempt_enable();
 			return 0;
 		}
 		symnum -= kallsyms->num_symtab;
 	}
-	preempt_enable();
 	return -ERANGE;
 }
 
-- 
2.47.1

[PATCH v3 06/28] module: Use RCU in find_module_all().

[Sebastian Andrzej Siewior]

The modules list and module::kallsyms can be accessed under RCU
assumption.

Remove module_assert_mutex_or_preempt() from find_module_all() so it can
be used under RCU protection without warnings. Update its callers to use
RCU protection instead of preempt_disable().

Cc: Jiri Kosina <jikos@kernel.org>
Cc: Joe Lawrence <joe.lawrence@redhat.com>
Cc: Josh Poimboeuf <jpoimboe@kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Miroslav Benes <mbenes@suse.cz>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-trace-kernel@vger.kernel.org
Cc: live-patching@vger.kernel.org
Reviewed-by: Petr Mladek <pmladek@suse.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/module.h      | 2 +-
 kernel/livepatch/core.c     | 4 +---
 kernel/module/kallsyms.c    | 1 +
 kernel/module/main.c        | 6 ++----
 kernel/trace/trace_kprobe.c | 9 +++------
 5 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 94acbacdcdf18..5c1f7ea76c8cb 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -663,7 +663,7 @@ static inline bool within_module(unsigned long addr, const struct module *mod)
 	return within_module_init(addr, mod) || within_module_core(addr, mod);
 }
 
-/* Search for module by name: must be in a RCU-sched critical section. */
+/* Search for module by name: must be in a RCU critical section. */
 struct module *find_module(const char *name);
 
 extern void __noreturn __module_put_and_kthread_exit(struct module *mod,
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 3c21c31796db0..f8932c63b08e3 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -59,7 +59,7 @@ static void klp_find_object_module(struct klp_object *obj)
 	if (!klp_is_module(obj))
 		return;
 
-	rcu_read_lock_sched();
+	guard(rcu)();
 	/*
 	 * We do not want to block removal of patched modules and therefore
 	 * we do not take a reference here. The patches are removed by
@@ -75,8 +75,6 @@ static void klp_find_object_module(struct klp_object *obj)
 	 */
 	if (mod && mod->klp_alive)
 		obj->mod = mod;
-
-	rcu_read_unlock_sched();
 }
 
 static bool klp_initialized(void)
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index 4eef518204eb5..3cba9f933b24f 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -450,6 +450,7 @@ unsigned long module_kallsyms_lookup_name(const char *name)
 	unsigned long ret;
 
 	/* Don't lock: we're in enough trouble already. */
+	guard(rcu)();
 	preempt_disable();
 	ret = __module_kallsyms_lookup_name(name);
 	preempt_enable();
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 5cce4a92d7ba3..5aa56ec8e203e 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -374,16 +374,14 @@ bool find_symbol(struct find_symbol_arg *fsa)
 }
 
 /*
- * Search for module by name: must hold module_mutex (or preempt disabled
- * for read-only access).
+ * Search for module by name: must hold module_mutex (or RCU for read-only
+ * access).
  */
 struct module *find_module_all(const char *name, size_t len,
 			       bool even_unformed)
 {
 	struct module *mod;
 
-	module_assert_mutex_or_preempt();
-
 	list_for_each_entry_rcu(mod, &modules, list,
 				lockdep_is_held(&module_mutex)) {
 		if (!even_unformed && mod->state == MODULE_STATE_UNFORMED)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 935a886af40c9..37ff78ee17fe0 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -123,9 +123,8 @@ static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
 	if (!p)
 		return true;
 	*p = '\0';
-	rcu_read_lock_sched();
-	ret = !!find_module(tk->symbol);
-	rcu_read_unlock_sched();
+	scoped_guard(rcu)
+		ret = !!find_module(tk->symbol);
 	*p = ':';
 
 	return ret;
@@ -800,12 +799,10 @@ static struct module *try_module_get_by_name(const char *name)
 {
 	struct module *mod;
 
-	rcu_read_lock_sched();
+	guard(rcu)();
 	mod = find_module(name);
 	if (mod && !try_module_get(mod))
 		mod = NULL;
-	rcu_read_unlock_sched();
-
 	return mod;
 }
 #else
-- 
2.47.1

[PATCH v3 07/28] module: Use RCU in __find_kallsyms_symbol_value().

[Sebastian Andrzej Siewior]

module::kallsyms can be accessed under RCU assumption.

Use rcu_dereference() to access module::kallsyms.
Update callers.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/module/kallsyms.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index 3cba9f933b24f..e3c55bc879c11 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -407,7 +407,7 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
 static unsigned long __find_kallsyms_symbol_value(struct module *mod, const char *name)
 {
 	unsigned int i;
-	struct mod_kallsyms *kallsyms = rcu_dereference_sched(mod->kallsyms);
+	struct mod_kallsyms *kallsyms = rcu_dereference(mod->kallsyms);
 
 	for (i = 0; i < kallsyms->num_symtab; i++) {
 		const Elf_Sym *sym = &kallsyms->symtab[i];
@@ -447,24 +447,15 @@ static unsigned long __module_kallsyms_lookup_name(const char *name)
 /* Look for this name: can be of form module:name. */
 unsigned long module_kallsyms_lookup_name(const char *name)
 {
-	unsigned long ret;
-
 	/* Don't lock: we're in enough trouble already. */
 	guard(rcu)();
-	preempt_disable();
-	ret = __module_kallsyms_lookup_name(name);
-	preempt_enable();
-	return ret;
+	return __module_kallsyms_lookup_name(name);
 }
 
 unsigned long find_kallsyms_symbol_value(struct module *mod, const char *name)
 {
-	unsigned long ret;
-
-	preempt_disable();
-	ret = __find_kallsyms_symbol_value(mod, name);
-	preempt_enable();
-	return ret;
+	guard(rcu)();
+	return __find_kallsyms_symbol_value(mod, name);
 }
 
 int module_kallsyms_on_each_symbol(const char *modname,
-- 
2.47.1

[PATCH v3 08/28] module: Use RCU in module_kallsyms_on_each_symbol().

[Sebastian Andrzej Siewior]

module::kallsyms can be accessed under RCU assumption.

Use rcu_dereference() to access module::kallsyms.
Update callers.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/module/kallsyms.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index e3c55bc879c11..0e8ec6486d95c 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -476,10 +476,8 @@ int module_kallsyms_on_each_symbol(const char *modname,
 		if (modname && strcmp(modname, mod->name))
 			continue;
 
-		/* Use rcu_dereference_sched() to remain compliant with the sparse tool */
-		preempt_disable();
-		kallsyms = rcu_dereference_sched(mod->kallsyms);
-		preempt_enable();
+		kallsyms = rcu_dereference_check(mod->kallsyms,
+						 lockdep_is_held(&module_mutex));
 
 		for (i = 0; i < kallsyms->num_symtab; i++) {
 			const Elf_Sym *sym = &kallsyms->symtab[i];
-- 
2.47.1

[PATCH v3 09/28] module: Remove module_assert_mutex_or_preempt() from try_add_tainted_module().

[Sebastian Andrzej Siewior]

module_assert_mutex_or_preempt() is not needed in
try_add_tainted_module(). The function checks for RCU-sched or the
module_mutex to be acquired. The list_for_each_entry_rcu() below does
the same check.

Remove module_assert_mutex_or_preempt() from try_add_tainted_module().

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/module/tracking.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/module/tracking.c b/kernel/module/tracking.c
index 16742d1c630c6..4fefec5b683c6 100644
--- a/kernel/module/tracking.c
+++ b/kernel/module/tracking.c
@@ -21,8 +21,6 @@ int try_add_tainted_module(struct module *mod)
 {
 	struct mod_unload_taint *mod_taint;
 
-	module_assert_mutex_or_preempt();
-
 	if (!mod->taints)
 		goto out;
 
-- 
2.47.1

[PATCH v3 10/28] module: Use RCU in find_symbol().

[Sebastian Andrzej Siewior]

module_assert_mutex_or_preempt() is not needed in find_symbol(). The
function checks for RCU-sched or the module_mutex to be acquired. The
list_for_each_entry_rcu() below does the same check.

Remove module_assert_mutex_or_preempt() from try_add_tainted_module().
Use RCU protection to invoke find_symbol() and update callers.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/module/main.c    | 30 ++++++++++++------------------
 kernel/module/version.c | 14 +++++++-------
 2 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 5aa56ec8e203e..71e73deed76c0 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -331,7 +331,7 @@ static bool find_exported_symbol_in_section(const struct symsearch *syms,
 
 /*
  * Find an exported symbol and return it, along with, (optional) crc and
- * (optional) module which owns it.  Needs preempt disabled or module_mutex.
+ * (optional) module which owns it. Needs RCU or module_mutex.
  */
 bool find_symbol(struct find_symbol_arg *fsa)
 {
@@ -345,8 +345,6 @@ bool find_symbol(struct find_symbol_arg *fsa)
 	struct module *mod;
 	unsigned int i;
 
-	module_assert_mutex_or_preempt();
-
 	for (i = 0; i < ARRAY_SIZE(arr); i++)
 		if (find_exported_symbol_in_section(&arr[i], NULL, fsa))
 			return true;
@@ -812,10 +810,9 @@ void __symbol_put(const char *symbol)
 		.gplok	= true,
 	};
 
-	preempt_disable();
+	guard(rcu)();
 	BUG_ON(!find_symbol(&fsa));
 	module_put(fsa.owner);
-	preempt_enable();
 }
 EXPORT_SYMBOL(__symbol_put);
 
@@ -1369,21 +1366,18 @@ void *__symbol_get(const char *symbol)
 		.warn	= true,
 	};
 
-	preempt_disable();
-	if (!find_symbol(&fsa))
-		goto fail;
-	if (fsa.license != GPL_ONLY) {
-		pr_warn("failing symbol_get of non-GPLONLY symbol %s.\n",
-			symbol);
-		goto fail;
+	scoped_guard(rcu) {
+		if (!find_symbol(&fsa))
+			return NULL;
+		if (fsa.license != GPL_ONLY) {
+			pr_warn("failing symbol_get of non-GPLONLY symbol %s.\n",
+				symbol);
+			return NULL;
+		}
+		if (strong_try_module_get(fsa.owner))
+			return NULL;
 	}
-	if (strong_try_module_get(fsa.owner))
-		goto fail;
-	preempt_enable();
 	return (void *)kernel_symbol_value(fsa.sym);
-fail:
-	preempt_enable();
-	return NULL;
 }
 EXPORT_SYMBOL_GPL(__symbol_get);
 
diff --git a/kernel/module/version.c b/kernel/module/version.c
index 53f43ac5a73e9..65ef8f2a821da 100644
--- a/kernel/module/version.c
+++ b/kernel/module/version.c
@@ -62,17 +62,17 @@ int check_modstruct_version(const struct load_info *info,
 		.name	= "module_layout",
 		.gplok	= true,
 	};
+	bool have_symbol;
 
 	/*
 	 * Since this should be found in kernel (which can't be removed), no
-	 * locking is necessary -- use preempt_disable() to placate lockdep.
+	 * locking is necessary. Regardless use a RCU read section to keep
+	 * lockdep happy.
 	 */
-	preempt_disable();
-	if (!find_symbol(&fsa)) {
-		preempt_enable();
-		BUG();
-	}
-	preempt_enable();
+	scoped_guard(rcu)
+		have_symbol = find_symbol(&fsa);
+	BUG_ON(!have_symbol);
+
 	return check_version(info, "module_layout", mod, fsa.crc);
 }
 
-- 
2.47.1

[PATCH v3 11/28] module: Use RCU in __is_module_percpu_address().

[Sebastian Andrzej Siewior]

The modules list can be accessed under RCU assumption.

Use RCU protection instead preempt_disable().

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/module/main.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 71e73deed76c0..126f7f05dedf8 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -450,8 +450,7 @@ bool __is_module_percpu_address(unsigned long addr, unsigned long *can_addr)
 	struct module *mod;
 	unsigned int cpu;
 
-	preempt_disable();
-
+	guard(rcu)();
 	list_for_each_entry_rcu(mod, &modules, list) {
 		if (mod->state == MODULE_STATE_UNFORMED)
 			continue;
@@ -468,13 +467,10 @@ bool __is_module_percpu_address(unsigned long addr, unsigned long *can_addr)
 						per_cpu_ptr(mod->percpu,
 							    get_boot_cpu_id());
 				}
-				preempt_enable();
 				return true;
 			}
 		}
 	}
-
-	preempt_enable();
 	return false;
 }
 
-- 
2.47.1

[PATCH v3 12/28] module: Allow __module_address() to be called from RCU section.

[Sebastian Andrzej Siewior]

mod_find() uses either the modules list to find a module or a tree
lookup (CONFIG_MODULES_TREE_LOOKUP). The list and the tree can both be
iterated under RCU assumption (as well as RCU-sched).

Remove module_assert_mutex_or_preempt() from __module_address() and
entirely since __module_address() is the last user.
Update comments.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/module/internal.h | 11 -----------
 kernel/module/main.c     |  4 +---
 2 files changed, 1 insertion(+), 14 deletions(-)

diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index daef2be839022..030d2ed175fa8 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -122,17 +122,6 @@ char *module_next_tag_pair(char *string, unsigned long *secsize);
 #define for_each_modinfo_entry(entry, info, name) \
 	for (entry = get_modinfo(info, name); entry; entry = get_next_modinfo(info, name, entry))
 
-static inline void module_assert_mutex_or_preempt(void)
-{
-#ifdef CONFIG_LOCKDEP
-	if (unlikely(!debug_locks))
-		return;
-
-	WARN_ON_ONCE(!rcu_read_lock_sched_held() &&
-		     !lockdep_is_held(&module_mutex));
-#endif
-}
-
 static inline unsigned long kernel_symbol_value(const struct kernel_symbol *sym)
 {
 #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 126f7f05dedf8..686b74c7c17f5 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3667,7 +3667,7 @@ bool is_module_address(unsigned long addr)
  * __module_address() - get the module which contains an address.
  * @addr: the address.
  *
- * Must be called with preempt disabled or module mutex held so that
+ * Must be called within RCU read section or module mutex held so that
  * module doesn't get freed during this.
  */
 struct module *__module_address(unsigned long addr)
@@ -3685,8 +3685,6 @@ struct module *__module_address(unsigned long addr)
 	return NULL;
 
 lookup:
-	module_assert_mutex_or_preempt();
-
 	mod = mod_find(addr, &mod_tree);
 	if (mod) {
 		BUG_ON(!within_module(addr, mod));
-- 
2.47.1

[PATCH v3 13/28] module: Use RCU in search_module_extables().

[Sebastian Andrzej Siewior]

search_module_extables() returns an exception_table_entry belonging to a
module. The lookup via __module_address() can be performed with RCU
protection.
The returned exception_table_entry remains valid because the passed
address usually belongs to a module that is currently executed. So the
module can not be removed because "something else" holds a reference to
it, ensuring that it can not be removed.
Exceptions here are:
- kprobe, acquires a reference on the module beforehand
- MCE, invokes the function from within a timer and the RCU lifetime
  guarantees (of the timer) are sufficient.

Therefore it is safe to return the exception_table_entry outside the RCU
section which provided the module.

Use RCU for the lookup in search_module_extables() and update the
comment.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/module/main.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 686b74c7c17f5..74b9e9ddb4b65 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3621,28 +3621,23 @@ char *module_flags(struct module *mod, char *buf, bool show_state)
 /* Given an address, look for it in the module exception tables. */
 const struct exception_table_entry *search_module_extables(unsigned long addr)
 {
-	const struct exception_table_entry *e = NULL;
 	struct module *mod;
 
-	preempt_disable();
+	guard(rcu)();
 	mod = __module_address(addr);
 	if (!mod)
-		goto out;
+		return NULL;
 
 	if (!mod->num_exentries)
-		goto out;
-
-	e = search_extable(mod->extable,
-			   mod->num_exentries,
-			   addr);
-out:
-	preempt_enable();
-
+		return NULL;
 	/*
-	 * Now, if we found one, we are running inside it now, hence
-	 * we cannot unload the module, hence no refcnt needed.
+	 * The address passed here belongs to a module that is currently
+	 * invoked (we are running inside it). Therefore its module::refcnt
+	 * needs already be >0 to ensure that it is not removed at this stage.
+	 * All other user need to invoke this function within a RCU read
+	 * section.
 	 */
-	return e;
+	return search_extable(mod->extable, mod->num_exentries, addr);
 }
 
 /**
-- 
2.47.1

[PATCH v3 14/28] module: Use RCU in all users of __module_address().

[Sebastian Andrzej Siewior]

__module_address() can be invoked within a RCU section, there is no
requirement to have preemption disabled.

Replace the preempt_disable() section around __module_address() with
RCU.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/kallsyms.h | 3 +--
 kernel/module/kallsyms.c | 5 +----
 kernel/module/main.c     | 9 ++-------
 3 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index 1c6a6c1704d8d..d5dd54c53ace6 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -55,12 +55,11 @@ static inline void *dereference_symbol_descriptor(void *ptr)
 	if (is_ksym_addr((unsigned long)ptr))
 		return ptr;
 
-	preempt_disable();
+	guard(rcu)();
 	mod = __module_address((unsigned long)ptr);
 
 	if (mod)
 		ptr = dereference_module_function_descriptor(mod, ptr);
-	preempt_enable();
 #endif
 	return ptr;
 }
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index 0e8ec6486d95c..00a60796327c0 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -316,7 +316,7 @@ void * __weak dereference_module_function_descriptor(struct module *mod,
 
 /*
  * For kallsyms to ask for address resolution.  NULL means not found.  Careful
- * not to lock to avoid deadlock on oopses, simply disable preemption.
+ * not to lock to avoid deadlock on oopses, RCU is enough.
  */
 int module_address_lookup(unsigned long addr,
 			  unsigned long *size,
@@ -330,7 +330,6 @@ int module_address_lookup(unsigned long addr,
 	struct module *mod;
 
 	guard(rcu)();
-	preempt_disable();
 	mod = __module_address(addr);
 	if (mod) {
 		if (modname)
@@ -348,8 +347,6 @@ int module_address_lookup(unsigned long addr,
 		if (sym)
 			ret = strscpy(namebuf, sym, KSYM_NAME_LEN);
 	}
-	preempt_enable();
-
 	return ret;
 }
 
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 74b9e9ddb4b65..80877741ac7e5 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3649,13 +3649,8 @@ const struct exception_table_entry *search_module_extables(unsigned long addr)
  */
 bool is_module_address(unsigned long addr)
 {
-	bool ret;
-
-	preempt_disable();
-	ret = __module_address(addr) != NULL;
-	preempt_enable();
-
-	return ret;
+	guard(rcu)();
+	return __module_address(addr) != NULL;
 }
 
 /**
-- 
2.47.1

[PATCH v3 15/28] module: Use RCU in all users of __module_text_address().

[Sebastian Andrzej Siewior]

__module_text_address() can be invoked within a RCU section, there is no
requirement to have preemption disabled.

Replace the preempt_disable() section around __module_text_address()
with RCU.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/module/main.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 80877741ac7e5..6a99076146cbc 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -823,13 +823,12 @@ void symbol_put_addr(void *addr)
 
 	/*
 	 * Even though we hold a reference on the module; we still need to
-	 * disable preemption in order to safely traverse the data structure.
+	 * RCU read section in order to safely traverse the data structure.
 	 */
-	preempt_disable();
+	guard(rcu)();
 	modaddr = __module_text_address(a);
 	BUG_ON(!modaddr);
 	module_put(modaddr);
-	preempt_enable();
 }
 EXPORT_SYMBOL_GPL(symbol_put_addr);
 
@@ -3694,20 +3693,15 @@ struct module *__module_address(unsigned long addr)
  */
 bool is_module_text_address(unsigned long addr)
 {
-	bool ret;
-
-	preempt_disable();
-	ret = __module_text_address(addr) != NULL;
-	preempt_enable();
-
-	return ret;
+	guard(rcu)();
+	return __module_text_address(addr) != NULL;
 }
 
 /**
  * __module_text_address() - get the module whose code contains an address.
  * @addr: the address.
  *
- * Must be called with preempt disabled or module mutex held so that
+ * Must be called within RCU read section or module mutex held so that
  * module doesn't get freed during this.
  */
 struct module *__module_text_address(unsigned long addr)
-- 
2.47.1

Re: [PATCH v3 15/28] module: Use RCU in all users of __module_text_address().

[Benjamin Berg]

Hi,

On Wed, 2025-01-08 at 10:04 +0100, Sebastian Andrzej Siewior wrote:
> __module_text_address() can be invoked within a RCU section, there is no
> requirement to have preemption disabled.
> 
> Replace the preempt_disable() section around __module_text_address()
> with RCU.

Unfortunately, this patch causes a performance regression for us. The
trouble is that we enable kmemleak and run trace-cmd so a lot of stack
traces need to be collected. Obviously, we also have lockdep enabled.

Now, combine this with the UML stack dumping code calling into
__kernel_text_address a lot[1] and it really has a relevant performance
impact. I saw the kernel spending 40% of its own CPU time just on the
lock in is_module_text_address.

Maybe kernel_text_address should leave the RCU handling to the caller
and assume that the RCU read lock is already taken?

Benjamin

[1] The UM arch dump_stack function reads every "unsigned long" on the
stack and tests it using __kernel_text_address.

> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  kernel/module/main.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 80877741ac7e5..6a99076146cbc 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -823,13 +823,12 @@ void symbol_put_addr(void *addr)
>  
>  	/*
>  	 * Even though we hold a reference on the module; we still
> need to
> -	 * disable preemption in order to safely traverse the data
> structure.
> +	 * RCU read section in order to safely traverse the data
> structure.
>  	 */
> -	preempt_disable();
> +	guard(rcu)();
>  	modaddr = __module_text_address(a);
>  	BUG_ON(!modaddr);
>  	module_put(modaddr);
> -	preempt_enable();
>  }
>  EXPORT_SYMBOL_GPL(symbol_put_addr);
>  
> @@ -3694,20 +3693,15 @@ struct module *__module_address(unsigned long
> addr)
>   */
>  bool is_module_text_address(unsigned long addr)
>  {
> -	bool ret;
> -
> -	preempt_disable();
> -	ret = __module_text_address(addr) != NULL;
> -	preempt_enable();
> -
> -	return ret;
> +	guard(rcu)();
> +	return __module_text_address(addr) != NULL;
>  }
>  
>  /**
>   * __module_text_address() - get the module whose code contains an
> address.
>   * @addr: the address.
>   *
> - * Must be called with preempt disabled or module mutex held so that
> + * Must be called within RCU read section or module mutex held so
> that
>   * module doesn't get freed during this.
>   */
>  struct module *__module_text_address(unsigned long addr)

Re: [PATCH v3 15/28] module: Use RCU in all users of __module_text_address().

[Paul E. McKenney]

On Wed, Apr 23, 2025 at 05:17:31PM +0200, Benjamin Berg wrote:
> Hi,
> 
> On Wed, 2025-01-08 at 10:04 +0100, Sebastian Andrzej Siewior wrote:
> > __module_text_address() can be invoked within a RCU section, there is no
> > requirement to have preemption disabled.
> > 
> > Replace the preempt_disable() section around __module_text_address()
> > with RCU.
> 
> Unfortunately, this patch causes a performance regression for us. The
> trouble is that we enable kmemleak and run trace-cmd so a lot of stack
> traces need to be collected. Obviously, we also have lockdep enabled.
> 
> Now, combine this with the UML stack dumping code calling into
> __kernel_text_address a lot[1] and it really has a relevant performance
> impact. I saw the kernel spending 40% of its own CPU time just on the
> lock in is_module_text_address.
> 
> Maybe kernel_text_address should leave the RCU handling to the caller
> and assume that the RCU read lock is already taken?
> 
> Benjamin
> 
> [1] The UM arch dump_stack function reads every "unsigned long" on the
> stack and tests it using __kernel_text_address.

Use of a single guard(rcu)() is regressing performance?  Interesting and
quite unexpected.  That said, tiven the amount of debug you have enabled,
I am not so sure that people are going to be all that excited about a
further performance regression.

But is this regression due to the cleanup hook that guard(rcu)()
registers?  If so, please feel free to try using rcu_read_lock()
and rcu_read_unlock() instead.  I would be surprised if this makes a
difference, but then again, your initial regression report also comes
as a surprise, so...

Another way to reduce guard(rcu)() overhead is to build your kernel
with CONFIG_PREEMPT_NONE=y.  Not so good for real-time response, but
then again, neither are your debug options.

							Thanx, Paul

> > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > ---
> >  kernel/module/main.c | 16 +++++-----------
> >  1 file changed, 5 insertions(+), 11 deletions(-)
> > 
> > diff --git a/kernel/module/main.c b/kernel/module/main.c
> > index 80877741ac7e5..6a99076146cbc 100644
> > --- a/kernel/module/main.c
> > +++ b/kernel/module/main.c
> > @@ -823,13 +823,12 @@ void symbol_put_addr(void *addr)
> >  
> >  	/*
> >  	 * Even though we hold a reference on the module; we still
> > need to
> > -	 * disable preemption in order to safely traverse the data
> > structure.
> > +	 * RCU read section in order to safely traverse the data
> > structure.
> >  	 */
> > -	preempt_disable();
> > +	guard(rcu)();
> >  	modaddr = __module_text_address(a);
> >  	BUG_ON(!modaddr);
> >  	module_put(modaddr);
> > -	preempt_enable();
> >  }
> >  EXPORT_SYMBOL_GPL(symbol_put_addr);
> >  
> > @@ -3694,20 +3693,15 @@ struct module *__module_address(unsigned long
> > addr)
> >   */
> >  bool is_module_text_address(unsigned long addr)
> >  {
> > -	bool ret;
> > -
> > -	preempt_disable();
> > -	ret = __module_text_address(addr) != NULL;
> > -	preempt_enable();
> > -
> > -	return ret;
> > +	guard(rcu)();
> > +	return __module_text_address(addr) != NULL;
> >  }
> >  
> >  /**
> >   * __module_text_address() - get the module whose code contains an
> > address.
> >   * @addr: the address.
> >   *
> > - * Must be called with preempt disabled or module mutex held so that
> > + * Must be called within RCU read section or module mutex held so
> > that
> >   * module doesn't get freed during this.
> >   */
> >  struct module *__module_text_address(unsigned long addr)
> 

Re: [PATCH v3 15/28] module: Use RCU in all users of __module_text_address().

[Sebastian Andrzej Siewior]

On 2025-04-23 11:16:49 [-0700], Paul E. McKenney wrote:
> On Wed, Apr 23, 2025 at 05:17:31PM +0200, Benjamin Berg wrote:
> > Hi,
> > 
> > On Wed, 2025-01-08 at 10:04 +0100, Sebastian Andrzej Siewior wrote:
> > > __module_text_address() can be invoked within a RCU section, there is no
> > > requirement to have preemption disabled.
> > > 
> > > Replace the preempt_disable() section around __module_text_address()
> > > with RCU.
> > 
> > Unfortunately, this patch causes a performance regression for us. The
> > trouble is that we enable kmemleak and run trace-cmd so a lot of stack
> > traces need to be collected. Obviously, we also have lockdep enabled.
> > 
> > Now, combine this with the UML stack dumping code calling into
> > __kernel_text_address a lot[1] and it really has a relevant performance
> > impact. I saw the kernel spending 40% of its own CPU time just on the
> > lock in is_module_text_address.
> > 
> > Maybe kernel_text_address should leave the RCU handling to the caller
> > and assume that the RCU read lock is already taken?
> > 
> > Benjamin
> > 
> > [1] The UM arch dump_stack function reads every "unsigned long" on the
> > stack and tests it using __kernel_text_address.
> 
> Use of a single guard(rcu)() is regressing performance?  Interesting and
> quite unexpected.  That said, tiven the amount of debug you have enabled,
> I am not so sure that people are going to be all that excited about a
> further performance regression.
> 
> But is this regression due to the cleanup hook that guard(rcu)()
> registers?  If so, please feel free to try using rcu_read_lock()
> and rcu_read_unlock() instead.  I would be surprised if this makes a
> difference, but then again, your initial regression report also comes
> as a surprise, so...
> 
> Another way to reduce guard(rcu)() overhead is to build your kernel
> with CONFIG_PREEMPT_NONE=y.  Not so good for real-time response, but
> then again, neither are your debug options.

The guard notation is not regression I guess it is just the plenty of
rcu_read_lock()/ unlock(). We had one regression which was "fixed" by
commit ee57ab5a32129 ("locking/lockdep: Disable KASAN instrumentation of lockdep.c").

My guess would be that this is a preemptible kernel and the preempt
disable/ enable is cheaper that the RCU version. So going back to a
non-preemtible kernel should "fix" it.

Looking at kernel_text_address(), is_bpf_text_address() has also a
RCU read section so probably subject to the same trouble. And
is_ftrace_trampoline() could be also converted to RCU which would
increase the trouble. 

Improving the stack trace on UM or caching some of the most common one
might help. Not sure if disabling kmemleak for lockdep is possible/
makes a difference.

> 							Thanx, Paul

Sebastian

Re: [PATCH v3 15/28] module: Use RCU in all users of __module_text_address().

[Benjamin Berg]

On Thu, 2025-04-24 at 11:05 +0200, Sebastian Andrzej Siewior wrote:
> On 2025-04-23 11:16:49 [-0700], Paul E. McKenney wrote:
> > On Wed, Apr 23, 2025 at 05:17:31PM +0200, Benjamin Berg wrote:
> > > Hi,
> > > 
> > > On Wed, 2025-01-08 at 10:04 +0100, Sebastian Andrzej Siewior wrote:
> > > > __module_text_address() can be invoked within a RCU section, there is no
> > > > requirement to have preemption disabled.
> > > > 
> > > > Replace the preempt_disable() section around __module_text_address()
> > > > with RCU.
> > > 
> > > Unfortunately, this patch causes a performance regression for us. The
> > > trouble is that we enable kmemleak and run trace-cmd so a lot of stack
> > > traces need to be collected. Obviously, we also have lockdep enabled.
> > > 
> > > Now, combine this with the UML stack dumping code calling into
> > > __kernel_text_address a lot[1] and it really has a relevant performance
> > > impact. I saw the kernel spending 40% of its own CPU time just on the
> > > lock in is_module_text_address.
> > > 
> > > Maybe kernel_text_address should leave the RCU handling to the caller
> > > and assume that the RCU read lock is already taken?
> > > 
> > > Benjamin
> > > 
> > > [1] The UM arch dump_stack function reads every "unsigned long" on the
> > > stack and tests it using __kernel_text_address.
> > 
> > Use of a single guard(rcu)() is regressing performance?  Interesting and
> > quite unexpected.  That said, tiven the amount of debug you have enabled,
> > I am not so sure that people are going to be all that excited about a
> > further performance regression.
> > 
> > But is this regression due to the cleanup hook that guard(rcu)()
> > registers?  If so, please feel free to try using rcu_read_lock()
> > and rcu_read_unlock() instead.  I would be surprised if this makes a
> > difference, but then again, your initial regression report also comes
> > as a surprise, so...
> > 
> > Another way to reduce guard(rcu)() overhead is to build your kernel
> > with CONFIG_PREEMPT_NONE=y.  Not so good for real-time response, but
> > then again, neither are your debug options.
> 
> The guard notation is not regression I guess it is just the plenty of
> rcu_read_lock()/ unlock(). We had one regression which was "fixed" by
> commit ee57ab5a32129 ("locking/lockdep: Disable KASAN instrumentation of lockdep.c").

Yup, we really pretty much created a micro-benchmark for grabbing stack
traces.

> My guess would be that this is a preemptible kernel and the preempt
> disable/ enable is cheaper that the RCU version. So going back to a
> non-preemtible kernel should "fix" it.

Yes, preempt_disable() is extremely cheap.

> Looking at kernel_text_address(), is_bpf_text_address() has also a
> RCU read section so probably subject to the same trouble. And
> is_ftrace_trampoline() could be also converted to RCU which would
> increase the trouble. 
> 
> Improving the stack trace on UM or caching some of the most common one
> might help. Not sure if disabling kmemleak for lockdep is possible/
> makes a difference.

What does seem to help is to simply disable lockdep inside dump_trace.
That should be good enough for us at least, bringing the overhead down
to a manageable amount when running these tests.
Some unscientific numbers:

config                               dump_trace     locking
----
no locking (preempt_disable)            6 %         -
guard(rcu)() + lockdep_off             15 %         58 % of that
rcu_read_lock + lockdep_off            17 %         60 % of that
guard(rcu)()                           48 %         91 % of that

That confirms that guard(rcu)() really is not a problem. There might be
slight overhead, but it is probably within the margin of error. Turning
lockdep off/on inside the UML dump_trace() function brings down the
overhead a lot and I guess that should be an acceptable level for us.

Not sure if something like that would be desirable upstream. This is
happening for us when running the hostap "hwsim" tests inside UML (with
time-travel). At least internally, we could carry a custom patch to add
the lockdep_off()/lockdep_on() to dump_trace in order to work around
it[1].

Benjamin

[1] Actually, now I am reminded that we already have that for kmemleak
as lockdep was considerably slowing down the scanning.

> 
> > 							Thanx, Paul
> 
> Sebastian
> 

Re: [PATCH v3 15/28] module: Use RCU in all users of __module_text_address().

[Paul E. McKenney]

On Thu, Apr 24, 2025 at 11:30:39AM +0200, Benjamin Berg wrote:
> On Thu, 2025-04-24 at 11:05 +0200, Sebastian Andrzej Siewior wrote:
> > On 2025-04-23 11:16:49 [-0700], Paul E. McKenney wrote:
> > > On Wed, Apr 23, 2025 at 05:17:31PM +0200, Benjamin Berg wrote:
> > > > Hi,
> > > > 
> > > > On Wed, 2025-01-08 at 10:04 +0100, Sebastian Andrzej Siewior wrote:
> > > > > __module_text_address() can be invoked within a RCU section, there is no
> > > > > requirement to have preemption disabled.
> > > > > 
> > > > > Replace the preempt_disable() section around __module_text_address()
> > > > > with RCU.
> > > > 
> > > > Unfortunately, this patch causes a performance regression for us. The
> > > > trouble is that we enable kmemleak and run trace-cmd so a lot of stack
> > > > traces need to be collected. Obviously, we also have lockdep enabled.
> > > > 
> > > > Now, combine this with the UML stack dumping code calling into
> > > > __kernel_text_address a lot[1] and it really has a relevant performance
> > > > impact. I saw the kernel spending 40% of its own CPU time just on the
> > > > lock in is_module_text_address.
> > > > 
> > > > Maybe kernel_text_address should leave the RCU handling to the caller
> > > > and assume that the RCU read lock is already taken?
> > > > 
> > > > Benjamin
> > > > 
> > > > [1] The UM arch dump_stack function reads every "unsigned long" on the
> > > > stack and tests it using __kernel_text_address.
> > > 
> > > Use of a single guard(rcu)() is regressing performance?  Interesting and
> > > quite unexpected.  That said, tiven the amount of debug you have enabled,
> > > I am not so sure that people are going to be all that excited about a
> > > further performance regression.
> > > 
> > > But is this regression due to the cleanup hook that guard(rcu)()
> > > registers?  If so, please feel free to try using rcu_read_lock()
> > > and rcu_read_unlock() instead.  I would be surprised if this makes a
> > > difference, but then again, your initial regression report also comes
> > > as a surprise, so...
> > > 
> > > Another way to reduce guard(rcu)() overhead is to build your kernel
> > > with CONFIG_PREEMPT_NONE=y.  Not so good for real-time response, but
> > > then again, neither are your debug options.
> > 
> > The guard notation is not regression I guess it is just the plenty of
> > rcu_read_lock()/ unlock(). We had one regression which was "fixed" by
> > commit ee57ab5a32129 ("locking/lockdep: Disable KASAN instrumentation of lockdep.c").
> 
> Yup, we really pretty much created a micro-benchmark for grabbing stack
> traces.
> 
> > My guess would be that this is a preemptible kernel and the preempt
> > disable/ enable is cheaper that the RCU version. So going back to a
> > non-preemtible kernel should "fix" it.
> 
> Yes, preempt_disable() is extremely cheap.
> 
> > Looking at kernel_text_address(), is_bpf_text_address() has also a
> > RCU read section so probably subject to the same trouble. And
> > is_ftrace_trampoline() could be also converted to RCU which would
> > increase the trouble. 
> > 
> > Improving the stack trace on UM or caching some of the most common one
> > might help. Not sure if disabling kmemleak for lockdep is possible/
> > makes a difference.
> 
> What does seem to help is to simply disable lockdep inside dump_trace.
> That should be good enough for us at least, bringing the overhead down
> to a manageable amount when running these tests.
> Some unscientific numbers:
> 
> config                               dump_trace     locking
> ----
> no locking (preempt_disable)            6 %         -
> guard(rcu)() + lockdep_off             15 %         58 % of that
> rcu_read_lock + lockdep_off            17 %         60 % of that
> guard(rcu)()                           48 %         91 % of that
> 
> That confirms that guard(rcu)() really is not a problem. There might be
> slight overhead, but it is probably within the margin of error. Turning
> lockdep off/on inside the UML dump_trace() function brings down the
> overhead a lot and I guess that should be an acceptable level for us.

Whew!!!  ;-)

> Not sure if something like that would be desirable upstream. This is
> happening for us when running the hostap "hwsim" tests inside UML (with
> time-travel). At least internally, we could carry a custom patch to add
> the lockdep_off()/lockdep_on() to dump_trace in order to work around
> it[1].

That makes sense to me, but I am not the maintainer of that code.  ;-)

							Thanx, Paul

> Benjamin
> 
> [1] Actually, now I am reminded that we already have that for kmemleak
> as lockdep was considerably slowing down the scanning.
> 
> > 
> > > 							Thanx, Paul
> > 
> > Sebastian
> > 
> 

Re: [PATCH v3 15/28] module: Use RCU in all users of __module_text_address().

[Peter Zijlstra]

On Thu, Apr 24, 2025 at 11:30:39AM +0200, Benjamin Berg wrote:

> Not sure if something like that would be desirable upstream. This is
> happening for us when running the hostap "hwsim" tests inside UML (with
> time-travel). At least internally, we could carry a custom patch to add
> the lockdep_off()/lockdep_on() to dump_trace in order to work around
> it[1].

Urgh, so lockdep_off() usage is a really bad sign.

Having just done a git-grep, I see there's crap to clean out :-(

Some day I'll manage to remove that thing.

[PATCH v3 16/28] ARM: module: Use RCU in all users of __module_text_address().

[Sebastian Andrzej Siewior]

__module_text_address() can be invoked within a RCU section, there is no
requirement to have preemption disabled.

Replace the preempt_disable() section around __module_text_address()
with RCU.

Cc: Russell King <linux@armlinux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/arm/kernel/module-plts.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/arm/kernel/module-plts.c b/arch/arm/kernel/module-plts.c
index da2ee8d6ef1a7..354ce16d83cb5 100644
--- a/arch/arm/kernel/module-plts.c
+++ b/arch/arm/kernel/module-plts.c
@@ -285,11 +285,9 @@ bool in_module_plt(unsigned long loc)
 	struct module *mod;
 	bool ret;
 
-	preempt_disable();
+	guard(rcu)();
 	mod = __module_text_address(loc);
 	ret = mod && (loc - (u32)mod->arch.core.plt_ent < mod->arch.core.plt_count * PLT_ENT_SIZE ||
 		      loc - (u32)mod->arch.init.plt_ent < mod->arch.init.plt_count * PLT_ENT_SIZE);
-	preempt_enable();
-
 	return ret;
 }
-- 
2.47.1

[PATCH v3 17/28] arm64: module: Use RCU in all users of __module_text_address().

[Sebastian Andrzej Siewior]

__module_text_address() can be invoked within a RCU section, there is no
requirement to have preemption disabled.

Replace the preempt_disable() section around __module_text_address()
with RCU.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Deacon <will@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-trace-kernel@vger.kernel.org
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/arm64/kernel/ftrace.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
index 245cb419ca24d..2b76939b6304f 100644
--- a/arch/arm64/kernel/ftrace.c
+++ b/arch/arm64/kernel/ftrace.c
@@ -257,14 +257,13 @@ static bool ftrace_find_callable_addr(struct dyn_ftrace *rec,
 	 * dealing with an out-of-range condition, we can assume it
 	 * is due to a module being loaded far away from the kernel.
 	 *
-	 * NOTE: __module_text_address() must be called with preemption
-	 * disabled, but we can rely on ftrace_lock to ensure that 'mod'
+	 * NOTE: __module_text_address() must be called within a RCU read
+	 * section, but we can rely on ftrace_lock to ensure that 'mod'
 	 * retains its validity throughout the remainder of this code.
 	 */
 	if (!mod) {
-		preempt_disable();
+		guard(rcu)();
 		mod = __module_text_address(pc);
-		preempt_enable();
 	}
 
 	if (WARN_ON(!mod))
-- 
2.47.1

[PATCH v3 18/28] LoongArch/orc: Use RCU in all users of __module_address().

[Sebastian Andrzej Siewior]

__module_address() can be invoked within a RCU section, there is no
requirement to have preemption disabled.

Replace the preempt_disable() section around __module_address() with
RCU.

Cc: Huacai Chen <chenhuacai@kernel.org>
Cc: WANG Xuerui <kernel@xen0n.name>
Cc: loongarch@lists.linux.dev
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/loongarch/kernel/unwind_orc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/loongarch/kernel/unwind_orc.c b/arch/loongarch/kernel/unwind_orc.c
index b257228763317..d623935a75471 100644
--- a/arch/loongarch/kernel/unwind_orc.c
+++ b/arch/loongarch/kernel/unwind_orc.c
@@ -399,7 +399,7 @@ bool unwind_next_frame(struct unwind_state *state)
 		return false;
 
 	/* Don't let modules unload while we're reading their ORC data. */
-	preempt_disable();
+	guard(rcu)();
 
 	if (is_entry_func(state->pc))
 		goto end;
@@ -514,14 +514,12 @@ bool unwind_next_frame(struct unwind_state *state)
 	if (!__kernel_text_address(state->pc))
 		goto err;
 
-	preempt_enable();
 	return true;
 
 err:
 	state->error = true;
 
 end:
-	preempt_enable();
 	state->stack_info.type = STACK_TYPE_UNKNOWN;
 	return false;
 }
-- 
2.47.1

[PATCH v3 19/28] LoongArch: ftrace: Use RCU in all users of __module_text_address().

[Sebastian Andrzej Siewior]

__module_text_address() can be invoked within a RCU section, there is no
requirement to have preemption disabled.

Replace the preempt_disable() section around __module_text_address()
with RCU.

Cc: Huacai Chen <chenhuacai@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: WANG Xuerui <kernel@xen0n.name>
Cc: linux-trace-kernel@vger.kernel.org
Cc: loongarch@lists.linux.dev
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/loongarch/kernel/ftrace_dyn.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/loongarch/kernel/ftrace_dyn.c b/arch/loongarch/kernel/ftrace_dyn.c
index 18056229e22e4..5b7b8ac13e350 100644
--- a/arch/loongarch/kernel/ftrace_dyn.c
+++ b/arch/loongarch/kernel/ftrace_dyn.c
@@ -85,14 +85,13 @@ static bool ftrace_find_callable_addr(struct dyn_ftrace *rec, struct module *mod
 	 * dealing with an out-of-range condition, we can assume it
 	 * is due to a module being loaded far away from the kernel.
 	 *
-	 * NOTE: __module_text_address() must be called with preemption
-	 * disabled, but we can rely on ftrace_lock to ensure that 'mod'
+	 * NOTE: __module_text_address() must be called within a RCU read
+	 * section, but we can rely on ftrace_lock to ensure that 'mod'
 	 * retains its validity throughout the remainder of this code.
 	 */
 	if (!mod) {
-		preempt_disable();
-		mod = __module_text_address(pc);
-		preempt_enable();
+		scoped_guard(rcu)
+			mod = __module_text_address(pc);
 	}
 
 	if (WARN_ON(!mod))
-- 
2.47.1

[PATCH v3 20/28] powerpc/ftrace: Use RCU in all users of __module_text_address().

[Sebastian Andrzej Siewior]

__module_text_address() can be invoked within a RCU section, there is no
requirement to have preemption disabled.

Replace the preempt_disable() section around __module_text_address()
with RCU.

Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Naveen N Rao <naveen@kernel.org>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-trace-kernel@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/powerpc/kernel/trace/ftrace.c       | 6 ++----
 arch/powerpc/kernel/trace/ftrace_64_pg.c | 6 ++----
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 5ccd791761e8f..558d7f4e4bea6 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -115,10 +115,8 @@ static unsigned long ftrace_lookup_module_stub(unsigned long ip, unsigned long a
 {
 	struct module *mod = NULL;
 
-	preempt_disable();
-	mod = __module_text_address(ip);
-	preempt_enable();
-
+	scoped_guard(rcu)
+		mod = __module_text_address(ip);
 	if (!mod)
 		pr_err("No module loaded at addr=%lx\n", ip);
 
diff --git a/arch/powerpc/kernel/trace/ftrace_64_pg.c b/arch/powerpc/kernel/trace/ftrace_64_pg.c
index 98787376eb87c..531d40f10c8a1 100644
--- a/arch/powerpc/kernel/trace/ftrace_64_pg.c
+++ b/arch/powerpc/kernel/trace/ftrace_64_pg.c
@@ -120,10 +120,8 @@ static struct module *ftrace_lookup_module(struct dyn_ftrace *rec)
 {
 	struct module *mod;
 
-	preempt_disable();
-	mod = __module_text_address(rec->ip);
-	preempt_enable();
-
+	scoped_guard(rcu)
+		mod = __module_text_address(rec->ip);
 	if (!mod)
 		pr_err("No module loaded at addr=%lx\n", rec->ip);
 
-- 
2.47.1

Re: [PATCH v3 20/28] powerpc/ftrace: Use RCU in all users of __module_text_address().

[Shrikanth Hegde]

On 1/8/25 14:34, Sebastian Andrzej Siewior wrote:
> __module_text_address() can be invoked within a RCU section, there is no
> requirement to have preemption disabled.
> 
> Replace the preempt_disable() section around __module_text_address()
> with RCU.
> 
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Naveen N Rao <naveen@kernel.org>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: linux-trace-kernel@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>   arch/powerpc/kernel/trace/ftrace.c       | 6 ++----
>   arch/powerpc/kernel/trace/ftrace_64_pg.c | 6 ++----
>   2 files changed, 4 insertions(+), 8 deletions(-)
> 

Ran ftrace (function graph) on preempt=full kernel with rcutorture while doing modprobe/rmmod.
rcutorture succeeded and didn't see any splats.
If there is any other method to test it out, please let me know.

So for powerpc bits:
Tested-by: Shrikanth Hegde <sshegde@linux.ibm.com>

> diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
> index 5ccd791761e8f..558d7f4e4bea6 100644
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -115,10 +115,8 @@ static unsigned long ftrace_lookup_module_stub(unsigned long ip, unsigned long a
>   {
>   	struct module *mod = NULL;
>   
> -	preempt_disable();
> -	mod = __module_text_address(ip);
> -	preempt_enable();
> -
> +	scoped_guard(rcu)
> +		mod = __module_text_address(ip);
>   	if (!mod)
>   		pr_err("No module loaded at addr=%lx\n", ip);
>   
> diff --git a/arch/powerpc/kernel/trace/ftrace_64_pg.c b/arch/powerpc/kernel/trace/ftrace_64_pg.c
> index 98787376eb87c..531d40f10c8a1 100644
> --- a/arch/powerpc/kernel/trace/ftrace_64_pg.c
> +++ b/arch/powerpc/kernel/trace/ftrace_64_pg.c
> @@ -120,10 +120,8 @@ static struct module *ftrace_lookup_module(struct dyn_ftrace *rec)
>   {
>   	struct module *mod;
>   
> -	preempt_disable();
> -	mod = __module_text_address(rec->ip);
> -	preempt_enable();
> -
> +	scoped_guard(rcu)
> +		mod = __module_text_address(rec->ip);
>   	if (!mod)
>   		pr_err("No module loaded at addr=%lx\n", rec->ip);
>   

[PATCH v3 21/28] cfi: Use RCU while invoking __module_address().

[Sebastian Andrzej Siewior]

__module_address() can be invoked within a RCU section, there is no
requirement to have preemption disabled.

The _notrace() variant was introduced in commit 14c4c8e41511a ("cfi: Use
rcu_read_{un}lock_sched_notrace"). The recursive case where
__cfi_slowpath_diag() could end up calling itself is no longer present,
as all that logic is gone since commit 89245600941e ("cfi: Switch to
-fsanitize=kcfi").
Sami Tolvanen said that KCFI checks don't perform function calls.

Elliot Berman verified it with
| modprobe -a dummy_stm stm_ftrace stm_p_basic
| mkdir -p /sys/kernel/config/stp-policy/dummy_stm.0.my-policy/default
| echo function > /sys/kernel/tracing/current_tracer
| echo 1 > /sys/kernel/tracing/tracing_on
| echo dummy_stm.0 > /sys/class/stm_source/ftrace/stm_source_link

Replace the rcu_read_lock_sched_notrace() section around
__module_address() with RCU.

Cc: Elliot Berman <quic_eberman@quicinc.com>
Cc: Kees Cook <kees@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: llvm@lists.linux.dev
Tested-by: Elliot Berman <elliot.berman@oss.qualcomm.com>  # sm8650-qrd
Link: https://lore.kernel.org/all/20241230185812429-0800.eberman@hu-eberman-lv.qualcomm.com
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/cfi.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/kernel/cfi.c b/kernel/cfi.c
index 08caad7767176..abcd4d1f98eab 100644
--- a/kernel/cfi.c
+++ b/kernel/cfi.c
@@ -71,14 +71,11 @@ static bool is_module_cfi_trap(unsigned long addr)
 	struct module *mod;
 	bool found = false;
 
-	rcu_read_lock_sched_notrace();
-
+	guard(rcu)();
 	mod = __module_address(addr);
 	if (mod)
 		found = is_trap(addr, mod->kcfi_traps, mod->kcfi_traps_end);
 
-	rcu_read_unlock_sched_notrace();
-
 	return found;
 }
 #else /* CONFIG_MODULES */
-- 
2.47.1

[PATCH v3 22/28] x86: Use RCU in all users of __module_address().

[Sebastian Andrzej Siewior]

__module_address() can be invoked within a RCU section, there is no
requirement to have preemption disabled.

Replace the preempt_disable() section around __module_address() with
RCU.

Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Josh Poimboeuf <jpoimboe@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: x86@kernel.org
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/x86/kernel/callthunks.c | 3 +--
 arch/x86/kernel/unwind_orc.c | 4 +---
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c
index f17d166078823..276b5368ff6b0 100644
--- a/arch/x86/kernel/callthunks.c
+++ b/arch/x86/kernel/callthunks.c
@@ -98,11 +98,10 @@ static inline bool within_module_coretext(void *addr)
 #ifdef CONFIG_MODULES
 	struct module *mod;
 
-	preempt_disable();
+	guard(rcu)();
 	mod = __module_address((unsigned long)addr);
 	if (mod && within_module_core((unsigned long)addr, mod))
 		ret = true;
-	preempt_enable();
 #endif
 	return ret;
 }
diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index d4705a348a804..977ee75e047c8 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -476,7 +476,7 @@ bool unwind_next_frame(struct unwind_state *state)
 		return false;
 
 	/* Don't let modules unload while we're reading their ORC data. */
-	preempt_disable();
+	guard(rcu)();
 
 	/* End-of-stack check for user tasks: */
 	if (state->regs && user_mode(state->regs))
@@ -669,14 +669,12 @@ bool unwind_next_frame(struct unwind_state *state)
 		goto err;
 	}
 
-	preempt_enable();
 	return true;
 
 err:
 	state->error = true;
 
 the_end:
-	preempt_enable();
 	state->stack_info.type = STACK_TYPE_UNKNOWN;
 	return false;
 }
-- 
2.47.1

Re: [PATCH v3 22/28] x86: Use RCU in all users of __module_address().

[Michal Pecio]

> x86: Use RCU in all users of __module_address().
>
> __module_address() can be invoked within a RCU section, there is no
> requirement to have preemption disabled.
> 
> Replace the preempt_disable() section around __module_address() with
> RCU.
> 
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Josh Poimboeuf <jpoimboe@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: x86@kernel.org
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  arch/x86/kernel/callthunks.c | 3 +--
>  arch/x86/kernel/unwind_orc.c | 4 +---
>  2 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c
> index f17d166078823..276b5368ff6b0 100644
> --- a/arch/x86/kernel/callthunks.c
> +++ b/arch/x86/kernel/callthunks.c
> @@ -98,11 +98,10 @@ static inline bool within_module_coretext(void *addr)
>  #ifdef CONFIG_MODULES
>  	struct module *mod;
>  
> -	preempt_disable();
> +	guard(rcu)();
>  	mod = __module_address((unsigned long)addr);
>  	if (mod && within_module_core((unsigned long)addr, mod))
>  		ret = true;
> -	preempt_enable();
>  #endif
>  	return ret;
>  }
> diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
> index d4705a348a804..977ee75e047c8 100644
> --- a/arch/x86/kernel/unwind_orc.c
> +++ b/arch/x86/kernel/unwind_orc.c
> @@ -476,7 +476,7 @@ bool unwind_next_frame(struct unwind_state *state)
>  		return false;
>  
>  	/* Don't let modules unload while we're reading their ORC data. */
> -	preempt_disable();
> +	guard(rcu)();
>  
>  	/* End-of-stack check for user tasks: */
>  	if (state->regs && user_mode(state->regs))
> @@ -669,14 +669,12 @@ bool unwind_next_frame(struct unwind_state *state)
>  		goto err;
>  	}
>  
> -	preempt_enable();
>  	return true;

Hi,

There is a regression report on a distribution forum which involves
an out of tree module on a patched kernel (yes, I know) calling
stack_trace_save() in task context, which arrives here and apparently
calls the various deref_stack_xxx() functions with preemption enabled,
which in turn call stack_access_ok() leading to a BUG:

Nov 02 21:44:30 ArchBasement kernel: BUG: using smp_processor_id() in preemptible [00000000] code: Xorg/1183
Nov 02 21:44:30 ArchBasement kernel: caller is in_entry_stack+0x11/0x60
Nov 02 21:44:30 ArchBasement kernel: CPU: 0 UID: 1000 PID: 1183 Comm: Xorg Tainted: P           OE       6.16.12-hardened1-1-hardened #1 PREEMPT(full)  6edb90a7a07fab33bbee72d6d5ef53ba6eec3b9c
Nov 02 21:44:30 ArchBasement kernel: Tainted: [P]=PROPRIETARY_MODULE, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
Nov 02 21:44:30 ArchBasement kernel: Hardware name: ASUS All Series/Z97-E, BIOS 0803 02/23/2016
Nov 02 21:44:30 ArchBasement kernel: Call Trace:
Nov 02 21:44:30 ArchBasement kernel:  <TASK>
Nov 02 21:44:30 ArchBasement kernel:  dump_stack_lvl+0x5d/0x80
Nov 02 21:44:30 ArchBasement kernel:  check_preemption_disabled+0xe5/0xf0
Nov 02 21:44:30 ArchBasement kernel:  in_entry_stack+0x11/0x60
Nov 02 21:44:30 ArchBasement kernel:  get_stack_info+0x2c/0x80
Nov 02 21:44:30 ArchBasement kernel:  stack_access_ok+0x51/0xa0
Nov 02 21:44:30 ArchBasement kernel:  unwind_next_frame+0x1cb/0x7b0
Nov 02 21:44:30 ArchBasement kernel:  ? _nv003168kms+0x42/0x50 [nvidia_modeset 90775ea8a26c5e58b97ef4b3f46eb45efa040eb2]
Nov 02 21:44:30 ArchBasement kernel:  ? __pfx_stack_trace_consume_entry+0x10/0x10
Nov 02 21:44:30 ArchBasement kernel:  arch_stack_walk+0xa6/0x110
Nov 02 21:44:30 ArchBasement kernel:  ? _nv003168kms+0x42/0x50 [nvidia_modeset 90775ea8a26c5e58b97ef4b3f46eb45efa040eb2]
Nov 02 21:44:30 ArchBasement kernel:  stack_trace_save+0x4d/0x70

Is this nvidia doing something wrong, or a problem with this commit?

The removed code suggests that preemption is allowed here, and as far
as I see, this call trace is still possible on vanilla 6.18. Perhaps
preempt_disable() needs to be restored around this code?

Regards,
Michal

Re: [PATCH v3 22/28] x86: Use RCU in all users of __module_address().

[Sebastian Andrzej Siewior]

On 2025-11-03 11:08:35 [+0100], Michal Pecio wrote:
> Hi,
Hi,

> There is a regression report on a distribution forum which involves
> an out of tree module on a patched kernel (yes, I know) calling
> stack_trace_save() in task context, which arrives here and apparently
> calls the various deref_stack_xxx() functions with preemption enabled,
> which in turn call stack_access_ok() leading to a BUG:
> 
> Nov 02 21:44:30 ArchBasement kernel: BUG: using smp_processor_id() in preemptible [00000000] code: Xorg/1183
> Nov 02 21:44:30 ArchBasement kernel: caller is in_entry_stack+0x11/0x60
> Nov 02 21:44:30 ArchBasement kernel: CPU: 0 UID: 1000 PID: 1183 Comm: Xorg Tainted: P           OE       6.16.12-hardened1-1-hardened #1 PREEMPT(full)  6edb90a7a07fab33bbee72d6d5ef53ba6eec3b9c
> Nov 02 21:44:30 ArchBasement kernel: Tainted: [P]=PROPRIETARY_MODULE, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
> Nov 02 21:44:30 ArchBasement kernel: Hardware name: ASUS All Series/Z97-E, BIOS 0803 02/23/2016
> Nov 02 21:44:30 ArchBasement kernel: Call Trace:
> Nov 02 21:44:30 ArchBasement kernel:  <TASK>
> Nov 02 21:44:30 ArchBasement kernel:  dump_stack_lvl+0x5d/0x80
> Nov 02 21:44:30 ArchBasement kernel:  check_preemption_disabled+0xe5/0xf0
> Nov 02 21:44:30 ArchBasement kernel:  in_entry_stack+0x11/0x60
> Nov 02 21:44:30 ArchBasement kernel:  get_stack_info+0x2c/0x80
> Nov 02 21:44:30 ArchBasement kernel:  stack_access_ok+0x51/0xa0
> Nov 02 21:44:30 ArchBasement kernel:  unwind_next_frame+0x1cb/0x7b0
> Nov 02 21:44:30 ArchBasement kernel:  ? _nv003168kms+0x42/0x50 [nvidia_modeset 90775ea8a26c5e58b97ef4b3f46eb45efa040eb2]
> Nov 02 21:44:30 ArchBasement kernel:  ? __pfx_stack_trace_consume_entry+0x10/0x10
> Nov 02 21:44:30 ArchBasement kernel:  arch_stack_walk+0xa6/0x110
> Nov 02 21:44:30 ArchBasement kernel:  ? _nv003168kms+0x42/0x50 [nvidia_modeset 90775ea8a26c5e58b97ef4b3f46eb45efa040eb2]
> Nov 02 21:44:30 ArchBasement kernel:  stack_trace_save+0x4d/0x70
> 
> Is this nvidia doing something wrong, or a problem with this commit?
> 
> The removed code suggests that preemption is allowed here, and as far
> as I see, this call trace is still possible on vanilla 6.18. Perhaps
> preempt_disable() needs to be restored around this code?

Do you have the complete backtrace? Is this SMP or UP build?

> Regards,
> Michal

Sebastian

Re: [PATCH v3 22/28] x86: Use RCU in all users of __module_address().

[Michal Pecio]

On Mon, 3 Nov 2025 11:34:34 +0100, Sebastian Andrzej Siewior wrote:
> On 2025-11-03 11:08:35 [+0100], Michal Pecio wrote:
> > Hi,  
> Hi,
> 
> > There is a regression report on a distribution forum which involves
> > an out of tree module on a patched kernel (yes, I know) calling
> > stack_trace_save() in task context, which arrives here and apparently
> > calls the various deref_stack_xxx() functions with preemption enabled,
> > which in turn call stack_access_ok() leading to a BUG:
> > 
> > Nov 02 21:44:30 ArchBasement kernel: BUG: using smp_processor_id() in preemptible [00000000] code: Xorg/1183
> > Nov 02 21:44:30 ArchBasement kernel: caller is in_entry_stack+0x11/0x60
> > Nov 02 21:44:30 ArchBasement kernel: CPU: 0 UID: 1000 PID: 1183 Comm: Xorg Tainted: P           OE       6.16.12-hardened1-1-hardened #1 PREEMPT(full)  6edb90a7a07fab33bbee72d6d5ef53ba6eec3b9c
> > Nov 02 21:44:30 ArchBasement kernel: Tainted: [P]=PROPRIETARY_MODULE, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
> > Nov 02 21:44:30 ArchBasement kernel: Hardware name: ASUS All Series/Z97-E, BIOS 0803 02/23/2016
> > Nov 02 21:44:30 ArchBasement kernel: Call Trace:
> > Nov 02 21:44:30 ArchBasement kernel:  <TASK>
> > Nov 02 21:44:30 ArchBasement kernel:  dump_stack_lvl+0x5d/0x80
> > Nov 02 21:44:30 ArchBasement kernel:  check_preemption_disabled+0xe5/0xf0
> > Nov 02 21:44:30 ArchBasement kernel:  in_entry_stack+0x11/0x60
> > Nov 02 21:44:30 ArchBasement kernel:  get_stack_info+0x2c/0x80
> > Nov 02 21:44:30 ArchBasement kernel:  stack_access_ok+0x51/0xa0
> > Nov 02 21:44:30 ArchBasement kernel:  unwind_next_frame+0x1cb/0x7b0
> > Nov 02 21:44:30 ArchBasement kernel:  ? _nv003168kms+0x42/0x50 [nvidia_modeset 90775ea8a26c5e58b97ef4b3f46eb45efa040eb2]
> > Nov 02 21:44:30 ArchBasement kernel:  ? __pfx_stack_trace_consume_entry+0x10/0x10
> > Nov 02 21:44:30 ArchBasement kernel:  arch_stack_walk+0xa6/0x110
> > Nov 02 21:44:30 ArchBasement kernel:  ? _nv003168kms+0x42/0x50 [nvidia_modeset 90775ea8a26c5e58b97ef4b3f46eb45efa040eb2]
> > Nov 02 21:44:30 ArchBasement kernel:  stack_trace_save+0x4d/0x70
> > 
> > Is this nvidia doing something wrong, or a problem with this commit?
> > 
> > The removed code suggests that preemption is allowed here, and as far
> > as I see, this call trace is still possible on vanilla 6.18. Perhaps
> > preempt_disable() needs to be restored around this code?  
> 
> Do you have the complete backtrace? Is this SMP or UP build?

Sorry, I forgot to include the link. There is also a similar warning
regarding __this_cpu_read(). Pretty sure the kernel is SMP.

https://bbs.archlinux.org/viewtopic.php?id=309960

Re: [PATCH v3 22/28] x86: Use RCU in all users of __module_address().

[Sebastian Andrzej Siewior]

On 2025-11-03 11:39:07 [+0100], Michal Pecio wrote:
> Sorry, I forgot to include the link. There is also a similar warning
> regarding __this_cpu_read(). Pretty sure the kernel is SMP.
> 
> https://bbs.archlinux.org/viewtopic.php?id=309960

The stack trace is a bit odd. The compressed version is:
| BUG: using smp_processor_id() in preemptible [00000000] code: Xorg/1183
| caller is in_entry_stack+0x11/0x60
| CPU: 3 UID: 1000 PID: 1183 Comm: Xorg Tainted: P           OE       6.16.12-hardened1-1-hardened #1 PREEMPT(full)  6edb90a7a07fab33bbee72d6d5ef53ba6eec3b9c
| Tainted: [P]=PROPRIETARY_MODULE, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
| Hardware name: ASUS All Series/Z97-E, BIOS 0803 02/23/2016
| Call Trace:
|  <TASK>
|  dump_stack_lvl+0x5d/0x80
|  check_preemption_disabled+0xe5/0xf0
|  in_entry_stack+0x11/0x60
|  get_stack_info+0x2c/0x80
|  stack_access_ok+0x51/0xa0
|  unwind_next_frame+0x1cb/0x7b0
|  arch_stack_walk+0xa6/0x110
|  stack_trace_save+0x4d/0x70
|  __kfence_alloc+0xb7/0x6f0
|  __kmalloc_noprof+0x520/0x560
|  os_alloc_mem+0x108/0x120 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
|  _nv015295rm+0x34/0x50 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
|  _nv015297rm+0x2b/0xd0 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
|  _nv016352rm+0x1c/0x90 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
|  _nv059298rm+0x65/0xb0 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
|  _nv054041rm+0x20f/0x360 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
|  _nv056165rm+0x54/0xd0 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
|  _nv056096rm+0xa0/0x500 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
|  _nv015919rm+0x424/0x680 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
|  _nv054015rm+0x69/0xd0 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
|  _nv014185rm+0x86/0xa0 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
|  _nv000652rm+0x5e/0x70 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
|  rm_kernel_rmapi_op+0x167/0x273 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
|  nvkms_call_rm+0x4c/0x80 [nvidia_modeset 90775ea8a26c5e58b97ef4b3f46eb45efa040eb2]
|  _nv003168kms+0x42/0x50 [nvidia_modeset 90775ea8a26c5e58b97ef4b3f46eb45efa040eb2]
|  ? do_syscall_64+0x82/0x8d0
|  ? entry_SYSCALL_64_after_hwframe+0x76/0x7e
|  </TASK>

The last two entries start with a '?' which means it did not originate
from the "stack unwind" but was laying around while passing through.
I would expect the last two entries to be there without the '?' because
userland (as in X here) enters the kernel via a proper syscall entry
which should be part of the stack strace.

Now, get_stack_info() where the warning originates: It starts with a
check to see if the stack pointer belongs to the current task's stack
frame which it does not. Then it checks if the task found is the
currently running task. That it does. So in that case, we must be
serving an exception (such as an IRQ) because the stack does not belong
to the current task.  However preemption is not disabled which indicates
that we do not do this.
This in turn suggests that nvidia replaced the stack from while entering
the syscall probably in _nv003168kms() or the binary blob which invokes
the kernel function does not have a proper ORC entry which leads to a
wrong turn in the process.

So the warning is well deserved.

Sebastian

Re: [PATCH v3 22/28] x86: Use RCU in all users of __module_address().

[Michal Pecio]

On Mon, 3 Nov 2025 12:37:50 +0100, Sebastian Andrzej Siewior wrote:
> Now, get_stack_info() where the warning originates: It starts with a
> check to see if the stack pointer belongs to the current task's stack
> frame which it does not. Then it checks if the task found is the
> currently running task. That it does. So in that case, we must be
> serving an exception (such as an IRQ) because the stack does not
> belong to the current task.  However preemption is not disabled which
> indicates that we do not do this.
> This in turn suggests that nvidia replaced the stack from while
> entering the syscall probably in _nv003168kms() or the binary blob
> which invokes the kernel function does not have a proper ORC entry
> which leads to a wrong turn in the process.

OK, I see, preemption should only be enabled in the first case, so
others are free to assume it's disabled. No bug.

Thank you.

[PATCH v3 23/28] jump_label: Use RCU in all users of __module_address().

[Sebastian Andrzej Siewior]

__module_address() can be invoked within a RCU section, there is no
requirement to have preemption disabled.

Replace the preempt_disable() section around __module_address() with RCU.

Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Jason Baron <jbaron@akamai.com>
Cc: Josh Poimboeuf <jpoimboe@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/jump_label.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 93a822d3c468c..7fcf4017cb383 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -746,9 +746,9 @@ static int jump_label_add_module(struct module *mod)
 				kfree(jlm);
 				return -ENOMEM;
 			}
-			preempt_disable();
-			jlm2->mod = __module_address((unsigned long)key);
-			preempt_enable();
+			scoped_guard(rcu)
+				jlm2->mod = __module_address((unsigned long)key);
+
 			jlm2->entries = static_key_entries(key);
 			jlm2->next = NULL;
 			static_key_set_mod(key, jlm2);
@@ -906,13 +906,13 @@ static void jump_label_update(struct static_key *key)
 		return;
 	}
 
-	preempt_disable();
-	mod = __module_address((unsigned long)key);
-	if (mod) {
-		stop = mod->jump_entries + mod->num_jump_entries;
-		init = mod->state == MODULE_STATE_COMING;
+	scoped_guard(rcu) {
+		mod = __module_address((unsigned long)key);
+		if (mod) {
+			stop = mod->jump_entries + mod->num_jump_entries;
+			init = mod->state == MODULE_STATE_COMING;
+		}
 	}
-	preempt_enable();
 #endif
 	entry = static_key_entries(key);
 	/* if there are no users, entry can be NULL */
-- 
2.47.1

[PATCH v3 24/28] jump_label: Use RCU in all users of __module_text_address().

[Sebastian Andrzej Siewior]

__module_text_address() can be invoked within a RCU section, there is no
requirement to have preemption disabled.

Replace the preempt_disable() section around __module_text_address()
with RCU.

Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Jason Baron <jbaron@akamai.com>
Cc: Josh Poimboeuf <jpoimboe@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/jump_label.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 7fcf4017cb383..7cb19e6014266 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -653,13 +653,12 @@ static int __jump_label_mod_text_reserved(void *start, void *end)
 	struct module *mod;
 	int ret;
 
-	preempt_disable();
-	mod = __module_text_address((unsigned long)start);
-	WARN_ON_ONCE(__module_text_address((unsigned long)end) != mod);
-	if (!try_module_get(mod))
-		mod = NULL;
-	preempt_enable();
-
+	scoped_guard(rcu) {
+		mod = __module_text_address((unsigned long)start);
+		WARN_ON_ONCE(__module_text_address((unsigned long)end) != mod);
+		if (!try_module_get(mod))
+			mod = NULL;
+	}
 	if (!mod)
 		return 0;
 
-- 
2.47.1

[PATCH v3 25/28] bpf: Use RCU in all users of __module_text_address().

[Sebastian Andrzej Siewior]

__module_address() can be invoked within a RCU section, there is no
requirement to have preemption disabled.

Replace the preempt_disable() section around __module_address() with
RCU.

Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Eduard Zingerman <eddyz87@gmail.com>
Cc: Hao Luo <haoluo@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: KP Singh <kpsingh@kernel.org>
Cc: Martin KaFai Lau <martin.lau@linux.dev>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Matt Bobrowski <mattbobrowski@google.com>
Cc: Song Liu <song@kernel.org>
Cc: Stanislav Fomichev <sdf@fomichev.me>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Yonghong Song <yonghong.song@linux.dev>
Cc: bpf@vger.kernel.org
Cc: linux-trace-kernel@vger.kernel.org
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/trace/bpf_trace.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 1b8db5aee9d38..020df7b6ff90c 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2336,10 +2336,9 @@ void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp)
 {
 	struct module *mod;
 
-	preempt_disable();
+	guard(rcu)();
 	mod = __module_address((unsigned long)btp);
 	module_put(mod);
-	preempt_enable();
 }
 
 static __always_inline
@@ -2907,16 +2906,14 @@ static int get_modules_for_addrs(struct module ***mods, unsigned long *addrs, u3
 	for (i = 0; i < addrs_cnt; i++) {
 		struct module *mod;
 
-		preempt_disable();
-		mod = __module_address(addrs[i]);
-		/* Either no module or we it's already stored  */
-		if (!mod || has_module(&arr, mod)) {
-			preempt_enable();
-			continue;
+		scoped_guard(rcu) {
+			mod = __module_address(addrs[i]);
+			/* Either no module or we it's already stored  */
+			if (!mod || has_module(&arr, mod))
+				continue;
+			if (!try_module_get(mod))
+				err = -EINVAL;
 		}
-		if (!try_module_get(mod))
-			err = -EINVAL;
-		preempt_enable();
 		if (err)
 			break;
 		err = add_module(&arr, mod);
-- 
2.47.1

Re: [PATCH v3 25/28] bpf: Use RCU in all users of __module_text_address().

[Alexei Starovoitov]

On Wed, Jan 8, 2025 at 1:05 AM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> __module_address() can be invoked within a RCU section, there is no
> requirement to have preemption disabled.
>
> Replace the preempt_disable() section around __module_address() with
> RCU.
>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Andrii Nakryiko <andrii@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Eduard Zingerman <eddyz87@gmail.com>
> Cc: Hao Luo <haoluo@google.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: John Fastabend <john.fastabend@gmail.com>
> Cc: KP Singh <kpsingh@kernel.org>
> Cc: Martin KaFai Lau <martin.lau@linux.dev>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Matt Bobrowski <mattbobrowski@google.com>
> Cc: Song Liu <song@kernel.org>
> Cc: Stanislav Fomichev <sdf@fomichev.me>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Yonghong Song <yonghong.song@linux.dev>
> Cc: bpf@vger.kernel.org
> Cc: linux-trace-kernel@vger.kernel.org
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  kernel/trace/bpf_trace.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 1b8db5aee9d38..020df7b6ff90c 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2336,10 +2336,9 @@ void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp)
>  {
>         struct module *mod;
>
> -       preempt_disable();
> +       guard(rcu)();
>         mod = __module_address((unsigned long)btp);
>         module_put(mod);
> -       preempt_enable();
>  }
>
>  static __always_inline
> @@ -2907,16 +2906,14 @@ static int get_modules_for_addrs(struct module ***mods, unsigned long *addrs, u3
>         for (i = 0; i < addrs_cnt; i++) {
>                 struct module *mod;
>
> -               preempt_disable();
> -               mod = __module_address(addrs[i]);
> -               /* Either no module or we it's already stored  */
> -               if (!mod || has_module(&arr, mod)) {
> -                       preempt_enable();
> -                       continue;
> +               scoped_guard(rcu) {
> +                       mod = __module_address(addrs[i]);
> +                       /* Either no module or we it's already stored  */
> +                       if (!mod || has_module(&arr, mod))
> +                               continue;
> +                       if (!try_module_get(mod))
> +                               err = -EINVAL;

lgtm.
Should we take into bpf-next or the whole set is handled together
somewhere?

Re: [PATCH v3 25/28] bpf: Use RCU in all users of __module_text_address().

[Sebastian Andrzej Siewior]

On 2025-01-09 10:38:03 [-0800], Alexei Starovoitov wrote:
> lgtm.
> Should we take into bpf-next or the whole set is handled together
> somewhere?

If you don't mind, I would hope to route the whole series via the
modules tree. Some of the lower functions (__module_address()) check for
disabled preemption and will trigger warnings at runtime if this gets
applied before (earlier in the series) the check gets replaced.

Sebastian

Re: [PATCH v3 25/28] bpf: Use RCU in all users of __module_text_address().

[Alexei Starovoitov]

On Thu, Jan 9, 2025 at 12:54 PM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> On 2025-01-09 10:38:03 [-0800], Alexei Starovoitov wrote:
> > lgtm.
> > Should we take into bpf-next or the whole set is handled together
> > somewhere?
>
> If you don't mind, I would hope to route the whole series via the
> modules tree. Some of the lower functions (__module_address()) check for
> disabled preemption and will trigger warnings at runtime if this gets
> applied before (earlier in the series) the check gets replaced.

I see. Then

Acked-by: Alexei Starovoitov <ast@kernel.org>

[PATCH v3.5 25/28] bpf: Use RCU in all users of __module_text_address().

[Sebastian Andrzej Siewior]

__module_address() can be invoked within a RCU section, there is no
requirement to have preemption disabled.

Replace the preempt_disable() section around __module_address() with
RCU.

Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Eduard Zingerman <eddyz87@gmail.com>
Cc: Hao Luo <haoluo@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: KP Singh <kpsingh@kernel.org>
Cc: Martin KaFai Lau <martin.lau@linux.dev>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Matt Bobrowski <mattbobrowski@google.com>
Cc: Song Liu <song@kernel.org>
Cc: Stanislav Fomichev <sdf@fomichev.me>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Yonghong Song <yonghong.song@linux.dev>
Cc: bpf@vger.kernel.org
Cc: linux-trace-kernel@vger.kernel.org
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---

The previous version was broken in terms that the break statement broke
out of the scoped_guard loop and added something to the list. This is
now fixed by adding the "skip_add" bool.
While at it, I updated the comment by removing the "we".

 kernel/trace/bpf_trace.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index adc947587eb81..e6a17a60d8787 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2345,10 +2345,9 @@ void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp)
 {
 	struct module *mod;
 
-	preempt_disable();
+	guard(rcu)();
 	mod = __module_address((unsigned long)btp);
 	module_put(mod);
-	preempt_enable();
 }
 
 static __always_inline
@@ -2932,18 +2931,21 @@ static int get_modules_for_addrs(struct module ***mods, unsigned long *addrs, u3
 	u32 i, err = 0;
 
 	for (i = 0; i < addrs_cnt; i++) {
+		bool skip_add = false;
 		struct module *mod;
 
-		preempt_disable();
-		mod = __module_address(addrs[i]);
-		/* Either no module or we it's already stored  */
-		if (!mod || has_module(&arr, mod)) {
-			preempt_enable();
-			continue;
+		scoped_guard(rcu) {
+			mod = __module_address(addrs[i]);
+			/* Either no module or it's already stored  */
+			if (!mod || has_module(&arr, mod)) {
+				skip_add = true;
+				break; /* scoped_guard */
+			}
+			if (!try_module_get(mod))
+				err = -EINVAL;
 		}
-		if (!try_module_get(mod))
-			err = -EINVAL;
-		preempt_enable();
+		if (skip_add)
+			continue;
 		if (err)
 			break;
 		err = add_module(&arr, mod);
-- 
2.47.2

[PATCH v3 26/28] kprobes: Use RCU in all users of __module_text_address().

[Sebastian Andrzej Siewior]

__module_text_address() can be invoked within a RCU section, there is no
requirement to have preemption disabled.

Replace the preempt_disable() section around __module_text_address()
with RCU.

Cc: David S. Miller <davem@davemloft.net>
Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Naveen N Rao <naveen@kernel.org>
Cc: linux-trace-kernel@vger.kernel.org
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/kprobes.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index b027a4030976a..22e47a27df4aa 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1566,7 +1566,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
 	if (ret)
 		return ret;
 	jump_label_lock();
-	preempt_disable();
+	rcu_read_lock();
 
 	/* Ensure the address is in a text area, and find a module if exists. */
 	*probed_mod = NULL;
@@ -1612,7 +1612,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
 	}
 
 out:
-	preempt_enable();
+	rcu_read_unlock();
 	jump_label_unlock();
 
 	return ret;
-- 
2.47.1

Re: [PATCH v3 26/28] kprobes: Use RCU in all users of __module_text_address().

[Masami Hiramatsu]

On Wed,  8 Jan 2025 10:04:55 +0100
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> __module_text_address() can be invoked within a RCU section, there is no
> requirement to have preemption disabled.
> 
> Replace the preempt_disable() section around __module_text_address()
> with RCU.

Hi,

Since the below cleanup has been merged, this patch needs to be updated.

https://lore.kernel.org/all/20241121-kprobes-preempt-v1-1-fd581ee7fcbb@linutronix.de/

(may need to change the guard(preempt)() -> guard(rcu)() in the new version.)

Thank you,

> 
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Naveen N Rao <naveen@kernel.org>
> Cc: linux-trace-kernel@vger.kernel.org
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  kernel/kprobes.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index b027a4030976a..22e47a27df4aa 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1566,7 +1566,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
>  	if (ret)
>  		return ret;
>  	jump_label_lock();
> -	preempt_disable();
> +	rcu_read_lock();
>  
>  	/* Ensure the address is in a text area, and find a module if exists. */
>  	*probed_mod = NULL;
> @@ -1612,7 +1612,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
>  	}
>  
>  out:
> -	preempt_enable();
> +	rcu_read_unlock();
>  	jump_label_unlock();
>  
>  	return ret;
> -- 
> 2.47.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH v3 26/28] kprobes: Use RCU in all users of __module_text_address().

[Sebastian Andrzej Siewior]

On 2025-01-28 14:44:52 [+0900], Masami Hiramatsu wrote:
> Hi,

Hi,

> Since the below cleanup has been merged, this patch needs to be updated.
> 
> https://lore.kernel.org/all/20241121-kprobes-preempt-v1-1-fd581ee7fcbb@linutronix.de/
> 
> (may need to change the guard(preempt)() -> guard(rcu)() in the new version.)

Thanks, I'm on it.

> Thank you,

Sebastian

[PATCH v3.5 25/28] kprobes: Use RCU in all users of __module_text_address().

[Sebastian Andrzej Siewior]

__module_text_address() can be invoked within a RCU section, there is no
requirement to have preemption disabled.

Replace the preempt_disable() section around __module_text_address()
with RCU.

Cc: David S. Miller <davem@davemloft.net>
Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Naveen N Rao <naveen@kernel.org>
Cc: linux-trace-kernel@vger.kernel.org
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
Update due to the collision during the merge window.

 kernel/kprobes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 0305692106709..9c8afbb957b0f 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1547,7 +1547,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
 	/* Ensure the address is in a text area, and find a module if exists. */
 	*probed_mod = NULL;
 	if (!core_kernel_text((unsigned long) p->addr)) {
-		guard(preempt)();
+		guard(rcu)();
 		*probed_mod = __module_text_address((unsigned long) p->addr);
 		if (!(*probed_mod))
 			return -EINVAL;
-- 
2.47.2

[PATCH v3 27/28] static_call: Use RCU in all users of __module_text_address().

[Sebastian Andrzej Siewior]

__module_text_address() can be invoked within a RCU section, there is no
requirement to have preemption disabled.

Replace the preempt_disable() section around __module_text_address()
with RCU.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/static_call_inline.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/kernel/static_call_inline.c b/kernel/static_call_inline.c
index bb7d066a7c397..c2c59e6ef35d0 100644
--- a/kernel/static_call_inline.c
+++ b/kernel/static_call_inline.c
@@ -325,13 +325,12 @@ static int __static_call_mod_text_reserved(void *start, void *end)
 	struct module *mod;
 	int ret;
 
-	preempt_disable();
-	mod = __module_text_address((unsigned long)start);
-	WARN_ON_ONCE(__module_text_address((unsigned long)end) != mod);
-	if (!try_module_get(mod))
-		mod = NULL;
-	preempt_enable();
-
+	scoped_guard(rcu) {
+		mod = __module_text_address((unsigned long)start);
+		WARN_ON_ONCE(__module_text_address((unsigned long)end) != mod);
+		if (!try_module_get(mod))
+			mod = NULL;
+	}
 	if (!mod)
 		return 0;
 
-- 
2.47.1

[PATCH v3 28/28] bug: Use RCU instead RCU-sched to protect module_bug_list.

[Sebastian Andrzej Siewior]

The list module_bug_list relies on module_mutex for writer
synchronisation. The list is already RCU style.
The list removal is synchronized with modules' synchronize_rcu() in
free_module().

Use RCU read lock protection instead of RCU-sched.

Cc: Andrew Morton <akpm@linux-foundation.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 lib/bug.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/lib/bug.c b/lib/bug.c
index e0ff219899902..b1f07459c2ee3 100644
--- a/lib/bug.c
+++ b/lib/bug.c
@@ -66,23 +66,19 @@ static LIST_HEAD(module_bug_list);
 
 static struct bug_entry *module_find_bug(unsigned long bugaddr)
 {
+	struct bug_entry *bug;
 	struct module *mod;
-	struct bug_entry *bug = NULL;
 
-	rcu_read_lock_sched();
+	guard(rcu)();
 	list_for_each_entry_rcu(mod, &module_bug_list, bug_list) {
 		unsigned i;
 
 		bug = mod->bug_table;
 		for (i = 0; i < mod->num_bugs; ++i, ++bug)
 			if (bugaddr == bug_addr(bug))
-				goto out;
+				return bug;
 	}
-	bug = NULL;
-out:
-	rcu_read_unlock_sched();
-
-	return bug;
+	return NULL;
 }
 
 void module_bug_finalize(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs,
@@ -235,11 +231,11 @@ void generic_bug_clear_once(void)
 #ifdef CONFIG_MODULES
 	struct module *mod;
 
-	rcu_read_lock_sched();
-	list_for_each_entry_rcu(mod, &module_bug_list, bug_list)
-		clear_once_table(mod->bug_table,
-				 mod->bug_table + mod->num_bugs);
-	rcu_read_unlock_sched();
+	scoped_guard(rcu) {
+		list_for_each_entry_rcu(mod, &module_bug_list, bug_list)
+			clear_once_table(mod->bug_table,
+					 mod->bug_table + mod->num_bugs);
+	}
 #endif
 
 	clear_once_table(__start___bug_table, __stop___bug_table);
-- 
2.47.1

Re: [PATCH v3 00/28] module: Use RCU instead of RCU-sched.

[Petr Pavlu]

On 1/8/25 10:04, Sebastian Andrzej Siewior wrote:
> This is an updated version of the initial post after PeterZ made me
> aware that there are users outside of the module directory.
> The goal is replace the mix auf rcu_read_lock(), rcu_read_lock_sched()
> and preempt_disable() with just rcu_read_lock().

Thanks for this cleanup. I've queued the fix in patch #1 on
modules-fixes. For the rest, I plan to give folks more time to look at
the changes as this affects a number of subsystems. If there are no
other concerns, I'd then add the series on modules-next.

-- 
Cheers,
Petr

Re: [PATCH v3 00/28] module: Use RCU instead of RCU-sched.

[Sebastian Andrzej Siewior]

On 2025-01-13 12:09:27 [+0100], Petr Pavlu wrote:
> Thanks for this cleanup. I've queued the fix in patch #1 on
> modules-fixes. For the rest, I plan to give folks more time to look at
> the changes as this affects a number of subsystems. If there are no
> other concerns, I'd then add the series on modules-next.
Good, thanks for the update.

Sebastian

Re: [PATCH v3 00/28] module: Use RCU instead of RCU-sched.

[Sebastian Andrzej Siewior]

On 2025-01-13 12:09:27 [+0100], Petr Pavlu wrote:
> Thanks for this cleanup. I've queued the fix in patch #1 on
> modules-fixes. For the rest, I plan to give folks more time to look at
> the changes as this affects a number of subsystems. If there are no
> other concerns, I'd then add the series on modules-next.

#26 (kprobes) clashes with the changes that have been merged upstream.
Do you want me to resend the whole series or just #26? The other patches
apply cleanly so far.

Sebastian

Re: [PATCH v3 00/28] module: Use RCU instead of RCU-sched.

[Petr Pavlu]

On 1/24/25 18:49, Sebastian Andrzej Siewior wrote:
> On 2025-01-13 12:09:27 [+0100], Petr Pavlu wrote:
>> Thanks for this cleanup. I've queued the fix in patch #1 on
>> modules-fixes. For the rest, I plan to give folks more time to look at
>> the changes as this affects a number of subsystems. If there are no
>> other concerns, I'd then add the series on modules-next.
> 
> #26 (kprobes) clashes with the changes that have been merged upstream.
> Do you want me to resend the whole series or just #26? The other patches
> apply cleanly so far.

I think sending only the updated patch #26 should be sufficient in this
case, it's only a small adjustment. Please preferably post it as a reply
to the email with that specific patch.

-- 
Thanks,
Petr

Re: [PATCH v3 00/28] module: Use RCU instead of RCU-sched.

[Sebastian Andrzej Siewior]

On 2025-01-27 13:22:17 [+0100], Petr Pavlu wrote:
> On 1/24/25 18:49, Sebastian Andrzej Siewior wrote:
> > On 2025-01-13 12:09:27 [+0100], Petr Pavlu wrote:
> >> Thanks for this cleanup. I've queued the fix in patch #1 on
> >> modules-fixes. For the rest, I plan to give folks more time to look at
> >> the changes as this affects a number of subsystems. If there are no
> >> other concerns, I'd then add the series on modules-next.
> > 
> > #26 (kprobes) clashes with the changes that have been merged upstream.
> > Do you want me to resend the whole series or just #26? The other patches
> > apply cleanly so far.
> 
> I think sending only the updated patch #26 should be sufficient in this
> case, it's only a small adjustment. Please preferably post it as a reply
> to the email with that specific patch.

I just sent two updates:

 [PATCH v3.5 25/28] bpf: Use RCU in all users of __module_text_address().
 [PATCH v3.5 26/28] kprobes: Use RCU in all users of __module_text_address().

Sebastian

Re: [PATCH v3 00/28] module: Use RCU instead of RCU-sched.

[Petr Pavlu]

On 1/29/25 09:52, Sebastian Andrzej Siewior wrote:
> On 2025-01-27 13:22:17 [+0100], Petr Pavlu wrote:
>> On 1/24/25 18:49, Sebastian Andrzej Siewior wrote:
>>> On 2025-01-13 12:09:27 [+0100], Petr Pavlu wrote:
>>>> Thanks for this cleanup. I've queued the fix in patch #1 on
>>>> modules-fixes. For the rest, I plan to give folks more time to look at
>>>> the changes as this affects a number of subsystems. If there are no
>>>> other concerns, I'd then add the series on modules-next.
>>>
>>> #26 (kprobes) clashes with the changes that have been merged upstream.
>>> Do you want me to resend the whole series or just #26? The other patches
>>> apply cleanly so far.
>>
>> I think sending only the updated patch #26 should be sufficient in this
>> case, it's only a small adjustment. Please preferably post it as a reply
>> to the email with that specific patch.
> 
> I just sent two updates:
> 
>  [PATCH v3.5 25/28] bpf: Use RCU in all users of __module_text_address().
>  [PATCH v3.5 26/28] kprobes: Use RCU in all users of __module_text_address().

I've now queued the series and its two updated patches #25 and #26 on
modules-next (for 6.15-rc1).

-- 
Thanks,
Petr