[dpdk-dev] [RFC v2 04/17] mempool: add op to populate objects using provided memory

Andrew Rybchenko arybchenko at solarflare.com
Thu Feb 1 09:51:27 CET 2018


On 01/31/2018 07:45 PM, Olivier Matz wrote:
> On Tue, Jan 23, 2018 at 01:15:59PM +0000, Andrew Rybchenko wrote:
>> The callback allows to customize how objects are stored in the
>> memory chunk. Default implementation of the callback which simply
>> puts objects one by one is available.
>>
>> Suggested-by: Olivier Matz <olivier.matz at 6wind.com>
>> Signed-off-by: Andrew Rybchenko <arybchenko at solarflare.com>
> ...
>
>>   
>> +int
>> +rte_mempool_populate_one_by_one(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)
> We shall find a better name for this function.
> Unfortunatly rte_mempool_populate_default() already exists...

I have no better idea right now, but we'll try in the next version.
May be rte_mempool_op_populate_default()?

> I'm also wondering if having a file rte_mempool_ops_default.c
> with all the default behaviors would make sense?

I think it is a good idea. Will do.

> ...
>
>> @@ -466,16 +487,13 @@ rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr,
>>   	else
>>   		off = RTE_PTR_ALIGN_CEIL(vaddr, RTE_CACHE_LINE_SIZE) - vaddr;
>>   
>> -	while (off + total_elt_sz <= len && mp->populated_size < mp->size) {
>> -		off += mp->header_size;
>> -		if (iova == RTE_BAD_IOVA)
>> -			mempool_add_elem(mp, (char *)vaddr + off,
>> -				RTE_BAD_IOVA);
>> -		else
>> -			mempool_add_elem(mp, (char *)vaddr + off, iova + off);
>> -		off += mp->elt_size + mp->trailer_size;
>> -		i++;
>> -	}
>> +	if (off > len)
>> +		return -EINVAL;
>> +
>> +	i = rte_mempool_ops_populate(mp, mp->size - mp->populated_size,
>> +		(char *)vaddr + off,
>> +		(iova == RTE_BAD_IOVA) ? RTE_BAD_IOVA : (iova + off),
>> +		len - off, mempool_add_elem);
> My initial idea was to provide populate_iova(), populate_virt(), ...
> as mempool ops. I don't see any strong requirement for doing it now, but
> on the other hand it would break the API to do it later. What's
> your opinion?

Suggested solution keeps only generic house-keeping inside
rte_mempool_populate_iova() (driver-data alloc/init, generic
check if the pool is already populated, maintenance of the memory
chunks list and object cache-alignment requirements). I think that
only the last item is questionable, but cache-line alignment is
hard-wired in object size calculation as well which is not
customizable yet. May be we should add callback for object size
calculation with default fallback and move object cache-line
alignment into populate() callback.

As for populate_virt() etc right now all these functions finally
come to populate_iova(). I have no customization usecases
for these functions in my mind, so it is hard to guess required
set of parameters. That's why I kept it as is for now.
(In general I prefer to avoid overkill solutions since chances
of success (100% guess of the prototype) are small)

May be someone else on the list have usecases in mind?

> Also, I see that mempool_add_elem() is passed as a callback to
> rte_mempool_ops_populate(). Instead, would it make sense to
> export mempool_add_elem() and let the implementation of populate()
> ops to call it?

I think callback gives a bit more freedom and allows to pass own
function which does some actions (e.g. filtering) per object.
In fact I think opaque parameter should be added to the callback
prototype to make it really useful for customization (to provide
specific context and make it possible to chain callbacks).


More information about the dev mailing list