DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 21.02] mem: don't warn about base addr if not requested
@ 2020-11-09 15:27 Anatoly Burakov
  2020-11-09 15:47 ` [dpdk-dev] [PATCH 21.02 v2] " Anatoly Burakov
  0 siblings, 1 reply; 5+ messages in thread
From: Anatoly Burakov @ 2020-11-09 15:27 UTC (permalink / raw)
  To: dev; +Cc: Damjan Marion

Any EAL memory allocation often goes through eal_get_virtual_area()
function, which will print a warning whenever the resulting allocation
didn't match the specified address requirements. This is useful for
when we have requested a specific base virtual address, to let the user
know that the mapping has deviated from that address.

However, on Linux, we also have a default base address that's there to
ensure better chances of successful secondary process initialization,
as well as higher likelihood of the virtual areas to fit inside the
IOMMU address width. Because of this default base address, there are
warnings printed even when no base address was explicitly requested,
which can be confusing to the user.

Disable this warning unless base address was explicitly requested.

Cc: Damjan Marion <damarion@cisco.com>

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

Notes:
    I'm not entirely sure the trade off between user confusion and helpful debug
    information is worth it, but in my experience, i've stopped getting any emails
    about secondary processes a long time ago and this isn't a widely used feature,
    so i believe this is worth it.

 lib/librte_eal/common/eal_common_memory.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c
index 33917fa835..6e0d6a0b1c 100644
--- a/lib/librte_eal/common/eal_common_memory.c
+++ b/lib/librte_eal/common/eal_common_memory.c
@@ -139,7 +139,8 @@ eal_get_virtual_area(void *requested_addr, size_t *size,
 		rte_errno = EADDRNOTAVAIL;
 		return NULL;
 	} else if (requested_addr != NULL && addr_is_hint &&
-			aligned_addr != requested_addr) {
+			aligned_addr != requested_addr &&
+			internal_conf->base_virtaddr != 0) {
 		RTE_LOG(WARNING, EAL, "WARNING! Base virtual address hint (%p != %p) not respected!\n",
 			requested_addr, aligned_addr);
 		RTE_LOG(WARNING, EAL, "   This may cause issues with mapping memory into secondary processes\n");
-- 
2.17.1

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

* [dpdk-dev] [PATCH 21.02 v2] mem: don't warn about base addr if not requested
  2020-11-09 15:27 [dpdk-dev] [PATCH 21.02] mem: don't warn about base addr if not requested Anatoly Burakov
@ 2020-11-09 15:47 ` Anatoly Burakov
  2021-01-12 10:29   ` David Marchand
  2021-01-22 17:21   ` Burakov, Anatoly
  0 siblings, 2 replies; 5+ messages in thread
From: Anatoly Burakov @ 2020-11-09 15:47 UTC (permalink / raw)
  To: dev; +Cc: Damjan Marion

Any EAL memory allocation often goes through eal_get_virtual_area()
function, which will print a warning whenever the resulting allocation
didn't match the specified address requirements. This is useful for
when we have requested a specific base virtual address, to let the user
know that the mapping has deviated from that address.

However, on Linux, we also have a default base address that's there to
ensure better chances of successful secondary process initialization,
as well as higher likelihood of the virtual areas to fit inside the
IOMMU address width. Because of this default base address, there are
warnings printed even when no base address was explicitly requested,
which can be confusing to the user.

Emit this warning with debug level unless base address was explicitly
requested by the user.

Cc: Damjan Marion <damarion@cisco.com>

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

Notes:
    v2:
    - Fix the condition to not update the address incorrectly
    - Instead of removing the warning, let it have debug level unless base address
      was explicitly specified by the user
    
    I'm not entirely sure the trade off between user confusion and helpful debug
    information is worth it, but in my experience, i've stopped getting any emails
    about secondary processes a long time ago and this isn't a widely used feature,
    so i believe this is worth it.

 lib/librte_eal/common/eal_common_memory.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c
index 33917fa835..1b50c2099d 100644
--- a/lib/librte_eal/common/eal_common_memory.c
+++ b/lib/librte_eal/common/eal_common_memory.c
@@ -140,9 +140,19 @@ eal_get_virtual_area(void *requested_addr, size_t *size,
 		return NULL;
 	} else if (requested_addr != NULL && addr_is_hint &&
 			aligned_addr != requested_addr) {
-		RTE_LOG(WARNING, EAL, "WARNING! Base virtual address hint (%p != %p) not respected!\n",
-			requested_addr, aligned_addr);
-		RTE_LOG(WARNING, EAL, "   This may cause issues with mapping memory into secondary processes\n");
+		/*
+		 * demote this warning to debug if we did not explicitly request
+		 * a base virtual address.
+		 */
+		if (internal_conf->base_virtaddr != 0) {
+			RTE_LOG(WARNING, EAL, "WARNING! Base virtual address hint (%p != %p) not respected!\n",
+				requested_addr, aligned_addr);
+			RTE_LOG(WARNING, EAL, "   This may cause issues with mapping memory into secondary processes\n");
+		} else {
+			RTE_LOG(DEBUG, EAL, "WARNING! Base virtual address hint (%p != %p) not respected!\n",
+				requested_addr, aligned_addr);
+			RTE_LOG(DEBUG, EAL, "   This may cause issues with mapping memory into secondary processes\n");
+		}
 	} else if (next_baseaddr != NULL) {
 		next_baseaddr = RTE_PTR_ADD(aligned_addr, *size);
 	}
-- 
2.17.1

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

* Re: [dpdk-dev] [PATCH 21.02 v2] mem: don't warn about base addr if not requested
  2020-11-09 15:47 ` [dpdk-dev] [PATCH 21.02 v2] " Anatoly Burakov
@ 2021-01-12 10:29   ` David Marchand
  2021-01-22 17:21   ` Burakov, Anatoly
  1 sibling, 0 replies; 5+ messages in thread
From: David Marchand @ 2021-01-12 10:29 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: dev, Damjan Marion

On Mon, Nov 9, 2020 at 4:48 PM Anatoly Burakov
<anatoly.burakov@intel.com> wrote:
>
> Any EAL memory allocation often goes through eal_get_virtual_area()
> function, which will print a warning whenever the resulting allocation
> didn't match the specified address requirements. This is useful for
> when we have requested a specific base virtual address, to let the user
> know that the mapping has deviated from that address.
>
> However, on Linux, we also have a default base address that's there to
> ensure better chances of successful secondary process initialization,
> as well as higher likelihood of the virtual areas to fit inside the
> IOMMU address width. Because of this default base address, there are
> warnings printed even when no base address was explicitly requested,
> which can be confusing to the user.
>
> Emit this warning with debug level unless base address was explicitly
> requested by the user.
>
> Cc: Damjan Marion <damarion@cisco.com>
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>
> Notes:
>     v2:
>     - Fix the condition to not update the address incorrectly
>     - Instead of removing the warning, let it have debug level unless base address
>       was explicitly specified by the user
>
>     I'm not entirely sure the trade off between user confusion and helpful debug
>     information is worth it, but in my experience, i've stopped getting any emails
>     about secondary processes a long time ago and this isn't a widely used feature,
>     so i believe this is worth it.
>
>  lib/librte_eal/common/eal_common_memory.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c
> index 33917fa835..1b50c2099d 100644
> --- a/lib/librte_eal/common/eal_common_memory.c
> +++ b/lib/librte_eal/common/eal_common_memory.c
> @@ -140,9 +140,19 @@ eal_get_virtual_area(void *requested_addr, size_t *size,
>                 return NULL;
>         } else if (requested_addr != NULL && addr_is_hint &&
>                         aligned_addr != requested_addr) {
> -               RTE_LOG(WARNING, EAL, "WARNING! Base virtual address hint (%p != %p) not respected!\n",
> -                       requested_addr, aligned_addr);
> -               RTE_LOG(WARNING, EAL, "   This may cause issues with mapping memory into secondary processes\n");
> +               /*
> +                * demote this warning to debug if we did not explicitly request
> +                * a base virtual address.
> +                */
> +               if (internal_conf->base_virtaddr != 0) {
> +                       RTE_LOG(WARNING, EAL, "WARNING! Base virtual address hint (%p != %p) not respected!\n",
> +                               requested_addr, aligned_addr);
> +                       RTE_LOG(WARNING, EAL, "   This may cause issues with mapping memory into secondary processes\n");
> +               } else {
> +                       RTE_LOG(DEBUG, EAL, "WARNING! Base virtual address hint (%p != %p) not respected!\n",
> +                               requested_addr, aligned_addr);
> +                       RTE_LOG(DEBUG, EAL, "   This may cause issues with mapping memory into secondary processes\n");
> +               }
>         } else if (next_baseaddr != NULL) {
>                 next_baseaddr = RTE_PTR_ADD(aligned_addr, *size);
>         }
> --
> 2.17.1
>

EAL options like --in-memory or --no-shconf makes MP unusable.
If we add a rte_mp_disable() for them, we could check here for MP
status here and display nothing at all.
WDYT?


-- 
David Marchand


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

* Re: [dpdk-dev] [PATCH 21.02 v2] mem: don't warn about base addr if not requested
  2020-11-09 15:47 ` [dpdk-dev] [PATCH 21.02 v2] " Anatoly Burakov
  2021-01-12 10:29   ` David Marchand
@ 2021-01-22 17:21   ` Burakov, Anatoly
  2022-01-28 11:17     ` Thomas Monjalon
  1 sibling, 1 reply; 5+ messages in thread
From: Burakov, Anatoly @ 2021-01-22 17:21 UTC (permalink / raw)
  To: dev; +Cc: Damjan Marion

On 09-Nov-20 3:47 PM, Anatoly Burakov wrote:
> Any EAL memory allocation often goes through eal_get_virtual_area()
> function, which will print a warning whenever the resulting allocation
> didn't match the specified address requirements. This is useful for
> when we have requested a specific base virtual address, to let the user
> know that the mapping has deviated from that address.
> 
> However, on Linux, we also have a default base address that's there to
> ensure better chances of successful secondary process initialization,
> as well as higher likelihood of the virtual areas to fit inside the
> IOMMU address width. Because of this default base address, there are
> warnings printed even when no base address was explicitly requested,
> which can be confusing to the user.
> 
> Emit this warning with debug level unless base address was explicitly
> requested by the user.
> 
> Cc: Damjan Marion <damarion@cisco.com>
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> 
> Notes:
>      v2:
>      - Fix the condition to not update the address incorrectly
>      - Instead of removing the warning, let it have debug level unless base address
>        was explicitly specified by the user
>      
>      I'm not entirely sure the trade off between user confusion and helpful debug
>      information is worth it, but in my experience, i've stopped getting any emails
>      about secondary processes a long time ago and this isn't a widely used feature,
>      so i believe this is worth it.

For some reason i didn't get David's comment in my inbox, so i'll copy 
it here:

 > EAL options like --in-memory or --no-shconf makes MP unusable.
 > If we add a rte_mp_disable() for them, we could check here for MP
 > status here and display nothing at all.
 > WDYT?

That sounds like a nice idea, but this patch addresses a different issue.

-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH 21.02 v2] mem: don't warn about base addr if not requested
  2021-01-22 17:21   ` Burakov, Anatoly
@ 2022-01-28 11:17     ` Thomas Monjalon
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Monjalon @ 2022-01-28 11:17 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev, Damjan Marion, dmitry.kozliuk

22/01/2021 18:21, Burakov, Anatoly:
> On 09-Nov-20 3:47 PM, Anatoly Burakov wrote:
> > Any EAL memory allocation often goes through eal_get_virtual_area()
> > function, which will print a warning whenever the resulting allocation
> > didn't match the specified address requirements. This is useful for
> > when we have requested a specific base virtual address, to let the user
> > know that the mapping has deviated from that address.
> > 
> > However, on Linux, we also have a default base address that's there to
> > ensure better chances of successful secondary process initialization,
> > as well as higher likelihood of the virtual areas to fit inside the
> > IOMMU address width. Because of this default base address, there are
> > warnings printed even when no base address was explicitly requested,
> > which can be confusing to the user.
> > 
> > Emit this warning with debug level unless base address was explicitly
> > requested by the user.
> > 
> > Cc: Damjan Marion <damarion@cisco.com>
> > 
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > ---
> > 
> > Notes:
> >      v2:
> >      - Fix the condition to not update the address incorrectly
> >      - Instead of removing the warning, let it have debug level unless base address
> >        was explicitly specified by the user
> >      
> >      I'm not entirely sure the trade off between user confusion and helpful debug
> >      information is worth it, but in my experience, i've stopped getting any emails
> >      about secondary processes a long time ago and this isn't a widely used feature,
> >      so i believe this is worth it.
> 
> For some reason i didn't get David's comment in my inbox, so i'll copy 
> it here:
> 
>  > EAL options like --in-memory or --no-shconf makes MP unusable.
>  > If we add a rte_mp_disable() for them, we could check here for MP
>  > status here and display nothing at all.
>  > WDYT?
> 
> That sounds like a nice idea, but this patch addresses a different issue.

I think it is the same issue, pushed further.
Anyway, let's take this patch (waiting for one year)
and wait for another one removing the log completely
in case secondary process is disabled.

Applied




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

end of thread, other threads:[~2022-01-28 11:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-09 15:27 [dpdk-dev] [PATCH 21.02] mem: don't warn about base addr if not requested Anatoly Burakov
2020-11-09 15:47 ` [dpdk-dev] [PATCH 21.02 v2] " Anatoly Burakov
2021-01-12 10:29   ` David Marchand
2021-01-22 17:21   ` Burakov, Anatoly
2022-01-28 11: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).