I just sent a (much bigger, see below) patch to the same effect to the
x86 Linux maintainers - in Xen, all the operations modifying bits do
have "memory" clobbers, so it's just the test_bit() constraint that's
wrong.
However, I wonder whether the non-atomic ops aren't limiting things too
much by having "memory" clobbers, they would much better be restricted
to indicate just the changing memory location. This, however, would
probably require some additional consideration given that Xen (other
than Linux) isn't using -fno-strict-aliasing. Furthermore, these
non-atomic operations, according to their comments, can be re-ordered,
which contradicts the use of __asm__ __volatile__ (but note that
removing this would probably require extra precautions to meet strict
aliasing rules).
Further, using 'void *' for the 'addr' parameter appears dangerous,
since bt{,c,r,s} access the full 32 bits (if 'unsigned long' was used
properly here, 64 bits for x86-64) pointed at, so invalid uses like
referencing a 'char' array cannot currently be caught.
Finally I find the leading 'd' constraints in the 'nr' assembly
operands quite odd - what is the purpose of that? Linux is using just
"Ir" here...
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
Index: 2008-03-05/xen/include/asm-x86/bitops.h
===================================================================
--- 2008-03-05.orig/xen/include/asm-x86/bitops.h 2007-09-14
11:03:32.000000000 +0200
+++ 2008-03-05/xen/include/asm-x86/bitops.h 2008-03-13 10:20:34.000000000
+0100
@@ -254,7 +254,8 @@ static __inline__ int variable_test_bit(
__asm__ __volatile__(
"btl %2,%1\n\tsbbl %0,%0"
:"=r" (oldbit)
- :"m" (CONST_ADDR),"dIr" (nr));
+ :"m" (CONST_ADDR), "dIr" (nr),
+ "m" (((const volatile int *)addr)[nr >> 5]));
return oldbit;
}
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home