[Qemu-devel] [PATCH v3 0/4] libqos: add a simple first-fit memory allocator

[Qemu-devel] [PATCH v3 0/4] libqos: add a simple first-fit memory allocator

[John Snow]

This set collects two patches by Marc Marí already on the mailing list,
but goes further by adding a simple memory allocator that allows us to
track and debug freed memory, and optionally keep track of any leaks.

For convenience: https://github.com/jnsnow/qemu/tree/libqos-alloc

v2: use QTAILQ as a basis for the linked list implementation instead.
    Correct an error in the size of the initial node.
v3: remove mlist wrappers around QTAILQ interface for clarity.
    adjust the options controlling when to do allocation list debugging.

John Snow (2):
  libqos: add a simple first-fit memory allocator
  qtest/ide: Uninitialize PC allocator

Marc Marí (2):
  libqos: Correct mask to align size to PAGE_SIZE in malloc-pc
  libqos: Change free function called in malloc

 tests/ide-test.c         |   2 +
 tests/libqos/malloc-pc.c | 280 +++++++++++++++++++++++++++++++++++++++++++++--
 tests/libqos/malloc-pc.h |   9 ++
 tests/libqos/malloc.h    |   2 +-
 4 files changed, 282 insertions(+), 11 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH v3 1/4] libqos: Correct mask to align size to PAGE_SIZE in malloc-pc

[John Snow]

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

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/libqos/malloc-pc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
index db1496c..2efd095 100644
--- a/tests/libqos/malloc-pc.c
+++ b/tests/libqos/malloc-pc.c
@@ -36,7 +36,7 @@ static uint64_t pc_alloc(QGuestAllocator *allocator, size_t size)
 
 
     size += (PAGE_SIZE - 1);
-    size &= PAGE_SIZE;
+    size &= -PAGE_SIZE;
 
     g_assert_cmpint((s->start + size), <=, s->end);
 
-- 
1.9.3

[Qemu-devel] [PATCH v3 2/4] libqos: Change free function called in malloc

[John Snow]

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

Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/libqos/malloc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
index 46f6000..5565381 100644
--- a/tests/libqos/malloc.h
+++ b/tests/libqos/malloc.h
@@ -32,7 +32,7 @@ static inline uint64_t guest_alloc(QGuestAllocator *allocator, size_t size)
 
 static inline void guest_free(QGuestAllocator *allocator, uint64_t addr)
 {
-    allocator->alloc(allocator, addr);
+    allocator->free(allocator, addr);
 }
 
 #endif
-- 
1.9.3

[Qemu-devel] [PATCH v3 3/4] libqos: add a simple first-fit memory allocator

[John Snow]

Implement a simple first-fit memory allocator that
attempts to keep track of leased blocks of memory
in order to be able to re-use blocks.

Additionally, allow the user to specify when
initializing the device that upon cleanup,
we would like to assert that there are no
blocks in use. This may be useful for identifying
problems in qtests that use more complicated
set-up and tear-down routines.

This functionality is used in my upcoming ahci-test v2
patch set, but I didn't see fit to enable it for any
existing tests, which will continue to operate the
same as they have prior.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/libqos/malloc-pc.c | 280 +++++++++++++++++++++++++++++++++++++++++++++--
 tests/libqos/malloc-pc.h |   9 ++
 2 files changed, 279 insertions(+), 10 deletions(-)

diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
index 2efd095..ba316d3 100644
--- a/tests/libqos/malloc-pc.c
+++ b/tests/libqos/malloc-pc.c
@@ -17,45 +17,294 @@
 #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 uint64_t pc_alloc(QGuestAllocator *allocator, size_t size)
+static MemBlock *mlist_new(uint64_t addr, uint64_t size)
 {
-    PCAlloc *s = container_of(allocator, PCAlloc, alloc);
-    uint64_t addr;
+    MemBlock *block;
+
+    if (!size) {
+        return NULL;
+    }
+    block = g_malloc0(sizeof(MemBlock));
 
+    block->addr = addr;
+    block->size = size;
 
-    size += (PAGE_SIZE - 1);
-    size &= -PAGE_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_cmpint((s->start + size), <=, s->end);
+    g_assert(freenode);
+    g_assert_cmpint(freenode->size, >=, size);
 
-    addr = s->start;
-    s->start += 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);
 }
 
-QGuestAllocator *pc_alloc_init(void)
+QGuestAllocator *pc_alloc_init_flags(PCAllocOpts flags)
 {
     PCAlloc *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;
 
@@ -67,5 +316,16 @@ QGuestAllocator *pc_alloc_init(void)
     /* Respect PCI hole */
     s->end = MIN(ram_size, 0xE0000000);
 
+    QTAILQ_INIT(&s->used);
+    QTAILQ_INIT(&s->free);
+
+    node = mlist_new(s->start, s->end - s->start);
+    QTAILQ_INSERT_HEAD(&s->free, node, MLIST_ENTNAME);
+
     return &s->alloc;
 }
+
+inline QGuestAllocator *pc_alloc_init(void)
+{
+    return pc_alloc_init_flags(PC_ALLOC_NO_FLAGS);
+}
diff --git a/tests/libqos/malloc-pc.h b/tests/libqos/malloc-pc.h
index ff964ab..9f525e3 100644
--- a/tests/libqos/malloc-pc.h
+++ b/tests/libqos/malloc-pc.h
@@ -15,6 +15,15 @@
 
 #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);
 
 #endif
-- 
1.9.3

[Qemu-devel] [PATCH v3 4/4] qtest/ide: Uninitialize PC allocator

[John Snow]

Use the new call to pc_alloc_uninit
as a test for the new pathways.

The leak checking / assert pathways are
not enabled in this patch, leaving this
as an option to future test writers.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/ide-test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/ide-test.c b/tests/ide-test.c
index 4a0d97f..ad32328 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -123,6 +123,8 @@ static void ide_test_start(const char *cmdline_fmt, ...)
 
 static void ide_test_quit(void)
 {
+    pc_alloc_uninit(guest_malloc);
+    guest_malloc = NULL;
     qtest_end();
 }
 
-- 
1.9.3

Re: [Qemu-devel] [PATCH v3 0/4] libqos: add a simple first-fit memory allocator

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1221 bytes --]

On Fri, Aug 01, 2014 at 11:38:55AM -0400, John Snow wrote:
> This set collects two patches by Marc Marí already on the mailing list,
> but goes further by adding a simple memory allocator that allows us to
> track and debug freed memory, and optionally keep track of any leaks.
> 
> For convenience: https://github.com/jnsnow/qemu/tree/libqos-alloc
> 
> v2: use QTAILQ as a basis for the linked list implementation instead.
>     Correct an error in the size of the initial node.
> v3: remove mlist wrappers around QTAILQ interface for clarity.
>     adjust the options controlling when to do allocation list debugging.
> 
> John Snow (2):
>   libqos: add a simple first-fit memory allocator
>   qtest/ide: Uninitialize PC allocator
> 
> Marc Marí (2):
>   libqos: Correct mask to align size to PAGE_SIZE in malloc-pc
>   libqos: Change free function called in malloc
> 
>  tests/ide-test.c         |   2 +
>  tests/libqos/malloc-pc.c | 280 +++++++++++++++++++++++++++++++++++++++++++++--
>  tests/libqos/malloc-pc.h |   9 ++
>  tests/libqos/malloc.h    |   2 +-
>  4 files changed, 282 insertions(+), 11 deletions(-)
> 
> -- 
> 1.9.3
> 

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v3 0/4] libqos: add a simple first-fit memory allocator

[John Snow]

On 08/05/2014 05:46 AM, Stefan Hajnoczi wrote:
> On Fri, Aug 01, 2014 at 11:38:55AM -0400, John Snow wrote:
>> This set collects two patches by Marc Marí already on the mailing list,
>> but goes further by adding a simple memory allocator that allows us to
>> track and debug freed memory, and optionally keep track of any leaks.
>>
>> For convenience: https://github.com/jnsnow/qemu/tree/libqos-alloc
>>
>> v2: use QTAILQ as a basis for the linked list implementation instead.
>>      Correct an error in the size of the initial node.
>> v3: remove mlist wrappers around QTAILQ interface for clarity.
>>      adjust the options controlling when to do allocation list debugging.
>>
>> John Snow (2):
>>    libqos: add a simple first-fit memory allocator
>>    qtest/ide: Uninitialize PC allocator
>>
>> Marc Marí (2):
>>    libqos: Correct mask to align size to PAGE_SIZE in malloc-pc
>>    libqos: Change free function called in malloc
>>
>>   tests/ide-test.c         |   2 +
>>   tests/libqos/malloc-pc.c | 280 +++++++++++++++++++++++++++++++++++++++++++++--
>>   tests/libqos/malloc-pc.h |   9 ++
>>   tests/libqos/malloc.h    |   2 +-
>>   4 files changed, 282 insertions(+), 11 deletions(-)
>>
>> --
>> 1.9.3
>>
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>

Ping?

(The first two patches in this series have been merged into 
upstream/master already.)

-- 
—js

Re: [Qemu-devel] [PATCH v3 0/4] libqos: add a simple first-fit memory allocator

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1266 bytes --]

On Fri, Aug 01, 2014 at 11:38:55AM -0400, John Snow wrote:
> This set collects two patches by Marc Marí already on the mailing list,
> but goes further by adding a simple memory allocator that allows us to
> track and debug freed memory, and optionally keep track of any leaks.
> 
> For convenience: https://github.com/jnsnow/qemu/tree/libqos-alloc
> 
> v2: use QTAILQ as a basis for the linked list implementation instead.
>     Correct an error in the size of the initial node.
> v3: remove mlist wrappers around QTAILQ interface for clarity.
>     adjust the options controlling when to do allocation list debugging.
> 
> John Snow (2):
>   libqos: add a simple first-fit memory allocator
>   qtest/ide: Uninitialize PC allocator
> 
> Marc Marí (2):
>   libqos: Correct mask to align size to PAGE_SIZE in malloc-pc
>   libqos: Change free function called in malloc
> 
>  tests/ide-test.c         |   2 +
>  tests/libqos/malloc-pc.c | 280 +++++++++++++++++++++++++++++++++++++++++++++--
>  tests/libqos/malloc-pc.h |   9 ++
>  tests/libqos/malloc.h    |   2 +-
>  4 files changed, 282 insertions(+), 11 deletions(-)
> 
> -- 
> 1.9.3
> 
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]