Re: [PATCH] kmem/tracing: Add kmem name to kmem_cache_alloc tracepoint

[Steven Rostedt]

On Mon, 3 Feb 2025 09:34:29 +0100
Vlastimil Babka <vbabka@xxxxxxx> wrote:

> On 1/30/25 19:16, Wander Lairson Costa wrote:
> > The kmem_cache_free tracepoint includes a "name" field, which allows
> > for easy identification and filtering of specific kmem's. However, the
> > kmem_cache_alloc tracepoint lacks this field, making it difficult to
> > pair corresponding alloc and free events for analysis.  
> 
> Hm yeah looks like the free one was added in commit 3544de8ee6e48 and their
> use case didn't need alloc too, but it makes sense.
> 
> > Add the "name" field to kmem_cache_alloc to enable consistent tracking
> > and correlation of kmem alloc and free events.
> > 
> > Signed-off-by: Wander Lairson Costa <wander@xxxxxxxxxx>  
> 
> I will take this in the slab tree, unless Steven objects.

No objections but...

> 
> Thanks,
> Vlastimil
> 
> > ---
> >  include/trace/events/kmem.h | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/trace/events/kmem.h b/include/trace/events/kmem.h
> > index b37eb0a7060f..8ffb022f024c 100644
> > --- a/include/trace/events/kmem.h
> > +++ b/include/trace/events/kmem.h
> > @@ -22,6 +22,7 @@ TRACE_EVENT(kmem_cache_alloc,
> >  	TP_STRUCT__entry(
> >  		__field(	unsigned long,	call_site	)
> >  		__field(	const void *,	ptr		)
> > +		__string(	name,		s->name		)

The string place holder in the "struct" is 4 bytes and not a pointer. So I
would place it either at the end or next to another "int" to avoid any
holes caused by it being placed next to a 8 byte field.

-- Steve


> >  		__field(	size_t,		bytes_req	)
> >  		__field(	size_t,		bytes_alloc	)
> >  		__field(	unsigned long,	gfp_flags	)
> > @@ -32,6 +33,7 @@ TRACE_EVENT(kmem_cache_alloc,
> >  	TP_fast_assign(
> >  		__entry->call_site	= call_site;
> >  		__entry->ptr		= ptr;
> > +		__assign_str(name);
> >  		__entry->bytes_req	= s->object_size;
> >  		__entry->bytes_alloc	= s->size;
> >  		__entry->gfp_flags	= (__force unsigned long)gfp_flags;
> > @@ -41,9 +43,10 @@ TRACE_EVENT(kmem_cache_alloc,
> >  					  (s->flags & SLAB_ACCOUNT)) : false;
> >  	),
> >  
> > -	TP_printk("call_site=%pS ptr=%p bytes_req=%zu bytes_alloc=%zu gfp_flags=%s node=%d accounted=%s",
> > +	TP_printk("call_site=%pS ptr=%p name=%s bytes_req=%zu bytes_alloc=%zu gfp_flags=%s node=%d accounted=%s",
> >  		(void *)__entry->call_site,
> >  		__entry->ptr,
> > +		__get_str(name),
> >  		__entry->bytes_req,
> >  		__entry->bytes_alloc,
> >  		show_gfp_flags(__entry->gfp_flags),