linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Trond Myklebust <trondmy@hammerspace.com>
Cc: "mark.rutland@arm.com" <mark.rutland@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"ralf@linux-mips.org" <ralf@linux-mips.org>,
	"jlayton@kernel.org" <jlayton@kernel.org>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	"bfields@fieldses.org" <bfields@fieldses.org>,
	"linux-mips@linux-mips.org" <linux-mips@linux-mips.org>,
	"linux@roeck-us.net" <linux@roeck-us.net>,
	"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"will.deacon@arm.com" <will.deacon@arm.com>,
	"boqun.feng@gmail.com" <boqun.feng@gmail.com>,
	"paul.burton@mips.com" <paul.burton@mips.com>,
	"anna.schumaker@netapp.com" <anna.schumaker@netapp.com>,
	"jhogan@kernel.org" <jhogan@kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"arnd@arndb.de" <arnd@arndb.de>,
	"paulus@samba.org" <paulus@samba.org>,
	"mpe@ellerman.id.au" <mpe@ellerman.id.au>,
	"benh@kernel.crashing.org" <benh@kernel.crashing.org>,
	Paul McKenney <paulmck@linux.vnet.ibm.com>,
	aryabinin@virtuozzo.com, dvyukov@google.com
Subject: Re: [RFC PATCH] lib: Introduce generic __cmpxchg_u64() and use it where needed
Date: Thu, 1 Nov 2018 17:32:12 +0100	[thread overview]
Message-ID: <20181101163212.GF3159@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <f38e272f7a96e983549e4281aa9fd02833a4277a.camel@hammerspace.com>

On Thu, Nov 01, 2018 at 03:22:15PM +0000, Trond Myklebust wrote:
> On Thu, 2018-11-01 at 15:59 +0100, Peter Zijlstra wrote:
> > On Thu, Nov 01, 2018 at 01:18:46PM +0000, Mark Rutland wrote:

> > > > 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 :-)

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.

> 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.

  reply	other threads:[~2018-11-01 16:35 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1541015538-11382-1-git-send-email-linux@roeck-us.net>
     [not found] ` <20181031213240.zhh7dfcm47ucuyfl@pburton-laptop>
     [not found]   ` <20181031220253.GA15505@roeck-us.net>
2018-10-31 23:32     ` [RFC PATCH] lib: Introduce generic __cmpxchg_u64() and use it where needed Paul Burton
2018-11-01  0:17       ` Trond Myklebust
2018-11-01 13:18         ` Mark Rutland
2018-11-01 14:59           ` Peter Zijlstra
2018-11-01 15:22             ` Trond Myklebust
2018-11-01 16:32               ` Peter Zijlstra [this message]
2018-11-01 16:59                 ` Eric Dumazet
2018-11-01 17:14                   ` Peter Zijlstra
2018-11-01 17:27                     ` Peter Zijlstra
2018-11-01 20:29                       ` Paul E. McKenney
2018-11-01 21:38                         ` Peter Zijlstra
2018-11-01 22:26                           ` Paul E. McKenney
2018-11-01 17:43                     ` Paul E. McKenney
2018-11-01 17:01                 ` Paul E. McKenney
2018-11-01 17:18                   ` Peter Zijlstra
2018-11-01 17:34                     ` Paul E. McKenney
2018-11-01 17:46                     ` Dmitry Vyukov
2018-11-01 21:45                       ` Peter Zijlstra
2018-11-02 10:56                   ` David Laight
2018-11-02 12:23                     ` Peter Zijlstra
2018-11-02 13:38                       ` Paul E. McKenney
2018-11-02 13:37                     ` Paul E. McKenney
2018-11-02 16:19                 ` Andrey Ryabinin
2018-11-05 10:38                   ` Peter Zijlstra
2018-11-05 14:24                   ` Peter Zijlstra
2018-11-01 17:51             ` [PATCH] SUNRPC: Use atomic(64)_t for seq_send(64) Paul Burton
2018-11-01 17:57               ` Trond Myklebust
2018-11-01 17:54         ` [RFC PATCH] lib: Introduce generic __cmpxchg_u64() and use it where needed Paul Burton
2018-11-01  1:18       ` Guenter Roeck
2018-11-01  6:30         ` Trond Myklebust
2018-11-01 15:28           ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181101163212.GF3159@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=anna.schumaker@netapp.com \
    --cc=arnd@arndb.de \
    --cc=aryabinin@virtuozzo.com \
    --cc=benh@kernel.crashing.org \
    --cc=bfields@fieldses.org \
    --cc=boqun.feng@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dvyukov@google.com \
    --cc=jhogan@kernel.org \
    --cc=jlayton@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mark.rutland@arm.com \
    --cc=mpe@ellerman.id.au \
    --cc=netdev@vger.kernel.org \
    --cc=paul.burton@mips.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=paulus@samba.org \
    --cc=ralf@linux-mips.org \
    --cc=trondmy@hammerspace.com \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).