diff mbox series

malloc: fix memory element size in case of padding

Message ID 1574346302-1263-1-git-send-email-xuemingl@mellanox.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series malloc: fix memory element size in case of padding |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-compilation success Compile Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation fail Compilation issues
ci/travis-robot success Travis build: passed

Commit Message

Xueming Li Nov. 21, 2019, 2:25 p.m. UTC 
  This patch fixes wrong inner memory element size when joining two
elements.

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

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

Comments

Anatoly Burakov Nov. 21, 2019, 3:14 p.m. UTC | #1 
On 21-Nov-19 2:25 PM, Xueming Li wrote:
> This patch fixes wrong inner memory element size when joining two
> elements.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> ---
>   lib/librte_eal/common/malloc_elem.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/lib/librte_eal/common/malloc_elem.c b/lib/librte_eal/common/malloc_elem.c
> index afacb1813c..885d00424b 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 = elem1->size - elem1->pad;
> +	}
>   }
>   
>   struct malloc_elem *
> 

Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>
Thomas Monjalon Nov. 25, 2019, 11:24 p.m. UTC | #2 
21/11/2019 16:14, Burakov, Anatoly:
> On 21-Nov-19 2:25 PM, Xueming Li wrote:
> > This patch fixes wrong inner memory element size when joining two
> > elements.
> > 
> > Fixes: af75078fece3 ("first public release")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> > ---
> > --- 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 = elem1->size - elem1->pad;
> > +	}
> >   }
> 
> Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>

I don't understand this patch.
The variable inner is never used.
What am I missing?
Anatoly Burakov Nov. 26, 2019, 12:57 p.m. UTC | #3 
On 25-Nov-19 11:24 PM, Thomas Monjalon wrote:
> 21/11/2019 16:14, Burakov, Anatoly:
>> On 21-Nov-19 2:25 PM, Xueming Li wrote:
>>> This patch fixes wrong inner memory element size when joining two
>>> elements.
>>>
>>> Fixes: af75078fece3 ("first public release")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
>>> ---
>>> --- 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 = elem1->size - elem1->pad;
>>> +	}
>>>    }
>>
>> Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> I don't understand this patch.
> The variable inner is never used.
> What am I missing?
> 

For padded elements, malloc element has two headers - the "outer" header 
with empty space after it, and the "inner" header, after which the user 
memory actually starts. This makes it so that, when joining elements, if 
the outer element had a pad, we also update the inner element size to match.
Thomas Monjalon Nov. 26, 2019, 1:30 p.m. UTC | #4 
26/11/2019 13:57, Burakov, Anatoly:
> On 25-Nov-19 11:24 PM, Thomas Monjalon wrote:
> > 21/11/2019 16:14, Burakov, Anatoly:
> >> On 21-Nov-19 2:25 PM, Xueming Li wrote:
> >>> This patch fixes wrong inner memory element size when joining two
> >>> elements.
> >>>
> >>> Fixes: af75078fece3 ("first public release")
> >>> Cc: stable@dpdk.org
> >>>
> >>> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> >>> ---
> >>> --- 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 = elem1->size - elem1->pad;
> >>> +	}
> >>>    }
> >>
> >> Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > 
> > I don't understand this patch.
> > The variable inner is never used.
> > What am I missing?
> > 
> 
> For padded elements, malloc element has two headers - the "outer" header 
> with empty space after it, and the "inner" header, after which the user 
> memory actually starts. This makes it so that, when joining elements, if 
> the outer element had a pad, we also update the inner element size to match.

Where the variable "inner" is used in this function?
Xueming Li Nov. 26, 2019, 1:39 p.m. UTC | #5 
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, November 26, 2019 9:30 PM
> To: Burakov, Anatoly <anatoly.burakov@intel.com>
> Cc: Xueming(Steven) Li <xuemingl@mellanox.com>; dev@dpdk.org; Asaf
> Penso <asafp@mellanox.com>; stable@dpdk.org;
> david.marchand@redhat.com
> Subject: Re: [dpdk-dev] [PATCH] malloc: fix memory element size in case of
> padding
> 
> 26/11/2019 13:57, Burakov, Anatoly:
> > On 25-Nov-19 11:24 PM, Thomas Monjalon wrote:
> > > 21/11/2019 16:14, Burakov, Anatoly:
> > >> On 21-Nov-19 2:25 PM, Xueming Li wrote:
> > >>> This patch fixes wrong inner memory element size when joining two
> > >>> elements.
> > >>>
> > >>> Fixes: af75078fece3 ("first public release")
> > >>> Cc: stable@dpdk.org
> > >>>
> > >>> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> > >>> ---
> > >>> --- 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 = elem1->size - elem1->pad;
> > >>> +	}
> > >>>    }
> > >>
> > >> Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > >
> > > I don't understand this patch.
> > > The variable inner is never used.
> > > What am I missing?
> > >
> >
> > For padded elements, malloc element has two headers - the "outer"
> > header with empty space after it, and the "inner" header, after which
> > the user memory actually starts. This makes it so that, when joining
> > elements, if the outer element had a pad, we also update the inner
> element size to match.
> 
> Where the variable "inner" is used in this function?
> 
Rte_realloc, inner size is used to copy data.
Thomas Monjalon Nov. 26, 2019, 1:45 p.m. UTC | #6 
26/11/2019 14:39, Xueming(Steven) Li:
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Tuesday, November 26, 2019 9:30 PM
> > To: Burakov, Anatoly <anatoly.burakov@intel.com>
> > Cc: Xueming(Steven) Li <xuemingl@mellanox.com>; dev@dpdk.org; Asaf
> > Penso <asafp@mellanox.com>; stable@dpdk.org;
> > david.marchand@redhat.com
> > Subject: Re: [dpdk-dev] [PATCH] malloc: fix memory element size in case of
> > padding
> > 
> > 26/11/2019 13:57, Burakov, Anatoly:
> > > On 25-Nov-19 11:24 PM, Thomas Monjalon wrote:
> > > > 21/11/2019 16:14, Burakov, Anatoly:
> > > >> On 21-Nov-19 2:25 PM, Xueming Li wrote:
> > > >>> This patch fixes wrong inner memory element size when joining two
> > > >>> elements.
> > > >>>
> > > >>> Fixes: af75078fece3 ("first public release")
> > > >>> Cc: stable@dpdk.org
> > > >>>
> > > >>> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> > > >>> ---
> > > >>> --- 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 = elem1->size - elem1->pad;
> > > >>> +	}
> > > >>>    }
> > > >>
> > > >> Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > > >
> > > > I don't understand this patch.
> > > > The variable inner is never used.
> > > > What am I missing?
> > > >
> > >
> > > For padded elements, malloc element has two headers - the "outer"
> > > header with empty space after it, and the "inner" header, after which
> > > the user memory actually starts. This makes it so that, when joining
> > > elements, if the outer element had a pad, we also update the inner
> > element size to match.
> > 
> > Where the variable "inner" is used in this function?
> > 
> Rte_realloc, inner size is used to copy data.

I still don't get it. Am I missing half of the patch?
Please give explicit line number.
Anatoly Burakov Nov. 26, 2019, 2:55 p.m. UTC | #7 
On 26-Nov-19 1:45 PM, Thomas Monjalon wrote:
> 26/11/2019 14:39, Xueming(Steven) Li:
>>
>>> -----Original Message-----
>>> From: Thomas Monjalon <thomas@monjalon.net>
>>> Sent: Tuesday, November 26, 2019 9:30 PM
>>> To: Burakov, Anatoly <anatoly.burakov@intel.com>
>>> Cc: Xueming(Steven) Li <xuemingl@mellanox.com>; dev@dpdk.org; Asaf
>>> Penso <asafp@mellanox.com>; stable@dpdk.org;
>>> david.marchand@redhat.com
>>> Subject: Re: [dpdk-dev] [PATCH] malloc: fix memory element size in case of
>>> padding
>>>
>>> 26/11/2019 13:57, Burakov, Anatoly:
>>>> On 25-Nov-19 11:24 PM, Thomas Monjalon wrote:
>>>>> 21/11/2019 16:14, Burakov, Anatoly:
>>>>>> On 21-Nov-19 2:25 PM, Xueming Li wrote:
>>>>>>> This patch fixes wrong inner memory element size when joining two
>>>>>>> elements.
>>>>>>>
>>>>>>> Fixes: af75078fece3 ("first public release")
>>>>>>> Cc: stable@dpdk.org
>>>>>>>
>>>>>>> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
>>>>>>> ---
>>>>>>> --- 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 = elem1->size - elem1->pad;
>>>>>>> +	}
>>>>>>>     }
>>>>>>
>>>>>> Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>>>>
>>>>> I don't understand this patch.
>>>>> The variable inner is never used.
>>>>> What am I missing?
>>>>>
>>>>
>>>> For padded elements, malloc element has two headers - the "outer"
>>>> header with empty space after it, and the "inner" header, after which
>>>> the user memory actually starts. This makes it so that, when joining
>>>> elements, if the outer element had a pad, we also update the inner
>>> element size to match.
>>>
>>> Where the variable "inner" is used in this function?

Right on the next line after it is created :)
Thomas Monjalon Nov. 26, 2019, 3:01 p.m. UTC | #8 
26/11/2019 14:45, Thomas Monjalon:
> 26/11/2019 14:39, Xueming(Steven) Li:
> > 
> > > -----Original Message-----
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > Sent: Tuesday, November 26, 2019 9:30 PM
> > > To: Burakov, Anatoly <anatoly.burakov@intel.com>
> > > Cc: Xueming(Steven) Li <xuemingl@mellanox.com>; dev@dpdk.org; Asaf
> > > Penso <asafp@mellanox.com>; stable@dpdk.org;
> > > david.marchand@redhat.com
> > > Subject: Re: [dpdk-dev] [PATCH] malloc: fix memory element size in case of
> > > padding
> > > 
> > > 26/11/2019 13:57, Burakov, Anatoly:
> > > > On 25-Nov-19 11:24 PM, Thomas Monjalon wrote:
> > > > > 21/11/2019 16:14, Burakov, Anatoly:
> > > > >> On 21-Nov-19 2:25 PM, Xueming Li wrote:
> > > > >>> This patch fixes wrong inner memory element size when joining two
> > > > >>> elements.
> > > > >>>
> > > > >>> Fixes: af75078fece3 ("first public release")
> > > > >>> Cc: stable@dpdk.org
> > > > >>>
> > > > >>> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> > > > >>> ---
> > > > >>> --- 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 = elem1->size - elem1->pad;
> > > > >>> +	}
> > > > >>>    }
> > > > >>
> > > > >> Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > > > >
> > > > > I don't understand this patch.
> > > > > The variable inner is never used.
> > > > > What am I missing?
> > > > >
> > > >
> > > > For padded elements, malloc element has two headers - the "outer"
> > > > header with empty space after it, and the "inner" header, after which
> > > > the user memory actually starts. This makes it so that, when joining
> > > > elements, if the outer element had a pad, we also update the inner
> > > element size to match.
> > > 
> > > Where the variable "inner" is used in this function?
> > > 
> > Rte_realloc, inner size is used to copy data.
> 
> I still don't get it. Am I missing half of the patch?
> Please give explicit line number.

OK after a quick chat, I understood my miss:
inner is a pointer helping to reach the size field.
Sorry for the noise.
Thomas Monjalon Nov. 26, 2019, 3:26 p.m. UTC | #9 
21/11/2019 16:14, Burakov, Anatoly:
> On 21-Nov-19 2:25 PM, Xueming Li wrote:
> > This patch fixes wrong inner memory element size when joining two
> > elements.
> > 
> > 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>

Applied, thanks
diff mbox series

Patch

diff --git a/lib/librte_eal/common/malloc_elem.c b/lib/librte_eal/common/malloc_elem.c
index afacb1813c..885d00424b 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 = elem1->size - elem1->pad;
+	}
 }
 
 struct malloc_elem *