From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>
Subject: [for-next][PATCH 2/4] tracing: Process constants for (un)likely() profiler
Date: Thu, 19 Jan 2017 09:00:11 -0500 [thread overview]
Message-ID: <20170119140037.229983730@goodmis.org> (raw)
In-Reply-To: 20170119140009.735308162@goodmis.org
[-- Attachment #1: 0002-tracing-Process-constants-for-un-likely-profiler.patch --]
[-- Type: text/plain, Size: 3827 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
When running the likely/unlikely profiler, one of the results did not look
accurate. It noted that the unlikely() in link_path_walk() was 100%
incorrect. When I added a trace_printk() to see what was happening there, it
became 80% correct! Looking deeper into what whas happening, I found that
gcc split that if statement into two paths. One where the if statement
became a constant, the other path a variable. The other path had the if
statement always hit (making the unlikely there, always false), but since
the #define unlikely() has:
#define unlikely() (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
Where constants are ignored by the branch profiler, the "constant" path
made by the compiler was ignored, even though it was hit 80% of the time.
By just passing the constant value to the __branch_check__() function and
tracing it out of line (as always correct, as likely/unlikely isn't a factor
for constants), then we get back the accurate readings of branches that were
optimized by gcc causing part of the execution to become constant.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
include/linux/compiler.h | 14 ++++++++------
kernel/trace/trace_branch.c | 6 +++++-
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index cf0fa5d86059..bbbe1570de1c 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -107,12 +107,13 @@ struct ftrace_branch_data {
*/
#if defined(CONFIG_TRACE_BRANCH_PROFILING) \
&& !defined(DISABLE_BRANCH_PROFILING) && !defined(__CHECKER__)
-void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
+void ftrace_likely_update(struct ftrace_branch_data *f, int val,
+ int expect, int is_constant);
#define likely_notrace(x) __builtin_expect(!!(x), 1)
#define unlikely_notrace(x) __builtin_expect(!!(x), 0)
-#define __branch_check__(x, expect) ({ \
+#define __branch_check__(x, expect, is_constant) ({ \
int ______r; \
static struct ftrace_branch_data \
__attribute__((__aligned__(4))) \
@@ -122,8 +123,9 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
.file = __FILE__, \
.line = __LINE__, \
}; \
- ______r = likely_notrace(x); \
- ftrace_likely_update(&______f, ______r, expect); \
+ ______r = __builtin_expect(!!(x), expect); \
+ ftrace_likely_update(&______f, ______r, \
+ expect, is_constant); \
______r; \
})
@@ -133,10 +135,10 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
* written by Daniel Walker.
*/
# ifndef likely
-# define likely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 1))
+# define likely(x) (__branch_check__(x, 1, __builtin_constant_p(x)))
# endif
# ifndef unlikely
-# define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
+# define unlikely(x) (__branch_check__(x, 0, __builtin_constant_p(x)))
# endif
#ifdef CONFIG_PROFILE_ALL_BRANCHES
diff --git a/kernel/trace/trace_branch.c b/kernel/trace/trace_branch.c
index 75489de546b6..7afe426ea528 100644
--- a/kernel/trace/trace_branch.c
+++ b/kernel/trace/trace_branch.c
@@ -200,8 +200,12 @@ void trace_likely_condition(struct ftrace_branch_data *f, int val, int expect)
}
#endif /* CONFIG_BRANCH_TRACER */
-void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect)
+void ftrace_likely_update(struct ftrace_branch_data *f, int val,
+ int expect, int is_constant)
{
+ /* A constant is always correct */
+ if (is_constant)
+ val = expect;
/*
* I would love to have a trace point here instead, but the
* trace point code is so inundated with unlikely and likely
--
2.10.2
next prev parent reply other threads:[~2017-01-19 14:02 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-19 14:00 [for-next][PATCH 0/4] tracing: Updates for v4.11 Steven Rostedt
2017-01-19 14:00 ` [for-next][PATCH 1/4] uprobe: Find last occurrence of : when parsing uprobe PATH:OFFSET Steven Rostedt
2017-01-19 14:00 ` Steven Rostedt [this message]
2017-01-19 14:00 ` [for-next][PATCH 3/4] tracing: Show number of constants profiled in likely profiler Steven Rostedt
2017-01-19 14:00 ` [for-next][PATCH 4/4] tracing: Add the constant count for branch tracer Steven Rostedt
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=20170119140037.229983730@goodmis.org \
--to=rostedt@goodmis.org \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
/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.