DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] mem: skip memory locking on failure
@ 2016-06-13 10:26 Olivier Matz
  2016-06-14 13:21 ` Panu Matilainen
  2016-06-27 15:58 ` [dpdk-dev] [PATCH v2] mem: revert page locking when not using hugepages Olivier Matz
  0 siblings, 2 replies; 6+ messages in thread
From: Olivier Matz @ 2016-06-13 10:26 UTC (permalink / raw)
  To: sergio.gonzalez.monroy, david.marchand, pmatilai, thomas.monjalon, dev

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));
-- 
2.8.0.rc3

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] mem: skip memory locking on failure
  2016-06-13 10:26 [dpdk-dev] [PATCH] mem: skip memory locking on failure Olivier Matz
@ 2016-06-14 13:21 ` Panu Matilainen
  2016-06-14 14:12   ` Olivier MATZ
  2016-06-27 15:58 ` [dpdk-dev] [PATCH v2] mem: revert page locking when not using hugepages Olivier Matz
  1 sibling, 1 reply; 6+ messages in thread
From: Panu Matilainen @ 2016-06-14 13:21 UTC (permalink / raw)
  To: Olivier Matz, sergio.gonzalez.monroy, david.marchand,
	thomas.monjalon, dev

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 :)

Are there actual plans to make --no-huge work with real devices? If not 
then documenting --no-huge to imply unlocked memory is one option I guess.

	- Panu -

	- Panu -

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] mem: skip memory locking on failure
  2016-06-14 13:21 ` Panu Matilainen
@ 2016-06-14 14:12   ` Olivier MATZ
  2016-06-21 11:58     ` Panu Matilainen
  0 siblings, 1 reply; 6+ messages in thread
From: Olivier MATZ @ 2016-06-14 14:12 UTC (permalink / raw)
  To: Panu Matilainen, sergio.gonzalez.monroy, david.marchand,
	thomas.monjalon, dev, Mcnamara, John

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] mem: skip memory locking on failure
  2016-06-14 14:12   ` Olivier MATZ
@ 2016-06-21 11:58     ` Panu Matilainen
  0 siblings, 0 replies; 6+ messages in thread
From: Panu Matilainen @ 2016-06-21 11:58 UTC (permalink / raw)
  To: Olivier MATZ, sergio.gonzalez.monroy, david.marchand,
	thomas.monjalon, dev, Mcnamara, John

On 06/14/2016 05:12 PM, Olivier MATZ wrote:
> 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.

I suppose you mean revert the memlock commit, ie this?

commit 593a084afc2b441895aeca78a2c4465e450d0ef5
Author: Olivier Matz <olivier.matz@6wind.com>
Date:   Wed May 18 13:04:42 2016 +0200

     mem: lock pages when not using hugepages

Reverting that would help in the sense that then we could make the 
test-suite runnable by regular users (I've some patches for this), and 
once that is in place it would sort of force dealing with the issue one 
way or the other in future work in this area :)

>
>> 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

Right, so it wouldn't be a regression at least.

	- Panu -

>
> 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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [dpdk-dev] [PATCH v2] mem: revert page locking when not using hugepages
  2016-06-13 10:26 [dpdk-dev] [PATCH] mem: skip memory locking on failure Olivier Matz
  2016-06-14 13:21 ` Panu Matilainen
@ 2016-06-27 15:58 ` Olivier Matz
  2016-06-30 17:17   ` Thomas Monjalon
  1 sibling, 1 reply; 6+ messages in thread
From: Olivier Matz @ 2016-06-27 15:58 UTC (permalink / raw)
  To: dev, sergio.gonzalez.monroy, david.marchand, pmatilai, thomas.monjalon

This reverts commit 593a084afc2b441895aeca78a2c4465e450d0ef5.

Since recently [1], it is not possible to run the dpdk with
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,
for instance because he uses virtual drivers.

So this commit reverts the use of MAP_LOCKED in mmap() flags.

[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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
index 9251a5b..1fc7545 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memory.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
@@ -1074,7 +1074,7 @@ rte_eal_hugepage_init(void)
 	/* hugetlbfs can be disabled */
 	if (internal_config.no_hugetlbfs) {
 		addr = mmap(NULL, internal_config.memory, PROT_READ | PROT_WRITE,
-			MAP_LOCKED | MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
+				MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
 		if (addr == MAP_FAILED) {
 			RTE_LOG(ERR, EAL, "%s: mmap() failed: %s\n", __func__,
 					strerror(errno));
-- 
2.8.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH v2] mem: revert page locking when not using hugepages
  2016-06-27 15:58 ` [dpdk-dev] [PATCH v2] mem: revert page locking when not using hugepages Olivier Matz
@ 2016-06-30 17:17   ` Thomas Monjalon
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Monjalon @ 2016-06-30 17:17 UTC (permalink / raw)
  To: Olivier Matz; +Cc: dev, sergio.gonzalez.monroy, david.marchand, pmatilai

2016-06-27 17:58, Olivier Matz:
> This reverts commit 593a084afc2b441895aeca78a2c4465e450d0ef5.
> 
> Since recently [1], it is not possible to run the dpdk with
> 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,
> for instance because he uses virtual drivers.
> 
> So this commit reverts the use of MAP_LOCKED in mmap() flags.
> 
> [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>

Applied, thanks

Let's keep in mind to make things working with --no-huge.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-06-30 17:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-13 10:26 [dpdk-dev] [PATCH] mem: skip memory locking on failure Olivier Matz
2016-06-14 13:21 ` Panu Matilainen
2016-06-14 14:12   ` Olivier MATZ
2016-06-21 11:58     ` Panu Matilainen
2016-06-27 15:58 ` [dpdk-dev] [PATCH v2] mem: revert page locking when not using hugepages Olivier Matz
2016-06-30 17:17   ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).