Messages in this thread Patch in this message | | | Date | Thu, 4 Jun 2020 10:19:28 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH] compiler.h: Move instrumentation_begin()/end() into new <linux/instrumentation.h> header |
| |
* Ingo Molnar <mingo@kernel.org> wrote:
> > * Linus Torvalds <torvalds@linux-foundation.org> wrote: > > > On Mon, Jun 1, 2020 at 6:08 AM Ingo Molnar <mingo@kernel.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"
| |