mm, x86/mm: Move creating the tlb_flush event back to x86 code

[PATCH] mm, x86/mm: Move creating the tlb_flush event back to x86 code

Posted by Steven Rostedt 4 months ago

From: Steven Rostedt <rostedt@goodmis.org>

Commit e73ad5ff2f76 ("mm, x86/mm: Make the batched unmap TLB flush API
more generic") moved the trace_tlb_flush out of mm/rmap.c and back into
x86 specific architecture, but it kept the include to the events/tlb.h
file, even though it didn't use that event.

Then another commit came in and added more events to the mm/rmap.c file
and moved the #define CREATE_TRACE_POINTS define from the x86 specific
architecture to the generic mm/rmap.h file to create both the tlb_flush
tracepoint and the new tracepoints.

But since the tlb_flush tracepoint is only x86 specific, it now creates
that tracepoint for all other architectures and this wastes approximately
5K of text and meta data that will not be used.

Remove the events/tlb.h from mm/rmap.c and add the define
CREATE_TRACE_POINTS back in the x86 code.

Fixes: 4cc79b3303f22 ("mm/migration: add trace events for base page and HugeTLB migrations")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
Note, I will be adding code soon that will make unused events cause a warning.

 arch/x86/mm/init.c | 1 +
 mm/rmap.c          | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 7456df985d96..f85313a8b5a9 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -34,6 +34,7 @@
  * We need to define the tracepoints somewhere, and tlb.c
  * is only compiled when SMP=y.
  */
+#define CREATE_TRACE_POINTS
 #include <trace/events/tlb.h>
 
 #include "mm_internal.h"
diff --git a/mm/rmap.c b/mm/rmap.c
index fb63d9256f09..0bc00668970f 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -79,7 +79,6 @@
 #include <asm/tlbflush.h>
 
 #define CREATE_TRACE_POINTS
-#include <trace/events/tlb.h>
 #include <trace/events/migrate.h>
 
 #include "internal.h"
-- 
2.47.2

Re: [PATCH] mm, x86/mm: Move creating the tlb_flush event back to x86 code

Posted by Lorenzo Stoakes 3 months, 4 weeks ago

On Thu, Jun 12, 2025 at 10:03:13AM -0400, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> Commit e73ad5ff2f76 ("mm, x86/mm: Make the batched unmap TLB flush API
> more generic") moved the trace_tlb_flush out of mm/rmap.c and back into
> x86 specific architecture, but it kept the include to the events/tlb.h
> file, even though it didn't use that event.
>
> Then another commit came in and added more events to the mm/rmap.c file
> and moved the #define CREATE_TRACE_POINTS define from the x86 specific
> architecture to the generic mm/rmap.h file to create both the tlb_flush
> tracepoint and the new tracepoints.
>
> But since the tlb_flush tracepoint is only x86 specific, it now creates
> that tracepoint for all other architectures and this wastes approximately
> 5K of text and meta data that will not be used.
>
> Remove the events/tlb.h from mm/rmap.c and add the define
> CREATE_TRACE_POINTS back in the x86 code.
>
> Fixes: 4cc79b3303f22 ("mm/migration: add trace events for base page and HugeTLB migrations")
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>

LGTM so:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
> Note, I will be adding code soon that will make unused events cause a warning.
>
>  arch/x86/mm/init.c | 1 +
>  mm/rmap.c          | 1 -
>  2 files changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> index 7456df985d96..f85313a8b5a9 100644
> --- a/arch/x86/mm/init.c
> +++ b/arch/x86/mm/init.c
> @@ -34,6 +34,7 @@
>   * We need to define the tracepoints somewhere, and tlb.c
>   * is only compiled when SMP=y.
>   */
> +#define CREATE_TRACE_POINTS
>  #include <trace/events/tlb.h>
>
>  #include "mm_internal.h"
> diff --git a/mm/rmap.c b/mm/rmap.c
> index fb63d9256f09..0bc00668970f 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -79,7 +79,6 @@
>  #include <asm/tlbflush.h>
>
>  #define CREATE_TRACE_POINTS
> -#include <trace/events/tlb.h>
>  #include <trace/events/migrate.h>
>
>  #include "internal.h"
> --
> 2.47.2
>

Re: [PATCH] mm, x86/mm: Move creating the tlb_flush event back to x86 code

Posted by Steven Rostedt 3 months, 3 weeks ago

On Thu, 12 Jun 2025 18:24:30 +0100
Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:

> > Remove the events/tlb.h from mm/rmap.c and add the define
> > CREATE_TRACE_POINTS back in the x86 code.
> >
> > Fixes: 4cc79b3303f22 ("mm/migration: add trace events for base page and HugeTLB migrations")
> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>  
> 
> LGTM so:
> 
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

Thanks.

Should I take this or should this go through the mm tree?

-- Steve

Re: [PATCH] mm, x86/mm: Move creating the tlb_flush event back to x86 code

Posted by David Hildenbrand 3 months, 3 weeks ago

On 16.06.25 21:09, Steven Rostedt wrote:
> On Thu, 12 Jun 2025 18:24:30 +0100
> Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
> 
>>> Remove the events/tlb.h from mm/rmap.c and add the define
>>> CREATE_TRACE_POINTS back in the x86 code.
>>>
>>> Fixes: 4cc79b3303f22 ("mm/migration: add trace events for base page and HugeTLB migrations")
>>> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
>>
>> LGTM so:
>>
>> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> 
> Thanks.
> 
> Should I take this or should this go through the mm tree?

I suspect this can go through your tree. (@Andrew)

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH] mm, x86/mm: Move creating the tlb_flush event back to x86 code

Posted by Andrew Morton 3 months, 3 weeks ago

On Mon, 16 Jun 2025 21:19:18 +0200 David Hildenbrand <david@redhat.com> wrote:

> >>> Fixes: 4cc79b3303f22 ("mm/migration: add trace events for base page and HugeTLB migrations")
> >>> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> >>
> >> LGTM so:
> >>
> >> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > 
> > Thanks.
> > 
> > Should I take this or should this go through the mm tree?
> 
> I suspect this can go through your tree. (@Andrew)
> 

Sure.

Re: [PATCH] mm, x86/mm: Move creating the tlb_flush event back to x86 code

Posted by Steven Rostedt 1 month, 3 weeks ago

On Mon, 16 Jun 2025 16:17:35 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Mon, 16 Jun 2025 21:19:18 +0200 David Hildenbrand <david@redhat.com> wrote:
> 
> > >>> Fixes: 4cc79b3303f22 ("mm/migration: add trace events for base page and HugeTLB migrations")
> > >>> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>  
> > >>
> > >> LGTM so:
> > >>
> > >> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>  
> > > 
> > > Thanks.
> > > 
> > > Should I take this or should this go through the mm tree?  
> > 
> > I suspect this can go through your tree. (@Andrew)
> >   
> 
> Sure.

Did this fall through the cracks?

-- Steve

Re: [PATCH] mm, x86/mm: Move creating the tlb_flush event back to x86 code

Posted by Andrew Morton 1 month, 3 weeks ago

On Tue, 19 Aug 2025 18:42:55 -0400 Steven Rostedt <rostedt@goodmis.org> wrote:

> On Mon, 16 Jun 2025 16:17:35 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Mon, 16 Jun 2025 21:19:18 +0200 David Hildenbrand <david@redhat.com> wrote:
> > 
> > > >>> Fixes: 4cc79b3303f22 ("mm/migration: add trace events for base page and HugeTLB migrations")
> > > >>> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>  
> > > >>
> > > >> LGTM so:
> > > >>
> > > >> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>  
> > > > 
> > > > Thanks.
> > > > 
> > > > Should I take this or should this go through the mm tree?  
> > > 
> > > I suspect this can go through your tree. (@Andrew)
> > >   
> > 
> > Sure.
> 
> Did this fall through the cracks?

I guess so.

<elides lewd crack about "cracks">

I added it with your revised

Fixes: e73ad5ff2f76 ("mm, x86/mm: Make the batched unmap TLB flush API more gene
ric")

Re: [PATCH] mm, x86/mm: Move creating the tlb_flush event back to x86 code

Posted by Dave Hansen 4 months ago

On 6/12/25 07:03, Steven Rostedt wrote:
> Remove the events/tlb.h from mm/rmap.c and add the define
> CREATE_TRACE_POINTS back in the x86 code.
> 
> Fixes: 4cc79b3303f22 ("mm/migration: add trace events for base page and HugeTLB migrations")

Is this a big enough problem that it deserves a cc:stable@?

Re: [PATCH] mm, x86/mm: Move creating the tlb_flush event back to x86 code

Posted by Steven Rostedt 4 months ago

On Thu, 12 Jun 2025 07:06:47 -0700
Dave Hansen <dave.hansen@intel.com> wrote:

> On 6/12/25 07:03, Steven Rostedt wrote:
> > Remove the events/tlb.h from mm/rmap.c and add the define
> > CREATE_TRACE_POINTS back in the x86 code.
> > 
> > Fixes: 4cc79b3303f22 ("mm/migration: add trace events for base page and HugeTLB migrations")  
> 
> Is this a big enough problem that it deserves a cc:stable@?

That's questionable. Without this patch, some memory is simply wasted.
So it matters how much you care about wasted memory? On memory tight
systems it may make a difference. But it's just one trace event, which
would free up around 5K of memory.

I put in the fixes tag here because it was an obvious mistake that the
header file wasn't removed from rmap.c when the call to the tracepoint
was. Hmm, I think I put in the wrong fixes tag. I should probably be
the other commit:

Fixes: e73ad5ff2f76 ("mm, x86/mm: Make the batched unmap TLB flush API more generic")

-- Steve