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: Thu, 4 Jun 2020 10:19:28 +0200 From: Ingo Molnar <mingo@...nel.org> To: Linus Torvalds <torvalds@...ux-foundation.org> Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, Peter Zijlstra <a.p.zijlstra@...llo.nl>, Thomas Gleixner <tglx@...utronix.de>, Borislav Petkov <bp@...en8.de>, Andrew Morton <akpm@...ux-foundation.org> Subject: Re: [PATCH] compiler.h: Move instrumentation_begin()/end() into new <linux/instrumentation.h> header * Ingo Molnar <mingo@...nel.org> wrote: > > * Linus Torvalds <torvalds@...ux-foundation.org> wrote: > > > On Mon, Jun 1, 2020 at 6:08 AM Ingo Molnar <mingo@...nel.org> wrote: > > > > > > include/linux/compiler.h | 53 +++++++++++++++++++++++ > > > > I have pulled this, but do we really want to add this to a header file > > that is _so_ core that it gets included for basically every single > > file built? > > > > I don't even see those instrumentation_begin/end() things used > > anywhere right now. > > > > It seems excessive. That 53 lines is maybe not a lot, but it pushed > > that header file to over 12kB, and while it's mostly comments, it's > > extra IO and parsing basically for _every_ single file compiled in the > > kernel. > > > > For what appears to be absolutely zero upside right now, and I really > > don't see why this should be in such a core header file! > > > > I don't even see this as having anything at all to do with > > "compiler.h" in the first place. > > > > I really think we should think twice about making core header files > > bigger like this. No, we're nowhere the disaster that C++ project > > headers are, but tokenization and parsing is actually a pretty big > > part of the build costs (which may surprise some people who think it's > > all the fancy optimizations that cost a lot of CPU time). > > Fully agreed - and I made the attached patch to address this. > > The code got cleaner and better structured, but it didn't help much in > terms of inclusion count: > > 2616 total .o files > > 2447 <linux/types.h> > 2436 <linux/compiler.h> > 2404 <linux/bug.h> > > The reason is that <linux/bug.h> is included almost everywhere as > well, and the instrumentation_begin()/end() annotations affect the > BUG*() and WARN*() primitives as well. > > At least non-x86 would have less instrumentation related noise, for > now at least. > arch/x86/include/asm/bug.h | 1 + > include/linux/compiler.h | 53 ------------------------------------- > include/linux/context_tracking.h | 2 ++ > include/linux/instrumentation.h | 57 ++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 60 insertions(+), 53 deletions(-) The tested v2 version of the patch also needed the include in asm-generic/bug.h (see the fix attached below), because for completeness the generic version was annotated as well - even though only x86 has objtool support for now. The readability improvement is real though. Thanks, Ingo diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h index 384b5c835ced..c43b5906e0dc 100644 --- a/include/asm-generic/bug.h +++ b/include/asm-generic/bug.h @@ -3,6 +3,7 @@ #define _ASM_GENERIC_BUG_H #include <linux/compiler.h> +#include <linux/instrumentation.h> #define CUT_HERE "------------[ cut here ]------------\n"
Powered by blists - more mailing lists