Skip to content

Commit d19e789

Browse files
author
Ingo Molnar
committed
compiler.h: Move instrumentation_begin()/end() to new <linux/instrumentation.h> header
Linus pointed out that compiler.h - which is a key header that gets included in every single one of the 28,000+ kernel files during a kernel build - was bloated in: 6553896: ("vmlinux.lds.h: Create section for protection against instrumentation") Linus noted: > 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! Move these primitives into a new header: <linux/instrumentation.h>, and include that header in the headers that make use of it. Unfortunately one of these headers is asm-generic/bug.h, which does get included in a lot of places, similarly to compiler.h. So the de-bloating effect isn't as good as we'd like it to be - but at least the interfaces are defined separately. No change to functionality intended. Reported-by: Linus Torvalds <[email protected]> Signed-off-by: Ingo Molnar <[email protected]> Link: https://lore.kernel.org/r/[email protected] Cc: Thomas Gleixner <[email protected]> Cc: Borislav Petkov <[email protected]> Cc: Peter Zijlstra <[email protected]>
1 parent f37e99a commit d19e789

File tree

5 files changed

+61
-53
lines changed

5 files changed

+61
-53
lines changed

arch/x86/include/asm/bug.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#define _ASM_X86_BUG_H
44

55
#include <linux/stringify.h>
6+
#include <linux/instrumentation.h>
67

78
/*
89
* Despite that some emulators terminate on UD2, we use it for WARN().

include/asm-generic/bug.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#define _ASM_GENERIC_BUG_H
44

55
#include <linux/compiler.h>
6+
#include <linux/instrumentation.h>
67

78
#define CUT_HERE "------------[ cut here ]------------\n"
89

include/linux/compiler.h

Lines changed: 0 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -120,65 +120,12 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
120120
/* Annotate a C jump table to allow objtool to follow the code flow */
121121
#define __annotate_jump_table __section(.rodata..c_jump_table)
122122

123-
#ifdef CONFIG_DEBUG_ENTRY
124-
/* Begin/end of an instrumentation safe region */
125-
#define instrumentation_begin() ({ \
126-
asm volatile("%c0: nop\n\t" \
127-
".pushsection .discard.instr_begin\n\t" \
128-
".long %c0b - .\n\t" \
129-
".popsection\n\t" : : "i" (__COUNTER__)); \
130-
})
131-
132-
/*
133-
* Because instrumentation_{begin,end}() can nest, objtool validation considers
134-
* _begin() a +1 and _end() a -1 and computes a sum over the instructions.
135-
* When the value is greater than 0, we consider instrumentation allowed.
136-
*
137-
* There is a problem with code like:
138-
*
139-
* noinstr void foo()
140-
* {
141-
* instrumentation_begin();
142-
* ...
143-
* if (cond) {
144-
* instrumentation_begin();
145-
* ...
146-
* instrumentation_end();
147-
* }
148-
* bar();
149-
* instrumentation_end();
150-
* }
151-
*
152-
* If instrumentation_end() would be an empty label, like all the other
153-
* annotations, the inner _end(), which is at the end of a conditional block,
154-
* would land on the instruction after the block.
155-
*
156-
* If we then consider the sum of the !cond path, we'll see that the call to
157-
* bar() is with a 0-value, even though, we meant it to happen with a positive
158-
* value.
159-
*
160-
* To avoid this, have _end() be a NOP instruction, this ensures it will be
161-
* part of the condition block and does not escape.
162-
*/
163-
#define instrumentation_end() ({ \
164-
asm volatile("%c0: nop\n\t" \
165-
".pushsection .discard.instr_end\n\t" \
166-
".long %c0b - .\n\t" \
167-
".popsection\n\t" : : "i" (__COUNTER__)); \
168-
})
169-
#endif /* CONFIG_DEBUG_ENTRY */
170-
171123
#else
172124
#define annotate_reachable()
173125
#define annotate_unreachable()
174126
#define __annotate_jump_table
175127
#endif
176128

177-
#ifndef instrumentation_begin
178-
#define instrumentation_begin() do { } while(0)
179-
#define instrumentation_end() do { } while(0)
180-
#endif
181-
182129
#ifndef ASM_UNREACHABLE
183130
# define ASM_UNREACHABLE
184131
#endif

include/linux/context_tracking.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
#include <linux/sched.h>
66
#include <linux/vtime.h>
77
#include <linux/context_tracking_state.h>
8+
#include <linux/instrumentation.h>
9+
810
#include <asm/ptrace.h>
911

1012

include/linux/instrumentation.h

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/* SPDX-License-Identifier: GPL-2.0 */
2+
#ifndef __LINUX_INSTRUMENTATION_H
3+
#define __LINUX_INSTRUMENTATION_H
4+
5+
#if defined(CONFIG_DEBUG_ENTRY) && defined(CONFIG_STACK_VALIDATION)
6+
7+
/* Begin/end of an instrumentation safe region */
8+
#define instrumentation_begin() ({ \
9+
asm volatile("%c0: nop\n\t" \
10+
".pushsection .discard.instr_begin\n\t" \
11+
".long %c0b - .\n\t" \
12+
".popsection\n\t" : : "i" (__COUNTER__)); \
13+
})
14+
15+
/*
16+
* Because instrumentation_{begin,end}() can nest, objtool validation considers
17+
* _begin() a +1 and _end() a -1 and computes a sum over the instructions.
18+
* When the value is greater than 0, we consider instrumentation allowed.
19+
*
20+
* There is a problem with code like:
21+
*
22+
* noinstr void foo()
23+
* {
24+
* instrumentation_begin();
25+
* ...
26+
* if (cond) {
27+
* instrumentation_begin();
28+
* ...
29+
* instrumentation_end();
30+
* }
31+
* bar();
32+
* instrumentation_end();
33+
* }
34+
*
35+
* If instrumentation_end() would be an empty label, like all the other
36+
* annotations, the inner _end(), which is at the end of a conditional block,
37+
* would land on the instruction after the block.
38+
*
39+
* If we then consider the sum of the !cond path, we'll see that the call to
40+
* bar() is with a 0-value, even though, we meant it to happen with a positive
41+
* value.
42+
*
43+
* To avoid this, have _end() be a NOP instruction, this ensures it will be
44+
* part of the condition block and does not escape.
45+
*/
46+
#define instrumentation_end() ({ \
47+
asm volatile("%c0: nop\n\t" \
48+
".pushsection .discard.instr_end\n\t" \
49+
".long %c0b - .\n\t" \
50+
".popsection\n\t" : : "i" (__COUNTER__)); \
51+
})
52+
#else
53+
# define instrumentation_begin() do { } while(0)
54+
# define instrumentation_end() do { } while(0)
55+
#endif
56+
57+
#endif /* __LINUX_INSTRUMENTATION_H */

0 commit comments

Comments
 (0)