| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2007248757.12536.1399462930995.JavaMail.zimbra@efficios.com>
Date: Wed, 7 May 2014 11:42:11 +0000 (UTC)
From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Javi Merino <javi.merino@....com>,
David Howells <dhowells@...hat.com>,
Ingo Molnar <mingo@...nel.org>
Subject: Re: [RFA][PATCH v2] tracing: Add trace_<tracepoint>_enabled()
function
----- Original Message -----
> From: "Steven Rostedt" <rostedt@...dmis.org>
> To: "LKML" <linux-kernel@...r.kernel.org>
> Cc: "Mathieu Desnoyers" <mathieu.desnoyers@...icios.com>, "Andrew Morton" <akpm@...ux-foundation.org>, "Javi Merino"
> <javi.merino@....com>, "David Howells" <dhowells@...hat.com>, "Ingo Molnar" <mingo@...nel.org>
> Sent: Tuesday, May 6, 2014 11:10:59 PM
> Subject: [RFA][PATCH v2] tracing: Add trace_<tracepoint>_enabled() function
>
>
> There are some code paths in the kernel that need to do some preparations
> before it calls a tracepoint. As that code is worthless overhead when
> the tracepoint is not enabled, it would be prudent to have that code
> only run when the tracepoint is active. To accomplish this, all tracepoints
> now get a static inline function called "trace_<tracepoint-name>_enabled()"
> which returns true when the tracepoint is enabled and false otherwise.
>
> As an added bonus, that function uses the static_key of the tracepoint
> such that no branch is needed.
>
> if (trace_mytracepoint_enabled()) {
> arg = process_tp_arg();
> trace_mytracepoint(arg);
> }
>
> Will keep the "process_tp_arg()" (which may be expensive to run) from
> being executed when the tracepoint isn't enabled.
>
> It's best to encapsulate the tracepoint itself in the if statement
> just to keep races. For example, if you had:
>
> if (trace_mytracepoint_enabled())
> arg = process_tp_arg();
> trace_mytracepoint(arg);
>
> There's a chance that the tracepoint could be enabled just after the
> if statement, and arg will be undefined when calling the tracepoint.
>
> Link: http://lkml.kernel.org/r/20140506094407.507b6435@gandalf.local.home
>
> Signed-off-by: Steven Rostedt <rostedt@...dmis.org>
Acked-by: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
> ---
> Changes since v1: Added some documentation about using this function.
> ---
> Documentation/trace/tracepoints.txt | 24 ++++++++++++++++++++++++
> include/linux/tracepoint.h | 10 ++++++++++
> 2 files changed, 34 insertions(+)
>
> diff --git a/Documentation/trace/tracepoints.txt
> b/Documentation/trace/tracepoints.txt
> index 6b018b5..a3efac6 100644
> --- a/Documentation/trace/tracepoints.txt
> +++ b/Documentation/trace/tracepoints.txt
> @@ -115,6 +115,30 @@ If the tracepoint has to be used in kernel modules, an
> EXPORT_TRACEPOINT_SYMBOL_GPL() or EXPORT_TRACEPOINT_SYMBOL() can be
> used to export the defined tracepoints.
>
> +If you need to do a bit of work for a tracepoint parameter, and
> +that work is only used for the tracepoint, that work can be encapsulated
> +within an if statement with the following:
> +
> + if (trace_foo_bar_enabled()) {
> + int i;
> + int tot = 0;
> +
> + for (i = 0; i < count; i++)
> + tot += calculate_nuggets();
> +
> + trace_foo_bar(tot);
> + }
> +
> +All trace_<tracepoint>() calls have a matching trace_<tracepoint>_enabled()
> +function defined that returns true if the tracepoint is enabled and
> +false otherwise. The trace_<tracepoint>() should always be within the
> +block of the if (trace_<tracepoint>_enabled()) to prevent races between
> +the tracepoint being enabled and the check being seen.
> +
> +The advantage of using the trace_<tracepoint>_enabled() is that it uses
> +the static_key of the tracepoint to allow the if statement to be implemented
> +with jump labels and avoid conditional branches.
> +
> Note: The convenience macro TRACE_EVENT provides an alternative way to
> define tracepoints. Check http://lwn.net/Articles/379903,
> http://lwn.net/Articles/381064 and http://lwn.net/Articles/383362
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index 9d30ee4..2e2a5f7 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -185,6 +185,11 @@ extern void syscall_unregfunc(void);
> static inline void \
> check_trace_callback_type_##name(void (*cb)(data_proto)) \
> { \
> + } \
> + static inline bool \
> + trace_##name##_enabled(void) \
> + { \
> + return static_key_false(&__tracepoint_##name.key); \
> }
>
> /*
> @@ -230,6 +235,11 @@ extern void syscall_unregfunc(void);
> } \
> static inline void check_trace_callback_type_##name(void (*cb)(data_proto))
> \
> { \
> + } \
> + static inline bool \
> + trace_##name##_enabled(void) \
> + { \
> + return false; \
> }
>
> #define DEFINE_TRACE_FN(name, reg, unreg)
> --
> 1.8.1.4
>
>
>
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists