[PATCH] uprobes: Remove redundant guard(rcu) in find_active_uprobe_speculative()

[PATCH] uprobes: Remove redundant guard(rcu) in find_active_uprobe_speculative()

[Masami Hiramatsu (Google)]

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

find_active_uprobe_speculative() is called from find_active_uprobe_rcu(),
which is invoked by handle_swbp() and handle_syscall_uprobe().
Both callers already hold RCU Tasks Trace (via rcu_read_lock_trace() or
guard(rcu_tasks_trace)()).

Calling guard(rcu)() inside find_active_uprobe_speculative() is redundant
because the caller already guarantees RCU Tasks Trace protection.
Furthermore, standard RCU (rcu_read_lock()) does not protect uprobes_tree
against asynchronous uprobe destruction (call_rcu_tasks_trace()).

Remove the redundant guard(rcu)() from find_active_uprobe_speculative() and
replace it with lockdep_assert(rcu_read_lock_trace_held()) and a comment
documenting that callers must hold RCU Tasks Trace protection, matching the
convention used by find_active_uprobe_rcu() and find_uprobe_rcu().

Fixes: e0925f2dc4de ("uprobes: add speculative lockless VMA-to-inode-to-uprobe resolution")
Assisted-by: Antigravity:gemini-3.6-flash
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/events/uprobes.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 4084e926e284..b4e253d43b30 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -2424,6 +2424,7 @@ static int is_trap_at_addr(struct mm_struct *mm, unsigned long vaddr)
 	return is_trap_insn(&opcode);
 }
 
+/* assumes being inside RCU Tasks Trace protected region */
 static struct uprobe *find_active_uprobe_speculative(unsigned long bp_vaddr)
 {
 	struct mm_struct *mm = current->mm;
@@ -2433,7 +2434,7 @@ static struct uprobe *find_active_uprobe_speculative(unsigned long bp_vaddr)
 	loff_t offset;
 	unsigned int seq;
 
-	guard(rcu)();
+	lockdep_assert(rcu_read_lock_trace_held());
 
 	if (!mmap_lock_speculate_try_begin(mm, &seq))
 		return NULL;

Re: [PATCH] uprobes: Remove redundant guard(rcu) in find_active_uprobe_speculative()

[Masami Hiramatsu]

On Thu, 30 Jul 2026 10:45:39 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> find_active_uprobe_speculative() is called from find_active_uprobe_rcu(),
> which is invoked by handle_swbp() and handle_syscall_uprobe().
> Both callers already hold RCU Tasks Trace (via rcu_read_lock_trace() or
> guard(rcu_tasks_trace)()).
> 
> Calling guard(rcu)() inside find_active_uprobe_speculative() is redundant
> because the caupller already guarantees RCU Tasks Trace protection.
> Furthermore, standard RCU (rcu_read_lock()) does not protect uprobes_tree
> against asynchronous uprobe destruction (call_rcu_tasks_trace()).
> 
> Remove the redundant guard(rcu)() from find_active_uprobe_speculative() and
> replace it with lockdep_assert(rcu_read_lock_trace_held()) and a comment
> documenting that callers must hold RCU Tasks Trace protection, matching the
> convention used by find_active_uprobe_rcu() and find_uprobe_rcu().
> 
> Fixes: e0925f2dc4de ("uprobes: add speculative lockless VMA-to-inode-to-uprobe resolution")
> Assisted-by: Antigravity:gemini-3.6-flash
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Sashiko mentioned that this guard is not for uprobes_tree, but VMA lookup.
Please ignore this.

Thanks,

> ---
>  kernel/events/uprobes.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 4084e926e284..b4e253d43b30 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -2424,6 +2424,7 @@ static int is_trap_at_addr(struct mm_struct *mm, unsigned long vaddr)
>  	return is_trap_insn(&opcode);
>  }
>  
> +/* assumes being inside RCU Tasks Trace protected region */
>  static struct uprobe *find_active_uprobe_speculative(unsigned long bp_vaddr)
>  {
>  	struct mm_struct *mm = current->mm;
> @@ -2433,7 +2434,7 @@ static struct uprobe *find_active_uprobe_speculative(unsigned long bp_vaddr)
>  	loff_t offset;
>  	unsigned int seq;
>  
> -	guard(rcu)();
> +	lockdep_assert(rcu_read_lock_trace_held());
>  
>  	if (!mmap_lock_speculate_try_begin(mm, &seq))
>  		return NULL;
> 


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

Re: [PATCH] uprobes: Remove redundant guard(rcu) in find_active_uprobe_speculative()

[Peter Zijlstra]

On Thu, Jul 30, 2026 at 10:45:39AM +0900, Masami Hiramatsu (Google) wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> find_active_uprobe_speculative() is called from find_active_uprobe_rcu(),
> which is invoked by handle_swbp() and handle_syscall_uprobe().
> Both callers already hold RCU Tasks Trace (via rcu_read_lock_trace() or
> guard(rcu_tasks_trace)()).
> 
> Calling guard(rcu)() inside find_active_uprobe_speculative() is redundant
> because the caller already guarantees RCU Tasks Trace protection.
> Furthermore, standard RCU (rcu_read_lock()) does not protect uprobes_tree
> against asynchronous uprobe destruction (call_rcu_tasks_trace()).
> 
> Remove the redundant guard(rcu)() from find_active_uprobe_speculative() and
> replace it with lockdep_assert(rcu_read_lock_trace_held()) and a comment
> documenting that callers must hold RCU Tasks Trace protection, matching the
> convention used by find_active_uprobe_rcu() and find_uprobe_rcu().
> 
> Fixes: e0925f2dc4de ("uprobes: add speculative lockless VMA-to-inode-to-uprobe resolution")
> Assisted-by: Antigravity:gemini-3.6-flash
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  kernel/events/uprobes.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 4084e926e284..b4e253d43b30 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -2424,6 +2424,7 @@ static int is_trap_at_addr(struct mm_struct *mm, unsigned long vaddr)
>  	return is_trap_insn(&opcode);
>  }
>  
> +/* assumes being inside RCU Tasks Trace protected region */
>  static struct uprobe *find_active_uprobe_speculative(unsigned long bp_vaddr)
>  {
>  	struct mm_struct *mm = current->mm;
> @@ -2433,7 +2434,7 @@ static struct uprobe *find_active_uprobe_speculative(unsigned long bp_vaddr)
>  	loff_t offset;
>  	unsigned int seq;
>  
> -	guard(rcu)();
> +	lockdep_assert(rcu_read_lock_trace_held());
>  
>  	if (!mmap_lock_speculate_try_begin(mm, &seq))
>  		return NULL;

Seems fine, but it this really worthy of Fixes? I mean, its just a micro
optimization, no?