| Message ID | 20211214151850.1183-1-gaoxiangliu0@163.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | David Marchand |
| Headers | show |
| Series | [v3] eal: allow to exclude memseg from core dump | expand |
| Context | Check | Description |
|---|---|---|
| ci/Intel-compilation | success | Compilation OK |
| ci/iol-aarch64-unit-testing | success | Testing PASS |
| ci/iol-intel-Performance | success | Performance Testing PASS |
| ci/iol-intel-Functional | success | Functional Testing PASS |
| ci/iol-broadcom-Functional | success | Functional Testing PASS |
| ci/iol-x86_64-compile-testing | success | Testing PASS |
| ci/github-robot: build | success | github build: passed |
| ci/iol-x86_64-unit-testing | success | Testing PASS |
| ci/iol-aarch64-compile-testing | success | Testing PASS |
| ci/iol-mellanox-Performance | success | Performance Testing PASS |
| ci/iol-broadcom-Performance | success | Performance Testing PASS |
| ci/intel-Testing | success | Testing PASS |
| ci/checkpatch | warning | coding style issues |
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.
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. + 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)); rte_errno = errno; - } + } else + RTE_LOG(INFO, EAL, "madvise(%p, %#zx, %d) success: %s\n", + virt, size, flags, __func__); + return ret; }
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(-)