Re: [Xen-devel] [PATCH v3 1/2] atomic: add atomic_and operations

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Feb 24, 2020 at 10:29:40AM +0000, Julien Grall wrote:
> Hi,
> 
> On 24/02/2020 10:23, Roger Pau Monné wrote:
> > On Mon, Feb 24, 2020 at 10:19:44AM +0000, Julien Grall wrote:
> > > Hi,
> > > 
> > > On 24/02/2020 10:09, Roger Pau Monné wrote:
> > > > On Mon, Feb 24, 2020 at 10:02:53AM +0000, Julien Grall wrote:
> > > > > Hi Roger,
> > > > > 
> > > > > The logic for Arm64 and Arm32 looks good to me. I just have one 
> > > > > question
> > > > > below.
> > > > > 
> > > > > On 24/02/2020 08:43, Roger Pau Monne wrote:
> > > > > > To x86 and Arm. This performs an atomic AND operation against an
> > > > > > atomic_t variable with the provided mask.
> > > > > > 
> > > > > > Requested-by: Jan Beulich <jbeulich@xxxxxxxx>
> > > > > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > > > > > ---
> > > > > >     xen/include/asm-arm/arm32/atomic.h | 17 +++++++++++++++++
> > > > > >     xen/include/asm-arm/arm64/atomic.h | 14 ++++++++++++++
> > > > > >     xen/include/asm-x86/atomic.h       |  8 ++++++++
> > > > > >     3 files changed, 39 insertions(+)
> > > > > > 
> > > > > > diff --git a/xen/include/asm-arm/arm32/atomic.h 
> > > > > > b/xen/include/asm-arm/arm32/atomic.h
> > > > > > index c03eb684cd..4637381bcc 100644
> > > > > > --- a/xen/include/asm-arm/arm32/atomic.h
> > > > > > +++ b/xen/include/asm-arm/arm32/atomic.h
> > > > > > @@ -96,6 +96,23 @@ static inline int atomic_sub_return(int i, 
> > > > > > atomic_t *v)
> > > > > >             return result;
> > > > > >     }
> > > > > > +static inline void atomic_and(unsigned int m, atomic_t *v)
> > > > > 
> > > > > All the atomic helpers have taken a signed int so far because the 
> > > > > counter is
> > > > > an int. Any reason to diverge from that?
> > > > 
> > > > Since this is not an arithmetic operation I felt unsigned int was a
> > > > more suitable type to describe a bitmask: it felt weird to pass a
> > > > bitmask with type int, because signedness doesn't make sense when
> > > > referring to a mask.
> > > 
> > > At some point I would like to have macro generating all the atomics in on
> > > Arm in the same way a Linux (see asm-generic/atomic.h). This is to avoid
> > > duplication and make easy to introduce Armv8.1 LSE atomics. So I would 
> > > like
> > > to avoid introducing difference in the prototype unless it is stricly
> > > necessary.
> > 
> > Why not have the macro generator function get passed the type of the
> > parameter?
> 
> It is not worth it for a simple operation and I don't want to diverge too
> much of atomics from Linux.

Hm, I'm would rather keep it as unsigned for type coherency, but if
x86 maintainers are also happy to have the type changed to int on x86
then I won't oppose to it.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel