* [dpdk-dev] [PATCH] mem: fix cleanup on Windows @ 2021-03-24 18:01 Dmitry Kozlyuk 2021-03-24 18:35 ` David Marchand 2021-03-24 19:32 ` [dpdk-dev] [PATCH v2] mem: fix cleanup when multi-process is disabled Dmitry Kozlyuk 0 siblings, 2 replies; 7+ messages in thread From: Dmitry Kozlyuk @ 2021-03-24 18:01 UTC (permalink / raw) To: dev; +Cc: Dmitry Kozlyuk, Anatoly Burakov, Jie Zhou, David Marchand rte_mem_unmap() called from rte_eal_memory_detach() fails on Windows with message "EAL: Could not unmap memory: No error". This is because on Windows memory is not allocated using mapping. Confusing "No error" is caused by using errno instead of rte_errno set by rte_mem_unmap(). Multi-process is not supported on Windows and --in-memory is forced, so detaching memory is not needed on cleanup. Bypass the function in this case. Fix error handling to produce proper log message. Fixes: dfbc61a2f9a6 ("mem: detach memsegs on cleanup") Cc: Anatoly Burakov <anatoly.burakov@intel.com> Reported-by: Jie Zhou <jizh@microsoft.com> Suggested-by: David Marchand <david.marchand@redhat.com> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> --- lib/librte_eal/common/eal_common_memory.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c index 0e99986d3d..9ef9c65ac8 100644 --- a/lib/librte_eal/common/eal_common_memory.c +++ b/lib/librte_eal/common/eal_common_memory.c @@ -1010,6 +1010,13 @@ rte_eal_memory_detach(void) size_t page_sz = rte_mem_page_size(); unsigned int i; +#ifdef RTE_EXEC_ENV_WINDOWS + /* Multi-process is not supported, detaching is not needed. + * mcfg->mp_status can't be used: it's always "unknown" on Windows. + */ + return 0; +#endif + rte_rwlock_write_lock(&mcfg->memory_hotplug_lock); /* detach internal memory subsystem data first */ @@ -1032,7 +1039,7 @@ rte_eal_memory_detach(void) if (!msl->external) if (rte_mem_unmap(msl->base_va, msl->len) != 0) RTE_LOG(ERR, EAL, "Could not unmap memory: %s\n", - strerror(errno)); + rte_strerror(rte_errno)); /* * we are detaching the fbarray rather than destroying because -- 2.29.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH] mem: fix cleanup on Windows 2021-03-24 18:01 [dpdk-dev] [PATCH] mem: fix cleanup on Windows Dmitry Kozlyuk @ 2021-03-24 18:35 ` David Marchand 2021-03-24 19:32 ` [dpdk-dev] [PATCH v2] mem: fix cleanup when multi-process is disabled Dmitry Kozlyuk 1 sibling, 0 replies; 7+ messages in thread From: David Marchand @ 2021-03-24 18:35 UTC (permalink / raw) To: Dmitry Kozlyuk, Anatoly Burakov; +Cc: dev, Jie Zhou On Wed, Mar 24, 2021 at 7:01 PM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote: > > rte_mem_unmap() called from rte_eal_memory_detach() fails on Windows > with message "EAL: Could not unmap memory: No error". This is because > on Windows memory is not allocated using mapping. Confusing "No error" > is caused by using errno instead of rte_errno set by rte_mem_unmap(). > > Multi-process is not supported on Windows and --in-memory is forced, > so detaching memory is not needed on cleanup. Bypass the function > in this case. Fix error handling to produce proper log message. > > Fixes: dfbc61a2f9a6 ("mem: detach memsegs on cleanup") > Cc: Anatoly Burakov <anatoly.burakov@intel.com> > > Reported-by: Jie Zhou <jizh@microsoft.com> > Suggested-by: David Marchand <david.marchand@redhat.com> > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> > --- > lib/librte_eal/common/eal_common_memory.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c > index 0e99986d3d..9ef9c65ac8 100644 > --- a/lib/librte_eal/common/eal_common_memory.c > +++ b/lib/librte_eal/common/eal_common_memory.c > @@ -1010,6 +1010,13 @@ rte_eal_memory_detach(void) > size_t page_sz = rte_mem_page_size(); > unsigned int i; > > +#ifdef RTE_EXEC_ENV_WINDOWS > + /* Multi-process is not supported, detaching is not needed. > + * mcfg->mp_status can't be used: it's always "unknown" on Windows. > + */ > + return 0; > +#endif > + My suggestion was more like: if (eal_get_internal_configuration()->in_memory == 1) return 0; -- David Marchand ^ permalink raw reply [flat|nested] 7+ messages in thread
* [dpdk-dev] [PATCH v2] mem: fix cleanup when multi-process is disabled 2021-03-24 18:01 [dpdk-dev] [PATCH] mem: fix cleanup on Windows Dmitry Kozlyuk 2021-03-24 18:35 ` David Marchand @ 2021-03-24 19:32 ` Dmitry Kozlyuk 2021-03-25 15:39 ` Ranjit Menon ` (3 more replies) 1 sibling, 4 replies; 7+ messages in thread From: Dmitry Kozlyuk @ 2021-03-24 19:32 UTC (permalink / raw) To: dev; +Cc: Dmitry Kozlyuk, Anatoly Burakov, Jie Zhou, David Marchand rte_eal_memory_detach() did not account for cases where multi-process mode is disabled: --in-memory and --no-shconf. This resulted in unmapping memory that had not been mapped, which caused errors: EAL: Could not unmap memory: No error (Windows) EAL: Cannot munmap(0x1d47f40, 0x7000): Invalid argument (Linux) Confusing "No error" was caused by using errno instead of rte_errno set by rte_mem_unmap(). Skip detaching memory altogether when --in-memory is specified. Skip unmapping configuration when it's not shared. Fix and add error handling to produce proper log messages. Fixes: dfbc61a2f9a6 ("mem: detach memsegs on cleanup") Cc: Anatoly Burakov <anatoly.burakov@intel.com> Reported-by: Jie Zhou <jizh@microsoft.com> Suggested-by: David Marchand <david.marchand@redhat.com> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> --- lib/librte_eal/common/eal_common_memory.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c index 0e99986d3d..9495170c86 100644 --- a/lib/librte_eal/common/eal_common_memory.c +++ b/lib/librte_eal/common/eal_common_memory.c @@ -1006,10 +1006,15 @@ rte_extmem_detach(void *va_addr, size_t len) int rte_eal_memory_detach(void) { + const struct internal_config *internal_conf = + eal_get_internal_configuration(); struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config; size_t page_sz = rte_mem_page_size(); unsigned int i; + if (internal_conf->in_memory == 1) + return 0; + rte_rwlock_write_lock(&mcfg->memory_hotplug_lock); /* detach internal memory subsystem data first */ @@ -1032,7 +1037,7 @@ rte_eal_memory_detach(void) if (!msl->external) if (rte_mem_unmap(msl->base_va, msl->len) != 0) RTE_LOG(ERR, EAL, "Could not unmap memory: %s\n", - strerror(errno)); + rte_strerror(rte_errno)); /* * we are detaching the fbarray rather than destroying because @@ -1050,7 +1055,10 @@ rte_eal_memory_detach(void) * config - we can't zero it out because it might still be referenced * by other processes. */ - rte_mem_unmap(mcfg, RTE_ALIGN(sizeof(*mcfg), page_sz)); + if (internal_conf->no_shconf == 0) + if (rte_mem_unmap(mcfg, RTE_ALIGN(sizeof(*mcfg), page_sz)) != 0) + RTE_LOG(ERR, EAL, "Could not unmap shared memory config: %s\n", + rte_strerror(rte_errno)); rte_eal_get_configuration()->mem_config = NULL; return 0; -- 2.29.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v2] mem: fix cleanup when multi-process is disabled 2021-03-24 19:32 ` [dpdk-dev] [PATCH v2] mem: fix cleanup when multi-process is disabled Dmitry Kozlyuk @ 2021-03-25 15:39 ` Ranjit Menon 2021-03-26 12:34 ` Burakov, Anatoly ` (2 subsequent siblings) 3 siblings, 0 replies; 7+ messages in thread From: Ranjit Menon @ 2021-03-25 15:39 UTC (permalink / raw) To: Dmitry Kozlyuk, dev; +Cc: Anatoly Burakov, Jie Zhou, David Marchand On 3/24/2021 12:32 PM, Dmitry Kozlyuk wrote: > rte_eal_memory_detach() did not account for cases where multi-process > mode is disabled: --in-memory and --no-shconf. This resulted > in unmapping memory that had not been mapped, which caused errors: > > EAL: Could not unmap memory: No error (Windows) > EAL: Cannot munmap(0x1d47f40, 0x7000): Invalid argument (Linux) > > Confusing "No error" was caused by using errno instead of rte_errno > set by rte_mem_unmap(). > > Skip detaching memory altogether when --in-memory is specified. > Skip unmapping configuration when it's not shared. > Fix and add error handling to produce proper log messages. > > Fixes: dfbc61a2f9a6 ("mem: detach memsegs on cleanup") > Cc: Anatoly Burakov <anatoly.burakov@intel.com> > > Reported-by: Jie Zhou <jizh@microsoft.com> > Suggested-by: David Marchand <david.marchand@redhat.com> > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> > --- > lib/librte_eal/common/eal_common_memory.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c > index 0e99986d3d..9495170c86 100644 > --- a/lib/librte_eal/common/eal_common_memory.c > +++ b/lib/librte_eal/common/eal_common_memory.c > @@ -1006,10 +1006,15 @@ rte_extmem_detach(void *va_addr, size_t len) > int > rte_eal_memory_detach(void) > { > + const struct internal_config *internal_conf = > + eal_get_internal_configuration(); > struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config; > size_t page_sz = rte_mem_page_size(); > unsigned int i; > > + if (internal_conf->in_memory == 1) > + return 0; > + > rte_rwlock_write_lock(&mcfg->memory_hotplug_lock); > > /* detach internal memory subsystem data first */ > @@ -1032,7 +1037,7 @@ rte_eal_memory_detach(void) > if (!msl->external) > if (rte_mem_unmap(msl->base_va, msl->len) != 0) > RTE_LOG(ERR, EAL, "Could not unmap memory: %s\n", > - strerror(errno)); > + rte_strerror(rte_errno)); > > /* > * we are detaching the fbarray rather than destroying because > @@ -1050,7 +1055,10 @@ rte_eal_memory_detach(void) > * config - we can't zero it out because it might still be referenced > * by other processes. > */ > - rte_mem_unmap(mcfg, RTE_ALIGN(sizeof(*mcfg), page_sz)); > + if (internal_conf->no_shconf == 0) > + if (rte_mem_unmap(mcfg, RTE_ALIGN(sizeof(*mcfg), page_sz)) != 0) > + RTE_LOG(ERR, EAL, "Could not unmap shared memory config: %s\n", > + rte_strerror(rte_errno)); > rte_eal_get_configuration()->mem_config = NULL; > > return 0; Acked-by: Ranjit Menon <ranjit.menon@intel.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v2] mem: fix cleanup when multi-process is disabled 2021-03-24 19:32 ` [dpdk-dev] [PATCH v2] mem: fix cleanup when multi-process is disabled Dmitry Kozlyuk 2021-03-25 15:39 ` Ranjit Menon @ 2021-03-26 12:34 ` Burakov, Anatoly 2021-03-26 16:15 ` David Marchand 2021-04-09 12:00 ` David Marchand 3 siblings, 0 replies; 7+ messages in thread From: Burakov, Anatoly @ 2021-03-26 12:34 UTC (permalink / raw) To: Dmitry Kozlyuk, dev; +Cc: Jie Zhou, David Marchand On 24-Mar-21 7:32 PM, Dmitry Kozlyuk wrote: > rte_eal_memory_detach() did not account for cases where multi-process > mode is disabled: --in-memory and --no-shconf. This resulted > in unmapping memory that had not been mapped, which caused errors: > > EAL: Could not unmap memory: No error (Windows) > EAL: Cannot munmap(0x1d47f40, 0x7000): Invalid argument (Linux) > > Confusing "No error" was caused by using errno instead of rte_errno > set by rte_mem_unmap(). > > Skip detaching memory altogether when --in-memory is specified. > Skip unmapping configuration when it's not shared. > Fix and add error handling to produce proper log messages. > > Fixes: dfbc61a2f9a6 ("mem: detach memsegs on cleanup") > Cc: Anatoly Burakov <anatoly.burakov@intel.com> > > Reported-by: Jie Zhou <jizh@microsoft.com> > Suggested-by: David Marchand <david.marchand@redhat.com> > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> > --- LGTM Acked-by: Anatoly Burakov <anatoly.burakov@intel.com> -- Thanks, Anatoly ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v2] mem: fix cleanup when multi-process is disabled 2021-03-24 19:32 ` [dpdk-dev] [PATCH v2] mem: fix cleanup when multi-process is disabled Dmitry Kozlyuk 2021-03-25 15:39 ` Ranjit Menon 2021-03-26 12:34 ` Burakov, Anatoly @ 2021-03-26 16:15 ` David Marchand 2021-04-09 12:00 ` David Marchand 3 siblings, 0 replies; 7+ messages in thread From: David Marchand @ 2021-03-26 16:15 UTC (permalink / raw) To: Dmitry Kozlyuk; +Cc: dev, Anatoly Burakov, Jie Zhou On Wed, Mar 24, 2021 at 8:32 PM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote: > > rte_eal_memory_detach() did not account for cases where multi-process > mode is disabled: --in-memory and --no-shconf. This resulted > in unmapping memory that had not been mapped, which caused errors: > > EAL: Could not unmap memory: No error (Windows) > EAL: Cannot munmap(0x1d47f40, 0x7000): Invalid argument (Linux) > > Confusing "No error" was caused by using errno instead of rte_errno > set by rte_mem_unmap(). > > Skip detaching memory altogether when --in-memory is specified. > Skip unmapping configuration when it's not shared. > Fix and add error handling to produce proper log messages. > > Fixes: dfbc61a2f9a6 ("mem: detach memsegs on cleanup") > Cc: Anatoly Burakov <anatoly.burakov@intel.com> > > Reported-by: Jie Zhou <jizh@microsoft.com> > Suggested-by: David Marchand <david.marchand@redhat.com> > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> Acked-by: Ranjit Menon <ranjit.menon@intel.com> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com> Applied, thanks Dmitry. -- David Marchand ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v2] mem: fix cleanup when multi-process is disabled 2021-03-24 19:32 ` [dpdk-dev] [PATCH v2] mem: fix cleanup when multi-process is disabled Dmitry Kozlyuk ` (2 preceding siblings ...) 2021-03-26 16:15 ` David Marchand @ 2021-04-09 12:00 ` David Marchand 3 siblings, 0 replies; 7+ messages in thread From: David Marchand @ 2021-04-09 12:00 UTC (permalink / raw) To: Dmitry Kozlyuk, Anatoly Burakov; +Cc: dev, Jie Zhou On Wed, Mar 24, 2021 at 8:32 PM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote: > @@ -1050,7 +1055,10 @@ rte_eal_memory_detach(void) > * config - we can't zero it out because it might still be referenced > * by other processes. > */ > - rte_mem_unmap(mcfg, RTE_ALIGN(sizeof(*mcfg), page_sz)); > + if (internal_conf->no_shconf == 0) > + if (rte_mem_unmap(mcfg, RTE_ALIGN(sizeof(*mcfg), page_sz)) != 0) > + RTE_LOG(ERR, EAL, "Could not unmap shared memory config: %s\n", > + rte_strerror(rte_errno)); > rte_eal_get_configuration()->mem_config = NULL; > > return 0; We have another issue if eal init fails early, then the application exits calling rte_exit() -> rte_eal_cleanup() -> rte_eal_memory_detach() The issue itself is not related to this current change but rather to dfbc61a2f9a6 ("mem: detach memsegs on cleanup"), but it became visible with the above log. Example: $ ./build/app/dpdk-testpmd --plop ... EAL: FATAL: Invalid 'command line' arguments. EAL: Invalid 'command line' arguments. EAL: Error - exiting with code: 1 Cause: Cannot init EAL: Invalid argument EAL: Could not unmap shared memory config: Invalid argument -- David Marchand ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-04-09 12:00 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-24 18:01 [dpdk-dev] [PATCH] mem: fix cleanup on Windows Dmitry Kozlyuk 2021-03-24 18:35 ` David Marchand 2021-03-24 19:32 ` [dpdk-dev] [PATCH v2] mem: fix cleanup when multi-process is disabled Dmitry Kozlyuk 2021-03-25 15:39 ` Ranjit Menon 2021-03-26 12:34 ` Burakov, Anatoly 2021-03-26 16:15 ` David Marchand 2021-04-09 12:00 ` David Marchand
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.