From patchwork Sat May 2 00:02:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Honnappa Nagarahalli X-Patchwork-Id: 69658 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 51453A04B2; Sat, 2 May 2020 02:03:12 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B13F81D9A2; Sat, 2 May 2020 02:03:05 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by dpdk.org (Postfix) with ESMTP id A67ED1D999; Sat, 2 May 2020 02:03:03 +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 DAF641063; Fri, 1 May 2020 17:03:02 -0700 (PDT) Received: from qc2400f-1.austin.arm.com (qc2400f-1.austin.arm.com [10.118.14.48]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C67C73F305; Fri, 1 May 2020 17:03:02 -0700 (PDT) From: Honnappa Nagarahalli To: dev@dpdk.org, phil.yang@arm.com, harry.van.haaren@intel.com Cc: thomas@monjalon.net, david.marchand@redhat.com, konstantin.ananyev@intel.com, jerinj@marvell.com, hemant.agrawal@nxp.com, gage.eads@intel.com, bruce.richardson@intel.com, honnappa.nagarahalli@arm.com, nd@arm.com, stable@dpdk.org Date: Fri, 1 May 2020 19:02:40 -0500 Message-Id: <20200502000245.11071-2-honnappa.nagarahalli@arm.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200502000245.11071-1-honnappa.nagarahalli@arm.com> References: <1587659482-27133-1-git-send-email-phil.yang@arm.com> <20200502000245.11071-1-honnappa.nagarahalli@arm.com> Subject: [dpdk-dev] [PATCH v3 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" 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. If the service is known to be mmapped on a single lcore, setting the service capability to MT safe will avoid taking the lock and improve the performance. Fixes: e9139a32f6e8 ("service: add function to run on app lcore") Cc: stable@dpdk.org Signed-off-by: Honnappa Nagarahalli Reviewed-by: Phil Yang Acked-by: Harry van Haaren --- lib/librte_eal/common/rte_service.c | 11 +++++------ lib/librte_eal/include/rte_service.h | 8 ++++++-- lib/librte_eal/include/rte_service_component.h | 6 +++++- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index 70d17a5d7..b8c465eb9 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; diff --git a/lib/librte_eal/include/rte_service.h b/lib/librte_eal/include/rte_service.h index d8701dd4c..3a1c735c5 100644 --- a/lib/librte_eal/include/rte_service.h +++ b/lib/librte_eal/include/rte_service.h @@ -104,12 +104,16 @@ int32_t rte_service_probe_capability(uint32_t id, uint32_t capability); * Each core can be added or removed from running a specific service. This * function enables or disables *lcore* to run *service_id*. * - * If multiple cores are enabled on a service, an atomic is used to ensure that - * only one cores runs the service at a time. The exception to this is when + * If multiple cores are enabled on a service, a lock is used to ensure that + * only one core runs the service at a time. The exception to this is when * a service indicates that it is multi-thread safe by setting the capability * called RTE_SERVICE_CAP_MT_SAFE. With the multi-thread safe capability set, * the service function can be run on multiple threads at the same time. * + * If the service is known to be mapped to a single lcore, setting the + * capability of the service to RTE_SERVICE_CAP_MT_SAFE can achieve + * better performance by avoiding the use of lock. + * * @param service_id the service to apply the lcore to * @param lcore The lcore that will be mapped to service * @param enable Zero to unmap or disable the core, non-zero to enable diff --git a/lib/librte_eal/include/rte_service_component.h b/lib/librte_eal/include/rte_service_component.h index 16eab79ee..b75aba11b 100644 --- a/lib/librte_eal/include/rte_service_component.h +++ b/lib/librte_eal/include/rte_service_component.h @@ -43,7 +43,7 @@ struct rte_service_spec { /** * Register a new service. * - * A service represents a component that the requires CPU time periodically to + * A service represents a component that requires CPU time periodically to * achieve its purpose. * * For example the eventdev SW PMD requires CPU cycles to perform its @@ -56,6 +56,10 @@ struct rte_service_spec { * *rte_service_component_runstate_set*, which indicates that the service * component is ready to be executed. * + * If the service is known to be mapped to a single lcore, setting the + * capability of the service to RTE_SERVICE_CAP_MT_SAFE can achieve + * better performance. + * * @param spec The specification of the service to register * @param[out] service_id A pointer to a uint32_t, which will be filled in * during registration of the service. It is set to the integers From patchwork Sat May 2 00:02:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Honnappa Nagarahalli X-Patchwork-Id: 69659 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 76822A04B2; Sat, 2 May 2020 02:03:22 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 160D51D9C6; Sat, 2 May 2020 02:03:07 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by dpdk.org (Postfix) with ESMTP id 22C601D99E; Sat, 2 May 2020 02:03:04 +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 A04E7106F; Fri, 1 May 2020 17:03:03 -0700 (PDT) Received: from qc2400f-1.austin.arm.com (qc2400f-1.austin.arm.com [10.118.14.48]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 8D3203F305; Fri, 1 May 2020 17:03:03 -0700 (PDT) From: Honnappa Nagarahalli To: dev@dpdk.org, phil.yang@arm.com, harry.van.haaren@intel.com Cc: thomas@monjalon.net, david.marchand@redhat.com, konstantin.ananyev@intel.com, jerinj@marvell.com, hemant.agrawal@nxp.com, gage.eads@intel.com, bruce.richardson@intel.com, honnappa.nagarahalli@arm.com, nd@arm.com, stable@dpdk.org Date: Fri, 1 May 2020 19:02:41 -0500 Message-Id: <20200502000245.11071-3-honnappa.nagarahalli@arm.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200502000245.11071-1-honnappa.nagarahalli@arm.com> References: <1587659482-27133-1-git-send-email-phil.yang@arm.com> <20200502000245.11071-1-honnappa.nagarahalli@arm.com> Subject: [dpdk-dev] [PATCH v3 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" 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 Acked-by: Harry van Haaren --- 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 b8c465eb9..c89472b83 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 Sat May 2 00:02:42 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Honnappa Nagarahalli X-Patchwork-Id: 69660 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 EFD67A04B2; Sat, 2 May 2020 02:03:31 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 646001D9E7; Sat, 2 May 2020 02:03:08 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by dpdk.org (Postfix) with ESMTP id C04F51D99E for ; Sat, 2 May 2020 02:03:04 +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 52154113E; Fri, 1 May 2020 17:03:04 -0700 (PDT) Received: from qc2400f-1.austin.arm.com (qc2400f-1.austin.arm.com [10.118.14.48]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3ECE13F305; Fri, 1 May 2020 17:03:04 -0700 (PDT) From: Honnappa Nagarahalli To: dev@dpdk.org, phil.yang@arm.com, harry.van.haaren@intel.com Cc: thomas@monjalon.net, david.marchand@redhat.com, konstantin.ananyev@intel.com, jerinj@marvell.com, hemant.agrawal@nxp.com, gage.eads@intel.com, bruce.richardson@intel.com, honnappa.nagarahalli@arm.com, nd@arm.com Date: Fri, 1 May 2020 19:02:42 -0500 Message-Id: <20200502000245.11071-4-honnappa.nagarahalli@arm.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200502000245.11071-1-honnappa.nagarahalli@arm.com> References: <1587659482-27133-1-git-send-email-phil.yang@arm.com> <20200502000245.11071-1-honnappa.nagarahalli@arm.com> Subject: [dpdk-dev] [PATCH v3 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" From: Phil Yang clean up rte prefix from static functions. remove unused parameter for service_dump_one function. Signed-off-by: Phil Yang Reviewed-by: Honnappa Nagarahalli Acked-by: Harry van Haaren --- 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 c89472b83..ed2070267 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 Sat May 2 00:02:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Honnappa Nagarahalli X-Patchwork-Id: 69661 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 D9F0AA04B2; Sat, 2 May 2020 02:03:39 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 55DC91D9F9; Sat, 2 May 2020 02:03:09 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by dpdk.org (Postfix) with ESMTP id 4C8E61D99E for ; Sat, 2 May 2020 02:03: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 D4AB711B3; Fri, 1 May 2020 17:03:04 -0700 (PDT) Received: from qc2400f-1.austin.arm.com (qc2400f-1.austin.arm.com [10.118.14.48]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id CB4803F305; Fri, 1 May 2020 17:03:04 -0700 (PDT) From: Honnappa Nagarahalli To: dev@dpdk.org, phil.yang@arm.com, harry.van.haaren@intel.com Cc: thomas@monjalon.net, david.marchand@redhat.com, konstantin.ananyev@intel.com, jerinj@marvell.com, hemant.agrawal@nxp.com, gage.eads@intel.com, bruce.richardson@intel.com, honnappa.nagarahalli@arm.com, nd@arm.com Date: Fri, 1 May 2020 19:02:43 -0500 Message-Id: <20200502000245.11071-5-honnappa.nagarahalli@arm.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200502000245.11071-1-honnappa.nagarahalli@arm.com> References: <1587659482-27133-1-git-send-email-phil.yang@arm.com> <20200502000245.11071-1-honnappa.nagarahalli@arm.com> Subject: [dpdk-dev] [PATCH v3 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" From: Phil Yang The service id validation is duplicated, remove the redundant code in the calling functions. Signed-off-by: Phil Yang Reviewed-by: Honnappa Nagarahalli Acked-by: Harry van Haaren --- 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 ed2070267..9c1a1d5cd 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 Sat May 2 00:02:44 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Honnappa Nagarahalli X-Patchwork-Id: 69662 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 0EDCFA04B2; Sat, 2 May 2020 02:03:48 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 3ECAB1DA08; Sat, 2 May 2020 02:03:10 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by dpdk.org (Postfix) with ESMTP id 095D31D9AD for ; Sat, 2 May 2020 02:03:06 +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 87E7B1045; Fri, 1 May 2020 17:03:05 -0700 (PDT) Received: from qc2400f-1.austin.arm.com (qc2400f-1.austin.arm.com [10.118.14.48]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 73C653F305; Fri, 1 May 2020 17:03:05 -0700 (PDT) From: Honnappa Nagarahalli To: dev@dpdk.org, phil.yang@arm.com, harry.van.haaren@intel.com Cc: thomas@monjalon.net, david.marchand@redhat.com, konstantin.ananyev@intel.com, jerinj@marvell.com, hemant.agrawal@nxp.com, gage.eads@intel.com, bruce.richardson@intel.com, honnappa.nagarahalli@arm.com, nd@arm.com Date: Fri, 1 May 2020 19:02:44 -0500 Message-Id: <20200502000245.11071-6-honnappa.nagarahalli@arm.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200502000245.11071-1-honnappa.nagarahalli@arm.com> References: <1587659482-27133-1-git-send-email-phil.yang@arm.com> <20200502000245.11071-1-honnappa.nagarahalli@arm.com> Subject: [dpdk-dev] [PATCH v3 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" From: Phil Yang 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 Acked-by: Harry van Haaren --- 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 9c1a1d5cd..8cac265c9 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 0267c3b9d..c2d7a6954 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 Sat May 2 00:02:45 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Honnappa Nagarahalli X-Patchwork-Id: 69663 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 C4C49A04B2; Sat, 2 May 2020 02:03:56 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 755461DA1D; Sat, 2 May 2020 02:03:11 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by dpdk.org (Postfix) with ESMTP id CC17B1D9B2 for ; Sat, 2 May 2020 02:03:06 +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 565B9106F; Fri, 1 May 2020 17:03:06 -0700 (PDT) Received: from qc2400f-1.austin.arm.com (qc2400f-1.austin.arm.com [10.118.14.48]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 4A22A3F305; Fri, 1 May 2020 17:03:06 -0700 (PDT) From: Honnappa Nagarahalli To: dev@dpdk.org, phil.yang@arm.com, harry.van.haaren@intel.com Cc: thomas@monjalon.net, david.marchand@redhat.com, konstantin.ananyev@intel.com, jerinj@marvell.com, hemant.agrawal@nxp.com, gage.eads@intel.com, bruce.richardson@intel.com, honnappa.nagarahalli@arm.com, nd@arm.com Date: Fri, 1 May 2020 19:02:45 -0500 Message-Id: <20200502000245.11071-7-honnappa.nagarahalli@arm.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200502000245.11071-1-honnappa.nagarahalli@arm.com> References: <1587659482-27133-1-git-send-email-phil.yang@arm.com> <20200502000245.11071-1-honnappa.nagarahalli@arm.com> Subject: [dpdk-dev] [PATCH v3 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" From: Phil Yang 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 Acked-by: Harry van Haaren --- 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 8cac265c9..dbb821139 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; }