Re: [patch V6 00/37] x86/entry: Rework leftovers and merge plan

2020-06-15 Thread Peter Zijlstra
On Sat, May 23, 2020 at 03:08:36PM +0200, Peter Zijlstra wrote:
> On Sat, May 23, 2020 at 10:52:24AM +0800, Lai Jiangshan wrote:
> 
> > Hello,
> > 
> > I, who don't know how does the objtool handle it, am just curious.
> > _begin() and _end() are symmetrical, which means if _end() (without nop)
> > can escape, so can _begin() in a reverse way. For example:
> > 
> > noinstr void foo()
> > {
> > instrumentation_begin();
> > do {
> > instrumentation_begin();
> > ...
> > instrumentation_end();
> > } while (cond);
> > bar();
> > instrumentation_end();
> > }
> > 
> > Here, the first _begin() can be "dragged" into the do-while block.
> > Expectedly, objtool validation should not complain here.
> > 
> > But objtool validation's not complaining means it can handle it
> > magically correctly (by distinguishing how many _begin()s should
> > be taken around the jmp target when jmp in a specific path), or
> > handle it by not checking if all paths have the same count onto
> > a jmp target (a little nervous to me), or other possible ways.
> 
> No, I tihnk you're right. It could be we never hit this particular
> problem. Even the one described, where end leaks out, is quite rare. For
> instance, the last one I debgged (that led to this patch) only showed
> itself with gcc-9, but not with gcc-8 for example.
> 
> Anyway, if we ever find the above, I'll add the NOP to begin too.

FYI, I just found one, I'll be making instrumentation_begin() a NOP
too.


Re: [patch V6 00/37] x86/entry: Rework leftovers and merge plan

2020-05-23 Thread Peter Zijlstra
On Sat, May 23, 2020 at 10:52:24AM +0800, Lai Jiangshan wrote:

> Hello,
> 
> I, who don't know how does the objtool handle it, am just curious.
> _begin() and _end() are symmetrical, which means if _end() (without nop)
> can escape, so can _begin() in a reverse way. For example:
> 
> noinstr void foo()
> {
> instrumentation_begin();
> do {
> instrumentation_begin();
> ...
> instrumentation_end();
> } while (cond);
> bar();
> instrumentation_end();
> }
> 
> Here, the first _begin() can be "dragged" into the do-while block.
> Expectedly, objtool validation should not complain here.
> 
> But objtool validation's not complaining means it can handle it
> magically correctly (by distinguishing how many _begin()s should
> be taken around the jmp target when jmp in a specific path), or
> handle it by not checking if all paths have the same count onto
> a jmp target (a little nervous to me), or other possible ways.

No, I tihnk you're right. It could be we never hit this particular
problem. Even the one described, where end leaks out, is quite rare. For
instance, the last one I debgged (that led to this patch) only showed
itself with gcc-9, but not with gcc-8 for example.

Anyway, if we ever find the above, I'll add the NOP to begin too.


Re: [patch V6 00/37] x86/entry: Rework leftovers and merge plan

2020-05-22 Thread Lai Jiangshan
On Tue, May 19, 2020 at 5:04 PM Peter Zijlstra  wrote:

> +#ifdef CONFIG_DEBUG_ENTRY
>  /* Begin/end of an instrumentation safe region */
> -#define instrumentation_begin() ({   
>   \
> +#define instrumentation_begin() ({ \
> asm volatile("%c0:\n\t" \
>  ".pushsection .discard.instr_begin\n\t"\
>  ".long %c0b - .\n\t"   \
>  ".popsection\n\t" : : "i" (__COUNTER__));  \
>  })
>
> -#define instrumentation_end() ({ 
>   \
> -   asm volatile("%c0:\n\t" \
> +/*
> + * Because instrumentation_{begin,end}() can nest, objtool validation 
> considers
> + * _begin() a +1 and _end() a -1 and computes a sum over the instructions.
> + * When the value is greater than 0, we consider instrumentation allowed.
> + *
> + * There is a problem with code like:
> + *
> + * noinstr void foo()
> + * {
> + * instrumentation_begin();
> + * ...
> + * if (cond) {
> + * instrumentation_begin();
> + * ...
> + * instrumentation_end();
> + * }
> + * bar();
> + * instrumentation_end();
> + * }
> + *
> + * If instrumentation_end() would be an empty label, like all the other
> + * annotations, the inner _end(), which is at the end of a conditional block,
> + * would land on the instruction after the block.
> + *
> + * If we then consider the sum of the !cond path, we'll see that the call to
> + * bar() is with a 0-value, even though, we meant it to happen with a 
> positive
> + * value.
> + *
> + * To avoid this, have _end() be a NOP instruction, this ensures it will be
> + * part of the condition block and does not escape.
> + */
> +#define instrumentation_end() ({   \
> +   asm volatile("%c0: nop\n\t" \
>  ".pushsection .discard.instr_end\n\t"  \
>  ".long %c0b - .\n\t"   \
>  ".popsection\n\t" : : "i" (__COUNTER__));  \
>  })

Hello,

I, who don't know how does the objtool handle it, am just curious.
_begin() and _end() are symmetrical, which means if _end() (without nop)
can escape, so can _begin() in a reverse way. For example:

noinstr void foo()
{
instrumentation_begin();
do {
instrumentation_begin();
...
instrumentation_end();
} while (cond);
bar();
instrumentation_end();
}

Here, the first _begin() can be "dragged" into the do-while block.
Expectedly, objtool validation should not complain here.

But objtool validation's not complaining means it can handle it
magically correctly (by distinguishing how many _begin()s should
be taken around the jmp target when jmp in a specific path), or
handle it by not checking if all paths have the same count onto
a jmp target (a little nervous to me), or other possible ways.

Sorry for my curiosity.
Thanks
Lai.


Re: [patch V6 00/37] x86/entry: Rework leftovers and merge plan

2020-05-19 Thread Steven Rostedt
On Tue, 19 May 2020 21:09:48 +0200
Thomas Gleixner  wrote:

> Steven Rostedt  writes:
> 
> > On Sat, 16 May 2020 01:45:47 +0200
> > Thomas Gleixner  wrote:
> >  
> >> The V6 leftover series is based on:
> >> 
> >>   git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git 
> >> entry-base-v6  
> >
> >
> > $ git fetch git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git 
> > entry-base-v6
> > fatal: couldn't find remote ref entry-base-v6  
> 
> try entry-v6-base or better entry-v8-base

Ah, I should have had a V8!

-- Steve


Re: [patch V6 00/37] x86/entry: Rework leftovers and merge plan

2020-05-19 Thread Thomas Gleixner
Steven Rostedt  writes:

> On Sat, 16 May 2020 01:45:47 +0200
> Thomas Gleixner  wrote:
>
>> The V6 leftover series is based on:
>> 
>>   git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git entry-base-v6
>
>
> $ git fetch git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git 
> entry-base-v6
> fatal: couldn't find remote ref entry-base-v6

try entry-v6-base or better entry-v8-base


Re: [patch V6 00/37] x86/entry: Rework leftovers and merge plan

2020-05-19 Thread Steven Rostedt
On Sat, 16 May 2020 01:45:47 +0200
Thomas Gleixner  wrote:

> The V6 leftover series is based on:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git entry-base-v6


$ git fetch git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git 
entry-base-v6
fatal: couldn't find remote ref entry-base-v6

-- Steve


Re: [patch V6 00/37] x86/entry: Rework leftovers and merge plan

2020-05-19 Thread Joel Fernandes
On Sat, May 16, 2020 at 10:18:45AM -0700, Paul E. McKenney wrote:
> On Sat, May 16, 2020 at 01:45:47AM +0200, Thomas Gleixner wrote:
> 
> [ . . . ]
> 
> >   - noinstr-rcu-nmi-2020-05-15
> > 
> > Based on the core/rcu branch in the tip tree. It has merged in
> > noinstr-lds-2020-05-15 and contains the nmi_enter/exit() changes along
> > with the noinstr section changes on top.
> > 
> > This tag is intended to be pulled by Paul into his rcu/next branch so
> > he can sort the conflicts and base further work on top.
> 
> And this sorting process is now allegedly complete and available on the
> -rcu tree's "dev" branch.  As you might have guessed, the major source
> of conflicts were with Joel's patches, including one conflict that was
> invisible to "git rebase":
> 
> 1b2530e7d0c3 ("Revert b8c17e6664c4 ("rcu: Maintain special bits at bottom of 
> ->dynticks counter")
> 03f31532d0ce ("rcu/tree: Add better tracing for dyntick-idle")
> a309d5ce2335 ("rcu/tree: Clean up dynticks counter usage")
> 5c6e734fbaeb ("rcu/tree: Remove dynticks_nmi_nesting counter")
> 
> This passes modest rcutorture testing.  So far, so good!  ;-)

Also I double checked these patches in the rcu/dev branch. It looks good to
me, thanks for resolving the conflicts!!

 - Joel



Re: [patch V6 00/37] x86/entry: Rework leftovers and merge plan

2020-05-19 Thread Thomas Gleixner
Peter Zijlstra  writes:
> On Mon, May 18, 2020 at 10:24:53PM +0200, Thomas Gleixner wrote:
> So tglx/entry-v8-full + below patch:
>
> $ make O=defconfig-build clean
> ...
> $ make CC=gcc-9 O=defconfig-build/ vmlinux -j40 -s
> vmlinux.o: warning: objtool: exc_debug()+0x158: call to 
> trace_hwlat_timestamp() leaves .noinstr.text section
> vmlinux.o: warning: objtool: exc_nmi()+0x190: call to trace_hwlat_timestamp() 
> leaves .noinstr.text section
> vmlinux.o: warning: objtool: do_machine_check()+0x46: call to mce_rdmsrl() 
> leaves .noinstr.text section
> $
>
> (it really isn't defconfig, but your config-fail + DEBUG_ENTRY)
>  
> +#ifdef CONFIG_DEBUG_ENTRY
>  /* Begin/end of an instrumentation safe region */
> -#define instrumentation_begin() ({   
> \
> +#define instrumentation_begin() ({   \
>   asm volatile("%c0:\n\t" \
>".pushsection .discard.instr_begin\n\t"\
>".long %c0b - .\n\t"   \
>".popsection\n\t" : : "i" (__COUNTER__));  \
>  })
>  
> -#define instrumentation_end() ({ 
> \
> - asm volatile("%c0:\n\t" \
> +#define instrumentation_end() ({ \
> + asm volatile("%c0: nop\n\t" \

Bah. I fatfingered that nop out when I fixed up that noinstr wreckage.
With that added back it does was it claims to do.


Re: [patch V6 00/37] x86/entry: Rework leftovers and merge plan

2020-05-19 Thread Peter Zijlstra
On Tue, May 19, 2020 at 10:38:26AM +0200, Peter Zijlstra wrote:
> $ make CC=gcc-9 O=defconfig-build/ vmlinux -j40 -s
> vmlinux.o: warning: objtool: exc_debug()+0x158: call to 
> trace_hwlat_timestamp() leaves .noinstr.text section
> vmlinux.o: warning: objtool: exc_nmi()+0x190: call to trace_hwlat_timestamp() 
> leaves .noinstr.text section
> vmlinux.o: warning: objtool: do_machine_check()+0x46: call to mce_rdmsrl() 
> leaves .noinstr.text section
> $
> 
> (it really isn't defconfig, but your config-fail + DEBUG_ENTRY)
> 

With comment on, as requested.

---
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index f128e5c2ed42..fb34ff641e0a 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -79,8 +79,8 @@ do {  
\
 do {   \
instrumentation_begin();\
_BUG_FLAGS(ASM_UD2, BUGFLAG_WARNING|(flags));   \
-   instrumentation_end();  \
annotate_reachable();   \
+   instrumentation_end();  \
 } while (0)
 
 #include 
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 7db5902f8f6e..4b8fabed46ae 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -120,25 +120,61 @@ void ftrace_likely_update(struct ftrace_likely_data *f, 
int val,
 /* Annotate a C jump table to allow objtool to follow the code flow */
 #define __annotate_jump_table __section(.rodata..c_jump_table)
 
+#ifdef CONFIG_DEBUG_ENTRY
 /* Begin/end of an instrumentation safe region */
-#define instrumentation_begin() ({ 
\
+#define instrumentation_begin() ({ \
asm volatile("%c0:\n\t" \
 ".pushsection .discard.instr_begin\n\t"\
 ".long %c0b - .\n\t"   \
 ".popsection\n\t" : : "i" (__COUNTER__));  \
 })
 
-#define instrumentation_end() ({   
\
-   asm volatile("%c0:\n\t" \
+/*
+ * Because instrumentation_{begin,end}() can nest, objtool validation considers
+ * _begin() a +1 and _end() a -1 and computes a sum over the instructions.
+ * When the value is greater than 0, we consider instrumentation allowed.
+ *
+ * There is a problem with code like:
+ *
+ * noinstr void foo()
+ * {
+ * instrumentation_begin();
+ * ...
+ * if (cond) {
+ * instrumentation_begin();
+ * ...
+ * instrumentation_end();
+ * }
+ * bar();
+ * instrumentation_end();
+ * }
+ *
+ * If instrumentation_end() would be an empty label, like all the other
+ * annotations, the inner _end(), which is at the end of a conditional block,
+ * would land on the instruction after the block.
+ *
+ * If we then consider the sum of the !cond path, we'll see that the call to
+ * bar() is with a 0-value, even though, we meant it to happen with a positive
+ * value.
+ *
+ * To avoid this, have _end() be a NOP instruction, this ensures it will be
+ * part of the condition block and does not escape.
+ */
+#define instrumentation_end() ({   \
+   asm volatile("%c0: nop\n\t" \
 ".pushsection .discard.instr_end\n\t"  \
 ".long %c0b - .\n\t"   \
 ".popsection\n\t" : : "i" (__COUNTER__));  \
 })
+#endif /* CONFIG_DEBUG_ENTRY */
 
 #else
 #define annotate_reachable()
 #define annotate_unreachable()
 #define __annotate_jump_table
+#endif
+
+#ifndef instrumentation_begin
 #define instrumentation_begin()do { } while(0)
 #define instrumentation_end()  do { } while(0)
 #endif


Re: [patch V6 00/37] x86/entry: Rework leftovers and merge plan

2020-05-19 Thread Peter Zijlstra
On Mon, May 18, 2020 at 10:24:53PM +0200, Thomas Gleixner wrote:
> Peter Zijlstra  writes:
> > So on top of you entry-v8-full; I had to chase one of those
> > instrumentation_end() escapes an (extended) basic block chase (again!).
> >
> > --- a/arch/x86/include/asm/bug.h
> > +++ b/arch/x86/include/asm/bug.h
> > @@ -79,8 +79,8 @@ do {  
> > \
> >  do {   \
> > instrumentation_begin();\
> > _BUG_FLAGS(ASM_UD2, BUGFLAG_WARNING|(flags));   \
> > -   instrumentation_end();  \
> > annotate_reachable();   \
> > +   instrumentation_end();  \
> >  } while (0)
> 
> I just applied this part and rebuilt:
> 
>  vmlinux.o: warning: objtool: rcu_eqs_enter.constprop.77()+0xa9: call to
>  rcu_preempt_deferred_qs() leaves .noinstr.text section
> 
> Did it go away after you disabled DEBUG_ENTRY perhaps?

Hehe, then all complaints would be gone :-)

So tglx/entry-v8-full + below patch:

$ make O=defconfig-build clean
...
$ make CC=gcc-9 O=defconfig-build/ vmlinux -j40 -s
vmlinux.o: warning: objtool: exc_debug()+0x158: call to trace_hwlat_timestamp() 
leaves .noinstr.text section
vmlinux.o: warning: objtool: exc_nmi()+0x190: call to trace_hwlat_timestamp() 
leaves .noinstr.text section
vmlinux.o: warning: objtool: do_machine_check()+0x46: call to mce_rdmsrl() 
leaves .noinstr.text section
$

(it really isn't defconfig, but your config-fail + DEBUG_ENTRY)

---
 arch/x86/include/asm/bug.h |  2 +-
 include/linux/compiler.h   | 11 ---
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index f128e5c2ed42..fb34ff641e0a 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -79,8 +79,8 @@ do {  
\
 do {   \
instrumentation_begin();\
_BUG_FLAGS(ASM_UD2, BUGFLAG_WARNING|(flags));   \
-   instrumentation_end();  \
annotate_reachable();   \
+   instrumentation_end();  \
 } while (0)
 
 #include 
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 7db5902f8f6e..f6f54e8e0797 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -120,25 +120,30 @@ void ftrace_likely_update(struct ftrace_likely_data *f, 
int val,
 /* Annotate a C jump table to allow objtool to follow the code flow */
 #define __annotate_jump_table __section(.rodata..c_jump_table)
 
+#ifdef CONFIG_DEBUG_ENTRY
 /* Begin/end of an instrumentation safe region */
-#define instrumentation_begin() ({ 
\
+#define instrumentation_begin() ({ \
asm volatile("%c0:\n\t" \
 ".pushsection .discard.instr_begin\n\t"\
 ".long %c0b - .\n\t"   \
 ".popsection\n\t" : : "i" (__COUNTER__));  \
 })
 
-#define instrumentation_end() ({   
\
-   asm volatile("%c0:\n\t" \
+#define instrumentation_end() ({   \
+   asm volatile("%c0: nop\n\t" \
 ".pushsection .discard.instr_end\n\t"  \
 ".long %c0b - .\n\t"   \
 ".popsection\n\t" : : "i" (__COUNTER__));  \
 })
+#endif /* CONFIG_DEBUG_ENTRY */
 
 #else
 #define annotate_reachable()
 #define annotate_unreachable()
 #define __annotate_jump_table
+#endif
+
+#ifndef instrumentation_begin
 #define instrumentation_begin()do { } while(0)
 #define instrumentation_end()  do { } while(0)
 #endif


Re: [patch V6 00/37] x86/entry: Rework leftovers and merge plan

2020-05-19 Thread Peter Zijlstra
On Mon, May 18, 2020 at 08:53:49PM +0200, Thomas Gleixner wrote:
> Peter Zijlstra  writes:
> > So on top of you entry-v8-full; I had to chase one of those
> > instrumentation_end() escapes an (extended) basic block chase (again!).
> >  
> > +#ifdef CONFIG_DEBUG_ENTRY
> 
> Why this? We lose the kprobes runtime protection that way.

Oh bugger indeed. I forgot about that :-(

I added the CONFIG_DEBUG_ENTRY dependency to
instrumentation_{begin,end}() because they now emit actual code, and I
figured we shouldn't bother 'production' kernels with all them extra
NOPs.

And then I figured (wrongly!) that since I have that, I might as well
add noinstr to is.

> > +/* Section for code which can't be instrumented at all */
> > +#define noinstr
> > \
> > +   noinline notrace __attribute((__section__(".noinstr.text")))
> > +
> >  /* Begin/end of an instrumentation safe region */
> > -#define instrumentation_begin() ({ 
> > \
> > +#define instrumentation_begin() ({ \
> > asm volatile("%c0:\n\t" \
> >  ".pushsection .discard.instr_begin\n\t"\
> >  ".long %c0b - .\n\t"   \
> >  ".popsection\n\t" : : "i" (__COUNTER__));
> 
> Nifty.

Yeah, took a bit of fiddling because objtool is a bit weird vs UD2, but
if you order it just right in the WARN thing it works :-)

You want a new delta without the noinstr thing on?


Re: [patch V6 00/37] x86/entry: Rework leftovers and merge plan

2020-05-18 Thread Thomas Gleixner
Peter Zijlstra  writes:
> So on top of you entry-v8-full; I had to chase one of those
> instrumentation_end() escapes an (extended) basic block chase (again!).
>
> --- a/arch/x86/include/asm/bug.h
> +++ b/arch/x86/include/asm/bug.h
> @@ -79,8 +79,8 @@ do {
> \
>  do { \
>   instrumentation_begin();\
>   _BUG_FLAGS(ASM_UD2, BUGFLAG_WARNING|(flags));   \
> - instrumentation_end();  \
>   annotate_reachable();   \
> + instrumentation_end();  \
>  } while (0)

I just applied this part and rebuilt:

 vmlinux.o: warning: objtool: rcu_eqs_enter.constprop.77()+0xa9: call to
 rcu_preempt_deferred_qs() leaves .noinstr.text section

Did it go away after you disabled DEBUG_ENTRY perhaps?


Re: [patch V6 00/37] x86/entry: Rework leftovers and merge plan

2020-05-18 Thread Thomas Gleixner
Peter Zijlstra  writes:
> So on top of you entry-v8-full; I had to chase one of those
> instrumentation_end() escapes an (extended) basic block chase (again!).
>  
> +#ifdef CONFIG_DEBUG_ENTRY

Why this? We lose the kprobes runtime protection that way.

> +/* Section for code which can't be instrumented at all */
> +#define noinstr  
> \
> + noinline notrace __attribute((__section__(".noinstr.text")))
> +
>  /* Begin/end of an instrumentation safe region */
> -#define instrumentation_begin() ({   
> \
> +#define instrumentation_begin() ({   \
>   asm volatile("%c0:\n\t" \
>".pushsection .discard.instr_begin\n\t"\
>".long %c0b - .\n\t"   \
>".popsection\n\t" : : "i" (__COUNTER__));

Nifty.

Thanks,

tglx


Re: [patch V6 00/37] x86/entry: Rework leftovers and merge plan

2020-05-18 Thread Peter Zijlstra


So on top of you entry-v8-full; I had to chase one of those
instrumentation_end() escapes an (extended) basic block chase (again!).

How about we do something like the below; that fixes the current case
(rcu_eqs_enter) but also kills the entire class.

---
 arch/x86/include/asm/bug.h |  2 +-
 include/linux/compiler.h   | 16 +---
 include/linux/compiler_types.h |  4 
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index f128e5c2ed42..fb34ff641e0a 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -79,8 +79,8 @@ do {  
\
 do {   \
instrumentation_begin();\
_BUG_FLAGS(ASM_UD2, BUGFLAG_WARNING|(flags));   \
-   instrumentation_end();  \
annotate_reachable();   \
+   instrumentation_end();  \
 } while (0)
 
 #include 
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 7db5902f8f6e..b4c248e6b76a 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -120,25 +120,35 @@ void ftrace_likely_update(struct ftrace_likely_data *f, 
int val,
 /* Annotate a C jump table to allow objtool to follow the code flow */
 #define __annotate_jump_table __section(.rodata..c_jump_table)
 
+#ifdef CONFIG_DEBUG_ENTRY
+/* Section for code which can't be instrumented at all */
+#define noinstr
\
+   noinline notrace __attribute((__section__(".noinstr.text")))
+
 /* Begin/end of an instrumentation safe region */
-#define instrumentation_begin() ({ 
\
+#define instrumentation_begin() ({ \
asm volatile("%c0:\n\t" \
 ".pushsection .discard.instr_begin\n\t"\
 ".long %c0b - .\n\t"   \
 ".popsection\n\t" : : "i" (__COUNTER__));  \
 })
 
-#define instrumentation_end() ({   
\
-   asm volatile("%c0:\n\t" \
+#define instrumentation_end() ({   \
+   asm volatile("%c0: nop\n\t" \
 ".pushsection .discard.instr_end\n\t"  \
 ".long %c0b - .\n\t"   \
 ".popsection\n\t" : : "i" (__COUNTER__));  \
 })
+#endif /* CONFIG_DEBUG_ENTRY */
 
 #else
 #define annotate_reachable()
 #define annotate_unreachable()
 #define __annotate_jump_table
+#endif
+
+#ifndef noinstr
+#define noinstr noinline notrace
 #define instrumentation_begin()do { } while(0)
 #define instrumentation_end()  do { } while(0)
 #endif
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index ea15ea99efb4..6ed0612bc143 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -118,10 +118,6 @@ struct ftrace_likely_data {
 #define notrace
__attribute__((__no_instrument_function__))
 #endif
 
-/* Section for code which can't be instrumented at all */
-#define noinstr
\
-   noinline notrace __attribute((__section__(".noinstr.text")))
-
 /*
  * it doesn't make sense on ARM (currently the only user of __naked)
  * to trace naked functions because then mcount is called without


Re: [patch V6 00/37] x86/entry: Rework leftovers and merge plan

2020-05-16 Thread Paul E. McKenney
On Sat, May 16, 2020 at 01:45:47AM +0200, Thomas Gleixner wrote:

[ . . . ]

>   - noinstr-rcu-nmi-2020-05-15
> 
> Based on the core/rcu branch in the tip tree. It has merged in
> noinstr-lds-2020-05-15 and contains the nmi_enter/exit() changes along
> with the noinstr section changes on top.
> 
> This tag is intended to be pulled by Paul into his rcu/next branch so
> he can sort the conflicts and base further work on top.

And this sorting process is now allegedly complete and available on the
-rcu tree's "dev" branch.  As you might have guessed, the major source
of conflicts were with Joel's patches, including one conflict that was
invisible to "git rebase":

1b2530e7d0c3 ("Revert b8c17e6664c4 ("rcu: Maintain special bits at bottom of 
->dynticks counter")
03f31532d0ce ("rcu/tree: Add better tracing for dyntick-idle")
a309d5ce2335 ("rcu/tree: Clean up dynticks counter usage")
5c6e734fbaeb ("rcu/tree: Remove dynticks_nmi_nesting counter")

This passes modest rcutorture testing.  So far, so good!  ;-)

Thanx, Paul


[patch V6 00/37] x86/entry: Rework leftovers and merge plan

2020-05-15 Thread Thomas Gleixner
Folks!

This is V6 of the rework series. V5 can be found here:

  https://lore.kernel.org/r/20200512210059.056244...@linutronix.de

The V6 leftover series is based on:

  git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git entry-base-v6

which is the reworked base series from part 1-4 of the original 5 part
series with a few changes which are described in detail below in the merge
plan section.

V6 has the following changes vs. V5:

- Rebased on top entry-base-v6

- Addressed Stevens request to split up the hardware latency detector.
  This are 3 patches now as I couldn't resist to cleanup the
  timestamping mess in that code before splitting it up.

- Dropped the KVM/SVM change as that is going to be routed
  differently. See below.

The full series is available from:

  git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git entry-v6-the-rest

On top of that the kvm changes are applied for completeness and available
from:

  git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git entry-v6-full


Merge plan:
---

After figuring out that the entry pile and next are not really happy with
each other, I spent quite some time to come up with a plan.

The goal was to:

- not let Stephen Rothwell grow more grey hair when trying to resolve
  the conflicts

- allow the affected trees (RCU and KVM) to take a small part of the
  series into their trees while making sure that the x86/entry branch
  is complete and contains the required RCU and KVM changes as well.

About 10 hours of patch tetris later the solution looks like this:

  I've reshuffled the patches so that they are grouped by subsystem instead
  of having the cross tree/subsystem patches close to the actual usage site
  in the x86/entry series.

  This allowed me to tag these subsytem parts and they contain just the
  minimal subset of changes to be able to build and boot.

The resulting tag list is:

  - noinstr-lds-2020-05-15

A single commit containing the vmlinux.lds.h change which introduces
the noinstr.text section.

  - noinstr-core-2020-05-15

Based on noinstr-lds-2020-05-15 and contains the core changes

  - noinstr-core-for-kvm-2020-05-15

Subset of noinstr-core-2020-05-15 which is required to let KVM pull
the KVM async pagefault cleanup and base the guest_enter/exit() and
noinstr changes on top.

  - noinstr-rcu-nmi-2020-05-15

Based on the core/rcu branch in the tip tree. It has merged in
noinstr-lds-2020-05-15 and contains the nmi_enter/exit() changes along
with the noinstr section changes on top.

This tag is intended to be pulled by Paul into his rcu/next branch so
he can sort the conflicts and base further work on top.

  - noinstr-core-2020-05-15

Based on noinstr-core-for-kvm-2020-05-15 and contains the async page
fault cleanup which goes into x86/entry so the IDTENTRY conversion of
#PF which also touches the async pagefault code can be applied on top

This tag is intended to be pulled by Paolo into his next branch so he
can work against these changes and the merged result is also target for
the rebased version of the KVM guest_enter/exit() changes. These are
not part of the entry-v6-base tag. I'm going to post them as a separate
series because the original ones are conflicting with work in that area
in the KVM tree.

  - noinstr-kcsan-2020-05015, noinstr-kprobes-2020-05-15,
noinstr-objtool-2020-05-15

TIP tree internal tags which I added to reduce the brain-melt.

The x86/entry branch is based on the TIP x86/entry branch and has the
following branches and tags merged and patches from part 1-4 applied:

- x86/asm because this has conflicting changes vs. #DF

- A small set of preparatory changes and fixes which are independent
  of the noinstr mechanics

- noinstr-objtool-2020-05-15
- noinstr-core-2020-05-15
- noinstr-kprobes-2020-05-15
- noinstr-rcu-nmi-2020-05-15
- noinstr-kcsan-2020-05015
- noinstr-x86-kvm-2020-05-15

- The part 1-4 patches up to

51336ff8b658 ("x86/entry: Convert double fault exception to 
IDTENTRY_DF")

  This is tagged as entry-v6-base

The remaining patches in this leftover series will be applied on top.

If this works for all maintainers involved, then I'm going to pull the tags
and branches into the tip-tree which makes them immutable.

If not, please tell me ASAP that I should restart the patch tetris session
after hiding in a brown paperbag for some time to recover from brain melt.

Thanks,

tglx