Re: [Xen-devel] [PATCH v3 1/2] x86: simplify nonâatomic bitops

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.03.15 at 18:49, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 20/03/15 14:53, Jan Beulich wrote:
>> - being non-atomic, their pointer arguments shouldn't be volatile-
>>    qualified
>> - their (half fake) memory operands can be a single "+m" instead of
>>    being both an output and an input
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> After further consideration, would it not be better to change the 
> non-atomic variants to being straight C.
> 
> e.g.
> 
> static inline void __set_bit(int nr, void *_addr)
> {
>      int *addr = _addr;
>      addr[nr / sizeof(int)] |= (1U << (nr % sizeof(int)));
> }
> 
> This would drop the memory clobber from the asm statement and allow the 
> compiler to optimise repeated __set_bit() calls to the same word into a 
> single action.

I wouldn't want to do this in this patch - a similar change that I
proposed years ago for Linux (not eliminating the asm(), but
dropping the clobbers) caused not really understood regressions and
hence needed to be reverted. We could still try whether this works in
Xen, but in another patch. Apart from that I doubt the operation
above would reliably get converted to BTS by the compiler
(independent of version).

> (On a separate note, I feel that all of these operations should be 
> acting on unsigned rather than signed ints, but that applies to all of 
> these operations, not just the non-atomic ones)

That makes zero difference for these ops - all that really matters
is the width. In the C version above however I agree that using
"unsigned int" would be more natural.

Jan


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