Re: [PATCH] x86: fix variable_test_bit()asmconstraints

["Jan Beulich" <jbeulich@novell.com>]

>>> Keir Fraser <keir.fraser@eu.citrix.com> 14.03.08 16:37 >>>
>On 14/3/08 14:11, "Jan Beulich" <jbeulich@novell.com> 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.
>> 
>> 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)