Re: [PATCH] qspinlock: use signed temporaries for cmpxchg

[Peter Zijlstra]

On Tue, Oct 27, 2020 at 09:33:32AM +0100, Arnd Bergmann wrote:
> On Tue, Oct 27, 2020 at 8:47 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> > On Mon, Oct 26, 2020 at 02:03:06PM -0400, Waiman Long wrote:
> > > On 10/26/20 12:57 PM, Arnd Bergmann wrote:
> > > Yes, it shouldn't really matter if the value is defined as int or u32.
> > > However, the only caveat that I see is queued_spin_lock_slowpath() is
> > > expecting a u32 argument. Maybe you should cast it back to (u32) when
> > > calling it.
> >
> > No, we're not going to confuse the code. That stuff is hard enough as it
> > is. This warning is garbage and just needs to stay off.
> 
> Ok, so the question then becomes: should we drop -Wpointer-sign from
> W=2 and move it to W=3, or instead disable it locally. I could add
> __diag_ignore(GCC, 4, "-Wpointer-sign") in the couple of header files
> that produce this kind of warning if there is a general feeling that it
> still helps to have this for drivers.

What is an actual geniune bug that this warning helps find?

Note that the kernel relies on -fno-strict-overflow to get rid of the
signed UB that is otherwise present in C.

If you add that __diag_ignore() it should go in atomic.h I suppose,
because all of atomic hard relies on this, and then the question becomes
how much code is left that doesn't include that header and consequently
doesn't ignore that warning.

So, is it useful to begin with in finding actual problems? and given we
have to annotate away a bucket-load, how much coverage will there remain
if we squish the known false-positives?