diff mbox

[RFC,3/5] powerpc: atomic: implement atomic{,64}_{add,sub}_return_* variants

Message ID 55E7145C.7050505@gmail.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Pranith Kumar Sept. 2, 2015, 3:23 p.m. UTC 
Hi Will,

On 09/02/2015 05:59 AM, Will Deacon wrote:
> I just thought it was worth making this point, because it is prohibited
> in SC and I don't want people to think that our RELEASE/ACQUIRE operations
> are SC (even though they happen to be on arm64).

This is interesting information. Does that mean that the following patch
should work? (I am not proposing to use it, just trying to understand if 
REL+ACQ will act as a full barrier on ARM64, which you say it does).

Thanks,
Pranith.

Comments

Pranith Kumar Sept. 2, 2015, 3:36 p.m. UTC | #1 
On Wed, Sep 2, 2015 at 11:23 AM, Pranith Kumar <bobby.prani@gmail.com> wrote:
> Hi Will,
>
> On 09/02/2015 05:59 AM, Will Deacon wrote:
>> I just thought it was worth making this point, because it is prohibited
>> in SC and I don't want people to think that our RELEASE/ACQUIRE operations
>> are SC (even though they happen to be on arm64).
>
> This is interesting information. Does that mean that the following patch
> should work? (I am not proposing to use it, just trying to understand if
> REL+ACQ will act as a full barrier on ARM64, which you say it does).
>
> Thanks,
> Pranith.
>
> diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h
> index d8c25b7..14a1b35 100644
> --- a/arch/arm64/include/asm/cmpxchg.h
> +++ b/arch/arm64/include/asm/cmpxchg.h
> @@ -68,8 +68,7 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size
>                 BUILD_BUG();
>         }
>
> -       smp_mb();
> -       return ret;
> +       return smp_load_acquire(ret);

I meant 'smp_load_acquire(&ret);'
Will Deacon Sept. 3, 2015, 10:31 a.m. UTC | #2 
On Wed, Sep 02, 2015 at 04:36:09PM +0100, Pranith Kumar wrote:
> On Wed, Sep 2, 2015 at 11:23 AM, Pranith Kumar <bobby.prani@gmail.com> wrote:
> > On 09/02/2015 05:59 AM, Will Deacon wrote:
> >> I just thought it was worth making this point, because it is prohibited
> >> in SC and I don't want people to think that our RELEASE/ACQUIRE operations
> >> are SC (even though they happen to be on arm64).
> >
> > This is interesting information. Does that mean that the following patch
> > should work? (I am not proposing to use it, just trying to understand if
> > REL+ACQ will act as a full barrier on ARM64, which you say it does).
> >
> > Thanks,
> > Pranith.
> >
> > diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h
> > index d8c25b7..14a1b35 100644
> > --- a/arch/arm64/include/asm/cmpxchg.h
> > +++ b/arch/arm64/include/asm/cmpxchg.h
> > @@ -68,8 +68,7 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size
> >                 BUILD_BUG();
> >         }
> >
> > -       smp_mb();
> > -       return ret;
> > +       return smp_load_acquire(ret);
> 
> I meant 'smp_load_acquire(&ret);'

Yes, I think that would work on arm64, but it's not portable between
architectures.

Will
diff mbox

Patch

diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h
index d8c25b7..14a1b35 100644
--- a/arch/arm64/include/asm/cmpxchg.h
+++ b/arch/arm64/include/asm/cmpxchg.h
@@ -68,8 +68,7 @@  static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size
                BUILD_BUG();
        }
 
-       smp_mb();
-       return ret;
+       return smp_load_acquire(ret);
 }
 
 #define xchg(ptr,x) \