[PATCH] eal: replace lcore direct use of pthread in the EAL

[Tyler Retzlaff]

hey folks,

any feedback here? incremental progress untangling pthread out of eal.

thanks!

On Thu, Dec 08, 2022 at 01:48:16PM -0800, Tyler Retzlaff wrote:
> * Replace the use of pthread_t in struct lcore_config with the EAL
>   rte_thread_t type.
> 
> * Replace the direct use of pthread_create(), pthread_self()
>   pthread_getaffinity_np() and pthread_setaffinity_np().
> 
> Minor tweaks to return value comparisons to align with current DPDK
> style.
> 
> Signed-off-by: Tyler Retzlaff <roretzla at linux.microsoft.com>
> ---
>  lib/eal/common/eal_common_options.c |  8 ++++----
>  lib/eal/common/eal_common_thread.c  | 13 ++++++-------
>  lib/eal/common/eal_private.h        |  2 +-
>  lib/eal/common/eal_thread.h         |  2 +-
>  lib/eal/freebsd/eal.c               | 13 ++++++++-----
>  lib/eal/linux/eal.c                 | 24 +++++++++++++++++-------
>  lib/eal/windows/eal.c               | 10 ++++++----
>  lib/eal/windows/eal_thread.c        | 25 -------------------------
>  lib/eal/windows/eal_windows.h       | 12 ------------
>  9 files changed, 43 insertions(+), 66 deletions(-)
> 
> diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
> index 2d65357..86a519c 100644
> --- a/lib/eal/common/eal_common_options.c
> +++ b/lib/eal/common/eal_common_options.c
> @@ -1943,8 +1943,8 @@ static int xdigit2val(unsigned char c)
>  	unsigned int removed = 0;
>  	rte_cpuset_t affinity_set;
>  
> -	if (pthread_getaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
> -				&affinity_set))
> +	if (rte_thread_get_affinity_by_id(rte_thread_self(),
> +				&affinity_set) != 0)
>  		CPU_ZERO(&affinity_set);
>  
>  	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
> @@ -1972,8 +1972,8 @@ static int xdigit2val(unsigned char c)
>  	}
>  	RTE_CPU_NOT(cpuset, cpuset);
>  
> -	if (pthread_getaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
> -				&default_set))
> +	if (rte_thread_get_affinity_by_id(rte_thread_self(),
> +				&default_set) != 0)
>  		CPU_ZERO(&default_set);
>  
>  	RTE_CPU_AND(cpuset, cpuset, &default_set);
> diff --git a/lib/eal/common/eal_common_thread.c b/lib/eal/common/eal_common_thread.c
> index c5d8b43..dbe52a4 100644
> --- a/lib/eal/common/eal_common_thread.c
> +++ b/lib/eal/common/eal_common_thread.c
> @@ -85,9 +85,9 @@ unsigned rte_socket_id(void)
>  int
>  rte_thread_set_affinity(rte_cpuset_t *cpusetp)
>  {
> -	if (pthread_setaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
> +	if (rte_thread_set_affinity_by_id(rte_thread_self(),
>  			cpusetp) != 0) {
> -		RTE_LOG(ERR, EAL, "pthread_setaffinity_np failed\n");
> +		RTE_LOG(ERR, EAL, "rte_thread_set_affinity_by_id failed\n");
>  		return -1;
>  	}
>  
> @@ -166,7 +166,7 @@ unsigned rte_socket_id(void)
>  }
>  
>  /* main loop of threads */
> -__rte_noreturn void *
> +__rte_noreturn uint32_t
>  eal_thread_loop(void *arg)
>  {
>  	unsigned int lcore_id = (uintptr_t)arg;
> @@ -223,8 +223,7 @@ unsigned rte_socket_id(void)
>  	}
>  
>  	/* never reached */
> -	/* pthread_exit(NULL); */
> -	/* return NULL; */
> +	/* return 0; */
>  }
>  
>  enum __rte_ctrl_thread_status {
> @@ -253,7 +252,7 @@ static void *ctrl_thread_init(void *arg)
>  	void *routine_arg = params->arg;
>  
>  	__rte_thread_init(rte_lcore_id(), cpuset);
> -	params->ret = pthread_setaffinity_np(pthread_self(), sizeof(*cpuset),
> +	params->ret = rte_thread_set_affinity_by_id(rte_thread_self(),
>  		cpuset);
>  	if (params->ret != 0) {
>  		__atomic_store_n(&params->ctrl_thread_status,
> @@ -338,7 +337,7 @@ static void *ctrl_thread_init(void *arg)
>  		rte_errno = EINVAL;
>  		return -1;
>  	}
> -	if (pthread_getaffinity_np(pthread_self(), sizeof(cpuset),
> +	if (rte_thread_get_affinity_by_id(rte_thread_self(),
>  			&cpuset) != 0)
>  		CPU_ZERO(&cpuset);
>  	lcore_id = eal_lcore_non_eal_allocate();
> diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h
> index 0f4d75b..d7f8377 100644
> --- a/lib/eal/common/eal_private.h
> +++ b/lib/eal/common/eal_private.h
> @@ -20,7 +20,7 @@
>   * Structure storing internal configuration (per-lcore)
>   */
>  struct lcore_config {
> -	pthread_t thread_id;       /**< pthread identifier */
> +	rte_thread_t thread_id;    /**< thread identifier */
>  	int pipe_main2worker[2];   /**< communication pipe with main */
>  	int pipe_worker2main[2];   /**< communication pipe with main */
>  
> diff --git a/lib/eal/common/eal_thread.h b/lib/eal/common/eal_thread.h
> index 32bfe5c..1c3c344 100644
> --- a/lib/eal/common/eal_thread.h
> +++ b/lib/eal/common/eal_thread.h
> @@ -14,7 +14,7 @@
>   * @param arg
>   *   The lcore_id (passed as an integer) of this worker thread.
>   */
> -__rte_noreturn void *eal_thread_loop(void *arg);
> +__rte_noreturn uint32_t eal_thread_loop(void *arg);
>  
>  /**
>   * Get the NUMA socket id from cpu id.
> diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
> index 607684c..1deee2c 100644
> --- a/lib/eal/freebsd/eal.c
> +++ b/lib/eal/freebsd/eal.c
> @@ -780,7 +780,7 @@ static void rte_eal_init_alert(const char *msg)
>  
>  	eal_check_mem_on_local_socket();
>  
> -	if (pthread_setaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
> +	if (rte_thread_set_affinity_by_id(rte_thread_self(),
>  			&lcore_config[config->main_lcore].cpuset) != 0) {
>  		rte_eal_init_alert("Cannot set affinity");
>  		rte_errno = EINVAL;
> @@ -809,7 +809,7 @@ static void rte_eal_init_alert(const char *msg)
>  		lcore_config[i].state = WAIT;
>  
>  		/* create a thread for each lcore */
> -		ret = pthread_create(&lcore_config[i].thread_id, NULL,
> +		ret = rte_thread_create(&lcore_config[i].thread_id, NULL,
>  				     eal_thread_loop, (void *)(uintptr_t)i);
>  		if (ret != 0)
>  			rte_panic("Cannot create thread\n");
> @@ -817,10 +817,13 @@ static void rte_eal_init_alert(const char *msg)
>  		/* Set thread_name for aid in debugging. */
>  		snprintf(thread_name, sizeof(thread_name),
>  				"rte-worker-%d", i);
> -		rte_thread_setname(lcore_config[i].thread_id, thread_name);
> +		rte_thread_setname(
> +			(pthread_t)lcore_config[i].thread_id.opaque_id,
> +			thread_name);
>  
> -		ret = pthread_setaffinity_np(lcore_config[i].thread_id,
> -			sizeof(rte_cpuset_t), &lcore_config[i].cpuset);
> +		ret = rte_thread_set_affinity_by_id(
> +			lcore_config[i].thread_id,
> +			&lcore_config[i].cpuset);
>  		if (ret != 0)
>  			rte_panic("Cannot set affinity\n");
>  	}
> diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
> index 8c118d0..edef77d 100644
> --- a/lib/eal/linux/eal.c
> +++ b/lib/eal/linux/eal.c
> @@ -909,6 +909,13 @@ static void rte_eal_init_alert(const char *msg)
>  	return n > 2;
>  }
>  
> +static
> +__rte_noreturn void *
> +eal_worker_thread_loop(void *arg)
> +{
> +	eal_thread_loop(arg);
> +}
> +
>  static int
>  eal_worker_thread_create(unsigned int lcore_id)
>  {
> @@ -943,8 +950,9 @@ static void rte_eal_init_alert(const char *msg)
>  		}
>  	}
>  
> -	if (pthread_create(&lcore_config[lcore_id].thread_id, attrp,
> -			eal_thread_loop, (void *)(uintptr_t)lcore_id) == 0)
> +	if (pthread_create(
> +	    (pthread_t *)&lcore_config[lcore_id].thread_id.opaque_id,
> +		attrp, eal_worker_thread_loop, (void *)(uintptr_t)lcore_id) == 0)
>  		ret = 0;
>  
>  out:
> @@ -1220,7 +1228,7 @@ static void rte_eal_init_alert(const char *msg)
>  
>  	eal_check_mem_on_local_socket();
>  
> -	if (pthread_setaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
> +	if (rte_thread_set_affinity_by_id(rte_thread_self(),
>  			&lcore_config[config->main_lcore].cpuset) != 0) {
>  		rte_eal_init_alert("Cannot set affinity");
>  		rte_errno = EINVAL;
> @@ -1255,14 +1263,16 @@ static void rte_eal_init_alert(const char *msg)
>  		/* Set thread_name for aid in debugging. */
>  		snprintf(thread_name, sizeof(thread_name),
>  			"rte-worker-%d", i);
> -		ret = rte_thread_setname(lcore_config[i].thread_id,
> -						thread_name);
> +		ret = rte_thread_setname(
> +			(pthread_t)lcore_config[i].thread_id.opaque_id,
> +			thread_name);
>  		if (ret != 0)
>  			RTE_LOG(DEBUG, EAL,
>  				"Cannot set name for lcore thread\n");
>  
> -		ret = pthread_setaffinity_np(lcore_config[i].thread_id,
> -			sizeof(rte_cpuset_t), &lcore_config[i].cpuset);
> +		ret = rte_thread_set_affinity_by_id(
> +			lcore_config[i].thread_id,
> +			&lcore_config[i].cpuset);
>  		if (ret != 0)
>  			rte_panic("Cannot set affinity\n");
>  	}
> diff --git a/lib/eal/windows/eal.c b/lib/eal/windows/eal.c
> index adb929a..a95cbc5 100644
> --- a/lib/eal/windows/eal.c
> +++ b/lib/eal/windows/eal.c
> @@ -404,7 +404,7 @@ enum rte_proc_type_t
>  		return -1;
>  	}
>  
> -	if (pthread_setaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
> +	if (rte_thread_set_affinity_by_id(rte_thread_self(),
>  			&lcore_config[config->main_lcore].cpuset) != 0) {
>  		rte_eal_init_alert("Cannot set affinity");
>  		rte_errno = EINVAL;
> @@ -434,10 +434,12 @@ enum rte_proc_type_t
>  		lcore_config[i].state = WAIT;
>  
>  		/* create a thread for each lcore */
> -		if (eal_thread_create(&lcore_config[i].thread_id, i) != 0)
> +		if (rte_thread_create(&lcore_config[i].thread_id, NULL,
> +			eal_thread_loop, (void *)(uintptr_t)i) != 0)
>  			rte_panic("Cannot create thread\n");
> -		ret = pthread_setaffinity_np(lcore_config[i].thread_id,
> -			sizeof(rte_cpuset_t), &lcore_config[i].cpuset);
> +		ret = rte_thread_set_affinity_by_id(
> +			lcore_config[i].thread_id,
> +			&lcore_config[i].cpuset);
>  		if (ret != 0)
>  			RTE_LOG(DEBUG, EAL, "Cannot set affinity\n");
>  	}
> diff --git a/lib/eal/windows/eal_thread.c b/lib/eal/windows/eal_thread.c
> index caffe68..464d510 100644
> --- a/lib/eal/windows/eal_thread.c
> +++ b/lib/eal/windows/eal_thread.c
> @@ -70,31 +70,6 @@
>  		rte_panic("cannot write on configuration pipe\n");
>  }
>  
> -/* function to create threads */
> -int
> -eal_thread_create(pthread_t *thread, unsigned int lcore_id)
> -{
> -	HANDLE th;
> -
> -	th = CreateThread(NULL, 0,
> -		(LPTHREAD_START_ROUTINE)(ULONG_PTR)eal_thread_loop,
> -						(LPVOID)(uintptr_t)lcore_id,
> -						CREATE_SUSPENDED,
> -						(LPDWORD)thread);
> -	if (!th)
> -		return -1;
> -
> -	SetPriorityClass(GetCurrentProcess(), NORMAL_PRIORITY_CLASS);
> -	SetThreadPriority(th, THREAD_PRIORITY_NORMAL);
> -
> -	if (ResumeThread(th) == (DWORD)-1) {
> -		(void)CloseHandle(th);
> -		return -1;
> -	}
> -
> -	return 0;
> -}
> -
>  /* get current thread ID */
>  int
>  rte_sys_gettid(void)
> diff --git a/lib/eal/windows/eal_windows.h b/lib/eal/windows/eal_windows.h
> index ab25814..43b228d 100644
> --- a/lib/eal/windows/eal_windows.h
> +++ b/lib/eal/windows/eal_windows.h
> @@ -36,18 +36,6 @@
>  int eal_create_cpu_map(void);
>  
>  /**
> - * Create a thread.
> - *
> - * @param thread
> - *   The location to store the thread id if successful.
> - * @param lcore_id
> - *   The lcore_id of this worker thread.
> - * @return
> - *   0 for success, -1 if the thread is not created.
> - */
> -int eal_thread_create(pthread_t *thread, unsigned int lcore_id);
> -
> -/**
>   * Get system NUMA node number for a socket ID.
>   *
>   * @param socket_id
> -- 
> 1.8.3.1