Re: [Qemu-devel] [PATCH v2 06/15] libqos: Update QGuestAllocator to be opaque

["Marc Marí" <marc.mari.barcelo@gmail.com>]

El Mon, 19 Jan 2015 15:15:54 -0500
John Snow <jsnow@redhat.com> escribió:
> To avoid the architecture-specific implementations of the generic
> qtest allocator having to know about fields within the allocator, add
> a page_size setter method for users or arch specializations to use.
> The allocator will assume a default page_size for general use, but it
> can always be overridden.
> 
> Since this was the last instance of code directly using properties of
> the QGuestAllocator object directly, modify the type to be opaque and
> move the structure inside of malloc.c.
> 
> mlist_new, which was previously exported, is made static local to
> malloc.c, as it has no external users.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/libqos/malloc-pc.c |  2 +-
>  tests/libqos/malloc.c    | 61
> ++++++++++++++++++++++++++++++++++++------------
> tests/libqos/malloc.h    | 22 +++-------------- 3 files changed, 50
> insertions(+), 35 deletions(-)
> 
> diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
> index 6a5fdf3..6e253b6 100644
> --- a/tests/libqos/malloc-pc.c
> +++ b/tests/libqos/malloc-pc.c
> @@ -38,7 +38,7 @@ QGuestAllocator *pc_alloc_init_flags(QAllocOpts
> flags) 
>      ram_size = qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE);
>      s = alloc_init_flags(flags, 1 << 20, MIN(ram_size, 0xE0000000));
> -    s->page_size = PAGE_SIZE;
> +    alloc_set_page_size(s, PAGE_SIZE);
>  
>      /* clean-up */
>      g_free(fw_cfg);
> diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c
> index 4ff260f..8cce1ba 100644
> --- a/tests/libqos/malloc.c
> +++ b/tests/libqos/malloc.c
> @@ -16,6 +16,26 @@
>  #include <inttypes.h>
>  #include <glib.h>
>  
> +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;
> +
> +#define DEFAULT_PAGE_SIZE 4096
> +
>  static void mlist_delete(MemList *list, MemBlock *node)
>  {
>      g_assert(list && node);
> @@ -103,6 +123,21 @@ static void mlist_coalesce(MemList *head,
> MemBlock *node) } while (merge);
>  }
>  
> +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 uint64_t mlist_fulfill(QGuestAllocator *s, MemBlock *freenode,
>                                                                  uint64_t
> size) {
> @@ -187,21 +222,6 @@ static void mlist_free(QGuestAllocator *s,
> uint64_t addr) 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.
> @@ -283,6 +303,8 @@ QGuestAllocator *alloc_init(uint64_t start,
> uint64_t end) node = mlist_new(s->start, s->end - s->start);
>      QTAILQ_INSERT_HEAD(&s->free, node, MLIST_ENTNAME);
>  
> +    s->page_size = DEFAULT_PAGE_SIZE;
> +
>      return s;
>  }
>  
> @@ -293,3 +315,12 @@ QGuestAllocator *alloc_init_flags(QAllocOpts
> opts, s->opts = opts;
>      return s;
>  }
> +
> +void alloc_set_page_size(QGuestAllocator *allocator, size_t
> page_size) +{
> +    /* Can't alter the page_size for an allocator in-use */
> +    g_assert(QTAILQ_EMPTY(&allocator->used));
> +
> +    g_assert(is_power_of_2(page_size));
> +    allocator->page_size = page_size;
> +}
> diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
> index 7b29547..a39dba4 100644
> --- a/tests/libqos/malloc.h
> +++ b/tests/libqos/malloc.h
> @@ -17,8 +17,6 @@
>  #include <sys/types.h>
>  #include "qemu/queue.h"
>  
> -#define MLIST_ENTNAME entries
> -
>  typedef enum {
>      ALLOC_NO_FLAGS    = 0x00,
>      ALLOC_LEAK_WARN   = 0x01,
> @@ -26,24 +24,8 @@ typedef enum {
>      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 QGuestAllocator;
>  
> -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 */
> @@ -53,4 +35,6 @@ void guest_free(QGuestAllocator *allocator,
> uint64_t addr); QGuestAllocator *alloc_init(uint64_t start, uint64_t
> end); QGuestAllocator *alloc_init_flags(QAllocOpts flags,
>                                    uint64_t start, uint64_t end);
> +void alloc_set_page_size(QGuestAllocator *allocator, size_t
> page_size); +
>  #endif

Reviewed-by: Marc Marí <marc.mari.barcelo@gmail.com>