Messages in this thread | | | Date | Thu, 4 Jun 2020 10:05:12 +0200 | From | Peter Zijlstra <> | Subject | 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(&rcu_data); > > > > > > > > - return !(atomic_read(&rdp->dynticks) & RCU_DYNTICK_CTRL_CTR); > > > > + return !(arch_atomic_read(&rdp->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(&rdp->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).
| |