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

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

[Olivier Matz]

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 <olivier.matz@6wind.com>
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));

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) {
-- 
2.11.0

Re: [dpdk-dev] [PATCH] mempool: fix alignment of memzone length when populating

[Burakov, Anatoly]

On 02-May-18 9: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 <olivier.matz@6wind.com>
> 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));
> 
> 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) {
> 

Either way is OK by me.

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH] mempool: fix alignment of memzone length when populating

[Andrew Rybchenko]

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 <olivier.matz@6wind.com>
> 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.

>
> 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) {

Re: [dpdk-dev] [PATCH] mempool: fix alignment of memzone length when populating

[Olivier Matz]

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 <olivier.matz@6wind.com>
> > 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) {
> 

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

[Olivier Matz]

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(-)

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) {
-- 
2.11.0

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

[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>

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

[Thomas Monjalon]

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