Linux Perf Users
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
Cc: Oleg Nesterov <oleg@redhat.com>, Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	James Clark <james.clark@linaro.org>,
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	linux-perf-users@vger.kernel.org
Subject: Re: [PATCH] uprobes: Remove redundant guard(rcu) in find_active_uprobe_speculative()
Date: Thu, 30 Jul 2026 12:30:10 +0200	[thread overview]
Message-ID: <20260730103010.GB751831@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <178537593930.153254.1296986092000402239.stgit@devnote2>

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?

      parent reply	other threads:[~2026-07-30 10:30 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-30  1:45 [PATCH] uprobes: Remove redundant guard(rcu) in find_active_uprobe_speculative() Masami Hiramatsu (Google)
2026-07-30  1:59 ` sashiko-bot
2026-07-30  3:36   ` Masami Hiramatsu
2026-07-30  3:38 ` Masami Hiramatsu
2026-07-30 10:30 ` Peter Zijlstra [this message]

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=20260730103010.GB751831@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=oleg@redhat.com \
    /path/to/YOUR_REPLY

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

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