From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_2 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BC691C433E0 for ; Thu, 25 Feb 2021 01:38:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7103864EC4 for ; Thu, 25 Feb 2021 01:38:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233895AbhBYBhy (ORCPT ); Wed, 24 Feb 2021 20:37:54 -0500 Received: from mail.kernel.org ([198.145.29.99]:49386 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235080AbhBYBhw (ORCPT ); Wed, 24 Feb 2021 20:37:52 -0500 Received: from oasis.local.home (cpe-66-24-58-225.stny.res.rr.com [66.24.58.225]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 660E564F03; Thu, 25 Feb 2021 01:37:10 +0000 (UTC) Date: Wed, 24 Feb 2021 20:37:08 -0500 From: Steven Rostedt To: Andrew Morton Cc: cl@linux.com, iamjoonsoo.kim@lge.com, jian.w.wen@oracle.com, linux-mm@kvack.org, mm-commits@vger.kernel.org, paulmck@linux.vnet.ibm.com, penberg@kernel.org, rientjes@google.com, torvalds@linux-foundation.org Subject: Re: [patch 014/173] mm, tracing: record slab name for kmem_cache_free() Message-ID: <20210224203708.4489755a@oasis.local.home> In-Reply-To: <20210224200055.U7Xz47kX5%akpm@linux-foundation.org> References: <20210224115824.1e289a6895087f10c41dd8d6@linux-foundation.org> <20210224200055.U7Xz47kX5%akpm@linux-foundation.org> X-Mailer: Claws Mail 3.17.3 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk Reply-To: linux-kernel@vger.kernel.org List-ID: X-Mailing-List: mm-commits@vger.kernel.org On Wed, 24 Feb 2021 12:00:55 -0800 Andrew Morton wrote: > From: Jacob Wen > Subject: mm, tracing: record slab name for kmem_cache_free() > > Currently, a trace record generated by the RCU core is as below. > > ... kmem_cache_free: call_site=rcu_core+0x1fd/0x610 ptr=00000000f3b49a66 > > It doesn't tell us what the RCU core has freed. > > This patch adds the slab name to trace_kmem_cache_free(). > The new format is as follows. > > ... kmem_cache_free: call_site=rcu_core+0x1fd/0x610 ptr=0000000037f79c8d name=dentry > ... kmem_cache_free: call_site=rcu_core+0x1fd/0x610 ptr=00000000f78cb7b5 name=sock_inode_cache > ... kmem_cache_free: call_site=rcu_core+0x1fd/0x610 ptr=0000000018768985 name=pool_workqueue > ... kmem_cache_free: call_site=rcu_core+0x1fd/0x610 ptr=000000006a6cb484 name=radix_tree_node > > We can use it to understand what the RCU core is going to free. For > example, some users maybe interested in when the RCU core starts > freeing reclaimable slabs like dentry to reduce memory pressure. > > Link: https://lkml.kernel.org/r/20201216072804.8838-1-jian.w.wen@oracle.com > Signed-off-by: Jacob Wen > Cc: Christoph Lameter > Cc: Pekka Enberg > Cc: David Rientjes > Cc: Joonsoo Kim > Cc: Steven Rostedt > Cc: "Paul E. McKenney" > Signed-off-by: Andrew Morton > --- > > include/trace/events/kmem.h | 24 ++++++++++++++++-------- > mm/slab.c | 2 +- > mm/slob.c | 2 +- > mm/slub.c | 2 +- > 4 files changed, 19 insertions(+), 11 deletions(-) > > --- a/include/trace/events/kmem.h~mm-tracing-record-slab-name-for-kmem_cache_free > +++ a/include/trace/events/kmem.h > @@ -115,7 +115,7 @@ DEFINE_EVENT(kmem_alloc_node, kmem_cache > TP_ARGS(call_site, ptr, bytes_req, bytes_alloc, gfp_flags, node) > ); > > -DECLARE_EVENT_CLASS(kmem_free, > +TRACE_EVENT(kfree, > > TP_PROTO(unsigned long call_site, const void *ptr), > > @@ -135,18 +135,26 @@ DECLARE_EVENT_CLASS(kmem_free, > (void *)__entry->call_site, __entry->ptr) > ); > > -DEFINE_EVENT(kmem_free, kfree, > +TRACE_EVENT(kmem_cache_free, > > - TP_PROTO(unsigned long call_site, const void *ptr), > + TP_PROTO(unsigned long call_site, const void *ptr, const char *name), > > - TP_ARGS(call_site, ptr) > -); > + TP_ARGS(call_site, ptr, name), > > -DEFINE_EVENT(kmem_free, kmem_cache_free, > + TP_STRUCT__entry( > + __field( unsigned long, call_site ) > + __field( const void *, ptr ) > + __field( const char *, name ) You must use __string() here! __string(name, name); > + ), > > - TP_PROTO(unsigned long call_site, const void *ptr), > + TP_fast_assign( > + __entry->call_site = call_site; > + __entry->ptr = ptr; > + __entry->name = name; This must be: __assign_str(name, name); > + ), > > - TP_ARGS(call_site, ptr) > + TP_printk("call_site=%pS ptr=%p name=%s", > + (void *)__entry->call_site, __entry->ptr, __entry->name) You must use __get_str(name) here! (see other users of this logic in include/trace/events/*.h) What is happening is that TP_fast_assign() is called by the tracepoint logic (at the time of the event), then much later (seconds, minutes, hours, days!), when the user does a "cat trace" of the file, the __entry->name is read and the printf logic is called against it. Well, the s->name that __entry->name points to, can be long gone by then! Instead, using __string() tells the TRACE_EVENT() macro that this is a dynamic string. The __assign_str() records the string into the ring buffer. The __get_str() retrieves the string from the ring buffer as part of the event, so it stays around as long as the event being read by the trace file is around. Please do not apply this patch as is, it is very buggy! -- Steve > ); > > TRACE_EVENT(mm_page_free, > --- a/mm/slab.c~mm-tracing-record-slab-name-for-kmem_cache_free > +++ a/mm/slab.c > @@ -3717,7 +3717,7 @@ void kmem_cache_free(struct kmem_cache * > __cache_free(cachep, objp, _RET_IP_); > local_irq_restore(flags); > > - trace_kmem_cache_free(_RET_IP_, objp); > + trace_kmem_cache_free(_RET_IP_, objp, cachep->name); > } > EXPORT_SYMBOL(kmem_cache_free); > > --- a/mm/slob.c~mm-tracing-record-slab-name-for-kmem_cache_free > +++ a/mm/slob.c > @@ -673,7 +673,7 @@ void kmem_cache_free(struct kmem_cache * > __kmem_cache_free(b, c->size); > } > > - trace_kmem_cache_free(_RET_IP_, b); > + trace_kmem_cache_free(_RET_IP_, b, c->name); > } > EXPORT_SYMBOL(kmem_cache_free); > > --- a/mm/slub.c~mm-tracing-record-slab-name-for-kmem_cache_free > +++ a/mm/slub.c > @@ -3157,7 +3157,7 @@ void kmem_cache_free(struct kmem_cache * > if (!s) > return; > slab_free(s, virt_to_head_page(x), x, NULL, 1, _RET_IP_); > - trace_kmem_cache_free(_RET_IP_, x); > + trace_kmem_cache_free(_RET_IP_, x, s->name); > } > EXPORT_SYMBOL(kmem_cache_free); > > _