From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: [PATCH] x86: fix variable_test_bit() asm constraints Date: Fri, 14 Mar 2008 11:23:47 +0000 Message-ID: <47DA6E53.76E4.0078.0@novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org 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 Index: 2008-03-05/xen/include/asm-x86/bitops.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- 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.0000000= 00 +0100 @@ -254,7 +254,8 @@ static __inline__ int variable_test_bit( __asm__ __volatile__( "btl %2,%1\n\tsbbl %0,%0" :"=3Dr" (oldbit) - :"m" (CONST_ADDR),"dIr" (nr)); + :"m" (CONST_ADDR), "dIr" (nr), + "m" (((const volatile int *)addr)[nr >> 5])); return oldbit; } =20