[dpdk-stable] [PATCH] mem: don't use address hint for mapping unless requested

[dpdk-stable] [PATCH] mem: don't use address hint for mapping unless requested

[Shahaf Shuler]

patch[1] added an address hint as starting address for 64 bit systems in
case an explicit base virtual address was not set by the user.

The justification for such hint was to help devices that work in VA
mode and has a address range limitation to work smoothly with the eal
memory subsystem.

While the base address value selected may work fine for the eal
initialization, it easily breaks when trying to register external memory
using rte_extmem_register API.

Trying to register anonymous memory on RH x86_64 machine took several
minutes, during them the function eal_get_virtual_area repeatedly
scanned for a good VA candidate.

The attempt to guess which VA address will be free for mapping will
always result in not portable, error prone code:
* different application may use different libraries along w/ DPDK. One
  can never guess which library was called first and how much virtual
  memory it consumed.
* external memory can be registered at any time in the application run
  time.

This patch removes the default address hint and use the address returned
by mmap.
devices with address limitations should suggest to their users a proper
base-virtaddr (EAL arg) to use.

Fixes: 1df21702873d ("mem: use address hint for mapping hugepages")
Cc: stable@dpdk.org
Cc: alejandro.lucero@netronome.com

[1] commit 1df21702873d ("mem: use address hint for mapping hugepages")

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
 lib/librte_eal/common/eal_common_memory.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c
index c9da69b164..09108f7a32 100644
--- a/lib/librte_eal/common/eal_common_memory.c
+++ b/lib/librte_eal/common/eal_common_memory.c
@@ -39,23 +39,6 @@
 static void *next_baseaddr;
 static uint64_t system_page_sz;
 
-#ifdef RTE_ARCH_64
-/*
- * Linux kernel uses a really high address as starting address for serving
- * mmaps calls. If there exists addressing limitations and IOVA mode is VA,
- * this starting address is likely too high for those devices. However, it
- * is possible to use a lower address in the process virtual address space
- * as with 64 bits there is a lot of available space.
- *
- * Current known limitations are 39 or 40 bits. Setting the starting address
- * at 4GB implies there are 508GB or 1020GB for mapping the available
- * hugepages. This is likely enough for most systems, although a device with
- * addressing limitations should call rte_mem_check_dma_mask for ensuring all
- * memory is within supported range.
- */
-static uint64_t baseaddr = 0x100000000;
-#endif
-
 void *
 eal_get_virtual_area(void *requested_addr, size_t *size,
 		size_t page_sz, int flags, int mmap_flags)
@@ -79,11 +62,6 @@ eal_get_virtual_area(void *requested_addr, size_t *size,
 			rte_eal_process_type() == RTE_PROC_PRIMARY)
 		next_baseaddr = (void *) internal_config.base_virtaddr;
 
-#ifdef RTE_ARCH_64
-	if (next_baseaddr == NULL && internal_config.base_virtaddr == 0 &&
-			rte_eal_process_type() == RTE_PROC_PRIMARY)
-		next_baseaddr = (void *) baseaddr;
-#endif
 	if (requested_addr == NULL && next_baseaddr != NULL) {
 		requested_addr = next_baseaddr;
 		requested_addr = RTE_PTR_ALIGN(requested_addr, page_sz);
-- 
2.12.0

Re: [dpdk-stable] [dpdk-dev] [PATCH] mem: don't use address hint for mapping unless requested

[Shahaf Shuler]

Anatoly, Alejandro,

Any comments on this one?

Thursday, March 21, 2019 10:22 PM, Shahaf Shuler:
> Subject: [dpdk-dev] [PATCH] mem: don't use address hint for mapping
> unless requested
> 
> patch[1] added an address hint as starting address for 64 bit systems in case
> an explicit base virtual address was not set by the user.
> 
> The justification for such hint was to help devices that work in VA mode and
> has a address range limitation to work smoothly with the eal memory
> subsystem.
> 
> While the base address value selected may work fine for the eal initialization,
> it easily breaks when trying to register external memory using
> rte_extmem_register API.
> 
> Trying to register anonymous memory on RH x86_64 machine took several
> minutes, during them the function eal_get_virtual_area repeatedly scanned
> for a good VA candidate.
> 
> The attempt to guess which VA address will be free for mapping will always
> result in not portable, error prone code:
> * different application may use different libraries along w/ DPDK. One
>   can never guess which library was called first and how much virtual
>   memory it consumed.
> * external memory can be registered at any time in the application run
>   time.
> 
> This patch removes the default address hint and use the address returned by
> mmap.
> devices with address limitations should suggest to their users a proper base-
> virtaddr (EAL arg) to use.
> 
> Fixes: 1df21702873d ("mem: use address hint for mapping hugepages")
> Cc: stable@dpdk.org
> Cc: alejandro.lucero@netronome.com
> 
> [1] commit 1df21702873d ("mem: use address hint for mapping hugepages")
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> ---
>  lib/librte_eal/common/eal_common_memory.c | 22 ----------------------
>  1 file changed, 22 deletions(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_memory.c
> b/lib/librte_eal/common/eal_common_memory.c
> index c9da69b164..09108f7a32 100644
> --- a/lib/librte_eal/common/eal_common_memory.c
> +++ b/lib/librte_eal/common/eal_common_memory.c
> @@ -39,23 +39,6 @@
>  static void *next_baseaddr;
>  static uint64_t system_page_sz;
> 
> -#ifdef RTE_ARCH_64
> -/*
> - * Linux kernel uses a really high address as starting address for serving
> - * mmaps calls. If there exists addressing limitations and IOVA mode is VA,
> - * this starting address is likely too high for those devices. However, it
> - * is possible to use a lower address in the process virtual address space
> - * as with 64 bits there is a lot of available space.
> - *
> - * Current known limitations are 39 or 40 bits. Setting the starting address
> - * at 4GB implies there are 508GB or 1020GB for mapping the available
> - * hugepages. This is likely enough for most systems, although a device with
> - * addressing limitations should call rte_mem_check_dma_mask for
> ensuring all
> - * memory is within supported range.
> - */
> -static uint64_t baseaddr = 0x100000000; -#endif
> -
>  void *
>  eal_get_virtual_area(void *requested_addr, size_t *size,
>  		size_t page_sz, int flags, int mmap_flags) @@ -79,11 +62,6
> @@ eal_get_virtual_area(void *requested_addr, size_t *size,
>  			rte_eal_process_type() == RTE_PROC_PRIMARY)
>  		next_baseaddr = (void *) internal_config.base_virtaddr;
> 
> -#ifdef RTE_ARCH_64
> -	if (next_baseaddr == NULL && internal_config.base_virtaddr == 0 &&
> -			rte_eal_process_type() == RTE_PROC_PRIMARY)
> -		next_baseaddr = (void *) baseaddr;
> -#endif
>  	if (requested_addr == NULL && next_baseaddr != NULL) {
>  		requested_addr = next_baseaddr;
>  		requested_addr = RTE_PTR_ALIGN(requested_addr,
> page_sz);
> --
> 2.12.0

Re: [dpdk-stable] [PATCH] mem: don't use address hint for mapping unless requested

[Burakov, Anatoly]

On 21-Mar-19 8:21 PM, Shahaf Shuler wrote:
> patch[1] added an address hint as starting address for 64 bit systems in
> case an explicit base virtual address was not set by the user.
> 
> The justification for such hint was to help devices that work in VA
> mode and has a address range limitation to work smoothly with the eal
> memory subsystem.
> 
> While the base address value selected may work fine for the eal
> initialization, it easily breaks when trying to register external memory
> using rte_extmem_register API.
> 
> Trying to register anonymous memory on RH x86_64 machine took several
> minutes, during them the function eal_get_virtual_area repeatedly
> scanned for a good VA candidate.
> 
> The attempt to guess which VA address will be free for mapping will
> always result in not portable, error prone code:
> * different application may use different libraries along w/ DPDK. One
>    can never guess which library was called first and how much virtual
>    memory it consumed.
> * external memory can be registered at any time in the application run
>    time.
> 
> This patch removes the default address hint and use the address returned
> by mmap.
> devices with address limitations should suggest to their users a proper
> base-virtaddr (EAL arg) to use.
> 
> Fixes: 1df21702873d ("mem: use address hint for mapping hugepages")
> Cc: stable@dpdk.org
> Cc: alejandro.lucero@netronome.com
> 
> [1] commit 1df21702873d ("mem: use address hint for mapping hugepages")
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> ---

I understand the motivation, however this will lower the reliability of 
secondary process initialization. Perhaps, the hinting could be improved 
by not trying to use the hint multiple times, but rather just once? I.e. 
if using hint succeeded - great, if not - try without one. Most of the 
time hint will work correctly, i think.

-- 
Thanks,
Anatoly

Re: [dpdk-stable] [PATCH] mem: don't use address hint for mapping unless requested

[Shahaf Shuler]

Wednesday, March 27, 2019 1:50 PM, Burakov, Anatoly:
> Subject: Re: [PATCH] mem: don't use address hint for mapping unless
> requested
> 
> On 21-Mar-19 8:21 PM, Shahaf Shuler wrote:
> > patch[1] added an address hint as starting address for 64 bit systems
> > in case an explicit base virtual address was not set by the user.
> >
> > The justification for such hint was to help devices that work in VA
> > mode and has a address range limitation to work smoothly with the eal
> > memory subsystem.
> >
> > While the base address value selected may work fine for the eal
> > initialization, it easily breaks when trying to register external
> > memory using rte_extmem_register API.
> >
> > Trying to register anonymous memory on RH x86_64 machine took several
> > minutes, during them the function eal_get_virtual_area repeatedly
> > scanned for a good VA candidate.
> >
> > The attempt to guess which VA address will be free for mapping will
> > always result in not portable, error prone code:
> > * different application may use different libraries along w/ DPDK. One
> >    can never guess which library was called first and how much virtual
> >    memory it consumed.
> > * external memory can be registered at any time in the application run
> >    time.
> >
> > This patch removes the default address hint and use the address
> > returned by mmap.
> > devices with address limitations should suggest to their users a
> > proper base-virtaddr (EAL arg) to use.
> >
> > Fixes: 1df21702873d ("mem: use address hint for mapping hugepages")
> > Cc: stable@dpdk.org
> > Cc: alejandro.lucero@netronome.com
> >
> > [1] commit 1df21702873d ("mem: use address hint for mapping
> > hugepages")
> >
> > Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> > ---
> 
> I understand the motivation, however this will lower the reliability of
> secondary process initialization. 

Can you explain why?
It is very bad if we base the DPDK secondary process design on some predefined VA address base to be available. 

Perhaps, the hinting could be improved by
> not trying to use the hint multiple times, but rather just once? I.e.
> if using hint succeeded - great, if not - try without one. Most of the time hint
> will work correctly, i think.
> 
> --
> Thanks,
> Anatoly

Re: [dpdk-stable] [PATCH] mem: don't use address hint for mapping unless requested

[Burakov, Anatoly]

On 28-Mar-19 8:45 AM, Shahaf Shuler wrote:
> Wednesday, March 27, 2019 1:50 PM, Burakov, Anatoly:
>> Subject: Re: [PATCH] mem: don't use address hint for mapping unless
>> requested
>>
>> On 21-Mar-19 8:21 PM, Shahaf Shuler wrote:
>>> patch[1] added an address hint as starting address for 64 bit systems
>>> in case an explicit base virtual address was not set by the user.
>>>
>>> The justification for such hint was to help devices that work in VA
>>> mode and has a address range limitation to work smoothly with the eal
>>> memory subsystem.
>>>
>>> While the base address value selected may work fine for the eal
>>> initialization, it easily breaks when trying to register external
>>> memory using rte_extmem_register API.
>>>
>>> Trying to register anonymous memory on RH x86_64 machine took several
>>> minutes, during them the function eal_get_virtual_area repeatedly
>>> scanned for a good VA candidate.
>>>
>>> The attempt to guess which VA address will be free for mapping will
>>> always result in not portable, error prone code:
>>> * different application may use different libraries along w/ DPDK. One
>>>     can never guess which library was called first and how much virtual
>>>     memory it consumed.
>>> * external memory can be registered at any time in the application run
>>>     time.
>>>
>>> This patch removes the default address hint and use the address
>>> returned by mmap.
>>> devices with address limitations should suggest to their users a
>>> proper base-virtaddr (EAL arg) to use.
>>>
>>> Fixes: 1df21702873d ("mem: use address hint for mapping hugepages")
>>> Cc: stable@dpdk.org
>>> Cc: alejandro.lucero@netronome.com
>>>
>>> [1] commit 1df21702873d ("mem: use address hint for mapping
>>> hugepages")
>>>
>>> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
>>> ---
>>
>> I understand the motivation, however this will lower the reliability of
>> secondary process initialization.
> 
> Can you explain why?
> It is very bad if we base the DPDK secondary process design on some predefined VA address base to be available.

Our whole secondary process is "very bad design" in the first place, 
because it relies on address space duplication. This change was 
unrelated to it, but it has been shown that setting address hint to this 
value (or setting --base-virtaddr to the same address) improves success 
rate for secondary process initialization to the point where it doesn't 
even register in our internal bug tracker (whereas previously i did 
routinely get pings about secondary process init failing).

This is just something we have to live with, and this change has been a 
net positive as far as reliability of secondary process initialization goes.

> 
> Perhaps, the hinting could be improved by
>> not trying to use the hint multiple times, but rather just once? I.e.
>> if using hint succeeded - great, if not - try without one. Most of the time hint
>> will work correctly, i think.
>>
>> --
>> Thanks,
>> Anatoly


-- 
Thanks,
Anatoly

[dpdk-stable] [PATCH v2] mem: limit use of address hint

[Shahaf Shuler]

patch[1] added an address hint as starting address for 64 bit systems in
case an explicit base virtual address was not set by the user.

The justification for such hint was to help devices that work in VA
mode and has a address range limitation to work smoothly with the eal
memory subsystem.

While the base address value selected may work fine for the eal
initialization, it easily breaks when trying to register external memory
using rte_extmem_register API.

Trying to register anonymous memory on RH x86_64 machine took several
minutes, during them the function eal_get_virtual_area repeatedly
scanned for a good VA candidate.

The attempt to guess which VA address will be free for mapping will
always result in not portable, error prone code:
* different application may use different libraries along w/ DPDK. One
  can never guess which library was called first and how much virtual
  memory it consumed.
* external memory can be registered at any time in the application run
  time.

In order not to break the existing secondary process design, this patch
only limits the max number of tries that will be done with the
address hint.
When the number of tries exceeds the threshold the code
will use the suggested address from kernel.

Fixes: 1df21702873d ("mem: use address hint for mapping hugepages")
Cc: stable@dpdk.org
Cc: alejandro.lucero@netronome.com

[1] commit 1df21702873d ("mem: use address hint for mapping hugepages")

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---

On v2:
 * instead of a complete remove of the hint limit the number of tries we allow.
---
 lib/librte_eal/common/eal_common_memory.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c
index c9da69b164..5ae8d0124d 100644
--- a/lib/librte_eal/common/eal_common_memory.c
+++ b/lib/librte_eal/common/eal_common_memory.c
@@ -56,6 +56,7 @@ static uint64_t system_page_sz;
 static uint64_t baseaddr = 0x100000000;
 #endif
 
+#define MAX_MMAP_WITH_DEFINED_ADDR_TRIES 5
 void *
 eal_get_virtual_area(void *requested_addr, size_t *size,
 		size_t page_sz, int flags, int mmap_flags)
@@ -63,6 +64,7 @@ eal_get_virtual_area(void *requested_addr, size_t *size,
 	bool addr_is_hint, allow_shrink, unmap, no_align;
 	uint64_t map_sz;
 	void *mapped_addr, *aligned_addr;
+	uint8_t try = 0;
 
 	if (system_page_sz == 0)
 		system_page_sz = sysconf(_SC_PAGESIZE);
@@ -118,11 +120,14 @@ eal_get_virtual_area(void *requested_addr, size_t *size,
 
 		if (mapped_addr != MAP_FAILED && addr_is_hint &&
 		    mapped_addr != requested_addr) {
-			/* hint was not used. Try with another offset */
-			munmap(mapped_addr, map_sz);
-			mapped_addr = MAP_FAILED;
+			try++;
 			next_baseaddr = RTE_PTR_ADD(next_baseaddr, page_sz);
-			requested_addr = next_baseaddr;
+			if (try <= MAX_MMAP_WITH_DEFINED_ADDR_TRIES) {
+				/* hint was not used. Try with another offset */
+				munmap(mapped_addr, map_sz);
+				mapped_addr = MAP_FAILED;
+				requested_addr = next_baseaddr;
+			}
 		}
 	} while ((allow_shrink || addr_is_hint) &&
 		 mapped_addr == MAP_FAILED && *size > 0);
-- 
2.12.0

Re: [dpdk-stable] [PATCH v2] mem: limit use of address hint

[Burakov, Anatoly]

On 31-Mar-19 9:43 AM, Shahaf Shuler wrote:
> patch[1] added an address hint as starting address for 64 bit systems in
> case an explicit base virtual address was not set by the user.
> 
> The justification for such hint was to help devices that work in VA
> mode and has a address range limitation to work smoothly with the eal
> memory subsystem.
> 
> While the base address value selected may work fine for the eal
> initialization, it easily breaks when trying to register external memory
> using rte_extmem_register API.
> 
> Trying to register anonymous memory on RH x86_64 machine took several
> minutes, during them the function eal_get_virtual_area repeatedly
> scanned for a good VA candidate.
> 
> The attempt to guess which VA address will be free for mapping will
> always result in not portable, error prone code:
> * different application may use different libraries along w/ DPDK. One
>    can never guess which library was called first and how much virtual
>    memory it consumed.
> * external memory can be registered at any time in the application run
>    time.
> 
> In order not to break the existing secondary process design, this patch
> only limits the max number of tries that will be done with the
> address hint.
> When the number of tries exceeds the threshold the code
> will use the suggested address from kernel.
> 
> Fixes: 1df21702873d ("mem: use address hint for mapping hugepages")
> Cc: stable@dpdk.org
> Cc: alejandro.lucero@netronome.com
> 
> [1] commit 1df21702873d ("mem: use address hint for mapping hugepages")
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> ---
> 
> On v2:
>   * instead of a complete remove of the hint limit the number of tries we allow.
> ---

LGTM

Tested-by: Anatoly Burakov <anatoly.burakov@intel.com>

We can always increase the number of tries later :)

-- 
Thanks,
Anatoly

Re: [dpdk-stable] [PATCH v2] mem: limit use of address hint

[Alejandro Lucero]

On Tue, Apr 2, 2019 at 5:13 PM Burakov, Anatoly <anatoly.burakov@intel.com>
wrote:

> On 31-Mar-19 9:43 AM, Shahaf Shuler wrote:
> > patch[1] added an address hint as starting address for 64 bit systems in
> > case an explicit base virtual address was not set by the user.
> >
> > The justification for such hint was to help devices that work in VA
> > mode and has a address range limitation to work smoothly with the eal
> > memory subsystem.
> >
> > While the base address value selected may work fine for the eal
> > initialization, it easily breaks when trying to register external memory
> > using rte_extmem_register API.
> >
> > Trying to register anonymous memory on RH x86_64 machine took several
> > minutes, during them the function eal_get_virtual_area repeatedly
> > scanned for a good VA candidate.
> >
> > The attempt to guess which VA address will be free for mapping will
> > always result in not portable, error prone code:
> > * different application may use different libraries along w/ DPDK. One
> >    can never guess which library was called first and how much virtual
> >    memory it consumed.
> > * external memory can be registered at any time in the application run
> >    time.
> >
> > In order not to break the existing secondary process design, this patch
> > only limits the max number of tries that will be done with the
> > address hint.
> > When the number of tries exceeds the threshold the code
> > will use the suggested address from kernel.
> >
> > Fixes: 1df21702873d ("mem: use address hint for mapping hugepages")
> > Cc: stable@dpdk.org
> > Cc: alejandro.lucero@netronome.com
> >
> > [1] commit 1df21702873d ("mem: use address hint for mapping hugepages")
> >
> > Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> > ---
> >
> > On v2:
> >   * instead of a complete remove of the hint limit the number of tries
> we allow.
> > ---
>
> LGTM
>
> Tested-by: Anatoly Burakov <anatoly.burakov@intel.com>
>
> We can always increase the number of tries later :)
>
>
This is also fine for me.
If the map address is not within the supported range by a device with
addressing limitations, the device will not be used.
Not sure how this is likely to happen, but I guess if it is become a
problem, another solution should be implemented.

Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>



> --
> Thanks,
> Anatoly
>

Re: [dpdk-stable] [PATCH v2] mem: limit use of address hint

[Thomas Monjalon]

02/04/2019 19:23, Alejandro Lucero:
> On Tue, Apr 2, 2019 at 5:13 PM Burakov, Anatoly <anatoly.burakov@intel.com>
> wrote:
> > On 31-Mar-19 9:43 AM, Shahaf Shuler wrote:
> > > patch[1] added an address hint as starting address for 64 bit systems in
> > > case an explicit base virtual address was not set by the user.
> > >
> > > The justification for such hint was to help devices that work in VA
> > > mode and has a address range limitation to work smoothly with the eal
> > > memory subsystem.
> > >
> > > While the base address value selected may work fine for the eal
> > > initialization, it easily breaks when trying to register external memory
> > > using rte_extmem_register API.
> > >
> > > Trying to register anonymous memory on RH x86_64 machine took several
> > > minutes, during them the function eal_get_virtual_area repeatedly
> > > scanned for a good VA candidate.
> > >
> > > The attempt to guess which VA address will be free for mapping will
> > > always result in not portable, error prone code:
> > > * different application may use different libraries along w/ DPDK. One
> > >    can never guess which library was called first and how much virtual
> > >    memory it consumed.
> > > * external memory can be registered at any time in the application run
> > >    time.
> > >
> > > In order not to break the existing secondary process design, this patch
> > > only limits the max number of tries that will be done with the
> > > address hint.
> > > When the number of tries exceeds the threshold the code
> > > will use the suggested address from kernel.
> > >
> > > Fixes: 1df21702873d ("mem: use address hint for mapping hugepages")
> > > Cc: stable@dpdk.org
> > > Cc: alejandro.lucero@netronome.com
> > >
> > > [1] commit 1df21702873d ("mem: use address hint for mapping hugepages")
> > >
> > > Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> > > ---
> > >
> > > On v2:
> > >   * instead of a complete remove of the hint limit the number of tries
> > we allow.
> > > ---
> >
> > LGTM
> >
> > Tested-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >
> > We can always increase the number of tries later :)
> >
> This is also fine for me.
> If the map address is not within the supported range by a device with
> addressing limitations, the device will not be used.
> Not sure how this is likely to happen, but I guess if it is become a
> problem, another solution should be implemented.
> 
> Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>

Applied, thanks