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 A154FA00BE; Mon, 28 Oct 2019 15:07:02 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 184331BF51; Mon, 28 Oct 2019 15:07:02 +0100 (CET) Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by dpdk.org (Postfix) with ESMTP id 3A7EA1BF4F for ; Mon, 28 Oct 2019 15:07:00 +0100 (CET) Received: by mail-wm1-f68.google.com with SMTP id g24so9599434wmh.5 for ; Mon, 28 Oct 2019 07:07:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=EjhMDpQATvynY1AC6e5wDl6r/0IPphAxSVfHldbByYg=; b=QyYvjLkDTmd6g9FIMziuOcTGvsiST5SXmJqjAAaaINMn4IoikKVZ47YuaiUPDtVSly eYXmgwTrx0AQZ2QCh6mdD9FRPuMrNBCP19m0qgDAGkDfiua17WqTjF7jyCfCAijgh0ur Sa69M1fLeF5hLluLEWdniu4GwTsZOMt5I7MGE/aWOd+G3WLx3RdQVFMPhTndj+1Mg3ZN tMqy5XLDew8RH9jA2cEWLJvqia/SQN5E2VmkyGlYNbT4cgjlYVv+uCEIPgLQ1RKRzhfr V2kiwvsyfASifmHTZxV5dMDO4wfmPMmrBl9o1pED+JCUBWesEhzhNCxZtj6ers/amKyo otSA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=EjhMDpQATvynY1AC6e5wDl6r/0IPphAxSVfHldbByYg=; b=moieVcwYzLv7npuzp47GEAWGokFib+stGm7YDa5fdgJch09Q55BbXqdNDCyELZ311a NmmXt295U9YZYNSeTQYd3OrPznbzNUUEXuPIDVOq7amnN4DPAswqTOEwCIJw1KXAW/P1 OFryh/Mu5vCSICdyFoEh6LLDwQ52whVRBEdmdaC7mxvaIXjixviJjbSiou8EMolPc7SN xhv1kdEshxDWQVqGQnwn0a6YzH0FMTSwPQjTHVKPxl2tT2iQZeiZZpdIm3zY4jv3dVI0 2Tr/iadY8kFmoF2/gpDwBil2HAS5omN3IR65UwliZEFzY+3+9IkWkW+jJm15fxH4RMzq vfYA== X-Gm-Message-State: APjAAAXQZhzGpt6zhfsgAdyijots61c8+OWO/8ontU8nISb2GTfCJhaD EiScDEgx5Akmam+Q0BzJBkaccA== X-Google-Smtp-Source: APXvYqw7yGD47UrTYXCQiExA7EzpY+mpMZkzfdRMBwXgRVHBoVZ/JeZ/bfVcWkGcq6IIRBbYoE8QGw== X-Received: by 2002:a7b:c302:: with SMTP id k2mr136946wmj.81.1572271619816; Mon, 28 Oct 2019 07:06:59 -0700 (PDT) Received: from 6wind.com (2a01cb0c0005a6000226b0fffeed02fc.ipv6.abo.wanadoo.fr. [2a01:cb0c:5:a600:226:b0ff:feed:2fc]) by smtp.gmail.com with ESMTPSA id u21sm14201105wmu.27.2019.10.28.07.06.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 28 Oct 2019 07:06:58 -0700 (PDT) Date: Mon, 28 Oct 2019 15:06:58 +0100 From: Olivier Matz To: Andrew Rybchenko Cc: Vamsi Krishna Attunuru , dev@dpdk.org, Thomas Monjalon , Anatoly Burakov , Jerin Jacob Kollanukkaran , Kokkilagadda , Ferruh Yigit Message-ID: <20191028140658.hxearxsj6a26zoep@platinum> References: <20190719133845.32432-1-olivier.matz@6wind.com> <20190719133845.32432-3-olivier.matz@6wind.com> <98e55711-c232-1184-5293-9f8ab1454a44@solarflare.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <98e55711-c232-1184-5293-9f8ab1454a44@solarflare.com> User-Agent: NeoMutt/20180716 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 Wed, Aug 07, 2019 at 06:21:20PM +0300, Andrew Rybchenko wrote: > 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 True, sorry. This is fixed in the new version. > > 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. I splited this change into 2 patches, that's indeed clearer. However I didn't get what kind do performance impact do you expect? > > 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, >