2015-06-20 20:21:55

by Tal Shorer

[permalink] [raw]
Subject: [Patch RFC 0/2] tracing: allow disabling compilation of specific trace systems

Currently, enabling CONFIG_TRACING on a system comes as all-or-nothing: either
tracepoints for all subsystems are compiled (with CONFIG_TRACING) or none of
them do (without it).

This caused me an unacceptable performance penalty (obviously SOME penalty was
expected, but not one so severe) which made me revert the changes in
configuration.

The first patch in this series modifies the files that actually define the
tracepoint to look for a preprocessor macro NOTRACE and define nops (as if
CONFIG_TRACING was not set) instead of them.

The second patch provides an example of how I see this working, with the gpio
subsystem as the example for absolutely no reason.
If this idea is deemed worth the time by the community, I'll create patches for
the other subsystems.

Tal Shorer (2):
tracing: allow disabling compilation of specific trace systems
tracing: gpio: add Kconfig option for enabling/disabling trace events

drivers/gpio/Kconfig | 6 ++++++
include/linux/tracepoint.h | 6 +++---
include/trace/define_trace.h | 2 +-
include/trace/events/gpio.h | 4 ++++
4 files changed, 14 insertions(+), 4 deletions(-)

--
2.2.2


2015-06-20 20:22:07

by Tal Shorer

[permalink] [raw]
Subject: [Patch RFC 1/2] tracing: allow disabling compilation of specific trace systems

Allow a trace events header file to disable compilation of its
trace events by defining the preprocessor macro NOTRACE.

This could be done, for example, according to a Kconfig option.

Signed-off-by: Tal Shorer <[email protected]>
---
include/linux/tracepoint.h | 6 +++---
include/trace/define_trace.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index a5f7f3e..c869f84 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -111,7 +111,7 @@ extern void syscall_unregfunc(void);
#define TP_ARGS(args...) args
#define TP_CONDITION(args...) args

-#ifdef CONFIG_TRACEPOINTS
+#if defined(CONFIG_TRACEPOINTS) && !defined(NOTRACE)

/*
* it_func[0] is never NULL because there is at least one element in the array
@@ -234,7 +234,7 @@ extern void syscall_unregfunc(void);
#define EXPORT_TRACEPOINT_SYMBOL(name) \
EXPORT_SYMBOL(__tracepoint_##name)

-#else /* !CONFIG_TRACEPOINTS */
+#else /* !(defined(CONFIG_TRACEPOINTS) && !defined(NOTRACE)) */
#define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \
static inline void trace_##name(proto) \
{ } \
@@ -266,7 +266,7 @@ extern void syscall_unregfunc(void);
#define EXPORT_TRACEPOINT_SYMBOL_GPL(name)
#define EXPORT_TRACEPOINT_SYMBOL(name)

-#endif /* CONFIG_TRACEPOINTS */
+#endif /* defined(CONFIG_TRACEPOINTS) && !defined(NOTRACE) */

#ifdef CONFIG_TRACING
/**
diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
index 02e1003..e847fd7 100644
--- a/include/trace/define_trace.h
+++ b/include/trace/define_trace.h
@@ -86,7 +86,7 @@
#undef DECLARE_TRACE
#define DECLARE_TRACE(name, proto, args)

-#ifdef CONFIG_EVENT_TRACING
+#if defined(CONFIG_EVENT_TRACING) && !defined(NOTRACE)
#include <trace/ftrace.h>
#endif

--
2.2.2

2015-06-20 20:22:17

by Tal Shorer

[permalink] [raw]
Subject: [Patch RFC 2/2] tracing: gpio: add Kconfig option for enabling/disabling trace events

Add a new options to gpio Kconfig, CONFIG_GPIO_TRACING, that is used
for enabling/disabling compilation of gpio function trace events.

Signed-off-by: Tal Shorer <[email protected]>
---
drivers/gpio/Kconfig | 6 ++++++
include/trace/events/gpio.h | 4 ++++
2 files changed, 10 insertions(+)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index caefe80..1480a22 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -88,6 +88,12 @@ config GPIO_SYSFS
config GPIO_GENERIC
tristate

+config GPIO_TRACING
+ bool "gpio tracing"
+ depends on TRACING
+ help
+ Enable tracing for gpio subsystem
+
# put drivers in the right section, in alphabetical order

# This symbol is selected by both I2C and SPI expanders
diff --git a/include/trace/events/gpio.h b/include/trace/events/gpio.h
index 927a8ad..09af636 100644
--- a/include/trace/events/gpio.h
+++ b/include/trace/events/gpio.h
@@ -1,6 +1,10 @@
#undef TRACE_SYSTEM
#define TRACE_SYSTEM gpio

+#ifndef CONFIG_GPIO_TRACING
+#define NOTRACE
+#endif
+
#if !defined(_TRACE_GPIO_H) || defined(TRACE_HEADER_MULTI_READ)
#define _TRACE_GPIO_H

--
2.2.2

2015-06-25 20:41:27

by Tal Shorer

[permalink] [raw]
Subject: Re: [Patch RFC 0/2] tracing: allow disabling compilation of specific trace systems

ping?

On Sat, Jun 20, 2015 at 11:21 PM, Tal Shorer <[email protected]> wrote:
> Currently, enabling CONFIG_TRACING on a system comes as all-or-nothing: either
> tracepoints for all subsystems are compiled (with CONFIG_TRACING) or none of
> them do (without it).
>
> This caused me an unacceptable performance penalty (obviously SOME penalty was
> expected, but not one so severe) which made me revert the changes in
> configuration.
>
> The first patch in this series modifies the files that actually define the
> tracepoint to look for a preprocessor macro NOTRACE and define nops (as if
> CONFIG_TRACING was not set) instead of them.
>
> The second patch provides an example of how I see this working, with the gpio
> subsystem as the example for absolutely no reason.
> If this idea is deemed worth the time by the community, I'll create patches for
> the other subsystems.
>
> Tal Shorer (2):
> tracing: allow disabling compilation of specific trace systems
> tracing: gpio: add Kconfig option for enabling/disabling trace events
>
> drivers/gpio/Kconfig | 6 ++++++
> include/linux/tracepoint.h | 6 +++---
> include/trace/define_trace.h | 2 +-
> include/trace/events/gpio.h | 4 ++++
> 4 files changed, 14 insertions(+), 4 deletions(-)
>
> --
> 2.2.2
>

2015-06-25 22:25:41

by Steven Rostedt

[permalink] [raw]
Subject: Re: [Patch RFC 0/2] tracing: allow disabling compilation of specific trace systems

On Thu, 25 Jun 2015 23:41:17 +0300
Tal Shorer <[email protected]> wrote:

> ping?

I'm planning on looking at this after I've finished everything for the
merge window. This came over the weekend (always a bad time), and the
merge window opened. All new code needs to take a backseat while the
merge window is opened.

I may not get to this till next week. Feel free to ping me again then.

-- Steve


>
> On Sat, Jun 20, 2015 at 11:21 PM, Tal Shorer <[email protected]> wrote:
> > Currently, enabling CONFIG_TRACING on a system comes as all-or-nothing: either
> > tracepoints for all subsystems are compiled (with CONFIG_TRACING) or none of
> > them do (without it).
> >
> > This caused me an unacceptable performance penalty (obviously SOME penalty was
> > expected, but not one so severe) which made me revert the changes in
> > configuration.
> >
> > The first patch in this series modifies the files that actually define the
> > tracepoint to look for a preprocessor macro NOTRACE and define nops (as if
> > CONFIG_TRACING was not set) instead of them.
> >
> > The second patch provides an example of how I see this working, with the gpio
> > subsystem as the example for absolutely no reason.
> > If this idea is deemed worth the time by the community, I'll create patches for
> > the other subsystems.
> >
> > Tal Shorer (2):
> > tracing: allow disabling compilation of specific trace systems
> > tracing: gpio: add Kconfig option for enabling/disabling trace events
> >
> > drivers/gpio/Kconfig | 6 ++++++
> > include/linux/tracepoint.h | 6 +++---
> > include/trace/define_trace.h | 2 +-
> > include/trace/events/gpio.h | 4 ++++
> > 4 files changed, 14 insertions(+), 4 deletions(-)
> >
> > --
> > 2.2.2
> >

2015-07-06 16:53:33

by Tal Shorer

[permalink] [raw]
Subject: Re: [Patch RFC 0/2] tracing: allow disabling compilation of specific trace systems

On Fri, Jun 26, 2015 at 1:25 AM, Steven Rostedt <[email protected]> wrote:
> On Thu, 25 Jun 2015 23:41:17 +0300
> Tal Shorer <[email protected]> wrote:
>
>> ping?
>
> I'm planning on looking at this after I've finished everything for the
> merge window. This came over the weekend (always a bad time), and the
> merge window opened. All new code needs to take a backseat while the
> merge window is opened.
>
> I may not get to this till next week. Feel free to ping me again then.
ping
>
> -- Steve
>
>
>>
>> On Sat, Jun 20, 2015 at 11:21 PM, Tal Shorer <[email protected]> wrote:
>> > Currently, enabling CONFIG_TRACING on a system comes as all-or-nothing: either
>> > tracepoints for all subsystems are compiled (with CONFIG_TRACING) or none of
>> > them do (without it).
>> >
>> > This caused me an unacceptable performance penalty (obviously SOME penalty was
>> > expected, but not one so severe) which made me revert the changes in
>> > configuration.
>> >
>> > The first patch in this series modifies the files that actually define the
>> > tracepoint to look for a preprocessor macro NOTRACE and define nops (as if
>> > CONFIG_TRACING was not set) instead of them.
>> >
>> > The second patch provides an example of how I see this working, with the gpio
>> > subsystem as the example for absolutely no reason.
>> > If this idea is deemed worth the time by the community, I'll create patches for
>> > the other subsystems.
>> >
>> > Tal Shorer (2):
>> > tracing: allow disabling compilation of specific trace systems
>> > tracing: gpio: add Kconfig option for enabling/disabling trace events
>> >
>> > drivers/gpio/Kconfig | 6 ++++++
>> > include/linux/tracepoint.h | 6 +++---
>> > include/trace/define_trace.h | 2 +-
>> > include/trace/events/gpio.h | 4 ++++
>> > 4 files changed, 14 insertions(+), 4 deletions(-)
>> >
>> > --
>> > 2.2.2
>> >
>

2015-07-13 18:35:04

by Steven Rostedt

[permalink] [raw]
Subject: Re: [Patch RFC 1/2] tracing: allow disabling compilation of specific trace systems

On Sat, 20 Jun 2015 23:21:18 +0300
Tal Shorer <[email protected]> wrote:

> Allow a trace events header file to disable compilation of its
> trace events by defining the preprocessor macro NOTRACE.
>
> This could be done, for example, according to a Kconfig option.
>
> Signed-off-by: Tal Shorer <[email protected]>
> ---
> include/linux/tracepoint.h | 6 +++---
> include/trace/define_trace.h | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index a5f7f3e..c869f84 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -111,7 +111,7 @@ extern void syscall_unregfunc(void);
> #define TP_ARGS(args...) args
> #define TP_CONDITION(args...) args
>
> -#ifdef CONFIG_TRACEPOINTS
> +#if defined(CONFIG_TRACEPOINTS) && !defined(NOTRACE)

Instead of the duplicate condition above, it would be better to make a
new macro at the top. And we can add a nice comment to it as well.

/*
* Individual subsystem my have a separate configuration to
* enable their tracepoints. By default, this file will create
* the tracepoints if CONFIG_TRACEPOINT is defined. If a subsystem
* wants to be able to disable its tracepoints from being created
* it can define NOTRACE before including the tracepoint headers.
*/
#if defined(CONFIG_TRACEPOINTS) && !defined(NOTRACE)
# define TRACEPOINTS_ENABLED
#endif

Then switch all the conditions below to:

#ifdef TRACEPOINTS_ENABLED

-- Steve


>
> /*
> * it_func[0] is never NULL because there is at least one element in the array
> @@ -234,7 +234,7 @@ extern void syscall_unregfunc(void);
> #define EXPORT_TRACEPOINT_SYMBOL(name) \
> EXPORT_SYMBOL(__tracepoint_##name)
>
> -#else /* !CONFIG_TRACEPOINTS */
> +#else /* !(defined(CONFIG_TRACEPOINTS) && !defined(NOTRACE)) */
> #define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \
> static inline void trace_##name(proto) \
> { } \
> @@ -266,7 +266,7 @@ extern void syscall_unregfunc(void);
> #define EXPORT_TRACEPOINT_SYMBOL_GPL(name)
> #define EXPORT_TRACEPOINT_SYMBOL(name)
>
> -#endif /* CONFIG_TRACEPOINTS */
> +#endif /* defined(CONFIG_TRACEPOINTS) && !defined(NOTRACE) */
>
> #ifdef CONFIG_TRACING
> /**
> diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
> index 02e1003..e847fd7 100644
> --- a/include/trace/define_trace.h
> +++ b/include/trace/define_trace.h
> @@ -86,7 +86,7 @@
> #undef DECLARE_TRACE
> #define DECLARE_TRACE(name, proto, args)
>
> -#ifdef CONFIG_EVENT_TRACING
> +#if defined(CONFIG_EVENT_TRACING) && !defined(NOTRACE)
> #include <trace/ftrace.h>
> #endif
>

2015-07-13 21:38:13

by Tal Shorer

[permalink] [raw]
Subject: Re: [Patch RFC 1/2] tracing: allow disabling compilation of specific trace systems

On Mon, Jul 13, 2015 at 9:34 PM, Steven Rostedt <[email protected]> wrote:
> On Sat, 20 Jun 2015 23:21:18 +0300
> Tal Shorer <[email protected]> wrote:
>
>> Allow a trace events header file to disable compilation of its
>> trace events by defining the preprocessor macro NOTRACE.
>>
>> This could be done, for example, according to a Kconfig option.
>>
>> Signed-off-by: Tal Shorer <[email protected]>
>> ---
>> include/linux/tracepoint.h | 6 +++---
>> include/trace/define_trace.h | 2 +-
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
>> index a5f7f3e..c869f84 100644
>> --- a/include/linux/tracepoint.h
>> +++ b/include/linux/tracepoint.h
>> @@ -111,7 +111,7 @@ extern void syscall_unregfunc(void);
>> #define TP_ARGS(args...) args
>> #define TP_CONDITION(args...) args
>>
>> -#ifdef CONFIG_TRACEPOINTS
>> +#if defined(CONFIG_TRACEPOINTS) && !defined(NOTRACE)
>
> Instead of the duplicate condition above, it would be better to make a
> new macro at the top. And we can add a nice comment to it as well.
Technically they're a little different, one with CONFIG_TRACEPOINTS
and the other with CONFIG_EVENT_TRACING.
Looking briefly at the Kconfig files, it appears you can't really have
CONFIG_TRACEPOINTS without also getting CONFIG_EVENT_TRACING along the
way.
If it's fine with you to effectively change the check for
CONFIG_EVENT_TRACING with a check for CONFIG_TRACEPOINTS (or vice
versa, let me know if you have a preference), it's fine with me and
will be added in v2.
>
> /*
> * Individual subsystem my have a separate configuration to
> * enable their tracepoints. By default, this file will create
> * the tracepoints if CONFIG_TRACEPOINT is defined. If a subsystem
> * wants to be able to disable its tracepoints from being created
> * it can define NOTRACE before including the tracepoint headers.
> */
Will be added in v2. I believe I'll have time to write and test it
during the week.
> #if defined(CONFIG_TRACEPOINTS) && !defined(NOTRACE)
> # define TRACEPOINTS_ENABLED
> #endif
>
> Then switch all the conditions below to:
>
> #ifdef TRACEPOINTS_ENABLED
>
> -- Steve
>
>
>>
>> /*
>> * it_func[0] is never NULL because there is at least one element in the array
>> @@ -234,7 +234,7 @@ extern void syscall_unregfunc(void);
>> #define EXPORT_TRACEPOINT_SYMBOL(name) \
>> EXPORT_SYMBOL(__tracepoint_##name)
>>
>> -#else /* !CONFIG_TRACEPOINTS */
>> +#else /* !(defined(CONFIG_TRACEPOINTS) && !defined(NOTRACE)) */
>> #define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \
>> static inline void trace_##name(proto) \
>> { } \
>> @@ -266,7 +266,7 @@ extern void syscall_unregfunc(void);
>> #define EXPORT_TRACEPOINT_SYMBOL_GPL(name)
>> #define EXPORT_TRACEPOINT_SYMBOL(name)
>>
>> -#endif /* CONFIG_TRACEPOINTS */
>> +#endif /* defined(CONFIG_TRACEPOINTS) && !defined(NOTRACE) */
>>
>> #ifdef CONFIG_TRACING
>> /**
>> diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
>> index 02e1003..e847fd7 100644
>> --- a/include/trace/define_trace.h
>> +++ b/include/trace/define_trace.h
>> @@ -86,7 +86,7 @@
>> #undef DECLARE_TRACE
>> #define DECLARE_TRACE(name, proto, args)
>>
>> -#ifdef CONFIG_EVENT_TRACING
>> +#if defined(CONFIG_EVENT_TRACING) && !defined(NOTRACE)
>> #include <trace/ftrace.h>
>> #endif
>>
>

2015-07-16 17:39:53

by Tal Shorer

[permalink] [raw]
Subject: [PATCH v2 0/2] tracing: allow disabling compilation of specific trace systems

Currently, enabling CONFIG_TRACING on a system comes as all-or-nothing: either
tracepoints for all subsystems are compiled (with CONFIG_TRACING) or none of
them do (without it).

This caused me an unacceptable performance penalty (obviously SOME penalty was
expected, but not one so severe) which made me revert the changes in
configuration.

The first patch in this series modifies the files that actually define the
tracepoint to look for a preprocessor macro NOTRACE and define nops (as if
CONFIG_TRACING was not set) instead of them.

The second patch provides an example of how I see this working, with the gpio
subsystem as the example for absolutely no reason.
If this idea is deemed worth the time by the community, I'll create patches for
the other subsystems.



Changelog:

v2:
- A comment in tracepoint.h explaining NOTRACE and its use
- Avoid duplication of the test for both NOTRACE and a config option by
defining TRACEPOINTS_ENABLED when both are present and using that to check
whether or not to define tracepoints

Tal Shorer (2):
tracing: allow disabling compilation of specific trace systems
tracing: gpio: add Kconfig option for enabling/disabling trace events

drivers/gpio/Kconfig | 7 +++++++
include/linux/tracepoint.h | 6 +++---
include/trace/define_trace.h | 2 +-
include/trace/events/gpio.h | 4 ++++
4 files changed, 15 insertions(+), 4 deletions(-)

--
2.4.3

2015-07-16 17:40:07

by Tal Shorer

[permalink] [raw]
Subject: [PATCH v2 1/2] tracing: allow disabling compilation of specific trace systems

Allow a trace events header file to disable compilation of its
trace events by defining the preprocessor macro NOTRACE.

This could be done, for example, according to a Kconfig option.

Signed-off-by: Tal Shorer <[email protected]>
---
include/linux/tracepoint.h | 17 ++++++++++++++---
include/trace/define_trace.h | 2 +-
2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index c728513..a8ddfd3 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -103,7 +103,18 @@ extern void syscall_unregfunc(void);
#define TP_ARGS(args...) args
#define TP_CONDITION(args...) args

-#ifdef CONFIG_TRACEPOINTS
+/*
+ * Individual subsystem my have a separate configuration to
+ * enable their tracepoints. By default, this file will create
+ * the tracepoints if CONFIG_TRACEPOINT is defined. If a subsystem
+ * wants to be able to disable its tracepoints from being created
+ * it can define NOTRACE before including the tracepoint headers.
+ */
+#if defined(CONFIG_TRACEPOINTS) && !defined(NOTRACE)
+#define TRACEPOINTS_ENABLED
+#endif
+
+#ifdef TRACEPOINTS_ENABLED

/*
* it_func[0] is never NULL because there is at least one element in the array
@@ -226,7 +237,7 @@ extern void syscall_unregfunc(void);
#define EXPORT_TRACEPOINT_SYMBOL(name) \
EXPORT_SYMBOL(__tracepoint_##name)

-#else /* !CONFIG_TRACEPOINTS */
+#else /* !TRACEPOINTS_ENABLED */
#define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \
static inline void trace_##name(proto) \
{ } \
@@ -258,7 +269,7 @@ extern void syscall_unregfunc(void);
#define EXPORT_TRACEPOINT_SYMBOL_GPL(name)
#define EXPORT_TRACEPOINT_SYMBOL(name)

-#endif /* CONFIG_TRACEPOINTS */
+#endif /* TRACEPOINTS_ENABLED */

#ifdef CONFIG_TRACING
/**
diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
index 02e1003..b763d3f 100644
--- a/include/trace/define_trace.h
+++ b/include/trace/define_trace.h
@@ -86,7 +86,7 @@
#undef DECLARE_TRACE
#define DECLARE_TRACE(name, proto, args)

-#ifdef CONFIG_EVENT_TRACING
+#ifdef TRACEPOINTS_ENABLED
#include <trace/ftrace.h>
#endif

--
2.4.3

2015-07-16 17:41:09

by Tal Shorer

[permalink] [raw]
Subject: [PATCH v2 2/2] tracing: gpio: add Kconfig option for enabling/disabling trace events

Add a new options to gpio Kconfig, CONFIG_GPIO_TRACING, that is used
for enabling/disabling compilation of gpio function trace events.

Signed-off-by: Tal Shorer <[email protected]>
---
drivers/gpio/Kconfig | 7 +++++++
include/trace/events/gpio.h | 4 ++++
2 files changed, 11 insertions(+)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index c1e2ca3..2829e8e 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -88,6 +88,13 @@ config GPIO_SYSFS
config GPIO_GENERIC
tristate

+config GPIO_TRACING
+ bool "gpio tracing"
+ depends on TRACING
+ help
+ Enable tracing for gpio subsystem
+
+
# put drivers in the right section, in alphabetical order

config GPIO_DA9052
diff --git a/include/trace/events/gpio.h b/include/trace/events/gpio.h
index 927a8ad..09af636 100644
--- a/include/trace/events/gpio.h
+++ b/include/trace/events/gpio.h
@@ -1,6 +1,10 @@
#undef TRACE_SYSTEM
#define TRACE_SYSTEM gpio

+#ifndef CONFIG_GPIO_TRACING
+#define NOTRACE
+#endif
+
#if !defined(_TRACE_GPIO_H) || defined(TRACE_HEADER_MULTI_READ)
#define _TRACE_GPIO_H

--
2.4.3

2015-07-17 18:36:10

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] tracing: gpio: add Kconfig option for enabling/disabling trace events

On Thu, 16 Jul 2015 20:39:36 +0300
Tal Shorer <[email protected]> wrote:

> Add a new options to gpio Kconfig, CONFIG_GPIO_TRACING, that is used
> for enabling/disabling compilation of gpio function trace events.
>

If I can get acks from the gpio maintainers, I can take this in my
tree, as it depends on code that modifies the core tracing facility.

-- Steve

> Signed-off-by: Tal Shorer <[email protected]>
> ---
> drivers/gpio/Kconfig | 7 +++++++
> include/trace/events/gpio.h | 4 ++++
> 2 files changed, 11 insertions(+)
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index c1e2ca3..2829e8e 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -88,6 +88,13 @@ config GPIO_SYSFS
> config GPIO_GENERIC
> tristate
>
> +config GPIO_TRACING
> + bool "gpio tracing"
> + depends on TRACING
> + help
> + Enable tracing for gpio subsystem
> +
> +
> # put drivers in the right section, in alphabetical order
>
> config GPIO_DA9052
> diff --git a/include/trace/events/gpio.h b/include/trace/events/gpio.h
> index 927a8ad..09af636 100644
> --- a/include/trace/events/gpio.h
> +++ b/include/trace/events/gpio.h
> @@ -1,6 +1,10 @@
> #undef TRACE_SYSTEM
> #define TRACE_SYSTEM gpio
>
> +#ifndef CONFIG_GPIO_TRACING
> +#define NOTRACE
> +#endif
> +
> #if !defined(_TRACE_GPIO_H) || defined(TRACE_HEADER_MULTI_READ)
> #define _TRACE_GPIO_H
>

2015-07-18 10:12:57

by Tal Shorer

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] tracing: gpio: add Kconfig option for enabling/disabling trace events

On second thought, I wonder if the config option shouldn't be in
kernel/trace/Kconfig
Imagine there's an option for every subsystem. Disabling all of them
but the one you actually care about is going to be a mess if they're
scattered all over the place.
Your thoughts?

On Fri, Jul 17, 2015 at 9:36 PM, Steven Rostedt <[email protected]> wrote:
> On Thu, 16 Jul 2015 20:39:36 +0300
> Tal Shorer <[email protected]> wrote:
>
>> Add a new options to gpio Kconfig, CONFIG_GPIO_TRACING, that is used
>> for enabling/disabling compilation of gpio function trace events.
>>
>
> If I can get acks from the gpio maintainers, I can take this in my
> tree, as it depends on code that modifies the core tracing facility.
>
> -- Steve
>
>> Signed-off-by: Tal Shorer <[email protected]>
>> ---
>> drivers/gpio/Kconfig | 7 +++++++
>> include/trace/events/gpio.h | 4 ++++
>> 2 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
>> index c1e2ca3..2829e8e 100644
>> --- a/drivers/gpio/Kconfig
>> +++ b/drivers/gpio/Kconfig
>> @@ -88,6 +88,13 @@ config GPIO_SYSFS
>> config GPIO_GENERIC
>> tristate
>>
>> +config GPIO_TRACING
>> + bool "gpio tracing"
>> + depends on TRACING
>> + help
>> + Enable tracing for gpio subsystem
>> +
>> +
>> # put drivers in the right section, in alphabetical order
>>
>> config GPIO_DA9052
>> diff --git a/include/trace/events/gpio.h b/include/trace/events/gpio.h
>> index 927a8ad..09af636 100644
>> --- a/include/trace/events/gpio.h
>> +++ b/include/trace/events/gpio.h
>> @@ -1,6 +1,10 @@
>> #undef TRACE_SYSTEM
>> #define TRACE_SYSTEM gpio
>>
>> +#ifndef CONFIG_GPIO_TRACING
>> +#define NOTRACE
>> +#endif
>> +
>> #if !defined(_TRACE_GPIO_H) || defined(TRACE_HEADER_MULTI_READ)
>> #define _TRACE_GPIO_H
>>
>

2015-07-27 11:57:52

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] tracing: gpio: add Kconfig option for enabling/disabling trace events

On Fri, Jul 17, 2015 at 8:36 PM, Steven Rostedt <[email protected]> wrote:
> On Thu, 16 Jul 2015 20:39:36 +0300
> Tal Shorer <[email protected]> wrote:
>
>> Add a new options to gpio Kconfig, CONFIG_GPIO_TRACING, that is used
>> for enabling/disabling compilation of gpio function trace events.
>>
>
> If I can get acks from the gpio maintainers, I can take this in my
> tree, as it depends on code that modifies the core tracing facility.

What works for the tracing maintainer works for me.
Acked-by: Linus Walleij <[email protected]>

Now you can decide if you want this or not... there are some
comments from Tal Shorer I see.

Yours,
Linus Walleij

2015-08-01 12:28:32

by Tal Shorer

[permalink] [raw]
Subject: [Patch v3 0/2] tracing: allow disabling compilation of specific trace systems

Currently, enabling CONFIG_TRACING on a system comes as all-or-nothing:
either tracepoints for all subsystems are compiled (with CONFIG_TRACING)
or none of them are (without it).

This caused me an unacceptable performance penalty (obviously SOME penalty
was expected, but not one so severe) which made me revert the changes in
configuration.

The first patch in this series modifies the files that actually define the
tracepoint to look for a preprocessor macro NOTRACE and define nops (as if
CONFIG_TRACING was not set) instead of them.

The second patch provides an example of how I see this working, with the
gpio subsystem as the example for absolutely no reason.



Changelog:

v2:
- A comment in tracepoint.h explaining NOTRACE and its use
- Avoid duplication of the test for both NOTRACE and a config option by
defining TRACEPOINTS_ENABLED when both are present and using that to check
whether or not to define tracepoints

v3:
- Fix Patch 1/2 to apply to changes in include/trace/define_trace.h
- Add Linus Walleij's ack to Patch 2/2
- Move the Kconfig option from drivers/gpio/Kconfig to kernel/trace/Kconfig
This was done to group up all TRACING_EVENTS_* options together in the
future to ease disabling everything you don't need.

Tal Shorer (2):
tracing: allow disabling compilation of specific trace systems
tracing: gpio: add Kconfig option for enabling/disabling trace events

include/linux/tracepoint.h | 17 ++++++++++++++---
include/trace/define_trace.h | 2 +-
include/trace/events/gpio.h | 4 ++++
kernel/trace/Kconfig | 7 +++++++
4 files changed, 26 insertions(+), 4 deletions(-)

--
2.4.6

2015-08-01 12:28:52

by Tal Shorer

[permalink] [raw]
Subject: [Patch v3 1/2] tracing: allow disabling compilation of specific trace systems

Allow a trace events header file to disable compilation of its
trace events by defining the preprocessor macro NOTRACE.

This could be done, for example, according to a Kconfig option.

Signed-off-by: Tal Shorer <[email protected]>
---
include/linux/tracepoint.h | 17 ++++++++++++++---
include/trace/define_trace.h | 2 +-
2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index a5f7f3e..afada36 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -111,7 +111,18 @@ extern void syscall_unregfunc(void);
#define TP_ARGS(args...) args
#define TP_CONDITION(args...) args

-#ifdef CONFIG_TRACEPOINTS
+/*
+ * Individual subsystem my have a separate configuration to
+ * enable their tracepoints. By default, this file will create
+ * the tracepoints if CONFIG_TRACEPOINT is defined. If a subsystem
+ * wants to be able to disable its tracepoints from being created
+ * it can define NOTRACE before including the tracepoint headers.
+ */
+#if defined(CONFIG_TRACEPOINTS) && !defined(NOTRACE)
+#define TRACEPOINTS_ENABLED
+#endif
+
+#ifdef TRACEPOINTS_ENABLED

/*
* it_func[0] is never NULL because there is at least one element in the array
@@ -234,7 +245,7 @@ extern void syscall_unregfunc(void);
#define EXPORT_TRACEPOINT_SYMBOL(name) \
EXPORT_SYMBOL(__tracepoint_##name)

-#else /* !CONFIG_TRACEPOINTS */
+#else /* !TRACEPOINTS_ENABLED */
#define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \
static inline void trace_##name(proto) \
{ } \
@@ -266,7 +277,7 @@ extern void syscall_unregfunc(void);
#define EXPORT_TRACEPOINT_SYMBOL_GPL(name)
#define EXPORT_TRACEPOINT_SYMBOL(name)

-#endif /* CONFIG_TRACEPOINTS */
+#endif /* TRACEPOINTS_ENABLED */

#ifdef CONFIG_TRACING
/**
diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
index 09b3880..2d8639e 100644
--- a/include/trace/define_trace.h
+++ b/include/trace/define_trace.h
@@ -86,7 +86,7 @@
#undef DECLARE_TRACE
#define DECLARE_TRACE(name, proto, args)

-#ifdef CONFIG_EVENT_TRACING
+#ifdef TRACEPOINTS_ENABLED
#include <trace/trace_events.h>
#include <trace/perf.h>
#endif
--
2.4.6

2015-08-01 12:29:00

by Tal Shorer

[permalink] [raw]
Subject: [Patch v3 2/2] tracing: gpio: add Kconfig option for enabling/disabling trace events

Add a new options to trace Kconfig, CONFIG_TRACING_EVENTS_GPIO, that is
used for enabling/disabling compilation of gpio function trace events.

Signed-off-by: Tal Shorer <[email protected]>
Acked-by: Linus Walleij <[email protected]>
---
include/trace/events/gpio.h | 4 ++++
kernel/trace/Kconfig | 7 +++++++
2 files changed, 11 insertions(+)

diff --git a/include/trace/events/gpio.h b/include/trace/events/gpio.h
index 927a8ad..2da73b9 100644
--- a/include/trace/events/gpio.h
+++ b/include/trace/events/gpio.h
@@ -1,6 +1,10 @@
#undef TRACE_SYSTEM
#define TRACE_SYSTEM gpio

+#ifndef CONFIG_TRACING_EVENTS_GPIO
+#define NOTRACE
+#endif
+
#if !defined(_TRACE_GPIO_H) || defined(TRACE_HEADER_MULTI_READ)
#define _TRACE_GPIO_H

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 3b9a48a..67a1731 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -635,6 +635,13 @@ config TRACE_ENUM_MAP_FILE

If unsure, say N

+config TRACING_EVENTS_GPIO
+ bool "Trace gpio events"
+ depends on GPIOLIB
+ default y
+ help
+ Enable tracing events for gpio subsystem
+
endif # FTRACE

endif # TRACING_SUPPORT
--
2.4.6

2015-08-11 19:45:37

by Tal Shorer

[permalink] [raw]
Subject: Re: [Patch v3 2/2] tracing: gpio: add Kconfig option for enabling/disabling trace events

ping?

On Sat, Aug 1, 2015 at 3:27 PM, Tal Shorer <[email protected]> wrote:
> Add a new options to trace Kconfig, CONFIG_TRACING_EVENTS_GPIO, that is
> used for enabling/disabling compilation of gpio function trace events.
>
> Signed-off-by: Tal Shorer <[email protected]>
> Acked-by: Linus Walleij <[email protected]>
> ---
> include/trace/events/gpio.h | 4 ++++
> kernel/trace/Kconfig | 7 +++++++
> 2 files changed, 11 insertions(+)
>
> diff --git a/include/trace/events/gpio.h b/include/trace/events/gpio.h
> index 927a8ad..2da73b9 100644
> --- a/include/trace/events/gpio.h
> +++ b/include/trace/events/gpio.h
> @@ -1,6 +1,10 @@
> #undef TRACE_SYSTEM
> #define TRACE_SYSTEM gpio
>
> +#ifndef CONFIG_TRACING_EVENTS_GPIO
> +#define NOTRACE
> +#endif
> +
> #if !defined(_TRACE_GPIO_H) || defined(TRACE_HEADER_MULTI_READ)
> #define _TRACE_GPIO_H
>
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index 3b9a48a..67a1731 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -635,6 +635,13 @@ config TRACE_ENUM_MAP_FILE
>
> If unsure, say N
>
> +config TRACING_EVENTS_GPIO
> + bool "Trace gpio events"
> + depends on GPIOLIB
> + default y
> + help
> + Enable tracing events for gpio subsystem
> +
> endif # FTRACE
>
> endif # TRACING_SUPPORT
> --
> 2.4.6
>

2019-03-27 16:13:07

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [Patch v3 2/2] tracing: gpio: add Kconfig option for enabling/disabling trace events

On Sat, Aug 01, 2015 at 03:27:58PM +0300, Tal Shorer wrote:
> Add a new options to trace Kconfig, CONFIG_TRACING_EVENTS_GPIO, that is
> used for enabling/disabling compilation of gpio function trace events.
>
> Signed-off-by: Tal Shorer <[email protected]>
> Acked-by: Linus Walleij <[email protected]>
> ---
> include/trace/events/gpio.h | 4 ++++
> kernel/trace/Kconfig | 7 +++++++
> 2 files changed, 11 insertions(+)
>
> diff --git a/include/trace/events/gpio.h b/include/trace/events/gpio.h
> index 927a8ad..2da73b9 100644
> --- a/include/trace/events/gpio.h
> +++ b/include/trace/events/gpio.h
> @@ -1,6 +1,10 @@
> #undef TRACE_SYSTEM
> #define TRACE_SYSTEM gpio
>
> +#ifndef CONFIG_TRACING_EVENTS_GPIO
> +#define NOTRACE
> +#endif
> +
> #if !defined(_TRACE_GPIO_H) || defined(TRACE_HEADER_MULTI_READ)
> #define _TRACE_GPIO_H
>
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index 3b9a48a..67a1731 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -635,6 +635,13 @@ config TRACE_ENUM_MAP_FILE
>
> If unsure, say N
>
> +config TRACING_EVENTS_GPIO
> + bool "Trace gpio events"
> + depends on GPIOLIB
> + default y
> + help
> + Enable tracing events for gpio subsystem
> +

It happened a few times to me since this patch was merged (in 4.4-rc1)
that I worked on a system with tracing enabled but
/sys/kernel/debug/tracing/events/gpio were missing because of
CONFIG_TRACING_EVENTS_GPIO=n. This is annoying and I wonder what is the
motivating purpose of allowing to disable gpio events. There is no other
event type that can be disabled with kconfig symbols. Theoretically the
overhead should be so small, that having these events enabled
unconditionally should be fine, shouldn't it?

Best regards
Uwe


Attachments:
(No filename) (1.78 kB)
signature.asc (499.00 B)
Download all attachments

2019-04-05 17:27:37

by Linus Walleij

[permalink] [raw]
Subject: Re: [Patch v3 2/2] tracing: gpio: add Kconfig option for enabling/disabling trace events

On Wed, Mar 27, 2019 at 11:12 PM Uwe Kleine-König
<[email protected]> wrote:

> It happened a few times to me since this patch was merged (in 4.4-rc1)
> that I worked on a system with tracing enabled but
> /sys/kernel/debug/tracing/events/gpio were missing because of
> CONFIG_TRACING_EVENTS_GPIO=n. This is annoying and I wonder what is the
> motivating purpose of allowing to disable gpio events. There is no other
> event type that can be disabled with kconfig symbols. Theoretically the
> overhead should be so small, that having these events enabled
> unconditionally should be fine, shouldn't it?

You have a good point. We should not do anything special.

I guess by the time this was merged I believed it was customary
for trancepoints to live behind a Kconfig option.

I guess it is not.

I happily merge a patch removing this again if no case
can be made for it.

Yours,
Linus Walleij