From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH] x86: fix variable_test_bit()asmconstraints Date: Fri, 14 Mar 2008 16:42:14 +0000 Message-ID: <47DAB8F6.76E4.0078.0@novell.com> References: <47DA9596.76E4.0078.0@novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: 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: Keir Fraser Cc: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org >>> Keir Fraser 14.03.08 16:37 >>> >On 14/3/08 14:11, "Jan Beulich" wrote: > >>> Regarding your other reply: I would actually be happy to change the = bitops >>> to work with longs only. I suspect, and would need to have demonstrated= >>> otherwise, that supporting bitops on arbitrary-width fields down to = the >>> instruction level is not really worthwhile. Either way, I accept that = what >>> we do now is dubious at best. >>=20 >> Hmm, change it to work with longs only but also make it work without >> casts? You mean then change all places where bitops are applied to >> exclusively use 'unsigned long' as the fundamental type? I didn't look >> at the number of places that would require changing, but I'm afraid = it'd >> be quite a few (and it might get you further away from Linux originals >> in some cases). > >Yes, I wouldn't have expected that too be too hard, really. Are there = lots >of places with arrays of 32-bit integers? I doubt 16-bit or 8-bit fields = are >at all common. Actually, just trying it out with set_bit() results in a number of cases where the field used is neither 32- nor 64-bit. The very first one I looked at even has only a byte-aligned (leaving out internal knowledge of the allocator) allocation that it accesses (domid_bitmap in xen/drivers/passthrough/vtd/iommu.c). Also, I'm somewhat reluctant to go with longs only - the REX prefix needed to operate on them on x86-64 could be saved generally, so I'd rather go with a slightly more complicated implementation like static __inline__ void __set_bit32(int nr, volatile void * addr) { __asm__ __volatile__( LOCK_PREFIX "btsl %1,%0" : : "m" (ADDR(int)), "Ir" (nr) : "memory"); } static __inline__ void __set_bit64(long nr, volatile void * addr) { __asm__ __volatile__( LOCK_PREFIX "btsq %1,%0" : : "m" (ADDR(long)), "Jr" (nr) : "memory"); } extern void __set_bit_unknown(int, volatile void *); #define set_bit(nr, addr) ({ \ switch ( min(sizeof(*(addr)), __alignof__(*(addr))) ) \ { \ case 8: __set_bit64(nr, addr); break; \ case 4: __set_bit32(nr, addr); break; \ default: __set_bit_unknown(nr, addr); break; \ } \ }) #undef ADDR #define ADDR (*(volatile long *) addr)