Re: [PATCH] mem: skip memory locking on failure

[Olivier MATZ <olivier.matz@6wind.com>]

Hi Panu,

On 06/14/2016 03:21 PM, Panu Matilainen wrote:
> On 06/13/2016 01:26 PM, Olivier Matz wrote:
>> Since recently [1], it is not possible to run the dpdk with user
>> (non-root) privileges and the --no-huge option. This is because the eal
>> layer tries to lock the memory. Using locked memory is mandatory for
>> physical devices because they reference physical addresses.
>>
>> But a user may want to start the dpdk without locked memory, because he
>> does not have the permission to do so, and/or does not have this need.
>>
>> Moreover, the option --no-huge is still not functional today since the
>> physical memory address is not properly filled, so the initial patch is
>> not really useful.
>>
>> This commit fixes this issue by retrying the mmap() wihout the
>> MAP_LOCKED flag if the first mmap() failed.
>>
>> [1] http://www.dpdk.org/ml/archives/dev/2016-May/039404.html
>>
>> Fixes: 593a084afc2b ("mem: lock pages when not using hugepages")
>> Reported-by: Panu Matilainen <pmatilai@redhat.com>
>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>> ---
>>  lib/librte_eal/linuxapp/eal/eal_memory.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c
>> b/lib/librte_eal/linuxapp/eal/eal_memory.c
>> index 79d1d2d..08692d1 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
>> @@ -1075,6 +1075,15 @@ rte_eal_hugepage_init(void)
>>      if (internal_config.no_hugetlbfs) {
>>          addr = mmap(NULL, internal_config.memory, PROT_READ |
>> PROT_WRITE,
>>              MAP_LOCKED | MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
>> +        /* retry without MAP_LOCKED */
>> +        if (addr == MAP_FAILED && errno == EAGAIN) {
>> +            addr = mmap(NULL, internal_config.memory,
>> +                PROT_READ | PROT_WRITE,
>> +                MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
>> +            if (addr != MAP_FAILED)
>> +                RTE_LOG(NOTICE, EAL,
>> +                    "Cannot lock memory: don't use physical devices\n");
>> +        }
>>          if (addr == MAP_FAILED) {
>>              RTE_LOG(ERR, EAL, "%s: mmap() failed: %s\n", __func__,
>>                      strerror(errno));
>>
>
> I'm not really that familiar with dpdk memory usage, but gut feeling
> says such a thing needs to be explicit - either you explicitly ask for
> memory that doesn't need to be locked, or this simply fails with no
> retries. Or something like that. But "maybe I did, maybe I didn't"
> doesn't seem like very good API semantics to me :)

Yes, you're right. Anyway as this commit is not useful today,
it would be better to revert it.


> Are there actual plans to make --no-huge work with real devices?

I think this is something that could be part of the memory
rework referenced by Thomas:
http://dpdk.org/ml/archives/dev/2016-April/037444.html

I don't know if it's planified yet.


> If not then documenting --no-huge to imply unlocked memory is one
> option I guess.

There is already some words in the known issues:
http://dpdk.org/doc/guides/rel_notes/known_issues.html?highlight=known%20issues#pmd-does-not-work-with-no-huge-eal-command-line-parameter

Maybe we could add something somewhere else, but I did not find
any doc referencing eal options. Only a guide for testpmd here:
http://dpdk.org/doc/guides/testpmd_app_ug/run_app.html#eal-command-line-options

John, maybe you have an idea?

Thanks
Olivier