From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2F696A00E6 for ; Wed, 7 Aug 2019 17:21:36 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id E8CC02BBD; Wed, 7 Aug 2019 17:21:35 +0200 (CEST) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id 98EB61D7 for ; Wed, 7 Aug 2019 17:21:33 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us2.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id 07E7EA4008F; Wed, 7 Aug 2019 15:21:32 +0000 (UTC) Received: from [192.168.1.11] (85.187.13.152) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Wed, 7 Aug 2019 16:21:25 +0100 To: Olivier Matz , Vamsi Krishna Attunuru , CC: Thomas Monjalon , Anatoly Burakov , Jerin Jacob Kollanukkaran , Kokkilagadda , Ferruh Yigit References: <20190719133845.32432-1-olivier.matz@6wind.com> <20190719133845.32432-3-olivier.matz@6wind.com> From: Andrew Rybchenko Message-ID: <98e55711-c232-1184-5293-9f8ab1454a44@solarflare.com> Date: Wed, 7 Aug 2019 18:21:20 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <20190719133845.32432-3-olivier.matz@6wind.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-Originating-IP: [85.187.13.152] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24828.000 X-TM-AS-Result: No-16.191600-8.000000-10 X-TMASE-MatchedRID: ZFzIhWOuIztq0U6EhO9EE4mfV7NNMGm+69aS+7/zbj+qvcIF1TcLYKEG Khm9baaN0wKYUXiwDuspYPdJucL2NzfOOPFtR30Sfc7cX82yHHkX2zxRNhh61X9nRLJB1yYQ9Vl GBjCDncjFG6rPbKnpuldOQryZMQKx0rly6TajiUniHyvyXeXh5sMA9JsxaUa3Sg8ufp5n3T7ccF +vY4fYYSiXjjQS3tHr6A/E3QXDjtxpokC3EjW38Khx+sYfmafeyoUTqBF1E5sGmHr1eMxt2UAc6 DyoS2rIXQS0szagh4PRo5gP/gf+mZ5DPlQD/Hz3CuDAUX+yO6aeutXlSdRVja4136m7d1MaQDov 87hVw1J4laxa1ZJtNf8a5GYWRdTP7cCiNMOXTK1fYa9W9OjitQRryDXHx6oX7L2+zGEubN51LAB 7GKWwE5jecj2e6HJGFec8Bmj61xYIyeWU2toSHjqf0DtPln4novA/6ONsv0qqagILjdvgWvjWAW a6XtbpEGZf0gNLwy0Cg/XkqUTEmnT3K3VineCfyf21YeIsPYZyqV0HbyMU8psoi2XrUn/JyeMtM D9QOgChMIDkR/KfwI2j49Ftap9EOwBXM346/+zRI8qKRhYjUAESu6AEf8DvbrCkEVexCEVjILLt OWd9KdHTfPmfXIB+ X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--16.191600-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24828.000 X-MDID: 1565191293-X954YbNwUXzL Subject: Re: [dpdk-dev] ***Spam*** [RFC 2/4] mempool: unalign size when calculating required mem amount 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 7/19/19 4:38 PM, Olivier Matz wrote: > The size returned by rte_mempool_op_calc_mem_size_default() is aligned > to the specified page size. This means that with big pages, the returned > amount is more that what we really need to populate the mempool. > > This problem is tempered by the allocation method of > rte_mempool_populate_default(): in some conditions (when > try_iova_contig_mempool=true), it first tries to allocate all objs > memory in an iova contiguous area, without the alignment constraint. If > it fails, it fallbacks to the big aligned allocation, that can also > fallback into several smaller allocations. > > This commit changes rte_mempool_op_calc_mem_size_default() to return the > unaligned amount of memory (the alignment constraint is still returned > via the *align argument), It does not as far as I can see. There is no changes in lib/librte_mempool/rte_mempool_ops_default.c > and removes the optimistic contiguous > allocation done when try_iova_contig_mempool=true. Unfortunately I don't understand why these 2 changes are combined into to one patch. The first looks good to me, the second is questionable. It can impact performance. > This will make the amount of allocated memory more predictible: it will > be more than the optimistic contiguous allocation, but less than the big > aligned allocation. > > This opens the door for the next commits that will try to prevent objets > from being located across pages. > > Signed-off-by: Olivier Matz > --- > lib/librte_mempool/rte_mempool.c | 44 ++++-------------------------------- > lib/librte_mempool/rte_mempool.h | 2 +- > lib/librte_mempool/rte_mempool_ops.c | 4 +++- > 3 files changed, 9 insertions(+), 41 deletions(-) > > diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c > index 0f29e8712..335032dc8 100644 > --- a/lib/librte_mempool/rte_mempool.c > +++ b/lib/librte_mempool/rte_mempool.c > @@ -430,7 +430,6 @@ rte_mempool_populate_default(struct rte_mempool *mp) > unsigned mz_id, n; > int ret; > bool need_iova_contig_obj; > - bool try_iova_contig_mempool; > bool alloc_in_ext_mem; > > ret = mempool_ops_alloc_once(mp); > @@ -477,18 +476,10 @@ rte_mempool_populate_default(struct rte_mempool *mp) > * wasting some space this way, but it's much nicer than looping around > * trying to reserve each and every page size. > * > - * However, since size calculation will produce page-aligned sizes, it > - * makes sense to first try and see if we can reserve the entire memzone > - * in one contiguous chunk as well (otherwise we might end up wasting a > - * 1G page on a 10MB memzone). If we fail to get enough contiguous > - * memory, then we'll go and reserve space page-by-page. > - * > * We also have to take into account the fact that memory that we're > * going to allocate from can belong to an externally allocated memory > * area, in which case the assumption of IOVA as VA mode being > - * synonymous with IOVA contiguousness will not hold. We should also try > - * to go for contiguous memory even if we're in no-huge mode, because > - * external memory may in fact be IOVA-contiguous. > + * synonymous with IOVA contiguousness will not hold. > */ > > /* check if we can retrieve a valid socket ID */ > @@ -497,7 +488,6 @@ rte_mempool_populate_default(struct rte_mempool *mp) > return -EINVAL; > alloc_in_ext_mem = (ret == 1); > need_iova_contig_obj = !(mp->flags & MEMPOOL_F_NO_IOVA_CONTIG); > - try_iova_contig_mempool = false; > > if (!need_iova_contig_obj) { > pg_sz = 0; > @@ -506,7 +496,6 @@ rte_mempool_populate_default(struct rte_mempool *mp) > pg_sz = 0; > pg_shift = 0; > } else if (rte_eal_has_hugepages() || alloc_in_ext_mem) { > - try_iova_contig_mempool = true; > pg_sz = get_min_page_size(mp->socket_id); > pg_shift = rte_bsf32(pg_sz); > } else { > @@ -518,12 +507,8 @@ rte_mempool_populate_default(struct rte_mempool *mp) > size_t min_chunk_size; > unsigned int flags; > > - if (try_iova_contig_mempool || pg_sz == 0) > - mem_size = rte_mempool_ops_calc_mem_size(mp, n, > - 0, &min_chunk_size, &align); > - else > - mem_size = rte_mempool_ops_calc_mem_size(mp, n, > - pg_shift, &min_chunk_size, &align); > + mem_size = rte_mempool_ops_calc_mem_size( > + mp, n, pg_shift, &min_chunk_size, &align); > > if (mem_size < 0) { > ret = mem_size; > @@ -542,31 +527,12 @@ rte_mempool_populate_default(struct rte_mempool *mp) > /* if we're trying to reserve contiguous memory, add appropriate > * memzone flag. > */ > - if (try_iova_contig_mempool) > + if (min_chunk_size == (size_t)mem_size) > flags |= RTE_MEMZONE_IOVA_CONTIG; > > mz = rte_memzone_reserve_aligned(mz_name, mem_size, > mp->socket_id, flags, align); > > - /* if we were trying to allocate contiguous memory, failed and > - * minimum required contiguous chunk fits minimum page, adjust > - * memzone size to the page size, and try again. > - */ > - if (mz == NULL && try_iova_contig_mempool && > - min_chunk_size <= pg_sz) { > - try_iova_contig_mempool = false; > - flags &= ~RTE_MEMZONE_IOVA_CONTIG; > - > - mem_size = rte_mempool_ops_calc_mem_size(mp, n, > - pg_shift, &min_chunk_size, &align); > - if (mem_size < 0) { > - ret = mem_size; > - goto fail; > - } > - > - mz = rte_memzone_reserve_aligned(mz_name, mem_size, > - mp->socket_id, flags, align); > - } > /* don't try reserving with 0 size if we were asked to reserve > * IOVA-contiguous memory. > */ > @@ -594,7 +560,7 @@ rte_mempool_populate_default(struct rte_mempool *mp) > else > iova = RTE_BAD_IOVA; > > - if (try_iova_contig_mempool || pg_sz == 0) > + if (pg_sz == 0) > ret = rte_mempool_populate_iova(mp, mz->addr, > iova, mz->len, > rte_mempool_memchunk_mz_free, > diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h > index 8053f7a04..7bc10e699 100644 > --- a/lib/librte_mempool/rte_mempool.h > +++ b/lib/librte_mempool/rte_mempool.h > @@ -458,7 +458,7 @@ typedef unsigned (*rte_mempool_get_count)(const struct rte_mempool *mp); > * @param[out] align > * Location for required memory chunk alignment. > * @return > - * Required memory size aligned at page boundary. > + * Required memory size. > */ > typedef ssize_t (*rte_mempool_calc_mem_size_t)(const struct rte_mempool *mp, > uint32_t obj_num, uint32_t pg_shift, > diff --git a/lib/librte_mempool/rte_mempool_ops.c b/lib/librte_mempool/rte_mempool_ops.c > index e02eb702c..22c5251eb 100644 > --- a/lib/librte_mempool/rte_mempool_ops.c > +++ b/lib/librte_mempool/rte_mempool_ops.c > @@ -100,7 +100,9 @@ rte_mempool_ops_get_count(const struct rte_mempool *mp) > return ops->get_count(mp); > } > > -/* wrapper to notify new memory area to external mempool */ > +/* wrapper to calculate the memory size required to store given number > + * of objects > + */ > ssize_t > rte_mempool_ops_calc_mem_size(const struct rte_mempool *mp, > uint32_t obj_num, uint32_t pg_shift,