* [Qemu-devel] [PATCH] libqos: Convert malloc-pc allocator to a generic allocator
@ 2014-10-18 9:24 Marc Marí
2014-10-22 16:01 ` Stefan Hajnoczi
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Marc Marí @ 2014-10-18 9:24 UTC (permalink / raw)
To: qemu-devel
Cc: Marc Marí, John Snow, Andreas Färber, Stefan Hajnoczi
The allocator in malloc-pc has been extracted, so it can be used in every arch.
This operation showed that both the alloc and free functions can be also
generic.
Because of this, the QGuestAllocator has been removed from is function to wrap
the alloc and free function, and now just contains the allocator parameters.
As a result, only the allocator initalizer and unitializer are arch dependent.
Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
tests/Makefile | 2 +-
tests/libqos/malloc-pc.c | 280 +---------------------------------------------
tests/libqos/malloc-pc.h | 11 +-
tests/libqos/malloc.c | 268 ++++++++++++++++++++++++++++++++++++++++++++
tests/libqos/malloc.h | 45 +++++---
5 files changed, 307 insertions(+), 299 deletions(-)
create mode 100644 tests/libqos/malloc.c
diff --git a/tests/Makefile b/tests/Makefile
index 834279c..15bc670 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -296,7 +296,7 @@ tests/test-opts-visitor$(EXESUF): tests/test-opts-visitor.o $(test-qapi-obj-y) l
tests/test-mul64$(EXESUF): tests/test-mul64.o libqemuutil.a
tests/test-bitops$(EXESUF): tests/test-bitops.o libqemuutil.a
-libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o
+libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o
libqos-obj-y += tests/libqos/i2c.o
libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
libqos-pc-obj-y += tests/libqos/malloc-pc.o
diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
index f4218c6..c9c48fd 100644
--- a/tests/libqos/malloc-pc.c
+++ b/tests/libqos/malloc-pc.c
@@ -17,296 +17,28 @@
#include "hw/nvram/fw_cfg.h"
#include "qemu-common.h"
-#include "qemu/queue.h"
#include <glib.h>
#define PAGE_SIZE (4096)
-#define MLIST_ENTNAME entries
-typedef QTAILQ_HEAD(MemList, MemBlock) MemList;
-typedef struct MemBlock {
- QTAILQ_ENTRY(MemBlock) MLIST_ENTNAME;
- uint64_t size;
- uint64_t addr;
-} MemBlock;
-
-typedef struct PCAlloc
-{
- QGuestAllocator alloc;
- PCAllocOpts opts;
- uint64_t start;
- uint64_t end;
-
- MemList used;
- MemList free;
-} PCAlloc;
-
-static MemBlock *mlist_new(uint64_t addr, uint64_t size)
-{
- MemBlock *block;
-
- if (!size) {
- return NULL;
- }
- block = g_malloc0(sizeof(MemBlock));
-
- block->addr = addr;
- block->size = size;
-
- return block;
-}
-
-static void mlist_delete(MemList *list, MemBlock *node)
-{
- g_assert(list && node);
- QTAILQ_REMOVE(list, node, MLIST_ENTNAME);
- g_free(node);
-}
-
-static MemBlock *mlist_find_key(MemList *head, uint64_t addr)
-{
- MemBlock *node;
- QTAILQ_FOREACH(node, head, MLIST_ENTNAME) {
- if (node->addr == addr) {
- return node;
- }
- }
- return NULL;
-}
-
-static MemBlock *mlist_find_space(MemList *head, uint64_t size)
-{
- MemBlock *node;
-
- QTAILQ_FOREACH(node, head, MLIST_ENTNAME) {
- if (node->size >= size) {
- return node;
- }
- }
- return NULL;
-}
-
-static MemBlock *mlist_sort_insert(MemList *head, MemBlock *insr)
-{
- MemBlock *node;
- g_assert(head && insr);
-
- QTAILQ_FOREACH(node, head, MLIST_ENTNAME) {
- if (insr->addr < node->addr) {
- QTAILQ_INSERT_BEFORE(node, insr, MLIST_ENTNAME);
- return insr;
- }
- }
-
- QTAILQ_INSERT_TAIL(head, insr, MLIST_ENTNAME);
- return insr;
-}
-
-static inline uint64_t mlist_boundary(MemBlock *node)
-{
- return node->size + node->addr;
-}
-
-static MemBlock *mlist_join(MemList *head, MemBlock *left, MemBlock *right)
-{
- g_assert(head && left && right);
-
- left->size += right->size;
- mlist_delete(head, right);
- return left;
-}
-
-static void mlist_coalesce(MemList *head, MemBlock *node)
-{
- g_assert(node);
- MemBlock *left;
- MemBlock *right;
- char merge;
-
- do {
- merge = 0;
- left = QTAILQ_PREV(node, MemList, MLIST_ENTNAME);
- right = QTAILQ_NEXT(node, MLIST_ENTNAME);
-
- /* clowns to the left of me */
- if (left && mlist_boundary(left) == node->addr) {
- node = mlist_join(head, left, node);
- merge = 1;
- }
-
- /* jokers to the right */
- if (right && mlist_boundary(node) == right->addr) {
- node = mlist_join(head, node, right);
- merge = 1;
- }
-
- } while (merge);
-}
-
-static uint64_t pc_mlist_fulfill(PCAlloc *s, MemBlock *freenode, uint64_t size)
-{
- uint64_t addr;
- MemBlock *usednode;
-
- g_assert(freenode);
- g_assert_cmpint(freenode->size, >=, size);
-
- addr = freenode->addr;
- if (freenode->size == size) {
- /* re-use this freenode as our used node */
- QTAILQ_REMOVE(&s->free, freenode, MLIST_ENTNAME);
- usednode = freenode;
- } else {
- /* adjust the free node and create a new used node */
- freenode->addr += size;
- freenode->size -= size;
- usednode = mlist_new(addr, size);
- }
-
- mlist_sort_insert(&s->used, usednode);
- return addr;
-}
-
-/* To assert the correctness of the list.
- * Used only if PC_ALLOC_PARANOID is set. */
-static void pc_mlist_check(PCAlloc *s)
-{
- MemBlock *node;
- uint64_t addr = s->start > 0 ? s->start - 1 : 0;
- uint64_t next = s->start;
-
- QTAILQ_FOREACH(node, &s->free, MLIST_ENTNAME) {
- g_assert_cmpint(node->addr, >, addr);
- g_assert_cmpint(node->addr, >=, next);
- addr = node->addr;
- next = node->addr + node->size;
- }
-
- addr = s->start > 0 ? s->start - 1 : 0;
- next = s->start;
- QTAILQ_FOREACH(node, &s->used, MLIST_ENTNAME) {
- g_assert_cmpint(node->addr, >, addr);
- g_assert_cmpint(node->addr, >=, next);
- addr = node->addr;
- next = node->addr + node->size;
- }
-}
-
-static uint64_t pc_mlist_alloc(PCAlloc *s, uint64_t size)
-{
- MemBlock *node;
-
- node = mlist_find_space(&s->free, size);
- if (!node) {
- fprintf(stderr, "Out of guest memory.\n");
- g_assert_not_reached();
- }
- return pc_mlist_fulfill(s, node, size);
-}
-
-static void pc_mlist_free(PCAlloc *s, uint64_t addr)
-{
- MemBlock *node;
-
- if (addr == 0) {
- return;
- }
-
- node = mlist_find_key(&s->used, addr);
- if (!node) {
- fprintf(stderr, "Error: no record found for an allocation at "
- "0x%016" PRIx64 ".\n",
- addr);
- g_assert_not_reached();
- }
-
- /* Rip it out of the used list and re-insert back into the free list. */
- QTAILQ_REMOVE(&s->used, node, MLIST_ENTNAME);
- mlist_sort_insert(&s->free, node);
- mlist_coalesce(&s->free, node);
-}
-
-static uint64_t pc_alloc(QGuestAllocator *allocator, size_t size)
-{
- PCAlloc *s = container_of(allocator, PCAlloc, alloc);
- uint64_t rsize = size;
- uint64_t naddr;
-
- rsize += (PAGE_SIZE - 1);
- rsize &= -PAGE_SIZE;
- g_assert_cmpint((s->start + rsize), <=, s->end);
- g_assert_cmpint(rsize, >=, size);
-
- naddr = pc_mlist_alloc(s, rsize);
- if (s->opts & PC_ALLOC_PARANOID) {
- pc_mlist_check(s);
- }
-
- return naddr;
-}
-
-static void pc_free(QGuestAllocator *allocator, uint64_t addr)
-{
- PCAlloc *s = container_of(allocator, PCAlloc, alloc);
-
- pc_mlist_free(s, addr);
- if (s->opts & PC_ALLOC_PARANOID) {
- pc_mlist_check(s);
- }
-}
-
/*
* Mostly for valgrind happiness, but it does offer
* a chokepoint for debugging guest memory leaks, too.
*/
void pc_alloc_uninit(QGuestAllocator *allocator)
{
- PCAlloc *s = container_of(allocator, PCAlloc, alloc);
- MemBlock *node;
- MemBlock *tmp;
- PCAllocOpts mask;
-
- /* Check for guest leaks, and destroy the list. */
- QTAILQ_FOREACH_SAFE(node, &s->used, MLIST_ENTNAME, tmp) {
- if (s->opts & (PC_ALLOC_LEAK_WARN | PC_ALLOC_LEAK_ASSERT)) {
- fprintf(stderr, "guest malloc leak @ 0x%016" PRIx64 "; "
- "size 0x%016" PRIx64 ".\n",
- node->addr, node->size);
- }
- if (s->opts & (PC_ALLOC_LEAK_ASSERT)) {
- g_assert_not_reached();
- }
- g_free(node);
- }
-
- /* If we have previously asserted that there are no leaks, then there
- * should be only one node here with a specific address and size. */
- mask = PC_ALLOC_LEAK_ASSERT | PC_ALLOC_PARANOID;
- QTAILQ_FOREACH_SAFE(node, &s->free, MLIST_ENTNAME, tmp) {
- if ((s->opts & mask) == mask) {
- if ((node->addr != s->start) ||
- (node->size != s->end - s->start)) {
- fprintf(stderr, "Free list is corrupted.\n");
- g_assert_not_reached();
- }
- }
-
- g_free(node);
- }
-
- g_free(s);
+ alloc_uninit(allocator);
}
-QGuestAllocator *pc_alloc_init_flags(PCAllocOpts flags)
+QGuestAllocator *pc_alloc_init_flags(QAllocOpts flags)
{
- PCAlloc *s = g_malloc0(sizeof(*s));
+ QGuestAllocator *s = g_malloc0(sizeof(*s));
uint64_t ram_size;
QFWCFG *fw_cfg = pc_fw_cfg_init();
MemBlock *node;
s->opts = flags;
- s->alloc.alloc = pc_alloc;
- s->alloc.free = pc_free;
+ s->page_size = PAGE_SIZE;
ram_size = qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE);
@@ -325,10 +57,10 @@ QGuestAllocator *pc_alloc_init_flags(PCAllocOpts flags)
node = mlist_new(s->start, s->end - s->start);
QTAILQ_INSERT_HEAD(&s->free, node, MLIST_ENTNAME);
- return &s->alloc;
+ return s;
}
inline QGuestAllocator *pc_alloc_init(void)
{
- return pc_alloc_init_flags(PC_ALLOC_NO_FLAGS);
+ return pc_alloc_init_flags(ALLOC_NO_FLAGS);
}
diff --git a/tests/libqos/malloc-pc.h b/tests/libqos/malloc-pc.h
index 9f525e3..86ab9f0 100644
--- a/tests/libqos/malloc-pc.h
+++ b/tests/libqos/malloc-pc.h
@@ -15,15 +15,8 @@
#include "libqos/malloc.h"
-typedef enum {
- PC_ALLOC_NO_FLAGS = 0x00,
- PC_ALLOC_LEAK_WARN = 0x01,
- PC_ALLOC_LEAK_ASSERT = 0x02,
- PC_ALLOC_PARANOID = 0x04
-} PCAllocOpts;
-
QGuestAllocator *pc_alloc_init(void);
-QGuestAllocator *pc_alloc_init_flags(PCAllocOpts flags);
-void pc_alloc_uninit(QGuestAllocator *allocator);
+QGuestAllocator *pc_alloc_init_flags(QAllocOpts flags);
+void pc_alloc_uninit(QGuestAllocator *allocator);
#endif
diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c
new file mode 100644
index 0000000..e145fd9
--- /dev/null
+++ b/tests/libqos/malloc.c
@@ -0,0 +1,268 @@
+/*
+ * libqos malloc support
+ *
+ * Copyright (c) 2014
+ *
+ * Author:
+ * John Snow <jsnow@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "libqos/malloc.h"
+#include "qemu-common.h"
+#include <stdio.h>
+#include <inttypes.h>
+#include <glib.h>
+
+static void mlist_delete(MemList *list, MemBlock *node)
+{
+ g_assert(list && node);
+ QTAILQ_REMOVE(list, node, MLIST_ENTNAME);
+ g_free(node);
+}
+
+static MemBlock *mlist_find_key(MemList *head, uint64_t addr)
+{
+ MemBlock *node;
+ QTAILQ_FOREACH(node, head, MLIST_ENTNAME) {
+ if (node->addr == addr) {
+ return node;
+ }
+ }
+ return NULL;
+}
+
+static MemBlock *mlist_find_space(MemList *head, uint64_t size)
+{
+ MemBlock *node;
+
+ QTAILQ_FOREACH(node, head, MLIST_ENTNAME) {
+ if (node->size >= size) {
+ return node;
+ }
+ }
+ return NULL;
+}
+
+static MemBlock *mlist_sort_insert(MemList *head, MemBlock *insr)
+{
+ MemBlock *node;
+ g_assert(head && insr);
+
+ QTAILQ_FOREACH(node, head, MLIST_ENTNAME) {
+ if (insr->addr < node->addr) {
+ QTAILQ_INSERT_BEFORE(node, insr, MLIST_ENTNAME);
+ return insr;
+ }
+ }
+
+ QTAILQ_INSERT_TAIL(head, insr, MLIST_ENTNAME);
+ return insr;
+}
+
+static inline uint64_t mlist_boundary(MemBlock *node)
+{
+ return node->size + node->addr;
+}
+
+static MemBlock *mlist_join(MemList *head, MemBlock *left, MemBlock *right)
+{
+ g_assert(head && left && right);
+
+ left->size += right->size;
+ mlist_delete(head, right);
+ return left;
+}
+
+static void mlist_coalesce(MemList *head, MemBlock *node)
+{
+ g_assert(node);
+ MemBlock *left;
+ MemBlock *right;
+ char merge;
+
+ do {
+ merge = 0;
+ left = QTAILQ_PREV(node, MemList, MLIST_ENTNAME);
+ right = QTAILQ_NEXT(node, MLIST_ENTNAME);
+
+ /* clowns to the left of me */
+ if (left && mlist_boundary(left) == node->addr) {
+ node = mlist_join(head, left, node);
+ merge = 1;
+ }
+
+ /* jokers to the right */
+ if (right && mlist_boundary(node) == right->addr) {
+ node = mlist_join(head, node, right);
+ merge = 1;
+ }
+
+ } while (merge);
+}
+
+static uint64_t mlist_fulfill(QGuestAllocator *s, MemBlock *freenode,
+ uint64_t size)
+{
+ uint64_t addr;
+ MemBlock *usednode;
+
+ g_assert(freenode);
+ g_assert_cmpint(freenode->size, >=, size);
+
+ addr = freenode->addr;
+ if (freenode->size == size) {
+ /* re-use this freenode as our used node */
+ QTAILQ_REMOVE(&s->free, freenode, MLIST_ENTNAME);
+ usednode = freenode;
+ } else {
+ /* adjust the free node and create a new used node */
+ freenode->addr += size;
+ freenode->size -= size;
+ usednode = mlist_new(addr, size);
+ }
+
+ mlist_sort_insert(&s->used, usednode);
+ return addr;
+}
+
+/* To assert the correctness of the list.
+ * Used only if ALLOC_PARANOID is set. */
+static void mlist_check(QGuestAllocator *s)
+{
+ MemBlock *node;
+ uint64_t addr = s->start > 0 ? s->start - 1 : 0;
+ uint64_t next = s->start;
+
+ QTAILQ_FOREACH(node, &s->free, MLIST_ENTNAME) {
+ g_assert_cmpint(node->addr, >, addr);
+ g_assert_cmpint(node->addr, >=, next);
+ addr = node->addr;
+ next = node->addr + node->size;
+ }
+
+ addr = s->start > 0 ? s->start - 1 : 0;
+ next = s->start;
+ QTAILQ_FOREACH(node, &s->used, MLIST_ENTNAME) {
+ g_assert_cmpint(node->addr, >, addr);
+ g_assert_cmpint(node->addr, >=, next);
+ addr = node->addr;
+ next = node->addr + node->size;
+ }
+}
+
+static uint64_t mlist_alloc(QGuestAllocator *s, uint64_t size)
+{
+ MemBlock *node;
+
+ node = mlist_find_space(&s->free, size);
+ if (!node) {
+ fprintf(stderr, "Out of guest memory.\n");
+ g_assert_not_reached();
+ }
+ return mlist_fulfill(s, node, size);
+}
+
+static void mlist_free(QGuestAllocator *s, uint64_t addr)
+{
+ MemBlock *node;
+
+ if (addr == 0) {
+ return;
+ }
+
+ node = mlist_find_key(&s->used, addr);
+ if (!node) {
+ fprintf(stderr, "Error: no record found for an allocation at "
+ "0x%016" PRIx64 ".\n",
+ addr);
+ g_assert_not_reached();
+ }
+
+ /* Rip it out of the used list and re-insert back into the free list. */
+ QTAILQ_REMOVE(&s->used, node, MLIST_ENTNAME);
+ mlist_sort_insert(&s->free, node);
+ mlist_coalesce(&s->free, node);
+}
+
+MemBlock *mlist_new(uint64_t addr, uint64_t size)
+{
+ MemBlock *block;
+
+ if (!size) {
+ return NULL;
+ }
+ block = g_malloc0(sizeof(MemBlock));
+
+ block->addr = addr;
+ block->size = size;
+
+ return block;
+}
+
+/*
+ * Mostly for valgrind happiness, but it does offer
+ * a chokepoint for debugging guest memory leaks, too.
+ */
+void alloc_uninit(QGuestAllocator *allocator)
+{
+ MemBlock *node;
+ MemBlock *tmp;
+ QAllocOpts mask;
+
+ /* Check for guest leaks, and destroy the list. */
+ QTAILQ_FOREACH_SAFE(node, &allocator->used, MLIST_ENTNAME, tmp) {
+ if (allocator->opts & (ALLOC_LEAK_WARN | ALLOC_LEAK_ASSERT)) {
+ fprintf(stderr, "guest malloc leak @ 0x%016" PRIx64 "; "
+ "size 0x%016" PRIx64 ".\n",
+ node->addr, node->size);
+ }
+ if (allocator->opts & (ALLOC_LEAK_ASSERT)) {
+ g_assert_not_reached();
+ }
+ g_free(node);
+ }
+
+ /* If we have previously asserted that there are no leaks, then there
+ * should be only one node here with a specific address and size. */
+ mask = ALLOC_LEAK_ASSERT | ALLOC_PARANOID;
+ QTAILQ_FOREACH_SAFE(node, &allocator->free, MLIST_ENTNAME, tmp) {
+ if ((allocator->opts & mask) == mask) {
+ if ((node->addr != allocator->start) ||
+ (node->size != allocator->end - allocator->start)) {
+ fprintf(stderr, "Free list is corrupted.\n");
+ g_assert_not_reached();
+ }
+ }
+
+ g_free(node);
+ }
+}
+
+uint64_t guest_alloc(QGuestAllocator *allocator, size_t size)
+{
+ uint64_t rsize = size;
+ uint64_t naddr;
+
+ rsize += (allocator->page_size - 1);
+ rsize &= -allocator->page_size;
+ g_assert_cmpint((allocator->start + rsize), <=, allocator->end);
+ g_assert_cmpint(rsize, >=, size);
+
+ naddr = mlist_alloc(allocator, rsize);
+ if (allocator->opts & ALLOC_PARANOID) {
+ mlist_check(allocator);
+ }
+
+ return naddr;
+}
+
+void guest_free(QGuestAllocator *allocator, uint64_t addr)
+{
+ mlist_free(allocator, addr);
+ if (allocator->opts & ALLOC_PARANOID) {
+ mlist_check(allocator);
+ }
+}
diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
index 5565381..465efeb 100644
--- a/tests/libqos/malloc.h
+++ b/tests/libqos/malloc.h
@@ -15,24 +15,39 @@
#include <stdint.h>
#include <sys/types.h>
+#include "qemu/queue.h"
-typedef struct QGuestAllocator QGuestAllocator;
+#define MLIST_ENTNAME entries
-struct QGuestAllocator
-{
- uint64_t (*alloc)(QGuestAllocator *allocator, size_t size);
- void (*free)(QGuestAllocator *allocator, uint64_t addr);
-};
+typedef enum {
+ ALLOC_NO_FLAGS = 0x00,
+ ALLOC_LEAK_WARN = 0x01,
+ ALLOC_LEAK_ASSERT = 0x02,
+ ALLOC_PARANOID = 0x04
+} QAllocOpts;
+
+typedef QTAILQ_HEAD(MemList, MemBlock) MemList;
+typedef struct MemBlock {
+ QTAILQ_ENTRY(MemBlock) MLIST_ENTNAME;
+ uint64_t size;
+ uint64_t addr;
+} MemBlock;
+
+typedef struct QGuestAllocator {
+ QAllocOpts opts;
+ uint64_t start;
+ uint64_t end;
+ uint32_t page_size;
+
+ MemList used;
+ MemList free;
+} QGuestAllocator;
+
+MemBlock *mlist_new(uint64_t addr, uint64_t size);
+void alloc_uninit(QGuestAllocator *allocator);
/* Always returns page aligned values */
-static inline uint64_t guest_alloc(QGuestAllocator *allocator, size_t size)
-{
- return allocator->alloc(allocator, size);
-}
-
-static inline void guest_free(QGuestAllocator *allocator, uint64_t addr)
-{
- allocator->free(allocator, addr);
-}
+uint64_t guest_alloc(QGuestAllocator *allocator, size_t size);
+void guest_free(QGuestAllocator *allocator, uint64_t addr);
#endif
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] libqos: Convert malloc-pc allocator to a generic allocator
2014-10-18 9:24 [Qemu-devel] [PATCH] libqos: Convert malloc-pc allocator to a generic allocator Marc Marí
@ 2014-10-22 16:01 ` Stefan Hajnoczi
2014-10-22 16:33 ` John Snow
2014-10-23 22:49 ` John Snow
2 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2014-10-22 16:01 UTC (permalink / raw)
To: Marc Marí
Cc: John Snow, qemu-devel, Stefan Hajnoczi, Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 987 bytes --]
On Sat, Oct 18, 2014 at 11:24:17AM +0200, Marc Marí wrote:
> The allocator in malloc-pc has been extracted, so it can be used in every arch.
> This operation showed that both the alloc and free functions can be also
> generic.
> Because of this, the QGuestAllocator has been removed from is function to wrap
> the alloc and free function, and now just contains the allocator parameters.
> As a result, only the allocator initalizer and unitializer are arch dependent.
>
> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
> tests/Makefile | 2 +-
> tests/libqos/malloc-pc.c | 280 +---------------------------------------------
> tests/libqos/malloc-pc.h | 11 +-
> tests/libqos/malloc.c | 268 ++++++++++++++++++++++++++++++++++++++++++++
> tests/libqos/malloc.h | 45 +++++---
> 5 files changed, 307 insertions(+), 299 deletions(-)
> create mode 100644 tests/libqos/malloc.c
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] libqos: Convert malloc-pc allocator to a generic allocator
2014-10-18 9:24 [Qemu-devel] [PATCH] libqos: Convert malloc-pc allocator to a generic allocator Marc Marí
2014-10-22 16:01 ` Stefan Hajnoczi
@ 2014-10-22 16:33 ` John Snow
2014-10-22 21:09 ` Marc Marí
2014-10-23 22:49 ` John Snow
2 siblings, 1 reply; 7+ messages in thread
From: John Snow @ 2014-10-22 16:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc Marí
On 10/18/2014 05:24 AM, Marc Marí wrote:
> The allocator in malloc-pc has been extracted, so it can be used in every arch.
> This operation showed that both the alloc and free functions can be also
> generic.
> Because of this, the QGuestAllocator has been removed from is function to wrap
> the alloc and free function, and now just contains the allocator parameters.
> As a result, only the allocator initalizer and unitializer are arch dependent.
>
> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
> tests/Makefile | 2 +-
> tests/libqos/malloc-pc.c | 280 +---------------------------------------------
> tests/libqos/malloc-pc.h | 11 +-
> tests/libqos/malloc.c | 268 ++++++++++++++++++++++++++++++++++++++++++++
> tests/libqos/malloc.h | 45 +++++---
> 5 files changed, 307 insertions(+), 299 deletions(-)
> create mode 100644 tests/libqos/malloc.c
>
> diff --git a/tests/Makefile b/tests/Makefile
> index 834279c..15bc670 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -296,7 +296,7 @@ tests/test-opts-visitor$(EXESUF): tests/test-opts-visitor.o $(test-qapi-obj-y) l
> tests/test-mul64$(EXESUF): tests/test-mul64.o libqemuutil.a
> tests/test-bitops$(EXESUF): tests/test-bitops.o libqemuutil.a
>
> -libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o
> +libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o
> libqos-obj-y += tests/libqos/i2c.o
> libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
> libqos-pc-obj-y += tests/libqos/malloc-pc.o
> diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
> index f4218c6..c9c48fd 100644
> --- a/tests/libqos/malloc-pc.c
> +++ b/tests/libqos/malloc-pc.c
> @@ -17,296 +17,28 @@
> #include "hw/nvram/fw_cfg.h"
>
> #include "qemu-common.h"
> -#include "qemu/queue.h"
> #include <glib.h>
>
> #define PAGE_SIZE (4096)
>
> -#define MLIST_ENTNAME entries
> -typedef QTAILQ_HEAD(MemList, MemBlock) MemList;
> -typedef struct MemBlock {
> - QTAILQ_ENTRY(MemBlock) MLIST_ENTNAME;
> - uint64_t size;
> - uint64_t addr;
> -} MemBlock;
> -
> -typedef struct PCAlloc
> -{
> - QGuestAllocator alloc;
> - PCAllocOpts opts;
> - uint64_t start;
> - uint64_t end;
> -
> - MemList used;
> - MemList free;
> -} PCAlloc;
> -
> -static MemBlock *mlist_new(uint64_t addr, uint64_t size)
> -{
> - MemBlock *block;
> -
> - if (!size) {
> - return NULL;
> - }
> - block = g_malloc0(sizeof(MemBlock));
> -
> - block->addr = addr;
> - block->size = size;
> -
> - return block;
> -}
> -
> -static void mlist_delete(MemList *list, MemBlock *node)
> -{
> - g_assert(list && node);
> - QTAILQ_REMOVE(list, node, MLIST_ENTNAME);
> - g_free(node);
> -}
> -
> -static MemBlock *mlist_find_key(MemList *head, uint64_t addr)
> -{
> - MemBlock *node;
> - QTAILQ_FOREACH(node, head, MLIST_ENTNAME) {
> - if (node->addr == addr) {
> - return node;
> - }
> - }
> - return NULL;
> -}
> -
> -static MemBlock *mlist_find_space(MemList *head, uint64_t size)
> -{
> - MemBlock *node;
> -
> - QTAILQ_FOREACH(node, head, MLIST_ENTNAME) {
> - if (node->size >= size) {
> - return node;
> - }
> - }
> - return NULL;
> -}
> -
> -static MemBlock *mlist_sort_insert(MemList *head, MemBlock *insr)
> -{
> - MemBlock *node;
> - g_assert(head && insr);
> -
> - QTAILQ_FOREACH(node, head, MLIST_ENTNAME) {
> - if (insr->addr < node->addr) {
> - QTAILQ_INSERT_BEFORE(node, insr, MLIST_ENTNAME);
> - return insr;
> - }
> - }
> -
> - QTAILQ_INSERT_TAIL(head, insr, MLIST_ENTNAME);
> - return insr;
> -}
> -
> -static inline uint64_t mlist_boundary(MemBlock *node)
> -{
> - return node->size + node->addr;
> -}
> -
> -static MemBlock *mlist_join(MemList *head, MemBlock *left, MemBlock *right)
> -{
> - g_assert(head && left && right);
> -
> - left->size += right->size;
> - mlist_delete(head, right);
> - return left;
> -}
> -
> -static void mlist_coalesce(MemList *head, MemBlock *node)
> -{
> - g_assert(node);
> - MemBlock *left;
> - MemBlock *right;
> - char merge;
> -
> - do {
> - merge = 0;
> - left = QTAILQ_PREV(node, MemList, MLIST_ENTNAME);
> - right = QTAILQ_NEXT(node, MLIST_ENTNAME);
> -
> - /* clowns to the left of me */
> - if (left && mlist_boundary(left) == node->addr) {
> - node = mlist_join(head, left, node);
> - merge = 1;
> - }
> -
> - /* jokers to the right */
> - if (right && mlist_boundary(node) == right->addr) {
> - node = mlist_join(head, node, right);
> - merge = 1;
> - }
> -
> - } while (merge);
> -}
> -
> -static uint64_t pc_mlist_fulfill(PCAlloc *s, MemBlock *freenode, uint64_t size)
> -{
> - uint64_t addr;
> - MemBlock *usednode;
> -
> - g_assert(freenode);
> - g_assert_cmpint(freenode->size, >=, size);
> -
> - addr = freenode->addr;
> - if (freenode->size == size) {
> - /* re-use this freenode as our used node */
> - QTAILQ_REMOVE(&s->free, freenode, MLIST_ENTNAME);
> - usednode = freenode;
> - } else {
> - /* adjust the free node and create a new used node */
> - freenode->addr += size;
> - freenode->size -= size;
> - usednode = mlist_new(addr, size);
> - }
> -
> - mlist_sort_insert(&s->used, usednode);
> - return addr;
> -}
> -
> -/* To assert the correctness of the list.
> - * Used only if PC_ALLOC_PARANOID is set. */
> -static void pc_mlist_check(PCAlloc *s)
> -{
> - MemBlock *node;
> - uint64_t addr = s->start > 0 ? s->start - 1 : 0;
> - uint64_t next = s->start;
> -
> - QTAILQ_FOREACH(node, &s->free, MLIST_ENTNAME) {
> - g_assert_cmpint(node->addr, >, addr);
> - g_assert_cmpint(node->addr, >=, next);
> - addr = node->addr;
> - next = node->addr + node->size;
> - }
> -
> - addr = s->start > 0 ? s->start - 1 : 0;
> - next = s->start;
> - QTAILQ_FOREACH(node, &s->used, MLIST_ENTNAME) {
> - g_assert_cmpint(node->addr, >, addr);
> - g_assert_cmpint(node->addr, >=, next);
> - addr = node->addr;
> - next = node->addr + node->size;
> - }
> -}
> -
> -static uint64_t pc_mlist_alloc(PCAlloc *s, uint64_t size)
> -{
> - MemBlock *node;
> -
> - node = mlist_find_space(&s->free, size);
> - if (!node) {
> - fprintf(stderr, "Out of guest memory.\n");
> - g_assert_not_reached();
> - }
> - return pc_mlist_fulfill(s, node, size);
> -}
> -
> -static void pc_mlist_free(PCAlloc *s, uint64_t addr)
> -{
> - MemBlock *node;
> -
> - if (addr == 0) {
> - return;
> - }
> -
> - node = mlist_find_key(&s->used, addr);
> - if (!node) {
> - fprintf(stderr, "Error: no record found for an allocation at "
> - "0x%016" PRIx64 ".\n",
> - addr);
> - g_assert_not_reached();
> - }
> -
> - /* Rip it out of the used list and re-insert back into the free list. */
> - QTAILQ_REMOVE(&s->used, node, MLIST_ENTNAME);
> - mlist_sort_insert(&s->free, node);
> - mlist_coalesce(&s->free, node);
> -}
> -
> -static uint64_t pc_alloc(QGuestAllocator *allocator, size_t size)
> -{
> - PCAlloc *s = container_of(allocator, PCAlloc, alloc);
> - uint64_t rsize = size;
> - uint64_t naddr;
> -
> - rsize += (PAGE_SIZE - 1);
> - rsize &= -PAGE_SIZE;
> - g_assert_cmpint((s->start + rsize), <=, s->end);
> - g_assert_cmpint(rsize, >=, size);
> -
> - naddr = pc_mlist_alloc(s, rsize);
> - if (s->opts & PC_ALLOC_PARANOID) {
> - pc_mlist_check(s);
> - }
> -
> - return naddr;
> -}
> -
> -static void pc_free(QGuestAllocator *allocator, uint64_t addr)
> -{
> - PCAlloc *s = container_of(allocator, PCAlloc, alloc);
> -
> - pc_mlist_free(s, addr);
> - if (s->opts & PC_ALLOC_PARANOID) {
> - pc_mlist_check(s);
> - }
> -}
> -
> /*
> * Mostly for valgrind happiness, but it does offer
> * a chokepoint for debugging guest memory leaks, too.
> */
> void pc_alloc_uninit(QGuestAllocator *allocator)
> {
> - PCAlloc *s = container_of(allocator, PCAlloc, alloc);
> - MemBlock *node;
> - MemBlock *tmp;
> - PCAllocOpts mask;
> -
> - /* Check for guest leaks, and destroy the list. */
> - QTAILQ_FOREACH_SAFE(node, &s->used, MLIST_ENTNAME, tmp) {
> - if (s->opts & (PC_ALLOC_LEAK_WARN | PC_ALLOC_LEAK_ASSERT)) {
> - fprintf(stderr, "guest malloc leak @ 0x%016" PRIx64 "; "
> - "size 0x%016" PRIx64 ".\n",
> - node->addr, node->size);
> - }
> - if (s->opts & (PC_ALLOC_LEAK_ASSERT)) {
> - g_assert_not_reached();
> - }
> - g_free(node);
> - }
> -
> - /* If we have previously asserted that there are no leaks, then there
> - * should be only one node here with a specific address and size. */
> - mask = PC_ALLOC_LEAK_ASSERT | PC_ALLOC_PARANOID;
> - QTAILQ_FOREACH_SAFE(node, &s->free, MLIST_ENTNAME, tmp) {
> - if ((s->opts & mask) == mask) {
> - if ((node->addr != s->start) ||
> - (node->size != s->end - s->start)) {
> - fprintf(stderr, "Free list is corrupted.\n");
> - g_assert_not_reached();
> - }
> - }
> -
> - g_free(node);
> - }
> -
> - g_free(s);
> + alloc_uninit(allocator);
> }
>
> -QGuestAllocator *pc_alloc_init_flags(PCAllocOpts flags)
> +QGuestAllocator *pc_alloc_init_flags(QAllocOpts flags)
> {
> - PCAlloc *s = g_malloc0(sizeof(*s));
> + QGuestAllocator *s = g_malloc0(sizeof(*s));
> uint64_t ram_size;
> QFWCFG *fw_cfg = pc_fw_cfg_init();
> MemBlock *node;
>
> s->opts = flags;
> - s->alloc.alloc = pc_alloc;
> - s->alloc.free = pc_free;
> + s->page_size = PAGE_SIZE;
>
> ram_size = qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE);
>
> @@ -325,10 +57,10 @@ QGuestAllocator *pc_alloc_init_flags(PCAllocOpts flags)
> node = mlist_new(s->start, s->end - s->start);
> QTAILQ_INSERT_HEAD(&s->free, node, MLIST_ENTNAME);
>
> - return &s->alloc;
> + return s;
> }
>
> inline QGuestAllocator *pc_alloc_init(void)
> {
> - return pc_alloc_init_flags(PC_ALLOC_NO_FLAGS);
> + return pc_alloc_init_flags(ALLOC_NO_FLAGS);
> }
> diff --git a/tests/libqos/malloc-pc.h b/tests/libqos/malloc-pc.h
> index 9f525e3..86ab9f0 100644
> --- a/tests/libqos/malloc-pc.h
> +++ b/tests/libqos/malloc-pc.h
> @@ -15,15 +15,8 @@
>
> #include "libqos/malloc.h"
>
> -typedef enum {
> - PC_ALLOC_NO_FLAGS = 0x00,
> - PC_ALLOC_LEAK_WARN = 0x01,
> - PC_ALLOC_LEAK_ASSERT = 0x02,
> - PC_ALLOC_PARANOID = 0x04
> -} PCAllocOpts;
> -
> QGuestAllocator *pc_alloc_init(void);
> -QGuestAllocator *pc_alloc_init_flags(PCAllocOpts flags);
> -void pc_alloc_uninit(QGuestAllocator *allocator);
> +QGuestAllocator *pc_alloc_init_flags(QAllocOpts flags);
> +void pc_alloc_uninit(QGuestAllocator *allocator);
>
> #endif
> diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c
> new file mode 100644
> index 0000000..e145fd9
> --- /dev/null
> +++ b/tests/libqos/malloc.c
> @@ -0,0 +1,268 @@
> +/*
> + * libqos malloc support
> + *
> + * Copyright (c) 2014
> + *
> + * Author:
> + * John Snow <jsnow@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "libqos/malloc.h"
> +#include "qemu-common.h"
> +#include <stdio.h>
> +#include <inttypes.h>
> +#include <glib.h>
> +
> +static void mlist_delete(MemList *list, MemBlock *node)
> +{
> + g_assert(list && node);
> + QTAILQ_REMOVE(list, node, MLIST_ENTNAME);
> + g_free(node);
> +}
> +
> +static MemBlock *mlist_find_key(MemList *head, uint64_t addr)
> +{
> + MemBlock *node;
> + QTAILQ_FOREACH(node, head, MLIST_ENTNAME) {
> + if (node->addr == addr) {
> + return node;
> + }
> + }
> + return NULL;
> +}
> +
> +static MemBlock *mlist_find_space(MemList *head, uint64_t size)
> +{
> + MemBlock *node;
> +
> + QTAILQ_FOREACH(node, head, MLIST_ENTNAME) {
> + if (node->size >= size) {
> + return node;
> + }
> + }
> + return NULL;
> +}
> +
> +static MemBlock *mlist_sort_insert(MemList *head, MemBlock *insr)
> +{
> + MemBlock *node;
> + g_assert(head && insr);
> +
> + QTAILQ_FOREACH(node, head, MLIST_ENTNAME) {
> + if (insr->addr < node->addr) {
> + QTAILQ_INSERT_BEFORE(node, insr, MLIST_ENTNAME);
> + return insr;
> + }
> + }
> +
> + QTAILQ_INSERT_TAIL(head, insr, MLIST_ENTNAME);
> + return insr;
> +}
> +
> +static inline uint64_t mlist_boundary(MemBlock *node)
> +{
> + return node->size + node->addr;
> +}
> +
> +static MemBlock *mlist_join(MemList *head, MemBlock *left, MemBlock *right)
> +{
> + g_assert(head && left && right);
> +
> + left->size += right->size;
> + mlist_delete(head, right);
> + return left;
> +}
> +
> +static void mlist_coalesce(MemList *head, MemBlock *node)
> +{
> + g_assert(node);
> + MemBlock *left;
> + MemBlock *right;
> + char merge;
> +
> + do {
> + merge = 0;
> + left = QTAILQ_PREV(node, MemList, MLIST_ENTNAME);
> + right = QTAILQ_NEXT(node, MLIST_ENTNAME);
> +
> + /* clowns to the left of me */
> + if (left && mlist_boundary(left) == node->addr) {
> + node = mlist_join(head, left, node);
> + merge = 1;
> + }
> +
> + /* jokers to the right */
> + if (right && mlist_boundary(node) == right->addr) {
> + node = mlist_join(head, node, right);
> + merge = 1;
> + }
> +
> + } while (merge);
> +}
> +
> +static uint64_t mlist_fulfill(QGuestAllocator *s, MemBlock *freenode,
> + uint64_t size)
> +{
> + uint64_t addr;
> + MemBlock *usednode;
> +
> + g_assert(freenode);
> + g_assert_cmpint(freenode->size, >=, size);
> +
> + addr = freenode->addr;
> + if (freenode->size == size) {
> + /* re-use this freenode as our used node */
> + QTAILQ_REMOVE(&s->free, freenode, MLIST_ENTNAME);
> + usednode = freenode;
> + } else {
> + /* adjust the free node and create a new used node */
> + freenode->addr += size;
> + freenode->size -= size;
> + usednode = mlist_new(addr, size);
> + }
> +
> + mlist_sort_insert(&s->used, usednode);
> + return addr;
> +}
> +
> +/* To assert the correctness of the list.
> + * Used only if ALLOC_PARANOID is set. */
> +static void mlist_check(QGuestAllocator *s)
> +{
> + MemBlock *node;
> + uint64_t addr = s->start > 0 ? s->start - 1 : 0;
> + uint64_t next = s->start;
> +
> + QTAILQ_FOREACH(node, &s->free, MLIST_ENTNAME) {
> + g_assert_cmpint(node->addr, >, addr);
> + g_assert_cmpint(node->addr, >=, next);
> + addr = node->addr;
> + next = node->addr + node->size;
> + }
> +
> + addr = s->start > 0 ? s->start - 1 : 0;
> + next = s->start;
> + QTAILQ_FOREACH(node, &s->used, MLIST_ENTNAME) {
> + g_assert_cmpint(node->addr, >, addr);
> + g_assert_cmpint(node->addr, >=, next);
> + addr = node->addr;
> + next = node->addr + node->size;
> + }
> +}
> +
> +static uint64_t mlist_alloc(QGuestAllocator *s, uint64_t size)
> +{
> + MemBlock *node;
> +
> + node = mlist_find_space(&s->free, size);
> + if (!node) {
> + fprintf(stderr, "Out of guest memory.\n");
> + g_assert_not_reached();
> + }
> + return mlist_fulfill(s, node, size);
> +}
> +
> +static void mlist_free(QGuestAllocator *s, uint64_t addr)
> +{
> + MemBlock *node;
> +
> + if (addr == 0) {
> + return;
> + }
> +
> + node = mlist_find_key(&s->used, addr);
> + if (!node) {
> + fprintf(stderr, "Error: no record found for an allocation at "
> + "0x%016" PRIx64 ".\n",
> + addr);
> + g_assert_not_reached();
> + }
> +
> + /* Rip it out of the used list and re-insert back into the free list. */
> + QTAILQ_REMOVE(&s->used, node, MLIST_ENTNAME);
> + mlist_sort_insert(&s->free, node);
> + mlist_coalesce(&s->free, node);
> +}
> +
> +MemBlock *mlist_new(uint64_t addr, uint64_t size)
> +{
> + MemBlock *block;
> +
> + if (!size) {
> + return NULL;
> + }
> + block = g_malloc0(sizeof(MemBlock));
> +
> + block->addr = addr;
> + block->size = size;
> +
> + return block;
> +}
> +
> +/*
> + * Mostly for valgrind happiness, but it does offer
> + * a chokepoint for debugging guest memory leaks, too.
> + */
> +void alloc_uninit(QGuestAllocator *allocator)
> +{
> + MemBlock *node;
> + MemBlock *tmp;
> + QAllocOpts mask;
> +
> + /* Check for guest leaks, and destroy the list. */
> + QTAILQ_FOREACH_SAFE(node, &allocator->used, MLIST_ENTNAME, tmp) {
> + if (allocator->opts & (ALLOC_LEAK_WARN | ALLOC_LEAK_ASSERT)) {
> + fprintf(stderr, "guest malloc leak @ 0x%016" PRIx64 "; "
> + "size 0x%016" PRIx64 ".\n",
> + node->addr, node->size);
> + }
> + if (allocator->opts & (ALLOC_LEAK_ASSERT)) {
> + g_assert_not_reached();
> + }
> + g_free(node);
> + }
> +
> + /* If we have previously asserted that there are no leaks, then there
> + * should be only one node here with a specific address and size. */
> + mask = ALLOC_LEAK_ASSERT | ALLOC_PARANOID;
> + QTAILQ_FOREACH_SAFE(node, &allocator->free, MLIST_ENTNAME, tmp) {
> + if ((allocator->opts & mask) == mask) {
> + if ((node->addr != allocator->start) ||
> + (node->size != allocator->end - allocator->start)) {
> + fprintf(stderr, "Free list is corrupted.\n");
> + g_assert_not_reached();
> + }
> + }
> +
> + g_free(node);
> + }
> +}
The original malloc-pc implementation for alloc_uninit() also deletes
the QGuestAllocator here, but this version doesn't, which creates a
small leak. Did you have a different cleanup pathway in mind?
> +uint64_t guest_alloc(QGuestAllocator *allocator, size_t size)
> +{
> + uint64_t rsize = size;
> + uint64_t naddr;
> +
> + rsize += (allocator->page_size - 1);
> + rsize &= -allocator->page_size;
> + g_assert_cmpint((allocator->start + rsize), <=, allocator->end);
> + g_assert_cmpint(rsize, >=, size);
> +
> + naddr = mlist_alloc(allocator, rsize);
> + if (allocator->opts & ALLOC_PARANOID) {
> + mlist_check(allocator);
> + }
> +
> + return naddr;
> +}
> +
> +void guest_free(QGuestAllocator *allocator, uint64_t addr)
> +{
> + mlist_free(allocator, addr);
> + if (allocator->opts & ALLOC_PARANOID) {
> + mlist_check(allocator);
> + }
> +}
> diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
> index 5565381..465efeb 100644
> --- a/tests/libqos/malloc.h
> +++ b/tests/libqos/malloc.h
> @@ -15,24 +15,39 @@
>
> #include <stdint.h>
> #include <sys/types.h>
> +#include "qemu/queue.h"
>
> -typedef struct QGuestAllocator QGuestAllocator;
> +#define MLIST_ENTNAME entries
>
> -struct QGuestAllocator
> -{
> - uint64_t (*alloc)(QGuestAllocator *allocator, size_t size);
> - void (*free)(QGuestAllocator *allocator, uint64_t addr);
> -};
> +typedef enum {
> + ALLOC_NO_FLAGS = 0x00,
> + ALLOC_LEAK_WARN = 0x01,
> + ALLOC_LEAK_ASSERT = 0x02,
> + ALLOC_PARANOID = 0x04
> +} QAllocOpts;
> +
> +typedef QTAILQ_HEAD(MemList, MemBlock) MemList;
> +typedef struct MemBlock {
> + QTAILQ_ENTRY(MemBlock) MLIST_ENTNAME;
> + uint64_t size;
> + uint64_t addr;
> +} MemBlock;
> +
> +typedef struct QGuestAllocator {
> + QAllocOpts opts;
> + uint64_t start;
> + uint64_t end;
> + uint32_t page_size;
> +
> + MemList used;
> + MemList free;
> +} QGuestAllocator;
> +
> +MemBlock *mlist_new(uint64_t addr, uint64_t size);
> +void alloc_uninit(QGuestAllocator *allocator);
>
> /* Always returns page aligned values */
> -static inline uint64_t guest_alloc(QGuestAllocator *allocator, size_t size)
> -{
> - return allocator->alloc(allocator, size);
> -}
> -
> -static inline void guest_free(QGuestAllocator *allocator, uint64_t addr)
> -{
> - allocator->free(allocator, addr);
> -}
> +uint64_t guest_alloc(QGuestAllocator *allocator, size_t size);
> +void guest_free(QGuestAllocator *allocator, uint64_t addr);
>
> #endif
>
--
—js
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] libqos: Convert malloc-pc allocator to a generic allocator
2014-10-22 16:33 ` John Snow
@ 2014-10-22 21:09 ` Marc Marí
0 siblings, 0 replies; 7+ messages in thread
From: Marc Marí @ 2014-10-22 21:09 UTC (permalink / raw)
To: John Snow; +Cc: qemu-devel
> > +void alloc_uninit(QGuestAllocator *allocator)
> > +{
> > + MemBlock *node;
> > + MemBlock *tmp;
> > + QAllocOpts mask;
> > +
> > + /* Check for guest leaks, and destroy the list. */
> > + QTAILQ_FOREACH_SAFE(node, &allocator->used, MLIST_ENTNAME,
> > tmp) {
> > + if (allocator->opts & (ALLOC_LEAK_WARN |
> > ALLOC_LEAK_ASSERT)) {
> > + fprintf(stderr, "guest malloc leak @ 0x%016" PRIx64 ";
> > "
> > + "size 0x%016" PRIx64 ".\n",
> > + node->addr, node->size);
> > + }
> > + if (allocator->opts & (ALLOC_LEAK_ASSERT)) {
> > + g_assert_not_reached();
> > + }
> > + g_free(node);
> > + }
> > +
> > + /* If we have previously asserted that there are no leaks,
> > then there
> > + * should be only one node here with a specific address and
> > size. */
> > + mask = ALLOC_LEAK_ASSERT | ALLOC_PARANOID;
> > + QTAILQ_FOREACH_SAFE(node, &allocator->free, MLIST_ENTNAME,
> > tmp) {
> > + if ((allocator->opts & mask) == mask) {
> > + if ((node->addr != allocator->start) ||
> > + (node->size != allocator->end - allocator->start))
> > {
> > + fprintf(stderr, "Free list is corrupted.\n");
> > + g_assert_not_reached();
> > + }
> > + }
> > +
> > + g_free(node);
> > + }
> > +}
>
> The original malloc-pc implementation for alloc_uninit() also deletes
> the QGuestAllocator here, but this version doesn't, which creates a
> small leak. Did you have a different cleanup pathway in mind?
When moving things, that free got lost, a little typo. I'll resend
tomorrow morning. Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] libqos: Convert malloc-pc allocator to a generic allocator
2014-10-18 9:24 [Qemu-devel] [PATCH] libqos: Convert malloc-pc allocator to a generic allocator Marc Marí
2014-10-22 16:01 ` Stefan Hajnoczi
2014-10-22 16:33 ` John Snow
@ 2014-10-23 22:49 ` John Snow
2014-10-24 9:00 ` Marc Marí
2 siblings, 1 reply; 7+ messages in thread
From: John Snow @ 2014-10-23 22:49 UTC (permalink / raw)
To: Marc Marí, qemu-devel; +Cc: Andreas Färber, Stefan Hajnoczi
On 10/18/2014 05:24 AM, Marc Marí wrote:
> The allocator in malloc-pc has been extracted, so it can be used in every arch.
> This operation showed that both the alloc and free functions can be also
> generic.
> Because of this, the QGuestAllocator has been removed from is function to wrap
> the alloc and free function, and now just contains the allocator parameters.
> As a result, only the allocator initalizer and unitializer are arch dependent.
>
> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
> tests/Makefile | 2 +-
> tests/libqos/malloc-pc.c | 280 +---------------------------------------------
> tests/libqos/malloc-pc.h | 11 +-
> tests/libqos/malloc.c | 268 ++++++++++++++++++++++++++++++++++++++++++++
> tests/libqos/malloc.h | 45 +++++---
> 5 files changed, 307 insertions(+), 299 deletions(-)
> create mode 100644 tests/libqos/malloc.c
>
> diff --git a/tests/Makefile b/tests/Makefile
> index 834279c..15bc670 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -296,7 +296,7 @@ tests/test-opts-visitor$(EXESUF): tests/test-opts-visitor.o $(test-qapi-obj-y) l
> tests/test-mul64$(EXESUF): tests/test-mul64.o libqemuutil.a
> tests/test-bitops$(EXESUF): tests/test-bitops.o libqemuutil.a
>
> -libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o
> +libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o
> libqos-obj-y += tests/libqos/i2c.o
> libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
> libqos-pc-obj-y += tests/libqos/malloc-pc.o
> diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
> index f4218c6..c9c48fd 100644
> --- a/tests/libqos/malloc-pc.c
> +++ b/tests/libqos/malloc-pc.c
> @@ -17,296 +17,28 @@
> #include "hw/nvram/fw_cfg.h"
>
> #include "qemu-common.h"
> -#include "qemu/queue.h"
> #include <glib.h>
>
> #define PAGE_SIZE (4096)
>
> -#define MLIST_ENTNAME entries
> -typedef QTAILQ_HEAD(MemList, MemBlock) MemList;
> -typedef struct MemBlock {
> - QTAILQ_ENTRY(MemBlock) MLIST_ENTNAME;
> - uint64_t size;
> - uint64_t addr;
> -} MemBlock;
> -
> -typedef struct PCAlloc
> -{
> - QGuestAllocator alloc;
> - PCAllocOpts opts;
> - uint64_t start;
> - uint64_t end;
> -
> - MemList used;
> - MemList free;
> -} PCAlloc;
> -
> -static MemBlock *mlist_new(uint64_t addr, uint64_t size)
> -{
> - MemBlock *block;
> -
> - if (!size) {
> - return NULL;
> - }
> - block = g_malloc0(sizeof(MemBlock));
> -
> - block->addr = addr;
> - block->size = size;
> -
> - return block;
> -}
> -
> -static void mlist_delete(MemList *list, MemBlock *node)
> -{
> - g_assert(list && node);
> - QTAILQ_REMOVE(list, node, MLIST_ENTNAME);
> - g_free(node);
> -}
> -
> -static MemBlock *mlist_find_key(MemList *head, uint64_t addr)
> -{
> - MemBlock *node;
> - QTAILQ_FOREACH(node, head, MLIST_ENTNAME) {
> - if (node->addr == addr) {
> - return node;
> - }
> - }
> - return NULL;
> -}
> -
> -static MemBlock *mlist_find_space(MemList *head, uint64_t size)
> -{
> - MemBlock *node;
> -
> - QTAILQ_FOREACH(node, head, MLIST_ENTNAME) {
> - if (node->size >= size) {
> - return node;
> - }
> - }
> - return NULL;
> -}
> -
> -static MemBlock *mlist_sort_insert(MemList *head, MemBlock *insr)
> -{
> - MemBlock *node;
> - g_assert(head && insr);
> -
> - QTAILQ_FOREACH(node, head, MLIST_ENTNAME) {
> - if (insr->addr < node->addr) {
> - QTAILQ_INSERT_BEFORE(node, insr, MLIST_ENTNAME);
> - return insr;
> - }
> - }
> -
> - QTAILQ_INSERT_TAIL(head, insr, MLIST_ENTNAME);
> - return insr;
> -}
> -
> -static inline uint64_t mlist_boundary(MemBlock *node)
> -{
> - return node->size + node->addr;
> -}
> -
> -static MemBlock *mlist_join(MemList *head, MemBlock *left, MemBlock *right)
> -{
> - g_assert(head && left && right);
> -
> - left->size += right->size;
> - mlist_delete(head, right);
> - return left;
> -}
> -
> -static void mlist_coalesce(MemList *head, MemBlock *node)
> -{
> - g_assert(node);
> - MemBlock *left;
> - MemBlock *right;
> - char merge;
> -
> - do {
> - merge = 0;
> - left = QTAILQ_PREV(node, MemList, MLIST_ENTNAME);
> - right = QTAILQ_NEXT(node, MLIST_ENTNAME);
> -
> - /* clowns to the left of me */
> - if (left && mlist_boundary(left) == node->addr) {
> - node = mlist_join(head, left, node);
> - merge = 1;
> - }
> -
> - /* jokers to the right */
> - if (right && mlist_boundary(node) == right->addr) {
> - node = mlist_join(head, node, right);
> - merge = 1;
> - }
> -
> - } while (merge);
> -}
> -
> -static uint64_t pc_mlist_fulfill(PCAlloc *s, MemBlock *freenode, uint64_t size)
> -{
> - uint64_t addr;
> - MemBlock *usednode;
> -
> - g_assert(freenode);
> - g_assert_cmpint(freenode->size, >=, size);
> -
> - addr = freenode->addr;
> - if (freenode->size == size) {
> - /* re-use this freenode as our used node */
> - QTAILQ_REMOVE(&s->free, freenode, MLIST_ENTNAME);
> - usednode = freenode;
> - } else {
> - /* adjust the free node and create a new used node */
> - freenode->addr += size;
> - freenode->size -= size;
> - usednode = mlist_new(addr, size);
> - }
> -
> - mlist_sort_insert(&s->used, usednode);
> - return addr;
> -}
> -
> -/* To assert the correctness of the list.
> - * Used only if PC_ALLOC_PARANOID is set. */
> -static void pc_mlist_check(PCAlloc *s)
> -{
> - MemBlock *node;
> - uint64_t addr = s->start > 0 ? s->start - 1 : 0;
> - uint64_t next = s->start;
> -
> - QTAILQ_FOREACH(node, &s->free, MLIST_ENTNAME) {
> - g_assert_cmpint(node->addr, >, addr);
> - g_assert_cmpint(node->addr, >=, next);
> - addr = node->addr;
> - next = node->addr + node->size;
> - }
> -
> - addr = s->start > 0 ? s->start - 1 : 0;
> - next = s->start;
> - QTAILQ_FOREACH(node, &s->used, MLIST_ENTNAME) {
> - g_assert_cmpint(node->addr, >, addr);
> - g_assert_cmpint(node->addr, >=, next);
> - addr = node->addr;
> - next = node->addr + node->size;
> - }
> -}
> -
> -static uint64_t pc_mlist_alloc(PCAlloc *s, uint64_t size)
> -{
> - MemBlock *node;
> -
> - node = mlist_find_space(&s->free, size);
> - if (!node) {
> - fprintf(stderr, "Out of guest memory.\n");
> - g_assert_not_reached();
> - }
> - return pc_mlist_fulfill(s, node, size);
> -}
> -
> -static void pc_mlist_free(PCAlloc *s, uint64_t addr)
> -{
> - MemBlock *node;
> -
> - if (addr == 0) {
> - return;
> - }
> -
> - node = mlist_find_key(&s->used, addr);
> - if (!node) {
> - fprintf(stderr, "Error: no record found for an allocation at "
> - "0x%016" PRIx64 ".\n",
> - addr);
> - g_assert_not_reached();
> - }
> -
> - /* Rip it out of the used list and re-insert back into the free list. */
> - QTAILQ_REMOVE(&s->used, node, MLIST_ENTNAME);
> - mlist_sort_insert(&s->free, node);
> - mlist_coalesce(&s->free, node);
> -}
> -
> -static uint64_t pc_alloc(QGuestAllocator *allocator, size_t size)
> -{
> - PCAlloc *s = container_of(allocator, PCAlloc, alloc);
> - uint64_t rsize = size;
> - uint64_t naddr;
> -
> - rsize += (PAGE_SIZE - 1);
> - rsize &= -PAGE_SIZE;
> - g_assert_cmpint((s->start + rsize), <=, s->end);
> - g_assert_cmpint(rsize, >=, size);
> -
> - naddr = pc_mlist_alloc(s, rsize);
> - if (s->opts & PC_ALLOC_PARANOID) {
> - pc_mlist_check(s);
> - }
> -
> - return naddr;
> -}
> -
> -static void pc_free(QGuestAllocator *allocator, uint64_t addr)
> -{
> - PCAlloc *s = container_of(allocator, PCAlloc, alloc);
> -
> - pc_mlist_free(s, addr);
> - if (s->opts & PC_ALLOC_PARANOID) {
> - pc_mlist_check(s);
> - }
> -}
> -
> /*
> * Mostly for valgrind happiness, but it does offer
> * a chokepoint for debugging guest memory leaks, too.
> */
> void pc_alloc_uninit(QGuestAllocator *allocator)
> {
> - PCAlloc *s = container_of(allocator, PCAlloc, alloc);
> - MemBlock *node;
> - MemBlock *tmp;
> - PCAllocOpts mask;
> -
> - /* Check for guest leaks, and destroy the list. */
> - QTAILQ_FOREACH_SAFE(node, &s->used, MLIST_ENTNAME, tmp) {
> - if (s->opts & (PC_ALLOC_LEAK_WARN | PC_ALLOC_LEAK_ASSERT)) {
> - fprintf(stderr, "guest malloc leak @ 0x%016" PRIx64 "; "
> - "size 0x%016" PRIx64 ".\n",
> - node->addr, node->size);
> - }
> - if (s->opts & (PC_ALLOC_LEAK_ASSERT)) {
> - g_assert_not_reached();
> - }
> - g_free(node);
> - }
> -
> - /* If we have previously asserted that there are no leaks, then there
> - * should be only one node here with a specific address and size. */
> - mask = PC_ALLOC_LEAK_ASSERT | PC_ALLOC_PARANOID;
> - QTAILQ_FOREACH_SAFE(node, &s->free, MLIST_ENTNAME, tmp) {
> - if ((s->opts & mask) == mask) {
> - if ((node->addr != s->start) ||
> - (node->size != s->end - s->start)) {
> - fprintf(stderr, "Free list is corrupted.\n");
> - g_assert_not_reached();
> - }
> - }
> -
> - g_free(node);
> - }
> -
> - g_free(s);
> + alloc_uninit(allocator);
> }
>
> -QGuestAllocator *pc_alloc_init_flags(PCAllocOpts flags)
> +QGuestAllocator *pc_alloc_init_flags(QAllocOpts flags)
> {
> - PCAlloc *s = g_malloc0(sizeof(*s));
> + QGuestAllocator *s = g_malloc0(sizeof(*s));
> uint64_t ram_size;
> QFWCFG *fw_cfg = pc_fw_cfg_init();
> MemBlock *node;
>
> s->opts = flags;
> - s->alloc.alloc = pc_alloc;
> - s->alloc.free = pc_free;
> + s->page_size = PAGE_SIZE;
>
> ram_size = qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE);
>
> @@ -325,10 +57,10 @@ QGuestAllocator *pc_alloc_init_flags(PCAllocOpts flags)
> node = mlist_new(s->start, s->end - s->start);
> QTAILQ_INSERT_HEAD(&s->free, node, MLIST_ENTNAME);
>
> - return &s->alloc;
> + return s;
> }
>
> inline QGuestAllocator *pc_alloc_init(void)
> {
> - return pc_alloc_init_flags(PC_ALLOC_NO_FLAGS);
> + return pc_alloc_init_flags(ALLOC_NO_FLAGS);
> }
> diff --git a/tests/libqos/malloc-pc.h b/tests/libqos/malloc-pc.h
> index 9f525e3..86ab9f0 100644
> --- a/tests/libqos/malloc-pc.h
> +++ b/tests/libqos/malloc-pc.h
> @@ -15,15 +15,8 @@
>
> #include "libqos/malloc.h"
>
> -typedef enum {
> - PC_ALLOC_NO_FLAGS = 0x00,
> - PC_ALLOC_LEAK_WARN = 0x01,
> - PC_ALLOC_LEAK_ASSERT = 0x02,
> - PC_ALLOC_PARANOID = 0x04
> -} PCAllocOpts;
> -
> QGuestAllocator *pc_alloc_init(void);
> -QGuestAllocator *pc_alloc_init_flags(PCAllocOpts flags);
> -void pc_alloc_uninit(QGuestAllocator *allocator);
> +QGuestAllocator *pc_alloc_init_flags(QAllocOpts flags);
> +void pc_alloc_uninit(QGuestAllocator *allocator);
>
> #endif
> diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c
> new file mode 100644
> index 0000000..e145fd9
> --- /dev/null
> +++ b/tests/libqos/malloc.c
> @@ -0,0 +1,268 @@
> +/*
> + * libqos malloc support
> + *
> + * Copyright (c) 2014
> + *
> + * Author:
> + * John Snow <jsnow@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "libqos/malloc.h"
> +#include "qemu-common.h"
> +#include <stdio.h>
> +#include <inttypes.h>
> +#include <glib.h>
> +
> +static void mlist_delete(MemList *list, MemBlock *node)
> +{
> + g_assert(list && node);
> + QTAILQ_REMOVE(list, node, MLIST_ENTNAME);
> + g_free(node);
> +}
> +
> +static MemBlock *mlist_find_key(MemList *head, uint64_t addr)
> +{
> + MemBlock *node;
> + QTAILQ_FOREACH(node, head, MLIST_ENTNAME) {
> + if (node->addr == addr) {
> + return node;
> + }
> + }
> + return NULL;
> +}
> +
> +static MemBlock *mlist_find_space(MemList *head, uint64_t size)
> +{
> + MemBlock *node;
> +
> + QTAILQ_FOREACH(node, head, MLIST_ENTNAME) {
> + if (node->size >= size) {
> + return node;
> + }
> + }
> + return NULL;
> +}
> +
> +static MemBlock *mlist_sort_insert(MemList *head, MemBlock *insr)
> +{
> + MemBlock *node;
> + g_assert(head && insr);
> +
> + QTAILQ_FOREACH(node, head, MLIST_ENTNAME) {
> + if (insr->addr < node->addr) {
> + QTAILQ_INSERT_BEFORE(node, insr, MLIST_ENTNAME);
> + return insr;
> + }
> + }
> +
> + QTAILQ_INSERT_TAIL(head, insr, MLIST_ENTNAME);
> + return insr;
> +}
> +
> +static inline uint64_t mlist_boundary(MemBlock *node)
> +{
> + return node->size + node->addr;
> +}
> +
> +static MemBlock *mlist_join(MemList *head, MemBlock *left, MemBlock *right)
> +{
> + g_assert(head && left && right);
> +
> + left->size += right->size;
> + mlist_delete(head, right);
> + return left;
> +}
> +
> +static void mlist_coalesce(MemList *head, MemBlock *node)
> +{
> + g_assert(node);
> + MemBlock *left;
> + MemBlock *right;
> + char merge;
> +
> + do {
> + merge = 0;
> + left = QTAILQ_PREV(node, MemList, MLIST_ENTNAME);
> + right = QTAILQ_NEXT(node, MLIST_ENTNAME);
> +
> + /* clowns to the left of me */
> + if (left && mlist_boundary(left) == node->addr) {
> + node = mlist_join(head, left, node);
> + merge = 1;
> + }
> +
> + /* jokers to the right */
> + if (right && mlist_boundary(node) == right->addr) {
> + node = mlist_join(head, node, right);
> + merge = 1;
> + }
> +
> + } while (merge);
> +}
> +
> +static uint64_t mlist_fulfill(QGuestAllocator *s, MemBlock *freenode,
> + uint64_t size)
> +{
> + uint64_t addr;
> + MemBlock *usednode;
> +
> + g_assert(freenode);
> + g_assert_cmpint(freenode->size, >=, size);
> +
> + addr = freenode->addr;
> + if (freenode->size == size) {
> + /* re-use this freenode as our used node */
> + QTAILQ_REMOVE(&s->free, freenode, MLIST_ENTNAME);
> + usednode = freenode;
> + } else {
> + /* adjust the free node and create a new used node */
> + freenode->addr += size;
> + freenode->size -= size;
> + usednode = mlist_new(addr, size);
> + }
> +
> + mlist_sort_insert(&s->used, usednode);
> + return addr;
> +}
> +
> +/* To assert the correctness of the list.
> + * Used only if ALLOC_PARANOID is set. */
> +static void mlist_check(QGuestAllocator *s)
> +{
> + MemBlock *node;
> + uint64_t addr = s->start > 0 ? s->start - 1 : 0;
> + uint64_t next = s->start;
> +
> + QTAILQ_FOREACH(node, &s->free, MLIST_ENTNAME) {
> + g_assert_cmpint(node->addr, >, addr);
> + g_assert_cmpint(node->addr, >=, next);
> + addr = node->addr;
> + next = node->addr + node->size;
> + }
> +
> + addr = s->start > 0 ? s->start - 1 : 0;
> + next = s->start;
> + QTAILQ_FOREACH(node, &s->used, MLIST_ENTNAME) {
> + g_assert_cmpint(node->addr, >, addr);
> + g_assert_cmpint(node->addr, >=, next);
> + addr = node->addr;
> + next = node->addr + node->size;
> + }
> +}
> +
> +static uint64_t mlist_alloc(QGuestAllocator *s, uint64_t size)
> +{
> + MemBlock *node;
> +
> + node = mlist_find_space(&s->free, size);
> + if (!node) {
> + fprintf(stderr, "Out of guest memory.\n");
> + g_assert_not_reached();
> + }
> + return mlist_fulfill(s, node, size);
> +}
> +
> +static void mlist_free(QGuestAllocator *s, uint64_t addr)
> +{
> + MemBlock *node;
> +
> + if (addr == 0) {
> + return;
> + }
> +
> + node = mlist_find_key(&s->used, addr);
> + if (!node) {
> + fprintf(stderr, "Error: no record found for an allocation at "
> + "0x%016" PRIx64 ".\n",
> + addr);
> + g_assert_not_reached();
> + }
> +
> + /* Rip it out of the used list and re-insert back into the free list. */
> + QTAILQ_REMOVE(&s->used, node, MLIST_ENTNAME);
> + mlist_sort_insert(&s->free, node);
> + mlist_coalesce(&s->free, node);
> +}
> +
> +MemBlock *mlist_new(uint64_t addr, uint64_t size)
> +{
> + MemBlock *block;
> +
> + if (!size) {
> + return NULL;
> + }
> + block = g_malloc0(sizeof(MemBlock));
> +
> + block->addr = addr;
> + block->size = size;
> +
> + return block;
> +}
> +
> +/*
> + * Mostly for valgrind happiness, but it does offer
> + * a chokepoint for debugging guest memory leaks, too.
> + */
> +void alloc_uninit(QGuestAllocator *allocator)
> +{
> + MemBlock *node;
> + MemBlock *tmp;
> + QAllocOpts mask;
> +
> + /* Check for guest leaks, and destroy the list. */
> + QTAILQ_FOREACH_SAFE(node, &allocator->used, MLIST_ENTNAME, tmp) {
> + if (allocator->opts & (ALLOC_LEAK_WARN | ALLOC_LEAK_ASSERT)) {
> + fprintf(stderr, "guest malloc leak @ 0x%016" PRIx64 "; "
> + "size 0x%016" PRIx64 ".\n",
> + node->addr, node->size);
> + }
> + if (allocator->opts & (ALLOC_LEAK_ASSERT)) {
> + g_assert_not_reached();
> + }
> + g_free(node);
> + }
> +
> + /* If we have previously asserted that there are no leaks, then there
> + * should be only one node here with a specific address and size. */
> + mask = ALLOC_LEAK_ASSERT | ALLOC_PARANOID;
> + QTAILQ_FOREACH_SAFE(node, &allocator->free, MLIST_ENTNAME, tmp) {
> + if ((allocator->opts & mask) == mask) {
> + if ((node->addr != allocator->start) ||
> + (node->size != allocator->end - allocator->start)) {
> + fprintf(stderr, "Free list is corrupted.\n");
> + g_assert_not_reached();
> + }
> + }
> +
> + g_free(node);
> + }
> +}
> +
> +uint64_t guest_alloc(QGuestAllocator *allocator, size_t size)
> +{
> + uint64_t rsize = size;
> + uint64_t naddr;
> +
> + rsize += (allocator->page_size - 1);
> + rsize &= -allocator->page_size;
> + g_assert_cmpint((allocator->start + rsize), <=, allocator->end);
> + g_assert_cmpint(rsize, >=, size);
> +
> + naddr = mlist_alloc(allocator, rsize);
> + if (allocator->opts & ALLOC_PARANOID) {
> + mlist_check(allocator);
> + }
> +
> + return naddr;
> +}
> +
> +void guest_free(QGuestAllocator *allocator, uint64_t addr)
> +{
> + mlist_free(allocator, addr);
> + if (allocator->opts & ALLOC_PARANOID) {
> + mlist_check(allocator);
> + }
> +}
> diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
> index 5565381..465efeb 100644
> --- a/tests/libqos/malloc.h
> +++ b/tests/libqos/malloc.h
> @@ -15,24 +15,39 @@
>
> #include <stdint.h>
> #include <sys/types.h>
> +#include "qemu/queue.h"
>
> -typedef struct QGuestAllocator QGuestAllocator;
> +#define MLIST_ENTNAME entries
>
> -struct QGuestAllocator
> -{
> - uint64_t (*alloc)(QGuestAllocator *allocator, size_t size);
> - void (*free)(QGuestAllocator *allocator, uint64_t addr);
> -};
> +typedef enum {
> + ALLOC_NO_FLAGS = 0x00,
> + ALLOC_LEAK_WARN = 0x01,
> + ALLOC_LEAK_ASSERT = 0x02,
> + ALLOC_PARANOID = 0x04
> +} QAllocOpts;
> +
> +typedef QTAILQ_HEAD(MemList, MemBlock) MemList;
> +typedef struct MemBlock {
> + QTAILQ_ENTRY(MemBlock) MLIST_ENTNAME;
> + uint64_t size;
> + uint64_t addr;
> +} MemBlock;
> +
> +typedef struct QGuestAllocator {
> + QAllocOpts opts;
> + uint64_t start;
> + uint64_t end;
> + uint32_t page_size;
> +
> + MemList used;
> + MemList free;
> +} QGuestAllocator;
> +
> +MemBlock *mlist_new(uint64_t addr, uint64_t size);
> +void alloc_uninit(QGuestAllocator *allocator);
>
> /* Always returns page aligned values */
> -static inline uint64_t guest_alloc(QGuestAllocator *allocator, size_t size)
> -{
> - return allocator->alloc(allocator, size);
> -}
> -
> -static inline void guest_free(QGuestAllocator *allocator, uint64_t addr)
> -{
> - allocator->free(allocator, addr);
> -}
> +uint64_t guest_alloc(QGuestAllocator *allocator, size_t size);
> +void guest_free(QGuestAllocator *allocator, uint64_t addr);
>
> #endif
>
Reviewed-by: John Snow <jsnow@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] libqos: Convert malloc-pc allocator to a generic allocator
2014-10-23 22:49 ` John Snow
@ 2014-10-24 9:00 ` Marc Marí
2014-10-24 15:02 ` John Snow
0 siblings, 1 reply; 7+ messages in thread
From: Marc Marí @ 2014-10-24 9:00 UTC (permalink / raw)
To: John Snow; +Cc: qemu-devel, Stefan Hajnoczi, Andreas Färber
El Thu, 23 Oct 2014 18:49:03 -0400
John Snow <jsnow@redhat.com> escribió:
> Reviewed-by: John Snow <jsnow@redhat.com>
Did you mean this for the v2, which has the extra free?
Marc
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] libqos: Convert malloc-pc allocator to a generic allocator
2014-10-24 9:00 ` Marc Marí
@ 2014-10-24 15:02 ` John Snow
0 siblings, 0 replies; 7+ messages in thread
From: John Snow @ 2014-10-24 15:02 UTC (permalink / raw)
To: Marc Marí; +Cc: qemu-devel, Stefan Hajnoczi, Andreas Färber
On 10/24/2014 05:00 AM, Marc Marí wrote:
> El Thu, 23 Oct 2014 18:49:03 -0400
> John Snow <jsnow@redhat.com> escribió:
>> Reviewed-by: John Snow <jsnow@redhat.com>
>
> Did you mean this for the v2, which has the extra free?
>
> Marc
>
Yes. No more late-night replies to patches :\
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-10-24 15:03 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-18 9:24 [Qemu-devel] [PATCH] libqos: Convert malloc-pc allocator to a generic allocator Marc Marí
2014-10-22 16:01 ` Stefan Hajnoczi
2014-10-22 16:33 ` John Snow
2014-10-22 21:09 ` Marc Marí
2014-10-23 22:49 ` John Snow
2014-10-24 9:00 ` Marc Marí
2014-10-24 15:02 ` John Snow
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).