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. > # 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