[dpdk-dev,v4,1/2] service: fix lcore role after delete

Message ID 1515505061-12112-1-git-send-email-harry.van.haaren@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Van Haaren, Harry Jan. 9, 2018, 1:37 p.m. UTC
  This patch fixes the reset of the service core,
that when rte_service_lcore_del() is called, the
lcore_role is restored to RTE.

This issue was reported as when running the unit tests, an
error was thrown that "failed to allocate lcore". Investigating
revealed that the state of the service-cores after del() was
not allowing a core to be re-used at a later point in time.

Fixes: 21698354c832 ("service: introduce service cores concept")
+CC stable@dpdk.org

Reported-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

v4:
- Improve commit title (Bruce)

v3:
- Fix whitespace issue introduced in v2 (Doh :)

v2:
- Only update state on service core ids (Pavan)

@Stable maintainers; this is an EXPERIMENTAL tagged API, so I'm
not sure what the expectation is in terms of backporting.
---
 lib/librte_eal/common/rte_service.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)
  

Comments

Pavan Nikhilesh Jan. 10, 2018, 10:23 a.m. UTC | #1
On Tue, Jan 09, 2018 at 01:37:40PM +0000, Harry van Haaren wrote:
> This patch fixes the reset of the service core,
> that when rte_service_lcore_del() is called, the
> lcore_role is restored to RTE.
>
> This issue was reported as when running the unit tests, an
> error was thrown that "failed to allocate lcore". Investigating
> revealed that the state of the service-cores after del() was
> not allowing a core to be re-used at a later point in time.
>
> Fixes: 21698354c832 ("service: introduce service cores concept")
> +CC stable@dpdk.org
>
> Reported-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
>
> ---
>
> v4:
> - Improve commit title (Bruce)
>
> v3:
> - Fix whitespace issue introduced in v2 (Doh :)
>
> v2:
> - Only update state on service core ids (Pavan)
>
> @Stable maintainers; this is an EXPERIMENTAL tagged API, so I'm
> not sure what the expectation is in terms of backporting.
> ---
>  lib/librte_eal/common/rte_service.c | 36 +++++++++++++++++++-----------------
>  1 file changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c
> index 372d0bb..44a988a 100644
> --- a/lib/librte_eal/common/rte_service.c
> +++ b/lib/librte_eal/common/rte_service.c
> @@ -554,23 +554,6 @@ rte_service_map_lcore_get(uint32_t id, uint32_t lcore)
>  	return ret;
>  }
>
> -int32_t rte_service_lcore_reset_all(void)
> -{
> -	/* loop over cores, reset all to mask 0 */
> -	uint32_t i;
> -	for (i = 0; i < RTE_MAX_LCORE; i++) {
> -		lcore_states[i].service_mask = 0;
> -		lcore_states[i].is_service_core = 0;
> -		lcore_states[i].runstate = RUNSTATE_STOPPED;
> -	}
> -	for (i = 0; i < RTE_SERVICE_NUM_MAX; i++)
> -		rte_atomic32_set(&rte_services[i].num_mapped_cores, 0);
> -
> -	rte_smp_wmb();
> -
> -	return 0;
> -}
> -
>  static void
>  set_lcore_state(uint32_t lcore, int32_t state)
>  {
> @@ -585,6 +568,25 @@ set_lcore_state(uint32_t lcore, int32_t state)
>  	lcore_states[lcore].is_service_core = (state == ROLE_SERVICE);
>  }
>
> +int32_t rte_service_lcore_reset_all(void)
> +{
> +	/* loop over cores, reset all to mask 0 */
> +	uint32_t i;
> +	for (i = 0; i < RTE_MAX_LCORE; i++) {
> +		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;
> +		}
> +	}
> +	for (i = 0; i < RTE_SERVICE_NUM_MAX; i++)
> +		rte_atomic32_set(&rte_services[i].num_mapped_cores, 0);
> +
> +	rte_smp_wmb();
> +
> +	return 0;
> +}
> +
>  int32_t
>  rte_service_lcore_add(uint32_t lcore)
>  {
> --
> 2.7.4
>

LGTM

Series-Acked-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
  
Thomas Monjalon Jan. 11, 2018, 10:30 p.m. UTC | #2
10/01/2018 11:23, Pavan Nikhilesh:
> On Tue, Jan 09, 2018 at 01:37:40PM +0000, Harry van Haaren wrote:
> > This patch fixes the reset of the service core,
> > that when rte_service_lcore_del() is called, the
> > lcore_role is restored to RTE.
> >
> > This issue was reported as when running the unit tests, an
> > error was thrown that "failed to allocate lcore". Investigating
> > revealed that the state of the service-cores after del() was
> > not allowing a core to be re-used at a later point in time.
> >
> > Fixes: 21698354c832 ("service: introduce service cores concept")
> > +CC stable@dpdk.org

The canonical format is:
Cc: stable@dpdk.org

> > Reported-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> 
> Series-Acked-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>

Applied, thanks
  

Patch

diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c
index 372d0bb..44a988a 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -554,23 +554,6 @@  rte_service_map_lcore_get(uint32_t id, uint32_t lcore)
 	return ret;
 }
 
-int32_t rte_service_lcore_reset_all(void)
-{
-	/* loop over cores, reset all to mask 0 */
-	uint32_t i;
-	for (i = 0; i < RTE_MAX_LCORE; i++) {
-		lcore_states[i].service_mask = 0;
-		lcore_states[i].is_service_core = 0;
-		lcore_states[i].runstate = RUNSTATE_STOPPED;
-	}
-	for (i = 0; i < RTE_SERVICE_NUM_MAX; i++)
-		rte_atomic32_set(&rte_services[i].num_mapped_cores, 0);
-
-	rte_smp_wmb();
-
-	return 0;
-}
-
 static void
 set_lcore_state(uint32_t lcore, int32_t state)
 {
@@ -585,6 +568,25 @@  set_lcore_state(uint32_t lcore, int32_t state)
 	lcore_states[lcore].is_service_core = (state == ROLE_SERVICE);
 }
 
+int32_t rte_service_lcore_reset_all(void)
+{
+	/* loop over cores, reset all to mask 0 */
+	uint32_t i;
+	for (i = 0; i < RTE_MAX_LCORE; i++) {
+		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;
+		}
+	}
+	for (i = 0; i < RTE_SERVICE_NUM_MAX; i++)
+		rte_atomic32_set(&rte_services[i].num_mapped_cores, 0);
+
+	rte_smp_wmb();
+
+	return 0;
+}
+
 int32_t
 rte_service_lcore_add(uint32_t lcore)
 {