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.marchand@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 "*/"
@@ -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();
+
+ thread_update_affinity(cpuset);
+
+ __rte_trace_mem_per_thread_alloc();
+}
struct rte_thread_ctrl_params {
void *(*start_routine)(void *);
@@ -154,16 +167,14 @@ struct rte_thread_ctrl_params {
pthread_barrier_t configured;
};
-static void *rte_thread_init(void *arg)
+static void *ctrl_thread_init(void *arg)
{
int ret;
- rte_cpuset_t *cpuset = &internal_config.ctrl_cpuset;
struct rte_thread_ctrl_params *params = arg;
void *(*start_routine)(void *) = params->start_routine;
void *routine_arg = params->arg;
- /* Store cpuset in TLS for quick access */
- memmove(&RTE_PER_LCORE(_cpuset), cpuset, sizeof(rte_cpuset_t));
+ rte_thread_init(rte_lcore_id(), &internal_config.ctrl_cpuset);
ret = pthread_barrier_wait(¶ms->configured);
if (ret == PTHREAD_BARRIER_SERIAL_THREAD) {
@@ -171,8 +182,6 @@ static void *rte_thread_init(void *arg)
free(params);
}
- __rte_trace_mem_per_thread_alloc();
-
return start_routine(routine_arg);
}
@@ -194,7 +203,7 @@ rte_ctrl_thread_create(pthread_t *thread, const char *name,
pthread_barrier_init(¶ms->configured, NULL, 2);
- ret = pthread_create(thread, attr, rte_thread_init, (void *)params);
+ ret = pthread_create(thread, attr, ctrl_thread_init, (void *)params);
if (ret != 0) {
free(params);
return -ret;
@@ -16,12 +16,14 @@
__rte_noreturn void *eal_thread_loop(void *arg);
/**
- * Init per-lcore info for master thread
+ * Init per-lcore info in current thread.
*
* @param lcore_id
- * identifier of master lcore
+ * identifier of lcore.
+ * @param cpuset
+ * CPU affinity for this thread.
*/
-void eal_thread_init_master(unsigned lcore_id);
+void rte_thread_init(unsigned int lcore_id, rte_cpuset_t *cpuset);
/**
* Get the NUMA socket id from cpu id.
@@ -877,7 +877,14 @@ rte_eal_init(int argc, char **argv)
eal_check_mem_on_local_socket();
- eal_thread_init_master(rte_config.master_lcore);
+ if (pthread_setaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
+ &lcore_config[rte_config.master_lcore].cpuset) != 0) {
+ rte_eal_init_alert("Cannot set affinity");
+ rte_errno = EINVAL;
+ return -1;
+ }
+ rte_thread_init(rte_config.master_lcore,
+ &lcore_config[rte_config.master_lcore].cpuset);
ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
@@ -908,6 +915,11 @@ rte_eal_init(int argc, char **argv)
snprintf(thread_name, sizeof(thread_name),
"lcore-slave-%d", i);
rte_thread_setname(lcore_config[i].thread_id, thread_name);
+
+ ret = pthread_setaffinity_np(lcore_config[i].thread_id,
+ sizeof(rte_cpuset_t), &lcore_config[i].cpuset);
+ if (ret != 0)
+ rte_panic("Cannot set affinity\n");
}
/*
@@ -66,29 +66,6 @@ rte_eal_remote_launch(int (*f)(void *), void *arg, unsigned slave_id)
return rc;
}
-/* set affinity for current thread */
-static int
-eal_thread_set_affinity(void)
-{
- unsigned lcore_id = rte_lcore_id();
-
- /* acquire system unique id */
- rte_gettid();
-
- /* update EAL thread core affinity */
- return rte_thread_set_affinity(&lcore_config[lcore_id].cpuset);
-}
-
-void eal_thread_init_master(unsigned lcore_id)
-{
- /* set the lcore ID in per-lcore memory area */
- RTE_PER_LCORE(_lcore_id) = lcore_id;
-
- /* set CPU affinity */
- if (eal_thread_set_affinity() < 0)
- rte_panic("cannot set affinity\n");
-}
-
/* main loop of threads */
__rte_noreturn void *
eal_thread_loop(__rte_unused void *arg)
@@ -113,19 +90,12 @@ eal_thread_loop(__rte_unused void *arg)
m2s = lcore_config[lcore_id].pipe_master2slave[0];
s2m = lcore_config[lcore_id].pipe_slave2master[1];
- /* set the lcore ID in per-lcore memory area */
- RTE_PER_LCORE(_lcore_id) = lcore_id;
-
- /* set CPU affinity */
- if (eal_thread_set_affinity() < 0)
- rte_panic("cannot set affinity\n");
+ rte_thread_init(lcore_id, &lcore_config[lcore_id].cpuset);
ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
-
RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%p;cpuset=[%s%s])\n",
lcore_id, thread_id, cpuset, ret == 0 ? "" : "...");
- __rte_trace_mem_per_thread_alloc();
rte_eal_trace_thread_lcore_ready(lcore_id, cpuset);
/* read on our pipe to get commands */
@@ -1205,10 +1205,16 @@ rte_eal_init(int argc, char **argv)
eal_check_mem_on_local_socket();
- eal_thread_init_master(rte_config.master_lcore);
+ if (pthread_setaffinity_np(pthread_self(), sizeof(rte_cpuset_t),
+ &lcore_config[rte_config.master_lcore].cpuset) != 0) {
+ rte_eal_init_alert("Cannot set affinity");
+ rte_errno = EINVAL;
+ return -1;
+ }
+ rte_thread_init(rte_config.master_lcore,
+ &lcore_config[rte_config.master_lcore].cpuset);
ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
-
RTE_LOG(DEBUG, EAL, "Master lcore %u is ready (tid=%zx;cpuset=[%s%s])\n",
rte_config.master_lcore, (uintptr_t)thread_id, cpuset,
ret == 0 ? "" : "...");
@@ -1240,6 +1246,11 @@ rte_eal_init(int argc, char **argv)
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);
+ if (ret != 0)
+ rte_panic("Cannot set affinity\n");
}
/*
@@ -66,29 +66,6 @@ rte_eal_remote_launch(int (*f)(void *), void *arg, unsigned slave_id)
return rc;
}
-/* set affinity for current EAL thread */
-static int
-eal_thread_set_affinity(void)
-{
- unsigned lcore_id = rte_lcore_id();
-
- /* acquire system unique id */
- rte_gettid();
-
- /* update EAL thread core affinity */
- return rte_thread_set_affinity(&lcore_config[lcore_id].cpuset);
-}
-
-void eal_thread_init_master(unsigned lcore_id)
-{
- /* set the lcore ID in per-lcore memory area */
- RTE_PER_LCORE(_lcore_id) = lcore_id;
-
- /* set CPU affinity */
- if (eal_thread_set_affinity() < 0)
- rte_panic("cannot set affinity\n");
-}
-
/* main loop of threads */
__rte_noreturn void *
eal_thread_loop(__rte_unused void *arg)
@@ -113,19 +90,12 @@ eal_thread_loop(__rte_unused void *arg)
m2s = lcore_config[lcore_id].pipe_master2slave[0];
s2m = lcore_config[lcore_id].pipe_slave2master[1];
- /* set the lcore ID in per-lcore memory area */
- RTE_PER_LCORE(_lcore_id) = lcore_id;
-
- /* set CPU affinity */
- if (eal_thread_set_affinity() < 0)
- rte_panic("cannot set affinity\n");
+ rte_thread_init(lcore_id, &lcore_config[lcore_id].cpuset);
ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
-
RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%zx;cpuset=[%s%s])\n",
lcore_id, (uintptr_t)thread_id, cpuset, ret == 0 ? "" : "...");
- __rte_trace_mem_per_thread_alloc();
rte_eal_trace_thread_lcore_ready(lcore_id, cpuset);
/* read on our pipe to get commands */
@@ -367,7 +367,8 @@ rte_eal_init(int argc, char **argv)
return -1;
}
- eal_thread_init_master(rte_config.master_lcore);
+ rte_thread_init(rte_config.master_lcore,
+ &lcore_config[rte_config.master_lcore].cpuset);
RTE_LCORE_FOREACH_SLAVE(i) {
@@ -53,13 +53,6 @@ rte_eal_remote_launch(lcore_function_t *f, void *arg, unsigned int slave_id)
return 0;
}
-void
-eal_thread_init_master(unsigned int lcore_id)
-{
- /* set the lcore ID in per-lcore memory area */
- RTE_PER_LCORE(_lcore_id) = lcore_id;
-}
-
/* main loop of threads */
void *
eal_thread_loop(void *arg __rte_unused)
@@ -84,8 +77,7 @@ eal_thread_loop(void *arg __rte_unused)
m2s = lcore_config[lcore_id].pipe_master2slave[0];
s2m = lcore_config[lcore_id].pipe_slave2master[1];
- /* set the lcore ID in per-lcore memory area */
- RTE_PER_LCORE(_lcore_id) = lcore_id;
+ rte_thread_init(lcore_id, &lcore_config[lcore_id].cpuset);
RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%zx;cpuset=[%s])\n",
lcore_id, (uintptr_t)thread_id, cpuset);