Re: [PATCH] mm: remove unused mmap tracepoints

Re: [PATCH] mm: remove unused mmap tracepoints

[Liam R. Howlett]

+ linux-mm

* Caleb Sander Mateos <csander@purestorage.com> [250411 12:18]:
> The vma_mas_szero and vma_store tracepoints are unused since commit
> fbcc3104b843 ("mmap: convert __vma_adjust() to use vma iterator").
> Remove them so they are no longer listed as available tracepoints.
> 
> Reported-by: Eric Mueller <emueller@purestorage.com>
> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> ---
>  include/trace/events/mmap.h | 52 -------------------------------------


Just a note about this file, it is currently not listed in MEMORY
MAPPING, but it seems other files in the trace/events are handled by
entries in the MAINTAINERS file.

I'll send out a change to MAINTAINERS to do this.

Thanks,
Liam

Re: [PATCH] mm: remove unused mmap tracepoints

[Caleb Sander Mateos]

Hi MM folks,
Would you mind picking up this patch to remove some old tracepoints?
Liam has already given it a review.

Thanks,
Caleb

On Fri, Apr 11, 2025 at 10:23 AM Liam R. Howlett
<Liam.Howlett@oracle.com> wrote:
>
> * Caleb Sander Mateos <csander@purestorage.com> [250411 12:18]:
> > The vma_mas_szero and vma_store tracepoints are unused since commit
> > fbcc3104b843 ("mmap: convert __vma_adjust() to use vma iterator").
> > Remove them so they are no longer listed as available tracepoints.
> >
>
> Thanks for doing this.
>
> Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
>
> > Reported-by: Eric Mueller <emueller@purestorage.com>
> > Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> > ---
> >  include/trace/events/mmap.h | 52 -------------------------------------
> >  1 file changed, 52 deletions(-)
> >
> > diff --git a/include/trace/events/mmap.h b/include/trace/events/mmap.h
> > index f8d61485de16..ee2843a5daef 100644
> > --- a/include/trace/events/mmap.h
> > +++ b/include/trace/events/mmap.h
> > @@ -41,62 +41,10 @@ TRACE_EVENT(vm_unmapped_area,
> >               __entry->total_vm, __entry->flags, __entry->length,
> >               __entry->low_limit, __entry->high_limit, __entry->align_mask,
> >               __entry->align_offset)
> >  );
> >
> > -TRACE_EVENT(vma_mas_szero,
> > -     TP_PROTO(struct maple_tree *mt, unsigned long start,
> > -              unsigned long end),
> > -
> > -     TP_ARGS(mt, start, end),
> > -
> > -     TP_STRUCT__entry(
> > -                     __field(struct maple_tree *, mt)
> > -                     __field(unsigned long, start)
> > -                     __field(unsigned long, end)
> > -     ),
> > -
> > -     TP_fast_assign(
> > -                     __entry->mt             = mt;
> > -                     __entry->start          = start;
> > -                     __entry->end            = end;
> > -     ),
> > -
> > -     TP_printk("mt_mod %p, (NULL), SNULL, %lu, %lu,",
> > -               __entry->mt,
> > -               (unsigned long) __entry->start,
> > -               (unsigned long) __entry->end
> > -     )
> > -);
> > -
> > -TRACE_EVENT(vma_store,
> > -     TP_PROTO(struct maple_tree *mt, struct vm_area_struct *vma),
> > -
> > -     TP_ARGS(mt, vma),
> > -
> > -     TP_STRUCT__entry(
> > -                     __field(struct maple_tree *, mt)
> > -                     __field(struct vm_area_struct *, vma)
> > -                     __field(unsigned long, vm_start)
> > -                     __field(unsigned long, vm_end)
> > -     ),
> > -
> > -     TP_fast_assign(
> > -                     __entry->mt             = mt;
> > -                     __entry->vma            = vma;
> > -                     __entry->vm_start       = vma->vm_start;
> > -                     __entry->vm_end         = vma->vm_end - 1;
> > -     ),
> > -
> > -     TP_printk("mt_mod %p, (%p), STORE, %lu, %lu,",
> > -               __entry->mt, __entry->vma,
> > -               (unsigned long) __entry->vm_start,
> > -               (unsigned long) __entry->vm_end
> > -     )
> > -);
> > -
> > -
> >  TRACE_EVENT(exit_mmap,
> >       TP_PROTO(struct mm_struct *mm),
> >
> >       TP_ARGS(mm),
> >
> > --
> > 2.45.2
> >

Re: [PATCH] mm: remove unused mmap tracepoints

[Lorenzo Stoakes]

Sorry this wasn't picked up.

Andrew - one to pick up after merge window? :)

Caleb - I'd ping again after the merge window is over if it's not picked up
this week. This isn't a great time as people tend to be busy sending stuff
over to Linus.

On Wed, May 28, 2025 at 08:16:24AM -0700, Caleb Sander Mateos wrote:
> Hi MM folks,
> Would you mind picking up this patch to remove some old tracepoints?
> Liam has already given it a review.

I am actually curious about reintroducing these at some point, but no harm
removing them for now as they are as you say, unused. If I do work on
reintroducing them I can obviously... reintroduce them :)

>
> Thanks,
> Caleb
>
> On Fri, Apr 11, 2025 at 10:23 AM Liam R. Howlett
> <Liam.Howlett@oracle.com> wrote:
> >
> > * Caleb Sander Mateos <csander@purestorage.com> [250411 12:18]:
> > > The vma_mas_szero and vma_store tracepoints are unused since commit
> > > fbcc3104b843 ("mmap: convert __vma_adjust() to use vma iterator").
> > > Remove them so they are no longer listed as available tracepoints.
> > >
> >
> > Thanks for doing this.
> >
> > Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

LGTM so:

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

> >
> > > Reported-by: Eric Mueller <emueller@purestorage.com>
> > > Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> > > ---
> > >  include/trace/events/mmap.h | 52 -------------------------------------
> > >  1 file changed, 52 deletions(-)
> > >
> > > diff --git a/include/trace/events/mmap.h b/include/trace/events/mmap.h
> > > index f8d61485de16..ee2843a5daef 100644
> > > --- a/include/trace/events/mmap.h
> > > +++ b/include/trace/events/mmap.h
> > > @@ -41,62 +41,10 @@ TRACE_EVENT(vm_unmapped_area,
> > >               __entry->total_vm, __entry->flags, __entry->length,
> > >               __entry->low_limit, __entry->high_limit, __entry->align_mask,
> > >               __entry->align_offset)
> > >  );
> > >
> > > -TRACE_EVENT(vma_mas_szero,
> > > -     TP_PROTO(struct maple_tree *mt, unsigned long start,
> > > -              unsigned long end),
> > > -
> > > -     TP_ARGS(mt, start, end),
> > > -
> > > -     TP_STRUCT__entry(
> > > -                     __field(struct maple_tree *, mt)
> > > -                     __field(unsigned long, start)
> > > -                     __field(unsigned long, end)
> > > -     ),
> > > -
> > > -     TP_fast_assign(
> > > -                     __entry->mt             = mt;
> > > -                     __entry->start          = start;
> > > -                     __entry->end            = end;
> > > -     ),
> > > -
> > > -     TP_printk("mt_mod %p, (NULL), SNULL, %lu, %lu,",
> > > -               __entry->mt,
> > > -               (unsigned long) __entry->start,
> > > -               (unsigned long) __entry->end
> > > -     )
> > > -);
> > > -
> > > -TRACE_EVENT(vma_store,
> > > -     TP_PROTO(struct maple_tree *mt, struct vm_area_struct *vma),
> > > -
> > > -     TP_ARGS(mt, vma),
> > > -
> > > -     TP_STRUCT__entry(
> > > -                     __field(struct maple_tree *, mt)
> > > -                     __field(struct vm_area_struct *, vma)
> > > -                     __field(unsigned long, vm_start)
> > > -                     __field(unsigned long, vm_end)
> > > -     ),
> > > -
> > > -     TP_fast_assign(
> > > -                     __entry->mt             = mt;
> > > -                     __entry->vma            = vma;
> > > -                     __entry->vm_start       = vma->vm_start;
> > > -                     __entry->vm_end         = vma->vm_end - 1;
> > > -     ),
> > > -
> > > -     TP_printk("mt_mod %p, (%p), STORE, %lu, %lu,",
> > > -               __entry->mt, __entry->vma,
> > > -               (unsigned long) __entry->vm_start,
> > > -               (unsigned long) __entry->vm_end
> > > -     )
> > > -);
> > > -
> > > -
> > >  TRACE_EVENT(exit_mmap,
> > >       TP_PROTO(struct mm_struct *mm),
> > >
> > >       TP_ARGS(mm),
> > >
> > > --
> > > 2.45.2
> > >

Re: [PATCH] mm: remove unused mmap tracepoints

[Caleb Sander Mateos]

On Wed, May 28, 2025 at 8:23 AM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
>
> Sorry this wasn't picked up.

No worries, I think I didn't send it to the relevant people and
mailing list originally since it wasn't in the MAINTAINERS file (which
Liam has since fixed).

>
> Andrew - one to pick up after merge window? :)
>
> Caleb - I'd ping again after the merge window is over if it's not picked up
> this week. This isn't a great time as people tend to be busy sending stuff
> over to Linus.

Sure, no rush on this. Just spring cleaning my inbox :)

>
> On Wed, May 28, 2025 at 08:16:24AM -0700, Caleb Sander Mateos wrote:
> > Hi MM folks,
> > Would you mind picking up this patch to remove some old tracepoints?
> > Liam has already given it a review.
>
> I am actually curious about reintroducing these at some point, but no harm
> removing them for now as they are as you say, unused. If I do work on
> reintroducing them I can obviously... reintroduce them :)
>
> >
> > Thanks,
> > Caleb
> >
> > On Fri, Apr 11, 2025 at 10:23 AM Liam R. Howlett
> > <Liam.Howlett@oracle.com> wrote:
> > >
> > > * Caleb Sander Mateos <csander@purestorage.com> [250411 12:18]:
> > > > The vma_mas_szero and vma_store tracepoints are unused since commit
> > > > fbcc3104b843 ("mmap: convert __vma_adjust() to use vma iterator").
> > > > Remove them so they are no longer listed as available tracepoints.
> > > >
> > >
> > > Thanks for doing this.
> > >
> > > Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
>
> LGTM so:
>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

Thanks,
Caleb

Re: [PATCH] mm: remove unused mmap tracepoints

[Steven Rostedt]

On Wed, 28 May 2025 08:16:24 -0700
Caleb Sander Mateos <csander@purestorage.com> wrote:

> Would you mind picking up this patch to remove some old tracepoints?
> Liam has already given it a review.

Someday I need to add code that lists the events that are never called. I
can imagine that there's several tracepoints that are created and never
used, and they still do waste memory :-(

-- Steve

Re: [PATCH] mm: remove unused mmap tracepoints

[Lorenzo Stoakes]

On Wed, May 28, 2025 at 11:45:49AM -0400, Steven Rostedt wrote:
> On Wed, 28 May 2025 08:16:24 -0700
> Caleb Sander Mateos <csander@purestorage.com> wrote:
>
> > Would you mind picking up this patch to remove some old tracepoints?
> > Liam has already given it a review.
>
> Someday I need to add code that lists the events that are never called. I
> can imagine that there's several tracepoints that are created and never
> used, and they still do waste memory :-(
>
> -- Steve

Who traces the tracepoints? A question as old as time...

Maybe reasonably straightforward with a clangd/ctags/etc. backend or
something to track usage?

In fact could do a quick-an'-dirty script to grep invocations and have a
person run down the results to double-check or sth?

Re: [PATCH] mm: remove unused mmap tracepoints

[Steven Rostedt]

On Wed, 28 May 2025 16:48:10 +0100
Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:

> Maybe reasonably straightforward with a clangd/ctags/etc. backend or
> something to track usage?
> 
> In fact could do a quick-an'-dirty script to grep invocations and have a
> person run down the results to double-check or sth?

Yeah, I have a patch that shows how many static key instances exist and I
test that. But I probably could also add an option to the macro that
creates the trace function to also add something to a section when used,
and report when it isn't. Shouldn't be too hard.

-- Steve

Re: [PATCH] mm: remove unused mmap tracepoints

[Steven Rostedt]

On Wed, 28 May 2025 12:24:11 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> Yeah, I have a patch that shows how many static key instances exist and I
> test that. But I probably could also add an option to the macro that
> creates the trace function to also add something to a section when used,
> and report when it isn't. Shouldn't be too hard.

Done:  https://lore.kernel.org/all/20250529130138.544ffec4@gandalf.local.home/

-- Steve

Re: [PATCH] mm: remove unused mmap tracepoints

[Steven Rostedt]

On Thu, 29 May 2025 14:43:27 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 28 May 2025 12:24:11 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > Yeah, I have a patch that shows how many static key instances exist and I
> > test that. But I probably could also add an option to the macro that
> > creates the trace function to also add something to a section when used,
> > and report when it isn't. Shouldn't be too hard.  
> 
> Done:  https://lore.kernel.org/all/20250529130138.544ffec4@gandalf.local.home/
> 

And going through the list, I almost sent a patch that removed the two
events that this patch removes!

Luckily, when I ran "get_maintainers.pl" on the patch, I noticed you were
one of the maintainers and then I thought "Hmm, is this the events that are
removed that started all this?" And sure enough, it was!

This was going to be my change log:

Subject: [PATCH] mmap: Remove unused events vma_mas_szero and vma_store

When the __vma_adjust() was converted to use the vma iterator it removed
the functions vma_mas_store() and vma_mas_remove(). These functions called
the tracepoints trace_vma_mas_store() and trace_vma_mas_szero()
respectively. The calls to these tracepoints were removed but the trace
events that created the tracepoints were not removed. Each trace event can
take up to 5K of memory, and it is allocated regardless of if they are
called or not.

Remove the unused trace events.

Link: https://lore.kernel.org/all/20250529130138.544ffec4@gandalf.local.home/

Fixes: fbcc3104b843 ("mmap: convert __vma_adjust() to use vma iterator")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>

-- Steve