[Qemu-devel] [PULL v3 20/38] libqos: Convert malloc-pc allocator to a generic allocator

[Stefan Hajnoczi <stefanha@redhat.com>]

From: Marc Marí <marc.mari.barcelo@gmail.com>

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>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/Makefile           |   2 +-
 tests/libqos/malloc-pc.c | 280 +----------------------------------------------
 tests/libqos/malloc-pc.h |  11 +-
 tests/libqos/malloc.c    | 270 +++++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/malloc.h    |  45 +++++---
 5 files changed, 309 insertions(+), 299 deletions(-)
 create mode 100644 tests/libqos/malloc.c

diff --git a/tests/Makefile b/tests/Makefile
index e4ddb6a..c2e2e52 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -298,7 +298,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..5debf18
--- /dev/null
+++ b/tests/libqos/malloc.c
@@ -0,0 +1,270 @@
+/*
+ * 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);
+    }
+
+    g_free(allocator);
+}
+
+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
-- 
2.1.0