From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id B4BDC1D7 for ; Thu, 3 May 2018 12:05:06 +0200 (CEST) Received: from lfbn-lil-1-169-73.w90-45.abo.wanadoo.fr ([90.45.25.73] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1fEB6t-0006Pt-5k; Thu, 03 May 2018 12:05:13 +0200 Received: by droids-corp.org (sSMTP sendmail emulation); Thu, 03 May 2018 12:04:59 +0200 Date: Thu, 3 May 2018 12:04:59 +0200 From: Olivier Matz To: Andrew Rybchenko Cc: dev@dpdk.org, Anatoly Burakov Message-ID: <20180503100459.bka5srecyqa4peqb@platinum> References: <20180502201349.15568-1-olivier.matz@6wind.com> <366ffbd0-f5ab-c2d0-2cde-df1f904b2745@solarflare.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <366ffbd0-f5ab-c2d0-2cde-df1f904b2745@solarflare.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH] mempool: fix alignment of memzone length when populating 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: Thu, 03 May 2018 10:05:06 -0000 On Thu, May 03, 2018 at 12:34:59PM +0300, Andrew Rybchenko wrote: > Hi Olivier, > > On 05/02/2018 11:13 PM, Olivier Matz wrote: > > When populating a mempool with the default function, if there is not > > enough virtually contiguous memory for the whole mempool, it will be > > populated with several chunks. A chunk of the maximum available length > > is requested with: > > > > mz = rte_memzone_reserve_aligned(..., len=0, ..., align=x) > > > > If align is smaller than the page size, the length of the memzone may > > not be a multiple of the page size. This makes > > rte_mempool_populate_virt() to fail because it requires a page-aligned > > length. This patch forces the memzone length to be a multiple of page > > size. > > > > The problem can be reproduced easily by allocating more than available > > memory: > > ./build/app/testpmd -l 0,1 -- --total-num-mbufs=65536 > > ... > > Cause: Creation of mbuf pool for socket 0 failed: Invalid argument > > > > After the patch, the error code is correct: > > ./build/app/testpmd -l 0,1 -- --total-num-mbufs=65536 > > ... > > Cause: Creation of mbuf pool for socket 0 failed: Cannot allocate memory > > > > Signed-off-by: Olivier Matz > > Fixes: ba0009560c30 ("mempool: support new allocation methods") > > --- > > > > Hi Anatoly, > > > > Another option to fix this issue could be to ensure that > > rte_memzone_reserve_aligned(..., len=0, ..., align=x) returns a length > > that is multiple of page size. Something like: > > > > mz = rte_memzone_reserve_aligned(mz_name, 0, > > - mp->socket_id, flags, align); > > + mp->socket_id, flags, RTE_MAX(pg_sz, align)); > > As far as I can see rte_mempool_populate_virt() checks that both address and > length are page size aligned. So, I think both should be used. This one to > be sure > that address is page-aligned, below to ensure that length is page size > aligned. > May be one of them will be the property of the allocated region any way, but > it is safer to guarantee both restrictions. You are right, we should also ensure that the address is aligned, so the second patch is needed. Will send a v2. Thanks. > > > > > Let me know if you prefer this way. > > > > Thanks, > > Olivier > > > > > > lib/librte_mempool/rte_mempool.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c > > index cf5d124ec..78c3e95ec 100644 > > --- a/lib/librte_mempool/rte_mempool.c > > +++ b/lib/librte_mempool/rte_mempool.c > > @@ -709,7 +709,7 @@ rte_mempool_populate_default(struct rte_mempool *mp) > > (void *)(uintptr_t)mz); > > else > > ret = rte_mempool_populate_virt(mp, mz->addr, > > - mz->len, pg_sz, > > + RTE_ALIGN_FLOOR(mz->len, pg_sz), pg_sz, > > rte_mempool_memchunk_mz_free, > > (void *)(uintptr_t)mz); > > if (ret < 0) { >