From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id 0884EDD2; Fri, 4 May 2018 13:02:55 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 May 2018 04:02:54 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,362,1520924400"; d="scan'208";a="52382033" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.252.18.186]) ([10.252.18.186]) by fmsmga001.fm.intel.com with ESMTP; 04 May 2018 04:02:53 -0700 To: gowrishankar muthukrishnan , Sergio Gonzalez Monroy Cc: dev@dpdk.org, Thomas Monjalon , stable@dpdk.org, Chao Zhu References: <68d1bc7b-a90d-2adc-c98b-560276589b15@intel.com> <78f363f1-79d8-0369-05eb-8f853d9c76a5@linux.vnet.ibm.com> From: "Burakov, Anatoly" Message-ID: <42587911-ea23-d7a6-6285-65e1180915e6@intel.com> Date: Fri, 4 May 2018 12:02:52 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <78f363f1-79d8-0369-05eb-8f853d9c76a5@linux.vnet.ibm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 1/2] eal/malloc: merge malloc_elems in heap if they are contiguous 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: Fri, 04 May 2018 11:02:56 -0000 On 04-May-18 11:41 AM, gowrishankar muthukrishnan wrote: > Hi Anatoly, > > On Friday 04 May 2018 02:59 PM, Burakov, Anatoly wrote: >> On 03-May-18 11:11 AM, Gowrishankar wrote: >>> From: Gowrishankar Muthukrishnan >>> >>> During malloc heap init, if there are malloc_elems contiguous in >>> virt addresses, they could be merged so that, merged malloc_elem >>> would guarantee larger free memory size than its actual hugepage >>> size, it was created for. >>> >>> Fixes: fafcc11985 ("mem: rework memzone to be allocated by malloc") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Gowrishankar Muthukrishnan >>> >>> --- >> >> Hi Gowrishankar, >> >> I haven't looked at the patchset in detail yet, however i have a >> general question: how do we end up with VA-contiguous memsegs that are >> not part of the same memseg in the first place? Is there something >> wrong with memseg sorting code? Alternatively, if they were broken up, >> presumably they were broken up for a reason, namely while they may be >> VA contiguous, they weren't IOVA-contiguous. > > In powerpc, when *nr_overcommit_hugepages set* (to respect address hint > in get_virtual_area() as requested by secondary process), mmap() would > not be allocate one big VA chunk for all the available hugepages. In > order to support secondary process be in same VA > range, we need to add anonymous and hugetlb flags in mmap calls while > remapping. As mmap can only create max VA at the size of hugepage > (MAP_HUGETLB) and also to respect address hint (MAP_ANONYMOUS), multiple > VA chunks are created, even though both VA and IOVA are contiguous in > most of the cases. OK, suppose on PPC64, that may happen. Still (and please correct me if i'm misunderstanding the patchset - as i said, i haven't looked at it in detail, and have only taken a cursory look), there are two issues i see here: 1) there's no check for IOVA-contiguousness, only VA-contiguousness, which means you are risking accidentally concatenating segments that aren't IOVA-contiguous. Prior to 18.05, the rest of DPDK expects all segments to be VA- and IOVA-contiguous. 2) i don't think this problem should be solved in malloc. Malloc elements have memseg pointers in them, and if you concatenate multiple segments, you will end up having malloc elements which point to wrong segments. Instead, you should fix memseg allocation code to do concatenate seemingly disparate segments, and avoid the problem with malloc elements in the first place. Maybe do another sorting pass, or something. In any case, memseg allocation code is the correct place to fix this, IMO. > >> >> Can you provide a dump of physmem layout where memory would have been >> VA and IOVA-contiguous while belonging to different memsegs? > > Please find here: https://pastebin.com/tDNEaxdU > > As you notice malloc_heaps, its index for heap size is 8 which is > supposedly 11. That's a bit hard to read. There's a rte_eal_dump_physmem_layout() function that should help display this in a more user-friendly manner :) > > To note, these are not problems with memory rework done in latest code > base. So, I refered code until v18.02. > -- Thanks, Anatoly