Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/56185/?format=api
{ "id": 56185, "url": "http://patches.dpdk.org/api/patches/56185/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/patch/c4df88d88459bb157656e18f1bdf5549ce541527.1562347316.git.anatoly.burakov@intel.com/", "project": { "id": 1, "url": "http://patches.dpdk.org/api/projects/1/?format=api", "name": "DPDK", "link_name": "dpdk", "list_id": "dev.dpdk.org", "list_email": "dev@dpdk.org", "web_url": "http://core.dpdk.org", "scm_url": "git://dpdk.org/dpdk", "webscm_url": "http://git.dpdk.org/dpdk", "list_archive_url": "https://inbox.dpdk.org/dev", "list_archive_url_format": "https://inbox.dpdk.org/dev/{}", "commit_url_format": "" }, "msgid": "<c4df88d88459bb157656e18f1bdf5549ce541527.1562347316.git.anatoly.burakov@intel.com>", "list_archive_url": "https://inbox.dpdk.org/dev/c4df88d88459bb157656e18f1bdf5549ce541527.1562347316.git.anatoly.burakov@intel.com", "date": "2019-07-05T17:22:44", "name": "[v3,1/1] timer: fix resource leak in finalize", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": true, "hash": "a3fea5ea8507f4d810a039df0527d285882563aa", "submitter": { "id": 4, "url": "http://patches.dpdk.org/api/people/4/?format=api", "name": "Anatoly Burakov", "email": "anatoly.burakov@intel.com" }, "delegate": null, "mbox": "http://patches.dpdk.org/project/dpdk/patch/c4df88d88459bb157656e18f1bdf5549ce541527.1562347316.git.anatoly.burakov@intel.com/mbox/", "series": [ { "id": 5378, "url": "http://patches.dpdk.org/api/series/5378/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/list/?series=5378", "date": "2019-07-05T17:22:43", "name": "Fix timer resource leak", "version": 3, "mbox": "http://patches.dpdk.org/series/5378/mbox/" } ], "comments": "http://patches.dpdk.org/api/patches/56185/comments/", "check": "fail", "checks": "http://patches.dpdk.org/api/patches/56185/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id 22BCA1BE32;\n\tFri, 5 Jul 2019 19:22:51 +0200 (CEST)", "from mga12.intel.com (mga12.intel.com [192.55.52.136])\n\tby dpdk.org (Postfix) with ESMTP id 13CF5559A\n\tfor <dev@dpdk.org>; Fri, 5 Jul 2019 19:22:46 +0200 (CEST)", "from fmsmga001.fm.intel.com ([10.253.24.23])\n\tby fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t05 Jul 2019 10:22:46 -0700", "from silpixa00399498.ir.intel.com (HELO\n\tsilpixa00399498.ger.corp.intel.com) ([10.237.223.125])\n\tby fmsmga001.fm.intel.com with ESMTP; 05 Jul 2019 10:22:45 -0700" ], "X-Amp-Result": "SKIPPED(no attachment in message)", "X-Amp-File-Uploaded": "False", "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.63,455,1557212400\"; d=\"scan'208\";a=\"185217218\"", "From": "Anatoly Burakov <anatoly.burakov@intel.com>", "To": "dev@dpdk.org", "Cc": "Robert Sanford <rsanford@akamai.com>,\n\tErik Gabriel Carrillo <erik.g.carrillo@intel.com>", "Date": "Fri, 5 Jul 2019 18:22:44 +0100", "Message-Id": "<c4df88d88459bb157656e18f1bdf5549ce541527.1562347316.git.anatoly.burakov@intel.com>", "X-Mailer": "git-send-email 2.17.1", "In-Reply-To": [ "<cover.1562347316.git.anatoly.burakov@intel.com>", "<cover.1562347316.git.anatoly.burakov@intel.com>" ], "References": [ "<cover.1562347316.git.anatoly.burakov@intel.com>", "<cover.1562332788.git.anatoly.burakov@intel.com>\n\t<cover.1562347316.git.anatoly.burakov@intel.com>" ], "Subject": "[dpdk-dev] [PATCH v3 1/1] 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>,\n\t<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>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "content": "Currently, whenever timer library is initialized, the memory\nis leaked because there is no telling when primary or secondary\nprocesses get to use the state, and there is no way to\ninitialize/deinitialize timer library state without race\nconditions [1] because the data itself must live in shared memory.\n\nAdd a spinlock to the shared mem config to have a way to\nexclusively initialize/deinitialize the timer library without\nany races, and implement the synchronization mechanism based\non this lock in the timer library.\n\nAlso, update the API doc. Note that the behavior of the API\nitself did not change - the requirement to call init in every\nprocess was simply not documented explicitly.\n\nFixes: c0749f7096c7 (\"timer: allow management in shared memory\")\n\n[1] See the following email thread:\nhttps://mails.dpdk.org/archives/dev/2019-May/131498.html\n\nSigned-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>\nSigned-off-by: Anatoly Burakov <anatoly.burakov@intel.com>\nAcked-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>\nReviewed-by: David Marchand <david.marchand@redhat.com>\n---\n lib/librte_eal/common/eal_common_mcfg.c | 14 +++++++\n lib/librte_eal/common/eal_memcfg.h | 2 +\n .../common/include/rte_eal_memconfig.h | 22 ++++++++++\n lib/librte_eal/rte_eal_version.map | 2 +\n lib/librte_timer/rte_timer.c | 41 +++++++++++++------\n lib/librte_timer/rte_timer.h | 5 ++-\n 6 files changed, 71 insertions(+), 15 deletions(-)", "diff": "diff --git a/lib/librte_eal/common/eal_common_mcfg.c b/lib/librte_eal/common/eal_common_mcfg.c\nindex 1825d9083..066549432 100644\n--- a/lib/librte_eal/common/eal_common_mcfg.c\n+++ b/lib/librte_eal/common/eal_common_mcfg.c\n@@ -147,3 +147,17 @@ rte_mcfg_mempool_write_unlock(void)\n \tstruct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;\n \trte_rwlock_write_unlock(&mcfg->mplock);\n }\n+\n+void\n+rte_mcfg_timer_lock(void)\n+{\n+\tstruct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;\n+\trte_spinlock_lock(&mcfg->tlock);\n+}\n+\n+void\n+rte_mcfg_timer_unlock(void)\n+{\n+\tstruct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;\n+\trte_spinlock_unlock(&mcfg->tlock);\n+}\ndiff --git a/lib/librte_eal/common/eal_memcfg.h b/lib/librte_eal/common/eal_memcfg.h\nindex 030f903ad..359beb216 100644\n--- a/lib/librte_eal/common/eal_memcfg.h\n+++ b/lib/librte_eal/common/eal_memcfg.h\n@@ -11,6 +11,7 @@\n #include <rte_memory.h>\n #include <rte_memzone.h>\n #include <rte_pause.h>\n+#include <rte_spinlock.h>\n #include <rte_rwlock.h>\n #include <rte_tailq.h>\n \n@@ -36,6 +37,7 @@ struct rte_mem_config {\n \trte_rwlock_t mlock; /**< used by memzones for thread safety. */\n \trte_rwlock_t qlock; /**< used by tailqs for thread safety. */\n \trte_rwlock_t mplock; /**< used by mempool library for thread safety. */\n+\trte_spinlock_t tlock; /**< used by timer library for thread safety. */\n \n \trte_rwlock_t memory_hotplug_lock;\n \t/**< Indicates whether memory hotplug request is in progress. */\ndiff --git a/lib/librte_eal/common/include/rte_eal_memconfig.h b/lib/librte_eal/common/include/rte_eal_memconfig.h\nindex dc61a6fed..34b0e44a0 100644\n--- a/lib/librte_eal/common/include/rte_eal_memconfig.h\n+++ b/lib/librte_eal/common/include/rte_eal_memconfig.h\n@@ -5,6 +5,8 @@\n #ifndef _RTE_EAL_MEMCONFIG_H_\n #define _RTE_EAL_MEMCONFIG_H_\n \n+#include <rte_compat.h>\n+\n /**\n * @file\n *\n@@ -87,6 +89,26 @@ rte_mcfg_mempool_write_lock(void);\n void\n rte_mcfg_mempool_write_unlock(void);\n \n+/**\n+ * @warning\n+ * @b EXPERIMENTAL: this API may change without prior notice\n+ *\n+ * Lock the internal EAL Timer Library lock for exclusive access.\n+ */\n+__rte_experimental\n+void\n+rte_mcfg_timer_lock(void);\n+\n+/**\n+ * @warning\n+ * @b EXPERIMENTAL: this API may change without prior notice\n+ *\n+ * Unlock the internal EAL Timer Library lock for exclusive access.\n+ */\n+__rte_experimental\n+void\n+rte_mcfg_timer_unlock(void);\n+\n #ifdef __cplusplus\n }\n #endif\ndiff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map\nindex cc4ef04a0..d0ee67dbf 100644\n--- a/lib/librte_eal/rte_eal_version.map\n+++ b/lib/librte_eal/rte_eal_version.map\n@@ -405,4 +405,6 @@ EXPERIMENTAL {\n \t# added in 19.08\n \trte_lcore_cpuset;\n \trte_lcore_to_cpu_id;\n+\trte_mcfg_timer_lock;\n+\trte_mcfg_timer_unlock;\n };\ndiff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c\nindex eaeafd74f..71dffd23b 100644\n--- a/lib/librte_timer/rte_timer.c\n+++ b/lib/librte_timer/rte_timer.c\n@@ -13,6 +13,7 @@\n #include <rte_atomic.h>\n #include <rte_common.h>\n #include <rte_cycles.h>\n+#include <rte_eal_memconfig.h>\n #include <rte_per_lcore.h>\n #include <rte_memory.h>\n #include <rte_launch.h>\n@@ -61,6 +62,8 @@ struct rte_timer_data {\n };\n \n #define RTE_MAX_DATA_ELS 64\n+static const struct rte_memzone *rte_timer_data_mz;\n+static int *volatile rte_timer_mz_refcnt;\n static struct rte_timer_data *rte_timer_data_arr;\n static const uint32_t default_data_id;\n static uint32_t rte_timer_subsystem_initialized;\n@@ -157,28 +160,30 @@ rte_timer_subsystem_init_v1905(void)\n \tint i, lcore_id;\n \tstatic const char *mz_name = \"rte_timer_mz\";\n \tconst size_t data_arr_size =\n-\t\t\t\tRTE_MAX_DATA_ELS * sizeof(*rte_timer_data_arr);\n+\t\t\tRTE_MAX_DATA_ELS * sizeof(*rte_timer_data_arr);\n+\tconst size_t mem_size = data_arr_size + sizeof(*rte_timer_mz_refcnt);\n \tbool do_full_init = true;\n \n \tif (rte_timer_subsystem_initialized)\n \t\treturn -EALREADY;\n \n-reserve:\n-\trte_errno = 0;\n-\tmz = rte_memzone_reserve_aligned(mz_name, data_arr_size, SOCKET_ID_ANY,\n-\t\t\t\t\t 0, RTE_CACHE_LINE_SIZE);\n+\trte_mcfg_timer_lock();\n+\n+\tmz = rte_memzone_lookup(mz_name);\n \tif (mz == NULL) {\n-\t\tif (rte_errno == EEXIST) {\n-\t\t\tmz = rte_memzone_lookup(mz_name);\n-\t\t\tif (mz == NULL)\n-\t\t\t\tgoto reserve;\n-\n-\t\t\tdo_full_init = false;\n-\t\t} else\n+\t\tmz = rte_memzone_reserve_aligned(mz_name, mem_size,\n+\t\t\t\tSOCKET_ID_ANY, 0, RTE_CACHE_LINE_SIZE);\n+\t\tif (mz == NULL) {\n+\t\t\trte_mcfg_timer_unlock();\n \t\t\treturn -ENOMEM;\n-\t}\n+\t\t}\n+\t\tdo_full_init = true;\n+\t} else\n+\t\tdo_full_init = false;\n \n+\trte_timer_data_mz = mz;\n \trte_timer_data_arr = mz->addr;\n+\trte_timer_mz_refcnt = (void *)((char *)mz->addr + data_arr_size);\n \n \tif (do_full_init) {\n \t\tfor (i = 0; i < RTE_MAX_DATA_ELS; i++) {\n@@ -195,6 +200,9 @@ rte_timer_subsystem_init_v1905(void)\n \t}\n \n \trte_timer_data_arr[default_data_id].internal_flags |= FL_ALLOCATED;\n+\t(*rte_timer_mz_refcnt)++;\n+\n+\trte_mcfg_timer_unlock();\n \n \trte_timer_subsystem_initialized = 1;\n \n@@ -210,6 +218,13 @@ rte_timer_subsystem_finalize(void)\n \tif (!rte_timer_subsystem_initialized)\n \t\treturn;\n \n+\trte_mcfg_timer_lock();\n+\n+\tif (--(*rte_timer_mz_refcnt) == 0)\n+\t\trte_memzone_free(rte_timer_data_mz);\n+\n+\trte_mcfg_timer_unlock();\n+\n \trte_timer_subsystem_initialized = 0;\n }\n \ndiff --git a/lib/librte_timer/rte_timer.h b/lib/librte_timer/rte_timer.h\nindex 1b0d5f6a5..05d287d8f 100644\n--- a/lib/librte_timer/rte_timer.h\n+++ b/lib/librte_timer/rte_timer.h\n@@ -172,10 +172,11 @@ int rte_timer_data_dealloc(uint32_t id);\n * Initializes internal variables (list, locks and so on) for the RTE\n * timer library.\n *\n+ * @note\n+ * This function must be called in every process before using the library.\n+ *\n * @return\n * - 0: Success\n- * - -EEXIST: Returned in secondary process when primary process has not\n- * yet initialized the timer subsystem\n * - -ENOMEM: Unable to allocate memory needed to initialize timer\n * subsystem\n */\n", "prefixes": [ "v3", "1/1" ] }