[dpdk-dev] [PATCH v1 8/9] mempool: ensure the mempool is initialized before populating

[Andrew Rybchenko]

On 03/19/2018 08:06 PM, Olivier Matz wrote:
> On Sat, Mar 10, 2018 at 03:39:41PM +0000, Andrew Rybchenko wrote:
>> From: "Artem V. Andreev" <Artem.Andreev at oktetlabs.ru>
>>
>> Callback to calculate required memory area size may require mempool
>> driver data to be already allocated and initialized.
>>
>> Signed-off-by: Artem V. Andreev <Artem.Andreev at oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <arybchenko at solarflare.com>
>> ---
>> RFCv2 -> v1:
>>   - rename helper function as mempool_ops_alloc_once()
>>
>>   lib/librte_mempool/rte_mempool.c | 29 ++++++++++++++++++++++-------
>>   1 file changed, 22 insertions(+), 7 deletions(-)
>>
>> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
>> index 844d907..12085cd 100644
>> --- a/lib/librte_mempool/rte_mempool.c
>> +++ b/lib/librte_mempool/rte_mempool.c
>> @@ -322,6 +322,21 @@ rte_mempool_free_memchunks(struct rte_mempool *mp)
>>   	}
>>   }
>>   
>> +static int
>> +mempool_ops_alloc_once(struct rte_mempool *mp)
>> +{
>> +	int ret;
>> +
>> +	/* create the internal ring if not already done */
>> +	if ((mp->flags & MEMPOOL_F_POOL_CREATED) == 0) {
>> +		ret = rte_mempool_ops_alloc(mp);
>> +		if (ret != 0)
>> +			return ret;
>> +		mp->flags |= MEMPOOL_F_POOL_CREATED;
>> +	}
>> +	return 0;
>> +}
>> +
>>   /* Add objects in the pool, using a physically contiguous memory
>>    * zone. Return the number of objects added, or a negative value
>>    * on error.
>> @@ -336,13 +351,9 @@ rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr,
>>   	struct rte_mempool_memhdr *memhdr;
>>   	int ret;
>>   
>> -	/* create the internal ring if not already done */
>> -	if ((mp->flags & MEMPOOL_F_POOL_CREATED) == 0) {
>> -		ret = rte_mempool_ops_alloc(mp);
>> -		if (ret != 0)
>> -			return ret;
>> -		mp->flags |= MEMPOOL_F_POOL_CREATED;
>> -	}
>> +	ret = mempool_ops_alloc_once(mp);
>> +	if (ret != 0)
>> +		return ret;
>>   
>>   	/* mempool is already populated */
>>   	if (mp->populated_size >= mp->size)
>> @@ -515,6 +526,10 @@ rte_mempool_populate_default(struct rte_mempool *mp)
>>   	unsigned mz_id, n;
>>   	int ret;
>>   
>> +	ret = mempool_ops_alloc_once(mp);
>> +	if (ret != 0)
>> +		return ret;
>> +
>>   	/* mempool must not be populated */
>>   	if (mp->nb_mem_chunks != 0)
>>   		return -EEXIST;
>
> Is there a reason why we need to add it in
> rte_mempool_populate_default() but not in rte_mempool_populate_virt() and
> rte_mempool_populate_iova_tab()?

The reason is rte_mempool_ops_calc_mem_size() call
from rte_mempool_populate_default(). rte_mempool_ops_*() are not
called directly from rte_mempool_populate_virt() and
rte_mempool_populate_iova_tab().

In fact I've found out that rte_mempool_ops_calc_mem_size() is called
from get_anon_size() which is called from rte_mempool_populate_anon().
So, we need to add to get_anon_size() as well.

May be it is even better to make the patch the first in series to make
sure that it is already OK when rte_mempool_ops_calc_mem_size()
is added. What do you think?