[dpdk-dev] [PATCH 3/5] mempool: remove optimistic IOVA-contiguous allocation

Andrew Rybchenko arybchenko at solarflare.com
Wed Oct 30 08:44:04 CET 2019


On 10/30/19 10:36 AM, Andrew Rybchenko wrote:
> On 10/29/19 8:20 PM, Olivier Matz wrote:
>> On Tue, Oct 29, 2019 at 01:25:10PM +0300, Andrew Rybchenko wrote:
>>> On 10/28/19 5:01 PM, Olivier Matz wrote:
>>>> The previous commit reduced the amount of required memory when
>>>> populating the mempool with non iova-contiguous memory.
>>>>
>>>> Since there is no big advantage to have a fully iova-contiguous 
>>>> mempool
>>>> if it is not explicitly asked, remove this code, it simplifies the
>>>> populate function.
>>>>
>>>> Signed-off-by: Olivier Matz <olivier.matz at 6wind.com>
>>> One comment below, other than that
>>> Reviewed-by: Andrew Rybchenko <arybchenko at solarflare.com>
>>>
>>>> ---
>>>>    lib/librte_mempool/rte_mempool.c | 47 
>>>> ++++++--------------------------
>>>>    1 file changed, 8 insertions(+), 39 deletions(-)
>>>>
>>>> diff --git a/lib/librte_mempool/rte_mempool.c 
>>>> b/lib/librte_mempool/rte_mempool.c
>>>> index 3275e48c9..5e1f202dc 100644
>>>> --- a/lib/librte_mempool/rte_mempool.c
>>>> +++ b/lib/librte_mempool/rte_mempool.c
>>> [snip]
>>>
>>>> @@ -531,36 +521,15 @@ rte_mempool_populate_default(struct 
>>>> rte_mempool *mp)
>>>>                goto fail;
>>>>            }
>>>> -        flags = mz_flags;
>>>> -
>>>>            /* if we're trying to reserve contiguous memory, add 
>>>> appropriate
>>>>             * memzone flag.
>>>>             */
>>>> -        if (try_iova_contig_mempool)
>>>> -            flags |= RTE_MEMZONE_IOVA_CONTIG;
>>>> +        if (min_chunk_size == (size_t)mem_size)
>>>> +            mz_flags |= RTE_MEMZONE_IOVA_CONTIG;
>>>>            mz = rte_memzone_reserve_aligned(mz_name, mem_size,
>>>> -                mp->socket_id, flags, align);
>>>> -
>>>> -        /* if we were trying to allocate contiguous memory, failed 
>>>> and
>>>> -         * minimum required contiguous chunk fits minimum page, 
>>>> adjust
>>>> -         * memzone size to the page size, and try again.
>>>> -         */
>>>> -        if (mz == NULL && try_iova_contig_mempool &&
>>>> -                min_chunk_size <= pg_sz) {
>>>> -            try_iova_contig_mempool = false;
>>>> -            flags &= ~RTE_MEMZONE_IOVA_CONTIG;
>>>> -
>>>> -            mem_size = rte_mempool_ops_calc_mem_size(mp, n,
>>>> -                    pg_shift, &min_chunk_size, &align);
>>>> -            if (mem_size < 0) {
>>>> -                ret = mem_size;
>>>> -                goto fail;
>>>> -            }
>>>> +                mp->socket_id, mz_flags, align);
>>>> -            mz = rte_memzone_reserve_aligned(mz_name, mem_size,
>>>> -                mp->socket_id, flags, align);
>>>> -        }
>>>>            /* don't try reserving with 0 size if we were asked to 
>>>> reserve
>>>>             * IOVA-contiguous memory.
>>>>             */
>>> [snip]
>>>
>>>> @@ -587,7 +556,7 @@ rte_mempool_populate_default(struct rte_mempool 
>>>> *mp)
>>>>            else
>>>>                iova = RTE_BAD_IOVA;
>>>> -        if (try_iova_contig_mempool || pg_sz == 0)
>>>> +        if (pg_sz == 0)
>>> I think (mz_flags & RTE_MEMZONE_IOVA_CONTIG) is lost here.
>> Do you mean we should do instead
>>     if (pg_sz == 0 || (mz_flags & RTE_MEMZONE_IOVA_CONTIG))
>>         populate_iova()
>>     else
>>         populate_virt()
>> ?
>
> Yes.
>
>> I would say yes, but it should be removed in patch 5/5.
>> At the end, we don't want objects to be across pages, even
>> with RTE_MEMZONE_IOVA_CONTIG.

Thinking more about it I don't understand why. If we know that
the memzone is IOVA contiguous, why we should not populate
it this way. It does not prevent patch 5/5 to do the job.

>>>>                ret = rte_mempool_populate_iova(mp, mz->addr,
>>>>                    iova, mz->len,
>>>>                    rte_mempool_memchunk_mz_free,



More information about the dev mailing list