Re: [PATCH] rcu: Fixup noinstr warnings

[Paul E. McKenney]

On Wed, Jun 03, 2020 at 12:52:06PM +0200, Peter Zijlstra wrote:
> On Wed, Jun 03, 2020 at 02:59:32AM -0700, Paul E. McKenney wrote:
> > On Wed, Jun 03, 2020 at 10:48:18AM +0200, Peter Zijlstra wrote:
> > > On Tue, Jun 02, 2020 at 09:38:53PM +0200, Peter Zijlstra wrote:
> > > 
> > > > That said; noinstr's __no_sanitize combined with atomic_t might be
> > > > 'interesting', because the regular atomic things have explicit
> > > > annotations in them. That should give validation warnings for the right
> > > > .config, I'll have to go try -- so far I've made sure to never enable
> > > > the *SAN stuff.
> > > 
> > > ---
> > > Subject: rcu: Fixup noinstr warnings
> > > 
> > > A KCSAN build revealed we have explicit annoations through atomic_t
> > > 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) <peterz@xxxxxxxxxxxxx>
> > 
> > This one does not apply cleanly onto the -rcu tree's "dev" branch, so
> > I am guessing that it is intended to be carried in -tip with yours and
> > Thomas's patch series.
> 
> Right, I've not played patch tetris yet so see how it should all fit
> together. I also didn't know you feel about loosing the instrumentation
> in these functions.

It would be very good for KCSAN to be able to detect misuse of ->dynticks!

> One option would be do add explicit: instrument_atomic_write() calls
> before instrument_end() / after instrument_begin() in
> the respective callers that have that.

One good thing: The atomic_andnot() goes away in -rcu patches slated
for v5.9.  However, the others remain.

So if today's -next is any guide, this instrument_atomic_write()
would be added (for one example) in the functions that invoke
rcu_dynticks_eqs_enter(), since it is noinstr.  Rather annoying, and
will require careful commenting.  But there are only two such calls and
they are both in the same file and it is very low-level code, so this
should be doable.

I should also add some commentary to the RCU requirements document
say why all of this is happening.

> Anyway, I'll shortly be posting a pile of patches resulting from various
> KCSAN and KASAN builds. The good news is that GCC-KASAN seems to behave
> quite well with Marco's patches, the bad news is that GCC-KASAN is
> retarded wrt inline and needs a bunch of kicks.
> 
> That is, it out-of-lines:
> 
> static inline bool foo(..)
> {
> 	return false;
> }
> 
> just because..

Compilers!!!  :-/

							Thanx, Paul