Re: [dpdk-dev] [dpdk-stable] [PATCH v5 2/2] eal: use base address hint to reserve space for mem config

[David Marchand <david.marchand@redhat.com>]

On Thu, Oct 24, 2019 at 2:37 PM Anatoly Burakov
<anatoly.burakov@intel.com> wrote:
>
> Currently, mem config will be mapped without using the virtual
> area reservation infrastructure, which means it will be mapped
> at an arbitrary location. This may cause failures to map the
> shared config in secondary process due to things like PCI
> whitelist arguments allocating memory in a space where the
> primary has allocated the shared mem config.
>
> Fix this by using virtual area reservation to reserve space for
> the mem config, thereby avoiding the problem and reserving the
> shared config (hopefully) far away from any normal memory
> allocations.
>
> Cc: stable@dpdk.org
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>
> Notes:
>     v4:
>     - Fix mem config length to always be page-aligned
>
>     v3:
>     - Fix alignment issues with base address
>
>     v2:
>     - Fix issue with unneeded ADDR_IS_HINT flag that broke things
>       on 32-bit builds
>
>  lib/librte_eal/freebsd/eal/eal.c | 28 +++++++++++++++++++++------
>  lib/librte_eal/linux/eal/eal.c   | 33 +++++++++++++++++++++++---------
>  2 files changed, 46 insertions(+), 15 deletions(-)
>
> diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
> index f86e9aa318..5b869b895a 100644
> --- a/lib/librte_eal/freebsd/eal/eal.c
> +++ b/lib/librte_eal/freebsd/eal/eal.c
> @@ -218,7 +218,10 @@ eal_parse_sysfs_value(const char *filename, unsigned long *val)
>  static int
>  rte_eal_config_create(void)
>  {
> -       void *rte_mem_cfg_addr;
> +       size_t page_sz = sysconf(_SC_PAGE_SIZE);
> +       size_t cfg_len = sizeof(*rte_config.mem_config);
> +       size_t cfg_len_aligned = RTE_ALIGN(cfg_len, page_sz);
> +       void *rte_mem_cfg_addr, *mapped_mem_cfg_addr;
>         int retval;
>
>         const char *pathname = eal_runtime_config_path();

Updated existing call to sysconf with page_sz variable.

       if (internal_config.base_virtaddr != 0)
               rte_mem_cfg_addr = (void *)
                       RTE_ALIGN_FLOOR(internal_config.base_virtaddr -
-                      sizeof(struct rte_mem_config), sysconf(_SC_PAGE_SIZE));
+                      sizeof(struct rte_mem_config), page_sz);
       else
               rte_mem_cfg_addr = NULL;


> @@ -235,7 +238,7 @@ rte_eal_config_create(void)
>                 }
>         }
>
> -       retval = ftruncate(mem_cfg_fd, sizeof(*rte_config.mem_config));
> +       retval = ftruncate(mem_cfg_fd, cfg_len);
>         if (retval < 0){
>                 close(mem_cfg_fd);
>                 mem_cfg_fd = -1;
> @@ -253,15 +256,28 @@ rte_eal_config_create(void)
>                 return -1;
>         }
>
> -       rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
> -                               PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0);
> -
> -       if (rte_mem_cfg_addr == MAP_FAILED){
> +       /* reserve space for config */
> +       rte_mem_cfg_addr = eal_get_virtual_area(NULL, &cfg_len_aligned, page_sz,
> +                       0, 0);

Fixed conflict with base-virtaddr patches.

> +       if (rte_mem_cfg_addr == NULL) {
>                 RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config\n");
>                 close(mem_cfg_fd);
>                 mem_cfg_fd = -1;
>                 return -1;
>         }

Applied, thanks.


--
David Marchand