[dpdk-dev] [RFC v2] eal: simplify the implementation of rte_ctrl_thread_create
Mattias Rönnblom
hofors at lysator.liu.se
Wed Aug 25 17:12:32 CEST 2021
On 2021-08-02 07:16, Honnappa Nagarahalli wrote:
> The current described behaviour of rte_ctrl_thread_create is
> rigid which makes the implementation of the function complex.
> The behavior is abstracted to allow for simplified implementation.
>
Have you considered using a POSIX condition variable instead of atomics
for synchronization?
> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli at arm.com>
> ---
> v2: Used compiler's C++11 atomic built-ins to access the shared variable.
>
> lib/eal/common/eal_common_thread.c | 65 +++++++++++++-----------------
> lib/eal/include/rte_lcore.h | 8 ++--
> 2 files changed, 32 insertions(+), 41 deletions(-)
>
> diff --git a/lib/eal/common/eal_common_thread.c b/lib/eal/common/eal_common_thread.c
> index 1a52f42a2b..e3e0bf4bff 100644
> --- a/lib/eal/common/eal_common_thread.c
> +++ b/lib/eal/common/eal_common_thread.c
> @@ -169,35 +169,35 @@ __rte_thread_uninit(void)
> struct rte_thread_ctrl_params {
> void *(*start_routine)(void *);
> void *arg;
> - pthread_barrier_t configured;
> - unsigned int refcnt;
> + int ret;
> + /* Synchronization variable between the control thread
> + * and the thread calling rte_ctrl_thread_create function.
> + * 0 - Initialized
> + * 1 - Control thread is running successfully
> + * 2 - Control thread encountered an error. 'ret' has the
> + * error code.
> + */
> + unsigned int sync;
> };
>
> -static void ctrl_params_free(struct rte_thread_ctrl_params *params)
> -{
> - if (__atomic_sub_fetch(¶ms->refcnt, 1, __ATOMIC_ACQ_REL) == 0) {
> - (void)pthread_barrier_destroy(¶ms->configured);
> - free(params);
> - }
> -}
> -
> static void *ctrl_thread_init(void *arg)
> {
> struct internal_config *internal_conf =
> eal_get_internal_configuration();
> rte_cpuset_t *cpuset = &internal_conf->ctrl_cpuset;
> struct rte_thread_ctrl_params *params = arg;
> - void *(*start_routine)(void *);
> + void *(*start_routine)(void *) = params->start_routine;
> void *routine_arg = params->arg;
>
> __rte_thread_init(rte_lcore_id(), cpuset);
> -
> - pthread_barrier_wait(¶ms->configured);
> - start_routine = params->start_routine;
> - ctrl_params_free(params);
> -
> - if (start_routine == NULL)
> + params->ret = pthread_setaffinity_np(pthread_self(),
> + sizeof(*cpuset), cpuset);
> + if (params->ret != 0) {
> + __atomic_store_n(¶ms->sync, 2, __ATOMIC_RELEASE);
> return NULL;
> + }
> +
> + __atomic_store_n(¶ms->sync, 1, __ATOMIC_RELEASE);
>
> return start_routine(routine_arg);
> }
> @@ -207,9 +207,6 @@ rte_ctrl_thread_create(pthread_t *thread, const char *name,
> const pthread_attr_t *attr,
> void *(*start_routine)(void *), void *arg)
> {
> - struct internal_config *internal_conf =
> - eal_get_internal_configuration();
> - rte_cpuset_t *cpuset = &internal_conf->ctrl_cpuset;
> struct rte_thread_ctrl_params *params;
> int ret;
>
> @@ -219,15 +216,12 @@ rte_ctrl_thread_create(pthread_t *thread, const char *name,
>
> params->start_routine = start_routine;
> params->arg = arg;
> - params->refcnt = 2;
> -
> - ret = pthread_barrier_init(¶ms->configured, NULL, 2);
> - if (ret != 0)
> - goto fail_no_barrier;
> + params->ret = 0;
> + params->sync = 0;
>
> ret = pthread_create(thread, attr, ctrl_thread_init, (void *)params);
> if (ret != 0)
> - goto fail_with_barrier;
> + goto thread_create_failed;
>
> if (name != NULL) {
> ret = rte_thread_setname(*thread, name);
> @@ -236,24 +230,21 @@ rte_ctrl_thread_create(pthread_t *thread, const char *name,
> "Cannot set name for ctrl thread\n");
> }
>
> - ret = pthread_setaffinity_np(*thread, sizeof(*cpuset), cpuset);
> - if (ret != 0)
> - params->start_routine = NULL;
> + /* Wait for the control thread to initialize successfully */
> + while (!__atomic_load_n(¶ms->sync, __ATOMIC_ACQUIRE))
> + rte_pause();
> + ret = params->ret;
>
> - pthread_barrier_wait(¶ms->configured);
> - ctrl_params_free(params);
> + free(params);
>
> - if (ret != 0)
> - /* start_routine has been set to NULL above; */
> - /* ctrl thread will exit immediately */
> + if (__atomic_load_n(¶ms->sync, __ATOMIC_ACQUIRE) != 1)
> + /* ctrl thread is exiting */
> pthread_join(*thread, NULL);
>
> return -ret;
>
> -fail_with_barrier:
> - (void)pthread_barrier_destroy(¶ms->configured);
> +thread_create_failed:
>
> -fail_no_barrier:
> free(params);
>
> return -ret;
> diff --git a/lib/eal/include/rte_lcore.h b/lib/eal/include/rte_lcore.h
> index 1550b75da0..f1cc5e38dc 100644
> --- a/lib/eal/include/rte_lcore.h
> +++ b/lib/eal/include/rte_lcore.h
> @@ -420,10 +420,10 @@ rte_thread_unregister(void);
> /**
> * Create a control thread.
> *
> - * Wrapper to pthread_create(), pthread_setname_np() and
> - * pthread_setaffinity_np(). The affinity of the new thread is based
> - * on the CPU affinity retrieved at the time rte_eal_init() was called,
> - * the dataplane and service lcores are then excluded.
> + * Creates a control thread with the given name and attributes. The
> + * affinity of the new thread is based on the CPU affinity retrieved
> + * at the time rte_eal_init() was called, the dataplane and service
> + * lcores are then excluded.
> *
> * @param thread
> * Filled with the thread id of the new created thread.
>
More information about the dev
mailing list