All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Yafang Shao <laoar.shao@gmail.com>,
	Axel Rasmussen <axelrasmussen@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Vlastimil Babka <vbabka@suse.cz>,
	Michel Lespinasse <walken@google.com>,
	Daniel Jordan <daniel.m.jordan@oracle.com>,
	Davidlohr Bueso <dbueso@suse.de>, Linux MM <linux-mm@kvack.org>,
	Ingo Molnar <mingo@kernel.org>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>
Subject: [PATCH 1/3 v2] tracepoints: Add helper to test if tracepoint is enabled in a header
Date: Fri, 25 Sep 2020 17:12:07 -0400	[thread overview]
Message-ID: <20200925211819.767526657@goodmis.org> (raw)
In-Reply-To: 20200925211206.423598568@goodmis.org

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

As tracepoints are discouraged from being added in a header because it can
cause side effects if other tracepoints are in headers, as well as bloat the
kernel as the trace_<tracepoint>() function is not a small inline, the common
workaround is to add a function call that calls a wrapper function in a
C file that then calls the tracepoint. But as function calls add overhead,
this function should only be called when the tracepoint in question is
enabled. To get around this overhead, a static_branch can be used to only
have the tracepoint wrapper get called when the tracepoint is enabled.

Add a tracepoint_enabled(tp) macro that gets passed the name of the
tracepoint, and this becomes a static_branch that is enabled when the
tracepoint is enabled and is a nop when the tracepoint is disabled.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/trace/tracepoints.rst | 27 +++++++++++++++++++++++
 include/linux/tracepoint-defs.h     | 34 +++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/Documentation/trace/tracepoints.rst b/Documentation/trace/tracepoints.rst
index 6e3ce3bf3593..68579ebd1e4c 100644
--- a/Documentation/trace/tracepoints.rst
+++ b/Documentation/trace/tracepoints.rst
@@ -146,3 +146,30 @@ with jump labels and avoid conditional branches.
       define tracepoints. Check http://lwn.net/Articles/379903,
       http://lwn.net/Articles/381064 and http://lwn.net/Articles/383362
       for a series of articles with more details.
+
+If you require calling a tracepoint from a header file, it is not
+recommended to call one directly or to use the trace_<tracepoint>_enabled()
+function call, as tracepoints in header files can have side effects if a
+header is included from a file that has CREATE_TRACE_POINTS set, as
+well as the trace_<tracepoint>() is not that small of an inline
+and can bloat the kernel if used by other inlined functions. Instead,
+include tracepoint-defs.h and use tracepoint_enabled().
+
+In a C file::
+
+	void do_trace_foo_bar_wrapper(args)
+	{
+		trace_foo_bar(args);
+	}
+
+In the header file::
+
+	DECLEARE_TRACEPOINT(foo_bar);
+
+	static inline void some_inline_function()
+	{
+		[..]
+		if (tracepoint_enabled(foo_bar))
+			do_trace_foo_bar_wrapper(args);
+		[..]
+	}
diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
index b29950a19205..60625973faaf 100644
--- a/include/linux/tracepoint-defs.h
+++ b/include/linux/tracepoint-defs.h
@@ -48,4 +48,38 @@ struct bpf_raw_event_map {
 	u32			writable_size;
 } __aligned(32);
 
+/*
+ * If a tracepoint needs to be called from a header file, it is not
+ * recommended to call it directly, as tracepoints in header files
+ * may cause side-effects and bloat the kernel. Instead, use
+ * tracepoint_enabled() to test if the tracepoint is enabled, then if
+ * it is, call a wrapper function defined in a C file that will then
+ * call the tracepoint.
+ *
+ * For "trace_foo_bar()", you would need to create a wrapper function
+ * in a C file to call trace_foo_bar():
+ *   void do_trace_foo_bar(args) { trace_foo_bar(args); }
+ * Then in the header file, declare the tracepoint:
+ *   DECLARE_TRACEPOINT(foo_bar);
+ * And call your wrapper:
+ *   static inline void some_inlined_function() {
+ *            [..]
+ *            if (tracepoint_enabled(foo_bar))
+ *                    do_trace_foo_bar(args);
+ *            [..]
+ *   }
+ *
+ * Note: tracepoint_enabled(foo_bar) is equivalent to trace_foo_bar_enabled()
+ *   but is safe to have in headers, where trace_foo_bar_enabled() is not.
+ */
+#define DECLARE_TRACEPOINT(tp) \
+	extern struct tracepoint __tracepoint_##tp
+
+#ifdef CONFIG_TRACEPOINTS
+# define tracepoint_enabled(tp) \
+	static_key_false(&(__tracepoint_##tp).key)
+#else
+# define tracepoint_enabled(tracepoint) false
+#endif
+
 #endif
-- 
2.28.0



  reply	other threads:[~2020-09-25 21:18 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-25 21:12 [PATCH 0/3 v2] tracing/mm: Add tracepoint_enabled() helper function for headers Steven Rostedt
2020-09-25 21:12 ` Steven Rostedt [this message]
2020-09-25 21:36   ` [PATCH 1/3 v2] tracepoints: Add helper to test if tracepoint is enabled in a header Axel Rasmussen
2020-09-25 21:36     ` Axel Rasmussen
2020-09-25 21:59     ` Steven Rostedt
2020-10-20 11:59   ` Vlastimil Babka
2020-10-20 20:43     ` Steven Rostedt
2020-09-25 21:12 ` [PATCH 2/3 v2] mm/page_ref: Convert the open coded tracepoint enabled to the new helper Steven Rostedt
2020-10-20 12:10   ` Vlastimil Babka
2020-09-25 21:12 ` [PATCH 3/3 v2] x86: Use tracepoint_enabled() for msr tracepoints instead of open coding it Steven Rostedt
2020-09-26  6:55   ` kernel test robot
2020-09-26  6:55     ` kernel test robot
2020-09-25 21:26 ` [PATCH 0/3 v2] tracing/mm: Add tracepoint_enabled() helper function for headers Steven Rostedt
2020-09-25 22:23   ` Mathieu Desnoyers
2020-09-25 22:23     ` Mathieu Desnoyers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200925211819.767526657@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=axelrasmussen@google.com \
    --cc=daniel.m.jordan@oracle.com \
    --cc=dbueso@suse.de \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=laoar.shao@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mingo@kernel.org \
    --cc=vbabka@suse.cz \
    --cc=walken@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.