| 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
| ||
|
Date: Fri, 19 Apr 2019 12:08:37 +0200
From: Ingo Molnar <mingo@...nel.org>
To: Peter Zijlstra <peterz@...radead.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Steven Rostedt <rostedt@...dmis.org>
Cc: Steven Rostedt <rostedt@...dmis.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Josh Poimboeuf <jpoimboe@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
Peter Anvin <hpa@...or.com>,
Julien Thierry <julien.thierry@....com>,
Will Deacon <will.deacon@....com>,
Andy Lutomirski <luto@...capital.net>,
Catalin Marinas <catalin.marinas@....com>,
James Morse <james.morse@....com>, valentin.schneider@....com,
Brian Gerst <brgerst@...il.com>,
Andrew Lutomirski <luto@...nel.org>,
Borislav Petkov <bp@...en8.de>,
Denys Vlasenko <dvlasenk@...hat.com>,
Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
Dmitry Vyukov <dvyukov@...gle.com>,
Slavomir Kaslev <kaslevs@...are.com>
Subject: [PATCH] compiler.h, tracing: Remove CONFIG_PROFILE_ALL_BRANCHES
* Peter Zijlstra <peterz@...radead.org> wrote:
> On Thu, Mar 07, 2019 at 01:18:41PM -0500, Steven Rostedt wrote:
> > On Thu, 7 Mar 2019 09:45:35 -0800
> > Linus Torvalds <torvalds@...ux-foundation.org> wrote:
> >
> > > On Thu, Mar 7, 2019 at 9:38 AM Peter Zijlstra <peterz@...radead.org> wrote:
> > > >
> > > > Also; it seems to me that something PT, or maybe even simply:
> > > >
> > > > perf -e branches -e branch-misses
> > > >
> > > > would get you similar or sufficient information.
>
> > I currently have one of my engineers looking at the data and may be
> > sending patches soon. It's basically an entry level way to get into
> > kernel development. Note, no patch will be sent just because of the
> > data from the profiling. The task is to look at and understand the
> > code, and see if it can be optimized (with likely/unlikely or flow
> > changes). It's a way to get a better understanding of the kernel in
> > various locations. It is by no means "profiler said this, lets change
> > it." All changes must be rational, and make sense. The profiler is only
> > used to help find those places.
>
> Can't you just have those same engineers look at perf data? This seems
> like a very expensive and convoluted way of getting something.
So since no-one offered objections to using perf branch profiling instead
(which method allows so much more than CONFIG_PROFILE_ALL_BRANCHES: such
as profiling glibc and other user-space, or allowing to branch-profile
the kernel is an uninstrumented form not distorted by
CONFIG_PROFILE_ALL_BRANCHES code generation artifacts), lemme propose the
attached patch to remove if-tracing.
If the CONFIG_PROFILE_ALL_BRANCHES=y feature is required for anyone it
can still be reverted privately or maintained out of tree - no need to
burden the mainline kernel with this.
I've build tested this and it Looks Perfect Here™.
Thanks,
Ingo
=============================>
>From 3f689ed8a1555aabead90e015a47aefddd2a4e25 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@...nel.org>
Date: Fri, 19 Apr 2019 11:42:43 +0200
Subject: [PATCH] compiler.h, tracing: Remove CONFIG_PROFILE_ALL_BRANCHES
Redefining 'if' in compiler.h was a hideously wonderful hack back in 2008 when
we merged it via:
2bcd521a684c: ("trace: profile all if conditionals")
Meanwhile the 'wonderful' novelty part wore off a bit in that decade, and
what remained is the 'hideous', mostly. ;-)
Meanwhile #2: we also merged perf and hardware branch tracing
capabilities (branch-miss events, but also BTS and aux hw tracing),
which can collect similar data and so much more:
$ perf -e branches -e branch-misses
So let's remove this constant source of headaches for good. Anyone truly
interested in this feature can revert this commit and/or maintain it out
of tree - but the upstream pain isn't really worth it.
Signed-off-by: Ingo Molnar <mingo@...nel.org>
---
include/asm-generic/vmlinux.lds.h | 11 +----
include/linux/compiler.h | 24 -----------
kernel/trace/Kconfig | 17 --------
kernel/trace/trace_branch.c | 66 -----------------------------
tools/perf/tests/bpf-script-test-prologue.c | 9 ----
5 files changed, 1 insertion(+), 126 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index f8f6f04c4453..9c477b2136c2 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -126,14 +126,6 @@
#define LIKELY_PROFILE()
#endif
-#ifdef CONFIG_PROFILE_ALL_BRANCHES
-#define BRANCH_PROFILE() __start_branch_profile = .; \
- KEEP(*(_ftrace_branch)) \
- __stop_branch_profile = .;
-#else
-#define BRANCH_PROFILE()
-#endif
-
#ifdef CONFIG_KPROBES
#define KPROBE_BLACKLIST() . = ALIGN(8); \
__start_kprobe_blacklist = .; \
@@ -266,8 +258,7 @@
__start___verbose = .; \
KEEP(*(__verbose)) \
__stop___verbose = .; \
- LIKELY_PROFILE() \
- BRANCH_PROFILE() \
+ LIKELY_PROFILE() \
TRACE_PRINTKS() \
BPF_RAW_TP() \
TRACEPOINT_STR()
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index d58aa0db05f9..c63105451c6a 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -48,30 +48,6 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
# define unlikely(x) (__branch_check__(x, 0, __builtin_constant_p(x)))
# endif
-#ifdef CONFIG_PROFILE_ALL_BRANCHES
-/*
- * "Define 'is'", Bill Clinton
- * "Define 'if'", Steven Rostedt
- */
-#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
-#define __trace_if(cond) \
- if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
- ({ \
- int ______r; \
- static struct ftrace_branch_data \
- __aligned(4) \
- __section("_ftrace_branch") \
- ______f = { \
- .func = __func__, \
- .file = __FILE__, \
- .line = __LINE__, \
- }; \
- ______r = !!(cond); \
- ______r ? ______f.miss_hit[1]++ : ______f.miss_hit[0]++;\
- ______r; \
- }))
-#endif /* CONFIG_PROFILE_ALL_BRANCHES */
-
#else
# define likely(x) __builtin_expect(!!(x), 1)
# define unlikely(x) __builtin_expect(!!(x), 0)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 8bd1d6d001d7..169c34e0f16d 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -366,23 +366,6 @@ config PROFILE_ANNOTATED_BRANCHES
Note: this will add a significant overhead; only turn this
on if you need to profile the system's use of these macros.
-
-config PROFILE_ALL_BRANCHES
- bool "Profile all if conditionals" if !FORTIFY_SOURCE
- select TRACE_BRANCH_PROFILING
- imply CC_DISABLE_WARN_MAYBE_UNINITIALIZED # avoid false positives
- help
- This tracer profiles all branch conditions. Every if ()
- taken in the kernel is recorded whether it hit or miss.
- The results will be displayed in:
-
- /sys/kernel/debug/tracing/trace_stat/branch_all
-
- This option also enables the likely/unlikely profiler.
-
- This configuration, when enabled, will impose a great overhead
- on the system. This should only be enabled when the system
- is to be analyzed in much detail.
endchoice
config TRACING_BRANCHES
diff --git a/kernel/trace/trace_branch.c b/kernel/trace/trace_branch.c
index 3ea65cdff30d..be75301a9963 100644
--- a/kernel/trace/trace_branch.c
+++ b/kernel/trace/trace_branch.c
@@ -387,69 +387,3 @@ __init static int init_annotated_branch_stats(void)
return 0;
}
fs_initcall(init_annotated_branch_stats);
-
-#ifdef CONFIG_PROFILE_ALL_BRANCHES
-
-extern unsigned long __start_branch_profile[];
-extern unsigned long __stop_branch_profile[];
-
-static int all_branch_stat_headers(struct seq_file *m)
-{
- seq_puts(m, " miss hit % "
- " Function "
- " File Line\n"
- " ------- --------- - "
- " -------- "
- " ---- ----\n");
- return 0;
-}
-
-static void *all_branch_stat_start(struct tracer_stat *trace)
-{
- return __start_branch_profile;
-}
-
-static void *
-all_branch_stat_next(void *v, int idx)
-{
- struct ftrace_branch_data *p = v;
-
- ++p;
-
- if ((void *)p >= (void *)__stop_branch_profile)
- return NULL;
-
- 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 = all_branch_stat_show
-};
-
-__init static int all_annotated_branch_stats(void)
-{
- int ret;
-
- ret = register_stat_tracer(&all_branch_stats);
- if (!ret) {
- printk(KERN_WARNING "Warning: could not register "
- "all branches stats\n");
- return 1;
- }
- return 0;
-}
-fs_initcall(all_annotated_branch_stats);
-#endif /* CONFIG_PROFILE_ALL_BRANCHES */
diff --git a/tools/perf/tests/bpf-script-test-prologue.c b/tools/perf/tests/bpf-script-test-prologue.c
index 43f1e16486f4..1f048bd89b0d 100644
--- a/tools/perf/tests/bpf-script-test-prologue.c
+++ b/tools/perf/tests/bpf-script-test-prologue.c
@@ -10,15 +10,6 @@
#include <uapi/linux/fs.h>
-/*
- * If CONFIG_PROFILE_ALL_BRANCHES is selected,
- * 'if' is redefined after include kernel header.
- * Recover 'if' for BPF object code.
- */
-#ifdef if
-# undef if
-#endif
-
#define FMODE_READ 0x1
#define FMODE_WRITE 0x2
Powered by blists - more mailing lists