[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] Ping: [PATCH] bitmap_*() should cope with zero size bitmaps
>>> On 03.06.13 at 15:33, Jan Beulich wrote: > ... to match expectations set by memset()/memcpy(). > > Similarly for find_{first,next}_{,zero_}_bit() on x86. > > __bitmap_shift_{left,right}() would also need fixing (they more > generally can't cope with the shift count being larger than the bitmap > size, and they perform undefined operations by possibly shifting an > unsigned long value by BITS_PER_LONG bits), but since these functions > aren't really used anywhere I wonder if we wouldn't better simply get > rid of them. > > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> > > --- a/xen/include/asm-x86/bitops.h > +++ b/xen/include/asm-x86/bitops.h > @@ -327,10 +327,7 @@ static inline unsigned int __scanbit(uns > * Returns the bit-number of the first set bit, not the number of the byte > * containing a bit. > */ > -#define find_first_bit(addr,size) \ > -((__builtin_constant_p(size) && (size) <= BITS_PER_LONG ? \ > - (__scanbit(*(const unsigned long *)addr, size)) : \ > - __find_first_bit(addr,size))) > +#define find_first_bit(addr, size) find_next_bit(addr, size, 0) > > /** > * find_next_bit - find the first set bit in a memory region > @@ -338,10 +335,24 @@ static inline unsigned int __scanbit(uns > * @offset: The bitnumber to start searching at > * @size: The maximum size to search > */ > -#define find_next_bit(addr,size,off) \ > -((__builtin_constant_p(size) && (size) <= BITS_PER_LONG ? \ > - ((off) + (__scanbit((*(const unsigned long *)addr) >> (off), size))) : \ > - __find_next_bit(addr,size,off))) > +#define find_next_bit(addr, size, off) ({ \ > + unsigned int r__ = (size); \ > + unsigned int o__ = (off); \ > + switch ( -!__builtin_constant_p(size) | r__ ) \ > + { \ > + case 0: (void)(addr); break; \ > + case 1 ... BITS_PER_LONG: \ > + r__ = o__ + __scanbit(*(const unsigned long *)(addr) >> o__, r__); > \ > + break; \ > + default: \ > + if ( __builtin_constant_p(off) && !o__ ) \ > + r__ = __find_first_bit(addr, r__); \ > + else \ > + r__ = __find_next_bit(addr, r__, o__); \ > + break; \ > + } \ > + r__; \ > +}) > > /** > * find_first_zero_bit - find the first zero bit in a memory region > @@ -351,10 +362,7 @@ static inline unsigned int __scanbit(uns > * Returns the bit-number of the first zero bit, not the number of the byte > * containing a bit. > */ > -#define find_first_zero_bit(addr,size) \ > -((__builtin_constant_p(size) && (size) <= BITS_PER_LONG ? \ > - (__scanbit(~*(const unsigned long *)addr, size)) : \ > - __find_first_zero_bit(addr,size))) > +#define find_first_zero_bit(addr, size) find_next_zero_bit(addr, size, 0) > > /** > * find_next_zero_bit - find the first zero bit in a memory region > @@ -362,10 +370,24 @@ static inline unsigned int __scanbit(uns > * @offset: The bitnumber to start searching at > * @size: The maximum size to search > */ > -#define find_next_zero_bit(addr,size,off) > \ > -((__builtin_constant_p(size) && (size) <= BITS_PER_LONG ? > \ > - ((off)+(__scanbit(~(((*(const unsigned long *)addr)) >> (off)), size))) : > \ > - __find_next_zero_bit(addr,size,off))) > +#define find_next_zero_bit(addr, size, off) ({ \ > + unsigned int r__ = (size); \ > + unsigned int o__ = (off); \ > + switch ( -!__builtin_constant_p(size) | r__ ) \ > + { \ > + case 0: (void)(addr); break; \ > + case 1 ... BITS_PER_LONG: \ > + r__ = o__ + __scanbit(~*(const unsigned long *)(addr) >> o__, r__); > \ > + break; \ > + default: \ > + if ( __builtin_constant_p(off) && !o__ ) \ > + r__ = __find_first_zero_bit(addr, r__); \ > + else \ > + r__ = __find_next_zero_bit(addr, r__, o__); \ > + break; \ > + } \ > + r__; \ > +}) > > /** > * find_first_set_bit - find the first set bit in @word > --- a/xen/include/xen/bitmap.h > +++ b/xen/include/xen/bitmap.h > @@ -108,123 +108,122 @@ extern int bitmap_allocate_region(unsign > (1UL<<((nbits) % BITS_PER_LONG))-1 : ~0UL \ > ) > > +#define bitmap_bytes(nbits) (BITS_TO_LONGS(nbits) * sizeof(unsigned long)) > + > +#define bitmap_switch(nbits, zero_ret, small, large) \ > + switch (-!__builtin_constant_p(nbits) | (nbits)) { \ > + case 0: return zero_ret; \ > + case 1 ... BITS_PER_LONG: \ > + small; break; \ > + default: \ > + large; break; \ > + } > + > static inline void bitmap_zero(unsigned long *dst, int nbits) > { > - if (nbits <= BITS_PER_LONG) > - *dst = 0UL; > - else { > - int len = BITS_TO_LONGS(nbits) * sizeof(unsigned long); > - memset(dst, 0, len); > - } > + bitmap_switch(nbits,, > + *dst = 0UL, > + memset(dst, 0, bitmap_bytes(nbits))); > } > > static inline void bitmap_fill(unsigned long *dst, int nbits) > { > size_t nlongs = BITS_TO_LONGS(nbits); > - if (nlongs > 1) { > - int len = (nlongs - 1) * sizeof(unsigned long); > - memset(dst, 0xff, len); > + > + switch (nlongs) { > + default: > + memset(dst, -1, (nlongs - 1) * sizeof(unsigned long)); > + /* fall through */ > + case 1: > + dst[nlongs - 1] = BITMAP_LAST_WORD_MASK(nbits); > + break; > } > - dst[nlongs - 1] = BITMAP_LAST_WORD_MASK(nbits); > } > > static inline void bitmap_copy(unsigned long *dst, const unsigned long > *src, > int nbits) > { > - if (nbits <= BITS_PER_LONG) > - *dst = *src; > - else { > - int len = BITS_TO_LONGS(nbits) * sizeof(unsigned long); > - memcpy(dst, src, len); > - } > + bitmap_switch(nbits,, > + *dst = *src, > + memcpy(dst, src, bitmap_bytes(nbits))); > } > > static inline void bitmap_and(unsigned long *dst, const unsigned long > *src1, > const unsigned long *src2, int nbits) > { > - if (nbits <= BITS_PER_LONG) > - *dst = *src1 & *src2; > - else > - __bitmap_and(dst, src1, src2, nbits); > + bitmap_switch(nbits,, > + *dst = *src1 & *src2, > + __bitmap_and(dst, src1, src2, nbits)); > } > > static inline void bitmap_or(unsigned long *dst, const unsigned long *src1, > const unsigned long *src2, int nbits) > { > - if (nbits <= BITS_PER_LONG) > - *dst = *src1 | *src2; > - else > - __bitmap_or(dst, src1, src2, nbits); > + bitmap_switch(nbits,, > + *dst = *src1 | *src2, > + __bitmap_or(dst, src1, src2, nbits)); > } > > static inline void bitmap_xor(unsigned long *dst, const unsigned long > *src1, > const unsigned long *src2, int nbits) > { > - if (nbits <= BITS_PER_LONG) > - *dst = *src1 ^ *src2; > - else > - __bitmap_xor(dst, src1, src2, nbits); > + bitmap_switch(nbits,, > + *dst = *src1 ^ *src2, > + __bitmap_xor(dst, src1, src2, nbits)); > } > > static inline void bitmap_andnot(unsigned long *dst, const unsigned long > *src1, > const unsigned long *src2, int nbits) > { > - if (nbits <= BITS_PER_LONG) > - *dst = *src1 & ~(*src2); > - else > - __bitmap_andnot(dst, src1, src2, nbits); > + bitmap_switch(nbits,, > + *dst = *src1 & ~*src2, > + __bitmap_andnot(dst, src1, src2, nbits)); > } > > static inline void bitmap_complement(unsigned long *dst, const unsigned > long *src, > int nbits) > { > - if (nbits <= BITS_PER_LONG) > - *dst = ~(*src) & BITMAP_LAST_WORD_MASK(nbits); > - else > - __bitmap_complement(dst, src, nbits); > + bitmap_switch(nbits,, > + *dst = ~*src & BITMAP_LAST_WORD_MASK(nbits), > + __bitmap_complement(dst, src, nbits)); > } > > static inline int bitmap_equal(const unsigned long *src1, > const unsigned long *src2, int nbits) > { > - if (nbits <= BITS_PER_LONG) > - return ! ((*src1 ^ *src2) & BITMAP_LAST_WORD_MASK(nbits)); > - else > - return __bitmap_equal(src1, src2, nbits); > + bitmap_switch(nbits, -1, > + return !((*src1 ^ *src2) & BITMAP_LAST_WORD_MASK(nbits)), > + return __bitmap_equal(src1, src2, nbits)); > } > > static inline int bitmap_intersects(const unsigned long *src1, > const unsigned long *src2, int nbits) > { > - if (nbits <= BITS_PER_LONG) > - return ((*src1 & *src2) & BITMAP_LAST_WORD_MASK(nbits)) != 0; > - else > - return __bitmap_intersects(src1, src2, nbits); > + bitmap_switch(nbits, -1, > + return ((*src1 & *src2) & BITMAP_LAST_WORD_MASK(nbits)) != 0, > + return __bitmap_intersects(src1, src2, nbits)); > } > > static inline int bitmap_subset(const unsigned long *src1, > const unsigned long *src2, int nbits) > { > - if (nbits <= BITS_PER_LONG) > - return ! ((*src1 & ~(*src2)) & BITMAP_LAST_WORD_MASK(nbits)); > - else > - return __bitmap_subset(src1, src2, nbits); > + bitmap_switch(nbits, -1, > + return !((*src1 & ~*src2) & BITMAP_LAST_WORD_MASK(nbits)), > + return __bitmap_subset(src1, src2, nbits)); > } > > static inline int bitmap_empty(const unsigned long *src, int nbits) > { > - if (nbits <= BITS_PER_LONG) > - return ! (*src & BITMAP_LAST_WORD_MASK(nbits)); > - else > - return __bitmap_empty(src, nbits); > + bitmap_switch(nbits, -1, > + return !(*src & BITMAP_LAST_WORD_MASK(nbits)), > + return __bitmap_empty(src, nbits)); > } > > static inline int bitmap_full(const unsigned long *src, int nbits) > { > - if (nbits <= BITS_PER_LONG) > - return ! (~(*src) & BITMAP_LAST_WORD_MASK(nbits)); > - else > - return __bitmap_full(src, nbits); > + bitmap_switch(nbits, -1, > + return !(~*src & BITMAP_LAST_WORD_MASK(nbits)), > + return __bitmap_full(src, nbits)); > } > > static inline int bitmap_weight(const unsigned long *src, int nbits) > @@ -235,21 +234,22 @@ static inline int bitmap_weight(const un > static inline void bitmap_shift_right(unsigned long *dst, > const unsigned long *src, int n, int nbits) > { > - if (nbits <= BITS_PER_LONG) > - *dst = *src >> n; > - else > - __bitmap_shift_right(dst, src, n, nbits); > + bitmap_switch(nbits,, > + *dst = *src >> n, > + __bitmap_shift_right(dst, src, n, nbits)); > } > > static inline void bitmap_shift_left(unsigned long *dst, > const unsigned long *src, int n, int nbits) > { > - if (nbits <= BITS_PER_LONG) > - *dst = (*src << n) & BITMAP_LAST_WORD_MASK(nbits); > - else > - __bitmap_shift_left(dst, src, n, nbits); > + bitmap_switch(nbits,, > + *dst = (*src << n) & BITMAP_LAST_WORD_MASK(nbits), > + __bitmap_shift_left(dst, src, n, nbits)); > } > > +#undef bitmap_switch > +#undef bitmap_bytes > + > void bitmap_long_to_byte(uint8_t *bp, const unsigned long *lp, int nbits); > void bitmap_byte_to_long(unsigned long *lp, const uint8_t *bp, int nbits); > > > > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |