On 7/24/19 10:09 AM, Vamsi Krishna Attunuru wrote:
-----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.?
No, you can't violate min_chunk_size requirement. If it is unacceptable,
error should be returned. So, you should not check total_elt_sz above
separately.
