On Mon, 3 Feb 2025 09:34:29 +0100
Vlastimil Babka <[email protected]> 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 <[email protected]>  
> 
> 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),  


Reply via email to