On Thu, Nov 01, 2018 at 05:32:12PM +0100, Peter Zijlstra wrote:

On Thu, Nov 01, 2018 at 03:22:15PM +0000, Trond Myklebust wrote:

quoted

On Thu, 2018-11-01 at 15:59 +0100, Peter Zijlstra wrote:

quoted

On Thu, Nov 01, 2018 at 01:18:46PM +0000, Mark Rutland wrote:

quoted

quoted

quoted

quoted

My one question (and the reason why I went with cmpxchg() in the
first place) would be about the overflow behaviour for
atomic_fetch_inc() and friends. I believe those functions should
be OK on x86, so that when we overflow the counter, it behaves
like an unsigned value and wraps back around.  Is that the case
for all architectures?

i.e. are atomic_t/atomic64_t always guaranteed to behave like
u32/u64 on increment?

I could not find any documentation that explicitly stated that
they should.

Peter, Will, I understand that the atomic_t/atomic64_t ops are
required to wrap per 2's-complement. IIUC the refcount code relies
on this.

Can you confirm?

There is quite a bit of core code that hard assumes 2s-complement.
Not only for atomics but for any signed integer type. Also see the
kernel using -fno-strict-overflow which implies -fwrapv, which
defines signed overflow to behave like 2s-complement (and rids us of
that particular UB).

Fair enough, but there have also been bugfixes to explicitly fix unsafe
C standards assumptions for signed integers. See, for instance commit
5a581b367b5d "jiffies: Avoid undefined behavior from signed overflow"
from Paul McKenney.

Yes, I feel Paul has been to too many C/C++ committee meetings and got
properly paranoid. Which isn't always a bad thing :-)

Even the C standard defines 2s complement for atomics.  Just not for
normal arithmetic, where yes, signed overflow is UB.  And yes, I do
know about -fwrapv, but I would like to avoid at least some copy-pasta
UB from my kernel code to who knows what user-mode environment.  :-/

At least where it is reasonably easy to do so.

And there is a push to define C++ signed arithmetic as 2s complement,
but there are still 1s complement systems with C compilers.  Just not
C++ compilers.  Legacy...

But for us using -fno-strict-overflow which actually defines signed
overflow, I myself am really not worried. I'm also not sure if KASAN has
been taught about this, or if it will still (incorrectly) warn about UB
for signed types.

UBSAN gave me a signed-overflow warning a few days ago.  Which I have
fixed, even though 2s complement did the right thing.  I am also taking
advantage of the change to use better naming.

quoted

Anyhow, if the atomic maintainers are willing to stand up and state for
the record that the atomic counters are guaranteed to wrap modulo 2^n
just like unsigned integers, then I'm happy to take Paul's patch.

I myself am certainly relying on it.

Color me confused.  My 5a581b367b5d is from 2013.  Or is "Paul" instead
intended to mean Paul Mackerras, who happens to be on CC?

							Thanx, Paul