| Message ID | 1458056124-8024-23-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 15/03/16 15:35, Andrew Cooper wrote: > /* calculate required space for number of longs needed to hold nr_bits */ > static inline int bitmap_size(int nr_bits) > { > - int nr_long, nr_bytes; > - nr_long = (nr_bits + BITS_PER_LONG - 1) >> ORDER_LONG; > - nr_bytes = nr_long * sizeof(unsigned long); > - return nr_bytes; > + return (nr_bits + 7) >> 8; Turns out this is a bug and should be / 8 rather than >> 8. I have it fixed locally, but won't send a v4 quite yet. ~Andrew
On Tue, Mar 15, 2016 at 03:35:18PM +0000, Andrew Cooper wrote: > The type of the pointer to a bitmap is not interesting; it does not affect the > representation of the block of bits being pointed to. > > Make the libxc functions consistent with those in Xen, so they can work just > as well with 'unsigned int *' based bitmaps. > > As part of doing so, change the implementation to be in terms of char rather > than unsigned long. This fixes alignment concerns with ARM. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Ian Jackson <Ian.Jackson@eu.citrix.com> > CC: Wei Liu <wei.liu2@citrix.com> > > v3: > * Implement in terms of char rather than unsigned long to fix alignment > issues for ARM. I'm very illiterate on this subject so I've CC Julien and Stefano for input. > v2: > * New > --- > tools/libxc/xc_bitops.h | 37 ++++++++++++++++++++----------------- > 1 file changed, 20 insertions(+), 17 deletions(-) > > diff --git a/tools/libxc/xc_bitops.h b/tools/libxc/xc_bitops.h > index cd749f4..c2c96d3 100644 > --- a/tools/libxc/xc_bitops.h > +++ b/tools/libxc/xc_bitops.h > @@ -6,70 +6,73 @@ > #include <stdlib.h> > #include <string.h> > > +/* Needed by several includees, but no longer used for bitops. */ > #define BITS_PER_LONG (sizeof(unsigned long) * 8) > #define ORDER_LONG (sizeof(unsigned long) == 4 ? 5 : 6) > > -#define BITMAP_ENTRY(_nr,_bmap) ((_bmap))[(_nr)/BITS_PER_LONG] > -#define BITMAP_SHIFT(_nr) ((_nr) % BITS_PER_LONG) > +#define BITMAP_ENTRY(_nr,_bmap) ((_bmap))[(_nr) / 8] > +#define BITMAP_SHIFT(_nr) ((_nr) % 8) > > /* calculate required space for number of longs needed to hold nr_bits */ > static inline int bitmap_size(int nr_bits) > { > - int nr_long, nr_bytes; > - nr_long = (nr_bits + BITS_PER_LONG - 1) >> ORDER_LONG; > - nr_bytes = nr_long * sizeof(unsigned long); > - return nr_bytes; > + return (nr_bits + 7) >> 8; > } > > -static inline unsigned long *bitmap_alloc(int nr_bits) > +static inline void *bitmap_alloc(int nr_bits) > { > return calloc(1, bitmap_size(nr_bits)); > } > > -static inline void bitmap_set(unsigned long *addr, int nr_bits) > +static inline void bitmap_set(void *addr, int nr_bits) > { > memset(addr, 0xff, bitmap_size(nr_bits)); > } > > -static inline void bitmap_clear(unsigned long *addr, int nr_bits) > +static inline void bitmap_clear(void *addr, int nr_bits) > { > memset(addr, 0, bitmap_size(nr_bits)); > } > > -static inline int test_bit(int nr, unsigned long *addr) > +static inline int test_bit(int nr, const void *_addr) > { > + const char *addr = _addr; > return (BITMAP_ENTRY(nr, addr) >> BITMAP_SHIFT(nr)) & 1; > } > > -static inline void clear_bit(int nr, unsigned long *addr) > +static inline void clear_bit(int nr, void *_addr) > { > + char *addr = _addr; > BITMAP_ENTRY(nr, addr) &= ~(1UL << BITMAP_SHIFT(nr)); > } > > -static inline void set_bit(int nr, unsigned long *addr) > +static inline void set_bit(int nr, void *_addr) > { > + char *addr = _addr; > BITMAP_ENTRY(nr, addr) |= (1UL << BITMAP_SHIFT(nr)); > } > > -static inline int test_and_clear_bit(int nr, unsigned long *addr) > +static inline int test_and_clear_bit(int nr, void *addr) > { > int oldbit = test_bit(nr, addr); > clear_bit(nr, addr); > return oldbit; > } > > -static inline int test_and_set_bit(int nr, unsigned long *addr) > +static inline int test_and_set_bit(int nr, void *addr) > { > int oldbit = test_bit(nr, addr); > set_bit(nr, addr); > return oldbit; > } > > -static inline void bitmap_or(unsigned long *dst, const unsigned long *other, > +static inline void bitmap_or(void *_dst, const void *_other, > int nr_bits) > { > - int i, nr_longs = (bitmap_size(nr_bits) / sizeof(unsigned long)); > - for ( i = 0; i < nr_longs; ++i ) > + char *dst = _dst; > + const char *other = _other; > + int i; > + for ( i = 0; i < bitmap_size(nr_bits); ++i ) > dst[i] |= other[i]; > } > > -- > 2.1.4 >
diff --git a/tools/libxc/xc_bitops.h b/tools/libxc/xc_bitops.h index cd749f4..c2c96d3 100644 --- a/tools/libxc/xc_bitops.h +++ b/tools/libxc/xc_bitops.h @@ -6,70 +6,73 @@ #include <stdlib.h> #include <string.h> +/* Needed by several includees, but no longer used for bitops. */ #define BITS_PER_LONG (sizeof(unsigned long) * 8) #define ORDER_LONG (sizeof(unsigned long) == 4 ? 5 : 6) -#define BITMAP_ENTRY(_nr,_bmap) ((_bmap))[(_nr)/BITS_PER_LONG] -#define BITMAP_SHIFT(_nr) ((_nr) % BITS_PER_LONG) +#define BITMAP_ENTRY(_nr,_bmap) ((_bmap))[(_nr) / 8] +#define BITMAP_SHIFT(_nr) ((_nr) % 8) /* calculate required space for number of longs needed to hold nr_bits */ static inline int bitmap_size(int nr_bits) { - int nr_long, nr_bytes; - nr_long = (nr_bits + BITS_PER_LONG - 1) >> ORDER_LONG; - nr_bytes = nr_long * sizeof(unsigned long); - return nr_bytes; + return (nr_bits + 7) >> 8; } -static inline unsigned long *bitmap_alloc(int nr_bits) +static inline void *bitmap_alloc(int nr_bits) { return calloc(1, bitmap_size(nr_bits)); } -static inline void bitmap_set(unsigned long *addr, int nr_bits) +static inline void bitmap_set(void *addr, int nr_bits) { memset(addr, 0xff, bitmap_size(nr_bits)); } -static inline void bitmap_clear(unsigned long *addr, int nr_bits) +static inline void bitmap_clear(void *addr, int nr_bits) { memset(addr, 0, bitmap_size(nr_bits)); } -static inline int test_bit(int nr, unsigned long *addr) +static inline int test_bit(int nr, const void *_addr) { + const char *addr = _addr; return (BITMAP_ENTRY(nr, addr) >> BITMAP_SHIFT(nr)) & 1; } -static inline void clear_bit(int nr, unsigned long *addr) +static inline void clear_bit(int nr, void *_addr) { + char *addr = _addr; BITMAP_ENTRY(nr, addr) &= ~(1UL << BITMAP_SHIFT(nr)); } -static inline void set_bit(int nr, unsigned long *addr) +static inline void set_bit(int nr, void *_addr) { + char *addr = _addr; BITMAP_ENTRY(nr, addr) |= (1UL << BITMAP_SHIFT(nr)); } -static inline int test_and_clear_bit(int nr, unsigned long *addr) +static inline int test_and_clear_bit(int nr, void *addr) { int oldbit = test_bit(nr, addr); clear_bit(nr, addr); return oldbit; } -static inline int test_and_set_bit(int nr, unsigned long *addr) +static inline int test_and_set_bit(int nr, void *addr) { int oldbit = test_bit(nr, addr); set_bit(nr, addr); return oldbit; } -static inline void bitmap_or(unsigned long *dst, const unsigned long *other, +static inline void bitmap_or(void *_dst, const void *_other, int nr_bits) { - int i, nr_longs = (bitmap_size(nr_bits) / sizeof(unsigned long)); - for ( i = 0; i < nr_longs; ++i ) + char *dst = _dst; + const char *other = _other; + int i; + for ( i = 0; i < bitmap_size(nr_bits); ++i ) dst[i] |= other[i]; }
The type of the pointer to a bitmap is not interesting; it does not affect the representation of the block of bits being pointed to. Make the libxc functions consistent with those in Xen, so they can work just as well with 'unsigned int *' based bitmaps. As part of doing so, change the implementation to be in terms of char rather than unsigned long. This fixes alignment concerns with ARM. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Ian Jackson <Ian.Jackson@eu.citrix.com> CC: Wei Liu <wei.liu2@citrix.com> v3: * Implement in terms of char rather than unsigned long to fix alignment issues for ARM. v2: * New --- tools/libxc/xc_bitops.h | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-)