[dpdk-dev] mempool: fix alignment of memzone length when populating
Checks
Commit Message
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(-)
Comments
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>
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) {
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) {
>
@@ -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) {