From mboxrd@z Thu Jan 1 00:00:00 1970 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) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Jun 2017 07:18:40 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,306,1493708400"; d="scan'208";a="977395138" Received: from smonroyx-mobl.ger.corp.intel.com (HELO [10.237.221.32]) ([10.237.221.32]) by orsmga003.jf.intel.com with ESMTP; 06 Jun 2017 07:18:39 -0700 To: "dev@dpdk.org" , Jamie Lavigne References: <1496189340-27813-1-git-send-email-lavignen@amazon.com> <1496189818-2307-1-git-send-email-lavignen@amazon.com> From: Sergio Gonzalez Monroy Message-ID: <3b80fa21-3d3e-73ae-235b-7581db5e8670@intel.com> Date: Tue, 6 Jun 2017 15:18:39 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <1496189818-2307-1-git-send-email-lavignen@amazon.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2] Correctly handle malloc_elem resize with padding X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Jun 2017 14:18:42 -0000 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