From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id A8065A0096 for ; Thu, 9 May 2019 00:36:45 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 91A3F34F0; Thu, 9 May 2019 00:36:44 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 906DF2C2B for ; Thu, 9 May 2019 00:36:42 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 May 2019 15:36:41 -0700 X-ExtLoop1: 1 Received: from txasoft-yocto.an.intel.com ([10.123.72.192]) by fmsmga001.fm.intel.com with ESMTP; 08 May 2019 15:36:40 -0700 From: Erik Gabriel Carrillo To: rsanford@akamai.com, thomas@monjalon.net, anatoly.burakov@intel.com Cc: dev@dpdk.org Date: Wed, 8 May 2019 17:35:06 -0500 Message-Id: <1557354906-2500-1-git-send-email-erik.g.carrillo@intel.com> X-Mailer: git-send-email 1.7.10 In-Reply-To: <1556924082-22535-1-git-send-email-erik.g.carrillo@intel.com> References: <1556924082-22535-1-git-send-email-erik.g.carrillo@intel.com> Subject: [dpdk-dev] [PATCH v3] timer: fix resource leak in finalize X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Content-Type: text/plain; charset="UTF-8" Message-ID: <20190508223506.6jx36E33FP1W1nulZlzaSHHiyBTxf9l7mYo_kUXcmm4@z> 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 --- 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 #include #include +#include #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