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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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