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

[Jerin Jacob]

On Wed, Oct 30, 2019 at 8:03 PM Olivier Matz <olivier.matz at 6wind.com> wrote:
>
> 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.

Thanks, Olivier. We will test it.

>
> > # 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)?

I will start a new thread on this CCing all arch maintainers.


>
> Thanks,
> Olivier