[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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





On 25/02/2020 16:15, Jan Beulich wrote:
On 24.02.2020 11:29, Julien Grall wrote:
On 24/02/2020 10:23, Roger Pau Monné wrote:
On Mon, Feb 24, 2020 at 10:19:44AM +0000, Julien Grall wrote:
On 24/02/2020 10:09, Roger Pau Monné wrote:
On Mon, Feb 24, 2020 at 10:02:53AM +0000, Julien Grall wrote:
On 24/02/2020 08:43, Roger Pau Monne wrote:
--- 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.

So, having reached agreement to use plain int, would you be willing
to give your ack provided the adjustments get made while committing,
to save another round trip?

Yes, the implementation of the atomic for arm looked correct:

Reviewed-by: Julien Grall <julien@xxxxxxx>

Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.