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

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



On 26/03/15 08:00, Jan Beulich wrote:
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.

Presumably some code was lacking barrier()s

  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).

Ok - lets leave that for v2.

Both patches Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>


(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


 


Rackspace

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