[v3] eal: allow to exclude memseg from core dump

  Some DPDK application is allocated storage partition of 8G(or smaller)
If coredump happens, the application doesn't work because of
insufficient storage space.
The patch provides a config that means whether the memseg memory
is allowed to exclude from core dump.
The DPDK application can choose to open it according to the actual
situation.

Fixes: d72e4042c5eb ("mem: exclude unused memory from core dump")
Cc: stable@dpdk.org

Signed-off-by: Gaoxiang Liu <liugaoxiang@huawei.com>

---
v2:
* Fixed compile issues.

v3:
* Fixed review issues. madvise is replaced by eal_mem_set_dump(),
* and The type of huge_dont_dump_flag has been changed to bool.
---
 doc/guides/linux_gsg/linux_eal_parameters.rst | 4 ++++
 lib/eal/common/eal_internal_cfg.h             | 1 +
 lib/eal/common/eal_options.h                  | 3 ++-
 lib/eal/linux/eal.c                           | 4 ++++
 lib/eal/linux/eal_memalloc.c                  | 3 +++
 lib/eal/unix/eal_unix_memory.c                | 7 +++++--
 6 files changed, 19 insertions(+), 3 deletions(-)
  

On Tue, Dec 14, 2021 at 8:49 PM Gaoxiang Liu <gaoxiangliu0@163.com> wrote:
>
> Some DPDK application is allocated storage partition of 8G(or smaller)
> If coredump happens, the application doesn't work because of
> insufficient storage space.
> The patch provides a config that means whether the memseg memory
> is allowed to exclude from core dump.
> The DPDK application can choose to open it according to the actual
> situation.
>
> Fixes: d72e4042c5eb ("mem: exclude unused memory from core dump")

This is patch is a feature, not a fix. So you can remove this

> Cc: stable@dpdk.org

This is patch is a feature, not a fix. So you can remove this


>
> Signed-off-by: Gaoxiang Liu <liugaoxiang@huawei.com>
>
> ---
> v2:
> * Fixed compile issues.
>
> v3:
> * Fixed review issues. madvise is replaced by eal_mem_set_dump(),
> * and The type of huge_dont_dump_flag has been changed to bool.
> ---
>  doc/guides/linux_gsg/linux_eal_parameters.rst | 4 ++++
>  lib/eal/common/eal_internal_cfg.h             | 1 +
>  lib/eal/common/eal_options.h                  | 3 ++-
>  lib/eal/linux/eal.c                           | 4 ++++
>  lib/eal/linux/eal_memalloc.c                  | 3 +++
>  lib/eal/unix/eal_unix_memory.c                | 7 +++++--
>  6 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/doc/guides/linux_gsg/linux_eal_parameters.rst b/doc/guides/linux_gsg/linux_eal_parameters.rst

Since you are adding it in unix/eal_unix_memory.c it is applicable for
freebsd too.
Please update options and documentation for freebsd.

> index 74df2611b5..b6805bc6df 100644
> --- a/doc/guides/linux_gsg/linux_eal_parameters.rst
> +++ b/doc/guides/linux_gsg/linux_eal_parameters.rst
> @@ -93,6 +93,10 @@ Memory-related options
>
>      Free hugepages back to system exactly as they were originally allocated.
>
> +*   ``--memseg-dont-dump``
> +
> +    Allow to exclude memseg from core dump.
> +
>  Other options
>  ~~~~~~~~~~~~~
>
> diff --git a/lib/eal/common/eal_internal_cfg.h b/lib/eal/common/eal_internal_cfg.h
> index d6c0470eb8..a7c34b88db 100644
> --- a/lib/eal/common/eal_internal_cfg.h
> +++ b/lib/eal/common/eal_internal_cfg.h
> @@ -87,6 +87,7 @@ struct internal_config {
>                         /**< user defined mbuf pool ops name */
>         unsigned num_hugepage_sizes;      /**< how many sizes on this system */
>         struct hugepage_info hugepage_info[MAX_HUGEPAGE_SIZES];
> +       bool memseg_dont_dump_flag;
>         enum rte_iova_mode iova_mode ;    /**< Set IOVA mode on this system  */
>         rte_cpuset_t ctrl_cpuset;         /**< cpuset for ctrl threads */
>         volatile unsigned int init_complete;
> diff --git a/lib/eal/common/eal_options.h b/lib/eal/common/eal_options.h
> index 8e4f7202a2..013aad4cfc 100644
> --- a/lib/eal/common/eal_options.h
> +++ b/lib/eal/common/eal_options.h
> @@ -87,7 +87,8 @@ enum {
>         OPT_NO_TELEMETRY_NUM,
>  #define OPT_FORCE_MAX_SIMD_BITWIDTH  "force-max-simd-bitwidth"
>         OPT_FORCE_MAX_SIMD_BITWIDTH_NUM,
> -
> +#define OPT_MEMSEG_DONT_DUMP     "memseg-dont-dump"
> +       OPT_MEMSEG_DONT_DUMP_NUM,
>         OPT_LONG_MAX_NUM
>  };
>
> diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
> index 60b4924838..8a47bf758a 100644
> --- a/lib/eal/linux/eal.c
> +++ b/lib/eal/linux/eal.c
> @@ -817,6 +817,10 @@ eal_parse_args(int argc, char **argv)
>                         internal_conf->match_allocations = 1;
>                         break;
>
> +               case OPT_MEMSEG_DONT_DUMP_NUM:
> +                       internal_conf->memseg_dont_dump_flag = 1;
> +                       break;
> +
>                 default:
>                         if (opt < OPT_LONG_MIN_NUM && isprint(opt)) {
>                                 RTE_LOG(ERR, EAL, "Option %c is not supported "
> diff --git a/lib/eal/linux/eal_memalloc.c b/lib/eal/linux/eal_memalloc.c
> index 337f2bc739..8e41537355 100644
> --- a/lib/eal/linux/eal_memalloc.c
> +++ b/lib/eal/linux/eal_memalloc.c
> @@ -663,6 +663,9 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
>         ms->iova = iova;
>         ms->socket_id = socket_id;
>
> +       if (internal_conf->memseg_dont_dump_flag)
> +               eal_mem_set_dump(addr, alloc_sz, false);
> +
>         return 0;
>
>  mapped:
> diff --git a/lib/eal/unix/eal_unix_memory.c b/lib/eal/unix/eal_unix_memory.c
> index 68ae93bd6e..44227aee95 100644
> --- a/lib/eal/unix/eal_unix_memory.c
> +++ b/lib/eal/unix/eal_unix_memory.c
> @@ -83,10 +83,13 @@ eal_mem_set_dump(void *virt, size_t size, bool dump)
>         int flags = dump ? EAL_DODUMP : EAL_DONTDUMP;
>         int ret = madvise(virt, size, flags);
>         if (ret) {
> -               RTE_LOG(DEBUG, EAL, "madvise(%p, %#zx, %d) failed: %s\n",
> +               RTE_LOG(INFO, EAL, "madvise(%p, %#zx, %d) failed: %s\n",
>                                 virt, size, flags, strerror(rte_errno));

probably ERR instead of INFO.


>                 rte_errno = errno;
> -       }
> +       } else
> +               RTE_LOG(INFO, EAL, "madvise(%p, %#zx, %d) success: %s\n",
> +                               virt, size, flags, __func__);

This should be DEBUG. Not INFO.

With above changes
Reviewed-by: Jerin Jacob <jerinj@marvell.com>


> +
>         return ret;
>  }
>
> --
> 2.32.0
>
>
  

2021-12-15 17:36 (UTC+0530), Jerin Jacob:
[...]
> > diff --git a/lib/eal/unix/eal_unix_memory.c b/lib/eal/unix/eal_unix_memory.c
> > index 68ae93bd6e..44227aee95 100644
> > --- a/lib/eal/unix/eal_unix_memory.c
> > +++ b/lib/eal/unix/eal_unix_memory.c
> > @@ -83,10 +83,13 @@ eal_mem_set_dump(void *virt, size_t size, bool dump)
> >         int flags = dump ? EAL_DODUMP : EAL_DONTDUMP;
> >         int ret = madvise(virt, size, flags);
> >         if (ret) {
> > -               RTE_LOG(DEBUG, EAL, "madvise(%p, %#zx, %d) failed: %s\n",
> > +               RTE_LOG(INFO, EAL, "madvise(%p, %#zx, %d) failed: %s\n",
> >                                 virt, size, flags, strerror(rte_errno));  
> 
> probably ERR instead of INFO.
> 
> 
> >                 rte_errno = errno;
> > -       }
> > +       } else
> > +               RTE_LOG(INFO, EAL, "madvise(%p, %#zx, %d) success: %s\n",
> > +                               virt, size, flags, __func__);  
> 
> This should be DEBUG. Not INFO.

Usually errors from OS API are reported as DEBUG in EAL,
so the first one should also remain DEBUG.
  

2021-12-14 23:18 (UTC+0800), Gaoxiang Liu:
> Some DPDK application is allocated storage partition of 8G(or smaller)
> If coredump happens, the application doesn't work because of
> insufficient storage space.
> The patch provides a config that means whether the memseg memory
> is allowed to exclude from core dump.
> The DPDK application can choose to open it according to the actual
> situation.

Does it need to be a DPDK option?
If eal_mem_set_dump() is exposed as rte_mem_set_dump(),
this feature can be implemented by the application using
rte_mem_event_callback_register().
On Linux only and for all hugepages (not only DPDK ones),
administrator can reset bits 5 and 6 of /proc/[pid]/core_filter
to solve the task without any programming at all.
https://man7.org/linux/man-pages/man5/core.5.html

[...]
> diff --git a/doc/guides/linux_gsg/linux_eal_parameters.rst b/doc/guides/linux_gsg/linux_eal_parameters.rst
> index 74df2611b5..b6805bc6df 100644
> --- a/doc/guides/linux_gsg/linux_eal_parameters.rst
> +++ b/doc/guides/linux_gsg/linux_eal_parameters.rst
> @@ -93,6 +93,10 @@ Memory-related options
>  
>      Free hugepages back to system exactly as they were originally allocated.
>  
> +*   ``--memseg-dont-dump``
> +
> +    Allow to exclude memseg from core dump.
> +

"Memory segment" is something that is understood by programmers, not users.
Suggesting --no-huge-dump, in line with --no-huge and --huge-unlink.