[v2,5/5] eal: fix memzone fbarray cleanup
Checks
Commit Message
The initialization of the Memzone file-backed array ensures its
uniqueness by employing an exclusive lock. This is crucial because only
one primary process can exist per specific shm_id, which is further
protected by the exclusive EAL runtime configuration lock.
However, during the process closure, the exclusive lock on both the
fbarray and the configuration is not explicitly released. The
responsibility of releasing these locks is left to the generic quit
procedure. This can lead to a potential race condition when the
configuration is released before the fbarray.
To address this, we propose explicitly closing the memzone fbarray. This
ensures proper order of operations during process closure and prevents
any potential race conditions arising from the mismatched lock release
timings.
Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org
Signed-off-by: Artemy Kovalyov <artemyko@nvidia.com>
---
lib/eal/common/eal_common_memzone.c | 12 ++++++++++++
lib/eal/common/eal_private.h | 5 +++++
lib/eal/linux/eal.c | 1 +
3 files changed, 18 insertions(+)
Comments
On 3/7/2024 8:01 AM, Artemy Kovalyov wrote:
> The initialization of the Memzone file-backed array ensures its
> uniqueness by employing an exclusive lock. This is crucial because only
> one primary process can exist per specific shm_id, which is further
> protected by the exclusive EAL runtime configuration lock.
>
I think you meant to say "prefix", not "shm_id".
> However, during the process closure, the exclusive lock on both the
> fbarray and the configuration is not explicitly released. The
> responsibility of releasing these locks is left to the generic quit
> procedure. This can lead to a potential race condition when the
> configuration is released before the fbarray.
>
> To address this, we propose explicitly closing the memzone fbarray. This
> ensures proper order of operations during process closure and prevents
> any potential race conditions arising from the mismatched lock release
> timings.
>
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
>
> Signed-off-by: Artemy Kovalyov <artemyko@nvidia.com>
> ---
I would suggest having a different Fixes: ID, because fbarrays were only
added in 18.05 when we added dynamic memory support. I propose using
this commit ID instead:
49df3db84883 ("memzone: replace memzone array with fbarray")
This is the first commit where memzones used fbarrays.
> +void
> +rte_eal_memzone_cleanup(void)
> +{
> + struct rte_mem_config *mcfg;
> +
> + mcfg = rte_eal_get_configuration()->mem_config;
> +
> + if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> + rte_fbarray_destroy(&mcfg->memzones);
> + }
Nitpick: extraneous brackets, this is a one liner so they're not needed.
With changes above,
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
@@ -447,6 +447,18 @@
return ret;
}
+void
+rte_eal_memzone_cleanup(void)
+{
+ struct rte_mem_config *mcfg;
+
+ mcfg = rte_eal_get_configuration()->mem_config;
+
+ if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+ rte_fbarray_destroy(&mcfg->memzones);
+ }
+}
+
/* Walk all reserved memory zones */
void rte_memzone_walk(void (*func)(const struct rte_memzone *, void *),
void *arg)
@@ -81,6 +81,11 @@ struct rte_config {
int rte_eal_memzone_init(void);
/**
+ * Cleanup the memzone subsystem (private to eal).
+ */
+void rte_eal_memzone_cleanup(void);
+
+/**
* Fill configuration with number of physical and logical processors
*
* This function is private to EAL.
@@ -1375,6 +1375,7 @@ static void rte_eal_init_alert(const char *msg)
eal_trace_fini();
eal_mp_dev_hotplug_cleanup();
rte_eal_alarm_cleanup();
+ rte_eal_memzone_cleanup();
/* after this point, any DPDK pointers will become dangling */
rte_eal_memory_detach();
rte_eal_malloc_heap_cleanup();