Hi David,
one minor comment below
On Fri, Jun 26, 2020 at 04:47:30PM +0200, David Marchand wrote:
> Introduce a helper responsible for initialising the per thread context.
> We can then have a unified context for EAL and non-EAL threads and
> remove copy/paste'd OS-specific helpers.
>
> Per EAL thread CPU affinity setting is separated from the thread init.
> It is to accommodate with Windows EAL where CPU affinity is not set at
> the moment.
> Besides, having affinity set by the master lcore in FreeBSD and Linux
> will make it possible to detect errors rather than panic in the child
> thread. But the cleanup when such an event happens is left for later.
>
> Signed-off-by: David Marchand <david.march...@redhat.com>
> ---
> Changes since v1:
> - rebased on master, removed Windows workarounds wrt gettid and traces
> support,
>
> ---
> lib/librte_eal/common/eal_common_thread.c | 51 +++++++++++++----------
> lib/librte_eal/common/eal_thread.h | 8 ++--
> lib/librte_eal/freebsd/eal.c | 14 ++++++-
> lib/librte_eal/freebsd/eal_thread.c | 32 +-------------
> lib/librte_eal/linux/eal.c | 15 ++++++-
> lib/librte_eal/linux/eal_thread.c | 32 +-------------
> lib/librte_eal/windows/eal.c | 3 +-
> lib/librte_eal/windows/eal_thread.c | 10 +----
> 8 files changed, 66 insertions(+), 99 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_thread.c
> b/lib/librte_eal/common/eal_common_thread.c
> index 280c64bb76..afb30236c5 100644
> --- a/lib/librte_eal/common/eal_common_thread.c
> +++ b/lib/librte_eal/common/eal_common_thread.c
> @@ -71,20 +71,10 @@ eal_cpuset_socket_id(rte_cpuset_t *cpusetp)
> return socket_id;
> }
>
> -int
> -rte_thread_set_affinity(rte_cpuset_t *cpusetp)
> +static void
> +thread_update_affinity(rte_cpuset_t *cpusetp)
> {
> - int s;
> - unsigned lcore_id;
> - pthread_t tid;
> -
> - tid = pthread_self();
> -
> - s = pthread_setaffinity_np(tid, sizeof(rte_cpuset_t), cpusetp);
> - if (s != 0) {
> - RTE_LOG(ERR, EAL, "pthread_setaffinity_np failed\n");
> - return -1;
> - }
> + unsigned int lcore_id = rte_lcore_id();
>
> /* store socket_id in TLS for quick access */
> RTE_PER_LCORE(_socket_id) =
> @@ -94,14 +84,24 @@ rte_thread_set_affinity(rte_cpuset_t *cpusetp)
> memmove(&RTE_PER_LCORE(_cpuset), cpusetp,
> sizeof(rte_cpuset_t));
>
> - lcore_id = rte_lcore_id();
> if (lcore_id != (unsigned)LCORE_ID_ANY) {
> /* EAL thread will update lcore_config */
> lcore_config[lcore_id].socket_id = RTE_PER_LCORE(_socket_id);
> memmove(&lcore_config[lcore_id].cpuset, cpusetp,
> sizeof(rte_cpuset_t));
> }
> +}
>
> +int
> +rte_thread_set_affinity(rte_cpuset_t *cpusetp)
> +{
> + if (pthread_setaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
> + cpusetp) != 0) {
> + RTE_LOG(ERR, EAL, "pthread_setaffinity_np failed\n");
> + return -1;
> + }
> +
> + thread_update_affinity(cpusetp);
> return 0;
> }
>
> @@ -147,6 +147,19 @@ eal_thread_dump_affinity(char *str, unsigned size)
> return ret;
> }
>
> +void
> +rte_thread_init(unsigned int lcore_id, rte_cpuset_t *cpuset)
> +{
> + /* set the lcore ID in per-lcore memory area */
> + RTE_PER_LCORE(_lcore_id) = lcore_id;
> +
> + /* acquire system unique id */
> + rte_gettid();
If I understand properly, rte_gettid() is now also called for control
thread. I don't think this behavior change can break anything, but it may
be good to highlight it in the commit log.
also, there are 2 spaces before "*/"
