[dpdk-dev] [PATCH v3] timer: fix resource leak in finalize

[Erik Gabriel Carrillo]

By using a lock added to the rte_mem_config (which lives in shared
memory), we can synchronize multiple processes in init/finalize and
safely free allocations made during init.

Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo at intel.com>
---
changes in v3:
 - The previous version had race condition.  This version fixes the race
   by adding a lock in shared memory outside of the DPDK heap area 
   that can be used to safely free the memzone reserved in the subsystem
   init call. (Anatoly)

   This patch depends on http://patches.dpdk.org/patch/53333/.
 
changes in v2:
 - Handle scenario where primary process exits before secondaries such
   that memzone is not freed early (Anatoly)

 lib/librte_eal/common/include/rte_eal_memconfig.h |  3 +++
 lib/librte_timer/rte_timer.c                      | 23 ++++++++++++++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/include/rte_eal_memconfig.h b/lib/librte_eal/common/include/rte_eal_memconfig.h
index 84aabe3..8cbc09c 100644
--- a/lib/librte_eal/common/include/rte_eal_memconfig.h
+++ b/lib/librte_eal/common/include/rte_eal_memconfig.h
@@ -64,6 +64,9 @@ struct rte_mem_config {
 	rte_rwlock_t memory_hotplug_lock;
 	/**< indicates whether memory hotplug request is in progress. */
 
+	rte_spinlock_t timer_subsystem_lock;
+	/**< indicates whether timer subsystem init/finalize is in progress. */
+
 	/* memory segments and zones */
 	struct rte_fbarray memzones; /**< Memzone descriptors. */
 
diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
index c0f5b87..a055d6e 100644
--- a/lib/librte_timer/rte_timer.c
+++ b/lib/librte_timer/rte_timer.c
@@ -26,6 +26,7 @@
 #include <rte_malloc.h>
 #include <rte_compat.h>
 #include <rte_errno.h>
+#include <rte_eal_memconfig.h>
 
 #include "rte_timer.h"
 
@@ -60,7 +61,12 @@ struct rte_timer_data {
 	uint8_t internal_flags;
 };
 
+#define RTE_TIMER_SUBSYSTEM_LOCK \
+	(&rte_eal_get_configuration()->mem_config->timer_subsystem_lock)
+
 #define RTE_MAX_DATA_ELS 64
+static const struct rte_memzone *rte_timer_data_mz;
+static int *rte_timer_mz_refcnt;
 static struct rte_timer_data *rte_timer_data_arr;
 static const uint32_t default_data_id;
 static uint32_t rte_timer_subsystem_initialized;
@@ -163,22 +169,27 @@ rte_timer_subsystem_init_v1905(void)
 	if (rte_timer_subsystem_initialized)
 		return -EALREADY;
 
+	rte_spinlock_lock(RTE_TIMER_SUBSYSTEM_LOCK);
 lookup:
 	mz = rte_memzone_lookup(mz_name);
 	if (mz == NULL) {
-		mz = rte_memzone_reserve_aligned(mz_name, data_arr_size,
+		mz = rte_memzone_reserve_aligned(mz_name,
+				data_arr_size + sizeof(*rte_timer_mz_refcnt),
 				SOCKET_ID_ANY, 0, RTE_CACHE_LINE_SIZE);
 		if (mz == NULL) {
 			if (rte_errno == EEXIST)
 				goto lookup;
 
+			rte_spinlock_unlock(RTE_TIMER_SUBSYSTEM_LOCK);
 			return -ENOMEM;
 		}
 
 		do_full_init = true;
 	}
 
+	rte_timer_data_mz = mz;
 	rte_timer_data_arr = mz->addr;
+	rte_timer_mz_refcnt = (void *)((char *)mz->addr + data_arr_size);
 
 	if (do_full_init) {
 		for (i = 0; i < RTE_MAX_DATA_ELS; i++) {
@@ -195,6 +206,9 @@ rte_timer_subsystem_init_v1905(void)
 	}
 
 	rte_timer_data_arr[default_data_id].internal_flags |= FL_ALLOCATED;
+	(*rte_timer_mz_refcnt)++;
+
+	rte_spinlock_unlock(RTE_TIMER_SUBSYSTEM_LOCK);
 
 	rte_timer_subsystem_initialized = 1;
 
@@ -210,6 +224,13 @@ rte_timer_subsystem_finalize(void)
 	if (!rte_timer_subsystem_initialized)
 		return;
 
+	rte_spinlock_lock(RTE_TIMER_SUBSYSTEM_LOCK);
+
+	if (--(*rte_timer_mz_refcnt) == 0)
+		rte_memzone_free(rte_timer_data_mz);
+
+	rte_spinlock_unlock(RTE_TIMER_SUBSYSTEM_LOCK);
+
 	rte_timer_subsystem_initialized = 0;
 }
 
-- 
2.6.4