On Wed, Oct 30, 2019 at 8:03 PM Olivier Matz <olivier.m...@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
> > <arybche...@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
