From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <erik.g.carrillo@intel.com>
Received: from mga03.intel.com (mga03.intel.com [134.134.136.65])
by dpdk.org (Postfix) with ESMTP id 906DF2C2B
for <dev@dpdk.org>; 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 <erik.g.carrillo@intel.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 08 May 2019 22:36:43 -0000
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@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
From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by dpdk.space (Postfix) with ESMTP id A8065A0096
for <public@inbox.dpdk.org>; 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 <dev@dpdk.org>; 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 <erik.g.carrillo@intel.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
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 <erik.g.carrillo@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