[dpdk-dev] [PATCH v8 1/5] mempool: populate mempool with page sized chunks of memory

[Andrew Rybchenko]

On 7/23/19 3:28 PM, Vamsi Krishna Attunuru wrote:
>> -----Original Message-----
>> From: Andrew Rybchenko <arybchenko at solarflare.com>
>> Sent: Tuesday, July 23, 2019 4:38 PM
>> To: Vamsi Krishna Attunuru <vattunuru at marvell.com>; dev at dpdk.org
>> Cc: thomas at monjalon.net; Jerin Jacob Kollanukkaran <jerinj at marvell.com>;
>> olivier.matz at 6wind.com; ferruh.yigit at intel.com;
>> anatoly.burakov at intel.com; Kiran Kumar Kokkilagadda
>> <kirankumark at 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, vattunuru at marvell.com wrote:
>>> From: Vamsi Attunuru <vattunuru at 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 <vattunuru at marvell.com>
>>> Signed-off-by: Kiran Kumar K <kirankumark at 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.