Hi Andrew, On Tue, Oct 29, 2019 at 12:21:27PM +0300, Andrew Rybchenko wrote: > On 10/28/19 5:01 PM, Olivier Matz wrote: > > rte_mempool_populate_virt() currently requires that both addr > > and length are page-aligned. > > > > Remove this uneeded constraint which can be annoying with big > > hugepages (ex: 1GB). > > > > Signed-off-by: Olivier Matz <olivier.m...@6wind.com> > > One note below, other than that > Reviewed-by: Andrew Rybchenko <arybche...@solarflare.com> > > > --- > > lib/librte_mempool/rte_mempool.c | 18 +++++++----------- > > lib/librte_mempool/rte_mempool.h | 3 +-- > > 2 files changed, 8 insertions(+), 13 deletions(-) > > > > diff --git a/lib/librte_mempool/rte_mempool.c > > b/lib/librte_mempool/rte_mempool.c > > index 0f29e8712..76cbacdf3 100644 > > --- a/lib/librte_mempool/rte_mempool.c > > +++ b/lib/librte_mempool/rte_mempool.c > > @@ -368,17 +368,11 @@ rte_mempool_populate_virt(struct rte_mempool *mp, > > char *addr, > > size_t off, phys_len; > > int ret, cnt = 0; > > - /* address and len must be page-aligned */ > > - if (RTE_PTR_ALIGN_CEIL(addr, pg_sz) != addr) > > - return -EINVAL; > > - if (RTE_ALIGN_CEIL(len, pg_sz) != len) > > - return -EINVAL; > > - > > if (mp->flags & MEMPOOL_F_NO_IOVA_CONTIG) > > return rte_mempool_populate_iova(mp, addr, RTE_BAD_IOVA, > > len, free_cb, opaque); > > - for (off = 0; off + pg_sz <= len && > > + for (off = 0; off < len && > > mp->populated_size < mp->size; off += phys_len) { > > iova = rte_mem_virt2iova(addr + off); > > @@ -389,7 +383,10 @@ rte_mempool_populate_virt(struct rte_mempool *mp, char > > *addr, > > } > > /* populate with the largest group of contiguous pages */ > > - for (phys_len = pg_sz; off + phys_len < len; phys_len += pg_sz) > > { > > + for (phys_len = RTE_PTR_ALIGN_CEIL(addr + off + 1, pg_sz) - > > + (addr + off); > > + off + phys_len < len; > > If the condition is false on the first check, below we'll populate memory > outside of specified length. So, we should either apply MIN above when > phys_len > is initialized or drop MIN in the next line, but apply MIN when > rte_mempool_populate_iova() is called.
Correct, I'll fix it by adding a RTE_MIN(). Maybe I'll just move it outside of the for. > Bonus question not directly related to the patch is iova == RTE_BAD_IOVA > case when !rte_eal_has_hugepages(): > Is it expected that we still use arithmetic iova + phys_len in this case? > I guess comparison will always be false and pages never merged, but it looks > suspicious anyway. Yes, this is not very elegant. I can change the test to if (iova_tmp != RTE_BAD_IOVA || iova_tmp != iova + phys_len) > > > + phys_len = RTE_MIN(phys_len + pg_sz, len - off)) { > > rte_iova_t iova_tmp; > > iova_tmp = rte_mem_virt2iova(addr + off + phys_len); > > @@ -575,8 +572,7 @@ rte_mempool_populate_default(struct rte_mempool *mp) > > * have > > */ > > mz = rte_memzone_reserve_aligned(mz_name, 0, > > - mp->socket_id, flags, > > - RTE_MAX(pg_sz, align)); > > + mp->socket_id, flags, align); > > } > > if (mz == NULL) { > > ret = -rte_errno; > > @@ -601,7 +597,7 @@ rte_mempool_populate_default(struct rte_mempool *mp) > > (void *)(uintptr_t)mz); > > else > > ret = rte_mempool_populate_virt(mp, mz->addr, > > - RTE_ALIGN_FLOOR(mz->len, pg_sz), pg_sz, > > + mz->len, pg_sz, > > rte_mempool_memchunk_mz_free, > > (void *)(uintptr_t)mz); > > if (ret < 0) { > > diff --git a/lib/librte_mempool/rte_mempool.h > > b/lib/librte_mempool/rte_mempool.h > > index 8053f7a04..0fe8aa7b8 100644 > > --- a/lib/librte_mempool/rte_mempool.h > > +++ b/lib/librte_mempool/rte_mempool.h > > @@ -1042,9 +1042,8 @@ int rte_mempool_populate_iova(struct rte_mempool *mp, > > char *vaddr, > > * A pointer to the mempool structure. > > * @param addr > > * The virtual address of memory that should be used to store objects. > > - * Must be page-aligned. > > * @param len > > - * The length of memory in bytes. Must be page-aligned. > > + * The length of memory in bytes. > > * @param pg_sz > > * The size of memory pages in this virtual area. > > * @param free_cb >