[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH] x86: fix variable_test_bit() asm constraints
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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |