On Tue, Aug 26, 2025 at 06:51:58AM +0100, David Laight wrote:
> On Thu, 21 Aug 2025 14:28:24 +0200
> Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> > Make is_optimized() return a tri-state and avoid return through
> > argument. This simplifies things a little.
> >
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> > ---
> > arch/x86/kernel/uprobes.c | 34 +++++++++++++---------------------
> > 1 file changed, 13 insertions(+), 21 deletions(-)
> >
> > --- a/arch/x86/kernel/uprobes.c
> > +++ b/arch/x86/kernel/uprobes.c
> > @@ -1047,7 +1047,7 @@ static bool __is_optimized(uprobe_opcode
> > return __in_uprobe_trampoline(vaddr + 5 + call->raddr);
> > }
> >
> > -static int is_optimized(struct mm_struct *mm, unsigned long vaddr, bool *optimized)
> > +static int is_optimized(struct mm_struct *mm, unsigned long vaddr)
> > {
> > uprobe_opcode_t insn[5];
> > int err;
> > @@ -1055,8 +1055,7 @@ static int is_optimized(struct mm_struct
> > err = copy_from_vaddr(mm, vaddr, &insn, 5);
> > if (err)
> > return err;
> > - *optimized = __is_optimized((uprobe_opcode_t *)&insn, vaddr);
> > - return 0;
> > + return __is_optimized((uprobe_opcode_t *)&insn, vaddr);
> > }
> >
> > static bool should_optimize(struct arch_uprobe *auprobe)
> > @@ -1069,17 +1068,14 @@ int set_swbp(struct arch_uprobe *auprobe
> > unsigned long vaddr)
> > {
> > if (should_optimize(auprobe)) {
> > - bool optimized = false;
> > - int err;
> > -
> > /*
> > * We could race with another thread that already optimized the probe,
> > * so let's not overwrite it with int3 again in this case.
> > */
> > - err = is_optimized(vma->vm_mm, vaddr, &optimized);
> > - if (err)
> > - return err;
> > - if (optimized)
> > + int ret = is_optimized(vma->vm_mm, vaddr);
> > + if (ret < 0)
> > + return ret;
> > + if (ret)
> > return 0;
>
> Looks like you should swap over 0 and 1.
> That would then be: if (ret <= 0) return ret;
hum, but if it's not optimized (ret == 0) we need to follow up with
installing breakpoint through following uprobe_write_opcode call
also I noticed we mix int/bool return, perhaps we could do fix below
jirka
---
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 0a8c0a4a5423..853abb2a5638 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -1064,7 +1064,7 @@ static int is_optimized(struct mm_struct *mm, unsigned long vaddr)
err = copy_from_vaddr(mm, vaddr, &insn, 5);
if (err)
return err;
- return __is_optimized((uprobe_opcode_t *)&insn, vaddr);
+ return __is_optimized((uprobe_opcode_t *)&insn, vaddr) ? 1 : 0;
}
static bool should_optimize(struct arch_uprobe *auprobe)
