[dpdk-dev,v2] mempool: fix alignment of memzone length when populating

Message ID 20180507081801.15050-1-olivier.matz@6wind.com (mailing list archive)
State Accepted, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Olivier Matz May 7, 2018, 8:18 a.m. UTC
  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 address and 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 them to be
page-aligned. This patch fixes that.

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 <olivier.matz@6wind.com>
Fixes: ba0009560c30 ("mempool: support new allocation methods")
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

v2:
* ensure that both address and length are page-aligned, as suggested by
  Andrew


 lib/librte_mempool/rte_mempool.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

Comments

Andrew Rybchenko May 7, 2018, 8:30 a.m. UTC | #1
On 05/07/2018 11:18 AM, 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 address and 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 them to be
> page-aligned. This patch fixes that.
>
> 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 <olivier.matz@6wind.com>
> Fixes: ba0009560c30 ("mempool: support new allocation methods")
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
  
Thomas Monjalon May 8, 2018, 1:59 p.m. UTC | #2
07/05/2018 10:30, Andrew Rybchenko:
> On 05/07/2018 11:18 AM, 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 address and 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 them to be
> > page-aligned. This patch fixes that.
> >
> > 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 <olivier.matz@6wind.com>
> > Fixes: ba0009560c30 ("mempool: support new allocation methods")
> > Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>

Applied, thanks
  

Patch

diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
index cf5d124ec..9f1a4253b 100644
--- a/lib/librte_mempool/rte_mempool.c
+++ b/lib/librte_mempool/rte_mempool.c
@@ -684,7 +684,8 @@  rte_mempool_populate_default(struct rte_mempool *mp)
 			 * have
 			 */
 			mz = rte_memzone_reserve_aligned(mz_name, 0,
-					mp->socket_id, flags, align);
+					mp->socket_id, flags,
+					RTE_MAX(pg_sz, align));
 		}
 		if (mz == NULL) {
 			ret = -rte_errno;
@@ -709,7 +710,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) {