* [XEN][vNUMA][PATCH 4/9] Basic cpumap utilities [not found] <1BEA8649F0C00540AB2811D7922ECB6C9338B4CD@orsmsx507.amr.corp.intel.com> @ 2010-07-02 23:54 ` Dulloor 2010-08-01 22:02 ` [vNUMA v2][PATCH 3/8] " Dulloor 0 siblings, 1 reply; 4+ messages in thread From: Dulloor @ 2010-07-02 23:54 UTC (permalink / raw) To: xen-devel [-- Attachment #1: Type: text/plain, Size: 136 bytes --] Implement basic utility functions to manipulate bitmasks. Used in later patches. -dulloor Signed-off-by : Dulloor <dulloor@gmail.com> [-- Attachment #2: xen-04-basic-cpumap-utils.patch --] [-- Type: text/x-patch, Size: 7352 bytes --] diff --git a/tools/libxc/Makefile b/tools/libxc/Makefile --- a/tools/libxc/Makefile +++ b/tools/libxc/Makefile @@ -27,6 +27,7 @@ CTRL_SRCS-y += xc_tmem.c CTRL_SRCS-y += xc_mem_event.c CTRL_SRCS-y += xc_mem_paging.c CTRL_SRCS-y += xc_memshr.c +CTRL_SRCS-y += xc_cpumap.c CTRL_SRCS-y += xtl_core.c CTRL_SRCS-y += xtl_logger_stdio.c CTRL_SRCS-$(CONFIG_X86) += xc_pagetab.c diff --git a/tools/libxc/xc_cpumap.c b/tools/libxc/xc_cpumap.c new file mode 100644 --- /dev/null +++ b/tools/libxc/xc_cpumap.c @@ -0,0 +1,88 @@ +#include "xc_cpumap.h" +#include <stdio.h> + +/* Author : Dulloor (dulloor@gatech.edu) */ + +uint32_t xc_cpumap_next(int cpu, struct xenctl_cpumap *srcp) +{ + uint8_t *p, pos; + uint8_t *addr = xc_cpumap_bits(srcp); + uint32_t size = xc_cpumap_len(srcp); + uint32_t offset = cpu+1; /* Find the next set cpu */ + + if (offset >= size) + return size; + + p = addr + XC_BITMAP_BYTE(offset); + pos = XC_BITMAP_BYTE_OFFSET(offset); + + do { + for (; (pos < XC_BITS_PER_BYTE) && !((*p)&(1<<pos)); pos++); + if (pos < XC_BITS_PER_BYTE) + break; + pos = 0; p++; + } while (p < (addr+size)); + + return (((p-addr)*XC_BITS_PER_BYTE) + pos); +} + +void __xc_cpumap_or(struct xenctl_cpumap *dstp, + struct xenctl_cpumap *src1p, struct xenctl_cpumap *src2p) +{ + uint8_t *dp = xc_cpumap_bits(dstp); + uint8_t *s1p = xc_cpumap_bits(src1p); + uint8_t *s2p = xc_cpumap_bits(src2p); + int nr = XC_BITS_TO_BYTES(xc_cpumap_len(dstp)); + int k; + for (k=0; k<nr; k++) + dp[k] = s1p[k] | s2p[k]; +} + +static inline uint8_t hweight8(uint8_t w) +{ + uint8_t res = (w & 0x55) + ((w >> 1) & 0x55); + res = (res & 0x33) + ((res >> 2) & 0x33); + return (res & 0x0F) + ((res >> 4) & 0x0F); +} + +int __xc_cpumap_weight(struct xenctl_cpumap *srcp) +{ + const uint8_t *sp = xc_cpumap_bits(srcp); + int k, w = 0, lim = XC_BITS_TO_BYTES(xc_cpumap_len(srcp)); + for (k=0; k <lim; k++) + w += hweight8(sp[k]); + return w; +} + +/* xenctl_cpumap print function */ +#define CHUNKSZ 8 +#define roundup_power2(val,modulus) (((val) + (modulus) - 1) & ~((modulus) - 1)) + +int __xc_cpumap_snprintf(char *buf, unsigned int buflen, + const struct xenctl_cpumap *cpumap) +{ + const uint8_t *maskp = xc_cpumap_bits(cpumap); + int nmaskbits = xc_cpumap_len(cpumap); + int i, word, bit, len = 0; + unsigned long val; + const char *sep = ""; + int chunksz; + uint8_t chunkmask; + + chunksz = nmaskbits & (CHUNKSZ - 1); + if (chunksz == 0) + chunksz = CHUNKSZ; + + i = roundup_power2(nmaskbits, CHUNKSZ) - CHUNKSZ; + for (; i >= 0; i -= CHUNKSZ) { + chunkmask = ((1ULL << chunksz) - 1); + word = i / XC_BITS_PER_BYTE; + bit = i % XC_BITS_PER_BYTE; + val = (maskp[word] >> bit) & chunkmask; + len += snprintf(buf+len, buflen-len, "%s%0*lx", sep, + (chunksz+3)/4, val); + chunksz = CHUNKSZ; + sep = ","; + } + return len; +} diff --git a/tools/libxc/xc_cpumap.h b/tools/libxc/xc_cpumap.h new file mode 100644 --- /dev/null +++ b/tools/libxc/xc_cpumap.h @@ -0,0 +1,113 @@ +#ifndef __XENCTL_CPUMAP_H +#define __XENCTL_CPUMAP_H + +#include "xc_private.h" +#include <stdint.h> +#include <string.h> + +#define XC_BITS_PER_BYTE 8 +#define XC_BITS_TO_BYTES(bits) \ + (((bits)+XC_BITS_PER_BYTE-1)/XC_BITS_PER_BYTE) +#define XC_BITMAP_BIT(nr) (1 << (nr)) +#define XC_BITMAP_BIT_MASK(nr) (1 << ((nr) % XC_BITS_PER_BYTE)) +#define XC_BITMAP_BYTE(nr) ((nr) / XC_BITS_PER_BYTE) +#define XC_BITMAP_BYTE_OFFSET(nr) ((nr) % XC_BITS_PER_BYTE) +#define XC_BITMAP_BYTE_MASK (0xFF) +#define XC_BITMAP_LAST_BYTE_MASK(nbits) \ + (((nbits) % XC_BITS_PER_BYTE) ? \ + ((1<<((nbits) % XC_BITS_PER_BYTE))-1) : \ + XC_BITMAP_BYTE_MASK) + +#define xc_cpumap_bits(maskp) \ + ({ uint8_t *bitmap; \ + get_xen_guest_handle(bitmap, (maskp)->bitmap); \ + bitmap; }) +#define xc_cpumap_len(maskp) ((maskp)->nr_cpus) + +/* For iterating over the cpus set in the cpumap */ +#define xc_for_each_cpu(cpu, mask) \ + __xc_for_each_cpu(cpu, &(mask)) +#define __xc_for_each_cpu(cpu, mask) \ + for ((cpu) = -1; \ + (cpu) = xc_cpumap_next((cpu), (mask)), \ + (cpu) < xc_cpumap_len(mask);) +extern uint32_t xc_cpumap_next(int n, struct xenctl_cpumap *srcp); + +#define xc_cpumap_set_cpu(cpu, dst) __xc_cpumap_set_cpu(cpu, &(dst)) +static inline void __xc_cpumap_set_cpu(int cpu, struct xenctl_cpumap *dstp) +{ + uint8_t mask = XC_BITMAP_BIT_MASK(cpu); + uint8_t *p = ((uint8_t *)xc_cpumap_bits(dstp)) + XC_BITMAP_BYTE(cpu); + *p |= mask; +} + +#define xc_cpumap_clear_cpu(cpu, dst) __xc_cpumap_clear_cpu(cpu, &(dst)) +static inline void __xc_cpumap_clear_cpu(int cpu, struct xenctl_cpumap *dstp) +{ + uint8_t mask = XC_BITMAP_BIT_MASK(cpu); + uint8_t *p = ((uint8_t *)xc_cpumap_bits(dstp)) + XC_BITMAP_BYTE(cpu); + *p &= ~mask; +} + +#define xc_cpumap_test_cpu(cpu, dst) __xc_cpumap_test_cpu(cpu, &(dst)) +static inline int __xc_cpumap_test_cpu(int cpu, struct xenctl_cpumap *dstp) +{ + uint8_t mask = XC_BITMAP_BIT_MASK(cpu); + uint8_t *p = ((uint8_t *)xc_cpumap_bits(dstp)) + XC_BITMAP_BYTE(cpu); + return *p & mask; +} + +#define xc_cpumap_setall(dst) __xc_cpumap_setall(&(dst)) +static inline void __xc_cpumap_setall(struct xenctl_cpumap *dstp) +{ + uint8_t *dp = xc_cpumap_bits(dstp); + int nbits = xc_cpumap_len(dstp); + size_t nbytes = XC_BITS_TO_BYTES(nbits); + if (nbytes > 1) + memset(dp, 0xff, nbytes); + dp[nbytes-1] = XC_BITMAP_LAST_BYTE_MASK(nbits); +} + +#define xc_cpumap_clearall(dst) __xc_cpumap_clearall(&(dst)) +static inline void __xc_cpumap_clearall(struct xenctl_cpumap *dstp) +{ + size_t nbytes = XC_BITS_TO_BYTES(xc_cpumap_len(dstp)); + if (nbytes > 1) + memset(xc_cpumap_bits(dstp), 0x00, nbytes); +} + +#define xc_cpumap_or(dst, src1, src2) \ + __xc_cpumap_or(&(dst), &(src1), &(src2)) +extern void __xc_cpumap_or(struct xenctl_cpumap *dstp, + struct xenctl_cpumap *src1p, struct xenctl_cpumap *src2p); + +#define xc_cpumap_weight(src) __xc_cpumap_weight(&(src)) +extern int __xc_cpumap_weight(struct xenctl_cpumap *srcp); + +#define xc_cpumap_snprintf(buf, len, src) \ + __xc_cpumap_snprintf((buf), (len), &(src)) +extern int __xc_cpumap_snprintf(char *buf, unsigned int len, + const struct xenctl_cpumap *srcp); + +/***********************************************************************/ +static inline int +xc_cpumap_lock_pages(struct xenctl_cpumap *map) +{ + uint8_t *bitmap; + uint32_t nr_bytes = XC_BITS_TO_BYTES(map->nr_cpus); + get_xen_guest_handle(bitmap, map->bitmap); + if (lock_pages(bitmap, nr_bytes)) + return -1; + return 0; +} + +static inline void +xc_cpumap_unlock_pages(struct xenctl_cpumap *map) +{ + uint8_t *bitmap; + uint32_t nr_bytes = XC_BITS_TO_BYTES(map->nr_cpus); + get_xen_guest_handle(bitmap, map->bitmap); + unlock_pages(bitmap, nr_bytes); +} + +#endif /* __XENCTL_CPUMAP_H */ [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* [vNUMA v2][PATCH 3/8] Basic cpumap utilities 2010-07-02 23:54 ` [XEN][vNUMA][PATCH 4/9] Basic cpumap utilities Dulloor @ 2010-08-01 22:02 ` Dulloor 2010-08-13 15:25 ` Andre Przywara 0 siblings, 1 reply; 4+ messages in thread From: Dulloor @ 2010-08-01 22:02 UTC (permalink / raw) To: xen-devel [-- Attachment #1: Type: text/plain, Size: 136 bytes --] Implement basic utility functions to manipulate bitmasks. Used in later patches. -dulloor Signed-off-by : Dulloor <dulloor@gmail.com> [-- Attachment #2: xen-03-basic-cpumap-utils.patch --] [-- Type: text/x-patch, Size: 7357 bytes --] vNUMA : Implement vNUMA strategies diff --git a/tools/libxc/Makefile b/tools/libxc/Makefile --- a/tools/libxc/Makefile +++ b/tools/libxc/Makefile @@ -27,6 +27,7 @@ CTRL_SRCS-y += xc_mem_event.c CTRL_SRCS-y += xc_mem_paging.c CTRL_SRCS-y += xc_memshr.c +CTRL_SRCS-y += xc_cpumap.c CTRL_SRCS-y += xtl_core.c CTRL_SRCS-y += xtl_logger_stdio.c CTRL_SRCS-$(CONFIG_X86) += xc_pagetab.c diff --git a/tools/libxc/xc_cpumap.c b/tools/libxc/xc_cpumap.c new file mode 100644 --- /dev/null +++ b/tools/libxc/xc_cpumap.c @@ -0,0 +1,88 @@ +#include "xc_cpumap.h" +#include <stdio.h> + +/* Author : Dulloor (dulloor@gatech.edu) */ + +uint32_t xc_cpumap_next(int cpu, struct xenctl_cpumap *srcp) +{ + uint8_t *p, pos; + uint8_t *addr = xc_cpumap_bits(srcp); + uint32_t size = xc_cpumap_len(srcp); + uint32_t offset = cpu+1; /* Find the next set cpu */ + + if (offset >= size) + return size; + + p = addr + XC_BITMAP_BYTE(offset); + pos = XC_BITMAP_BYTE_OFFSET(offset); + + do { + for (; (pos < XC_BITS_PER_BYTE) && !((*p)&(1<<pos)); pos++); + if (pos < XC_BITS_PER_BYTE) + break; + pos = 0; p++; + } while (p < (addr+size)); + + return (((p-addr)*XC_BITS_PER_BYTE) + pos); +} + +void __xc_cpumap_or(struct xenctl_cpumap *dstp, + struct xenctl_cpumap *src1p, struct xenctl_cpumap *src2p) +{ + uint8_t *dp = xc_cpumap_bits(dstp); + uint8_t *s1p = xc_cpumap_bits(src1p); + uint8_t *s2p = xc_cpumap_bits(src2p); + int nr = XC_BITS_TO_BYTES(xc_cpumap_len(dstp)); + int k; + for (k=0; k<nr; k++) + dp[k] = s1p[k] | s2p[k]; +} + +static inline uint8_t hweight8(uint8_t w) +{ + uint8_t res = (w & 0x55) + ((w >> 1) & 0x55); + res = (res & 0x33) + ((res >> 2) & 0x33); + return (res & 0x0F) + ((res >> 4) & 0x0F); +} + +int __xc_cpumap_weight(struct xenctl_cpumap *srcp) +{ + const uint8_t *sp = xc_cpumap_bits(srcp); + int k, w = 0, lim = XC_BITS_TO_BYTES(xc_cpumap_len(srcp)); + for (k=0; k <lim; k++) + w += hweight8(sp[k]); + return w; +} + +/* xenctl_cpumap print function */ +#define CHUNKSZ 8 +#define roundup_power2(val,modulus) (((val) + (modulus) - 1) & ~((modulus) - 1)) + +int __xc_cpumap_snprintf(char *buf, unsigned int buflen, + const struct xenctl_cpumap *cpumap) +{ + const uint8_t *maskp = xc_cpumap_bits(cpumap); + int nmaskbits = xc_cpumap_len(cpumap); + int i, word, bit, len = 0; + unsigned long val; + const char *sep = ""; + int chunksz; + uint8_t chunkmask; + + chunksz = nmaskbits & (CHUNKSZ - 1); + if (chunksz == 0) + chunksz = CHUNKSZ; + + i = roundup_power2(nmaskbits, CHUNKSZ) - CHUNKSZ; + for (; i >= 0; i -= CHUNKSZ) { + chunkmask = ((1ULL << chunksz) - 1); + word = i / XC_BITS_PER_BYTE; + bit = i % XC_BITS_PER_BYTE; + val = (maskp[word] >> bit) & chunkmask; + len += snprintf(buf+len, buflen-len, "%s%0*lx", sep, + (chunksz+3)/4, val); + chunksz = CHUNKSZ; + sep = ","; + } + return len; +} diff --git a/tools/libxc/xc_cpumap.h b/tools/libxc/xc_cpumap.h new file mode 100644 --- /dev/null +++ b/tools/libxc/xc_cpumap.h @@ -0,0 +1,113 @@ +#ifndef __XENCTL_CPUMAP_H +#define __XENCTL_CPUMAP_H + +#include "xc_private.h" +#include <stdint.h> +#include <string.h> + +#define XC_BITS_PER_BYTE 8 +#define XC_BITS_TO_BYTES(bits) \ + (((bits)+XC_BITS_PER_BYTE-1)/XC_BITS_PER_BYTE) +#define XC_BITMAP_BIT(nr) (1 << (nr)) +#define XC_BITMAP_BIT_MASK(nr) (1 << ((nr) % XC_BITS_PER_BYTE)) +#define XC_BITMAP_BYTE(nr) ((nr) / XC_BITS_PER_BYTE) +#define XC_BITMAP_BYTE_OFFSET(nr) ((nr) % XC_BITS_PER_BYTE) +#define XC_BITMAP_BYTE_MASK (0xFF) +#define XC_BITMAP_LAST_BYTE_MASK(nbits) \ + (((nbits) % XC_BITS_PER_BYTE) ? \ + ((1<<((nbits) % XC_BITS_PER_BYTE))-1) : \ + XC_BITMAP_BYTE_MASK) + +#define xc_cpumap_bits(maskp) \ + ({ uint8_t *bitmap; \ + get_xen_guest_handle(bitmap, (maskp)->bitmap); \ + bitmap; }) +#define xc_cpumap_len(maskp) ((maskp)->nr_cpus) + +/* For iterating over the cpus set in the cpumap */ +#define xc_for_each_cpu(cpu, mask) \ + __xc_for_each_cpu(cpu, &(mask)) +#define __xc_for_each_cpu(cpu, mask) \ + for ((cpu) = -1; \ + (cpu) = xc_cpumap_next((cpu), (mask)), \ + (cpu) < xc_cpumap_len(mask);) +extern uint32_t xc_cpumap_next(int n, struct xenctl_cpumap *srcp); + +#define xc_cpumap_set_cpu(cpu, dst) __xc_cpumap_set_cpu(cpu, &(dst)) +static inline void __xc_cpumap_set_cpu(int cpu, struct xenctl_cpumap *dstp) +{ + uint8_t mask = XC_BITMAP_BIT_MASK(cpu); + uint8_t *p = ((uint8_t *)xc_cpumap_bits(dstp)) + XC_BITMAP_BYTE(cpu); + *p |= mask; +} + +#define xc_cpumap_clear_cpu(cpu, dst) __xc_cpumap_clear_cpu(cpu, &(dst)) +static inline void __xc_cpumap_clear_cpu(int cpu, struct xenctl_cpumap *dstp) +{ + uint8_t mask = XC_BITMAP_BIT_MASK(cpu); + uint8_t *p = ((uint8_t *)xc_cpumap_bits(dstp)) + XC_BITMAP_BYTE(cpu); + *p &= ~mask; +} + +#define xc_cpumap_test_cpu(cpu, dst) __xc_cpumap_test_cpu(cpu, &(dst)) +static inline int __xc_cpumap_test_cpu(int cpu, struct xenctl_cpumap *dstp) +{ + uint8_t mask = XC_BITMAP_BIT_MASK(cpu); + uint8_t *p = ((uint8_t *)xc_cpumap_bits(dstp)) + XC_BITMAP_BYTE(cpu); + return *p & mask; +} + +#define xc_cpumap_setall(dst) __xc_cpumap_setall(&(dst)) +static inline void __xc_cpumap_setall(struct xenctl_cpumap *dstp) +{ + uint8_t *dp = xc_cpumap_bits(dstp); + int nbits = xc_cpumap_len(dstp); + size_t nbytes = XC_BITS_TO_BYTES(nbits); + if (nbytes > 1) + memset(dp, 0xff, nbytes); + dp[nbytes-1] = XC_BITMAP_LAST_BYTE_MASK(nbits); +} + +#define xc_cpumap_clearall(dst) __xc_cpumap_clearall(&(dst)) +static inline void __xc_cpumap_clearall(struct xenctl_cpumap *dstp) +{ + size_t nbytes = XC_BITS_TO_BYTES(xc_cpumap_len(dstp)); + if (nbytes > 1) + memset(xc_cpumap_bits(dstp), 0x00, nbytes); +} + +#define xc_cpumap_or(dst, src1, src2) \ + __xc_cpumap_or(&(dst), &(src1), &(src2)) +extern void __xc_cpumap_or(struct xenctl_cpumap *dstp, + struct xenctl_cpumap *src1p, struct xenctl_cpumap *src2p); + +#define xc_cpumap_weight(src) __xc_cpumap_weight(&(src)) +extern int __xc_cpumap_weight(struct xenctl_cpumap *srcp); + +#define xc_cpumap_snprintf(buf, len, src) \ + __xc_cpumap_snprintf((buf), (len), &(src)) +extern int __xc_cpumap_snprintf(char *buf, unsigned int len, + const struct xenctl_cpumap *srcp); + +/***********************************************************************/ +static inline int +xc_cpumap_lock_pages(struct xenctl_cpumap *map) +{ + uint8_t *bitmap; + uint32_t nr_bytes = XC_BITS_TO_BYTES(map->nr_cpus); + get_xen_guest_handle(bitmap, map->bitmap); + if (lock_pages(bitmap, nr_bytes)) + return -1; + return 0; +} + +static inline void +xc_cpumap_unlock_pages(struct xenctl_cpumap *map) +{ + uint8_t *bitmap; + uint32_t nr_bytes = XC_BITS_TO_BYTES(map->nr_cpus); + get_xen_guest_handle(bitmap, map->bitmap); + unlock_pages(bitmap, nr_bytes); +} + +#endif /* __XENCTL_CPUMAP_H */ [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [vNUMA v2][PATCH 3/8] Basic cpumap utilities 2010-08-01 22:02 ` [vNUMA v2][PATCH 3/8] " Dulloor @ 2010-08-13 15:25 ` Andre Przywara 2010-08-14 4:38 ` Dulloor 0 siblings, 1 reply; 4+ messages in thread From: Andre Przywara @ 2010-08-13 15:25 UTC (permalink / raw) To: Dulloor; +Cc: xen-devel@lists.xensource.com Dulloor wrote: > Implement basic utility functions to manipulate bitmasks. Used in later patches. > > -dulloor > > Signed-off-by : Dulloor <dulloor@gmail.com> > In general this looks OK, although a bit too sophisticated for my personal taste. Only some minor comments: It seems that these functions are somewhat generic, so it may be worth to create a generic interface instead and somehow tie the connection to xenctl_cpumap later with the instantiation. The generic functions could be prefixed with xc_bitmap_*. QEMU is about to also get a generic bitmap library: http://lists.gnu.org/archive/html/qemu-devel/2010-08/msg00517.html maybe one could leverage this. > +++ b/tools/libxc/xc_cpumap.c > +void __xc_cpumap_or(struct xenctl_cpumap *dstp, > + struct xenctl_cpumap *src1p, struct xenctl_cpumap *src2p) > +{ > + uint8_t *dp = xc_cpumap_bits(dstp); > + uint8_t *s1p = xc_cpumap_bits(src1p); > + uint8_t *s2p = xc_cpumap_bits(src2p); > + int nr = XC_BITS_TO_BYTES(xc_cpumap_len(dstp)); > + int k; > + for (k=0; k<nr; k++) > + dp[k] = s1p[k] | s2p[k]; > +} Shouldn't we observe the special case with different source length here? If one bitmap contains garbage after it's end, then the result would be bogus. I think the bitmap or'ing should be stopped after both input bitmaps came to an end. I think xc_cpumap_setall() does it correct. > + > +static inline uint8_t hweight8(uint8_t w) > +{ > + uint8_t res = (w & 0x55) + ((w >> 1) & 0x55); > + res = (res & 0x33) + ((res >> 2) & 0x33); > + return (res & 0x0F) + ((res >> 4) & 0x0F); > +} > + > +int __xc_cpumap_weight(struct xenctl_cpumap *srcp) > +{ > + const uint8_t *sp = xc_cpumap_bits(srcp); > + int k, w = 0, lim = XC_BITS_TO_BYTES(xc_cpumap_len(srcp)); > + for (k=0; k <lim; k++) > + w += hweight8(sp[k]); > + return w; > +} We should stop counting after hitting the maximum specified length, otherwise possible garbage bits would be counted in. > + > +/* xenctl_cpumap print function */ > +#define CHUNKSZ 8 > +#define roundup_power2(val,modulus) (((val) + (modulus) - 1) & ~((modulus) - 1)) > + > +int __xc_cpumap_snprintf(char *buf, unsigned int buflen, > + const struct xenctl_cpumap *cpumap) > +{ > + const uint8_t *maskp = xc_cpumap_bits(cpumap); > + int nmaskbits = xc_cpumap_len(cpumap); > + int i, word, bit, len = 0; > + unsigned long val; > + const char *sep = ""; > + int chunksz; > + uint8_t chunkmask; > + > + chunksz = nmaskbits & (CHUNKSZ - 1); > + if (chunksz == 0) > + chunksz = CHUNKSZ; > + > + i = roundup_power2(nmaskbits, CHUNKSZ) - CHUNKSZ; > + for (; i >= 0; i -= CHUNKSZ) { > + chunkmask = ((1ULL << chunksz) - 1); > + word = i / XC_BITS_PER_BYTE; > + bit = i % XC_BITS_PER_BYTE; > + val = (maskp[word] >> bit) & chunkmask; > + len += snprintf(buf+len, buflen-len, "%s%0*lx", sep, > + (chunksz+3)/4, val); > + chunksz = CHUNKSZ; Isn't that line redundant? > + sep = ","; > + } > + return len; > +} Regards, Andre. -- Andre Przywara AMD-Operating System Research Center (OSRC), Dresden, Germany Tel: +49 351 448-3567-12 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [vNUMA v2][PATCH 3/8] Basic cpumap utilities 2010-08-13 15:25 ` Andre Przywara @ 2010-08-14 4:38 ` Dulloor 0 siblings, 0 replies; 4+ messages in thread From: Dulloor @ 2010-08-14 4:38 UTC (permalink / raw) To: Andre Przywara; +Cc: xen-devel@lists.xensource.com Hi Andre, Thanks for the reviews. I am going to be very busy this week, so I will take this up immediately after that. Also, as you mentioned elsewhere, we can have this code checked-in in an acceptable state and you could push your rebased changes on that. thanks dulloor On Fri, Aug 13, 2010 at 8:25 AM, Andre Przywara <andre.przywara@amd.com> wrote: > Dulloor wrote: >> >> Implement basic utility functions to manipulate bitmasks. Used in later >> patches. >> >> -dulloor >> >> Signed-off-by : Dulloor <dulloor@gmail.com> >> > In general this looks OK, although a bit too sophisticated for my > personal taste. Only some minor comments: > > It seems that these functions are somewhat generic, so it may be worth > to create a generic interface instead and somehow tie the connection > to xenctl_cpumap later with the instantiation. The generic functions > could be prefixed with xc_bitmap_*. > QEMU is about to also get a generic bitmap library: > http://lists.gnu.org/archive/html/qemu-devel/2010-08/msg00517.html > maybe one could leverage this. > >> +++ b/tools/libxc/xc_cpumap.c >> +void __xc_cpumap_or(struct xenctl_cpumap *dstp, >> + struct xenctl_cpumap *src1p, struct xenctl_cpumap *src2p) >> +{ >> + uint8_t *dp = xc_cpumap_bits(dstp); >> + uint8_t *s1p = xc_cpumap_bits(src1p); >> + uint8_t *s2p = xc_cpumap_bits(src2p); >> + int nr = XC_BITS_TO_BYTES(xc_cpumap_len(dstp)); >> + int k; >> + for (k=0; k<nr; k++) >> + dp[k] = s1p[k] | s2p[k]; >> +} > > Shouldn't we observe the special case with different source length here? If > one bitmap contains garbage after it's end, then the result would be bogus. > I think the bitmap or'ing should be stopped after both input bitmaps came to > an end. I think xc_cpumap_setall() does it correct. > >> + >> +static inline uint8_t hweight8(uint8_t w) >> +{ >> + uint8_t res = (w & 0x55) + ((w >> 1) & 0x55); >> + res = (res & 0x33) + ((res >> 2) & 0x33); >> + return (res & 0x0F) + ((res >> 4) & 0x0F); >> +} >> + >> +int __xc_cpumap_weight(struct xenctl_cpumap *srcp) >> +{ >> + const uint8_t *sp = xc_cpumap_bits(srcp); >> + int k, w = 0, lim = XC_BITS_TO_BYTES(xc_cpumap_len(srcp)); >> + for (k=0; k <lim; k++) >> + w += hweight8(sp[k]); >> + return w; >> +} > > We should stop counting after hitting the maximum specified length, > otherwise possible garbage bits would be counted in. > >> + >> +/* xenctl_cpumap print function */ >> +#define CHUNKSZ 8 >> +#define roundup_power2(val,modulus) (((val) + (modulus) - 1) & >> ~((modulus) - 1)) >> + >> +int __xc_cpumap_snprintf(char *buf, unsigned int buflen, >> + const struct xenctl_cpumap >> *cpumap) >> +{ >> + const uint8_t *maskp = xc_cpumap_bits(cpumap); >> + int nmaskbits = xc_cpumap_len(cpumap); >> + int i, word, bit, len = 0; >> + unsigned long val; >> + const char *sep = ""; >> + int chunksz; >> + uint8_t chunkmask; >> + >> + chunksz = nmaskbits & (CHUNKSZ - 1); >> + if (chunksz == 0) >> + chunksz = CHUNKSZ; >> + >> + i = roundup_power2(nmaskbits, CHUNKSZ) - CHUNKSZ; >> + for (; i >= 0; i -= CHUNKSZ) { >> + chunkmask = ((1ULL << chunksz) - 1); >> + word = i / XC_BITS_PER_BYTE; >> + bit = i % XC_BITS_PER_BYTE; >> + val = (maskp[word] >> bit) & chunkmask; >> + len += snprintf(buf+len, buflen-len, "%s%0*lx", sep, >> + (chunksz+3)/4, val); >> + chunksz = CHUNKSZ; > > Isn't that line redundant? >> >> + sep = ","; >> + } >> + return len; >> +} > > Regards, > Andre. > > -- > Andre Przywara > AMD-Operating System Research Center (OSRC), Dresden, Germany > Tel: +49 351 448-3567-12 > > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-08-14 4:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1BEA8649F0C00540AB2811D7922ECB6C9338B4CD@orsmsx507.amr.corp.intel.com>
2010-07-02 23:54 ` [XEN][vNUMA][PATCH 4/9] Basic cpumap utilities Dulloor
2010-08-01 22:02 ` [vNUMA v2][PATCH 3/8] " Dulloor
2010-08-13 15:25 ` Andre Przywara
2010-08-14 4:38 ` Dulloor
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).