Re: [PATCH v2 3/4] perf lock contention: Resolve slab object name using BPF

[Namhyung Kim <namhyung@kernel.org>]

Hello,

On Wed, Nov 13, 2024 at 03:20:43PM +0100, Vlastimil Babka wrote:
> On 11/12/24 15:50, Arnaldo Carvalho de Melo wrote:
> > On Tue, Nov 12, 2024 at 12:09:24PM +0100, Vlastimil Babka wrote:
> > +               /* look slab_hash for dynamic locks in a slab object */
> > +               if (hashmap__find(&slab_hash, flags & LCB_F_SLAB_ID_MASK, &slab_data)) {
> > +                       snprintf(name_buf, sizeof(name_buf), "&%s", slab_data->name);
> > +                       return name_buf;
> > +        	}
> > 
> > He wants to avoid storing 64 bytes (the slab cache pointer, 's'), instead
> > he wants to store a shorter 'id' and encode it in the upper bits of the
> > 'struct contention_data' 'flags' field.
> > 
> > The iterator, at the beggining of the session attributes this id,
> > starting from zero, to each of the slab caches, so it needs to map it
> > back from the address at contention_end tracepoint.
> > 
> > At post processing time it converts the id back to the name of the slab
> > cache.
> > 
> > I hope this helps,

Thanks Analdo for the explanation!

> 
> Thanks a lot, if it's a tradeoff to do a bit more work in order to store
> less data, then it makes sense to me.

Right, I don't want to increase the data size for this as we have some
unused bits in the flags.  It'd call one more bpf hashmap lookup during
record but I don't think it's gonna be a problem.

Thanks,
Namhyung

> > 
> >> - if it's postprocessing, it would be too late for bpf_get_kmem_cache() as
> >> the object might be gone already?
> >> 
> >> The second alternative would be worse as it could miss the cache or
> >> misattribute (in case page is reallocated by another cache), the first is
> >> just less efficient than possible.
> >> 
> >> > +			}
> >> > +		}
> >> >  
> >> >  		err = bpf_map_update_elem(&lock_stat, &key, &first, BPF_NOEXIST);
> >> >  		if (err < 0) {
>