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

[Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>]

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>