[dpdk-dev] [RFC PATCH 2/6] mempool: implement clustered object allocation

[santosh]

On Wednesday 17 January 2018 08:33 PM, Andrew Rybchenko wrote:
> On 12/14/2017 04:37 PM, Olivier MATZ wrote:
>> On Fri, Nov 24, 2017 at 04:06:27PM +0000, Andrew Rybchenko wrote:
>>> From: "Artem V. Andreev" <Artem.Andreev at oktetlabs.ru>
>>>
>>> Clustered allocation is required to simplify packaging objects into
>>> buckets and search of the bucket control structure by an object.
>>>
>>> Signed-off-by: Artem V. Andreev <Artem.Andreev at oktetlabs.ru>
>>> Signed-off-by: Andrew Rybchenko <arybchenko at solarflare.com>
>>> ---
>>>   lib/librte_mempool/rte_mempool.c | 39 +++++++++++++++++++++++++++++++++++----
>>>   lib/librte_mempool/rte_mempool.h | 23 +++++++++++++++++++++--
>>>   test/test/test_mempool.c         |  2 +-
>>>   3 files changed, 57 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
>>> index d50dba4..43455a3 100644
>>> --- a/lib/librte_mempool/rte_mempool.c
>>> +++ b/lib/librte_mempool/rte_mempool.c
>>> @@ -239,7 +239,8 @@ rte_mempool_calc_obj_size(uint32_t elt_size, uint32_t flags,
>>>    */
>>>   size_t
>>>   rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift,
>>> -              unsigned int flags)
>>> +              unsigned int flags,
>>> +              const struct rte_mempool_info *info)
>>>   {
>>>       size_t obj_per_page, pg_num, pg_sz;
>>>       unsigned int mask;
>>> @@ -252,6 +253,17 @@ rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift,
>>>       if (total_elt_sz == 0)
>>>           return 0;
>>>   +    if (flags & MEMPOOL_F_CAPA_ALLOCATE_IN_CLUSTERS) {
>>> +        unsigned int align_shift =
>>> +            rte_bsf32(
>>> +                rte_align32pow2(total_elt_sz *
>>> +                        info->cluster_size));
>>> +        if (pg_shift < align_shift) {
>>> +            return ((elt_num / info->cluster_size) + 2)
>>> +                << align_shift;
>>> +        }
>>> +    }
>>> +
>> +Cc Santosh for this
>>
>> To be honnest, that was my fear when introducing
>> MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS and MEMPOOL_F_CAPA_PHYS_CONTIG to see more
>> and more specific flags in generic code.
>>
>> I feel that the hidden meaning of these flags is more "if driver == foo",
>> which shows that something is wrong is the current design.
>>
>> We have to think about another way to do. Let me try to propose
>> something (to be deepen).
>>
>> The standard way to create a mempool is:
>>
>>    mp = create_empty(...)
>>    set_ops_by_name(mp, "my-driver")    // optional
>>    populate_default(mp)                // or populate_*()
>>    obj_iter(mp, callback, arg)         // optional, to init objects
>>    // and optional local func to init mempool priv
>>
>> First, we can consider deprecating some APIs like:
>>   - rte_mempool_xmem_create()
>>   - rte_mempool_xmem_size()
>>   - rte_mempool_xmem_usage()
>>   - rte_mempool_populate_iova_tab()
>>
>> These functions were introduced for xen, which was recently
>> removed. They are complex to use, and are not used anywhere else in
>> DPDK.
>>
>> Then, instead of having flags (quite hard to understand without knowing
>> the underlying driver), we can let the mempool drivers do the
>> populate_default() operation. For that we can add a populate_default
>> field in mempool ops. Same for populate_virt(), populate_anon(), and
>> populate_phys() which can return -ENOTSUP if this is not
>> implemented/implementable on a specific driver, or if flags
>> (NO_CACHE_ALIGN, NO_SPREAD, ...) are not supported. If the function
>> pointer is NULL, use the generic function.
>>
>> Thanks to this, the generic code would remain understandable and won't
>> have to care about how memory should be allocated for a specific driver.
>>
>> Thoughts?
>
> Yes, I agree. This week we'll provide updated version of the RFC which
> covers it including transition of the mempool/octeontx. I think it is sufficient
> to introduce two new ops:
>  1. To calculate memory space required to store specified number of objects
>  2. To populate objects in the provided memory chunk (the op will be called
>      from rte_mempool_populate_iova() which is a leaf function for all
>      rte_mempool_populate_*() calls.
> It will allow to avoid duplication and keep memchunks housekeeping inside
> mempool library.
>
There is also a downside of letting mempool driver to populate, which was raised in other thread.
http://dpdk.org/dev/patchwork/patch/31943/

Thanks.