Hi Jean,

There are some format issues with the patch:

You can run scripts/check-git-log.sh to check them:
Wrong headline format:
         eal: Fix misleading error messages, errno can't be trusted.
Wrong headline uppercase:
         eal: Fix misleading error messages, errno can't be trusted.
Missing 'Fixes' tag:
         eal: Fix misleading error messages, errno can't be trusted.

The script's output highlights the different issues.


On 21/09/2016 22:10, Jean Tourrilhes wrote:
>   lib/librte_eal/linuxapp/eal/eal.c        | 14 +++++++++++---
>   lib/librte_eal/linuxapp/eal/eal_memory.c | 16 ++++++++++++----
>   2 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c 
> b/lib/librte_eal/linuxapp/eal/eal.c
> index 3fb2188..5df9f6a 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -238,7 +238,8 @@ rte_eal_config_attach(void)
>       mem_config = (struct rte_mem_config *) mmap(NULL, sizeof(*mem_config),
>                       PROT_READ, MAP_SHARED, mem_cfg_fd, 0);
>       if (mem_config == MAP_FAILED)
> -             rte_panic("Cannot mmap memory for rte_config\n");
> +             rte_panic("Cannot mmap memory for rte_config! error %i (%s)\n",
> +                       errno, strerror(errno));
>   
>       rte_config.mem_config = mem_config;
>   }
> @@ -263,9 +264,16 @@ rte_eal_config_reattach(void)
>       mem_config = (struct rte_mem_config *) mmap(rte_mem_cfg_addr,
>                       sizeof(*mem_config), PROT_READ | PROT_WRITE, MAP_SHARED,
>                       mem_cfg_fd, 0);
> +     if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr) {
> +             if (mem_config != MAP_FAILED)
> +                     /* errno is stale, don't use */
> +                     rte_panic("Cannot mmap memory for rte_config at [%p], 
> got [%p] - please use '--base-virtaddr' option\n",
> +                               rte_mem_cfg_addr, mem_config);
> +             else
> +                     rte_panic("Cannot mmap memory for rte_config! error %i 
> (%s)\n",
> +                               errno, strerror(errno));
> +     }
>       close(mem_cfg_fd);
> -     if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr)
> -             rte_panic("Cannot mmap memory for rte_config\n");
>   

NIT but any reason you moved the check before closing the file 
descriptor? (not that it matters with current code as we panic anyway)

Thanks,
Sergio

>       rte_config.mem_config = mem_config;
>   }
> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c 
> b/lib/librte_eal/linuxapp/eal/eal_memory.c
> index 41e0a92..b036ffc 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
> @@ -1615,10 +1615,18 @@ rte_eal_hugepage_attach(void)
>                                PROT_READ, MAP_PRIVATE, fd_zero, 0);
>               if (base_addr == MAP_FAILED ||
>                   base_addr != mcfg->memseg[s].addr) {
> -                     RTE_LOG(ERR, EAL, "Could not mmap %llu bytes "
> -                             "in /dev/zero to requested address [%p]: 
> '%s'\n",
> -                             (unsigned long long)mcfg->memseg[s].len,
> -                             mcfg->memseg[s].addr, strerror(errno));
> +                     if (base_addr != MAP_FAILED)
> +                             /* errno is stale, don't use */
> +                             RTE_LOG(ERR, EAL, "Could not mmap %llu bytes "
> +                                     "in /dev/zero at [%p], got [%p] - "
> +                                     "please use '--base-virtaddr' option\n",
> +                                     (unsigned long long)mcfg->memseg[s].len,
> +                                     mcfg->memseg[s].addr, base_addr);
> +                     else
> +                             RTE_LOG(ERR, EAL, "Could not mmap %llu bytes "
> +                                     "in /dev/zero at [%p]: '%s'\n",
> +                                     (unsigned long long)mcfg->memseg[s].len,
> +                                     mcfg->memseg[s].addr, strerror(errno));
>                       if (aslr_enabled() > 0) {
>                               RTE_LOG(ERR, EAL, "It is recommended to "
>                                       "disable ASLR in the kernel "


Reply via email to