Re: [dpdk-dev] [RFC 4/4] mempool: prevent objects from being across pages

[Olivier Matz <olivier.matz@6wind.com>]

On Wed, Aug 07, 2019 at 06:21:58PM +0300, Andrew Rybchenko wrote:
> On 7/19/19 4:38 PM, Olivier Matz wrote:
> > When using iova contiguous memory and objets smaller than page size,
> > ensure that objects are not located across several pages.
> 
> It looks like as an attempt to make exception a generic rule and
> I think it is not a good idea.
> 
> mempool has a notion of IOVA contiguous/non-contiguous objects
> depending if PA or VA. rte_mempool_op_populate_default() gets
> a memory chunk which is contiguous in VA and, if iova is not bad,
> IOVA-contiguous. The patch always enforces page boundaries even
> if it is not required. For example, if memory chunk is IOVA_PA
> contiguous, the patch could result in holes and extra memory usage

Yes, it may increase memory usage, but the amount should be limited.
On the other hand, the new patchset provides enhancements that will
reduce the memory consumption.

More importantly, it will fix the KNI + IOVA=VA issue. I also wonder if
this problem couldn't happen in case IOVA=PA. Are there any guarantees
that on all architectures a PA-contiguous in always VA-contiguous in the
kernel?



> 
> > Signed-off-by: Vamsi Krishna Attunuru <vattunuru@marvell.com>
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > ---
> >   lib/librte_mempool/rte_mempool_ops_default.c | 39 ++++++++++++++++++++++++++--
> >   1 file changed, 37 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/librte_mempool/rte_mempool_ops_default.c b/lib/librte_mempool/rte_mempool_ops_default.c
> > index 4e2bfc82d..2bbd67367 100644
> > --- a/lib/librte_mempool/rte_mempool_ops_default.c
> > +++ b/lib/librte_mempool/rte_mempool_ops_default.c
> > @@ -45,19 +45,54 @@ rte_mempool_op_calc_mem_size_default(const struct rte_mempool *mp,
> >   	return mem_size;
> >   }
> > +/* Returns -1 if object falls on a page boundary, else returns 0 */
> > +static inline int
> > +mempool_check_obj_bounds(void *obj, uint64_t pg_sz, size_t elt_sz)
> > +{
> > +	uintptr_t page_end, elt_addr = (uintptr_t)obj;
> > +	uint32_t pg_shift;
> > +	uint64_t page_mask;
> > +
> > +	if (pg_sz == 0)
> > +		return 0;
> > +	if (elt_sz > pg_sz)
> > +		return 0;
> > +
> > +	pg_shift = rte_bsf32(pg_sz);
> > +	page_mask =  ~((1ull << pg_shift) - 1);
> > +	page_end = (elt_addr & page_mask) + pg_sz;
> > +
> > +	if (elt_addr + elt_sz > page_end)
> > +		return -1;
> > +
> > +	return 0;
> > +}
> > +
> >   int
> >   rte_mempool_op_populate_default(struct rte_mempool *mp, unsigned int max_objs,
> >   		void *vaddr, rte_iova_t iova, size_t len,
> >   		rte_mempool_populate_obj_cb_t *obj_cb, void *obj_cb_arg)
> >   {
> > -	size_t total_elt_sz;
> > +	size_t total_elt_sz, pg_sz;
> >   	size_t off;
> >   	unsigned int i;
> >   	void *obj;
> > +	rte_mempool_get_page_size(mp, &pg_sz);
> > +
> >   	total_elt_sz = mp->header_size + mp->elt_size + mp->trailer_size;
> > -	for (off = 0, i = 0; off + total_elt_sz <= len && i < max_objs; i++) {
> > +	for (off = 0, i = 0; i < max_objs; i++) {
> > +		/* align offset to next page start if required */
> > +		if (mempool_check_obj_bounds((char *)vaddr + off,
> > +						pg_sz, total_elt_sz) < 0) {
> > +			off += RTE_PTR_ALIGN_CEIL((char *)vaddr + off, pg_sz) -
> > +				((char *)vaddr + off);
> > +		}
> > +
> > +		if (off + total_elt_sz > len)
> > +			break;
> > +
> >   		off += mp->header_size;
> >   		obj = (char *)vaddr + off;
> >   		obj_cb(mp, obj_cb_arg, obj,
>