From patchwork Thu Apr 23 16:31:17 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Yang X-Patchwork-Id: 69199 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 7A0FCA034E; Thu, 23 Apr 2020 18:32:10 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 35EE71D161; Thu, 23 Apr 2020 18:32:01 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by dpdk.org (Postfix) with ESMTP id 84B211D152; Thu, 23 Apr 2020 18:31:59 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0E67730E; Thu, 23 Apr 2020 09:31:59 -0700 (PDT) Received: from phil-VirtualBox.arm.com (A010647.Arm.com [10.170.243.60]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id E1BD33F68F; Thu, 23 Apr 2020 09:31:55 -0700 (PDT) From: Phil Yang To: harry.van.haaren@intel.com, dev@dpdk.org Cc: thomas@monjalon.net, david.marchand@redhat.com, konstantin.ananyev@intel.com, jerinj@marvell.com, hemant.agrawal@nxp.com, Honnappa.Nagarahalli@arm.com, gavin.hu@arm.com, nd@arm.com, Honnappa Nagarahalli , stable@dpdk.org Date: Fri, 24 Apr 2020 00:31:17 +0800 Message-Id: <1587659482-27133-2-git-send-email-phil.yang@arm.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1587659482-27133-1-git-send-email-phil.yang@arm.com> References: <1584407863-774-8-git-send-email-phil.yang@arm.com> <1587659482-27133-1-git-send-email-phil.yang@arm.com> Subject: [dpdk-dev] [PATCH v2 1/6] service: fix race condition for MT unsafe service 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" From: Honnappa Nagarahalli The MT unsafe service might get configured to run on another core while the service is running currently. This might result in the MT unsafe service running on multiple cores simultaneously. Use 'execute_lock' always when the service is MT unsafe. Fixes: e9139a32f6e8 ("service: add function to run on app lcore") Cc: stable@dpdk.org Signed-off-by: Honnappa Nagarahalli Reviewed-by: Phil Yang --- lib/librte_eal/common/rte_service.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index 70d17a5..b8c465e 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -50,6 +50,10 @@ struct rte_service_spec_impl { uint8_t internal_flags; /* per service statistics */ + /* Indicates how many cores the service is mapped to run on. + * It does not indicate the number of cores the service is running + * on currently. + */ rte_atomic32_t num_mapped_cores; uint64_t calls; uint64_t cycles_spent; @@ -370,12 +374,7 @@ service_run(uint32_t i, struct core_state *cs, uint64_t service_mask, cs->service_active_on_lcore[i] = 1; - /* check do we need cmpset, if MT safe or <= 1 core - * mapped, atomic ops are not required. - */ - const int use_atomics = (service_mt_safe(s) == 0) && - (rte_atomic32_read(&s->num_mapped_cores) > 1); - if (use_atomics) { + if (service_mt_safe(s) == 0) { if (!rte_atomic32_cmpset((uint32_t *)&s->execute_lock, 0, 1)) return -EBUSY; From patchwork Thu Apr 23 16:31:18 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Yang X-Patchwork-Id: 69200 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 C7F7CA034E; Thu, 23 Apr 2020 18:32:21 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B52311D41F; Thu, 23 Apr 2020 18:32:06 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by dpdk.org (Postfix) with ESMTP id 84D8B1C2F5; Thu, 23 Apr 2020 18:32:05 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0B88730E; Thu, 23 Apr 2020 09:32:05 -0700 (PDT) Received: from phil-VirtualBox.arm.com (A010647.Arm.com [10.170.243.60]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id C77A93F68F; Thu, 23 Apr 2020 09:32:01 -0700 (PDT) From: Phil Yang To: harry.van.haaren@intel.com, dev@dpdk.org Cc: thomas@monjalon.net, david.marchand@redhat.com, konstantin.ananyev@intel.com, jerinj@marvell.com, hemant.agrawal@nxp.com, Honnappa.Nagarahalli@arm.com, gavin.hu@arm.com, nd@arm.com, Honnappa Nagarahalli , stable@dpdk.org Date: Fri, 24 Apr 2020 00:31:18 +0800 Message-Id: <1587659482-27133-3-git-send-email-phil.yang@arm.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1587659482-27133-1-git-send-email-phil.yang@arm.com> References: <1584407863-774-8-git-send-email-phil.yang@arm.com> <1587659482-27133-1-git-send-email-phil.yang@arm.com> Subject: [dpdk-dev] [PATCH v2 2/6] service: identify service running on another core correctly 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" From: Honnappa Nagarahalli The logic to identify if the MT unsafe service is running on another core can return -EBUSY spuriously. In such cases, running the service becomes costlier than using atomic operations. Assume that the application passes the right parameters and reduces the number of instructions for all cases. Cc: stable@dpdk.org Fixes: 8d39d3e237c2 ("service: fix race in service on app lcore function") Signed-off-by: Honnappa Nagarahalli Reviewed-by: Phil Yang --- lib/librte_eal/common/rte_service.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index b8c465e..c89472b 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -360,7 +360,7 @@ rte_service_runner_do_callback(struct rte_service_spec_impl *s, /* Expects the service 's' is valid. */ static int32_t service_run(uint32_t i, struct core_state *cs, uint64_t service_mask, - struct rte_service_spec_impl *s) + struct rte_service_spec_impl *s, uint32_t serialize_mt_unsafe) { if (!s) return -EINVAL; @@ -374,7 +374,7 @@ service_run(uint32_t i, struct core_state *cs, uint64_t service_mask, cs->service_active_on_lcore[i] = 1; - if (service_mt_safe(s) == 0) { + if ((service_mt_safe(s) == 0) && (serialize_mt_unsafe == 1)) { if (!rte_atomic32_cmpset((uint32_t *)&s->execute_lock, 0, 1)) return -EBUSY; @@ -412,24 +412,14 @@ rte_service_run_iter_on_app_lcore(uint32_t id, uint32_t serialize_mt_unsafe) SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); - /* Atomically add this core to the mapped cores first, then examine if - * we can run the service. This avoids a race condition between - * checking the value, and atomically adding to the mapped count. + /* Increment num_mapped_cores to indicate that the service + * is running on a core. */ - if (serialize_mt_unsafe) - rte_atomic32_inc(&s->num_mapped_cores); + rte_atomic32_inc(&s->num_mapped_cores); - if (service_mt_safe(s) == 0 && - rte_atomic32_read(&s->num_mapped_cores) > 1) { - if (serialize_mt_unsafe) - rte_atomic32_dec(&s->num_mapped_cores); - return -EBUSY; - } - - int ret = service_run(id, cs, UINT64_MAX, s); + int ret = service_run(id, cs, UINT64_MAX, s, serialize_mt_unsafe); - if (serialize_mt_unsafe) - rte_atomic32_dec(&s->num_mapped_cores); + rte_atomic32_dec(&s->num_mapped_cores); return ret; } @@ -449,7 +439,7 @@ rte_service_runner_func(void *arg) if (!service_valid(i)) continue; /* return value ignored as no change to code flow */ - service_run(i, cs, service_mask, service_get(i)); + service_run(i, cs, service_mask, service_get(i), 1); } cs->loops++; From patchwork Thu Apr 23 16:31:19 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Yang X-Patchwork-Id: 69201 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 AC193A034E; Thu, 23 Apr 2020 18:32:31 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 345941D408; Thu, 23 Apr 2020 18:32:13 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by dpdk.org (Postfix) with ESMTP id 2CAAF1C2F5 for ; Thu, 23 Apr 2020 18:32:11 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id AAF4F30E; Thu, 23 Apr 2020 09:32:10 -0700 (PDT) Received: from phil-VirtualBox.arm.com (A010647.Arm.com [10.170.243.60]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id D20463F68F; Thu, 23 Apr 2020 09:32:07 -0700 (PDT) From: Phil Yang To: harry.van.haaren@intel.com, dev@dpdk.org Cc: thomas@monjalon.net, david.marchand@redhat.com, konstantin.ananyev@intel.com, jerinj@marvell.com, hemant.agrawal@nxp.com, Honnappa.Nagarahalli@arm.com, gavin.hu@arm.com, nd@arm.com Date: Fri, 24 Apr 2020 00:31:19 +0800 Message-Id: <1587659482-27133-4-git-send-email-phil.yang@arm.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1587659482-27133-1-git-send-email-phil.yang@arm.com> References: <1584407863-774-8-git-send-email-phil.yang@arm.com> <1587659482-27133-1-git-send-email-phil.yang@arm.com> Subject: [dpdk-dev] [PATCH v2 3/6] service: remove rte prefix from static functions 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" clean up rte prefix from static functions. remove unused parameter for service_dump_one function. Signed-off-by: Phil Yang Reviewed-by: Honnappa Nagarahalli --- lib/librte_eal/common/rte_service.c | 34 +++++++++++----------------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index c89472b..ed20702 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -340,7 +340,7 @@ rte_service_runstate_get(uint32_t id) } static inline void -rte_service_runner_do_callback(struct rte_service_spec_impl *s, +service_runner_do_callback(struct rte_service_spec_impl *s, struct core_state *cs, uint32_t service_idx) { void *userdata = s->spec.callback_userdata; @@ -378,10 +378,10 @@ service_run(uint32_t i, struct core_state *cs, uint64_t service_mask, if (!rte_atomic32_cmpset((uint32_t *)&s->execute_lock, 0, 1)) return -EBUSY; - rte_service_runner_do_callback(s, cs, i); + service_runner_do_callback(s, cs, i); rte_atomic32_clear(&s->execute_lock); } else - rte_service_runner_do_callback(s, cs, i); + service_runner_do_callback(s, cs, i); return 0; } @@ -425,14 +425,14 @@ rte_service_run_iter_on_app_lcore(uint32_t id, uint32_t serialize_mt_unsafe) } static int32_t -rte_service_runner_func(void *arg) +service_runner_func(void *arg) { RTE_SET_USED(arg); uint32_t i; const int lcore = rte_lcore_id(); struct core_state *cs = &lcore_states[lcore]; - while (lcore_states[lcore].runstate == RUNSTATE_RUNNING) { + while (cs->runstate == RUNSTATE_RUNNING) { const uint64_t service_mask = cs->service_mask; for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) { @@ -693,9 +693,9 @@ rte_service_lcore_start(uint32_t lcore) /* set core to run state first, and then launch otherwise it will * return immediately as runstate keeps it in the service poll loop */ - lcore_states[lcore].runstate = RUNSTATE_RUNNING; + cs->runstate = RUNSTATE_RUNNING; - int ret = rte_eal_remote_launch(rte_service_runner_func, 0, lcore); + int ret = rte_eal_remote_launch(service_runner_func, 0, lcore); /* returns -EBUSY if the core is already launched, 0 on success */ return ret; } @@ -774,13 +774,9 @@ rte_service_lcore_attr_get(uint32_t lcore, uint32_t attr_id, } static void -rte_service_dump_one(FILE *f, struct rte_service_spec_impl *s, - uint64_t all_cycles, uint32_t reset) +service_dump_one(FILE *f, struct rte_service_spec_impl *s, uint32_t reset) { /* avoid divide by zero */ - if (all_cycles == 0) - all_cycles = 1; - int calls = 1; if (s->calls != 0) calls = s->calls; @@ -807,7 +803,7 @@ rte_service_attr_reset_all(uint32_t id) SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); int reset = 1; - rte_service_dump_one(NULL, s, 0, reset); + service_dump_one(NULL, s, reset); return 0; } @@ -851,21 +847,13 @@ rte_service_dump(FILE *f, uint32_t id) uint32_t i; int print_one = (id != UINT32_MAX); - uint64_t total_cycles = 0; - - for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) { - if (!service_valid(i)) - continue; - total_cycles += rte_services[i].cycles_spent; - } - /* print only the specified service */ if (print_one) { struct rte_service_spec_impl *s; SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); fprintf(f, "Service %s Summary\n", s->spec.name); uint32_t reset = 0; - rte_service_dump_one(f, s, total_cycles, reset); + service_dump_one(f, s, reset); return 0; } @@ -875,7 +863,7 @@ rte_service_dump(FILE *f, uint32_t id) if (!service_valid(i)) continue; uint32_t reset = 0; - rte_service_dump_one(f, &rte_services[i], total_cycles, reset); + service_dump_one(f, &rte_services[i], reset); } fprintf(f, "Service Cores Summary\n"); From patchwork Thu Apr 23 16:31:20 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Yang X-Patchwork-Id: 69202 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 B3A6BA034E; Thu, 23 Apr 2020 18:32:43 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D6E901D453; Thu, 23 Apr 2020 18:32:16 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by dpdk.org (Postfix) with ESMTP id C845B1D452 for ; Thu, 23 Apr 2020 18:32:15 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 52A2C30E; Thu, 23 Apr 2020 09:32:15 -0700 (PDT) Received: from phil-VirtualBox.arm.com (A010647.Arm.com [10.170.243.60]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 7FDEC3F68F; Thu, 23 Apr 2020 09:32:12 -0700 (PDT) From: Phil Yang To: harry.van.haaren@intel.com, dev@dpdk.org Cc: thomas@monjalon.net, david.marchand@redhat.com, konstantin.ananyev@intel.com, jerinj@marvell.com, hemant.agrawal@nxp.com, Honnappa.Nagarahalli@arm.com, gavin.hu@arm.com, nd@arm.com Date: Fri, 24 Apr 2020 00:31:20 +0800 Message-Id: <1587659482-27133-5-git-send-email-phil.yang@arm.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1587659482-27133-1-git-send-email-phil.yang@arm.com> References: <1584407863-774-8-git-send-email-phil.yang@arm.com> <1587659482-27133-1-git-send-email-phil.yang@arm.com> Subject: [dpdk-dev] [PATCH v2 4/6] service: remove redundant code 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" The service id validation is duplicated, remove the redundant code in the calling functions. Signed-off-by: Phil Yang Reviewed-by: Honnappa Nagarahalli --- lib/librte_eal/common/rte_service.c | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index ed20702..9c1a1d5 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -541,24 +541,12 @@ rte_service_start_with_defaults(void) } static int32_t -service_update(struct rte_service_spec *service, uint32_t lcore, +service_update(uint32_t sid, uint32_t lcore, uint32_t *set, uint32_t *enabled) { - uint32_t i; - int32_t sid = -1; - - for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) { - if ((struct rte_service_spec *)&rte_services[i] == service && - service_valid(i)) { - sid = i; - break; - } - } - - if (sid == -1 || lcore >= RTE_MAX_LCORE) - return -EINVAL; - - if (!lcore_states[lcore].is_service_core) + /* validate ID, or return error value */ + if (sid >= RTE_SERVICE_NUM_MAX || !service_valid(sid) || + lcore >= RTE_MAX_LCORE || !lcore_states[lcore].is_service_core) return -EINVAL; uint64_t sid_mask = UINT64_C(1) << sid; @@ -587,19 +575,15 @@ service_update(struct rte_service_spec *service, uint32_t lcore, int32_t rte_service_map_lcore_set(uint32_t id, uint32_t lcore, uint32_t enabled) { - struct rte_service_spec_impl *s; - SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); uint32_t on = enabled > 0; - return service_update(&s->spec, lcore, &on, 0); + return service_update(id, lcore, &on, 0); } int32_t rte_service_map_lcore_get(uint32_t id, uint32_t lcore) { - struct rte_service_spec_impl *s; - SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); uint32_t enabled; - int ret = service_update(&s->spec, lcore, 0, &enabled); + int ret = service_update(id, lcore, 0, &enabled); if (ret == 0) return enabled; return ret; From patchwork Thu Apr 23 16:31:21 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Yang X-Patchwork-Id: 69203 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 63133A034E; Thu, 23 Apr 2020 18:32:53 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 43DDA1D146; Thu, 23 Apr 2020 18:32:22 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by dpdk.org (Postfix) with ESMTP id 640901D529 for ; Thu, 23 Apr 2020 18:32:20 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D9CE930E; Thu, 23 Apr 2020 09:32:19 -0700 (PDT) Received: from phil-VirtualBox.arm.com (A010647.Arm.com [10.170.243.60]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 4D7BB3F68F; Thu, 23 Apr 2020 09:32:17 -0700 (PDT) From: Phil Yang To: harry.van.haaren@intel.com, dev@dpdk.org Cc: thomas@monjalon.net, david.marchand@redhat.com, konstantin.ananyev@intel.com, jerinj@marvell.com, hemant.agrawal@nxp.com, Honnappa.Nagarahalli@arm.com, gavin.hu@arm.com, nd@arm.com Date: Fri, 24 Apr 2020 00:31:21 +0800 Message-Id: <1587659482-27133-6-git-send-email-phil.yang@arm.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1587659482-27133-1-git-send-email-phil.yang@arm.com> References: <1584407863-774-8-git-send-email-phil.yang@arm.com> <1587659482-27133-1-git-send-email-phil.yang@arm.com> Subject: [dpdk-dev] [PATCH v2 5/6] service: optimize with c11 atomics 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" The num_mapped_cores is used as a statistics. Use c11 atomics with RELAXED ordering for num_mapped_cores instead of rte_atomic ops which enforce unnessary barriers on aarch64. Replace execute_lock operations to spinlock_try_lock to avoid duplicate code. Signed-off-by: Phil Yang Reviewed-by: Honnappa Nagarahalli --- lib/librte_eal/common/rte_service.c | 32 ++++++++++++++++++-------------- lib/librte_eal/meson.build | 4 ++++ 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index 9c1a1d5..8cac265 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -20,6 +20,7 @@ #include #include #include +#include #include "eal_private.h" @@ -38,11 +39,11 @@ struct rte_service_spec_impl { /* public part of the struct */ struct rte_service_spec spec; - /* atomic lock that when set indicates a service core is currently + /* spin lock that when set indicates a service core is currently * running this service callback. When not set, a core may take the * lock and then run the service callback. */ - rte_atomic32_t execute_lock; + rte_spinlock_t execute_lock; /* API set/get-able variables */ int8_t app_runstate; @@ -54,7 +55,7 @@ struct rte_service_spec_impl { * It does not indicate the number of cores the service is running * on currently. */ - rte_atomic32_t num_mapped_cores; + uint32_t num_mapped_cores; uint64_t calls; uint64_t cycles_spent; } __rte_cache_aligned; @@ -332,7 +333,8 @@ rte_service_runstate_get(uint32_t id) rte_smp_rmb(); int check_disabled = !(s->internal_flags & SERVICE_F_START_CHECK); - int lcore_mapped = (rte_atomic32_read(&s->num_mapped_cores) > 0); + int lcore_mapped = (__atomic_load_n(&s->num_mapped_cores, + __ATOMIC_RELAXED) > 0); return (s->app_runstate == RUNSTATE_RUNNING) && (s->comp_runstate == RUNSTATE_RUNNING) && @@ -375,11 +377,11 @@ service_run(uint32_t i, struct core_state *cs, uint64_t service_mask, cs->service_active_on_lcore[i] = 1; if ((service_mt_safe(s) == 0) && (serialize_mt_unsafe == 1)) { - if (!rte_atomic32_cmpset((uint32_t *)&s->execute_lock, 0, 1)) + if (!rte_spinlock_trylock(&s->execute_lock)) return -EBUSY; service_runner_do_callback(s, cs, i); - rte_atomic32_clear(&s->execute_lock); + rte_spinlock_unlock(&s->execute_lock); } else service_runner_do_callback(s, cs, i); @@ -415,11 +417,11 @@ rte_service_run_iter_on_app_lcore(uint32_t id, uint32_t serialize_mt_unsafe) /* Increment num_mapped_cores to indicate that the service * is running on a core. */ - rte_atomic32_inc(&s->num_mapped_cores); + __atomic_add_fetch(&s->num_mapped_cores, 1, __ATOMIC_RELAXED); int ret = service_run(id, cs, UINT64_MAX, s, serialize_mt_unsafe); - rte_atomic32_dec(&s->num_mapped_cores); + __atomic_sub_fetch(&s->num_mapped_cores, 1, __ATOMIC_RELAXED); return ret; } @@ -556,19 +558,19 @@ service_update(uint32_t sid, uint32_t lcore, if (*set && !lcore_mapped) { lcore_states[lcore].service_mask |= sid_mask; - rte_atomic32_inc(&rte_services[sid].num_mapped_cores); + __atomic_add_fetch(&rte_services[sid].num_mapped_cores, + 1, __ATOMIC_RELAXED); } if (!*set && lcore_mapped) { lcore_states[lcore].service_mask &= ~(sid_mask); - rte_atomic32_dec(&rte_services[sid].num_mapped_cores); + __atomic_sub_fetch(&rte_services[sid].num_mapped_cores, + 1, __ATOMIC_RELAXED); } } if (enabled) *enabled = !!(lcore_states[lcore].service_mask & (sid_mask)); - rte_smp_wmb(); - return 0; } @@ -616,7 +618,8 @@ rte_service_lcore_reset_all(void) } } for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) - rte_atomic32_set(&rte_services[i].num_mapped_cores, 0); + __atomic_store_n(&rte_services[i].num_mapped_cores, 0, + __ATOMIC_RELAXED); rte_smp_wmb(); @@ -699,7 +702,8 @@ rte_service_lcore_stop(uint32_t lcore) int32_t enabled = service_mask & (UINT64_C(1) << i); int32_t service_running = rte_service_runstate_get(i); int32_t only_core = (1 == - rte_atomic32_read(&rte_services[i].num_mapped_cores)); + __atomic_load_n(&rte_services[i].num_mapped_cores, + __ATOMIC_RELAXED)); /* if the core is mapped, and the service is running, and this * is the only core that is mapped, the service would cease to diff --git a/lib/librte_eal/meson.build b/lib/librte_eal/meson.build index 0267c3b..c2d7a69 100644 --- a/lib/librte_eal/meson.build +++ b/lib/librte_eal/meson.build @@ -21,3 +21,7 @@ endif if cc.has_header('getopt.h') cflags += ['-DHAVE_GETOPT_H', '-DHAVE_GETOPT', '-DHAVE_GETOPT_LONG'] endif +# for clang 32-bit compiles we need libatomic for 64-bit atomic ops +if cc.get_id() == 'clang' and dpdk_conf.get('RTE_ARCH_64') == false + ext_deps += cc.find_library('atomic') +endif From patchwork Thu Apr 23 16:31:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Yang X-Patchwork-Id: 69204 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 BBDACA034E; Thu, 23 Apr 2020 18:33:03 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8D07B1D538; Thu, 23 Apr 2020 18:32:26 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by dpdk.org (Postfix) with ESMTP id 541B11D532 for ; Thu, 23 Apr 2020 18:32:25 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id DC5A930E; Thu, 23 Apr 2020 09:32:24 -0700 (PDT) Received: from phil-VirtualBox.arm.com (A010647.Arm.com [10.170.243.60]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 117573F68F; Thu, 23 Apr 2020 09:32:21 -0700 (PDT) From: Phil Yang To: harry.van.haaren@intel.com, dev@dpdk.org Cc: thomas@monjalon.net, david.marchand@redhat.com, konstantin.ananyev@intel.com, jerinj@marvell.com, hemant.agrawal@nxp.com, Honnappa.Nagarahalli@arm.com, gavin.hu@arm.com, nd@arm.com Date: Fri, 24 Apr 2020 00:31:22 +0800 Message-Id: <1587659482-27133-7-git-send-email-phil.yang@arm.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1587659482-27133-1-git-send-email-phil.yang@arm.com> References: <1584407863-774-8-git-send-email-phil.yang@arm.com> <1587659482-27133-1-git-send-email-phil.yang@arm.com> Subject: [dpdk-dev] [PATCH v2 6/6] service: relax barriers with C11 atomics 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" The runstate, comp_runstate and app_runstate are used as guard variables in the service core lib. To guarantee the inter-threads visibility of these guard variables, it uses rte_smp_r/wmb. This patch use c11 atomic built-ins to relax these barriers. Signed-off-by: Phil Yang Reviewed-by: Honnappa Nagarahalli --- lib/librte_eal/common/rte_service.c | 115 ++++++++++++++++++++++++++---------- 1 file changed, 84 insertions(+), 31 deletions(-) diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index 8cac265..dbb8211 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -265,7 +265,6 @@ rte_service_component_register(const struct rte_service_spec *spec, s->spec = *spec; s->internal_flags |= SERVICE_F_REGISTERED | SERVICE_F_START_CHECK; - rte_smp_wmb(); rte_service_count++; if (id_ptr) @@ -282,7 +281,6 @@ rte_service_component_unregister(uint32_t id) SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); rte_service_count--; - rte_smp_wmb(); s->internal_flags &= ~(SERVICE_F_REGISTERED); @@ -301,12 +299,17 @@ rte_service_component_runstate_set(uint32_t id, uint32_t runstate) struct rte_service_spec_impl *s; SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); + /* comp_runstate act as the guard variable. Use store-release + * memory order. This synchronizes with load-acquire in + * service_run and service_runstate_get function. + */ if (runstate) - s->comp_runstate = RUNSTATE_RUNNING; + __atomic_store_n(&s->comp_runstate, RUNSTATE_RUNNING, + __ATOMIC_RELEASE); else - s->comp_runstate = RUNSTATE_STOPPED; + __atomic_store_n(&s->comp_runstate, RUNSTATE_STOPPED, + __ATOMIC_RELEASE); - rte_smp_wmb(); return 0; } @@ -316,12 +319,17 @@ rte_service_runstate_set(uint32_t id, uint32_t runstate) struct rte_service_spec_impl *s; SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); + /* app_runstate act as the guard variable. Use store-release + * memory order. This synchronizes with load-acquire in + * service_run runstate_get function. + */ if (runstate) - s->app_runstate = RUNSTATE_RUNNING; + __atomic_store_n(&s->app_runstate, RUNSTATE_RUNNING, + __ATOMIC_RELEASE); else - s->app_runstate = RUNSTATE_STOPPED; + __atomic_store_n(&s->app_runstate, RUNSTATE_STOPPED, + __ATOMIC_RELEASE); - rte_smp_wmb(); return 0; } @@ -330,15 +338,24 @@ rte_service_runstate_get(uint32_t id) { struct rte_service_spec_impl *s; SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); - rte_smp_rmb(); - int check_disabled = !(s->internal_flags & SERVICE_F_START_CHECK); - int lcore_mapped = (__atomic_load_n(&s->num_mapped_cores, + /* comp_runstate and app_runstate act as the guard variables. + * Use load-acquire memory order. This synchronizes with + * store-release in service state set functions. + */ + if (__atomic_load_n(&s->comp_runstate, + __ATOMIC_ACQUIRE) == RUNSTATE_RUNNING && + __atomic_load_n(&s->app_runstate, + __ATOMIC_ACQUIRE) == RUNSTATE_RUNNING) { + int check_disabled = !(s->internal_flags & + SERVICE_F_START_CHECK); + int lcore_mapped = (__atomic_load_n(&s->num_mapped_cores, __ATOMIC_RELAXED) > 0); - return (s->app_runstate == RUNSTATE_RUNNING) && - (s->comp_runstate == RUNSTATE_RUNNING) && - (check_disabled | lcore_mapped); + return (check_disabled | lcore_mapped); + } else + return 0; + } static inline void @@ -367,9 +384,15 @@ service_run(uint32_t i, struct core_state *cs, uint64_t service_mask, if (!s) return -EINVAL; - if (s->comp_runstate != RUNSTATE_RUNNING || - s->app_runstate != RUNSTATE_RUNNING || - !(service_mask & (UINT64_C(1) << i))) { + /* comp_runstate and app_runstate act as the guard variables. + * Use load-acquire memory order. This synchronizes with + * store-release in service state set functions. + */ + if (__atomic_load_n(&s->comp_runstate, + __ATOMIC_ACQUIRE) != RUNSTATE_RUNNING || + __atomic_load_n(&s->app_runstate, + __ATOMIC_ACQUIRE) != RUNSTATE_RUNNING || + !(service_mask & (UINT64_C(1) << i))) { cs->service_active_on_lcore[i] = 0; return -ENOEXEC; } @@ -434,7 +457,12 @@ service_runner_func(void *arg) const int lcore = rte_lcore_id(); struct core_state *cs = &lcore_states[lcore]; - while (cs->runstate == RUNSTATE_RUNNING) { + /* runstate act as the guard variable. Use load-acquire + * memory order here to synchronize with store-release + * in runstate update functions. + */ + while (__atomic_load_n(&cs->runstate, + __ATOMIC_ACQUIRE) == RUNSTATE_RUNNING) { const uint64_t service_mask = cs->service_mask; for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) { @@ -445,8 +473,6 @@ service_runner_func(void *arg) } cs->loops++; - - rte_smp_rmb(); } lcore_config[lcore].state = WAIT; @@ -614,15 +640,18 @@ rte_service_lcore_reset_all(void) if (lcore_states[i].is_service_core) { lcore_states[i].service_mask = 0; set_lcore_state(i, ROLE_RTE); - lcore_states[i].runstate = RUNSTATE_STOPPED; + /* runstate act as guard variable Use + * store-release memory order here to synchronize + * with load-acquire in runstate read functions. + */ + __atomic_store_n(&lcore_states[i].runstate, + RUNSTATE_STOPPED, __ATOMIC_RELEASE); } } for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) __atomic_store_n(&rte_services[i].num_mapped_cores, 0, __ATOMIC_RELAXED); - rte_smp_wmb(); - return 0; } @@ -638,9 +667,11 @@ rte_service_lcore_add(uint32_t lcore) /* ensure that after adding a core the mask and state are defaults */ lcore_states[lcore].service_mask = 0; - lcore_states[lcore].runstate = RUNSTATE_STOPPED; - - rte_smp_wmb(); + /* Use store-release memory order here to synchronize with + * load-acquire in runstate read functions. + */ + __atomic_store_n(&lcore_states[lcore].runstate, RUNSTATE_STOPPED, + __ATOMIC_RELEASE); return rte_eal_wait_lcore(lcore); } @@ -655,7 +686,12 @@ rte_service_lcore_del(uint32_t lcore) if (!cs->is_service_core) return -EINVAL; - if (cs->runstate != RUNSTATE_STOPPED) + /* runstate act as the guard variable. Use load-acquire + * memory order here to synchronize with store-release + * in runstate update functions. + */ + if (__atomic_load_n(&cs->runstate, + __ATOMIC_ACQUIRE) != RUNSTATE_STOPPED) return -EBUSY; set_lcore_state(lcore, ROLE_RTE); @@ -674,13 +710,21 @@ rte_service_lcore_start(uint32_t lcore) if (!cs->is_service_core) return -EINVAL; - if (cs->runstate == RUNSTATE_RUNNING) + /* runstate act as the guard variable. Use load-acquire + * memory order here to synchronize with store-release + * in runstate update functions. + */ + if (__atomic_load_n(&cs->runstate, + __ATOMIC_ACQUIRE) == RUNSTATE_RUNNING) return -EALREADY; /* set core to run state first, and then launch otherwise it will * return immediately as runstate keeps it in the service poll loop */ - cs->runstate = RUNSTATE_RUNNING; + /* Use load-acquire memory order here to synchronize with + * store-release in runstate update functions. + */ + __atomic_store_n(&cs->runstate, RUNSTATE_RUNNING, __ATOMIC_RELEASE); int ret = rte_eal_remote_launch(service_runner_func, 0, lcore); /* returns -EBUSY if the core is already launched, 0 on success */ @@ -693,7 +737,12 @@ rte_service_lcore_stop(uint32_t lcore) if (lcore >= RTE_MAX_LCORE) return -EINVAL; - if (lcore_states[lcore].runstate == RUNSTATE_STOPPED) + /* runstate act as the guard variable. Use load-acquire + * memory order here to synchronize with store-release + * in runstate update functions. + */ + if (__atomic_load_n(&lcore_states[lcore].runstate, + __ATOMIC_ACQUIRE) == RUNSTATE_STOPPED) return -EALREADY; uint32_t i; @@ -713,7 +762,11 @@ rte_service_lcore_stop(uint32_t lcore) return -EBUSY; } - lcore_states[lcore].runstate = RUNSTATE_STOPPED; + /* Use store-release memory order here to synchronize with + * load-acquire in runstate read functions. + */ + __atomic_store_n(&lcore_states[lcore].runstate, RUNSTATE_STOPPED, + __ATOMIC_RELEASE); return 0; }