linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] uprobe: Move arch_uprobe_optimize right after handlers execution
@ 2025-10-01 13:24 Jiri Olsa
  2025-10-01 16:39 ` Andrii Nakryiko
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Jiri Olsa @ 2025-10-01 13:24 UTC (permalink / raw)
  To: Oleg Nesterov, Peter Zijlstra, Andrii Nakryiko
  Cc: Linus Torvalds, bpf, linux-kernel, linux-trace-kernel, x86,
	Song Liu, Yonghong Song, John Fastabend, Hao Luo, Steven Rostedt,
	Masami Hiramatsu, Alan Maguire, David Laight,
	Thomas Weißschuh, Ingo Molnar, Jann Horn, Alejandro Colomar

It's less confusing to optimize uprobe right after handlers execution
and before we do the check for changed ip register to avoid situations
where changed ip register would skip uprobe optimization.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/events/uprobes.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 5dcf927310fd..c14ec27b976d 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -2765,6 +2765,9 @@ static void handle_swbp(struct pt_regs *regs)
 
 	handler_chain(uprobe, regs);
 
+	/* Try to optimize after first hit. */
+	arch_uprobe_optimize(&uprobe->arch, bp_vaddr);
+
 	/*
 	 * If user decided to take execution elsewhere, it makes little sense
 	 * to execute the original instruction, so let's skip it.
@@ -2772,9 +2775,6 @@ static void handle_swbp(struct pt_regs *regs)
 	if (instruction_pointer(regs) != bp_vaddr)
 		goto out;
 
-	/* Try to optimize after first hit. */
-	arch_uprobe_optimize(&uprobe->arch, bp_vaddr);
-
 	if (arch_uprobe_skip_sstep(&uprobe->arch, regs))
 		goto out;
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] uprobe: Move arch_uprobe_optimize right after handlers execution
  2025-10-01 13:24 [PATCH] uprobe: Move arch_uprobe_optimize right after handlers execution Jiri Olsa
@ 2025-10-01 16:39 ` Andrii Nakryiko
  2025-10-01 23:20 ` Masami Hiramatsu
  2025-10-03 11:48 ` Oleg Nesterov
  2 siblings, 0 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2025-10-01 16:39 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Oleg Nesterov, Peter Zijlstra, Andrii Nakryiko, Linus Torvalds,
	bpf, linux-kernel, linux-trace-kernel, x86, Song Liu,
	Yonghong Song, John Fastabend, Hao Luo, Steven Rostedt,
	Masami Hiramatsu, Alan Maguire, David Laight,
	Thomas Weißschuh, Ingo Molnar, Jann Horn, Alejandro Colomar

On Wed, Oct 1, 2025 at 6:25 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> It's less confusing to optimize uprobe right after handlers execution
> and before we do the check for changed ip register to avoid situations
> where changed ip register would skip uprobe optimization.
>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  kernel/events/uprobes.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>

makes sense

Acked-by: Andrii Nakryiko <andrii@kernel.org>

> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 5dcf927310fd..c14ec27b976d 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -2765,6 +2765,9 @@ static void handle_swbp(struct pt_regs *regs)
>
>         handler_chain(uprobe, regs);
>
> +       /* Try to optimize after first hit. */
> +       arch_uprobe_optimize(&uprobe->arch, bp_vaddr);
> +
>         /*
>          * If user decided to take execution elsewhere, it makes little sense
>          * to execute the original instruction, so let's skip it.
> @@ -2772,9 +2775,6 @@ static void handle_swbp(struct pt_regs *regs)
>         if (instruction_pointer(regs) != bp_vaddr)
>                 goto out;
>
> -       /* Try to optimize after first hit. */
> -       arch_uprobe_optimize(&uprobe->arch, bp_vaddr);
> -
>         if (arch_uprobe_skip_sstep(&uprobe->arch, regs))
>                 goto out;
>
> --
> 2.51.0
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] uprobe: Move arch_uprobe_optimize right after handlers execution
  2025-10-01 13:24 [PATCH] uprobe: Move arch_uprobe_optimize right after handlers execution Jiri Olsa
  2025-10-01 16:39 ` Andrii Nakryiko
@ 2025-10-01 23:20 ` Masami Hiramatsu
  2025-10-03 11:48 ` Oleg Nesterov
  2 siblings, 0 replies; 4+ messages in thread
From: Masami Hiramatsu @ 2025-10-01 23:20 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Oleg Nesterov, Peter Zijlstra, Andrii Nakryiko, Linus Torvalds,
	bpf, linux-kernel, linux-trace-kernel, x86, Song Liu,
	Yonghong Song, John Fastabend, Hao Luo, Steven Rostedt,
	Masami Hiramatsu, Alan Maguire, David Laight,
	Thomas Weißschuh, Ingo Molnar, Jann Horn, Alejandro Colomar

On Wed,  1 Oct 2025 15:24:49 +0200
Jiri Olsa <jolsa@kernel.org> wrote:

> It's less confusing to optimize uprobe right after handlers execution
> and before we do the check for changed ip register to avoid situations
> where changed ip register would skip uprobe optimization.
> 
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>

Looks good to me.

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


Thanks,

> ---
>  kernel/events/uprobes.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 5dcf927310fd..c14ec27b976d 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -2765,6 +2765,9 @@ static void handle_swbp(struct pt_regs *regs)
>  
>  	handler_chain(uprobe, regs);
>  
> +	/* Try to optimize after first hit. */
> +	arch_uprobe_optimize(&uprobe->arch, bp_vaddr);
> +
>  	/*
>  	 * If user decided to take execution elsewhere, it makes little sense
>  	 * to execute the original instruction, so let's skip it.
> @@ -2772,9 +2775,6 @@ static void handle_swbp(struct pt_regs *regs)
>  	if (instruction_pointer(regs) != bp_vaddr)
>  		goto out;
>  
> -	/* Try to optimize after first hit. */
> -	arch_uprobe_optimize(&uprobe->arch, bp_vaddr);
> -
>  	if (arch_uprobe_skip_sstep(&uprobe->arch, regs))
>  		goto out;
>  
> -- 
> 2.51.0
> 


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] uprobe: Move arch_uprobe_optimize right after handlers execution
  2025-10-01 13:24 [PATCH] uprobe: Move arch_uprobe_optimize right after handlers execution Jiri Olsa
  2025-10-01 16:39 ` Andrii Nakryiko
  2025-10-01 23:20 ` Masami Hiramatsu
@ 2025-10-03 11:48 ` Oleg Nesterov
  2 siblings, 0 replies; 4+ messages in thread
From: Oleg Nesterov @ 2025-10-03 11:48 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Peter Zijlstra, Andrii Nakryiko, Linus Torvalds, bpf,
	linux-kernel, linux-trace-kernel, x86, Song Liu, Yonghong Song,
	John Fastabend, Hao Luo, Steven Rostedt, Masami Hiramatsu,
	Alan Maguire, David Laight, Thomas Weißschuh, Ingo Molnar,
	Jann Horn, Alejandro Colomar

On 10/01, Jiri Olsa wrote:
>
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -2765,6 +2765,9 @@ static void handle_swbp(struct pt_regs *regs)
>  
>  	handler_chain(uprobe, regs);
>  
> +	/* Try to optimize after first hit. */
> +	arch_uprobe_optimize(&uprobe->arch, bp_vaddr);
> +
>  	/*
>  	 * If user decided to take execution elsewhere, it makes little sense
>  	 * to execute the original instruction, so let's skip it.
> @@ -2772,9 +2775,6 @@ static void handle_swbp(struct pt_regs *regs)
>  	if (instruction_pointer(regs) != bp_vaddr)
>  		goto out;
>  
> -	/* Try to optimize after first hit. */
> -	arch_uprobe_optimize(&uprobe->arch, bp_vaddr);
> -
>  	if (arch_uprobe_skip_sstep(&uprobe->arch, regs))
>  		goto out;

Acked-by: Oleg Nesterov <oleg@redhat.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-10-03 11:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-01 13:24 [PATCH] uprobe: Move arch_uprobe_optimize right after handlers execution Jiri Olsa
2025-10-01 16:39 ` Andrii Nakryiko
2025-10-01 23:20 ` Masami Hiramatsu
2025-10-03 11:48 ` Oleg Nesterov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).