On Tue, Aug 26, 2025 at 10:25:29AM +0200, Jiri Olsa wrote: > On Tue, Aug 26, 2025 at 06:51:58AM +0100, David Laight wrote: > > On Thu, 21 Aug 2025 14:28:24 +0200 > > Peter Zijlstra <[email protected]> wrote: > > > > > Make is_optimized() return a tri-state and avoid return through > > > argument. This simplifies things a little. > > > > > > Signed-off-by: Peter Zijlstra (Intel) <[email protected]> > > > --- > > > 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
ah u meant to swap the whole thing.. got it > > 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)
