[PATCH 1/2] mem: add write memory barrier before changing heap state

[PATCH 1/2] mem: add write memory barrier before changing heap state

[David Marchand]

From: Didier Pallard <didier.pallard-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>

a write memory barrier is needed before changing heap state
value, else some concurrent core may see state changing before
all initialization values are written to memory, causing
unpredictable results in malloc function.

Signed-off-by: Didier Pallard <didier.pallard-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
---
 lib/librte_malloc/malloc_heap.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_malloc/malloc_heap.c b/lib/librte_malloc/malloc_heap.c
index f4a0294..64668cb 100644
--- a/lib/librte_malloc/malloc_heap.c
+++ b/lib/librte_malloc/malloc_heap.c
@@ -147,6 +147,7 @@ malloc_heap_init(struct malloc_heap *heap)
 			 */
 			heap->numa_socket = heap - mcfg->malloc_heaps;
 			rte_spinlock_init(&heap->lock);
+			rte_wmb();
 			heap->initialised = INITIALISED;
 		}
 	}
-- 
1.7.10.4

[PATCH 2/2] mem: fix initialization check for malloc heap

[David Marchand]

From: Didier Pallard <didier.pallard-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>

initialised field must be checked against INITIALISED value before
allowing malloc to occur, else some core may pass the test and start
malloc while heap is in INITIALISING state and is not fully initialized.

Signed-off-by: Didier Pallard <didier.pallard-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
---
 lib/librte_malloc/malloc_heap.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_malloc/malloc_heap.c b/lib/librte_malloc/malloc_heap.c
index 64668cb..eed8a63 100644
--- a/lib/librte_malloc/malloc_heap.c
+++ b/lib/librte_malloc/malloc_heap.c
@@ -197,7 +197,7 @@ void *
 malloc_heap_alloc(struct malloc_heap *heap,
 		const char *type __attribute__((unused)), size_t size, unsigned align)
 {
-	if (!heap->initialised)
+	if (heap->initialised != INITIALISED)
 		malloc_heap_init(heap);
 
 	size = CACHE_LINE_ROUNDUP(size);
@@ -227,7 +227,7 @@ int
 malloc_heap_get_stats(const struct malloc_heap *heap,
 		struct rte_malloc_socket_stats *socket_stats)
 {
-	if (!heap->initialised)
+	if (heap->initialised != INITIALISED)
 		return -1;
 
 	struct malloc_elem *elem = heap->free_head;
-- 
1.7.10.4

Re: [PATCH 1/2] mem: add write memory barrier before changing heap state

[Richardson, Bruce]

> -----Original Message-----
> From: dev [mailto:dev-bounces-VfR2kkLFssw@public.gmane.org] On Behalf Of David Marchand
> Sent: Tuesday, April 15, 2014 2:51 PM
> To: dev-VfR2kkLFssw@public.gmane.org
> Subject: [dpdk-dev] [PATCH 1/2] mem: add write memory barrier before
> changing heap state
> 
> From: Didier Pallard <didier.pallard-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> 
> a write memory barrier is needed before changing heap state value, else some
> concurrent core may see state changing before all initialization values are
> written to memory, causing unpredictable results in malloc function.
> 
> Signed-off-by: Didier Pallard <didier.pallard-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>

No barrier should be necessary here. As in a number of other places, such as rings, compiler barriers can be used in place of write memory barriers, due to IA ordering rules. However, in this case, both variables referenced are volatile variables and so the assignments to them cannot be reordered by the compiler so no compiler barrier is necessary either.

Regards,
/Bruce

Re: [PATCH 1/2] mem: add write memory barrier before changing heap state

[didier.pallard]

On 04/15/2014 04:08 PM, Richardson, Bruce wrote:
>> -----Original Message-----
>> From: dev [mailto:dev-bounces-VfR2kkLFssw@public.gmane.org] On Behalf Of David Marchand
>> Sent: Tuesday, April 15, 2014 2:51 PM
>> To: dev-VfR2kkLFssw@public.gmane.org
>> Subject: [dpdk-dev] [PATCH 1/2] mem: add write memory barrier before
>> changing heap state
>>
>> From: Didier Pallard <didier.pallard-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
>>
>> a write memory barrier is needed before changing heap state value, else some
>> concurrent core may see state changing before all initialization values are
>> written to memory, causing unpredictable results in malloc function.
>>
>> Signed-off-by: Didier Pallard <didier.pallard-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> No barrier should be necessary here. As in a number of other places, such as rings, compiler barriers can be used in place of write memory barriers, due to IA ordering rules. However, in this case, both variables referenced are volatile variables and so the assignments to them cannot be reordered by the compiler so no compiler barrier is necessary either.
>
> Regards,
> /Bruce

Hi bruce,

Indeed a compiler barrier is absolutely needed here. volatile variable 
use is absolutely not a serializing instruction from compiler point of 
view; only atomic variable use is serializing, due to asm volatile 
(memory) directive use.
Here is the assembler generated with and without rte_wmb:


With rte_wmb

  142:   f0 45 0f b1 07          lock cmpxchg %r8d,(%r15)
  147:   0f 94 c0                sete   %al
  14a:   84 c0                   test   %al,%al
  14c:   74 ea                   je     138 <malloc_heap_alloc+0x68>
  14e:   49 c7 47 10 00 00 00    movq   $0x0,0x10(%r15)
  155:   00
  156:   41 c7 47 18 00 00 00    movl   $0x0,0x18(%r15)
  15d:   00
  15e:   41 c7 47 08 00 00 00    movl   $0x0,0x8(%r15)
  165:   00
  166:   41 c7 47 1c 00 00 00    movl   $0x0,0x1c(%r15)
  16d:   00
  16e:   49 c7 47 20 00 00 00    movq   $0x0,0x20(%r15)
  175:   00
  176:   45 89 57 04             mov    %r10d,0x4(%r15)
  17a:   0f ae f8                sfence
* 17d:   41 c7 07 02 00 00 00    movl   $0x2,(%r15)**
** 184:   41 8b 37                mov    (%r15),%esi**
* 187:   83 fe 02                cmp    $0x2,%esi
  18a:   75 b4                   jne    140 <malloc_heap_alloc+0x70>
  18c:   0f 1f 40 00             nopl   0x0(%rax)
  190:   48 83 c3 3f             add    $0x3f,%rbx


Without rte_wmb

  142:   f0 45 0f b1 07          lock cmpxchg %r8d,(%r15)
  147:   0f 94 c0                sete   %al
  14a:   84 c0                   test   %al,%al
  14c:   74 ea                   je     138 <malloc_heap_alloc+0x68>
  14e:   49 c7 47 10 00 00 00    movq   $0x0,0x10(%r15)
  155:   00
  156:   41 c7 47 08 00 00 00    movl   $0x0,0x8(%r15)
  15d:   00
* 15e:   41 c7 07 02 00 00 00    movl   $0x2,(%r15)**
** 165:   41 8b 37                mov    (%r15),%esi**
* 168:   41 c7 47 18 00 00 00    movl $0x0,0x18(%r15)
  16f:   00
  170:   41 c7 47 1c 00 00 00    movl   $0x0,0x1c(%r15)
  177:   00
  178:   49 c7 47 20 00 00 00    movq   $0x0,0x20(%r15)
  17f:   00
  180:   45 89 57 04             mov    %r10d,0x4(%r15)
  184:   83 fe 02                cmp    $0x2,%esi
  187:   75 b7                   jne    140 <malloc_heap_alloc+0x70>
  189:   0f 1f 80 00 00 00 00    nopl   0x0(%rax)
  190:   48 83 c3 3f             add    $0x3f,%rbx

It's clear that the *heap->initialised = INITIALISED;* instruction has 
been reordered by the compiler.

About rte_wmb and rte_rmb use, i agree with you that on intel 
architecture those macro should do nothing more than compiler barrier, 
due to Intel architecture choices.
But for code completness, i think those memory barriers should remain in 
place in the code, and rte_*mb should map to compiler barrier on intel 
architecture.

didier

Re: [PATCH 1/2] mem: add write memory barrier before changing heap state

[Neil Horman]

On Tue, Apr 15, 2014 at 03:50:58PM +0200, David Marchand wrote:
> From: Didier Pallard <didier.pallard-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> 
> a write memory barrier is needed before changing heap state
> value, else some concurrent core may see state changing before
> all initialization values are written to memory, causing
> unpredictable results in malloc function.
> 
> Signed-off-by: Didier Pallard <didier.pallard-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> ---
>  lib/librte_malloc/malloc_heap.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/librte_malloc/malloc_heap.c b/lib/librte_malloc/malloc_heap.c
> index f4a0294..64668cb 100644
> --- a/lib/librte_malloc/malloc_heap.c
> +++ b/lib/librte_malloc/malloc_heap.c
> @@ -147,6 +147,7 @@ malloc_heap_init(struct malloc_heap *heap)
>  			 */
>  			heap->numa_socket = heap - mcfg->malloc_heaps;
>  			rte_spinlock_init(&heap->lock);
> +			rte_wmb();
>  			heap->initialised = INITIALISED;
>  		}
>  	}
> -- 
> 1.7.10.4
> 
> 
This certainly won't hurt anything, but from my read we have a fixed array of
malloc_heaps in the mem_config array.  It would seem to me that the more
appropriate solution would be to initalize all of the malloc_heaps before
starting any additional threads or processes that access the dpdk.  I.e. call
malloc_heap_init for every entry in the array during rte_eal_init.  That
prevents you from even needing to keep the ->initalized member around because
its ready during the library initalization

Neil
 

[PATCH 0/2] rework heap initialisation

[David Marchand]

Following Neil's suggestion, here is a patchset that removes the need for a
synchronisation mechanism when initialising heap objects.
As a consequence, this patchset replaces the two patches Didier proposed
earlier.

-- 
David Marchand

David Marchand (2):
  malloc: get rid of numa_socket field
  malloc: simplify heap initialisation

 lib/librte_eal/common/include/rte_malloc_heap.h |    8 ----
 lib/librte_malloc/malloc_heap.c                 |   46 +++--------------------
 2 files changed, 5 insertions(+), 49 deletions(-)

-- 
1.7.10.4

[PATCH 1/2] malloc: get rid of numa_socket field

[David Marchand]

We don't really need this field as it is only used when creating the memzone
object associated to this heap.

Signed-off-by: David Marchand <david.marchand-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
---
 lib/librte_eal/common/include/rte_malloc_heap.h |    1 -
 lib/librte_malloc/malloc_heap.c                 |   13 +++++--------
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_malloc_heap.h b/lib/librte_eal/common/include/rte_malloc_heap.h
index d9e959d..ea2a3f5 100644
--- a/lib/librte_eal/common/include/rte_malloc_heap.h
+++ b/lib/librte_eal/common/include/rte_malloc_heap.h
@@ -48,7 +48,6 @@ enum heap_state {
  */
 struct malloc_heap {
 	enum heap_state volatile initialised;
-	unsigned numa_socket;
 	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 f4a0294..375f212 100644
--- a/lib/librte_malloc/malloc_heap.c
+++ b/lib/librte_malloc/malloc_heap.c
@@ -82,6 +82,8 @@ malloc_heap_add_memzone(struct malloc_heap *heap, size_t size, unsigned align)
 	/* ensure the data we want to allocate will fit in the memzone */
 	const size_t min_size = size + align + MALLOC_ELEM_OVERHEAD * 2;
 	const struct rte_memzone *mz = NULL;
+	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
+	unsigned numa_socket = heap - mcfg->malloc_heaps;
 
 	size_t mz_size = min_size;
 	if (mz_size < block_size)
@@ -89,14 +91,14 @@ malloc_heap_add_memzone(struct malloc_heap *heap, size_t size, unsigned align)
 
 	char mz_name[RTE_MEMZONE_NAMESIZE];
 	rte_snprintf(mz_name, sizeof(mz_name), "MALLOC_S%u_HEAP_%u",
-			heap->numa_socket, heap->mz_count++);
+		     numa_socket, heap->mz_count++);
 
 	/* try getting a block. if we fail and we don't need as big a block
 	 * as given in the config, we can shrink our request and try again
 	 */
 	do {
-		mz = rte_memzone_reserve(mz_name, mz_size,
-				heap->numa_socket, mz_flags);
+		mz = rte_memzone_reserve(mz_name, mz_size, numa_socket,
+					 mz_flags);
 		if (mz == NULL)
 			mz_size /= 2;
 	} while (mz == NULL && mz_size > min_size);
@@ -141,11 +143,6 @@ malloc_heap_init(struct malloc_heap *heap)
 			heap->mz_count = 0;
 			heap->alloc_count = 0;
 			heap->total_size = 0;
-			/*
-			 * Find NUMA socket of heap that is being initialised, so that
-			 * malloc_heaps[n].numa_socket == n
-			 */
-			heap->numa_socket = heap - mcfg->malloc_heaps;
 			rte_spinlock_init(&heap->lock);
 			heap->initialised = INITIALISED;
 		}
-- 
1.7.10.4

Re: [PATCH 1/2] malloc: get rid of numa_socket field

[Neil Horman]

On Fri, Apr 18, 2014 at 02:56:17PM +0200, David Marchand wrote:
> We don't really need this field as it is only used when creating the memzone
> object associated to this heap.
> 
> Signed-off-by: David Marchand <david.marchand-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
Acked-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

> ---
>  lib/librte_eal/common/include/rte_malloc_heap.h |    1 -
>  lib/librte_malloc/malloc_heap.c                 |   13 +++++--------
>  2 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/librte_eal/common/include/rte_malloc_heap.h b/lib/librte_eal/common/include/rte_malloc_heap.h
> index d9e959d..ea2a3f5 100644
> --- a/lib/librte_eal/common/include/rte_malloc_heap.h
> +++ b/lib/librte_eal/common/include/rte_malloc_heap.h
> @@ -48,7 +48,6 @@ enum heap_state {
>   */
>  struct malloc_heap {
>  	enum heap_state volatile initialised;
> -	unsigned numa_socket;
>  	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 f4a0294..375f212 100644
> --- a/lib/librte_malloc/malloc_heap.c
> +++ b/lib/librte_malloc/malloc_heap.c
> @@ -82,6 +82,8 @@ malloc_heap_add_memzone(struct malloc_heap *heap, size_t size, unsigned align)
>  	/* ensure the data we want to allocate will fit in the memzone */
>  	const size_t min_size = size + align + MALLOC_ELEM_OVERHEAD * 2;
>  	const struct rte_memzone *mz = NULL;
> +	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
> +	unsigned numa_socket = heap - mcfg->malloc_heaps;
>  
>  	size_t mz_size = min_size;
>  	if (mz_size < block_size)
> @@ -89,14 +91,14 @@ malloc_heap_add_memzone(struct malloc_heap *heap, size_t size, unsigned align)
>  
>  	char mz_name[RTE_MEMZONE_NAMESIZE];
>  	rte_snprintf(mz_name, sizeof(mz_name), "MALLOC_S%u_HEAP_%u",
> -			heap->numa_socket, heap->mz_count++);
> +		     numa_socket, heap->mz_count++);
>  
>  	/* try getting a block. if we fail and we don't need as big a block
>  	 * as given in the config, we can shrink our request and try again
>  	 */
>  	do {
> -		mz = rte_memzone_reserve(mz_name, mz_size,
> -				heap->numa_socket, mz_flags);
> +		mz = rte_memzone_reserve(mz_name, mz_size, numa_socket,
> +					 mz_flags);
>  		if (mz == NULL)
>  			mz_size /= 2;
>  	} while (mz == NULL && mz_size > min_size);
> @@ -141,11 +143,6 @@ malloc_heap_init(struct malloc_heap *heap)
>  			heap->mz_count = 0;
>  			heap->alloc_count = 0;
>  			heap->total_size = 0;
> -			/*
> -			 * Find NUMA socket of heap that is being initialised, so that
> -			 * malloc_heaps[n].numa_socket == n
> -			 */
> -			heap->numa_socket = heap - mcfg->malloc_heaps;
>  			rte_spinlock_init(&heap->lock);
>  			heap->initialised = INITIALISED;
>  		}
> -- 
> 1.7.10.4
> 
> 

Re: [PATCH 1/2] malloc: get rid of numa_socket field

[Thomas Monjalon]

2014-04-18 09:08, Neil Horman:
> On Fri, Apr 18, 2014 at 02:56:17PM +0200, David Marchand wrote:
> > We don't really need this field as it is only used when creating the
> > memzone object associated to this heap.
> > 
> > Signed-off-by: David Marchand <david.marchand-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> 
> Acked-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

Applied for version 1.6.0r2.

Thanks
-- 
Thomas

[PATCH 2/2] malloc: simplify heap initialisation

[David Marchand]

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-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
---
 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

Re: [PATCH 2/2] malloc: simplify heap initialisation

[Neil Horman]

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-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
Acked-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

> ---
>  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
> 
> 

Re: [PATCH 2/2] malloc: simplify heap initialisation

[Thomas Monjalon]

2014-04-18 09:09, Neil Horman:
> 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-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> 
> Acked-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

Applied for version 1.6.0r2.

Thanks
-- 
Thomas