Re: [PATCH v5 0/8] Add generic support for relaxed atomics

[Boqun Feng <boqun.feng@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2265 bytes --]

Hi Will,

On Wed, Aug 26, 2015 at 11:41:00AM +0100, Will Deacon wrote:
> Hi Boqun,
> 
> On Wed, Aug 26, 2015 at 05:28:34AM +0100, Boqun Feng wrote:
> > On Thu, Aug 06, 2015 at 05:54:36PM +0100, Will Deacon wrote:
> > > Will Deacon (8):
> > >   atomics: add acquire/release/relaxed variants of some atomic
> > >     operations
> > >   asm-generic: rework atomic-long.h to avoid bulk code duplication
> > >   asm-generic: add relaxed/acquire/release variants for atomic_long_t
> > >   lockref: remove homebrew cmpxchg64_relaxed macro definition
> > >   locking/qrwlock: implement queue_write_unlock using smp_store_release
> > >   locking/qrwlock: make use of acquire/release/relaxed atomics
> > >   include/llist: use linux/atomic.h instead of asm/cmpxchg.h
> > 
> > Should we step further to privatize asm/cmpxchg.h entirely? Keep it only
> > included in arch/*?
> > 
> > Because after your next patch, in some configurations, asm/cmpxchg.h of
> > ARM only provides the definition of {cmpxchg,xchg}_relaxed, others are
> > built in linux/atomic.h. Further more, other architecture may implement
> > asm/cmpxchg.h similarly in the future. So, IIUC, we actually don't
> > guarantee all cmpxchg(), xchg() and their variants are defined in
> > asm/cmpxchg.h.
> > 
> > Though current users of asm/cmpxchg.h outside arch/* are fine,
> > because they all happen to have got linux/atomic.h included. But we'd
> > better change the current users and call out that asm/cmpxchg.h is
> > privatized in the document.
> > 
> > >   ARM: atomics: define our SMP atomics in terms of _relaxed operations
> > 
> > 
> > Consider this patch maybe? I did a simple build test on X86.
> 
> I'm not sure that the addition to Documentation/atomic_ops.txt is really

Well.. I think at least we'd better say "to use {cmp,}xchg, include
linux/atomic.h" somewhere, because these two function families don't
have atomic_ prefix and don't take atomic{,64}_t as a parameter.

> worth it, but either way:
> 
>   Acked-by: Will Deacon <will.deacon@arm.com>

Thank you!

> 
> It's probably best to send this as a standalone patch, since the relaxed
> parts are already queued in -tip.

Good point. Will send a standalone patch.

Regards,
Boqun

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]