Re: [PATCH 1/3] eal: add rte control thread create API

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon,  5 Dec 2022 12:24:26 -0800
Tyler Retzlaff <roretzla@linux.microsoft.com> wrote:

> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
>  lib/eal/common/eal_common_thread.c | 93 ++++++++++++++++++++++++++++++++++----
>  lib/eal/include/rte_thread.h       | 29 ++++++++++++
>  lib/eal/version.map                |  3 ++
>  3 files changed, 117 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/eal/common/eal_common_thread.c b/lib/eal/common/eal_common_thread.c
> index c5d8b43..ca85c51 100644
> --- a/lib/eal/common/eal_common_thread.c
> +++ b/lib/eal/common/eal_common_thread.c
> @@ -234,7 +234,10 @@ enum __rte_ctrl_thread_status {
>  };
>  
>  struct rte_thread_ctrl_params {
> -	void *(*start_routine)(void *);
> +	union {
> +		void * (*start_routine)(void *);
> +		rte_thread_func thread_func;
> +	} u;

Why not just use rte_thread_func, this in internal.

>  	void *arg;
>  	int ret;
>  	/* Control thread status.
> @@ -243,14 +246,12 @@ struct rte_thread_ctrl_params {
>  	enum __rte_ctrl_thread_status ctrl_thread_status;
>  };
>  
> -static void *ctrl_thread_init(void *arg)
> +static int 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 *) = params->start_routine;
> -	void *routine_arg = params->arg;
>  
>  	__rte_thread_init(rte_lcore_id(), cpuset);
>  	params->ret = pthread_setaffinity_np(pthread_self(), sizeof(*cpuset),
> @@ -258,13 +259,35 @@ static void *ctrl_thread_init(void *arg)
>  	if (params->ret != 0) {
>  		__atomic_store_n(&params->ctrl_thread_status,
>  			CTRL_THREAD_ERROR, __ATOMIC_RELEASE);
> -		return NULL;
> +		return params->ret;
>  	}
>  
>  	__atomic_store_n(&params->ctrl_thread_status,
>  		CTRL_THREAD_RUNNING, __ATOMIC_RELEASE);
>  
> -	return start_routine(routine_arg);
> +	return 0;
> +}
> +
> +static void *ctrl_thread_start(void *arg)
> +{
> +	struct rte_thread_ctrl_params *params = arg;
> +	void *(*start_routine)(void *) = params->u.start_routine;
> +
> +	if (0 != ctrl_thread_init(arg))
> +		return NULL;

DPDK uses the Linux not Windows coding style.
Windows uses the constant on left side of comparison because of the common
programming error of putting assignment where conditional was intended.
Linux uses a compiler that puts out a warning, so the more natural
style is action != result