From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id C1FD9A0487 for ; Thu, 4 Jul 2019 12:44:27 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2EA741BDE5; Thu, 4 Jul 2019 12:44:27 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 33AE91BDE3 for ; Thu, 4 Jul 2019 12:44:25 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Jul 2019 03:44:24 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.63,450,1557212400"; d="scan'208";a="172398291" Received: from aburakov-mobl1.ger.corp.intel.com (HELO [10.237.220.126]) ([10.237.220.126]) by FMSMGA003.fm.intel.com with ESMTP; 04 Jul 2019 03:44:23 -0700 To: David Marchand Cc: dev , Erik Gabriel Carrillo References: <1557354906-2500-1-git-send-email-erik.g.carrillo@intel.com> <0743551eba840752223a6447ab4dcaf0731add39.1561478924.git.anatoly.burakov@intel.com> From: "Burakov, Anatoly" Message-ID: <8b99a36a-fbc6-93db-d0cd-6225c3f639d0@intel.com> Date: Thu, 4 Jul 2019 11:44:22 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 1/2] eal: add internal locks for timer lib into EAL 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" On 04-Jul-19 10:09 AM, David Marchand wrote: > On Tue, Jun 25, 2019 at 6:12 PM Anatoly Burakov > wrote: > >> Currently, timer library has a memory leak because there is no >> way to concurrently initialize/deinitialize shared memory because >> of race conditions [1]. >> >> Add a spinlock to the shared mem config to have a way to >> exclusively initialize/deinitialize the timer library without >> any races. >> >> [1] See the following email thread: >> https://mails.dpdk.org/archives/dev/2019-May/131498.html >> >> Signed-off-by: Anatoly Burakov >> --- >> lib/librte_eal/common/eal_common_mcfg.c | 14 ++++++++++++++ >> lib/librte_eal/common/eal_memcfg.h | 2 ++ >> .../common/include/rte_eal_memconfig.h | 18 ++++++++++++++++++ >> lib/librte_eal/rte_eal_version.map | 2 ++ >> 4 files changed, 36 insertions(+) >> >> diff --git a/lib/librte_eal/common/eal_common_mcfg.c >> b/lib/librte_eal/common/eal_common_mcfg.c >> index 1825d9083..066549432 100644 >> --- a/lib/librte_eal/common/eal_common_mcfg.c >> +++ b/lib/librte_eal/common/eal_common_mcfg.c >> @@ -147,3 +147,17 @@ rte_mcfg_mempool_write_unlock(void) >> struct rte_mem_config *mcfg = >> rte_eal_get_configuration()->mem_config; >> rte_rwlock_write_unlock(&mcfg->mplock); >> } >> + >> +void >> +rte_mcfg_timer_lock(void) >> +{ >> + struct rte_mem_config *mcfg = >> rte_eal_get_configuration()->mem_config; >> + rte_spinlock_lock(&mcfg->tlock); >> +} >> + >> +void >> +rte_mcfg_timer_unlock(void) >> +{ >> + struct rte_mem_config *mcfg = >> rte_eal_get_configuration()->mem_config; >> + rte_spinlock_unlock(&mcfg->tlock); >> +} >> diff --git a/lib/librte_eal/common/eal_memcfg.h >> b/lib/librte_eal/common/eal_memcfg.h >> index e1aae32df..00370cece 100644 >> --- a/lib/librte_eal/common/eal_memcfg.h >> +++ b/lib/librte_eal/common/eal_memcfg.h >> @@ -11,6 +11,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> >> @@ -38,6 +39,7 @@ struct rte_mem_config { >> rte_rwlock_t mlock; /**< only used by memzone LIB for >> thread-safe. */ >> rte_rwlock_t qlock; /**< used for tailq operation for thread >> safe. */ >> rte_rwlock_t mplock; /**< only used by mempool LIB for >> thread-safe. */ >> + rte_spinlock_t tlock; /**< needed for timer lib thread safety. */ >> > >> rte_rwlock_t memory_hotplug_lock; >> /**< indicates whether memory hotplug request is in progress. */ >> diff --git a/lib/librte_eal/common/include/rte_eal_memconfig.h >> b/lib/librte_eal/common/include/rte_eal_memconfig.h >> index 1b615c892..05a32e12a 100644 >> --- a/lib/librte_eal/common/include/rte_eal_memconfig.h >> +++ b/lib/librte_eal/common/include/rte_eal_memconfig.h >> @@ -109,6 +109,24 @@ rte_mcfg_mempool_write_lock(void); >> void >> rte_mcfg_mempool_write_unlock(void); >> >> +/** >> + * @warning >> + * @b EXPERIMENTAL: this API may change without prior notice >> + * >> + * Lock the internal EAL Timer Library lock for exclusive access. >> + */ >> +void __rte_experimental >> > > As mentionned by Erik, > > rte_experimental > void > > +rte_mcfg_timer_lock(void); >> + >> +/** >> + * @warning >> + * @b EXPERIMENTAL: this API may change without prior notice >> + * >> + * Unlock the internal EAL Timer Library lock for exclusive access. >> + */ >> +void __rte_experimental >> > > rte_experimental > void > > > >> +rte_mcfg_timer_unlock(void); >> + >> #ifdef __cplusplus >> } >> #endif >> diff --git a/lib/librte_eal/rte_eal_version.map >> b/lib/librte_eal/rte_eal_version.map >> index c28951f65..bc08fc4df 100644 >> --- a/lib/librte_eal/rte_eal_version.map >> +++ b/lib/librte_eal/rte_eal_version.map >> @@ -367,6 +367,8 @@ EXPERIMENTAL { >> rte_malloc_heap_memory_detach; >> rte_malloc_heap_memory_remove; >> rte_malloc_heap_socket_is_external; >> + rte_mcfg_timer_lock; >> + rte_mcfg_timer_unlock; >> rte_mem_alloc_validator_register; >> rte_mem_alloc_validator_unregister; >> rte_mem_check_dma_mask; >> -- >> 2.17.1 >> > > Can you put this at the end of the EXPERIMENTAL block under the 19.08 > comment? > OK, will do both for v2. -- Thanks, Anatoly