All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Marchand <david.marchand@redhat.com>
To: Anatoly Burakov <anatoly.burakov@intel.com>
Cc: dev <dev@dpdk.org>, Damjan Marion <damarion@cisco.com>
Subject: Re: [dpdk-dev] [PATCH 21.02 v2] mem: don't warn about base addr if not requested
Date: Tue, 12 Jan 2021 11:29:59 +0100	[thread overview]
Message-ID: <CAJFAV8ySCZH6F1nbLC3k+QA2kyFMnB3LnekUvH4QLpbb580RBQ@mail.gmail.com> (raw)
In-Reply-To: <8f49e252a7be2d8561f4b32193e5800f98c40b0e.1604936860.git.anatoly.burakov@intel.com>

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


  reply	other threads:[~2021-01-12 10:30 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2021-01-22 17:21   ` Burakov, Anatoly
2022-01-28 11:17     ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJFAV8ySCZH6F1nbLC3k+QA2kyFMnB3LnekUvH4QLpbb580RBQ@mail.gmail.com \
    --to=david.marchand@redhat.com \
    --cc=anatoly.burakov@intel.com \
    --cc=damarion@cisco.com \
    --cc=dev@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.