Re: [Xen-devel] [PATCHv2 4/6] x86: provide xadd()

[Tim Deegan <tim@xxxxxxx>]

At 12:38 +0100 on 16 Apr (1429187920), David Vrabel wrote:
> On 16/04/15 12:25, Tim Deegan wrote:
> > At 15:19 +0100 on 10 Apr (1428679195), David Vrabel wrote:
> >> xadd() atomically adds a value and returns the previous value.  This
> >> is needed to implement ticket locks.
> >>
> >> Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
> >> ---
> >>  xen/include/asm-x86/system.h |   29 +++++++++++++++++++++++++++++
> >>  1 file changed, 29 insertions(+)
> >>
> >> diff --git a/xen/include/asm-x86/system.h b/xen/include/asm-x86/system.h
> >> index 7111329..1e6c6a8 100644
> >> --- a/xen/include/asm-x86/system.h
> >> +++ b/xen/include/asm-x86/system.h
> >> @@ -117,6 +117,35 @@ static always_inline unsigned long __cmpxchg(
> >>                                     (unsigned long)__n,sizeof(*(ptr)))); \
> >>  })
> >>  
> >> +static always_inline unsigned long __xadd(
> >> +    volatile void *ptr, unsigned long v, int size)
> >> +{
> >> +    switch ( size )
> >> +    {
> >> +    case 1:
> >> +        asm volatile ( "lock; xaddb %b0,%1"
> >> +                       : "+r" (v), "+m" (*__xg((volatile void *)ptr)));
> >> +        return v;
> >> +    case 2:
> >> +        asm volatile ( "lock; xaddw %w0,%1"
> >> +                       : "+r" (v), "+m" (*__xg((volatile void *)ptr)));
> >> +        return v;
> >> +    case 4:
> >> +        asm volatile ( "lock; xaddl %k0,%1"
> >> +                       : "+r" (v), "+m" (*__xg((volatile void *)ptr)));
> >> +        return v;
> >> +    case 8:
> >> +        asm volatile ( "lock; xaddq %q0,%1"
> >> +                       : "+r" (v), "+m" (*__xg((volatile void *)ptr)));
> >> +        return v;
> >> +    }
> >> +    return 0;
> > 
> > ASSERT_UNREACHABLE(), rather?  And some appropriate BUILD_BUG_ON?
> > 
> > But also: AFAICS the GCC builtin __sync_fetch_and_add() does almost
> > exactly this (the difference being that those are also compiler
> > barriers where this is only a CPU barrier).  Should we be using it
> > instead?
> 
> I wasn't aware of the GCC built-ins -- I'll try them out. I think asm
> volatile is a compiler barrier though.

Nope - you need to specify a "memory" clobber for that (as the otehr
similar ops in this file do).

Cheers,

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel