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

[Corneliu ZUZU <czuzu@xxxxxxxxxxxxxxx>]

On 7/13/2016 3:12 PM, Andrew Cooper wrote:
> On 13/07/16 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.
> >
> > Also includes a minor adjustment asm-x86/atomic.h: reorder atomic_inc_and_test()
> > to follow after atomic_inc().
> Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Nice, didn't know that.

> > Signed-off-by: Corneliu ZUZU <czuzu@xxxxxxxxxxxxxxx>
> Thanks for doing this!
>
> > diff --git a/xen/include/xen/atomic.h b/xen/include/xen/atomic.h
> > new file mode 100644
> > index 0000000..5d5c051
> > --- /dev/null
> > +++ b/xen/include/xen/atomic.h
> > @@ -0,0 +1,155 @@
> > +/*
> > + * include/xen/atomic.h
> > + *
> > + * Common atomic operations entities (atomic_t, function prototypes).
> > + * Include _from_ arch-side <asm/atomic.h>.
> > + *
> > + * Copyright (c) 2016 Bitdefender S.R.L.
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along with
> > + * this program; If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#ifndef __XEN_ATOMIC_H__
> > +#define __XEN_ATOMIC_H__
> > +
> > +/*
> > + * 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.
> > + */
> I would recommend simply dropping this paragraph.  Is is very out of date.
Ack.

> > +typedef struct { int counter; } atomic_t;
> > +
> > +#define ATOMIC_INIT(i) { (i) }
> > +
> > +/**
> > + * atomic_read - read atomic variable
> > + * @v: pointer of type atomic_t
> > + *
> > + * Atomically reads the value of @v.
> > + */
> > +static inline int atomic_read(atomic_t *v);
> > +
> > +/**
> > + * _atomic_read - read atomic variable non-atomically
> > + * @v atomic_t
> > + *
> > + * Non-atomically reads the value of @v
> > + */
> > +static inline int _atomic_read(atomic_t v);
> > +
> > +/**
> > + * atomic_set - set atomic variable
> > + * @v: pointer of type atomic_t
> > + * @i: required value
> > + *
> > + * Atomically sets the value of @v to @i.
> > + */
> > +static inline void atomic_set(atomic_t *v, int i);
> > +
> > +/**
> > + * _atomic_set - set atomic variable non-atomically
> > + * @v: pointer of type atomic_t
> > + * @i: required value
> > + *
> > + * Non-atomically sets the value of @v to @i.
> > + */
> > +static inline void _atomic_set(atomic_t *v, int i);
> > +
> /**
>   * atomic_cmpxchg - compare and exchange an atomic variable
>   *...
>   */
Ack.

> > +static inline int atomic_cmpxchg(atomic_t *v, int old, int new);
> With those changes, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Thanks too for the prompt review,
Zuzu C.

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