Re: [dpdk-dev] [PATCH 2/2] malloc: simplify heap initialisation

[Neil Horman <nhorman@tuxdriver.com>]

On Fri, Apr 18, 2014 at 02:56:18PM +0200, David Marchand wrote:
> There should be no real need for this initialised field as the whole structure
> is set to 0 in rte_config_init() by primary process, and secondary processes
> wait for this to happen before anything else (looking at mem_config magic).
> 
> Signed-off-by: David Marchand <david.marchand@6wind.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>

> ---
>  lib/librte_eal/common/include/rte_malloc_heap.h |    7 -----
>  lib/librte_malloc/malloc_heap.c                 |   33 -----------------------
>  2 files changed, 40 deletions(-)
> 
> diff --git a/lib/librte_eal/common/include/rte_malloc_heap.h b/lib/librte_eal/common/include/rte_malloc_heap.h
> index ea2a3f5..5e139cf 100644
> --- a/lib/librte_eal/common/include/rte_malloc_heap.h
> +++ b/lib/librte_eal/common/include/rte_malloc_heap.h
> @@ -37,17 +37,10 @@
>  #include <stddef.h>
>  #include <rte_spinlock.h>
>  
> -enum heap_state {
> -	NOT_INITIALISED = 0,
> -	INITIALISING,
> -	INITIALISED
> -};
> -
>  /**
>   * Structure to hold malloc heap
>   */
>  struct malloc_heap {
> -	enum heap_state volatile initialised;
>  	rte_spinlock_t lock;
>  	struct malloc_elem * volatile free_head;
>  	unsigned mz_count;
> diff --git a/lib/librte_malloc/malloc_heap.c b/lib/librte_malloc/malloc_heap.c
> index 375f212..882749c 100644
> --- a/lib/librte_malloc/malloc_heap.c
> +++ b/lib/librte_malloc/malloc_heap.c
> @@ -123,33 +123,6 @@ malloc_heap_add_memzone(struct malloc_heap *heap, size_t size, unsigned align)
>  }
>  
>  /*
> - * initialise a malloc heap object. The heap is locked with a private
> - * lock while being initialised. This function should only be called the
> - * first time a thread calls malloc - if even then, as heaps are per-socket
> - * not per-thread.
> - */
> -static void
> -malloc_heap_init(struct malloc_heap *heap)
> -{
> -	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
> -
> -	rte_eal_mcfg_wait_complete(mcfg);
> -	while (heap->initialised != INITIALISED) {
> -		if (rte_atomic32_cmpset(
> -				(volatile uint32_t*)&heap->initialised,
> -				NOT_INITIALISED, INITIALISING)) {
> -
> -			heap->free_head = NULL;
> -			heap->mz_count = 0;
> -			heap->alloc_count = 0;
> -			heap->total_size = 0;
> -			rte_spinlock_init(&heap->lock);
> -			heap->initialised = INITIALISED;
> -		}
> -	}
> -}
> -
> -/*
>   * Iterates through the freelist for a heap to find a free element
>   * which can store data of the required size and with the requested alignment.
>   * Returns null on failure, or pointer to element on success, with the pointer
> @@ -193,9 +166,6 @@ void *
>  malloc_heap_alloc(struct malloc_heap *heap,
>  		const char *type __attribute__((unused)), size_t size, unsigned align)
>  {
> -	if (!heap->initialised)
> -		malloc_heap_init(heap);
> -
>  	size = CACHE_LINE_ROUNDUP(size);
>  	align = CACHE_LINE_ROUNDUP(align);
>  	rte_spinlock_lock(&heap->lock);
> @@ -223,9 +193,6 @@ int
>  malloc_heap_get_stats(const struct malloc_heap *heap,
>  		struct rte_malloc_socket_stats *socket_stats)
>  {
> -	if (!heap->initialised)
> -		return -1;
> -
>  	struct malloc_elem *elem = heap->free_head;
>  
>  	/* Initialise variables for heap */
> -- 
> 1.7.10.4
> 
>