From patchwork Mon Jul 20 14:38:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 74503 X-Patchwork-Delegate: david.marchand@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 34A25A0527; Mon, 20 Jul 2020 16:37:32 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id CA1FD1BFBC; Mon, 20 Jul 2020 16:37:30 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 8045CA69 for ; Mon, 20 Jul 2020 16:37:28 +0200 (CEST) IronPort-SDR: pqAQhn3uFbZ0KlRlYN0EsNGjLtnlsNszhVvJ7Cr9UqYxqc8fWJcDAqGH7pGU0IFZtqQdq2Rvjr 0JhiRGcGNddw== X-IronPort-AV: E=McAfee;i="6000,8403,9687"; a="138012118" X-IronPort-AV: E=Sophos;i="5.75,375,1589266800"; d="scan'208";a="138012118" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jul 2020 07:37:27 -0700 IronPort-SDR: G2D3vNlKKh9MEawzBOv78OfjzjomdsJtG3ovzfr8N6lWTloJLQ4pZLn7k4BIy0DtUCluufjevS VYMRvJPY66VQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.75,375,1589266800"; d="scan'208";a="326075383" Received: from silpixa00399779.ir.intel.com (HELO silpixa00399779.ger.corp.intel.com) ([10.237.222.209]) by FMSMGA003.fm.intel.com with ESMTP; 20 Jul 2020 07:37:25 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: david.marchand@redhat.com, igor.romanov@oktetlabs.ru, honnappa.nagarahalli@arm.com, ferruh.yigit@intel.com, nd@arm.com, aconole@redhat.com, l.wojciechow@partner.samsung.com, Harry van Haaren Date: Mon, 20 Jul 2020 15:38:28 +0100 Message-Id: <20200720143829.46280-1-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200720120938.34660-1-harry.van.haaren@intel.com> References: <20200720120938.34660-1-harry.van.haaren@intel.com> Subject: [dpdk-dev] [PATCH v2 1/2] service: add API to retrieve service core active 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" This commit adds a new experimental API which allows the user to retrieve the active state of an lcore. Knowing when the service lcore is completed its polling loop can be useful to applications to avoid race conditions when e.g. finalizing statistics. The service thread itself now has a variable to indicate if its thread is active. When zero the service thread has completed its service, and has returned from the service_runner_func() function. Suggested-by: Lukasz Wojciechowski Signed-off-by: Harry van Haaren Acked-by: Lukasz Wojciechowski Reviewed-by: Phil Yang --- Thanks for feedback Lukasz, please have a look and see if this was what you were expecting? Honnappa/Phil, are the __atomic_load/store's correct? --- lib/librte_eal/common/rte_service.c | 14 ++++++++++++++ lib/librte_eal/include/rte_service.h | 9 +++++++++ lib/librte_eal/rte_eal_version.map | 1 + 3 files changed, 24 insertions(+) diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index 6a0e0ff65..4c276b006 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -65,6 +65,7 @@ struct core_state { /* map of services IDs are run on this core */ uint64_t service_mask; uint8_t runstate; /* running or stopped */ + uint8_t thread_active; /* indicates when thread is in service_run() */ uint8_t is_service_core; /* set if core is currently a service core */ uint8_t service_active_on_lcore[RTE_SERVICE_NUM_MAX]; uint64_t loops; @@ -457,6 +458,8 @@ service_runner_func(void *arg) const int lcore = rte_lcore_id(); struct core_state *cs = &lcore_states[lcore]; + __atomic_store_n(&cs->thread_active, 1, __ATOMIC_RELAXED); + /* runstate act as the guard variable. Use load-acquire * memory order here to synchronize with store-release * in runstate update functions. @@ -475,9 +478,20 @@ service_runner_func(void *arg) cs->loops++; } + __atomic_store_n(&cs->thread_active, 0, __ATOMIC_RELAXED); return 0; } +int32_t +rte_service_lcore_active(uint32_t lcore) +{ + if (lcore >= RTE_MAX_LCORE || !lcore_states[lcore].is_service_core) + return -EINVAL; + + return __atomic_load_n(&lcore_states[lcore].thread_active, + __ATOMIC_RELAXED); +} + int32_t rte_service_lcore_count(void) { diff --git a/lib/librte_eal/include/rte_service.h b/lib/librte_eal/include/rte_service.h index e2d0a6dd3..f7134b5c0 100644 --- a/lib/librte_eal/include/rte_service.h +++ b/lib/librte_eal/include/rte_service.h @@ -261,6 +261,15 @@ int32_t rte_service_lcore_start(uint32_t lcore_id); */ int32_t rte_service_lcore_stop(uint32_t lcore_id); +/** + * Reports if a service lcore is currently running. + * @retval 0 Service thread is not active, and has been returned to EAL. + * @retval 1 Service thread is in the service core polling loop. + * @retval -EINVAL Invalid *lcore_id* provided. + */ +__rte_experimental +int32_t rte_service_lcore_active(uint32_t lcore_id); + /** * Adds lcore to the list of service cores. * diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map index bf0c17c23..d53d5d5b9 100644 --- a/lib/librte_eal/rte_eal_version.map +++ b/lib/librte_eal/rte_eal_version.map @@ -401,6 +401,7 @@ EXPERIMENTAL { rte_lcore_dump; rte_lcore_iterate; rte_mp_disable; + rte_service_lcore_active; rte_thread_register; rte_thread_unregister; }; From patchwork Mon Jul 20 14:38:29 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 74504 X-Patchwork-Delegate: david.marchand@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 07B24A0527; Mon, 20 Jul 2020 16:37:42 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A53C21BFC6; Mon, 20 Jul 2020 16:37:36 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 8DDC71BFC5 for ; Mon, 20 Jul 2020 16:37:34 +0200 (CEST) IronPort-SDR: Rgppq7zTk2UZ2WIzckpSlCG4768yi/ff78jtWAQ0DTV5sS/wUM8Srvkk43TXfo6xIh+ICBq/Gi xfjp7qQx4n2A== X-IronPort-AV: E=McAfee;i="6000,8403,9687"; a="214598026" X-IronPort-AV: E=Sophos;i="5.75,375,1589266800"; d="scan'208";a="214598026" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jul 2020 07:37:33 -0700 IronPort-SDR: G3Dj0SP+Bfl2xkvVMV8jlz5QivJcbWPf48mkWjFg8Q+2CA7MgMzOmnZygvnNWHOKrssqz5Ughc zWJOc5lEgKJw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.75,375,1589266800"; d="scan'208";a="326075407" Received: from silpixa00399779.ir.intel.com (HELO silpixa00399779.ger.corp.intel.com) ([10.237.222.209]) by FMSMGA003.fm.intel.com with ESMTP; 20 Jul 2020 07:37:31 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: david.marchand@redhat.com, igor.romanov@oktetlabs.ru, honnappa.nagarahalli@arm.com, ferruh.yigit@intel.com, nd@arm.com, aconole@redhat.com, l.wojciechow@partner.samsung.com, Harry van Haaren Date: Mon, 20 Jul 2020 15:38:29 +0100 Message-Id: <20200720143829.46280-2-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200720143829.46280-1-harry.van.haaren@intel.com> References: <20200720120938.34660-1-harry.van.haaren@intel.com> <20200720143829.46280-1-harry.van.haaren@intel.com> Subject: [dpdk-dev] [PATCH v2 2/2] test/service: fix race condition on stopping lcore 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" This commit fixes a potential race condition in the tests where the lcore running a service would increment a counter that was already reset by the test-suite thread. The resulting race-condition incremented value could cause CI failures, as indicated by DPDK's CI. This patch fixes the race-condition by making use of the added rte_service_lcore_active() API, which indicates when a service-core is no longer in the service-core polling loop. The unit test makes use of the above function to detect when all statistics increments are done in the service-core thread, and then the unit test continues finalizing and checking state. Fixes: f28f3594ded2 ("service: add attribute API") Reported-by: David Marchand Signed-off-by: Harry van Haaren Acked-by: Lukasz Wojciechowski Reviewed-by: Phil Yang --- Thanks for discussion on v1, this v2 fixup for the CI including previous feedback on ML. --- app/test/test_service_cores.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c index ef1d8fcb9..a45762915 100644 --- a/app/test/test_service_cores.c +++ b/app/test/test_service_cores.c @@ -362,6 +362,9 @@ service_lcore_attr_get(void) "Service core add did not return zero"); TEST_ASSERT_EQUAL(0, rte_service_map_lcore_set(id, slcore_id, 1), "Enabling valid service and core failed"); + /* Ensure service is not active before starting */ + TEST_ASSERT_EQUAL(0, rte_service_lcore_active(slcore_id), + "Not-active service core reported as active"); TEST_ASSERT_EQUAL(0, rte_service_lcore_start(slcore_id), "Starting service core failed"); @@ -382,7 +385,24 @@ service_lcore_attr_get(void) lcore_attr_id, &lcore_attr_value), "Invalid lcore attr didn't return -EINVAL"); - rte_service_lcore_stop(slcore_id); + /* Ensure service is active */ + TEST_ASSERT_EQUAL(1, rte_service_lcore_active(slcore_id), + "Active service core reported as not-active"); + + TEST_ASSERT_EQUAL(0, rte_service_map_lcore_set(id, slcore_id, 0), + "Disabling valid service and core failed"); + TEST_ASSERT_EQUAL(0, rte_service_lcore_stop(slcore_id), + "Failed to stop service lcore"); + + int i = 0; + while (rte_service_lcore_active(slcore_id) == 1) { + rte_delay_ms(1); + i++; + if (i > 100) + break; + } + TEST_ASSERT_EQUAL(0, rte_service_lcore_active(slcore_id), + "Service lcore not stopped after waiting."); TEST_ASSERT_EQUAL(0, rte_service_lcore_attr_reset_all(slcore_id), "Valid lcore_attr_reset_all() didn't return success");