On Tue, 26 Aug 2025 10:18:40 +0200
Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> On Tue, Aug 26, 2025 at 06:51:58AM +0100, David Laight wrote:
>
> > > @@ -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;
>
> I considered that, but that was actually more confusing. Yes the return
> check is neat, but urgh.
>
> The tri-state return is:
>
> <0 -- error
> 0 -- false
> 1 -- true
>
> and that is converted to the 'normal' convention:
>
> <0 -- error
> 0 -- success
>
>
> Making that intermediate:
>
> <0 -- error
> 0 -- true
> 1 -- false
>
> is just asking for trouble later.
I'm sure the function name could be changed to make it all work :-)
David
