* Re: [PATCH] mempool: fix alignment of memzone length when populating
2018-05-02 20:13 [PATCH] mempool: fix alignment of memzone length when populating Olivier Matz
@ 2018-05-03 8:03 ` Burakov, Anatoly
2018-05-03 9:34 ` Andrew Rybchenko
2018-05-07 8:18 ` [PATCH v2] " Olivier Matz
2 siblings, 0 replies; 7+ messages in thread
From: Burakov, Anatoly @ 2018-05-03 8:03 UTC (permalink / raw)
To: Olivier Matz, dev
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] mempool: fix alignment of memzone length when populating
2018-05-02 20:13 [PATCH] mempool: fix alignment of memzone length when populating Olivier Matz
2018-05-03 8:03 ` Burakov, Anatoly
@ 2018-05-03 9:34 ` Andrew Rybchenko
2018-05-03 10:04 ` Olivier Matz
2018-05-07 8:18 ` [PATCH v2] " Olivier Matz
2 siblings, 1 reply; 7+ messages in thread
From: Andrew Rybchenko @ 2018-05-03 9:34 UTC (permalink / raw)
To: Olivier Matz, dev; +Cc: Anatoly Burakov
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) {
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] mempool: fix alignment of memzone length when populating
2018-05-03 9:34 ` Andrew Rybchenko
@ 2018-05-03 10:04 ` Olivier Matz
0 siblings, 0 replies; 7+ messages in thread
From: Olivier Matz @ 2018-05-03 10:04 UTC (permalink / raw)
To: Andrew Rybchenko; +Cc: dev, Anatoly Burakov
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) {
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] mempool: fix alignment of memzone length when populating
2018-05-02 20:13 [PATCH] mempool: fix alignment of memzone length when populating Olivier Matz
2018-05-03 8:03 ` Burakov, Anatoly
2018-05-03 9:34 ` Andrew Rybchenko
@ 2018-05-07 8:18 ` Olivier Matz
2018-05-07 8:30 ` Andrew Rybchenko
2 siblings, 1 reply; 7+ messages in thread
From: Olivier Matz @ 2018-05-07 8:18 UTC (permalink / raw)
To: dev; +Cc: Anatoly Burakov, Andrew Rybchenko
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] mempool: fix alignment of memzone length when populating
2018-05-07 8:18 ` [PATCH v2] " Olivier Matz
@ 2018-05-07 8:30 ` Andrew Rybchenko
2018-05-08 13:59 ` Thomas Monjalon
0 siblings, 1 reply; 7+ messages in thread
From: Andrew Rybchenko @ 2018-05-07 8:30 UTC (permalink / raw)
To: Olivier Matz, dev; +Cc: Anatoly Burakov
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>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] mempool: fix alignment of memzone length when populating
2018-05-07 8:30 ` Andrew Rybchenko
@ 2018-05-08 13:59 ` Thomas Monjalon
0 siblings, 0 replies; 7+ messages in thread
From: Thomas Monjalon @ 2018-05-08 13:59 UTC (permalink / raw)
To: Olivier Matz; +Cc: dev, Andrew Rybchenko, Anatoly Burakov
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
^ permalink raw reply [flat|nested] 7+ messages in thread