Re: [Xen-devel] [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h)

[Corneliu ZUZU <czuzu@xxxxxxxxxxxxxxx>]

Hi Julien,

On 7/13/2016 3:14 PM, Julien Grall wrote:
> Hi Corneliu,
>
> On 13/07/2016 12:23, Corneliu ZUZU wrote:
> > Following Andrew Cooper's suggestion, create a common-side 
> > <xen/atomic.h> to
> > establish, among others, prototypes of atomic functions called from 
> > common-code.
> > Done to avoid introducing inconsistencies between arch-side 
> > <asm/atomic.h>
> > headers when we make subtle changes to one of them.
> >
> > Some arm-side macros had to be turned into inline functions in the 
> > process.
> You forgot to mention that you moved the code from 
> asm-arm/arm{32,64}/atomic.h to asm-arm/arm.h.
> Furthermore, this change should really be a separate patch.
Noted, will do.

> > Also includes a minor adjustment asm-x86/atomic.h: reorder 
> > atomic_inc_and_test()
> > to follow after atomic_inc().
> >
> > Signed-off-by: Corneliu ZUZU <czuzu@xxxxxxxxxxxxxxx>
> [...]
>
> > diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
> > index 29ab265..8e8c5d1 100644
> > --- a/xen/include/asm-arm/atomic.h
> > +++ b/xen/include/asm-arm/atomic.h
> > @@ -2,6 +2,7 @@
> >  #define __ARCH_ARM_ATOMIC__
> >
> >  #include <xen/config.h>
> > +#include <xen/atomic.h>
> >  #include <xen/prefetch.h>
> >  #include <asm/system.h>
> >
> > @@ -95,15 +96,6 @@ void __bad_atomic_size(void);
> >      default: __bad_atomic_size(); 
> > break;                                \
> > } \
> >  })
> > -
> > -/*
> > - * NB. I've pushed the volatile qualifier into the operations. This 
> > allows
> > - * fast accessors such as _atomic_read() and _atomic_set() which 
> > don't give
> > - * the compiler a fit.
> > - */
> > -typedef struct { int counter; } atomic_t;
> > -
> > -#define ATOMIC_INIT(i) { (i) }
> >
> >  /*
> >   * On ARM, ordinary assignment (str instruction) doesn't clear the 
> > local
> > @@ -138,6 +130,85 @@ static inline void _atomic_set(atomic_t *v, int i)
> >  # error "unknown ARM variant"
> >  #endif
> >
> > +#define atomic_inc_return(v)    (atomic_add_return(1, v))
> > +#define atomic_dec_return(v)    (atomic_sub_return(1, v))
> > +
> > +/**
> > + * atomic_sub_and_test - subtract value from variable and test result
> > + * @i: integer value to subtract
> > + * @v: pointer of type atomic_t
> > + *
> > + * Atomically subtracts @i from @v and returns
> > + * true if the result is zero, or false for all
> > + * other cases.
> > + */
> > +static inline int atomic_sub_and_test(int i, atomic_t *v)
> > +{
> > +    return 0 == atomic_sub_return(i, v);
> > +}
> Please don't use yoda condition. It's less readable and compiler have 
> safety nowadays to prevent using "=".
Oh yeah, given that Xen's compiled with -Wall -Werror that's not 
necessary. Ack.
> > +
> > +/**
> > + * atomic_inc - increment atomic variable
> > + * @v: pointer of type atomic_t
> > + *
> > + * Atomically increments @v by 1.
> > + */
> > +static inline void atomic_inc(atomic_t *v)
> > +{
> > +    atomic_add(1, v);
> > +}
> Regards,
Thanks,
Corneliu.

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