> -----Original Message-----
> From: Andrew Rybchenko <arybche...@solarflare.com>
> Sent: Wednesday, July 24, 2019 1:04 AM
> To: Vamsi Krishna Attunuru <vattun...@marvell.com>; dev@dpdk.org
> Cc: tho...@monjalon.net; Jerin Jacob Kollanukkaran <jer...@marvell.com>;
> olivier.m...@6wind.com; ferruh.yi...@intel.com; anatoly.bura...@intel.com;
> Kiran Kumar Kokkilagadda <kirankum...@marvell.com>
> Subject: Re: [dpdk-dev] [PATCH v8 1/5] mempool: populate mempool with page
> sized chunks of memory
>
> On 7/23/19 3:28 PM, Vamsi Krishna Attunuru wrote:
> >> -----Original Message-----
> >> From: Andrew Rybchenko <arybche...@solarflare.com>
> >> Sent: Tuesday, July 23, 2019 4:38 PM
> >> To: Vamsi Krishna Attunuru <vattun...@marvell.com>; dev@dpdk.org
> >> Cc: tho...@monjalon.net; Jerin Jacob Kollanukkaran
> >> <jer...@marvell.com>; olivier.m...@6wind.com; ferruh.yi...@intel.com;
> >> anatoly.bura...@intel.com; Kiran Kumar Kokkilagadda
> >> <kirankum...@marvell.com>
> >> Subject: Re: [dpdk-dev] [PATCH v8 1/5] mempool: populate mempool with
> >> page sized chunks of memory
> >>
> >> On 7/23/19 8:38 AM, vattun...@marvell.com wrote:
> >>> From: Vamsi Attunuru <vattun...@marvell.com>
> >>>
> >>> Patch adds a routine to populate mempool from page aligned and page
> >>> sized chunks of memory to ensures memory objs do not fall across the
> >>> page boundaries. It's useful for applications that require
> >>> physically contiguous mbuf memory while running in IOVA=VA mode.
> >>>
> >>> Signed-off-by: Vamsi Attunuru <vattun...@marvell.com>
> >>> Signed-off-by: Kiran Kumar K <kirankum...@marvell.com>
> >>> ---
>
> <...>
>
> >>> + int ret;
> >>> +
> >>> + ret = mempool_ops_alloc_once(mp);
> >>> + if (ret != 0)
> >>> + return ret;
> >>> +
> >>> + if (mp->nb_mem_chunks != 0)
> >>> + return -EEXIST;
> >>> +
> >>> + pg_sz = get_min_page_size(mp->socket_id);
> >>> + pg_shift = rte_bsf32(pg_sz);
> >>> +
> >>> + for (mz_id = 0, n = mp->size; n > 0; mz_id++, n -= ret) {
> >>> +
> >>> + rte_mempool_op_calc_mem_size_default(mp, n, pg_shift,
> >>> + &chunk_size, &align);
> >> It is incorrect to ignore mempool pool ops and enforce default
> >> handler. Use rte_mempool_ops_calc_mem_size().
> >> Also it is better to treat negative return value as an error as
> >> default function does.
> >> (May be it my mistake in return value description that it is not
> >> mentioned).
> >>
> > Yes, I thought so, but ops_calc_mem_size() would in turn call mempool
> > pmd's calc_mem_size() op which may/may not return required chunk_size
> > and align values in this case. Or else it would be skipped completely and
> > use
> pg_sz for both memzone len and align, anyways this page sized alignment will
> suits the pmd's specific align requirements.
>
> Anyway it is incorrect to violate driver ops. default is definitely wrong for
> bucket.
> min_chunk_size and align is mempool driver requirements. You can harden it,
> but should not violate it.
fine, I will modify the routine as below, call pmd's calc_mem_size() op and
over write min_chunk_size if it does not suit for this function's purpose.
+ total_elt_sz = mp->header_size + mp->elt_size + mp->trailer_size;
+ if (total_elt_sz > pg_sz)
+ return ret;
+ for (mz_id = 0, n = mp->size; n > 0; mz_id++, n -= ret) {
- rte_mempool_op_calc_mem_size_default(mp, n, pg_shift,
- &chunk_size, &align);
+ ret = rte_mempool_ops_calc_mem_size(mp, n, pg_shift,
+ &min_chunk_size, &align);
+
+ if (ret < 0)
goto fail;
+ if (min_chunk_size > pg_sz)
+ min_chunk_size = pg_sz;
Changes look fine.?
