Hi Xueming,
I think I have understood what you are doing and why the change is
required, but
I believe we can improve the commit message.
It might be even a good idea to split the patches:
1. RTE_MALLOC_DEBUG fix
2. malloc_elem_free fix
On 23/08/2017 03:29, Xueming Li wrote:
From: xuemingl <xuemi...@mellanox.com>
'From: ..' not needed.
DPDK uses it's own memory management, few regular memory profiler tool
support DPDK now. Malloc cookie check provides limited memory corruption
check, better than nothing. This patch fixes the following:
1. Replace broken generated configuration macro RTE_LIBRTE_MALLOC_DEBUG
with RTE_MALLOC_DEBUG
2. Fix malloc size calculation when RTE_MALLOC_DEBUG cookie check
enabled.
For the 2nd change, you should mention what is the problem.
The issue is that malloc_elem_free is clearing (setting to 0) the
trailer cookie when RTE_MALLOC_DEBUG is enabled.
You could also add that you would trigger this issue if the elem being
freed does not join the next elem.
Thanks,
Sergio
From real test, it IS very helpful to detect memory corruption.
A better designed DPDK application should quit nicely with resoure
releasing so that all allocated memory could be checked against cookie.
Fixes: af75078 ("first public release")
Cc: Sergio Gonzalez Monroy <sergio.gonzalez.mon...@intel.com>
Signed-off-by: Xueming Li <xuemi...@mellanox.com>
---
lib/librte_eal/common/malloc_elem.c | 8 ++++----
lib/librte_eal/common/malloc_elem.h | 4 ++--
test/test/test_malloc.c | 10 +++++-----
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/lib/librte_eal/common/malloc_elem.c
b/lib/librte_eal/common/malloc_elem.c
index 150769057..889dffd21 100644
--- a/lib/librte_eal/common/malloc_elem.c
+++ b/lib/librte_eal/common/malloc_elem.c
@@ -275,14 +275,14 @@ malloc_elem_free(struct malloc_elem *elem)
return -1;
rte_spinlock_lock(&(elem->heap->lock));
- size_t sz = elem->size - sizeof(*elem);
+ size_t sz = elem->size - sizeof(*elem) - MALLOC_ELEM_TRAILER_LEN;
uint8_t *ptr = (uint8_t *)&elem[1];
struct malloc_elem *next = RTE_PTR_ADD(elem, elem->size);
if (next->state == ELEM_FREE){
/* remove from free list, join to this one */
elem_free_list_remove(next);
join_elem(elem, next);
- sz += sizeof(*elem);
+ sz += (sizeof(*elem) + MALLOC_ELEM_TRAILER_LEN);
}
/* check if previous element is free, if so join with it and return,
@@ -291,8 +291,8 @@ malloc_elem_free(struct malloc_elem *elem)
if (elem->prev != NULL && elem->prev->state == ELEM_FREE) {
elem_free_list_remove(elem->prev);
join_elem(elem->prev, elem);
- sz += sizeof(*elem);
- ptr -= sizeof(*elem);
+ sz += (sizeof(*elem) + MALLOC_ELEM_TRAILER_LEN);
+ ptr -= (sizeof(*elem) + MALLOC_ELEM_TRAILER_LEN);
elem = elem->prev;
}
malloc_elem_free_list_insert(elem);
diff --git a/lib/librte_eal/common/malloc_elem.h
b/lib/librte_eal/common/malloc_elem.h
index f04b2d1e4..ce39129d9 100644
--- a/lib/librte_eal/common/malloc_elem.h
+++ b/lib/librte_eal/common/malloc_elem.h
@@ -53,13 +53,13 @@ struct malloc_elem {
volatile enum elem_state state;
uint32_t pad;
size_t size;
-#ifdef RTE_LIBRTE_MALLOC_DEBUG
+#ifdef RTE_MALLOC_DEBUG
uint64_t header_cookie; /* Cookie marking start of data */
/* trailer cookie at start + size */
#endif
} __rte_cache_aligned;
-#ifndef RTE_LIBRTE_MALLOC_DEBUG
+#ifndef RTE_MALLOC_DEBUG
static const unsigned MALLOC_ELEM_TRAILER_LEN = 0;
/* dummy function - just check if pointer is non-null */
diff --git a/test/test/test_malloc.c b/test/test/test_malloc.c
index 013fd4407..5558acda4 100644
--- a/test/test/test_malloc.c
+++ b/test/test/test_malloc.c
@@ -108,7 +108,7 @@ test_align_overlap_per_lcore(__attribute__((unused)) void
*arg)
}
for(j = 0; j < 1000 ; j++) {
if( *(char *)p1 != 0) {
- printf("rte_zmalloc didn't zero"
+ printf("rte_zmalloc didn't zero "
"the allocated memory\n");
ret = -1;
}
@@ -180,7 +180,7 @@ test_reordered_free_per_lcore(__attribute__((unused)) void
*arg)
}
for(j = 0; j < 1000 ; j++) {
if( *(char *)p1 != 0) {
- printf("rte_zmalloc didn't zero"
+ printf("rte_zmalloc didn't zero "
"the allocated memory\n");
ret = -1;
}
@@ -293,7 +293,7 @@ test_multi_alloc_statistics(void)
struct rte_malloc_socket_stats pre_stats, post_stats ,first_stats,
second_stats;
size_t size = 2048;
int align = 1024;
-#ifndef RTE_LIBRTE_MALLOC_DEBUG
+#ifndef RTE_MALLOC_DEBUG
int trailer_size = 0;
#else
int trailer_size = RTE_CACHE_LINE_SIZE;
@@ -623,7 +623,7 @@ test_rte_malloc_validate(void)
const size_t request_size = 1024;
size_t allocated_size;
char *data_ptr = rte_malloc(NULL, request_size, RTE_CACHE_LINE_SIZE);
-#ifdef RTE_LIBRTE_MALLOC_DEBUG
+#ifdef RTE_MALLOC_DEBUG
int retval;
char *over_write_vals = NULL;
#endif
@@ -645,7 +645,7 @@ test_rte_malloc_validate(void)
if (allocated_size < request_size)
err_return();
-#ifdef RTE_LIBRTE_MALLOC_DEBUG
+#ifdef RTE_MALLOC_DEBUG
/****** change the header to be bad */
char save_buf[64];
