Re: [dpdk-dev] [PATCH v2 1/2] service: add API to retrieve service core active

[Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>]

W dniu 20.07.2020 o 16:38, Harry van Haaren pisze:
> 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 <l.wojciechow@partner.samsung.com>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
>
> ---
>
> 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;
>   };
Acked-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>

-- 
Lukasz Wojciechowski
Principal Software Engineer

Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com