[dpdk-dev] [EXT] [PATCH 5/5] mempool: prevent objects from being across pages

[Olivier Matz]

Hi Jerin,

On Wed, Oct 30, 2019 at 02:08:40PM +0530, Jerin Jacob wrote:
> On Wed, Oct 30, 2019 at 1:16 PM Andrew Rybchenko
> <arybchenko at solarflare.com> wrote:
> >
> 
> > >>   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;
> > >> +    char *va = vaddr;
> > >> +    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 (check_obj_bounds(va + off, pg_sz, total_elt_sz) < 0)
> > >> +                    off += RTE_PTR_ALIGN_CEIL(va + off, pg_sz) - (va +
> > >> off);
> > > Moving offset to the start of next page and than freeing (vaddr + off + header_size) to pool, this scheme is not aligning with octeontx2 mempool's buf alignment requirement(buffer address needs to be multiple of buffer size).
> >
> > It sounds line octeontx2 mempool should have its own populate callback
> > which cares about it.
> 
> Driver specific populate function is not a bad idea. The only concern
> would be to
> 
> # We need to duplicate rte_mempool_op_populate_default() and
> rte_mempool_op_calc_mem_size_default()
> # We need to make sure if some one changes the
> rte_mempool_op_populate_default() and
> rte_mempool_op_calc_mem_size_default() then he/she needs to update the
> drivers too

Agree, we need to be careful. Hopefully we shouldn't change this code
very often.

I'm sending a v2 with a patch to the octeontx2 driver which --I hope--
should solve the issue.

> # I would like to add one more point here is that:
> - Calculation of  object pad requirements for MEMPOOL_F_NO_SPREAD i.e
> optimize_object_size()
> is NOT GENERIC. i.e get_gcd() based logic is not generic. DDR
> controller defines the address to DDR channel "spread" and it will be
> based on SoC or Mirco architecture.
> So we need to consider that as well.

Could you give some details about how it should be optimized on your
platforms, and what is the behavior today (advertised nb_rank and
nb_channels)?

Thanks,
Olivier