Message-ID: <2a0f5fc7-4096-dcb7-ac3b-29c4552a5462@infradead.org>
Date:   Wed, 26 Jul 2017 09:28:42 -0700
From:   Randy Dunlap <rdunlap@...radead.org>
To:     Peter Zijlstra <peterz@...radead.org>,
        Boqun Feng <boqun.feng@...il.com>
Cc:     Will Deacon <will.deacon@....com>,
        Paul McKenney <paulmck@...ux.vnet.ibm.com>,
        linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [RFC][PATCH v3]: documentation,atomic: Add new documents

nits... <chirping>

On 07/26/2017 04:53 AM, Peter Zijlstra wrote:
> ---
> Subject: documentation,atomic: Add new documents
> From: Peter Zijlstra <peterz@...radead.org>
> Date: Mon Jun 12 14:50:27 CEST 2017
> 
> Since we've vastly expanded the atomic_t interface in recent years the
> existing documentation is woefully out of date and people seem to get
> confused a bit.
> 
> Start a new document to hopefully better explain the current state of
> affairs.
> 
> The old atomic_ops.txt also covers bitmaps and a few more details so
> this is not a full replacement and we'll therefore keep that document
> around until such a time that we've managed to write more text to cover
> its entire.
> 
> Also please, ReST people, go away.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> ---
>  Documentation/atomic_bitops.txt   |   66 ++++++++++++
>  Documentation/atomic_t.txt        |  200 ++++++++++++++++++++++++++++++++++++++
>  Documentation/memory-barriers.txt |   88 ----------------
>  3 files changed, 269 insertions(+), 85 deletions(-)
> 
> --- /dev/null
> +++ b/Documentation/atomic_bitops.txt
> @@ -0,0 +1,66 @@
> +
> +On atomic bitops.
> +
> +
> +While our bitmap_{}() functions are non-atomic, we have a number of operations
> +operating on single bits in a bitmap that are atomic.
> +
> +
> +API
> +---
> +
> +The single bit operations are:
> +
> +Non RmW ops:

   Non-RmW ops:

> +
> +  test_bit()
> +

[]

> --- /dev/null
> +++ b/Documentation/atomic_t.txt
> @@ -0,0 +1,200 @@
> +
> +On atomic types (atomic_t atomic64_t and atomic_long_t).
> +
> +The atomic type provides an interface to the architecture's means of atomic
> +RmW operations between CPUs (atomic operations on MMIO are not supported and
> +can lead to fatal traps on some platforms).
> +
> +API
> +---
> +
> +The 'full' API consists of (atomic64_ and atomic_long_ prefixes omitted for
> +brevity):
> +
> +Non RmW ops:

   Non-RmW ops:

> +
> +  atomic_read(), atomic_set()
> +  atomic_read_acquire(), atomic_set_release()
> +
> +
> +RmW atomic operations:
> +
> +Arithmetic:
> +
> +  atomic_{add,sub,inc,dec}()
> +  atomic_{add,sub,inc,dec}_return{,_relaxed,_acquire,_release}()
> +  atomic_fetch_{add,sub,inc,dec}{,_relaxed,_acquire,_release}()
> +
> +
> +Bitwise:
> +
> +  atomic_{and,or,xor,andnot}()
> +  atomic_fetch_{and,or,xor,andnot}{,_relaxed,_acquire,_release}()
> +
> +
> +Swap:
> +
> +  atomic_xchg{,_relaxed,_acquire,_release}()
> +  atomic_cmpxchg{,_relaxed,_acquire,_release}()
> +  atomic_try_cmpxchg{,_relaxed,_acquire,_release}()
> +
> +
> +Reference count (but please see refcount_t):
> +
> +  atomic_add_unless(), atomic_inc_not_zero()
> +  atomic_sub_and_test(), atomic_dec_and_test()
> +
> +
> +Misc:
> +
> +  atomic_inc_and_test(), atomic_add_negative()
> +  atomic_dec_unless_positive(), atomic_inc_unless_negative()
> +
> +
> +Barriers:
> +
> +  smp_mb__{before,after}_atomic()
> +
> +
> +
> +SEMANTICS
> +---------
> +
> +Non RmW ops:

   Non-RmW ops:

> +
> +The non-RmW ops are (typically) regular LOADs and STOREs and are canonically
> +implemented using READ_ONCE(), WRITE_ONCE(), smp_load_acquire() and
> +smp_store_release() respectively.
> +
> +The one detail to this is that atomic_set{}() should be observable to the RmW
> +ops. That is:
> +
> +  C atomic-set
> +
> +  {
> +    atomic_set(v, 1);
> +  }
> +
> +  P1(atomic_t *v)
> +  {
> +    atomic_add_unless(v, 1, 0);
> +  }
> +
> +  P2(atomic_t *v)
> +  {
> +    atomic_set(v, 0);
> +  }
> +
> +  exists
> +  (v=2)
> +
> +In this case we would expect the atomic_set() from CPU1 to either happen
> +before the atomic_add_unless(), in which case that latter one would no-op, or
> +_after_ in which case we'd overwrite its result. In no case is "2" a valid
> +outcome.
> +
> +This is typically true on 'normal' platforms, where a regular competing STORE
> +will invalidate a LL/SC or fail a CMPXCHG.
> +
> +The obvious case where this is not so is when we need to implement atomic ops
> +with a lock:
> +
> +  CPU0						CPU1
> +
> +  atomic_add_unless(v, 1, 0);
> +    lock();
> +    ret = READ_ONCE(v->counter); // == 1
> +						atomic_set(v, 0);
> +    if (ret != u)				  WRITE_ONCE(v->counter, 0);
> +      WRITE_ONCE(v->counter, ret + 1);
> +    unlock();
> +
> +the typical solution is to then implement atomic_set{}() with atomic_xchg().
> +
> +
> +RmW ops:

I prefer (and have usually seen) RMW, but it's your document -- er, text file. :)


> +
> +These come in various forms:
> +

[]

> --- a/Documentation/memory-barriers.txt
> +++ b/Documentation/memory-barriers.txt
> @@ -498,7 +498,7 @@ VARIETIES OF MEMORY BARRIER
>       This means that ACQUIRE acts as a minimal "acquire" operation and
>       RELEASE acts as a minimal "release" operation.
>  
> -A subset of the atomic operations described in core-api/atomic_ops.rst have
> +A subset of the atomic operations described in atomic_t.txt have ACQUIRE
>  ACQUIRE and RELEASE variants in addition to fully-ordered and relaxed (no

duplicate ACQUIRE.

>  barrier semantics) definitions.  For compound atomics performing both a load
>  and a store, ACQUIRE semantics apply only to the load and RELEASE semantics



-- 
~Randy

Powered by blists - more mailing lists