From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sergio Gonzalez Monroy Subject: Re: [PATCH v2] Correctly handle malloc_elem resize with padding Date: Tue, 6 Jun 2017 15:18:39 +0100 Message-ID: <3b80fa21-3d3e-73ae-235b-7581db5e8670@intel.com> References: <1496189340-27813-1-git-send-email-lavignen@amazon.com> <1496189818-2307-1-git-send-email-lavignen@amazon.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit To: "dev@dpdk.org" , Jamie Lavigne Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id EDF07397D for ; Tue, 6 Jun 2017 16:18:41 +0200 (CEST) In-Reply-To: <1496189818-2307-1-git-send-email-lavignen@amazon.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Jamie, On 31/05/2017 01:16, Jamie Lavigne wrote: > Currently when a malloc_elem is split after resizing, any padding > present in the elem is ignored. This causes the resized elem to be too > small when padding is present, and user data can overwrite the beginning > of the following malloc_elem. > > Solve this by including the size of the padding when computing where to > split the malloc_elem. Nice catch! Could you please rework commit format a bit: - Add 'mem:' as prefix in your patch title - I would mention in the title that this is a fix - Provide 'Fixes' line in commit message > Signed-off-by: Jamie Lavigne > --- > lib/librte_eal/common/malloc_elem.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/lib/librte_eal/common/malloc_elem.c b/lib/librte_eal/common/malloc_elem.c > index 42568e1..8766fa8 100644 > --- a/lib/librte_eal/common/malloc_elem.c > +++ b/lib/librte_eal/common/malloc_elem.c > @@ -333,9 +333,11 @@ malloc_elem_resize(struct malloc_elem *elem, size_t size) > elem_free_list_remove(next); > join_elem(elem, next); > > - if (elem->size - new_size >= MIN_DATA_SIZE + MALLOC_ELEM_OVERHEAD){ > + const size_t new_total_size = new_size + elem->pad; > + > + if (elem->size - new_total_size >= MIN_DATA_SIZE + MALLOC_ELEM_OVERHEAD) { > /* now we have a big block together. Lets cut it down a bit, by splitting */ > - struct malloc_elem *split_pt = RTE_PTR_ADD(elem, new_size); > + struct malloc_elem *split_pt = RTE_PTR_ADD(elem, new_total_size); > split_pt = RTE_PTR_ALIGN_CEIL(split_pt, RTE_CACHE_LINE_SIZE); > split_elem(elem, split_pt); > malloc_elem_free_list_insert(split_pt); This indeed fixes the issue you have mentioned. I was thinking of the following fix instead: - Add elem->pad to new_size - Remove current_size var and instead use elem->size I think those changes should have the same result while removing a couple of vars from the function, which I hope would be easier to read. What do you think? Thanks, Sergio