DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] eal: fix modify data area after memset
@ 2023-09-12  9:04 Fengnan Chang
  2023-09-22  8:12 ` Fengnan Chang
  2023-10-17 13:32 ` Thomas Monjalon
  0 siblings, 2 replies; 7+ messages in thread
From: Fengnan Chang @ 2023-09-12  9:04 UTC (permalink / raw)
  To: anatoly.burakov, dev, xuemingl; +Cc: Fengnan Chang

Let's look at this path:
malloc_elem_free
   ->malloc_elem_join_adjacent_free
      ->join_elem(elem, elem->next)

0. cur elem's pad > 0
1. data area memset in malloc_elem_free first.
2. next elem is free, try to join cur elem and next.
3. in join_elem, try to modify inner->size, this address had
memset in step 1, it casue the content of addrees become non-zero.

If user call rte_zmalloc, and pick this elem, it can't get all
zero'd memory.

Fixes: 2808a12cc053 (malloc: fix memory element size in case of padding)
Signed-off-by: Fengnan Chang <changfengnan@bytedance.com>
---
 lib/eal/common/malloc_elem.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/eal/common/malloc_elem.c b/lib/eal/common/malloc_elem.c
index 619c040aa3..93a23fa8d4 100644
--- a/lib/eal/common/malloc_elem.c
+++ b/lib/eal/common/malloc_elem.c
@@ -492,7 +492,7 @@ malloc_elem_alloc(struct malloc_elem *elem, size_t size, unsigned align,
  * be contiguous in memory.
  */
 static inline void
-join_elem(struct malloc_elem *elem1, struct malloc_elem *elem2)
+join_elem(struct malloc_elem *elem1, struct malloc_elem *elem2, bool update_inner)
 {
 	struct malloc_elem *next = elem2->next;
 	elem1->size += elem2->size;
@@ -502,7 +502,7 @@ join_elem(struct malloc_elem *elem1, struct malloc_elem *elem2)
 		elem1->heap->last = elem1;
 	elem1->next = next;
 	elem1->dirty |= elem2->dirty;
-	if (elem1->pad) {
+	if (elem1->pad && update_inner) {
 		struct malloc_elem *inner = RTE_PTR_ADD(elem1, elem1->pad);
 		inner->size = elem1->size - elem1->pad;
 	}
@@ -526,7 +526,7 @@ malloc_elem_join_adjacent_free(struct malloc_elem *elem)
 
 		/* remove from free list, join to this one */
 		malloc_elem_free_list_remove(elem->next);
-		join_elem(elem, elem->next);
+		join_elem(elem, elem->next, false);
 
 		/* erase header, trailer and pad */
 		memset(erase, MALLOC_POISON, erase_len);
@@ -550,7 +550,7 @@ malloc_elem_join_adjacent_free(struct malloc_elem *elem)
 		malloc_elem_free_list_remove(elem->prev);
 
 		new_elem = elem->prev;
-		join_elem(new_elem, elem);
+		join_elem(new_elem, elem, false);
 
 		/* erase header, trailer and pad */
 		memset(erase, MALLOC_POISON, erase_len);
@@ -683,7 +683,7 @@ malloc_elem_resize(struct malloc_elem *elem, size_t size)
 	 * join the two
 	 */
 	malloc_elem_free_list_remove(elem->next);
-	join_elem(elem, elem->next);
+	join_elem(elem, elem->next, true);
 
 	if (elem->size - new_size >= MIN_DATA_SIZE + MALLOC_ELEM_OVERHEAD) {
 		/* now we have a big block together. Lets cut it down a bit, by splitting */
-- 
2.20.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] eal: fix modify data area after memset
  2023-09-12  9:04 [PATCH] eal: fix modify data area after memset Fengnan Chang
@ 2023-09-22  8:12 ` Fengnan Chang
  2023-10-22 20:22   ` Dmitry Kozlyuk
  2023-10-17 13:32 ` Thomas Monjalon
  1 sibling, 1 reply; 7+ messages in thread
From: Fengnan Chang @ 2023-09-22  8:12 UTC (permalink / raw)
  To: anatoly.burakov, dev, xuemingl

ping

Fengnan Chang <changfengnan@bytedance.com> 于2023年9月12日周二 17:05写道:
>
> Let's look at this path:
> malloc_elem_free
>    ->malloc_elem_join_adjacent_free
>       ->join_elem(elem, elem->next)
>
> 0. cur elem's pad > 0
> 1. data area memset in malloc_elem_free first.
> 2. next elem is free, try to join cur elem and next.
> 3. in join_elem, try to modify inner->size, this address had
> memset in step 1, it casue the content of addrees become non-zero.
>
> If user call rte_zmalloc, and pick this elem, it can't get all
> zero'd memory.
>
> Fixes: 2808a12cc053 (malloc: fix memory element size in case of padding)
> Signed-off-by: Fengnan Chang <changfengnan@bytedance.com>
> ---
>  lib/eal/common/malloc_elem.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/lib/eal/common/malloc_elem.c b/lib/eal/common/malloc_elem.c
> index 619c040aa3..93a23fa8d4 100644
> --- a/lib/eal/common/malloc_elem.c
> +++ b/lib/eal/common/malloc_elem.c
> @@ -492,7 +492,7 @@ malloc_elem_alloc(struct malloc_elem *elem, size_t size, unsigned align,
>   * be contiguous in memory.
>   */
>  static inline void
> -join_elem(struct malloc_elem *elem1, struct malloc_elem *elem2)
> +join_elem(struct malloc_elem *elem1, struct malloc_elem *elem2, bool update_inner)
>  {
>         struct malloc_elem *next = elem2->next;
>         elem1->size += elem2->size;
> @@ -502,7 +502,7 @@ join_elem(struct malloc_elem *elem1, struct malloc_elem *elem2)
>                 elem1->heap->last = elem1;
>         elem1->next = next;
>         elem1->dirty |= elem2->dirty;
> -       if (elem1->pad) {
> +       if (elem1->pad && update_inner) {
>                 struct malloc_elem *inner = RTE_PTR_ADD(elem1, elem1->pad);
>                 inner->size = elem1->size - elem1->pad;
>         }
> @@ -526,7 +526,7 @@ malloc_elem_join_adjacent_free(struct malloc_elem *elem)
>
>                 /* remove from free list, join to this one */
>                 malloc_elem_free_list_remove(elem->next);
> -               join_elem(elem, elem->next);
> +               join_elem(elem, elem->next, false);
>
>                 /* erase header, trailer and pad */
>                 memset(erase, MALLOC_POISON, erase_len);
> @@ -550,7 +550,7 @@ malloc_elem_join_adjacent_free(struct malloc_elem *elem)
>                 malloc_elem_free_list_remove(elem->prev);
>
>                 new_elem = elem->prev;
> -               join_elem(new_elem, elem);
> +               join_elem(new_elem, elem, false);
>
>                 /* erase header, trailer and pad */
>                 memset(erase, MALLOC_POISON, erase_len);
> @@ -683,7 +683,7 @@ malloc_elem_resize(struct malloc_elem *elem, size_t size)
>          * join the two
>          */
>         malloc_elem_free_list_remove(elem->next);
> -       join_elem(elem, elem->next);
> +       join_elem(elem, elem->next, true);
>
>         if (elem->size - new_size >= MIN_DATA_SIZE + MALLOC_ELEM_OVERHEAD) {
>                 /* now we have a big block together. Lets cut it down a bit, by splitting */
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] eal: fix modify data area after memset
  2023-09-12  9:04 [PATCH] eal: fix modify data area after memset Fengnan Chang
  2023-09-22  8:12 ` Fengnan Chang
@ 2023-10-17 13:32 ` Thomas Monjalon
  1 sibling, 0 replies; 7+ messages in thread
From: Thomas Monjalon @ 2023-10-17 13:32 UTC (permalink / raw)
  To: anatoly.burakov
  Cc: dev, xuemingl, Fengnan Chang, dmitry.kozliuk, Morten Brørup,
	stephen, bruce.richardson, david.marchand

We need a careful review here, please.


12/09/2023 11:04, Fengnan Chang:
> Let's look at this path:
> malloc_elem_free
>    ->malloc_elem_join_adjacent_free
>       ->join_elem(elem, elem->next)
> 
> 0. cur elem's pad > 0
> 1. data area memset in malloc_elem_free first.
> 2. next elem is free, try to join cur elem and next.
> 3. in join_elem, try to modify inner->size, this address had
> memset in step 1, it casue the content of addrees become non-zero.
> 
> If user call rte_zmalloc, and pick this elem, it can't get all
> zero'd memory.
> 
> Fixes: 2808a12cc053 (malloc: fix memory element size in case of padding)
> Signed-off-by: Fengnan Chang <changfengnan@bytedance.com>
> ---
>  lib/eal/common/malloc_elem.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/eal/common/malloc_elem.c b/lib/eal/common/malloc_elem.c
> index 619c040aa3..93a23fa8d4 100644
> --- a/lib/eal/common/malloc_elem.c
> +++ b/lib/eal/common/malloc_elem.c
> @@ -492,7 +492,7 @@ malloc_elem_alloc(struct malloc_elem *elem, size_t size, unsigned align,
>   * be contiguous in memory.
>   */
>  static inline void
> -join_elem(struct malloc_elem *elem1, struct malloc_elem *elem2)
> +join_elem(struct malloc_elem *elem1, struct malloc_elem *elem2, bool update_inner)
>  {
>  	struct malloc_elem *next = elem2->next;
>  	elem1->size += elem2->size;
> @@ -502,7 +502,7 @@ join_elem(struct malloc_elem *elem1, struct malloc_elem *elem2)
>  		elem1->heap->last = elem1;
>  	elem1->next = next;
>  	elem1->dirty |= elem2->dirty;
> -	if (elem1->pad) {
> +	if (elem1->pad && update_inner) {
>  		struct malloc_elem *inner = RTE_PTR_ADD(elem1, elem1->pad);
>  		inner->size = elem1->size - elem1->pad;
>  	}
> @@ -526,7 +526,7 @@ malloc_elem_join_adjacent_free(struct malloc_elem *elem)
>  
>  		/* remove from free list, join to this one */
>  		malloc_elem_free_list_remove(elem->next);
> -		join_elem(elem, elem->next);
> +		join_elem(elem, elem->next, false);
>  
>  		/* erase header, trailer and pad */
>  		memset(erase, MALLOC_POISON, erase_len);
> @@ -550,7 +550,7 @@ malloc_elem_join_adjacent_free(struct malloc_elem *elem)
>  		malloc_elem_free_list_remove(elem->prev);
>  
>  		new_elem = elem->prev;
> -		join_elem(new_elem, elem);
> +		join_elem(new_elem, elem, false);
>  
>  		/* erase header, trailer and pad */
>  		memset(erase, MALLOC_POISON, erase_len);
> @@ -683,7 +683,7 @@ malloc_elem_resize(struct malloc_elem *elem, size_t size)
>  	 * join the two
>  	 */
>  	malloc_elem_free_list_remove(elem->next);
> -	join_elem(elem, elem->next);
> +	join_elem(elem, elem->next, true);
>  
>  	if (elem->size - new_size >= MIN_DATA_SIZE + MALLOC_ELEM_OVERHEAD) {
>  		/* now we have a big block together. Lets cut it down a bit, by splitting */
> 






^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] eal: fix modify data area after memset
  2023-09-22  8:12 ` Fengnan Chang
@ 2023-10-22 20:22   ` Dmitry Kozlyuk
  2023-10-23  9:07     ` [External] " Fengnan Chang
  0 siblings, 1 reply; 7+ messages in thread
From: Dmitry Kozlyuk @ 2023-10-22 20:22 UTC (permalink / raw)
  To: Fengnan Chang; +Cc: anatoly.burakov, dev, xuemingl

2023-09-22 16:12 (UTC+0800), Fengnan Chang:
> ping
> 
> Fengnan Chang <changfengnan@bytedance.com> 于2023年9月12日周二 17:05写道:
> >
> > Let's look at this path:
> > malloc_elem_free  
> >    ->malloc_elem_join_adjacent_free
> >       ->join_elem(elem, elem->next)  
> >
> > 0. cur elem's pad > 0
> > 1. data area memset in malloc_elem_free first.
> > 2. next elem is free, try to join cur elem and next.
> > 3. in join_elem, try to modify inner->size, this address had
> > memset in step 1, it casue the content of addrees become non-zero.
> >
> > If user call rte_zmalloc, and pick this elem, it can't get all
> > zero'd memory.

malloc_elem_join_adjacent_free() always calls memset() after join_elem(),
for the next and the previous element respectively.
How to reproduce this bug?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [External] Re: [PATCH] eal: fix modify data area after memset
  2023-10-22 20:22   ` Dmitry Kozlyuk
@ 2023-10-23  9:07     ` Fengnan Chang
  2023-10-25 16:03       ` Stephen Hemminger
  0 siblings, 1 reply; 7+ messages in thread
From: Fengnan Chang @ 2023-10-23  9:07 UTC (permalink / raw)
  To: Dmitry Kozlyuk; +Cc: anatoly.burakov, dev, xuemingl

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

Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> 于2023年10月23日周一 04:22写道:
>
> 2023-09-22 16:12 (UTC+0800), Fengnan Chang:
> > ping
> >
> > Fengnan Chang <changfengnan@bytedance.com> 于2023年9月12日周二 17:05写道:
> > >
> > > Let's look at this path:
> > > malloc_elem_free
> > >    ->malloc_elem_join_adjacent_free
> > >       ->join_elem(elem, elem->next)
> > >
> > > 0. cur elem's pad > 0
> > > 1. data area memset in malloc_elem_free first.
> > > 2. next elem is free, try to join cur elem and next.
> > > 3. in join_elem, try to modify inner->size, this address had
> > > memset in step 1, it casue the content of addrees become non-zero.
> > >
> > > If user call rte_zmalloc, and pick this elem, it can't get all
> > > zero'd memory.
>
> malloc_elem_join_adjacent_free() always calls memset() after join_elem(),
> for the next and the previous element respectively.
when try to call join_elem() for the next element in
malloc_elem_join_adjacent_free(),
the memset is try to memset *next* element, but join_elem() is update
*current* element's
content, which shoudn't happen, it's two different element.

> How to reproduce this bug?
when I test this patch,
https://patches.dpdk.org/project/dpdk/patch/20230831111937.60975-1-changfengnan@bytedance.com/
I have a case try to alloc 64/128/192 size object and free with 16 threads,
after every
alloc I'll check wheather all content is 0 or not.
It's not easy to reproduce, you can have a try, it's easier to find
this problem in code level.

[-- Attachment #2: Type: text/html, Size: 2148 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [External] Re: [PATCH] eal: fix modify data area after memset
  2023-10-23  9:07     ` [External] " Fengnan Chang
@ 2023-10-25 16:03       ` Stephen Hemminger
  2023-10-30 12:31         ` Fengnan Chang
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2023-10-25 16:03 UTC (permalink / raw)
  To: Fengnan Chang; +Cc: Dmitry Kozlyuk, anatoly.burakov, dev, xuemingl

On Mon, 23 Oct 2023 17:07:21 +0800
Fengnan Chang <changfengnan@bytedance.com> wrote:

> Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> 于2023年10月23日周一 04:22写道:
> >
> > 2023-09-22 16:12 (UTC+0800), Fengnan Chang:  
> > > ping
> > >
> > > Fengnan Chang <changfengnan@bytedance.com> 于2023年9月12日周二 17:05写道:  
> > > >
> > > > Let's look at this path:
> > > > malloc_elem_free  
> > > >    ->malloc_elem_join_adjacent_free
> > > >       ->join_elem(elem, elem->next)  
> > > >
> > > > 0. cur elem's pad > 0
> > > > 1. data area memset in malloc_elem_free first.
> > > > 2. next elem is free, try to join cur elem and next.
> > > > 3. in join_elem, try to modify inner->size, this address had
> > > > memset in step 1, it casue the content of addrees become non-zero.
> > > >
> > > > If user call rte_zmalloc, and pick this elem, it can't get all
> > > > zero'd memory.  
> >
> > malloc_elem_join_adjacent_free() always calls memset() after join_elem(),
> > for the next and the previous element respectively.  
> when try to call join_elem() for the next element in
> malloc_elem_join_adjacent_free(),
> the memset is try to memset *next* element, but join_elem() is update
> *current* element's
> content, which shoudn't happen, it's two different element.
> 
> > How to reproduce this bug?  
> when I test this patch,
> https://patches.dpdk.org/project/dpdk/patch/20230831111937.60975-1-changfengnan@bytedance.com/
> I have a case try to alloc 64/128/192 size object and free with 16 threads,
> after every
> alloc I'll check wheather all content is 0 or not.
> It's not easy to reproduce, you can have a try, it's easier to find
> this problem in code level.

I tried to make a test that would reproduce the problem but it did not.

diff --git a/app/test/test_malloc.c b/app/test/test_malloc.c
index cd579c503cf5..cfd45d6a28eb 100644
--- a/app/test/test_malloc.c
+++ b/app/test/test_malloc.c
@@ -28,6 +28,7 @@
 #include <rte_string_fns.h>

 #define N 10000
+#define BINS 100

 static int
 is_mem_on_socket(int32_t socket);
@@ -69,13 +70,24 @@ is_aligned(void *p, int align)
 	return 1;
 }

+static bool is_all_zero(uint8_t *mem, size_t sz)
+{
+	size_t i;
+
+	for (i = 0; i < sz; i++)
+		if (mem[i] != 0)
+			return false;
+
+	return true;
+}
+
 static int
 test_align_overlap_per_lcore(__rte_unused void *arg)
 {
 	const unsigned align1 = 8,
 			align2 = 64,
 			align3 = 2048;
-	unsigned i,j;
+	unsigned int i;
 	void *p1 = NULL, *p2 = NULL, *p3 = NULL;
 	int ret = 0;

@@ -86,11 +98,12 @@ test_align_overlap_per_lcore(__rte_unused void *arg)
 			ret = -1;
 			break;
 		}
-		for(j = 0; j < 1000 ; j++) {
-			if( *(char *)p1 != 0) {
-				printf("rte_zmalloc didn't zero the allocated memory\n");
-				ret = -1;
-			}
+
+		if (!is_all_zero(p1, 1000)) {
+			printf("rte_zmalloc didn't zero the allocated memory\n");
+			ret = -1;
+			rte_free(p1);
+			break;
 		}
 		p2 = rte_malloc("dummy", 1000, align2);
 		if (!p2){
@@ -140,6 +153,66 @@ test_align_overlap_per_lcore(__rte_unused void *arg)
 	return ret;
 }

+/*
+ * Allocate random size chunks and make sure that they are
+ * always zero.
+ */
+static int
+test_zmalloc(__rte_unused void *arg)
+{
+	unsigned int i, n;
+	void *slots[BINS] = { };
+			void *p1;
+	size_t sz;
+
+	/* Allocate many variable size chunks */
+	for (i = 0; i < BINS; i++) {
+		sz = rte_rand_max(1024) + 1;
+		p1 = rte_zmalloc("slots", sz, 0);
+		if (p1 == NULL) {
+			printf("rte_zmalloc(%zu) returned NULL (i=%u)\n", sz, i);
+			goto fail;
+		}
+		slots[i] = p1;
+		if (!is_all_zero(p1, sz))
+			goto fail;
+	}
+
+	/* Drop one chunk per iteration */
+	for (n = BINS; n > 0; n--) {
+		/* Swap in a new block into a slot */
+		for (i = 0; i < N; i++) {
+			unsigned int bin = rte_rand_max(n);
+
+			sz = rte_rand_max(1024) + 1;
+			p1 = rte_zmalloc("swap", sz, 0);
+			if (!p1){
+				printf("rte_zmalloc(%zu) returned NULL (i=%u)\n", sz, i);
+				goto fail;
+			}
+
+			if (!is_all_zero(p1, sz)) {
+				printf("rte_zmalloc didn't zero the allocated memory\n");
+				goto fail;
+			}
+
+			rte_free(slots[bin]);
+			slots[bin] = p1;
+		}
+
+		/* Drop last bin */
+		rte_free(slots[n]);
+		slots[n] = NULL;
+	}
+
+	return 0;
+fail:
+	for (i = 0; i < BINS; i++)
+		rte_free(slots[i]);
+
+	return -1;
+}
+
 static int
 test_reordered_free_per_lcore(__rte_unused void *arg)
 {
@@ -1020,6 +1091,21 @@ test_malloc(void)
 	}
 	else printf("test_realloc() passed\n");

+	/*----------------------------*/
+	RTE_LCORE_FOREACH_WORKER(lcore_id) {
+		rte_eal_remote_launch(test_zmalloc, NULL, lcore_id);
+	}
+
+	RTE_LCORE_FOREACH_WORKER(lcore_id) {
+		if (rte_eal_wait_lcore(lcore_id) < 0)
+			ret = -1;
+	}
+	if (ret < 0){
+		printf("test_zmalloc() failed\n");
+		return ret;
+	}
+	else printf("test_zmalloc() passed\n");
+
 	/*----------------------------*/
 	RTE_LCORE_FOREACH_WORKER(lcore_id) {
 		rte_eal_remote_launch(test_align_overlap_per_lcore, NULL, lcore_id);

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [External] Re: [PATCH] eal: fix modify data area after memset
  2023-10-25 16:03       ` Stephen Hemminger
@ 2023-10-30 12:31         ` Fengnan Chang
  0 siblings, 0 replies; 7+ messages in thread
From: Fengnan Chang @ 2023-10-30 12:31 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Dmitry Kozlyuk, anatoly.burakov, dev, xuemingl

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

Thanks for your response, It's my fault,  I got mixed up,
this problem only can reproduce after apply
https://patches.dpdk.org/project/dpdk/patch/20230831111937.60975-1-changfengnan@bytedance.com/
,
I'll reorganize this to previous patch.
So sorry for waste your time.

Stephen Hemminger <stephen@networkplumber.org> 于2023年10月26日周四 00:04写道:

> On Mon, 23 Oct 2023 17:07:21 +0800
> Fengnan Chang <changfengnan@bytedance.com> wrote:
>
> > Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> 于2023年10月23日周一 04:22写道:
> > >
> > > 2023-09-22 16:12 (UTC+0800), Fengnan Chang:
> > > > ping
> > > >
> > > > Fengnan Chang <changfengnan@bytedance.com> 于2023年9月12日周二 17:05写道:
> > > > >
> > > > > Let's look at this path:
> > > > > malloc_elem_free
> > > > >    ->malloc_elem_join_adjacent_free
> > > > >       ->join_elem(elem, elem->next)
> > > > >
> > > > > 0. cur elem's pad > 0
> > > > > 1. data area memset in malloc_elem_free first.
> > > > > 2. next elem is free, try to join cur elem and next.
> > > > > 3. in join_elem, try to modify inner->size, this address had
> > > > > memset in step 1, it casue the content of addrees become non-zero.
> > > > >
> > > > > If user call rte_zmalloc, and pick this elem, it can't get all
> > > > > zero'd memory.
> > >
> > > malloc_elem_join_adjacent_free() always calls memset() after
> join_elem(),
> > > for the next and the previous element respectively.
> > when try to call join_elem() for the next element in
> > malloc_elem_join_adjacent_free(),
> > the memset is try to memset *next* element, but join_elem() is update
> > *current* element's
> > content, which shoudn't happen, it's two different element.
> >
> > > How to reproduce this bug?
> > when I test this patch,
> >
> https://patches.dpdk.org/project/dpdk/patch/20230831111937.60975-1-changfengnan@bytedance.com/
> > I have a case try to alloc 64/128/192 size object and free with 16
> threads,
> > after every
> > alloc I'll check wheather all content is 0 or not.
> > It's not easy to reproduce, you can have a try, it's easier to find
> > this problem in code level.
>
> I tried to make a test that would reproduce the problem but it did not.
>
> diff --git a/app/test/test_malloc.c b/app/test/test_malloc.c
> index cd579c503cf5..cfd45d6a28eb 100644
> --- a/app/test/test_malloc.c
> +++ b/app/test/test_malloc.c
> @@ -28,6 +28,7 @@
>  #include <rte_string_fns.h>
>
>  #define N 10000
> +#define BINS 100
>
>  static int
>  is_mem_on_socket(int32_t socket);
> @@ -69,13 +70,24 @@ is_aligned(void *p, int align)
>         return 1;
>  }
>
> +static bool is_all_zero(uint8_t *mem, size_t sz)
> +{
> +       size_t i;
> +
> +       for (i = 0; i < sz; i++)
> +               if (mem[i] != 0)
> +                       return false;
> +
> +       return true;
> +}
> +
>  static int
>  test_align_overlap_per_lcore(__rte_unused void *arg)
>  {
>         const unsigned align1 = 8,
>                         align2 = 64,
>                         align3 = 2048;
> -       unsigned i,j;
> +       unsigned int i;
>         void *p1 = NULL, *p2 = NULL, *p3 = NULL;
>         int ret = 0;
>
> @@ -86,11 +98,12 @@ test_align_overlap_per_lcore(__rte_unused void *arg)
>                         ret = -1;
>                         break;
>                 }
> -               for(j = 0; j < 1000 ; j++) {
> -                       if( *(char *)p1 != 0) {
> -                               printf("rte_zmalloc didn't zero the
> allocated memory\n");
> -                               ret = -1;
> -                       }
> +
> +               if (!is_all_zero(p1, 1000)) {
> +                       printf("rte_zmalloc didn't zero the allocated
> memory\n");
> +                       ret = -1;
> +                       rte_free(p1);
> +                       break;
>                 }
>                 p2 = rte_malloc("dummy", 1000, align2);
>                 if (!p2){
> @@ -140,6 +153,66 @@ test_align_overlap_per_lcore(__rte_unused void *arg)
>         return ret;
>  }
>
> +/*
> + * Allocate random size chunks and make sure that they are
> + * always zero.
> + */
> +static int
> +test_zmalloc(__rte_unused void *arg)
> +{
> +       unsigned int i, n;
> +       void *slots[BINS] = { };
> +                       void *p1;
> +       size_t sz;
> +
> +       /* Allocate many variable size chunks */
> +       for (i = 0; i < BINS; i++) {
> +               sz = rte_rand_max(1024) + 1;
> +               p1 = rte_zmalloc("slots", sz, 0);
> +               if (p1 == NULL) {
> +                       printf("rte_zmalloc(%zu) returned NULL (i=%u)\n",
> sz, i);
> +                       goto fail;
> +               }
> +               slots[i] = p1;
> +               if (!is_all_zero(p1, sz))
> +                       goto fail;
> +       }
> +
> +       /* Drop one chunk per iteration */
> +       for (n = BINS; n > 0; n--) {
> +               /* Swap in a new block into a slot */
> +               for (i = 0; i < N; i++) {
> +                       unsigned int bin = rte_rand_max(n);
> +
> +                       sz = rte_rand_max(1024) + 1;
> +                       p1 = rte_zmalloc("swap", sz, 0);
> +                       if (!p1){
> +                               printf("rte_zmalloc(%zu) returned NULL
> (i=%u)\n", sz, i);
> +                               goto fail;
> +                       }
> +
> +                       if (!is_all_zero(p1, sz)) {
> +                               printf("rte_zmalloc didn't zero the
> allocated memory\n");
> +                               goto fail;
> +                       }
> +
> +                       rte_free(slots[bin]);
> +                       slots[bin] = p1;
> +               }
> +
> +               /* Drop last bin */
> +               rte_free(slots[n]);
> +               slots[n] = NULL;
> +       }
> +
> +       return 0;
> +fail:
> +       for (i = 0; i < BINS; i++)
> +               rte_free(slots[i]);
> +
> +       return -1;
> +}
> +
>  static int
>  test_reordered_free_per_lcore(__rte_unused void *arg)
>  {
> @@ -1020,6 +1091,21 @@ test_malloc(void)
>         }
>         else printf("test_realloc() passed\n");
>
> +       /*----------------------------*/
> +       RTE_LCORE_FOREACH_WORKER(lcore_id) {
> +               rte_eal_remote_launch(test_zmalloc, NULL, lcore_id);
> +       }
> +
> +       RTE_LCORE_FOREACH_WORKER(lcore_id) {
> +               if (rte_eal_wait_lcore(lcore_id) < 0)
> +                       ret = -1;
> +       }
> +       if (ret < 0){
> +               printf("test_zmalloc() failed\n");
> +               return ret;
> +       }
> +       else printf("test_zmalloc() passed\n");
> +
>         /*----------------------------*/
>         RTE_LCORE_FOREACH_WORKER(lcore_id) {
>                 rte_eal_remote_launch(test_align_overlap_per_lcore, NULL,
> lcore_id);
>

[-- Attachment #2: Type: text/html, Size: 9471 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-10-30 12:31 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-12  9:04 [PATCH] eal: fix modify data area after memset Fengnan Chang
2023-09-22  8:12 ` Fengnan Chang
2023-10-22 20:22   ` Dmitry Kozlyuk
2023-10-23  9:07     ` [External] " Fengnan Chang
2023-10-25 16:03       ` Stephen Hemminger
2023-10-30 12:31         ` Fengnan Chang
2023-10-17 13:32 ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).