[PATCH] xfs: allow disabling xfs tracepoints via Kconfig

[PATCH] xfs: allow disabling xfs tracepoints via Kconfig

[Rasmus Villemoes]

linux/tracepoints.h allows individual subsystems to disable their
tracepoints. Add such a knob for xfs. Disabling XFS_TRACEPOINTS
reduces the resident size of xfs.ko by about a third, or ~350 KiB.

$ size /tmp/xfs/{a,b}/xfs.ko
   text    data     bss     dec     hex filename
 671726    4129     632  676487   a5287 /tmp/xfs/a/xfs.ko
 893192  166737     632 1060561  102ed1 /tmp/xfs/b/xfs.ko

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 fs/xfs/Kconfig     | 9 +++++++++
 fs/xfs/xfs_trace.h | 4 ++++
 2 files changed, 13 insertions(+)

diff --git a/fs/xfs/Kconfig b/fs/xfs/Kconfig
index 457ac9f97377..d099e6d9f6c5 100644
--- a/fs/xfs/Kconfig
+++ b/fs/xfs/Kconfig
@@ -141,3 +141,12 @@ config XFS_ASSERT_FATAL
 	  result in warnings.
 
 	  This behavior can be modified at runtime via sysfs.
+
+config XFS_TRACEPOINTS
+	bool "XFS tracepoints"
+	default y
+	depends on XFS_FS && TRACEPOINTS
+	help
+	  Say Y here to build XFS with tracepoints.
+
+	  You can say N here to reduce the size of the kernel image.
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 6fcc893dfc91..5fb3d5c0483e 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -6,6 +6,10 @@
 #undef TRACE_SYSTEM
 #define TRACE_SYSTEM xfs
 
+#ifndef CONFIG_XFS_TRACEPOINTS
+#define NOTRACE
+#endif
+
 #if !defined(_TRACE_XFS_H) || defined(TRACE_HEADER_MULTI_READ)
 #define _TRACE_XFS_H
 
-- 
2.20.1

Re: [PATCH] xfs: allow disabling xfs tracepoints via Kconfig

[Darrick J. Wong]

On Mon, Feb 04, 2019 at 10:20:35PM +0100, Rasmus Villemoes wrote:
> linux/tracepoints.h allows individual subsystems to disable their
> tracepoints. Add such a knob for xfs. Disabling XFS_TRACEPOINTS
> reduces the resident size of xfs.ko by about a third, or ~350 KiB.

LOL. :)

> $ size /tmp/xfs/{a,b}/xfs.ko
>    text    data     bss     dec     hex filename
>  671726    4129     632  676487   a5287 /tmp/xfs/a/xfs.ko
>  893192  166737     632 1060561  102ed1 /tmp/xfs/b/xfs.ko
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  fs/xfs/Kconfig     | 9 +++++++++
>  fs/xfs/xfs_trace.h | 4 ++++

I think a similar thing ought to apply to fs/xfs/scrub/trace.h, right?

More general questions:

(a) Do we want to be doing this on a per-subsystem basis?

(b) Since xfs_scrub is a separate trace domain, do we want a separate
Kconfig item for toggling scrub trace, or is one big one enough?

(I can't think of why I'd want one on and the other off.)

--D

>  2 files changed, 13 insertions(+)
> 
> diff --git a/fs/xfs/Kconfig b/fs/xfs/Kconfig
> index 457ac9f97377..d099e6d9f6c5 100644
> --- a/fs/xfs/Kconfig
> +++ b/fs/xfs/Kconfig
> @@ -141,3 +141,12 @@ config XFS_ASSERT_FATAL
>  	  result in warnings.
>  
>  	  This behavior can be modified at runtime via sysfs.
> +
> +config XFS_TRACEPOINTS
> +	bool "XFS tracepoints"
> +	default y
> +	depends on XFS_FS && TRACEPOINTS
> +	help
> +	  Say Y here to build XFS with tracepoints.
> +
> +	  You can say N here to reduce the size of the kernel image.
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 6fcc893dfc91..5fb3d5c0483e 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -6,6 +6,10 @@
>  #undef TRACE_SYSTEM
>  #define TRACE_SYSTEM xfs
>  
> +#ifndef CONFIG_XFS_TRACEPOINTS
> +#define NOTRACE
> +#endif
> +
>  #if !defined(_TRACE_XFS_H) || defined(TRACE_HEADER_MULTI_READ)
>  #define _TRACE_XFS_H
>  
> -- 
> 2.20.1
> 

Re: [PATCH] xfs: allow disabling xfs tracepoints via Kconfig

[Dave Chinner]

On Mon, Feb 04, 2019 at 10:20:35PM +0100, Rasmus Villemoes wrote:
> linux/tracepoints.h allows individual subsystems to disable their
> tracepoints. Add such a knob for xfs. Disabling XFS_TRACEPOINTS
> reduces the resident size of xfs.ko by about a third, or ~350 KiB.

Ok, now we can't debug typical problems on live production systems
if tracepoints are turned off on the user/distro kernels.  So under
what circumstances would we ever want to turn off tracepoints on
XFS?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] xfs: allow disabling xfs tracepoints via Kconfig

[Rasmus Villemoes]

On 04/02/2019 22.53, Dave Chinner wrote:
> On Mon, Feb 04, 2019 at 10:20:35PM +0100, Rasmus Villemoes wrote:
>> linux/tracepoints.h allows individual subsystems to disable their
>> tracepoints. Add such a knob for xfs. Disabling XFS_TRACEPOINTS
>> reduces the resident size of xfs.ko by about a third, or ~350 KiB.
> 
> Ok, now we can't debug typical problems on live production systems
> if tracepoints are turned off on the user/distro kernels.  So under
> what circumstances would we ever want to turn off tracepoints on
> XFS?

I don't expect any mainstream distros to turn it off. But for embedded
systems that use a hand-tuned .config, being able to shave off 100s of K
of the kernel image is quite valuable. Tracing _is_ useful,
also/especially when doing embedded development, which is why "just turn
off CONFIG_TRACING" isn't really an option.

Would the knob be more acceptable if it was under CONFIG_EXPERT?

Rasmus

Re: [PATCH] xfs: allow disabling xfs tracepoints via Kconfig

[Dave Chinner]

On Mon, Feb 04, 2019 at 11:12:57PM +0100, Rasmus Villemoes wrote:
> On 04/02/2019 22.53, Dave Chinner wrote:
> > On Mon, Feb 04, 2019 at 10:20:35PM +0100, Rasmus Villemoes wrote:
> >> linux/tracepoints.h allows individual subsystems to disable their
> >> tracepoints. Add such a knob for xfs. Disabling XFS_TRACEPOINTS
> >> reduces the resident size of xfs.ko by about a third, or ~350 KiB.
> > 
> > Ok, now we can't debug typical problems on live production systems
> > if tracepoints are turned off on the user/distro kernels.  So under
> > what circumstances would we ever want to turn off tracepoints on
> > XFS?
> 
> I don't expect any mainstream distros to turn it off. But for embedded
> systems that use a hand-tuned .config, being able to shave off 100s of K
> of the kernel image is quite valuable.

However, small embedded systems don't use XFS because of it's size,
dependence on 64 bit capability (ie. 64BIT || LBDAF), its CPU and
memory overhead in comparison to other filesystems, etc. 

Increasing kconfig options increases the test matrix (even just the
"does this changeset compile" matrix) so these things don't come for
free.  XFS is firmly focussed on the other end of the scale (big,
lots, fast), so I'm not sure that increasing the kconfig combination
matrix to cater for really low end embedded systems is really that
worthwhile.

> Tracing _is_ useful,
> also/especially when doing embedded development, which is why "just turn
> off CONFIG_TRACING" isn't really an option.

Do you have a specific need for this, or is it just "I noticed this,
here's a patch"? If you need it and will use it, great, let's add
it. But config options that don't ever get used tend to rot....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] xfs: allow disabling xfs tracepoints via Kconfig

[Rasmus Villemoes]

On 05/02/2019 00.13, Dave Chinner wrote:
> On Mon, Feb 04, 2019 at 11:12:57PM +0100, Rasmus Villemoes wrote:
>> On 04/02/2019 22.53, Dave Chinner wrote:
>>> On Mon, Feb 04, 2019 at 10:20:35PM +0100, Rasmus Villemoes wrote:
>>>> linux/tracepoints.h allows individual subsystems to disable their
>>>> tracepoints. Add such a knob for xfs. Disabling XFS_TRACEPOINTS
>>>> reduces the resident size of xfs.ko by about a third, or ~350 KiB.
> 
> Do you have a specific need for this, or is it just "I noticed this,
> here's a patch"? 

A little of both, actually. I noticed it, did the patch for the kernel I
use on my laptop, and thought others might find it useful. But no, I
don't currently deal with any customer boards where it'd be relevant.

> If you need it and will use it, great, let's add
> it. But config options that don't ever get used tend to rot....

Fair enough, I'll just keep it privately and resend if I encounter a use
case in the wild.

Rasmus