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