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

[Keir Fraser <keir.fraser@eu.citrix.com>]

On 14/3/08 16:42, "Jan Beulich" <jbeulich@novell.com> wrote:

> 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).

How did you find that one? It's void* so I would have thought you'd miss
that one as the compiler will happily cast void*. I hope there aren't too
many lurkers like that! Perhaps you were trying to do your automatic
field-width detection approach. I think that's not needed, but it would
conveniently find these void* callers. Perhaps we should wrap the bitops in
a macro that will fail on void*?

I'm happy to do this change (void* -> long*) myself, by the way, as it's the
kind of thing that's as much work to review as it is to do in the first
place.

> 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

There's no need to use 64-bit instruction forms even if we do take 'unsigned
long'. After all, the existing bitops implementations only act on 32-bit
words -- we should continue with this.

Where bitops are concerned the actual operand size doesn't really matter
(except that it shouldn't be so big as to overlap with adjacent fields which
may be updated in parallel). The only reason for not using even
smaller-width instructions is that byte-sized bit-twiddling instructions do
not exist, and the 16-bit ones are restricted in the size of index (by
comparison 32 bits is sufficient, as all 'nr' arguments to our bitops are
'int' type).

 -- Keir