* [for-next][PATCH 0/4] tracing: Updates for v4.11
@ 2017-01-19 14:00 Steven Rostedt
2017-01-19 14:00 ` [for-next][PATCH 1/4] uprobe: Find last occurrence of : when parsing uprobe PATH:OFFSET Steven Rostedt
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Steven Rostedt @ 2017-01-19 14:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
for-next
Head SHA1: 068f530b3f274d313395663bf8d674798d4858c6
Kenny Yu (1):
uprobe: Find last occurrence of ':' when parsing uprobe PATH:OFFSET
Steven Rostedt (VMware) (3):
tracing: Process constants for (un)likely() profiler
tracing: Show number of constants profiled in likely profiler
tracing: Add the constant count for branch tracer
----
include/linux/compiler.h | 27 ++++++++------
kernel/trace/trace_branch.c | 83 ++++++++++++++++++++++++++++++++++----------
kernel/trace/trace_entries.h | 6 ++--
kernel/trace/trace_uprobe.c | 3 +-
4 files changed, 88 insertions(+), 31 deletions(-)
^ permalink raw reply [flat|nested] 5+ messages in thread
* [for-next][PATCH 1/4] uprobe: Find last occurrence of : when parsing uprobe PATH:OFFSET
2017-01-19 14:00 [for-next][PATCH 0/4] tracing: Updates for v4.11 Steven Rostedt
@ 2017-01-19 14:00 ` Steven Rostedt
2017-01-19 14:00 ` [for-next][PATCH 2/4] tracing: Process constants for (un)likely() profiler Steven Rostedt
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2017-01-19 14:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Kenny Yu, Omar Sandoval
[-- Attachment #1: 0001-uprobe-Find-last-occurrence-of-when-parsing-uprobe-P.patch --]
[-- Type: text/plain, Size: 2385 bytes --]
From: Kenny Yu <kennyyu@fb.com>
Previously, `create_trace_uprobe` found the *first* occurence
of the ':' character when parsing `PATH:OFFSET` for a uprobe.
However, if the path contains a ':' character, then the function
would parse the path incorrectly. Even worse, if the path does not
exist, the subsequent call to `kern_path()` would set `ret` to
`ENOENT`, leading to very cryptic errno values in user space.
The fix is to find the *last* occurence of ':'.
How to repro:: The write fails with "No such file or directory", suggesting
incorrectly that the `uprobe_events` file does not exist.
$ mkdir testing && cd testing
$ cp /bin/bash .
$ cp /bin/bash ./bash:with:colon
$ echo "p:uprobes/p__root_testing_bash_0x6 /root/testing/bash:0x6" > /sys/kernel/debug/tracing/uprobe_events # this works
$ echo "p:uprobes/p__root_testing_bash_with_colon_0x6 /root/testing/bash:with:colon:0x6" >> /sys/kernel/debug/tracing/uprobe_events # this doesn't
-bash: echo: write error: No such file or directory
With the patch:
$ echo "p:uprobes/p__root_testing_bash_0x6 /root/testing/bash:0x6" > /sys/kernel/debug/tracing/uprobe_events # this still works
$ echo "p:uprobes/p__root_testing_bash_with_colon_0x6 /root/testing/bash:with:colon:0x6" >> /sys/kernel/debug/tracing/uprobe_events # this works now too!
$ cat /sys/kernel/debug/tracing/uprobe_events
p:uprobes/p__root_testing_bash_0x6 /root/testing/bash:0x0000000000000006
p:uprobes/p__root_testing_bash_with_colon_0x6 /root/testing/bash:with:colon:0x0000000000000006
Link: http://lkml.kernel.org/r/20170113165834.4081016-1-kennyyu@fb.com
Signed-off-by: Kenny Yu <kennyyu@fb.com>
Reviewed-by: Omar Sandoval <osandov@fb.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace_uprobe.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 0913693caf6e..4f2ba2bb11e0 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -431,7 +431,8 @@ static int create_trace_uprobe(int argc, char **argv)
pr_info("Probe point is not specified.\n");
return -EINVAL;
}
- arg = strchr(argv[1], ':');
+ /* Find the last occurrence, in case the path contains ':' too. */
+ arg = strrchr(argv[1], ':');
if (!arg) {
ret = -EINVAL;
goto fail_address_parse;
--
2.10.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [for-next][PATCH 2/4] tracing: Process constants for (un)likely() profiler
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
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
3 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2017-01-19 14:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
[-- 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [for-next][PATCH 3/4] tracing: Show number of constants profiled in likely profiler
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 ` [for-next][PATCH 2/4] tracing: Process constants for (un)likely() profiler Steven Rostedt
@ 2017-01-19 14:00 ` Steven Rostedt
2017-01-19 14:00 ` [for-next][PATCH 4/4] tracing: Add the constant count for branch tracer Steven Rostedt
3 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2017-01-19 14:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
[-- Attachment #1: 0003-tracing-Show-number-of-constants-profiled-in-likely-.patch --]
[-- Type: text/plain, Size: 5950 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Now that constants are traced, it is useful to see the number of constants
that are traced in the likely/unlikely profiler in order to know if they
should be ignored or not.
The likely/unlikely will display a number after the "correct" number if a
"constant" count exists.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
include/linux/compiler.h | 15 ++++++----
kernel/trace/trace_branch.c | 68 ++++++++++++++++++++++++++++++++++++---------
2 files changed, 65 insertions(+), 18 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index bbbe1570de1c..a73cc9afa784 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -101,13 +101,18 @@ struct ftrace_branch_data {
};
};
+struct ftrace_likely_data {
+ struct ftrace_branch_data data;
+ unsigned long constant;
+};
+
/*
* Note: DISABLE_BRANCH_PROFILING can be used by special lowlevel code
* to disable branch tracing on a per file basis.
*/
#if defined(CONFIG_TRACE_BRANCH_PROFILING) \
&& !defined(DISABLE_BRANCH_PROFILING) && !defined(__CHECKER__)
-void ftrace_likely_update(struct ftrace_branch_data *f, int val,
+void ftrace_likely_update(struct ftrace_likely_data *f, int val,
int expect, int is_constant);
#define likely_notrace(x) __builtin_expect(!!(x), 1)
@@ -115,13 +120,13 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val,
#define __branch_check__(x, expect, is_constant) ({ \
int ______r; \
- static struct ftrace_branch_data \
+ static struct ftrace_likely_data \
__attribute__((__aligned__(4))) \
__attribute__((section("_ftrace_annotated_branch"))) \
______f = { \
- .func = __func__, \
- .file = __FILE__, \
- .line = __LINE__, \
+ .data.func = __func__, \
+ .data.file = __FILE__, \
+ .data.line = __LINE__, \
}; \
______r = __builtin_expect(!!(x), expect); \
ftrace_likely_update(&______f, ______r, \
diff --git a/kernel/trace/trace_branch.c b/kernel/trace/trace_branch.c
index 7afe426ea528..fd483d74f8e1 100644
--- a/kernel/trace/trace_branch.c
+++ b/kernel/trace/trace_branch.c
@@ -200,25 +200,27 @@ 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,
+void ftrace_likely_update(struct ftrace_likely_data *f, int val,
int expect, int is_constant)
{
/* A constant is always correct */
- if (is_constant)
+ if (is_constant) {
+ f->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
* conditions that the recursive nightmare that exists is too
* much to try to get working. At least for now.
*/
- trace_likely_condition(f, val, expect);
+ trace_likely_condition(&f->data, val, expect);
/* FIXME: Make this atomic! */
if (val == expect)
- f->correct++;
+ f->data.correct++;
else
- f->incorrect++;
+ f->data.incorrect++;
}
EXPORT_SYMBOL(ftrace_likely_update);
@@ -249,29 +251,60 @@ static inline long get_incorrect_percent(struct ftrace_branch_data *p)
return percent;
}
-static int branch_stat_show(struct seq_file *m, void *v)
+static const char *branch_stat_process_file(struct ftrace_branch_data *p)
{
- struct ftrace_branch_data *p = v;
const char *f;
- long percent;
/* Only print the file, not the path */
f = p->file + strlen(p->file);
while (f >= p->file && *f != '/')
f--;
- f++;
+ return ++f;
+}
+
+static void branch_stat_show(struct seq_file *m,
+ struct ftrace_branch_data *p, const char *f)
+{
+ long percent;
/*
* The miss is overlayed on correct, and hit on incorrect.
*/
percent = get_incorrect_percent(p);
- seq_printf(m, "%8lu %8lu ", p->correct, p->incorrect);
if (percent < 0)
seq_puts(m, " X ");
else
seq_printf(m, "%3ld ", percent);
+
seq_printf(m, "%-30.30s %-20.20s %d\n", p->func, f, p->line);
+}
+
+static int branch_stat_show_normal(struct seq_file *m,
+ struct ftrace_branch_data *p, const char *f)
+{
+ seq_printf(m, "%8lu %8lu ", p->correct, p->incorrect);
+ branch_stat_show(m, p, f);
+ return 0;
+}
+
+static int annotate_branch_stat_show(struct seq_file *m, void *v)
+{
+ struct ftrace_likely_data *p = v;
+ const char *f;
+ int l;
+
+ f = branch_stat_process_file(&p->data);
+
+ if (!p->constant)
+ return branch_stat_show_normal(m, &p->data, f);
+
+ l = snprintf(NULL, 0, "/%lu", p->constant);
+ l = l > 8 ? 0 : 8 - l;
+
+ seq_printf(m, "%8lu/%lu %*lu ",
+ p->data.correct, p->constant, l, p->data.incorrect);
+ branch_stat_show(m, &p->data, f);
return 0;
}
@@ -283,7 +316,7 @@ static void *annotated_branch_stat_start(struct tracer_stat *trace)
static void *
annotated_branch_stat_next(void *v, int idx)
{
- struct ftrace_branch_data *p = v;
+ struct ftrace_likely_data *p = v;
++p;
@@ -332,7 +365,7 @@ static struct tracer_stat annotated_branch_stats = {
.stat_next = annotated_branch_stat_next,
.stat_cmp = annotated_branch_stat_cmp,
.stat_headers = annotated_branch_stat_headers,
- .stat_show = branch_stat_show
+ .stat_show = annotate_branch_stat_show
};
__init static int init_annotated_branch_stats(void)
@@ -383,12 +416,21 @@ all_branch_stat_next(void *v, int idx)
return p;
}
+static int all_branch_stat_show(struct seq_file *m, void *v)
+{
+ struct ftrace_branch_data *p = v;
+ const char *f;
+
+ f = branch_stat_process_file(p);
+ return branch_stat_show_normal(m, p, f);
+}
+
static struct tracer_stat all_branch_stats = {
.name = "branch_all",
.stat_start = all_branch_stat_start,
.stat_next = all_branch_stat_next,
.stat_headers = all_branch_stat_headers,
- .stat_show = branch_stat_show
+ .stat_show = all_branch_stat_show
};
__init static int all_annotated_branch_stats(void)
--
2.10.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [for-next][PATCH 4/4] tracing: Add the constant count for branch tracer
2017-01-19 14:00 [for-next][PATCH 0/4] tracing: Updates for v4.11 Steven Rostedt
` (2 preceding siblings ...)
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 ` Steven Rostedt
3 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2017-01-19 14:00 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
[-- Attachment #1: 0004-tracing-Add-the-constant-count-for-branch-tracer.patch --]
[-- Type: text/plain, Size: 3387 bytes --]
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
The unlikely/likely branch profiler now gets called even if the if statement
is a constant (always goes in one direction without a compare). Add a value
to denote this in the likely/unlikely tracer as well.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace_branch.c | 17 +++++++++--------
kernel/trace/trace_entries.h | 6 ++++--
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/kernel/trace/trace_branch.c b/kernel/trace/trace_branch.c
index fd483d74f8e1..4d8fdf3184dc 100644
--- a/kernel/trace/trace_branch.c
+++ b/kernel/trace/trace_branch.c
@@ -27,7 +27,7 @@ static DEFINE_MUTEX(branch_tracing_mutex);
static struct trace_array *branch_tracer;
static void
-probe_likely_condition(struct ftrace_branch_data *f, int val, int expect)
+probe_likely_condition(struct ftrace_likely_data *f, int val, int expect)
{
struct trace_event_call *call = &event_branch;
struct trace_array *tr = branch_tracer;
@@ -68,16 +68,17 @@ probe_likely_condition(struct ftrace_branch_data *f, int val, int expect)
entry = ring_buffer_event_data(event);
/* Strip off the path, only save the file */
- p = f->file + strlen(f->file);
- while (p >= f->file && *p != '/')
+ p = f->data.file + strlen(f->data.file);
+ while (p >= f->data.file && *p != '/')
p--;
p++;
- strncpy(entry->func, f->func, TRACE_FUNC_SIZE);
+ strncpy(entry->func, f->data.func, TRACE_FUNC_SIZE);
strncpy(entry->file, p, TRACE_FILE_SIZE);
entry->func[TRACE_FUNC_SIZE] = 0;
entry->file[TRACE_FILE_SIZE] = 0;
- entry->line = f->line;
+ entry->constant = f->constant;
+ entry->line = f->data.line;
entry->correct = val == expect;
if (!call_filter_check_discard(call, entry, buffer, event))
@@ -89,7 +90,7 @@ probe_likely_condition(struct ftrace_branch_data *f, int val, int expect)
}
static inline
-void trace_likely_condition(struct ftrace_branch_data *f, int val, int expect)
+void trace_likely_condition(struct ftrace_likely_data *f, int val, int expect)
{
if (!branch_tracing_enabled)
return;
@@ -195,7 +196,7 @@ core_initcall(init_branch_tracer);
#else
static inline
-void trace_likely_condition(struct ftrace_branch_data *f, int val, int expect)
+void trace_likely_condition(struct ftrace_likely_data *f, int val, int expect)
{
}
#endif /* CONFIG_BRANCH_TRACER */
@@ -214,7 +215,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
* conditions that the recursive nightmare that exists is too
* much to try to get working. At least for now.
*/
- trace_likely_condition(&f->data, val, expect);
+ trace_likely_condition(f, val, expect);
/* FIXME: Make this atomic! */
if (val == expect)
diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h
index eb7396b7e7c3..c203ac4df791 100644
--- a/kernel/trace/trace_entries.h
+++ b/kernel/trace/trace_entries.h
@@ -328,11 +328,13 @@ FTRACE_ENTRY(branch, trace_branch,
__array( char, func, TRACE_FUNC_SIZE+1 )
__array( char, file, TRACE_FILE_SIZE+1 )
__field( char, correct )
+ __field( char, constant )
),
- F_printk("%u:%s:%s (%u)",
+ F_printk("%u:%s:%s (%u)%s",
__entry->line,
- __entry->func, __entry->file, __entry->correct),
+ __entry->func, __entry->file, __entry->correct,
+ __entry->constant ? " CONSTANT" : ""),
FILTER_OTHER
);
--
2.10.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-01-19 14:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [for-next][PATCH 2/4] tracing: Process constants for (un)likely() profiler Steven Rostedt
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).