DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/2] malloc: fix realloc wrong copy size
@ 2019-11-12 14:50 Xueming Li
  2019-11-12 14:50 ` [dpdk-dev] [PATCH 2/2] malloc: fix realloc padded element size Xueming Li
  2019-11-14 15:11 ` [dpdk-dev] [PATCH 1/2] malloc: fix realloc wrong copy size Burakov, Anatoly
  0 siblings, 2 replies; 12+ messages in thread
From: Xueming Li @ 2019-11-12 14:50 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: Asaf Penso, dev, stable

In rte_realloc, if the old element has pad and need to allocate a new
memory, the padding size was not deducted, so more data was copied to
new data area.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
 lib/librte_eal/common/rte_malloc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/rte_malloc.c b/lib/librte_eal/common/rte_malloc.c
index 413e4aa004..d6026a2b17 100644
--- a/lib/librte_eal/common/rte_malloc.c
+++ b/lib/librte_eal/common/rte_malloc.c
@@ -150,7 +150,8 @@ rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
 	void *new_ptr = rte_malloc_socket(NULL, size, align, socket);
 	if (new_ptr == NULL)
 		return NULL;
-	const unsigned old_size = elem->size - MALLOC_ELEM_OVERHEAD;
+	/* elem: |pad|data_elem|data|trailer| */
+	const size_t old_size = elem->size - elem->pad - MALLOC_ELEM_OVERHEAD;
 	rte_memcpy(new_ptr, ptr, old_size < size ? old_size : size);
 	rte_free(ptr);
 
-- 
2.17.1


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

* [dpdk-dev] [PATCH 2/2] malloc: fix realloc padded element size
  2019-11-12 14:50 [dpdk-dev] [PATCH 1/2] malloc: fix realloc wrong copy size Xueming Li
@ 2019-11-12 14:50 ` Xueming Li
  2019-11-14 15:11   ` Burakov, Anatoly
  2019-11-19 20:47   ` [dpdk-dev] [dpdk-stable] " David Marchand
  2019-11-14 15:11 ` [dpdk-dev] [PATCH 1/2] malloc: fix realloc wrong copy size Burakov, Anatoly
  1 sibling, 2 replies; 12+ messages in thread
From: Xueming Li @ 2019-11-12 14:50 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: Asaf Penso, dev, stable

When resize a memory with next element, the original element size grows.
If the orginal element has padding, the real inner element size didn't
grow as well and this causes trailer verification failure when malloc
debug enabled.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
 lib/librte_eal/common/malloc_elem.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/librte_eal/common/malloc_elem.c b/lib/librte_eal/common/malloc_elem.c
index 658c9b5b79..afacb1813c 100644
--- a/lib/librte_eal/common/malloc_elem.c
+++ b/lib/librte_eal/common/malloc_elem.c
@@ -307,6 +307,11 @@ split_elem(struct malloc_elem *elem, struct malloc_elem *split_pt)
 	elem->next = split_pt;
 	elem->size = old_elem_size;
 	set_trailer(elem);
+	if (elem->pad) {
+		/* Update inner padding inner element size. */
+		elem = RTE_PTR_ADD(elem, elem->pad);
+		elem->size = old_elem_size - elem->pad;
+	}
 }
 
 /*
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH 1/2] malloc: fix realloc wrong copy size
  2019-11-12 14:50 [dpdk-dev] [PATCH 1/2] malloc: fix realloc wrong copy size Xueming Li
  2019-11-12 14:50 ` [dpdk-dev] [PATCH 2/2] malloc: fix realloc padded element size Xueming Li
@ 2019-11-14 15:11 ` Burakov, Anatoly
  1 sibling, 0 replies; 12+ messages in thread
From: Burakov, Anatoly @ 2019-11-14 15:11 UTC (permalink / raw)
  To: Xueming Li; +Cc: Asaf Penso, dev, stable

On 12-Nov-19 2:50 PM, Xueming Li wrote:
> In rte_realloc, if the old element has pad and need to allocate a new
> memory, the padding size was not deducted, so more data was copied to
> new data area.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> ---

Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH 2/2] malloc: fix realloc padded element size
  2019-11-12 14:50 ` [dpdk-dev] [PATCH 2/2] malloc: fix realloc padded element size Xueming Li
@ 2019-11-14 15:11   ` Burakov, Anatoly
  2019-11-19 20:47   ` [dpdk-dev] [dpdk-stable] " David Marchand
  1 sibling, 0 replies; 12+ messages in thread
From: Burakov, Anatoly @ 2019-11-14 15:11 UTC (permalink / raw)
  To: Xueming Li; +Cc: Asaf Penso, dev, stable

On 12-Nov-19 2:50 PM, Xueming Li wrote:
> When resize a memory with next element, the original element size grows.
> If the orginal element has padding, the real inner element size didn't
> grow as well and this causes trailer verification failure when malloc
> debug enabled.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> ---

Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded element size
  2019-11-12 14:50 ` [dpdk-dev] [PATCH 2/2] malloc: fix realloc padded element size Xueming Li
  2019-11-14 15:11   ` Burakov, Anatoly
@ 2019-11-19 20:47   ` David Marchand
  2019-11-20  2:12     ` Xueming(Steven) Li
  1 sibling, 1 reply; 12+ messages in thread
From: David Marchand @ 2019-11-19 20:47 UTC (permalink / raw)
  To: Xueming Li, Anatoly Burakov; +Cc: Asaf Penso, dev, dpdk stable

On Tue, Nov 12, 2019 at 3:50 PM Xueming Li <xuemingl@mellanox.com> wrote:
>
> When resize a memory with next element, the original element size grows.
> If the orginal element has padding, the real inner element size didn't
> grow as well and this causes trailer verification failure when malloc
> debug enabled.

I did not see this when running the malloc_autotest with debug enabled.
What is missing for me to catch it?


Just a bit chilly to apply this series.
The first patch seems an optimisation.
The second one seems more interesting if we fix the debug mode, but I
suppose we can live without them in 19.11.


-- 
David Marchand


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

* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded element size
  2019-11-19 20:47   ` [dpdk-dev] [dpdk-stable] " David Marchand
@ 2019-11-20  2:12     ` Xueming(Steven) Li
  2019-11-20 13:25       ` David Marchand
  0 siblings, 1 reply; 12+ messages in thread
From: Xueming(Steven) Li @ 2019-11-20  2:12 UTC (permalink / raw)
  To: David Marchand, Anatoly Burakov; +Cc: Asaf Penso, dev, dpdk stable

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Wednesday, November 20, 2019 4:47 AM
> To: Xueming(Steven) Li <xuemingl@mellanox.com>; Anatoly Burakov
> <anatoly.burakov@intel.com>
> Cc: Asaf Penso <asafp@mellanox.com>; dev <dev@dpdk.org>; dpdk stable
> <stable@dpdk.org>
> Subject: Re: [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded element
> size
> 
> On Tue, Nov 12, 2019 at 3:50 PM Xueming Li <xuemingl@mellanox.com>
> wrote:
> >
> > When resize a memory with next element, the original element size grows.
> > If the orginal element has padding, the real inner element size didn't
> > grow as well and this causes trailer verification failure when malloc
> > debug enabled.
> 
> I did not see this when running the malloc_autotest with debug enabled.
> What is missing for me to catch it?
> 
Yes, it happens rarely, depends on memory fragment. I only caught this in middle of a long test.

> 
> Just a bit chilly to apply this series.
> The first patch seems an optimisation.
> The second one seems more interesting if we fix the debug mode, but I
> suppose we can live without them in 19.11.
Few people enable memory debug option, they are there for years.

> 
> 
> --
> David Marchand


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

* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded element size
  2019-11-20  2:12     ` Xueming(Steven) Li
@ 2019-11-20 13:25       ` David Marchand
  2019-11-21 12:30         ` Burakov, Anatoly
  0 siblings, 1 reply; 12+ messages in thread
From: David Marchand @ 2019-11-20 13:25 UTC (permalink / raw)
  To: Xueming(Steven) Li; +Cc: Anatoly Burakov, Asaf Penso, dev, dpdk stable

On Wed, Nov 20, 2019 at 3:12 AM Xueming(Steven) Li
<xuemingl@mellanox.com> wrote:
>
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Wednesday, November 20, 2019 4:47 AM
> > To: Xueming(Steven) Li <xuemingl@mellanox.com>; Anatoly Burakov
> > <anatoly.burakov@intel.com>
> > Cc: Asaf Penso <asafp@mellanox.com>; dev <dev@dpdk.org>; dpdk stable
> > <stable@dpdk.org>
> > Subject: Re: [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded element
> > size
> >
> > On Tue, Nov 12, 2019 at 3:50 PM Xueming Li <xuemingl@mellanox.com>
> > wrote:
> > >
> > > When resize a memory with next element, the original element size grows.
> > > If the orginal element has padding, the real inner element size didn't
> > > grow as well and this causes trailer verification failure when malloc
> > > debug enabled.
> >
> > I did not see this when running the malloc_autotest with debug enabled.
> > What is missing for me to catch it?
> >
> Yes, it happens rarely, depends on memory fragment. I only caught this in middle of a long test.
>
> >
> > Just a bit chilly to apply this series.
> > The first patch seems an optimisation.
> > The second one seems more interesting if we fix the debug mode, but I
> > suppose we can live without them in 19.11.
> Few people enable memory debug option, they are there for years.

Had a discussion offlist with Anatoly.
Those two issues are hard to catch but the fixes are relevant and
Anatoly is confident.
I will take this in rc3.

Series applied, thanks.

-- 
David Marchand


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

* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded element size
  2019-11-20 13:25       ` David Marchand
@ 2019-11-21 12:30         ` Burakov, Anatoly
  2019-11-21 12:55           ` Xueming(Steven) Li
  0 siblings, 1 reply; 12+ messages in thread
From: Burakov, Anatoly @ 2019-11-21 12:30 UTC (permalink / raw)
  To: David Marchand, Xueming(Steven) Li; +Cc: Asaf Penso, dev, dpdk stable

On 20-Nov-19 1:25 PM, David Marchand wrote:
> On Wed, Nov 20, 2019 at 3:12 AM Xueming(Steven) Li
> <xuemingl@mellanox.com> wrote:
>>
>>> -----Original Message-----
>>> From: David Marchand <david.marchand@redhat.com>
>>> Sent: Wednesday, November 20, 2019 4:47 AM
>>> To: Xueming(Steven) Li <xuemingl@mellanox.com>; Anatoly Burakov
>>> <anatoly.burakov@intel.com>
>>> Cc: Asaf Penso <asafp@mellanox.com>; dev <dev@dpdk.org>; dpdk stable
>>> <stable@dpdk.org>
>>> Subject: Re: [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded element
>>> size
>>>
>>> On Tue, Nov 12, 2019 at 3:50 PM Xueming Li <xuemingl@mellanox.com>
>>> wrote:
>>>>
>>>> When resize a memory with next element, the original element size grows.
>>>> If the orginal element has padding, the real inner element size didn't
>>>> grow as well and this causes trailer verification failure when malloc
>>>> debug enabled.
>>>
>>> I did not see this when running the malloc_autotest with debug enabled.
>>> What is missing for me to catch it?
>>>
>> Yes, it happens rarely, depends on memory fragment. I only caught this in middle of a long test.
>>
>>>
>>> Just a bit chilly to apply this series.
>>> The first patch seems an optimisation.
>>> The second one seems more interesting if we fix the debug mode, but I
>>> suppose we can live without them in 19.11.
>> Few people enable memory debug option, they are there for years.
> 
> Had a discussion offlist with Anatoly.
> Those two issues are hard to catch but the fixes are relevant and
> Anatoly is confident.
> I will take this in rc3.
> 
> Series applied, thanks.
> 

To test them, i had to modify malloc to always create padded elements :)

-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded element size
  2019-11-21 12:30         ` Burakov, Anatoly
@ 2019-11-21 12:55           ` Xueming(Steven) Li
  2019-11-21 13:15             ` Burakov, Anatoly
  0 siblings, 1 reply; 12+ messages in thread
From: Xueming(Steven) Li @ 2019-11-21 12:55 UTC (permalink / raw)
  To: Burakov, Anatoly, David Marchand; +Cc: Asaf Penso, dev, dpdk stable

Hi Anatoly,

> -----Original Message-----
> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> Sent: Thursday, November 21, 2019 8:30 PM
> To: David Marchand <david.marchand@redhat.com>; Xueming(Steven) Li
> <xuemingl@mellanox.com>
> Cc: Asaf Penso <asafp@mellanox.com>; dev <dev@dpdk.org>; dpdk stable
> <stable@dpdk.org>
> Subject: Re: [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded element size
> 
> On 20-Nov-19 1:25 PM, David Marchand wrote:
> > On Wed, Nov 20, 2019 at 3:12 AM Xueming(Steven) Li
> > <xuemingl@mellanox.com> wrote:
> >>
> >>> -----Original Message-----
> >>> From: David Marchand <david.marchand@redhat.com>
> >>> Sent: Wednesday, November 20, 2019 4:47 AM
> >>> To: Xueming(Steven) Li <xuemingl@mellanox.com>; Anatoly Burakov
> >>> <anatoly.burakov@intel.com>
> >>> Cc: Asaf Penso <asafp@mellanox.com>; dev <dev@dpdk.org>; dpdk stable
> >>> <stable@dpdk.org>
> >>> Subject: Re: [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded
> >>> element size
> >>>
> >>> On Tue, Nov 12, 2019 at 3:50 PM Xueming Li <xuemingl@mellanox.com>
> >>> wrote:
> >>>>
> >>>> When resize a memory with next element, the original element size grows.
> >>>> If the orginal element has padding, the real inner element size
> >>>> didn't grow as well and this causes trailer verification failure
> >>>> when malloc debug enabled.
> >>>
> >>> I did not see this when running the malloc_autotest with debug enabled.
> >>> What is missing for me to catch it?
> >>>
> >> Yes, it happens rarely, depends on memory fragment. I only caught this in
> middle of a long test.
> >>
> >>>
> >>> Just a bit chilly to apply this series.
> >>> The first patch seems an optimisation.
> >>> The second one seems more interesting if we fix the debug mode, but
> >>> I suppose we can live without them in 19.11.
> >> Few people enable memory debug option, they are there for years.
> >
> > Had a discussion offlist with Anatoly.
> > Those two issues are hard to catch but the fixes are relevant and
> > Anatoly is confident.
> > I will take this in rc3.
> >
> > Series applied, thanks.
> >
> 
> To test them, i had to modify malloc to always create padded elements :)

I fix another issue in element join, as I made some local enhancement patch on memory, it's required, not sure whether it help on public code.

diff --git a/lib/librte_eal/common/malloc_elem.c b/lib/librte_eal/common/malloc_elem.c
index afacb1813c..c3fa0d1039 100644
--- a/lib/librte_eal/common/malloc_elem.c
+++ b/lib/librte_eal/common/malloc_elem.c
@@ -487,6 +487,10 @@ join_elem(struct malloc_elem *elem1, struct malloc_elem *elem2)
        else
                elem1->heap->last = elem1;
        elem1->next = next;
+       if (elem1->pad) {
+               struct malloc_elem *inner = RTE_PTR_ADD(elem1, elem1->pad);
+               inner->size += elem2->size;
+       }
 }



> 
> --
> Thanks,
> Anatoly

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

* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded element size
  2019-11-21 12:55           ` Xueming(Steven) Li
@ 2019-11-21 13:15             ` Burakov, Anatoly
  2019-11-21 13:17               ` Burakov, Anatoly
  0 siblings, 1 reply; 12+ messages in thread
From: Burakov, Anatoly @ 2019-11-21 13:15 UTC (permalink / raw)
  To: Xueming(Steven) Li, David Marchand; +Cc: Asaf Penso, dev, dpdk stable

On 21-Nov-19 12:55 PM, Xueming(Steven) Li wrote:
> Hi Anatoly,
> 
>> -----Original Message-----
>> From: Burakov, Anatoly <anatoly.burakov@intel.com>
>> Sent: Thursday, November 21, 2019 8:30 PM
>> To: David Marchand <david.marchand@redhat.com>; Xueming(Steven) Li
>> <xuemingl@mellanox.com>
>> Cc: Asaf Penso <asafp@mellanox.com>; dev <dev@dpdk.org>; dpdk stable
>> <stable@dpdk.org>
>> Subject: Re: [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded element size
>>
>> On 20-Nov-19 1:25 PM, David Marchand wrote:
>>> On Wed, Nov 20, 2019 at 3:12 AM Xueming(Steven) Li
>>> <xuemingl@mellanox.com> wrote:
>>>>
>>>>> -----Original Message-----
>>>>> From: David Marchand <david.marchand@redhat.com>
>>>>> Sent: Wednesday, November 20, 2019 4:47 AM
>>>>> To: Xueming(Steven) Li <xuemingl@mellanox.com>; Anatoly Burakov
>>>>> <anatoly.burakov@intel.com>
>>>>> Cc: Asaf Penso <asafp@mellanox.com>; dev <dev@dpdk.org>; dpdk stable
>>>>> <stable@dpdk.org>
>>>>> Subject: Re: [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded
>>>>> element size
>>>>>
>>>>> On Tue, Nov 12, 2019 at 3:50 PM Xueming Li <xuemingl@mellanox.com>
>>>>> wrote:
>>>>>>
>>>>>> When resize a memory with next element, the original element size grows.
>>>>>> If the orginal element has padding, the real inner element size
>>>>>> didn't grow as well and this causes trailer verification failure
>>>>>> when malloc debug enabled.
>>>>>
>>>>> I did not see this when running the malloc_autotest with debug enabled.
>>>>> What is missing for me to catch it?
>>>>>
>>>> Yes, it happens rarely, depends on memory fragment. I only caught this in
>> middle of a long test.
>>>>
>>>>>
>>>>> Just a bit chilly to apply this series.
>>>>> The first patch seems an optimisation.
>>>>> The second one seems more interesting if we fix the debug mode, but
>>>>> I suppose we can live without them in 19.11.
>>>> Few people enable memory debug option, they are there for years.
>>>
>>> Had a discussion offlist with Anatoly.
>>> Those two issues are hard to catch but the fixes are relevant and
>>> Anatoly is confident.
>>> I will take this in rc3.
>>>
>>> Series applied, thanks.
>>>
>>
>> To test them, i had to modify malloc to always create padded elements :)
> 
> I fix another issue in element join, as I made some local enhancement patch on memory, it's required, not sure whether it help on public code.
> 
> diff --git a/lib/librte_eal/common/malloc_elem.c b/lib/librte_eal/common/malloc_elem.c
> index afacb1813c..c3fa0d1039 100644
> --- a/lib/librte_eal/common/malloc_elem.c
> +++ b/lib/librte_eal/common/malloc_elem.c
> @@ -487,6 +487,10 @@ join_elem(struct malloc_elem *elem1, struct malloc_elem *elem2)
>          else
>                  elem1->heap->last = elem1;
>          elem1->next = next;
> +       if (elem1->pad) {
> +               struct malloc_elem *inner = RTE_PTR_ADD(elem1, elem1->pad);
> +               inner->size += elem2->size;
> +       }
>   }
> 
> 

This looks like a good change as well - while we /mostly/ join elements 
when they're free, there is one case where we can join a free element 
with one already occupied - and /that/ element could be padded, which we 
currently don't update. So, this patch would help the public code as well.

> 
>>
>> --
>> Thanks,
>> Anatoly


-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded element size
  2019-11-21 13:15             ` Burakov, Anatoly
@ 2019-11-21 13:17               ` Burakov, Anatoly
  2019-11-21 14:11                 ` Xueming(Steven) Li
  0 siblings, 1 reply; 12+ messages in thread
From: Burakov, Anatoly @ 2019-11-21 13:17 UTC (permalink / raw)
  To: Xueming(Steven) Li, David Marchand; +Cc: Asaf Penso, dev, dpdk stable

On 21-Nov-19 1:15 PM, Burakov, Anatoly wrote:
> On 21-Nov-19 12:55 PM, Xueming(Steven) Li wrote:
>> Hi Anatoly,
>>
>>> -----Original Message-----
>>> From: Burakov, Anatoly <anatoly.burakov@intel.com>
>>> Sent: Thursday, November 21, 2019 8:30 PM
>>> To: David Marchand <david.marchand@redhat.com>; Xueming(Steven) Li
>>> <xuemingl@mellanox.com>
>>> Cc: Asaf Penso <asafp@mellanox.com>; dev <dev@dpdk.org>; dpdk stable
>>> <stable@dpdk.org>
>>> Subject: Re: [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded 
>>> element size
>>>
>>> On 20-Nov-19 1:25 PM, David Marchand wrote:
>>>> On Wed, Nov 20, 2019 at 3:12 AM Xueming(Steven) Li
>>>> <xuemingl@mellanox.com> wrote:
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: David Marchand <david.marchand@redhat.com>
>>>>>> Sent: Wednesday, November 20, 2019 4:47 AM
>>>>>> To: Xueming(Steven) Li <xuemingl@mellanox.com>; Anatoly Burakov
>>>>>> <anatoly.burakov@intel.com>
>>>>>> Cc: Asaf Penso <asafp@mellanox.com>; dev <dev@dpdk.org>; dpdk stable
>>>>>> <stable@dpdk.org>
>>>>>> Subject: Re: [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded
>>>>>> element size
>>>>>>
>>>>>> On Tue, Nov 12, 2019 at 3:50 PM Xueming Li <xuemingl@mellanox.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> When resize a memory with next element, the original element size 
>>>>>>> grows.
>>>>>>> If the orginal element has padding, the real inner element size
>>>>>>> didn't grow as well and this causes trailer verification failure
>>>>>>> when malloc debug enabled.
>>>>>>
>>>>>> I did not see this when running the malloc_autotest with debug 
>>>>>> enabled.
>>>>>> What is missing for me to catch it?
>>>>>>
>>>>> Yes, it happens rarely, depends on memory fragment. I only caught 
>>>>> this in
>>> middle of a long test.
>>>>>
>>>>>>
>>>>>> Just a bit chilly to apply this series.
>>>>>> The first patch seems an optimisation.
>>>>>> The second one seems more interesting if we fix the debug mode, but
>>>>>> I suppose we can live without them in 19.11.
>>>>> Few people enable memory debug option, they are there for years.
>>>>
>>>> Had a discussion offlist with Anatoly.
>>>> Those two issues are hard to catch but the fixes are relevant and
>>>> Anatoly is confident.
>>>> I will take this in rc3.
>>>>
>>>> Series applied, thanks.
>>>>
>>>
>>> To test them, i had to modify malloc to always create padded elements :)
>>
>> I fix another issue in element join, as I made some local enhancement 
>> patch on memory, it's required, not sure whether it help on public code.
>>
>> diff --git a/lib/librte_eal/common/malloc_elem.c 
>> b/lib/librte_eal/common/malloc_elem.c
>> index afacb1813c..c3fa0d1039 100644
>> --- a/lib/librte_eal/common/malloc_elem.c
>> +++ b/lib/librte_eal/common/malloc_elem.c
>> @@ -487,6 +487,10 @@ join_elem(struct malloc_elem *elem1, struct 
>> malloc_elem *elem2)
>>          else
>>                  elem1->heap->last = elem1;
>>          elem1->next = next;
>> +       if (elem1->pad) {
>> +               struct malloc_elem *inner = RTE_PTR_ADD(elem1, 
>> elem1->pad);
>> +               inner->size += elem2->size;
>> +       }
>>   }
>>
>>
> 
> This looks like a good change as well - while we /mostly/ join elements 
> when they're free, there is one case where we can join a free element 
> with one already occupied - and /that/ element could be padded, which we 
> currently don't update. So, this patch would help the public code as well.

(that said, the += should be replaced with explicit calculation of 
correct size, as this joining may be run multiple times, and we don't 
want incorrect size accumulating...)

> 
>>
>>>
>>> -- 
>>> Thanks,
>>> Anatoly
> 
> 


-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded element size
  2019-11-21 13:17               ` Burakov, Anatoly
@ 2019-11-21 14:11                 ` Xueming(Steven) Li
  0 siblings, 0 replies; 12+ messages in thread
From: Xueming(Steven) Li @ 2019-11-21 14:11 UTC (permalink / raw)
  To: Burakov, Anatoly, David Marchand; +Cc: Asaf Penso, dev, dpdk stable



> -----Original Message-----
> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> Sent: Thursday, November 21, 2019 9:17 PM
> To: Xueming(Steven) Li <xuemingl@mellanox.com>; David Marchand
> <david.marchand@redhat.com>
> Cc: Asaf Penso <asafp@mellanox.com>; dev <dev@dpdk.org>; dpdk stable
> <stable@dpdk.org>
> Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded
> element size
> 
> On 21-Nov-19 1:15 PM, Burakov, Anatoly wrote:
> > On 21-Nov-19 12:55 PM, Xueming(Steven) Li wrote:
> >> Hi Anatoly,
> >>
> >>> -----Original Message-----
> >>> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> >>> Sent: Thursday, November 21, 2019 8:30 PM
> >>> To: David Marchand <david.marchand@redhat.com>; Xueming(Steven)
> Li
> >>> <xuemingl@mellanox.com>
> >>> Cc: Asaf Penso <asafp@mellanox.com>; dev <dev@dpdk.org>; dpdk
> stable
> >>> <stable@dpdk.org>
> >>> Subject: Re: [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded
> >>> element size
> >>>
> >>> On 20-Nov-19 1:25 PM, David Marchand wrote:
> >>>> On Wed, Nov 20, 2019 at 3:12 AM Xueming(Steven) Li
> >>>> <xuemingl@mellanox.com> wrote:
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: David Marchand <david.marchand@redhat.com>
> >>>>>> Sent: Wednesday, November 20, 2019 4:47 AM
> >>>>>> To: Xueming(Steven) Li <xuemingl@mellanox.com>; Anatoly
> Burakov
> >>>>>> <anatoly.burakov@intel.com>
> >>>>>> Cc: Asaf Penso <asafp@mellanox.com>; dev <dev@dpdk.org>; dpdk
> >>>>>> stable <stable@dpdk.org>
> >>>>>> Subject: Re: [dpdk-stable] [PATCH 2/2] malloc: fix realloc padded
> >>>>>> element size
> >>>>>>
> >>>>>> On Tue, Nov 12, 2019 at 3:50 PM Xueming Li
> >>>>>> <xuemingl@mellanox.com>
> >>>>>> wrote:
> >>>>>>>
> >>>>>>> When resize a memory with next element, the original element
> >>>>>>> size grows.
> >>>>>>> If the orginal element has padding, the real inner element size
> >>>>>>> didn't grow as well and this causes trailer verification failure
> >>>>>>> when malloc debug enabled.
> >>>>>>
> >>>>>> I did not see this when running the malloc_autotest with debug
> >>>>>> enabled.
> >>>>>> What is missing for me to catch it?
> >>>>>>
> >>>>> Yes, it happens rarely, depends on memory fragment. I only caught
> >>>>> this in
> >>> middle of a long test.
> >>>>>
> >>>>>>
> >>>>>> Just a bit chilly to apply this series.
> >>>>>> The first patch seems an optimisation.
> >>>>>> The second one seems more interesting if we fix the debug mode,
> >>>>>> but I suppose we can live without them in 19.11.
> >>>>> Few people enable memory debug option, they are there for years.
> >>>>
> >>>> Had a discussion offlist with Anatoly.
> >>>> Those two issues are hard to catch but the fixes are relevant and
> >>>> Anatoly is confident.
> >>>> I will take this in rc3.
> >>>>
> >>>> Series applied, thanks.
> >>>>
> >>>
> >>> To test them, i had to modify malloc to always create padded
> >>> elements :)
> >>
> >> I fix another issue in element join, as I made some local enhancement
> >> patch on memory, it's required, not sure whether it help on public code.
> >>
> >> diff --git a/lib/librte_eal/common/malloc_elem.c
> >> b/lib/librte_eal/common/malloc_elem.c
> >> index afacb1813c..c3fa0d1039 100644
> >> --- a/lib/librte_eal/common/malloc_elem.c
> >> +++ b/lib/librte_eal/common/malloc_elem.c
> >> @@ -487,6 +487,10 @@ join_elem(struct malloc_elem *elem1, struct
> >> malloc_elem *elem2)
> >>          else
> >>                  elem1->heap->last = elem1;
> >>          elem1->next = next;
> >> +       if (elem1->pad) {
> >> +               struct malloc_elem *inner = RTE_PTR_ADD(elem1,
> >> elem1->pad);
> >> +               inner->size += elem2->size;
> >> +       }
> >>   }
> >>
> >>
> >
> > This looks like a good change as well - while we /mostly/ join
> > elements when they're free, there is one case where we can join a free
> > element with one already occupied - and /that/ element could be
> > padded, which we currently don't update. So, this patch would help the
> public code as well.

Correct, it happens in rte_realloc.

> 
> (that said, the += should be replaced with explicit calculation of correct size,
> as this joining may be run multiple times, and we don't want incorrect size
> accumulating...)

I'll update and send a new patch.

> 
> >
> >>
> >>>
> >>> --
> >>> Thanks,
> >>> Anatoly
> >
> >
> 
> 
> --
> Thanks,
> Anatoly

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

end of thread, other threads:[~2019-11-21 14:12 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-12 14:50 [dpdk-dev] [PATCH 1/2] malloc: fix realloc wrong copy size Xueming Li
2019-11-12 14:50 ` [dpdk-dev] [PATCH 2/2] malloc: fix realloc padded element size Xueming Li
2019-11-14 15:11   ` Burakov, Anatoly
2019-11-19 20:47   ` [dpdk-dev] [dpdk-stable] " David Marchand
2019-11-20  2:12     ` Xueming(Steven) Li
2019-11-20 13:25       ` David Marchand
2019-11-21 12:30         ` Burakov, Anatoly
2019-11-21 12:55           ` Xueming(Steven) Li
2019-11-21 13:15             ` Burakov, Anatoly
2019-11-21 13:17               ` Burakov, Anatoly
2019-11-21 14:11                 ` Xueming(Steven) Li
2019-11-14 15:11 ` [dpdk-dev] [PATCH 1/2] malloc: fix realloc wrong copy size Burakov, Anatoly

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