Re: [PATCH] perf lock contention: Clear lock addr after use

[Namhyung Kim <namhyung@kernel.org>]

On Fri, Oct 13, 2023 at 4:00 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Thu, Sep 28, 2023 at 04:50:18PM -0700, Namhyung Kim escreveu:
> > It checks the current lock to calculated the delta of contention time.
>
> > The address is saved in the tstamp map which is allocated at begining of
> > contention and released at end of contention.
> >
> > But it's possible for bpf_map_delete_elem() to fail.  In that case, the
>
> How can it fail?
>
> You do:
>
>         pelem = bpf_map_lookup_elem(&tstamp, &pid);
>         if (!pelem || pelem->lock != ctx[0])
>                 return 0;
>
> So it is there, why would the removal using the same key fail?

It can fail when it doesn't get a lock for the internal bucket.
See kernel/bpf/hashtab.c::htab_map_delete_elem().

But I'm not sure whether that's actually possible in this case.

>
> The patch should work as-is, I'm just curious about what would make
> there removal of a map entry that was successfully looked up on the same
> contention_end prog to fail when being removed...

Now I'm seeing some rare error cases like a spinlock wait
is longer than a minute.  I suspect a bug in this code and
try to be more defensive.

Thanks,
Namhyung