Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Wed, Jun 24, 2020 at 09:52:49AM +0200, Peter Zijlstra wrote:
> On Tue, Jun 23, 2020 at 02:44:33PM -0700, Paul E. McKenney wrote:
> > On Tue, Jun 23, 2020 at 10:46:46PM +0200, Peter Zijlstra wrote:
> > > On Fri, Jun 19, 2020 at 03:15:55PM -0700, Paul E. McKenney wrote:
> > >
> > > > Just following up because I don't see this anywhere. If I am supposed
> > > > to take this (which is more plausible now that v5.8-rc1 is out), please
> > > > let me know.
> > >
> > > Sorry, I got distracted by that NULL ptr thing, but that seems sorted
> > > now. If you don't mind taking it through your rcu/urgent tree for -rc3
> > > or so that would be awesome.
> >
> > Will do!
> >
> > Just to double-check, this is the patch from you with Message-ID
> > 20200603114051.896465...@infradead.org, correct?
> >
> > Or, if you prefer, this commit now on -rcu?
> >
> > 5fe289eccfe5 ("rcu: Fixup noinstr warnings")
> >
> > If this is the correct commit, I will rebase it on top of v5.8-rc2,
> > and if it passes tests, send it along via rcu/urgent.
>
> Ah, I was thinking about:
>
>
> https://lore.kernel.org/lkml/20200615162427.gi2...@hirez.programming.kicks-ass.net/
>
> seeing how I added that instrumentation you wanted :-), but either
> version should work for now. KCSAN is sad without this.
Glad I asked! I will substitute the one you pointed out above.
Thanx, Paul
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Tue, Jun 23, 2020 at 02:44:33PM -0700, Paul E. McKenney wrote:
> On Tue, Jun 23, 2020 at 10:46:46PM +0200, Peter Zijlstra wrote:
> > On Fri, Jun 19, 2020 at 03:15:55PM -0700, Paul E. McKenney wrote:
> >
> > > Just following up because I don't see this anywhere. If I am supposed
> > > to take this (which is more plausible now that v5.8-rc1 is out), please
> > > let me know.
> >
> > Sorry, I got distracted by that NULL ptr thing, but that seems sorted
> > now. If you don't mind taking it through your rcu/urgent tree for -rc3
> > or so that would be awesome.
>
> Will do!
>
> Just to double-check, this is the patch from you with Message-ID
> 20200603114051.896465...@infradead.org, correct?
>
> Or, if you prefer, this commit now on -rcu?
>
> 5fe289eccfe5 ("rcu: Fixup noinstr warnings")
>
> If this is the correct commit, I will rebase it on top of v5.8-rc2,
> and if it passes tests, send it along via rcu/urgent.
Ah, I was thinking about:
https://lore.kernel.org/lkml/20200615162427.gi2...@hirez.programming.kicks-ass.net/
seeing how I added that instrumentation you wanted :-), but either
version should work for now. KCSAN is sad without this.
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Tue, Jun 23, 2020 at 10:46:46PM +0200, Peter Zijlstra wrote:
> On Fri, Jun 19, 2020 at 03:15:55PM -0700, Paul E. McKenney wrote:
>
> > Just following up because I don't see this anywhere. If I am supposed
> > to take this (which is more plausible now that v5.8-rc1 is out), please
> > let me know.
>
> Sorry, I got distracted by that NULL ptr thing, but that seems sorted
> now. If you don't mind taking it through your rcu/urgent tree for -rc3
> or so that would be awesome.
Will do!
Just to double-check, this is the patch from you with Message-ID
20200603114051.896465...@infradead.org, correct?
Or, if you prefer, this commit now on -rcu?
5fe289eccfe5 ("rcu: Fixup noinstr warnings")
If this is the correct commit, I will rebase it on top of v5.8-rc2,
and if it passes tests, send it along via rcu/urgent.
Thanx, Paul
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Fri, Jun 19, 2020 at 03:15:55PM -0700, Paul E. McKenney wrote: > Just following up because I don't see this anywhere. If I am supposed > to take this (which is more plausible now that v5.8-rc1 is out), please > let me know. Sorry, I got distracted by that NULL ptr thing, but that seems sorted now. If you don't mind taking it through your rcu/urgent tree for -rc3 or so that would be awesome.
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Mon, Jun 15, 2020 at 10:14:04AM -0700, Paul E. McKenney wrote:
> On Mon, Jun 15, 2020 at 06:24:27PM +0200, Peter Zijlstra wrote:
> > On Mon, Jun 15, 2020 at 05:55:13PM +0200, Peter Zijlstra wrote:
> > > On Mon, Jun 15, 2020 at 05:49:05PM +0200, Peter Zijlstra wrote:
> > > > @@ -983,13 +993,17 @@ noinstr void rcu_nmi_enter(void)
> > > > if (!in_nmi())
> > > > rcu_cleanup_after_idle();
> > > >
> > > > + instrumentation_begin();
> > > > + // instrumentation for the noinstr
> > > > rcu_dynticks_curr_cpu_in_eqs()
> > > > + instrument_atomic_read(>dynticks,
> > > > sizeof(rdp->dynticks));
> > > > + // instrumentation for the noinstr
> > > > rcu_dynticks_eqs_exit()
> > > > + instrument_atomic_write(>dynticks,
> > > > sizeof(rdp->dynticks));
> > > > +
> > > > incby = 1;
> > > > } else if (!in_nmi()) {
> > > > instrumentation_begin();
> > > > rcu_irq_enter_check_tick();
> > > > - instrumentation_end();
> > > > }
> > > > - instrumentation_begin();
> > > > trace_rcu_dyntick(incby == 1 ? TPS("Endirq") : TPS("++="),
> > > > rdp->dynticks_nmi_nesting,
> > > > rdp->dynticks_nmi_nesting + incby,
> > > > atomic_read(>dynticks));
> > >
> > > Oh, that's lost a possible instrumentation_begin() :/ But weirdly
> > > objtool didn't complain about that... Let me poke at that.
>
> This merge window has been quite the trainwreck, hasn't it? :-/
>
> > Like so then...
>
> Looks plausible, firing up some tests.
Just following up because I don't see this anywhere. If I am supposed
to take this (which is more plausible now that v5.8-rc1 is out), please
let me know.
Thanx, Paul
> > ---
> > Subject: rcu: Fixup noinstr warnings
> >
> > A KCSAN build revealed we have explicit annoations through atomic_*()
> > usage, switch to arch_atomic_*() for the respective functions.
> >
> > vmlinux.o: warning: objtool: rcu_nmi_exit()+0x4d: call to
> > __kcsan_check_access() leaves .noinstr.text section
> > vmlinux.o: warning: objtool: rcu_dynticks_eqs_enter()+0x25: call to
> > __kcsan_check_access() leaves .noinstr.text section
> > vmlinux.o: warning: objtool: rcu_nmi_enter()+0x4f: call to
> > __kcsan_check_access() leaves .noinstr.text section
> > vmlinux.o: warning: objtool: rcu_dynticks_eqs_exit()+0x2a: call to
> > __kcsan_check_access() leaves .noinstr.text section
> > vmlinux.o: warning: objtool: __rcu_is_watching()+0x25: call to
> > __kcsan_check_access() leaves .noinstr.text section
> >
> > Additionally, without the NOP in instrumentation_begin(), objtool would
> > not detect the lack of the 'else instrumentation_begin();' branch in
> > rcu_nmi_enter().
> >
> > Signed-off-by: Peter Zijlstra (Intel)
> > ---
> > include/linux/compiler.h |2 +-
> > kernel/rcu/tree.c| 33 +
> > 2 files changed, 26 insertions(+), 9 deletions(-)
> >
> > --- a/include/linux/compiler.h
> > +++ b/include/linux/compiler.h
> > @@ -123,7 +123,7 @@ void ftrace_likely_update(struct ftrace_
> > #ifdef CONFIG_DEBUG_ENTRY
> > /* Begin/end of an instrumentation safe region */
> > #define instrumentation_begin() ({ \
> > - asm volatile("%c0:\n\t" \
> > + asm volatile("%c0: nop\n\t"
> > \
> > ".pushsection .discard.instr_begin\n\t"\
> > ".long %c0b - .\n\t" \
> > ".popsection\n\t" : : "i" (__COUNTER__)); \
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -250,7 +250,7 @@ static noinstr void rcu_dynticks_eqs_ent
> > * next idle sojourn.
> > */
> > rcu_dynticks_task_trace_enter(); // Before ->dynticks update!
> > - seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > + seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > // RCU is no longer watching. Better be in extended quiescent state!
> > WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > (seq & RCU_DYNTICK_CTRL_CTR));
> > @@ -274,13 +274,13 @@ static noinstr void rcu_dynticks_eqs_exi
> > * and we also must force ordering with the next RCU read-side
> > * critical section.
> > */
> > - seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > + seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > // RCU is now watching. Better not be in an extended quiescent state!
> > rcu_dynticks_task_trace_exit(); // After ->dynticks update!
> > WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > !(seq & RCU_DYNTICK_CTRL_CTR));
> > if (seq & RCU_DYNTICK_CTRL_MASK) {
> > -
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Mon, Jun 15, 2020 at 10:14:04AM -0700, Paul E. McKenney wrote:
> On Mon, Jun 15, 2020 at 06:24:27PM +0200, Peter Zijlstra wrote:
> > On Mon, Jun 15, 2020 at 05:55:13PM +0200, Peter Zijlstra wrote:
> > > On Mon, Jun 15, 2020 at 05:49:05PM +0200, Peter Zijlstra wrote:
> > > > @@ -983,13 +993,17 @@ noinstr void rcu_nmi_enter(void)
> > > > if (!in_nmi())
> > > > rcu_cleanup_after_idle();
> > > >
> > > > + instrumentation_begin();
> > > > + // instrumentation for the noinstr
> > > > rcu_dynticks_curr_cpu_in_eqs()
> > > > + instrument_atomic_read(>dynticks,
> > > > sizeof(rdp->dynticks));
> > > > + // instrumentation for the noinstr
> > > > rcu_dynticks_eqs_exit()
> > > > + instrument_atomic_write(>dynticks,
> > > > sizeof(rdp->dynticks));
> > > > +
> > > > incby = 1;
> > > > } else if (!in_nmi()) {
> > > > instrumentation_begin();
> > > > rcu_irq_enter_check_tick();
> > > > - instrumentation_end();
> > > > }
> > > > - instrumentation_begin();
> > > > trace_rcu_dyntick(incby == 1 ? TPS("Endirq") : TPS("++="),
> > > > rdp->dynticks_nmi_nesting,
> > > > rdp->dynticks_nmi_nesting + incby,
> > > > atomic_read(>dynticks));
> > >
> > > Oh, that's lost a possible instrumentation_begin() :/ But weirdly
> > > objtool didn't complain about that... Let me poke at that.
>
> This merge window has been quite the trainwreck, hasn't it? :-/
>
> > Like so then...
>
> Looks plausible, firing up some tests.
And it passes light rcutorture testing across all the scenarios.
So looks even more plausible. ;-)
Thanx, Paul
> > ---
> > Subject: rcu: Fixup noinstr warnings
> >
> > A KCSAN build revealed we have explicit annoations through atomic_*()
> > usage, switch to arch_atomic_*() for the respective functions.
> >
> > vmlinux.o: warning: objtool: rcu_nmi_exit()+0x4d: call to
> > __kcsan_check_access() leaves .noinstr.text section
> > vmlinux.o: warning: objtool: rcu_dynticks_eqs_enter()+0x25: call to
> > __kcsan_check_access() leaves .noinstr.text section
> > vmlinux.o: warning: objtool: rcu_nmi_enter()+0x4f: call to
> > __kcsan_check_access() leaves .noinstr.text section
> > vmlinux.o: warning: objtool: rcu_dynticks_eqs_exit()+0x2a: call to
> > __kcsan_check_access() leaves .noinstr.text section
> > vmlinux.o: warning: objtool: __rcu_is_watching()+0x25: call to
> > __kcsan_check_access() leaves .noinstr.text section
> >
> > Additionally, without the NOP in instrumentation_begin(), objtool would
> > not detect the lack of the 'else instrumentation_begin();' branch in
> > rcu_nmi_enter().
> >
> > Signed-off-by: Peter Zijlstra (Intel)
> > ---
> > include/linux/compiler.h |2 +-
> > kernel/rcu/tree.c| 33 +
> > 2 files changed, 26 insertions(+), 9 deletions(-)
> >
> > --- a/include/linux/compiler.h
> > +++ b/include/linux/compiler.h
> > @@ -123,7 +123,7 @@ void ftrace_likely_update(struct ftrace_
> > #ifdef CONFIG_DEBUG_ENTRY
> > /* Begin/end of an instrumentation safe region */
> > #define instrumentation_begin() ({ \
> > - asm volatile("%c0:\n\t" \
> > + asm volatile("%c0: nop\n\t"
> > \
> > ".pushsection .discard.instr_begin\n\t"\
> > ".long %c0b - .\n\t" \
> > ".popsection\n\t" : : "i" (__COUNTER__)); \
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -250,7 +250,7 @@ static noinstr void rcu_dynticks_eqs_ent
> > * next idle sojourn.
> > */
> > rcu_dynticks_task_trace_enter(); // Before ->dynticks update!
> > - seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > + seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > // RCU is no longer watching. Better be in extended quiescent state!
> > WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > (seq & RCU_DYNTICK_CTRL_CTR));
> > @@ -274,13 +274,13 @@ static noinstr void rcu_dynticks_eqs_exi
> > * and we also must force ordering with the next RCU read-side
> > * critical section.
> > */
> > - seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > + seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > // RCU is now watching. Better not be in an extended quiescent state!
> > rcu_dynticks_task_trace_exit(); // After ->dynticks update!
> > WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > !(seq & RCU_DYNTICK_CTRL_CTR));
> > if (seq & RCU_DYNTICK_CTRL_MASK) {
> > - atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
> > +
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Mon, Jun 15, 2020 at 08:33:25PM +0200, Peter Zijlstra wrote: > On Mon, Jun 15, 2020 at 10:14:04AM -0700, Paul E. McKenney wrote: > > > This merge window has been quite the trainwreck, hasn't it? :-/ > > Keeps life interesting I suppose.. ;-) ;-) ;-) Thanx, Paul
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Mon, Jun 15, 2020 at 10:14:04AM -0700, Paul E. McKenney wrote: > This merge window has been quite the trainwreck, hasn't it? :-/ Keeps life interesting I suppose..
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Mon, Jun 15, 2020 at 06:24:27PM +0200, Peter Zijlstra wrote:
> On Mon, Jun 15, 2020 at 05:55:13PM +0200, Peter Zijlstra wrote:
> > On Mon, Jun 15, 2020 at 05:49:05PM +0200, Peter Zijlstra wrote:
> > > @@ -983,13 +993,17 @@ noinstr void rcu_nmi_enter(void)
> > > if (!in_nmi())
> > > rcu_cleanup_after_idle();
> > >
> > > + instrumentation_begin();
> > > + // instrumentation for the noinstr
> > > rcu_dynticks_curr_cpu_in_eqs()
> > > + instrument_atomic_read(>dynticks, sizeof(rdp->dynticks));
> > > + // instrumentation for the noinstr rcu_dynticks_eqs_exit()
> > > + instrument_atomic_write(>dynticks, sizeof(rdp->dynticks));
> > > +
> > > incby = 1;
> > > } else if (!in_nmi()) {
> > > instrumentation_begin();
> > > rcu_irq_enter_check_tick();
> > > - instrumentation_end();
> > > }
> > > - instrumentation_begin();
> > > trace_rcu_dyntick(incby == 1 ? TPS("Endirq") : TPS("++="),
> > > rdp->dynticks_nmi_nesting,
> > > rdp->dynticks_nmi_nesting + incby,
> > > atomic_read(>dynticks));
> >
> > Oh, that's lost a possible instrumentation_begin() :/ But weirdly
> > objtool didn't complain about that... Let me poke at that.
This merge window has been quite the trainwreck, hasn't it? :-/
> Like so then...
Looks plausible, firing up some tests.
Thanx, Paul
> ---
> Subject: rcu: Fixup noinstr warnings
>
> A KCSAN build revealed we have explicit annoations through atomic_*()
> usage, switch to arch_atomic_*() for the respective functions.
>
> vmlinux.o: warning: objtool: rcu_nmi_exit()+0x4d: call to
> __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: rcu_dynticks_eqs_enter()+0x25: call to
> __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: rcu_nmi_enter()+0x4f: call to
> __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: rcu_dynticks_eqs_exit()+0x2a: call to
> __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: __rcu_is_watching()+0x25: call to
> __kcsan_check_access() leaves .noinstr.text section
>
> Additionally, without the NOP in instrumentation_begin(), objtool would
> not detect the lack of the 'else instrumentation_begin();' branch in
> rcu_nmi_enter().
>
> Signed-off-by: Peter Zijlstra (Intel)
> ---
> include/linux/compiler.h |2 +-
> kernel/rcu/tree.c| 33 +
> 2 files changed, 26 insertions(+), 9 deletions(-)
>
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -123,7 +123,7 @@ void ftrace_likely_update(struct ftrace_
> #ifdef CONFIG_DEBUG_ENTRY
> /* Begin/end of an instrumentation safe region */
> #define instrumentation_begin() ({ \
> - asm volatile("%c0:\n\t" \
> + asm volatile("%c0: nop\n\t"
> \
>".pushsection .discard.instr_begin\n\t"\
>".long %c0b - .\n\t" \
>".popsection\n\t" : : "i" (__COUNTER__)); \
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -250,7 +250,7 @@ static noinstr void rcu_dynticks_eqs_ent
>* next idle sojourn.
>*/
> rcu_dynticks_task_trace_enter(); // Before ->dynticks update!
> - seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> + seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> // RCU is no longer watching. Better be in extended quiescent state!
> WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
>(seq & RCU_DYNTICK_CTRL_CTR));
> @@ -274,13 +274,13 @@ static noinstr void rcu_dynticks_eqs_exi
>* and we also must force ordering with the next RCU read-side
>* critical section.
>*/
> - seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> + seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> // RCU is now watching. Better not be in an extended quiescent state!
> rcu_dynticks_task_trace_exit(); // After ->dynticks update!
> WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
>!(seq & RCU_DYNTICK_CTRL_CTR));
> if (seq & RCU_DYNTICK_CTRL_MASK) {
> - atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
> + arch_atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
> smp_mb__after_atomic(); /* _exit after clearing mask. */
> }
> }
> @@ -313,7 +313,7 @@ static __always_inline bool rcu_dynticks
> {
> struct rcu_data *rdp = this_cpu_ptr(_data);
>
> - return !(atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
> + return !(arch_atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
> }
>
> /*
> @@ -633,6
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Mon, Jun 15, 2020 at 05:55:13PM +0200, Peter Zijlstra wrote:
> On Mon, Jun 15, 2020 at 05:49:05PM +0200, Peter Zijlstra wrote:
> > @@ -983,13 +993,17 @@ noinstr void rcu_nmi_enter(void)
> > if (!in_nmi())
> > rcu_cleanup_after_idle();
> >
> > + instrumentation_begin();
> > + // instrumentation for the noinstr
> > rcu_dynticks_curr_cpu_in_eqs()
> > + instrument_atomic_read(>dynticks, sizeof(rdp->dynticks));
> > + // instrumentation for the noinstr rcu_dynticks_eqs_exit()
> > + instrument_atomic_write(>dynticks, sizeof(rdp->dynticks));
> > +
> > incby = 1;
> > } else if (!in_nmi()) {
> > instrumentation_begin();
> > rcu_irq_enter_check_tick();
> > - instrumentation_end();
> > }
> > - instrumentation_begin();
> > trace_rcu_dyntick(incby == 1 ? TPS("Endirq") : TPS("++="),
> > rdp->dynticks_nmi_nesting,
> > rdp->dynticks_nmi_nesting + incby,
> > atomic_read(>dynticks));
>
> Oh, that's lost a possible instrumentation_begin() :/ But weirdly
> objtool didn't complain about that... Let me poke at that.
Like so then...
---
Subject: rcu: Fixup noinstr warnings
A KCSAN build revealed we have explicit annoations through atomic_*()
usage, switch to arch_atomic_*() for the respective functions.
vmlinux.o: warning: objtool: rcu_nmi_exit()+0x4d: call to
__kcsan_check_access() leaves .noinstr.text section
vmlinux.o: warning: objtool: rcu_dynticks_eqs_enter()+0x25: call to
__kcsan_check_access() leaves .noinstr.text section
vmlinux.o: warning: objtool: rcu_nmi_enter()+0x4f: call to
__kcsan_check_access() leaves .noinstr.text section
vmlinux.o: warning: objtool: rcu_dynticks_eqs_exit()+0x2a: call to
__kcsan_check_access() leaves .noinstr.text section
vmlinux.o: warning: objtool: __rcu_is_watching()+0x25: call to
__kcsan_check_access() leaves .noinstr.text section
Additionally, without the NOP in instrumentation_begin(), objtool would
not detect the lack of the 'else instrumentation_begin();' branch in
rcu_nmi_enter().
Signed-off-by: Peter Zijlstra (Intel)
---
include/linux/compiler.h |2 +-
kernel/rcu/tree.c| 33 +
2 files changed, 26 insertions(+), 9 deletions(-)
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -123,7 +123,7 @@ void ftrace_likely_update(struct ftrace_
#ifdef CONFIG_DEBUG_ENTRY
/* Begin/end of an instrumentation safe region */
#define instrumentation_begin() ({ \
- asm volatile("%c0:\n\t" \
+ asm volatile("%c0: nop\n\t"
\
".pushsection .discard.instr_begin\n\t"\
".long %c0b - .\n\t" \
".popsection\n\t" : : "i" (__COUNTER__)); \
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -250,7 +250,7 @@ static noinstr void rcu_dynticks_eqs_ent
* next idle sojourn.
*/
rcu_dynticks_task_trace_enter(); // Before ->dynticks update!
- seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
+ seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
// RCU is no longer watching. Better be in extended quiescent state!
WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
(seq & RCU_DYNTICK_CTRL_CTR));
@@ -274,13 +274,13 @@ static noinstr void rcu_dynticks_eqs_exi
* and we also must force ordering with the next RCU read-side
* critical section.
*/
- seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
+ seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
// RCU is now watching. Better not be in an extended quiescent state!
rcu_dynticks_task_trace_exit(); // After ->dynticks update!
WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
!(seq & RCU_DYNTICK_CTRL_CTR));
if (seq & RCU_DYNTICK_CTRL_MASK) {
- atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
+ arch_atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
smp_mb__after_atomic(); /* _exit after clearing mask. */
}
}
@@ -313,7 +313,7 @@ static __always_inline bool rcu_dynticks
{
struct rcu_data *rdp = this_cpu_ptr(_data);
- return !(atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
+ return !(arch_atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
}
/*
@@ -633,6 +633,10 @@ static noinstr void rcu_eqs_enter(bool u
do_nocb_deferred_wakeup(rdp);
rcu_prepare_for_idle();
rcu_preempt_deferred_qs(current);
+
+ // instrumentation for the noinstr rcu_dynticks_eqs_enter()
+ instrument_atomic_write(>dynticks, sizeof(rdp->dynticks));
+
instrumentation_end();
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Mon, Jun 15, 2020 at 08:52:20AM -0700, Paul E. McKenney wrote: > On Mon, Jun 15, 2020 at 05:30:52PM +0200, Peter Zijlstra wrote: > > What shall we do with this patch? > > I plan to submit it to the v5.9 merge window. Do you need it to get > to mainline earlier? Yeah, we need it this round, to make KASAN/KCSAN work with the noinstr stuff that just landed.
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Mon, Jun 15, 2020 at 05:49:05PM +0200, Peter Zijlstra wrote:
> @@ -983,13 +993,17 @@ noinstr void rcu_nmi_enter(void)
> if (!in_nmi())
> rcu_cleanup_after_idle();
>
> + instrumentation_begin();
> + // instrumentation for the noinstr
> rcu_dynticks_curr_cpu_in_eqs()
> + instrument_atomic_read(>dynticks, sizeof(rdp->dynticks));
> + // instrumentation for the noinstr rcu_dynticks_eqs_exit()
> + instrument_atomic_write(>dynticks, sizeof(rdp->dynticks));
> +
> incby = 1;
> } else if (!in_nmi()) {
> instrumentation_begin();
> rcu_irq_enter_check_tick();
> - instrumentation_end();
> }
> - instrumentation_begin();
> trace_rcu_dyntick(incby == 1 ? TPS("Endirq") : TPS("++="),
> rdp->dynticks_nmi_nesting,
> rdp->dynticks_nmi_nesting + incby,
> atomic_read(>dynticks));
Oh, that's lost a possible instrumentation_begin() :/ But weirdly
objtool didn't complain about that... Let me poke at that.
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Mon, Jun 15, 2020 at 05:30:52PM +0200, Peter Zijlstra wrote:
> On Wed, Jun 03, 2020 at 09:46:00AM -0700, Paul E. McKenney wrote:
>
> > > // RCU is now watching. Better not be in an extended quiescent state!
> > > rcu_dynticks_task_trace_exit(); // After ->dynticks update!
> > > WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > >!(seq & RCU_DYNTICK_CTRL_CTR));
> > > if (seq & RCU_DYNTICK_CTRL_MASK) {
> > > - atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
> > > + arch_atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
> >
> > This one is gone in -rcu.
>
> I'm still seeing that in mainline, was that removal scheduled for next
> round?
Yes. Unlike the few commits following it, this commit seems to work
fine even with the recent changes in mainline.
> > > smp_mb__after_atomic(); /* _exit after clearing mask. */
> > > }
> > > }
>
> What shall we do with this patch?
I plan to submit it to the v5.9 merge window. Do you need it to get
to mainline earlier?
Thanx, Paul
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Wed, Jun 03, 2020 at 01:40:16PM +0200, Peter Zijlstra wrote:
> A KCSAN build revealed we have explicit annoations through atomic_*()
> usage, switch to arch_atomic_*() for the respective functions.
>
> vmlinux.o: warning: objtool: rcu_nmi_exit()+0x4d: call to
> __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: rcu_dynticks_eqs_enter()+0x25: call to
> __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: rcu_nmi_enter()+0x4f: call to
> __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: rcu_dynticks_eqs_exit()+0x2a: call to
> __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: __rcu_is_watching()+0x25: call to
> __kcsan_check_access() leaves .noinstr.text section
>
> Signed-off-by: Peter Zijlstra (Intel)
> Link:
> https://lkml.kernel.org/r/20200603084818.gb2...@hirez.programming.kicks-ass.net
> ---
How's this then? It anticipiates the removal of that andnot thing.
---
kernel/rcu/tree.c | 30 ++
1 file changed, 22 insertions(+), 8 deletions(-)
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -250,7 +250,7 @@ static noinstr void rcu_dynticks_eqs_ent
* next idle sojourn.
*/
rcu_dynticks_task_trace_enter(); // Before ->dynticks update!
- seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
+ seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
// RCU is no longer watching. Better be in extended quiescent state!
WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
(seq & RCU_DYNTICK_CTRL_CTR));
@@ -274,13 +274,13 @@ static noinstr void rcu_dynticks_eqs_exi
* and we also must force ordering with the next RCU read-side
* critical section.
*/
- seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
+ seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
// RCU is now watching. Better not be in an extended quiescent state!
rcu_dynticks_task_trace_exit(); // After ->dynticks update!
WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
!(seq & RCU_DYNTICK_CTRL_CTR));
if (seq & RCU_DYNTICK_CTRL_MASK) {
- atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
+ arch_atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
smp_mb__after_atomic(); /* _exit after clearing mask. */
}
}
@@ -313,7 +313,7 @@ static __always_inline bool rcu_dynticks
{
struct rcu_data *rdp = this_cpu_ptr(_data);
- return !(atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
+ return !(arch_atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
}
/*
@@ -633,6 +633,10 @@ static noinstr void rcu_eqs_enter(bool u
do_nocb_deferred_wakeup(rdp);
rcu_prepare_for_idle();
rcu_preempt_deferred_qs(current);
+
+ // instrumentation for the noinstr rcu_dynticks_eqs_enter()
+ instrument_atomic_write(>dynticks, sizeof(rdp->dynticks));
+
instrumentation_end();
WRITE_ONCE(rdp->dynticks_nesting, 0); /* Avoid irq-access tearing. */
// RCU is watching here ...
@@ -692,6 +696,7 @@ noinstr void rcu_nmi_exit(void)
{
struct rcu_data *rdp = this_cpu_ptr(_data);
+ instrumentation_begin();
/*
* Check for ->dynticks_nmi_nesting underflow and bad ->dynticks.
* (We are exiting an NMI handler, so RCU better be paying attention
@@ -705,7 +710,6 @@ noinstr void rcu_nmi_exit(void)
* leave it in non-RCU-idle state.
*/
if (rdp->dynticks_nmi_nesting != 1) {
- instrumentation_begin();
trace_rcu_dyntick(TPS("--="), rdp->dynticks_nmi_nesting,
rdp->dynticks_nmi_nesting - 2,
atomic_read(>dynticks));
WRITE_ONCE(rdp->dynticks_nmi_nesting, /* No store tearing. */
@@ -714,13 +718,15 @@ noinstr void rcu_nmi_exit(void)
return;
}
- instrumentation_begin();
/* This NMI interrupted an RCU-idle CPU, restore RCU-idleness. */
trace_rcu_dyntick(TPS("Startirq"), rdp->dynticks_nmi_nesting, 0,
atomic_read(>dynticks));
WRITE_ONCE(rdp->dynticks_nmi_nesting, 0); /* Avoid store tearing. */
if (!in_nmi())
rcu_prepare_for_idle();
+
+ // instrumentation for the noinstr rcu_dynticks_eqs_enter()
+ instrument_atomic_write(>dynticks, sizeof(rdp->dynticks));
instrumentation_end();
// RCU is watching here ...
@@ -838,6 +844,10 @@ static void noinstr rcu_eqs_exit(bool us
rcu_dynticks_eqs_exit();
// ... but is watching here.
instrumentation_begin();
+
+ // instrumentation for the noinstr rcu_dynticks_eqs_exit()
+ instrument_atomic_write(>dynticks, sizeof(rdp->dynticks));
+
rcu_cleanup_after_idle();
trace_rcu_dyntick(TPS("End"),
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Wed, Jun 03, 2020 at 09:46:00AM -0700, Paul E. McKenney wrote:
> > // RCU is now watching. Better not be in an extended quiescent state!
> > rcu_dynticks_task_trace_exit(); // After ->dynticks update!
> > WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > !(seq & RCU_DYNTICK_CTRL_CTR));
> > if (seq & RCU_DYNTICK_CTRL_MASK) {
> > - atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
> > + arch_atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
>
> This one is gone in -rcu.
I'm still seeing that in mainline, was that removal scheduled for next
round?
> > smp_mb__after_atomic(); /* _exit after clearing mask. */
> > }
> > }
What shall we do with this patch?
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Thu, Jun 04, 2020 at 10:05:12AM +0200, Peter Zijlstra wrote:
> On Wed, Jun 03, 2020 at 08:34:09PM -0700, Paul E. McKenney wrote:
> > On Wed, Jun 03, 2020 at 07:13:20PM +0200, Peter Zijlstra wrote:
> > > On Wed, Jun 03, 2020 at 09:46:00AM -0700, Paul E. McKenney wrote:
>
> > > > > @@ -313,7 +313,7 @@ static __always_inline bool rcu_dynticks
> > > > > {
> > > > > struct rcu_data *rdp = this_cpu_ptr(_data);
> > > > >
> > > > > - return !(atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
> > > > > + return !(arch_atomic_read(>dynticks) &
> > > > > RCU_DYNTICK_CTRL_CTR);
> > >
> > > The above is actually instrumented by KCSAN, due to arch_atomic_read()
> > > being a READ_ONCE() and it now understanding volatile.
> > >
> > > > Also instrument_atomic_write(>dynticks, sizeof(rdp->dynticks)) as
> >
> > Right, this should instead be instrument_read(...).
> >
> > Though if KCSAN is unconditionally instrumenting volatile, how does
> > this help? Or does KCSAN's instrumentation of volatile somehow avoid
> > causing trouble?
>
> As Marco already explained, when used inside noinstr no instrumentation
> will be emitted, when used outside noinstr it will emit the right
> instrumentation.
>
> > > > o In theory in rcu_irq_exit_preempt(), but as this generates code
> > > > only in lockdep builds, it might not be worth worrying about.
> > > >
> > > > o Ditto for rcu_irq_exit_check_preempt().
> > > >
> > > > o Ditto for __rcu_irq_enter_check_tick().
> > >
> > > Not these, afaict they're all the above arch_atomic_read(), which is
> > > instrumented due to volatile in these cases.
>
> I this case, the above call-sites are all not noinstr (double negative!)
> and will thus cause instrumentation to be emitted.
>
> This is all a 'special' case for arch_atomic_read() (and _set()),
> because they're basically READ_ONCE() (and WRITE_ONCE() resp.). The
> normal atomics are asm() and it doesn't do anything for those (although
> I suppose clang could, since it has this internal assembler to parse the
> inline asm, but afaiu that's not something GCC ever wants to do).
Got it, and I had missed the inlining.
Again, commenting this will be interesting. And your earlier comment
about the compiler refusing to inline now makes sense...
Thanx, Paul
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Thu, Jun 04, 2020 at 08:02:31AM +0200, Marco Elver wrote:
> On Thu, 4 Jun 2020 at 05:34, Paul E. McKenney wrote:
> >
> > On Wed, Jun 03, 2020 at 07:13:20PM +0200, Peter Zijlstra wrote:
> > > On Wed, Jun 03, 2020 at 09:46:00AM -0700, Paul E. McKenney wrote:
> > >
> > > > > --- a/kernel/rcu/tree.c
> > > > > +++ b/kernel/rcu/tree.c
> > > > > @@ -250,7 +250,7 @@ static noinstr void rcu_dynticks_eqs_ent
> > > > >* next idle sojourn.
> > > > >*/
> > > > > rcu_dynticks_task_trace_enter(); // Before ->dynticks update!
> > > > > - seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > > > > + seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > > >
> > > > To preserve KCSAN's ability to see this, there would be something like
> > > > instrument_atomic_write(>dynticks, sizeof(rdp->dynticks)) prior
> > > > to the instrumentation_end() invoked before rcu_dynticks_eqs_enter()
> > > > in each of rcu_eqs_enter() and rcu_nmi_exit(), correct?
> > >
> > > Yes.
> > >
> > > > > // RCU is no longer watching. Better be in extended quiescent
> > > > > state!
> > > > > WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > > > >(seq & RCU_DYNTICK_CTRL_CTR));
> > > > > @@ -274,13 +274,13 @@ static noinstr void rcu_dynticks_eqs_exi
> > > > >* and we also must force ordering with the next RCU read-side
> > > > >* critical section.
> > > > >*/
> > > > > - seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > > > > + seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > > >
> > > > And same here, but after the instrumentation_begin() following
> > > > rcu_dynticks_eqs_exit() in both rcu_eqs_exit() and rcu_nmi_enter(),
> > > > correct?
> > >
> > > Yep.
> > >
> > > > > // RCU is now watching. Better not be in an extended quiescent
> > > > > state!
> > > > > rcu_dynticks_task_trace_exit(); // After ->dynticks update!
> > > > > WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > > > >!(seq & RCU_DYNTICK_CTRL_CTR));
> > > > > if (seq & RCU_DYNTICK_CTRL_MASK) {
> > > > > - atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
> > > > > + arch_atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
> > > >
> > > > This one is gone in -rcu.
> > >
> > > Good, because that would make things 'complicated' with the external
> > > instrumentation call. And is actually the reason I didn't even attempt
> > > it this time around.
> > >
> > > > > smp_mb__after_atomic(); /* _exit after clearing mask. */
> > > > > }
> > > > > }
> > > > > @@ -313,7 +313,7 @@ static __always_inline bool rcu_dynticks
> > > > > {
> > > > > struct rcu_data *rdp = this_cpu_ptr(_data);
> > > > >
> > > > > - return !(atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
> > > > > + return !(arch_atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
> > >
> > > The above is actually instrumented by KCSAN, due to arch_atomic_read()
> > > being a READ_ONCE() and it now understanding volatile.
> > >
> > > > Also instrument_atomic_write(>dynticks, sizeof(rdp->dynticks)) as
> >
> > Right, this should instead be instrument_read(...).
> >
> > Though if KCSAN is unconditionally instrumenting volatile, how does
> > this help? Or does KCSAN's instrumentation of volatile somehow avoid
> > causing trouble?
>
> When used normally outside noinstr functions, because this is an
> __always_inline function, it will be instrumented. Within noinstr
> (which imply __no_kcsan) functions it should not be instrumented.
Got it, thank you!
This is going to require some serious commenting. ;-)
Thanx, Paul
> Thanks,
> -- Marco
>
>
> > > > follows:
> > > >
> > > > o rcu_nmi_exit(): After each following instrumentation_begin().
> > >
> > > Yes
> > >
> > > > o In theory in rcu_irq_exit_preempt(), but as this generates code
> > > > only in lockdep builds, it might not be worth worrying about.
> > > >
> > > > o Ditto for rcu_irq_exit_check_preempt().
> > > >
> > > > o Ditto for __rcu_irq_enter_check_tick().
> > >
> > > Not these, afaict they're all the above arch_atomic_read(), which is
> > > instrumented due to volatile in these cases.
> > >
> > > > o rcu_nmi_enter(): After each following instrumentation_begin().
> > >
> > > Yes
> > >
> > > > o __rcu_is_watching() is itself noinstr:
> > > >
> > > > o idtentry_enter_cond_rcu(): After each following
> > > > instrumentation_begin().
> > > >
> > > > o rcu_is_watching(): Either before or after the call to
> > > > rcu_dynticks_curr_cpu_in_eqs().
> > >
> > > Something like that yes.
> > >
> > > > > }
> > > > >
> > > > > /*
> > > > > @@ -692,6 +692,7 @@ noinstr void rcu_nmi_exit(void)
> > > > > {
> > > > > struct rcu_data *rdp = this_cpu_ptr(_data);
> > > > >
> > > > > + instrumentation_begin();
> > > > > /*
> > > > >* Check for ->dynticks_nmi_nesting underflow and bad ->dynticks.
> > > > >* (We are exiting an NMI handler,
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Wed, Jun 03, 2020 at 08:34:09PM -0700, Paul E. McKenney wrote:
> On Wed, Jun 03, 2020 at 07:13:20PM +0200, Peter Zijlstra wrote:
> > On Wed, Jun 03, 2020 at 09:46:00AM -0700, Paul E. McKenney wrote:
> > > > @@ -313,7 +313,7 @@ static __always_inline bool rcu_dynticks
> > > > {
> > > > struct rcu_data *rdp = this_cpu_ptr(_data);
> > > >
> > > > - return !(atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
> > > > + return !(arch_atomic_read(>dynticks) &
> > > > RCU_DYNTICK_CTRL_CTR);
> >
> > The above is actually instrumented by KCSAN, due to arch_atomic_read()
> > being a READ_ONCE() and it now understanding volatile.
> >
> > > Also instrument_atomic_write(>dynticks, sizeof(rdp->dynticks)) as
>
> Right, this should instead be instrument_read(...).
>
> Though if KCSAN is unconditionally instrumenting volatile, how does
> this help? Or does KCSAN's instrumentation of volatile somehow avoid
> causing trouble?
As Marco already explained, when used inside noinstr no instrumentation
will be emitted, when used outside noinstr it will emit the right
instrumentation.
> > > o In theory in rcu_irq_exit_preempt(), but as this generates code
> > > only in lockdep builds, it might not be worth worrying about.
> > >
> > > o Ditto for rcu_irq_exit_check_preempt().
> > >
> > > o Ditto for __rcu_irq_enter_check_tick().
> >
> > Not these, afaict they're all the above arch_atomic_read(), which is
> > instrumented due to volatile in these cases.
I this case, the above call-sites are all not noinstr (double negative!)
and will thus cause instrumentation to be emitted.
This is all a 'special' case for arch_atomic_read() (and _set()),
because they're basically READ_ONCE() (and WRITE_ONCE() resp.). The
normal atomics are asm() and it doesn't do anything for those (although
I suppose clang could, since it has this internal assembler to parse the
inline asm, but afaiu that's not something GCC ever wants to do).
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Thu, 4 Jun 2020 at 05:34, Paul E. McKenney wrote:
>
> On Wed, Jun 03, 2020 at 07:13:20PM +0200, Peter Zijlstra wrote:
> > On Wed, Jun 03, 2020 at 09:46:00AM -0700, Paul E. McKenney wrote:
> >
> > > > --- a/kernel/rcu/tree.c
> > > > +++ b/kernel/rcu/tree.c
> > > > @@ -250,7 +250,7 @@ static noinstr void rcu_dynticks_eqs_ent
> > > >* next idle sojourn.
> > > >*/
> > > > rcu_dynticks_task_trace_enter(); // Before ->dynticks update!
> > > > - seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > > > + seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > >
> > > To preserve KCSAN's ability to see this, there would be something like
> > > instrument_atomic_write(>dynticks, sizeof(rdp->dynticks)) prior
> > > to the instrumentation_end() invoked before rcu_dynticks_eqs_enter()
> > > in each of rcu_eqs_enter() and rcu_nmi_exit(), correct?
> >
> > Yes.
> >
> > > > // RCU is no longer watching. Better be in extended quiescent state!
> > > > WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > > >(seq & RCU_DYNTICK_CTRL_CTR));
> > > > @@ -274,13 +274,13 @@ static noinstr void rcu_dynticks_eqs_exi
> > > >* and we also must force ordering with the next RCU read-side
> > > >* critical section.
> > > >*/
> > > > - seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > > > + seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > >
> > > And same here, but after the instrumentation_begin() following
> > > rcu_dynticks_eqs_exit() in both rcu_eqs_exit() and rcu_nmi_enter(),
> > > correct?
> >
> > Yep.
> >
> > > > // RCU is now watching. Better not be in an extended quiescent state!
> > > > rcu_dynticks_task_trace_exit(); // After ->dynticks update!
> > > > WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > > >!(seq & RCU_DYNTICK_CTRL_CTR));
> > > > if (seq & RCU_DYNTICK_CTRL_MASK) {
> > > > - atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
> > > > + arch_atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
> > >
> > > This one is gone in -rcu.
> >
> > Good, because that would make things 'complicated' with the external
> > instrumentation call. And is actually the reason I didn't even attempt
> > it this time around.
> >
> > > > smp_mb__after_atomic(); /* _exit after clearing mask. */
> > > > }
> > > > }
> > > > @@ -313,7 +313,7 @@ static __always_inline bool rcu_dynticks
> > > > {
> > > > struct rcu_data *rdp = this_cpu_ptr(_data);
> > > >
> > > > - return !(atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
> > > > + return !(arch_atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
> >
> > The above is actually instrumented by KCSAN, due to arch_atomic_read()
> > being a READ_ONCE() and it now understanding volatile.
> >
> > > Also instrument_atomic_write(>dynticks, sizeof(rdp->dynticks)) as
>
> Right, this should instead be instrument_read(...).
>
> Though if KCSAN is unconditionally instrumenting volatile, how does
> this help? Or does KCSAN's instrumentation of volatile somehow avoid
> causing trouble?
When used normally outside noinstr functions, because this is an
__always_inline function, it will be instrumented. Within noinstr
(which imply __no_kcsan) functions it should not be instrumented.
Thanks,
-- Marco
> > > follows:
> > >
> > > o rcu_nmi_exit(): After each following instrumentation_begin().
> >
> > Yes
> >
> > > o In theory in rcu_irq_exit_preempt(), but as this generates code
> > > only in lockdep builds, it might not be worth worrying about.
> > >
> > > o Ditto for rcu_irq_exit_check_preempt().
> > >
> > > o Ditto for __rcu_irq_enter_check_tick().
> >
> > Not these, afaict they're all the above arch_atomic_read(), which is
> > instrumented due to volatile in these cases.
> >
> > > o rcu_nmi_enter(): After each following instrumentation_begin().
> >
> > Yes
> >
> > > o __rcu_is_watching() is itself noinstr:
> > >
> > > o idtentry_enter_cond_rcu(): After each following
> > > instrumentation_begin().
> > >
> > > o rcu_is_watching(): Either before or after the call to
> > > rcu_dynticks_curr_cpu_in_eqs().
> >
> > Something like that yes.
> >
> > > > }
> > > >
> > > > /*
> > > > @@ -692,6 +692,7 @@ noinstr void rcu_nmi_exit(void)
> > > > {
> > > > struct rcu_data *rdp = this_cpu_ptr(_data);
> > > >
> > > > + instrumentation_begin();
> > > > /*
> > > >* Check for ->dynticks_nmi_nesting underflow and bad ->dynticks.
> > > >* (We are exiting an NMI handler, so RCU better be paying attention
> > > > @@ -705,7 +706,6 @@ noinstr void rcu_nmi_exit(void)
> > > >* leave it in non-RCU-idle state.
> > > >*/
> > > > if (rdp->dynticks_nmi_nesting != 1) {
> > > > - instrumentation_begin();
> > > > trace_rcu_dyntick(TPS("--="), rdp->dynticks_nmi_nesting,
> > > > rdp->dynticks_nmi_nesting - 2,
> > > > atomic_read(>dynticks));
> > > >
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Wed, Jun 03, 2020 at 07:13:20PM +0200, Peter Zijlstra wrote:
> On Wed, Jun 03, 2020 at 09:46:00AM -0700, Paul E. McKenney wrote:
>
> > > --- a/kernel/rcu/tree.c
> > > +++ b/kernel/rcu/tree.c
> > > @@ -250,7 +250,7 @@ static noinstr void rcu_dynticks_eqs_ent
> > >* next idle sojourn.
> > >*/
> > > rcu_dynticks_task_trace_enter(); // Before ->dynticks update!
> > > - seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > > + seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> >
> > To preserve KCSAN's ability to see this, there would be something like
> > instrument_atomic_write(>dynticks, sizeof(rdp->dynticks)) prior
> > to the instrumentation_end() invoked before rcu_dynticks_eqs_enter()
> > in each of rcu_eqs_enter() and rcu_nmi_exit(), correct?
>
> Yes.
>
> > > // RCU is no longer watching. Better be in extended quiescent state!
> > > WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > >(seq & RCU_DYNTICK_CTRL_CTR));
> > > @@ -274,13 +274,13 @@ static noinstr void rcu_dynticks_eqs_exi
> > >* and we also must force ordering with the next RCU read-side
> > >* critical section.
> > >*/
> > > - seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > > + seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> >
> > And same here, but after the instrumentation_begin() following
> > rcu_dynticks_eqs_exit() in both rcu_eqs_exit() and rcu_nmi_enter(),
> > correct?
>
> Yep.
>
> > > // RCU is now watching. Better not be in an extended quiescent state!
> > > rcu_dynticks_task_trace_exit(); // After ->dynticks update!
> > > WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > >!(seq & RCU_DYNTICK_CTRL_CTR));
> > > if (seq & RCU_DYNTICK_CTRL_MASK) {
> > > - atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
> > > + arch_atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
> >
> > This one is gone in -rcu.
>
> Good, because that would make things 'complicated' with the external
> instrumentation call. And is actually the reason I didn't even attempt
> it this time around.
>
> > > smp_mb__after_atomic(); /* _exit after clearing mask. */
> > > }
> > > }
> > > @@ -313,7 +313,7 @@ static __always_inline bool rcu_dynticks
> > > {
> > > struct rcu_data *rdp = this_cpu_ptr(_data);
> > >
> > > - return !(atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
> > > + return !(arch_atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
>
> The above is actually instrumented by KCSAN, due to arch_atomic_read()
> being a READ_ONCE() and it now understanding volatile.
>
> > Also instrument_atomic_write(>dynticks, sizeof(rdp->dynticks)) as
Right, this should instead be instrument_read(...).
Though if KCSAN is unconditionally instrumenting volatile, how does
this help? Or does KCSAN's instrumentation of volatile somehow avoid
causing trouble?
> > follows:
> >
> > o rcu_nmi_exit(): After each following instrumentation_begin().
>
> Yes
>
> > o In theory in rcu_irq_exit_preempt(), but as this generates code
> > only in lockdep builds, it might not be worth worrying about.
> >
> > o Ditto for rcu_irq_exit_check_preempt().
> >
> > o Ditto for __rcu_irq_enter_check_tick().
>
> Not these, afaict they're all the above arch_atomic_read(), which is
> instrumented due to volatile in these cases.
>
> > o rcu_nmi_enter(): After each following instrumentation_begin().
>
> Yes
>
> > o __rcu_is_watching() is itself noinstr:
> >
> > o idtentry_enter_cond_rcu(): After each following
> > instrumentation_begin().
> >
> > o rcu_is_watching(): Either before or after the call to
> > rcu_dynticks_curr_cpu_in_eqs().
>
> Something like that yes.
>
> > > }
> > >
> > > /*
> > > @@ -692,6 +692,7 @@ noinstr void rcu_nmi_exit(void)
> > > {
> > > struct rcu_data *rdp = this_cpu_ptr(_data);
> > >
> > > + instrumentation_begin();
> > > /*
> > >* Check for ->dynticks_nmi_nesting underflow and bad ->dynticks.
> > >* (We are exiting an NMI handler, so RCU better be paying attention
> > > @@ -705,7 +706,6 @@ noinstr void rcu_nmi_exit(void)
> > >* leave it in non-RCU-idle state.
> > >*/
> > > if (rdp->dynticks_nmi_nesting != 1) {
> > > - instrumentation_begin();
> > > trace_rcu_dyntick(TPS("--="), rdp->dynticks_nmi_nesting,
> > > rdp->dynticks_nmi_nesting - 2,
> > > atomic_read(>dynticks));
> > > WRITE_ONCE(rdp->dynticks_nmi_nesting, /* No store tearing. */
> > > @@ -714,7 +714,6 @@ noinstr void rcu_nmi_exit(void)
> > > return;
> > > }
> > >
> > > - instrumentation_begin();
> > > /* This NMI interrupted an RCU-idle CPU, restore RCU-idleness. */
> > > trace_rcu_dyntick(TPS("Startirq"), rdp->dynticks_nmi_nesting, 0,
> > > atomic_read(>dynticks));
> > > WRITE_ONCE(rdp->dynticks_nmi_nesting, 0); /* Avoid store tearing. */
> >
> > This one looks to be having no effect on
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Wed, Jun 03, 2020 at 09:46:00AM -0700, Paul E. McKenney wrote:
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -250,7 +250,7 @@ static noinstr void rcu_dynticks_eqs_ent
> > * next idle sojourn.
> > */
> > rcu_dynticks_task_trace_enter(); // Before ->dynticks update!
> > - seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > + seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
>
> To preserve KCSAN's ability to see this, there would be something like
> instrument_atomic_write(>dynticks, sizeof(rdp->dynticks)) prior
> to the instrumentation_end() invoked before rcu_dynticks_eqs_enter()
> in each of rcu_eqs_enter() and rcu_nmi_exit(), correct?
Yes.
> > // RCU is no longer watching. Better be in extended quiescent state!
> > WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > (seq & RCU_DYNTICK_CTRL_CTR));
> > @@ -274,13 +274,13 @@ static noinstr void rcu_dynticks_eqs_exi
> > * and we also must force ordering with the next RCU read-side
> > * critical section.
> > */
> > - seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> > + seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
>
> And same here, but after the instrumentation_begin() following
> rcu_dynticks_eqs_exit() in both rcu_eqs_exit() and rcu_nmi_enter(),
> correct?
Yep.
> > // RCU is now watching. Better not be in an extended quiescent state!
> > rcu_dynticks_task_trace_exit(); // After ->dynticks update!
> > WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> > !(seq & RCU_DYNTICK_CTRL_CTR));
> > if (seq & RCU_DYNTICK_CTRL_MASK) {
> > - atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
> > + arch_atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
>
> This one is gone in -rcu.
Good, because that would make things 'complicated' with the external
instrumentation call. And is actually the reason I didn't even attempt
it this time around.
> > smp_mb__after_atomic(); /* _exit after clearing mask. */
> > }
> > }
> > @@ -313,7 +313,7 @@ static __always_inline bool rcu_dynticks
> > {
> > struct rcu_data *rdp = this_cpu_ptr(_data);
> >
> > - return !(atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
> > + return !(arch_atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
The above is actually instrumented by KCSAN, due to arch_atomic_read()
being a READ_ONCE() and it now understanding volatile.
> Also instrument_atomic_write(>dynticks, sizeof(rdp->dynticks)) as
> follows:
>
> o rcu_nmi_exit(): After each following instrumentation_begin().
Yes
> o In theory in rcu_irq_exit_preempt(), but as this generates code
> only in lockdep builds, it might not be worth worrying about.
>
> o Ditto for rcu_irq_exit_check_preempt().
>
> o Ditto for __rcu_irq_enter_check_tick().
Not these, afaict they're all the above arch_atomic_read(), which is
instrumented due to volatile in these cases.
> o rcu_nmi_enter(): After each following instrumentation_begin().
Yes
> o __rcu_is_watching() is itself noinstr:
>
> o idtentry_enter_cond_rcu(): After each following
> instrumentation_begin().
>
> o rcu_is_watching(): Either before or after the call to
> rcu_dynticks_curr_cpu_in_eqs().
Something like that yes.
> > }
> >
> > /*
> > @@ -692,6 +692,7 @@ noinstr void rcu_nmi_exit(void)
> > {
> > struct rcu_data *rdp = this_cpu_ptr(_data);
> >
> > + instrumentation_begin();
> > /*
> > * Check for ->dynticks_nmi_nesting underflow and bad ->dynticks.
> > * (We are exiting an NMI handler, so RCU better be paying attention
> > @@ -705,7 +706,6 @@ noinstr void rcu_nmi_exit(void)
> > * leave it in non-RCU-idle state.
> > */
> > if (rdp->dynticks_nmi_nesting != 1) {
> > - instrumentation_begin();
> > trace_rcu_dyntick(TPS("--="), rdp->dynticks_nmi_nesting,
> > rdp->dynticks_nmi_nesting - 2,
> > atomic_read(>dynticks));
> > WRITE_ONCE(rdp->dynticks_nmi_nesting, /* No store tearing. */
> > @@ -714,7 +714,6 @@ noinstr void rcu_nmi_exit(void)
> > return;
> > }
> >
> > - instrumentation_begin();
> > /* This NMI interrupted an RCU-idle CPU, restore RCU-idleness. */
> > trace_rcu_dyntick(TPS("Startirq"), rdp->dynticks_nmi_nesting, 0,
> > atomic_read(>dynticks));
> > WRITE_ONCE(rdp->dynticks_nmi_nesting, 0); /* Avoid store tearing. */
>
> This one looks to be having no effect on instrumentation of atomics, but
> rather coalescing a pair of instrumentation_begin() into one.
>
> Do I understand correctly?
Almost, it puts the WARN_ON_ONCE()s under instrumentation_begin() too,
and that makes a differnce, iirc it was the
rcu_dynticks_curr_cpu_in_eqs() call that stood out. But that could've
been before I switched it to arch_atomic_read(). In any case, I find
this form a lot clearer.
Re: [PATCH 2/9] rcu: Fixup noinstr warnings
On Wed, Jun 03, 2020 at 01:40:16PM +0200, Peter Zijlstra wrote:
> A KCSAN build revealed we have explicit annoations through atomic_*()
> usage, switch to arch_atomic_*() for the respective functions.
>
> vmlinux.o: warning: objtool: rcu_nmi_exit()+0x4d: call to
> __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: rcu_dynticks_eqs_enter()+0x25: call to
> __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: rcu_nmi_enter()+0x4f: call to
> __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: rcu_dynticks_eqs_exit()+0x2a: call to
> __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: __rcu_is_watching()+0x25: call to
> __kcsan_check_access() leaves .noinstr.text section
>
> Signed-off-by: Peter Zijlstra (Intel)
> Link:
> https://lkml.kernel.org/r/20200603084818.gb2...@hirez.programming.kicks-ass.net
> ---
> kernel/rcu/tree.c | 11 +--
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -250,7 +250,7 @@ static noinstr void rcu_dynticks_eqs_ent
>* next idle sojourn.
>*/
> rcu_dynticks_task_trace_enter(); // Before ->dynticks update!
> - seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> + seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
To preserve KCSAN's ability to see this, there would be something like
instrument_atomic_write(>dynticks, sizeof(rdp->dynticks)) prior
to the instrumentation_end() invoked before rcu_dynticks_eqs_enter()
in each of rcu_eqs_enter() and rcu_nmi_exit(), correct?
> // RCU is no longer watching. Better be in extended quiescent state!
> WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
>(seq & RCU_DYNTICK_CTRL_CTR));
> @@ -274,13 +274,13 @@ static noinstr void rcu_dynticks_eqs_exi
>* and we also must force ordering with the next RCU read-side
>* critical section.
>*/
> - seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
> + seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
And same here, but after the instrumentation_begin() following
rcu_dynticks_eqs_exit() in both rcu_eqs_exit() and rcu_nmi_enter(),
correct?
> // RCU is now watching. Better not be in an extended quiescent state!
> rcu_dynticks_task_trace_exit(); // After ->dynticks update!
> WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
>!(seq & RCU_DYNTICK_CTRL_CTR));
> if (seq & RCU_DYNTICK_CTRL_MASK) {
> - atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
> + arch_atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
This one is gone in -rcu.
> smp_mb__after_atomic(); /* _exit after clearing mask. */
> }
> }
> @@ -313,7 +313,7 @@ static __always_inline bool rcu_dynticks
> {
> struct rcu_data *rdp = this_cpu_ptr(_data);
>
> - return !(atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
> + return !(arch_atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
Also instrument_atomic_write(>dynticks, sizeof(rdp->dynticks)) as
follows:
o rcu_nmi_exit(): After each following instrumentation_begin().
o In theory in rcu_irq_exit_preempt(), but as this generates code
only in lockdep builds, it might not be worth worrying about.
o Ditto for rcu_irq_exit_check_preempt().
o Ditto for __rcu_irq_enter_check_tick().
o rcu_nmi_enter(): After each following instrumentation_begin().
o __rcu_is_watching() is itself noinstr:
o idtentry_enter_cond_rcu(): After each following
instrumentation_begin().
o rcu_is_watching(): Either before or after the call to
rcu_dynticks_curr_cpu_in_eqs().
> }
>
> /*
> @@ -692,6 +692,7 @@ noinstr void rcu_nmi_exit(void)
> {
> struct rcu_data *rdp = this_cpu_ptr(_data);
>
> + instrumentation_begin();
> /*
>* Check for ->dynticks_nmi_nesting underflow and bad ->dynticks.
>* (We are exiting an NMI handler, so RCU better be paying attention
> @@ -705,7 +706,6 @@ noinstr void rcu_nmi_exit(void)
>* leave it in non-RCU-idle state.
>*/
> if (rdp->dynticks_nmi_nesting != 1) {
> - instrumentation_begin();
> trace_rcu_dyntick(TPS("--="), rdp->dynticks_nmi_nesting,
> rdp->dynticks_nmi_nesting - 2,
> atomic_read(>dynticks));
> WRITE_ONCE(rdp->dynticks_nmi_nesting, /* No store tearing. */
> @@ -714,7 +714,6 @@ noinstr void rcu_nmi_exit(void)
> return;
> }
>
> - instrumentation_begin();
> /* This NMI interrupted an RCU-idle CPU, restore RCU-idleness. */
> trace_rcu_dyntick(TPS("Startirq"), rdp->dynticks_nmi_nesting, 0,
> atomic_read(>dynticks));
> WRITE_ONCE(rdp->dynticks_nmi_nesting, 0); /* Avoid store tearing. */
This one looks to
[PATCH 2/9] rcu: Fixup noinstr warnings
A KCSAN build revealed we have explicit annoations through atomic_*()
usage, switch to arch_atomic_*() for the respective functions.
vmlinux.o: warning: objtool: rcu_nmi_exit()+0x4d: call to
__kcsan_check_access() leaves .noinstr.text section
vmlinux.o: warning: objtool: rcu_dynticks_eqs_enter()+0x25: call to
__kcsan_check_access() leaves .noinstr.text section
vmlinux.o: warning: objtool: rcu_nmi_enter()+0x4f: call to
__kcsan_check_access() leaves .noinstr.text section
vmlinux.o: warning: objtool: rcu_dynticks_eqs_exit()+0x2a: call to
__kcsan_check_access() leaves .noinstr.text section
vmlinux.o: warning: objtool: __rcu_is_watching()+0x25: call to
__kcsan_check_access() leaves .noinstr.text section
Signed-off-by: Peter Zijlstra (Intel)
Link:
https://lkml.kernel.org/r/20200603084818.gb2...@hirez.programming.kicks-ass.net
---
kernel/rcu/tree.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -250,7 +250,7 @@ static noinstr void rcu_dynticks_eqs_ent
* next idle sojourn.
*/
rcu_dynticks_task_trace_enter(); // Before ->dynticks update!
- seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
+ seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
// RCU is no longer watching. Better be in extended quiescent state!
WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
(seq & RCU_DYNTICK_CTRL_CTR));
@@ -274,13 +274,13 @@ static noinstr void rcu_dynticks_eqs_exi
* and we also must force ordering with the next RCU read-side
* critical section.
*/
- seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
+ seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, >dynticks);
// RCU is now watching. Better not be in an extended quiescent state!
rcu_dynticks_task_trace_exit(); // After ->dynticks update!
WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
!(seq & RCU_DYNTICK_CTRL_CTR));
if (seq & RCU_DYNTICK_CTRL_MASK) {
- atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
+ arch_atomic_andnot(RCU_DYNTICK_CTRL_MASK, >dynticks);
smp_mb__after_atomic(); /* _exit after clearing mask. */
}
}
@@ -313,7 +313,7 @@ static __always_inline bool rcu_dynticks
{
struct rcu_data *rdp = this_cpu_ptr(_data);
- return !(atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
+ return !(arch_atomic_read(>dynticks) & RCU_DYNTICK_CTRL_CTR);
}
/*
@@ -692,6 +692,7 @@ noinstr void rcu_nmi_exit(void)
{
struct rcu_data *rdp = this_cpu_ptr(_data);
+ instrumentation_begin();
/*
* Check for ->dynticks_nmi_nesting underflow and bad ->dynticks.
* (We are exiting an NMI handler, so RCU better be paying attention
@@ -705,7 +706,6 @@ noinstr void rcu_nmi_exit(void)
* leave it in non-RCU-idle state.
*/
if (rdp->dynticks_nmi_nesting != 1) {
- instrumentation_begin();
trace_rcu_dyntick(TPS("--="), rdp->dynticks_nmi_nesting,
rdp->dynticks_nmi_nesting - 2,
atomic_read(>dynticks));
WRITE_ONCE(rdp->dynticks_nmi_nesting, /* No store tearing. */
@@ -714,7 +714,6 @@ noinstr void rcu_nmi_exit(void)
return;
}
- instrumentation_begin();
/* This NMI interrupted an RCU-idle CPU, restore RCU-idleness. */
trace_rcu_dyntick(TPS("Startirq"), rdp->dynticks_nmi_nesting, 0,
atomic_read(>dynticks));
WRITE_ONCE(rdp->dynticks_nmi_nesting, 0); /* Avoid store tearing. */
