[PATCH 00/14] xfs: Remove unused trace events

Steven Rostedt posted 14 patches 3 months, 4 weeks ago
fs/xfs/scrub/trace.h |  2 +-
fs/xfs/xfs_trace.h   | 72 ++++++----------------------------------------------
2 files changed, 9 insertions(+), 65 deletions(-)
[PATCH 00/14] xfs: Remove unused trace events
Posted by Steven Rostedt 3 months, 4 weeks ago
Trace events take up to 5K in memory for text and meta data. I have code that
will trigger a warning when it detects unused tracepoints. The XFS file
system contains many events that are not called. Most of them used to be called
but due to code refactoring the calls were removed but the trace events stayed
behind.

Some events were added but never used. If they were recent, I just reported
them, but if they were older, this series simply removes them.

One is called only when CONFIG_COMPACT is defined, so an #ifdef was placed
around it.

A couple are only called in #if 0 code (left as a reminder to fix it), so
those events are wrapped by a #if 0 as well (with a comment).

Finally, one event is supposed to be a trace event class, but was created with
the TRACE_EVENT() macro and not the DECLARE_EVENT_CLASS() macro. This works
because a TRACE_EVENT() is simply a DECLARE_EVENT_CLASS() and DEFINE_EVENT()
where the class and event have the same name. But as this was a mistake, the
event created should not exist.

Each patch is a stand alone patch. If you ack them, I can take them in my
tree, or if you want, you can take them. I may be adding the warning code to
linux-next near the end of the cycle, so it would be good to have this cleaned
up before hand. As this is removing dead code, it may be even OK to send them
to Linus as a fix.


Steven Rostedt (14):
      xfs: tracing; Remove unused event xfs_reflink_cow_found
      xfs: Remove unused trace event xfs_attr_remove_iter_return
      xfs: Remove unused event xlog_iclog_want_sync
      xfs: Remove unused event xfs_ioctl_clone
      xfs: Remove unused xfs_reflink_compare_extents events
      xfs: Remove unused trace event xfs_attr_rmtval_set
      xfs: ifdef out unused xfs_attr events
      xfs: Remove unused event xfs_attr_node_removename
      xfs: Remove unused event xfs_alloc_near_error
      xfs: Remove unused event xfs_alloc_near_nominleft
      xfs: Remove unused event xfs_pagecache_inval
      xfs: Remove usused xfs_end_io_direct events
      xfs: Only create event xfs_file_compat_ioctl when CONFIG_COMPAT is configure
      xfs: Change xfs_xattr_class from a TRACE_EVENT() to DECLARE_EVENT_CLASS()

----
 fs/xfs/scrub/trace.h |  2 +-
 fs/xfs/xfs_trace.h   | 72 ++++++----------------------------------------------
 2 files changed, 9 insertions(+), 65 deletions(-)
Re: [PATCH 00/14] xfs: Remove unused trace events
Posted by Darrick J. Wong 3 months, 4 weeks ago
On Thu, Jun 12, 2025 at 05:24:05PM -0400, Steven Rostedt wrote:
> 
> Trace events take up to 5K in memory for text and meta data. I have code that

Under what circumstances do they eat up that much memory?  And is that
per-class?  Or per-tracepoint?

> will trigger a warning when it detects unused tracepoints. The XFS file
> system contains many events that are not called. Most of them used to be called
> but due to code refactoring the calls were removed but the trace events stayed
> behind.
> 
> Some events were added but never used. If they were recent, I just reported
> them, but if they were older, this series simply removes them.

TBH it'd be really nice if there was /something/ in the build path that
would complain about disused tracepoints.  I often put in tracepoints
while developing a feature, then the code gets massively rewritten
during review, and none of us remember to rip out the orphaned traces...

> One is called only when CONFIG_COMPACT is defined, so an #ifdef was placed
> around it.
> 
> A couple are only called in #if 0 code (left as a reminder to fix it), so
> those events are wrapped by a #if 0 as well (with a comment).
> 
> Finally, one event is supposed to be a trace event class, but was created with
> the TRACE_EVENT() macro and not the DECLARE_EVENT_CLASS() macro. This works
> because a TRACE_EVENT() is simply a DECLARE_EVENT_CLASS() and DEFINE_EVENT()
> where the class and event have the same name. But as this was a mistake, the
> event created should not exist.
> 
> Each patch is a stand alone patch. If you ack them, I can take them in my
> tree, or if you want, you can take them. I may be adding the warning code to
> linux-next near the end of the cycle, so it would be good to have this cleaned
> up before hand. As this is removing dead code, it may be even OK to send them
> to Linus as a fix.

...oh, you /do/ have a program and it's coming down the pipeline.
Excellent <tents fingers>. :)

--D

> 
> Steven Rostedt (14):
>       xfs: tracing; Remove unused event xfs_reflink_cow_found
>       xfs: Remove unused trace event xfs_attr_remove_iter_return
>       xfs: Remove unused event xlog_iclog_want_sync
>       xfs: Remove unused event xfs_ioctl_clone
>       xfs: Remove unused xfs_reflink_compare_extents events
>       xfs: Remove unused trace event xfs_attr_rmtval_set
>       xfs: ifdef out unused xfs_attr events
>       xfs: Remove unused event xfs_attr_node_removename
>       xfs: Remove unused event xfs_alloc_near_error
>       xfs: Remove unused event xfs_alloc_near_nominleft
>       xfs: Remove unused event xfs_pagecache_inval
>       xfs: Remove usused xfs_end_io_direct events
>       xfs: Only create event xfs_file_compat_ioctl when CONFIG_COMPAT is configure
>       xfs: Change xfs_xattr_class from a TRACE_EVENT() to DECLARE_EVENT_CLASS()
> 
> ----
>  fs/xfs/scrub/trace.h |  2 +-
>  fs/xfs/xfs_trace.h   | 72 ++++++----------------------------------------------
>  2 files changed, 9 insertions(+), 65 deletions(-)
>
Re: [PATCH 00/14] xfs: Remove unused trace events
Posted by Steven Rostedt 2 months, 2 weeks ago
On Fri, 13 Jun 2025 08:08:55 -0700
"Darrick J. Wong" <djwong@kernel.org> wrote:

> > Each patch is a stand alone patch. If you ack them, I can take them in my
> > tree, or if you want, you can take them. I may be adding the warning code to
> > linux-next near the end of the cycle, so it would be good to have this cleaned
> > up before hand. As this is removing dead code, it may be even OK to send them
> > to Linus as a fix.  
> 
> ...oh, you /do/ have a program and it's coming down the pipeline.
> Excellent <tents fingers>. :)

Well, Linus didn't like its current implementation, so it may be a release
or two or three before that program gets upstream :-/

-- Steve
Re: [PATCH 00/14] xfs: Remove unused trace events
Posted by Steven Rostedt 3 months, 4 weeks ago
On Fri, 13 Jun 2025 08:08:55 -0700
"Darrick J. Wong" <djwong@kernel.org> wrote:

> On Thu, Jun 12, 2025 at 05:24:05PM -0400, Steven Rostedt wrote:
> > 
> > Trace events take up to 5K in memory for text and meta data. I have code that  
> 
> Under what circumstances do they eat up that much memory?  And is that
> per-class?  Or per-tracepoint?

I just did an analysis of this:

  https://lore.kernel.org/lkml/20250613104240.509ff13c@batman.local.home/T/#md81abade0df19ba9062fd51ced4458161f885ac3

A TRACE_EVENT() is about 5K, and each DEFINE_EVENT() is about 1K.

> 
> > will trigger a warning when it detects unused tracepoints. The XFS file
> > system contains many events that are not called. Most of them used to be called
> > but due to code refactoring the calls were removed but the trace events stayed
> > behind.
> > 
> > Some events were added but never used. If they were recent, I just reported
> > them, but if they were older, this series simply removes them.  
> 
> TBH it'd be really nice if there was /something/ in the build path that
> would complain about disused tracepoints.  I often put in tracepoints
> while developing a feature, then the code gets massively rewritten
> during review, and none of us remember to rip out the orphaned traces...
> 

That's exactly what I'm doing. In the thread I did the analysis, it has
code that triggers a warning for unused events. I'm currently cleaning
them up (and asking others to do it too), so that I can add that code
without triggering all the current unused tracepoints.

Feel free to take those patches and it will warn at build time. Note,
it currently only works for built in code, so you have to build your
module as a built in and not a module.

> > One is called only when CONFIG_COMPACT is defined, so an #ifdef was placed
> > around it.
> > 
> > A couple are only called in #if 0 code (left as a reminder to fix it), so
> > those events are wrapped by a #if 0 as well (with a comment).
> > 
> > Finally, one event is supposed to be a trace event class, but was created with
> > the TRACE_EVENT() macro and not the DECLARE_EVENT_CLASS() macro. This works
> > because a TRACE_EVENT() is simply a DECLARE_EVENT_CLASS() and DEFINE_EVENT()
> > where the class and event have the same name. But as this was a mistake, the
> > event created should not exist.
> > 
> > Each patch is a stand alone patch. If you ack them, I can take them in my
> > tree, or if you want, you can take them. I may be adding the warning code to
> > linux-next near the end of the cycle, so it would be good to have this cleaned
> > up before hand. As this is removing dead code, it may be even OK to send them
> > to Linus as a fix.  
> 
> ...oh, you /do/ have a program and it's coming down the pipeline.
> Excellent <tents fingers>. :)

Ah you did notice ;-)

-- Steve
Re: [PATCH 00/14] xfs: Remove unused trace events
Posted by Christoph Hellwig 3 months, 3 weeks ago
On Fri, Jun 13, 2025 at 11:31:19AM -0400, Steven Rostedt wrote:
> On Fri, 13 Jun 2025 08:08:55 -0700
> "Darrick J. Wong" <djwong@kernel.org> wrote:
> 
> > On Thu, Jun 12, 2025 at 05:24:05PM -0400, Steven Rostedt wrote:
> > > 
> > > Trace events take up to 5K in memory for text and meta data. I have code that  
> > 
> > Under what circumstances do they eat up that much memory?  And is that
> > per-class?  Or per-tracepoint?
> 
> I just did an analysis of this:
> 
>   https://lore.kernel.org/lkml/20250613104240.509ff13c@batman.local.home/T/#md81abade0df19ba9062fd51ced4458161f885ac3
> 
> A TRACE_EVENT() is about 5K, and each DEFINE_EVENT() is about 1K.

That's really quite expensive.  And you only measured the tezt/data/bss
overhead and not even the dynamic memory overhead, which is probably
a lot more.
Re: [PATCH 00/14] xfs: Remove unused trace events
Posted by Steven Rostedt 3 months, 3 weeks ago
On Mon, 16 Jun 2025 07:31:19 +0200
Christoph Hellwig <hch@lst.de> wrote:

> > I just did an analysis of this:
> > 
> >   https://lore.kernel.org/lkml/20250613104240.509ff13c@batman.local.home/T/#md81abade0df19ba9062fd51ced4458161f885ac3
> > 
> > A TRACE_EVENT() is about 5K, and each DEFINE_EVENT() is about 1K.  
> 
> That's really quite expensive.  And you only measured the tezt/data/bss

Yes. This is something I've spent a bit of time over the years trying
to address. With moving a bunch of code into trace_event.c with the
added expense that trace events do function calls.

It looks like it's still growing as the last time I checked it was just
under 5K (something around 4800 bytes) and now it's over 5K, and the
tracepoint code grew 4x. I'll start looking into "why" later when I
have more time to deal with this. My time budget for removing unused
events has pretty much dried up.

> overhead and not even the dynamic memory overhead, which is probably
> a lot more.

Yes, and this is another area I look to make better. It was the
motivation for eventfs which saved over 20 megs of memory by having
trace event files dynamically created instead of being permanent.

-- Steve
Re: [PATCH 00/14] xfs: Remove unused trace events
Posted by Christoph Hellwig 3 months, 3 weeks ago
I just went over them, and modulo the nitpicks this looks fine.