* [dpdk-dev] [PATCH 1/4] eal: use sizeof to avoid a double use of a define
2018-02-27 14:46 ` [dpdk-dev] [PATCH 0/4] fix control thread affinities Olivier Matz
@ 2018-02-27 14:46 ` Olivier Matz
2018-02-27 14:46 ` [dpdk-dev] [PATCH 2/4] eal: new function to create control threads Olivier Matz
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Olivier Matz @ 2018-02-27 14:46 UTC (permalink / raw)
To: dev
Only a cosmetic change: the *_LEN defines are already used
when defining the buffer. Using sizeof() ensures that the length
stays consistent, even if the definition is modified.
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
lib/librte_eal/bsdapp/eal/eal.c | 2 +-
lib/librte_eal/bsdapp/eal/eal_thread.c | 2 +-
lib/librte_eal/linuxapp/eal/eal.c | 4 ++--
lib/librte_eal/linuxapp/eal/eal_thread.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 4eafcb5ad..0b0fb9973 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -632,7 +632,7 @@ rte_eal_init(int argc, char **argv)
eal_thread_init_master(rte_config.master_lcore);
- ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
+ ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
RTE_LOG(DEBUG, EAL, "Master lcore %u is ready (tid=%p;cpuset=[%s%s])\n",
rte_config.master_lcore, thread_id, cpuset,
diff --git a/lib/librte_eal/bsdapp/eal/eal_thread.c b/lib/librte_eal/bsdapp/eal/eal_thread.c
index d602daf81..309b58726 100644
--- a/lib/librte_eal/bsdapp/eal/eal_thread.c
+++ b/lib/librte_eal/bsdapp/eal/eal_thread.c
@@ -119,7 +119,7 @@ eal_thread_loop(__attribute__((unused)) void *arg)
if (eal_thread_set_affinity() < 0)
rte_panic("cannot set affinity\n");
- ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
+ 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 ? "" : "...");
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 38306bf5c..1cb87ca25 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -888,7 +888,7 @@ rte_eal_init(int argc, char **argv)
eal_thread_init_master(rte_config.master_lcore);
- ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
+ ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
RTE_LOG(DEBUG, EAL, "Master lcore %u is ready (tid=%x;cpuset=[%s%s])\n",
rte_config.master_lcore, (int)thread_id, cpuset,
@@ -919,7 +919,7 @@ rte_eal_init(int argc, char **argv)
rte_panic("Cannot create thread\n");
/* Set thread_name for aid in debugging. */
- snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN,
+ snprintf(thread_name, sizeof(thread_name),
"lcore-slave-%d", i);
ret = rte_thread_setname(lcore_config[i].thread_id,
thread_name);
diff --git a/lib/librte_eal/linuxapp/eal/eal_thread.c b/lib/librte_eal/linuxapp/eal/eal_thread.c
index 08e150b77..f652ff988 100644
--- a/lib/librte_eal/linuxapp/eal/eal_thread.c
+++ b/lib/librte_eal/linuxapp/eal/eal_thread.c
@@ -119,7 +119,7 @@ eal_thread_loop(__attribute__((unused)) void *arg)
if (eal_thread_set_affinity() < 0)
rte_panic("cannot set affinity\n");
- ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
+ ret = eal_thread_dump_affinity(cpuset, sizeof(cpuset));
RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%x;cpuset=[%s%s])\n",
lcore_id, (int)thread_id, cpuset, ret == 0 ? "" : "...");
--
2.11.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [dpdk-dev] [PATCH 2/4] eal: new function to create control threads
2018-02-27 14:46 ` [dpdk-dev] [PATCH 0/4] fix control thread affinities Olivier Matz
2018-02-27 14:46 ` [dpdk-dev] [PATCH 1/4] eal: use sizeof to avoid a double use of a define Olivier Matz
@ 2018-02-27 14:46 ` Olivier Matz
2018-02-27 14:46 ` [dpdk-dev] [PATCH 3/4] eal: set name when creating a control thread Olivier Matz
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Olivier Matz @ 2018-02-27 14:46 UTC (permalink / raw)
To: dev
Many parts of dpdk use their own management threads. Introduce a new
wrapper for thread creation that will be extended in next commits to set
the name and affinity.
To be consistent with other DPDK APIs, the return value is negative in
case of error, which was not the case for pthread_create().
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
drivers/net/kni/Makefile | 1 +
drivers/net/kni/rte_eth_kni.c | 2 +-
lib/librte_eal/common/eal_common_thread.c | 8 ++++++++
lib/librte_eal/common/include/rte_lcore.h | 21 +++++++++++++++++++++
lib/librte_eal/linuxapp/eal/eal_interrupts.c | 6 +++---
lib/librte_eal/linuxapp/eal/eal_timer.c | 2 +-
lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c | 2 +-
lib/librte_eal/rte_eal_version.map | 1 +
lib/librte_pdump/Makefile | 1 +
lib/librte_pdump/rte_pdump.c | 5 +++--
lib/librte_vhost/socket.c | 6 +++---
11 files changed, 44 insertions(+), 11 deletions(-)
diff --git a/drivers/net/kni/Makefile b/drivers/net/kni/Makefile
index 01eaef056..562e8d2da 100644
--- a/drivers/net/kni/Makefile
+++ b/drivers/net/kni/Makefile
@@ -10,6 +10,7 @@ LIB = librte_pmd_kni.a
CFLAGS += -O3
CFLAGS += $(WERROR_FLAGS)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
LDLIBS += -lpthread
LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
LDLIBS += -lrte_ethdev -lrte_net -lrte_kvargs -lrte_kni
diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index dc4e65f5d..26718eb3e 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -149,7 +149,7 @@ eth_kni_dev_start(struct rte_eth_dev *dev)
}
if (internals->no_request_thread == 0) {
- ret = pthread_create(&internals->thread, NULL,
+ ret = rte_ctrl_thread_create(&internals->thread, NULL,
kni_handle_request, internals);
if (ret) {
RTE_LOG(ERR, PMD,
diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c
index 40902e49b..efbccddbc 100644
--- a/lib/librte_eal/common/eal_common_thread.c
+++ b/lib/librte_eal/common/eal_common_thread.c
@@ -140,3 +140,11 @@ eal_thread_dump_affinity(char *str, unsigned size)
return ret;
}
+
+__rte_experimental int
+rte_ctrl_thread_create(pthread_t *thread,
+ const pthread_attr_t *attr,
+ void *(*start_routine)(void *), void *arg)
+{
+ return pthread_create(thread, attr, start_routine, arg);
+}
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index 047222030..f19075a88 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -247,6 +247,27 @@ void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
int rte_thread_setname(pthread_t id, const char *name);
/**
+ * Create a control thread.
+ *
+ * Wrapper to pthread_create().
+ *
+ * @param thread
+ * Filled with the thread id of the new created thread.
+ * @param attr
+ * Attributes for the new thread.
+ * @param start_routine
+ * Function to be executed by the new thread.
+ * @param arg
+ * Argument passed to start_routine.
+ * @return
+ * On success, returns 0; on error, it returns a negative value
+ * corresponding to the error number.
+ */
+__rte_experimental int
+rte_ctrl_thread_create(pthread_t *thread, const pthread_attr_t *attr,
+ void *(*start_routine)(void *), void *arg);
+
+/**
* Test if the core supplied has a specific role
*
* @param lcore_id
diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index f86f22f7b..d927fb45d 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -860,10 +860,10 @@ rte_eal_intr_init(void)
}
/* create the host thread to wait/handle the interrupt */
- ret = pthread_create(&intr_thread, NULL,
+ ret = rte_ctrl_thread_create(&intr_thread, NULL,
eal_intr_thread_main, NULL);
if (ret != 0) {
- rte_errno = ret;
+ rte_errno = -ret;
RTE_LOG(ERR, EAL,
"Failed to create thread for interrupt handling\n");
} else {
@@ -876,7 +876,7 @@ rte_eal_intr_init(void)
"Failed to set thread name for interrupt handling\n");
}
- return -ret;
+ return ret;
}
static void
diff --git a/lib/librte_eal/linuxapp/eal/eal_timer.c b/lib/librte_eal/linuxapp/eal/eal_timer.c
index 161322f23..f12d2e134 100644
--- a/lib/librte_eal/linuxapp/eal/eal_timer.c
+++ b/lib/librte_eal/linuxapp/eal/eal_timer.c
@@ -178,7 +178,7 @@ rte_eal_hpet_init(int make_default)
/* create a thread that will increment a global variable for
* msb (hpet is 32 bits by default under linux) */
- ret = pthread_create(&msb_inc_thread_id, NULL,
+ ret = rte_ctrl_thread_create(&msb_inc_thread_id, NULL,
(void *(*)(void *))hpet_msb_inc, NULL);
if (ret != 0) {
RTE_LOG(ERR, EAL, "ERROR: Cannot create HPET timer thread!\n");
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
index 7cc3c1527..072c0a978 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
@@ -374,7 +374,7 @@ vfio_mp_sync_setup(void)
return -1;
}
- ret = pthread_create(&socket_thread, NULL,
+ ret = rte_ctrl_thread_create(&socket_thread, NULL,
vfio_mp_sync_thread, NULL);
if (ret) {
RTE_LOG(ERR, EAL,
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index d12360235..24bb80567 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -214,6 +214,7 @@ DPDK_18.02 {
EXPERIMENTAL {
global:
+ rte_ctrl_thread_create;
rte_eal_cleanup;
rte_eal_devargs_insert;
rte_eal_devargs_parse;
diff --git a/lib/librte_pdump/Makefile b/lib/librte_pdump/Makefile
index 98fa752ec..d72a90a60 100644
--- a/lib/librte_pdump/Makefile
+++ b/lib/librte_pdump/Makefile
@@ -6,6 +6,7 @@ include $(RTE_SDK)/mk/rte.vars.mk
# library name
LIB = librte_pdump.a
+CFLAGS += -DALLOW_EXPERIMENTAL_API
CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3
CFLAGS += -D_GNU_SOURCE
LDLIBS += -lpthread
diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index ec8a5d84c..917a99957 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -548,11 +548,12 @@ rte_pdump_init(const char *path)
}
/* create the host thread to wait/handle pdump requests */
- ret = pthread_create(&pdump_thread, NULL, pdump_thread_main, NULL);
+ ret = rte_ctrl_thread_create(&pdump_thread, NULL,
+ pdump_thread_main, NULL);
if (ret != 0) {
RTE_LOG(ERR, PDUMP,
"Failed to create the pdump thread:%s, %s:%d\n",
- strerror(ret), __func__, __LINE__);
+ strerror(-ret), __func__, __LINE__);
return -1;
}
/* Set thread_name for aid in debugging. */
diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 83befdced..d261bf4b0 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -445,7 +445,7 @@ vhost_user_reconnect_init(void)
}
TAILQ_INIT(&reconn_list.head);
- ret = pthread_create(&reconn_tid, NULL,
+ ret = rte_ctrl_thread_create(&reconn_tid, NULL,
vhost_user_client_reconnect, NULL);
if (ret != 0) {
RTE_LOG(ERR, VHOST_CONFIG, "failed to create reconnect thread");
@@ -829,8 +829,8 @@ rte_vhost_driver_start(const char *path)
return -1;
if (fdset_tid == 0) {
- int ret = pthread_create(&fdset_tid, NULL, fdset_event_dispatch,
- &vhost_user.fdset);
+ int ret = rte_ctrl_thread_create(&fdset_tid, NULL,
+ fdset_event_dispatch, &vhost_user.fdset);
if (ret != 0)
RTE_LOG(ERR, VHOST_CONFIG,
"failed to create fdset handling thread");
--
2.11.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [dpdk-dev] [PATCH 3/4] eal: set name when creating a control thread
2018-02-27 14:46 ` [dpdk-dev] [PATCH 0/4] fix control thread affinities Olivier Matz
2018-02-27 14:46 ` [dpdk-dev] [PATCH 1/4] eal: use sizeof to avoid a double use of a define Olivier Matz
2018-02-27 14:46 ` [dpdk-dev] [PATCH 2/4] eal: new function to create control threads Olivier Matz
@ 2018-02-27 14:46 ` Olivier Matz
2018-02-27 14:46 ` [dpdk-dev] [PATCH 4/4] eal: set affinity for control threads Olivier Matz
2018-03-28 12:54 ` [dpdk-dev] [PATCH 0/4] fix control thread affinities Olivier Matz
4 siblings, 0 replies; 13+ messages in thread
From: Olivier Matz @ 2018-02-27 14:46 UTC (permalink / raw)
To: dev
To avoid code duplication, add a parameter to rte_ctrl_thread_create()
to specify the name of the thread.
This requires to add a wrapper for the thread start routine in
rte_thread_init(), which will first wait that the thread is configured.
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
drivers/net/kni/rte_eth_kni.c | 3 +-
lib/librte_eal/common/eal_common_thread.c | 52 ++++++++++++++++++++++++--
lib/librte_eal/common/include/rte_lcore.h | 7 +++-
lib/librte_eal/linuxapp/eal/eal_interrupts.c | 13 ++-----
lib/librte_eal/linuxapp/eal/eal_timer.c | 12 +-----
lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c | 10 +----
lib/librte_pdump/rte_pdump.c | 10 +----
lib/librte_vhost/socket.c | 7 ++--
8 files changed, 68 insertions(+), 46 deletions(-)
diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index 26718eb3e..6b036d8e1 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -149,7 +149,8 @@ eth_kni_dev_start(struct rte_eth_dev *dev)
}
if (internals->no_request_thread == 0) {
- ret = rte_ctrl_thread_create(&internals->thread, NULL,
+ ret = rte_ctrl_thread_create(&internals->thread,
+ "kni_handle_request", NULL,
kni_handle_request, internals);
if (ret) {
RTE_LOG(ERR, PMD,
diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c
index efbccddbc..575b03e9d 100644
--- a/lib/librte_eal/common/eal_common_thread.c
+++ b/lib/librte_eal/common/eal_common_thread.c
@@ -7,6 +7,7 @@
#include <stdint.h>
#include <unistd.h>
#include <pthread.h>
+#include <signal.h>
#include <sched.h>
#include <assert.h>
#include <string.h>
@@ -141,10 +142,53 @@ eal_thread_dump_affinity(char *str, unsigned size)
return ret;
}
+
+struct rte_thread_ctrl_params {
+ void *(*start_routine)(void *);
+ void *arg;
+ pthread_barrier_t configured;
+};
+
+static void *rte_thread_init(void *arg)
+{
+ struct rte_thread_ctrl_params *params = arg;
+ void *(*start_routine)(void *) = params->start_routine;
+ void *routine_arg = params->arg;
+
+ pthread_barrier_wait(¶ms->configured);
+
+ return start_routine(routine_arg);
+}
+
__rte_experimental int
-rte_ctrl_thread_create(pthread_t *thread,
- const pthread_attr_t *attr,
- void *(*start_routine)(void *), void *arg)
+rte_ctrl_thread_create(pthread_t *thread, const char *name,
+ const pthread_attr_t *attr,
+ void *(*start_routine)(void *), void *arg)
{
- return pthread_create(thread, attr, start_routine, arg);
+ struct rte_thread_ctrl_params params = {
+ .start_routine = start_routine,
+ .arg = arg,
+ };
+ int ret;
+
+ pthread_barrier_init(¶ms.configured, NULL, 2);
+
+ ret = pthread_create(thread, attr, rte_thread_init, (void *)¶ms);
+ if (ret != 0)
+ return ret;
+
+ if (name != NULL) {
+ ret = rte_thread_setname(*thread, name);
+ if (ret < 0)
+ goto fail;
+ }
+
+ pthread_barrier_wait(¶ms.configured);
+
+ return 0;
+
+fail:
+ pthread_kill(*thread, SIGTERM);
+ pthread_join(*thread, NULL);
+ return ret;
}
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index f19075a88..f3d9bbb91 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -249,10 +249,12 @@ int rte_thread_setname(pthread_t id, const char *name);
/**
* Create a control thread.
*
- * Wrapper to pthread_create().
+ * Wrapper to pthread_create() and pthread_setname_np().
*
* @param thread
* Filled with the thread id of the new created thread.
+ * @param name
+ * The name of the control thread (max 16 characters including '\0').
* @param attr
* Attributes for the new thread.
* @param start_routine
@@ -264,7 +266,8 @@ int rte_thread_setname(pthread_t id, const char *name);
* corresponding to the error number.
*/
__rte_experimental int
-rte_ctrl_thread_create(pthread_t *thread, const pthread_attr_t *attr,
+rte_ctrl_thread_create(pthread_t *thread, const char *name,
+ const pthread_attr_t *attr,
void *(*start_routine)(void *), void *arg);
/**
diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index d927fb45d..3f184bed3 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -844,7 +844,7 @@ eal_intr_thread_main(__rte_unused void *arg)
int
rte_eal_intr_init(void)
{
- int ret = 0, ret_1 = 0;
+ int ret = 0;
char thread_name[RTE_MAX_THREAD_NAME_LEN];
/* init the global interrupt source head */
@@ -860,20 +860,13 @@ rte_eal_intr_init(void)
}
/* create the host thread to wait/handle the interrupt */
- ret = rte_ctrl_thread_create(&intr_thread, NULL,
+ snprintf(thread_name, sizeof(thread_name), "eal-intr-thread");
+ ret = rte_ctrl_thread_create(&intr_thread, thread_name, NULL,
eal_intr_thread_main, NULL);
if (ret != 0) {
rte_errno = -ret;
RTE_LOG(ERR, EAL,
"Failed to create thread for interrupt handling\n");
- } else {
- /* Set thread_name for aid in debugging. */
- snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN,
- "eal-intr-thread");
- ret_1 = rte_thread_setname(intr_thread, thread_name);
- if (ret_1 != 0)
- RTE_LOG(DEBUG, EAL,
- "Failed to set thread name for interrupt handling\n");
}
return ret;
diff --git a/lib/librte_eal/linuxapp/eal/eal_timer.c b/lib/librte_eal/linuxapp/eal/eal_timer.c
index f12d2e134..17b178e80 100644
--- a/lib/librte_eal/linuxapp/eal/eal_timer.c
+++ b/lib/librte_eal/linuxapp/eal/eal_timer.c
@@ -178,7 +178,8 @@ rte_eal_hpet_init(int make_default)
/* create a thread that will increment a global variable for
* msb (hpet is 32 bits by default under linux) */
- ret = rte_ctrl_thread_create(&msb_inc_thread_id, NULL,
+ snprintf(thread_name, sizeof(thread_name), "hpet-msb-inc");
+ ret = rte_ctrl_thread_create(&msb_inc_thread_id, thread_name, NULL,
(void *(*)(void *))hpet_msb_inc, NULL);
if (ret != 0) {
RTE_LOG(ERR, EAL, "ERROR: Cannot create HPET timer thread!\n");
@@ -186,15 +187,6 @@ rte_eal_hpet_init(int make_default)
return -1;
}
- /*
- * Set thread_name for aid in debugging.
- */
- snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "hpet-msb-inc");
- ret = rte_thread_setname(msb_inc_thread_id, thread_name);
- if (ret != 0)
- RTE_LOG(DEBUG, EAL,
- "Cannot set HPET timer thread name!\n");
-
if (make_default)
eal_timer_source = EAL_TIMER_HPET;
return 0;
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
index 072c0a978..0cc726c9b 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
@@ -374,7 +374,8 @@ vfio_mp_sync_setup(void)
return -1;
}
- ret = rte_ctrl_thread_create(&socket_thread, NULL,
+ snprintf(thread_name, sizeof(thread_name), "vfio-sync");
+ ret = rte_ctrl_thread_create(&socket_thread, thread_name, NULL,
vfio_mp_sync_thread, NULL);
if (ret) {
RTE_LOG(ERR, EAL,
@@ -383,13 +384,6 @@ vfio_mp_sync_setup(void)
return -1;
}
- /* Set thread_name for aid in debugging. */
- snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "vfio-sync");
- ret = rte_thread_setname(socket_thread, thread_name);
- if (ret)
- RTE_LOG(DEBUG, EAL,
- "Failed to set thread name for secondary processes!\n");
-
return 0;
}
diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index 917a99957..824033f9a 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -548,7 +548,8 @@ rte_pdump_init(const char *path)
}
/* create the host thread to wait/handle pdump requests */
- ret = rte_ctrl_thread_create(&pdump_thread, NULL,
+ snprintf(thread_name, sizeof(thread_name), "pdump-thread");
+ ret = rte_ctrl_thread_create(&pdump_thread, thread_name, NULL,
pdump_thread_main, NULL);
if (ret != 0) {
RTE_LOG(ERR, PDUMP,
@@ -556,13 +557,6 @@ rte_pdump_init(const char *path)
strerror(-ret), __func__, __LINE__);
return -1;
}
- /* Set thread_name for aid in debugging. */
- snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "pdump-thread");
- ret = rte_thread_setname(pdump_thread, thread_name);
- if (ret != 0) {
- RTE_LOG(DEBUG, PDUMP,
- "Failed to set thread name for pdump handling\n");
- }
return 0;
}
diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index d261bf4b0..6fe5859d1 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -445,7 +445,7 @@ vhost_user_reconnect_init(void)
}
TAILQ_INIT(&reconn_list.head);
- ret = rte_ctrl_thread_create(&reconn_tid, NULL,
+ ret = rte_ctrl_thread_create(&reconn_tid, "vhost_reconnect", NULL,
vhost_user_client_reconnect, NULL);
if (ret != 0) {
RTE_LOG(ERR, VHOST_CONFIG, "failed to create reconnect thread");
@@ -829,8 +829,9 @@ rte_vhost_driver_start(const char *path)
return -1;
if (fdset_tid == 0) {
- int ret = rte_ctrl_thread_create(&fdset_tid, NULL,
- fdset_event_dispatch, &vhost_user.fdset);
+ int ret = rte_ctrl_thread_create(&fdset_tid,
+ "vhost_dispatch", NULL, fdset_event_dispatch,
+ &vhost_user.fdset);
if (ret != 0)
RTE_LOG(ERR, VHOST_CONFIG,
"failed to create fdset handling thread");
--
2.11.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [dpdk-dev] [PATCH 4/4] eal: set affinity for control threads
2018-02-27 14:46 ` [dpdk-dev] [PATCH 0/4] fix control thread affinities Olivier Matz
` (2 preceding siblings ...)
2018-02-27 14:46 ` [dpdk-dev] [PATCH 3/4] eal: set name when creating a control thread Olivier Matz
@ 2018-02-27 14:46 ` Olivier Matz
2018-03-29 8:04 ` Burakov, Anatoly
2018-03-28 12:54 ` [dpdk-dev] [PATCH 0/4] fix control thread affinities Olivier Matz
4 siblings, 1 reply; 13+ messages in thread
From: Olivier Matz @ 2018-02-27 14:46 UTC (permalink / raw)
To: dev
The management threads must not bother the dataplane or service cores.
Set the affinity of these threads accordingly.
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
lib/librte_eal/common/eal_common_thread.c | 20 +++++++++++++++++++-
lib/librte_eal/common/include/rte_lcore.h | 4 +++-
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c
index 575b03e9d..f2e588c97 100644
--- a/lib/librte_eal/common/eal_common_thread.c
+++ b/lib/librte_eal/common/eal_common_thread.c
@@ -16,6 +16,7 @@
#include <rte_memory.h>
#include <rte_log.h>
+#include "eal_private.h"
#include "eal_thread.h"
RTE_DECLARE_PER_LCORE(unsigned , _socket_id);
@@ -169,7 +170,9 @@ rte_ctrl_thread_create(pthread_t *thread, const char *name,
.start_routine = start_routine,
.arg = arg,
};
- int ret;
+ unsigned int lcore_id;
+ rte_cpuset_t cpuset;
+ int set_affinity, ret;
pthread_barrier_init(¶ms.configured, NULL, 2);
@@ -183,6 +186,21 @@ rte_ctrl_thread_create(pthread_t *thread, const char *name,
goto fail;
}
+ set_affinity = 0;
+ CPU_ZERO(&cpuset);
+ for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
+ if (eal_cpu_detected(lcore_id) &&
+ rte_lcore_has_role(lcore_id, ROLE_OFF)) {
+ CPU_SET(lcore_id, &cpuset);
+ set_affinity = 1;
+ }
+ }
+ if (set_affinity) {
+ ret = pthread_setaffinity_np(*thread, sizeof(cpuset), &cpuset);
+ if (ret < 0)
+ goto fail;
+ }
+
pthread_barrier_wait(¶ms.configured);
return 0;
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index f3d9bbb91..354717c5d 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -249,7 +249,9 @@ int rte_thread_setname(pthread_t id, const char *name);
/**
* Create a control thread.
*
- * Wrapper to pthread_create() and pthread_setname_np().
+ * Wrapper to pthread_create(), pthread_setname_np() and
+ * pthread_setaffinity_np(). The dataplane and service lcores are
+ * excluded from the affinity of the new thread.
*
* @param thread
* Filled with the thread id of the new created thread.
--
2.11.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [dpdk-dev] [PATCH 4/4] eal: set affinity for control threads
2018-02-27 14:46 ` [dpdk-dev] [PATCH 4/4] eal: set affinity for control threads Olivier Matz
@ 2018-03-29 8:04 ` Burakov, Anatoly
2018-03-30 7:30 ` Olivier Matz
0 siblings, 1 reply; 13+ messages in thread
From: Burakov, Anatoly @ 2018-03-29 8:04 UTC (permalink / raw)
To: Olivier Matz, dev
On 27-Feb-18 2:46 PM, Olivier Matz wrote:
> The management threads must not bother the dataplane or service cores.
> Set the affinity of these threads accordingly.
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
<...>
>
> + set_affinity = 0;
> + CPU_ZERO(&cpuset);
> + for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
> + if (eal_cpu_detected(lcore_id) &&
> + rte_lcore_has_role(lcore_id, ROLE_OFF)) {
> + CPU_SET(lcore_id, &cpuset);
> + set_affinity = 1;
> + }
> + }
> + if (set_affinity) {
> + ret = pthread_setaffinity_np(*thread, sizeof(cpuset), &cpuset);
> + if (ret < 0)
> + goto fail;
> + }
Hi Olivier,
Please correct me if i'm wrong here, but if all detected cores are busy
doing something (such as would be the case if DPDK is run without a
coremask specified), affinity is not set? Maybe set it to master lcore
instead (perhaps unconditionally - we usually recommend not using master
lcore for anything important - perhaps setting it to master core always
would be better, instead of trying to find unused lcores)?
--
Thanks,
Anatoly
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [dpdk-dev] [PATCH 4/4] eal: set affinity for control threads
2018-03-29 8:04 ` Burakov, Anatoly
@ 2018-03-30 7:30 ` Olivier Matz
0 siblings, 0 replies; 13+ messages in thread
From: Olivier Matz @ 2018-03-30 7:30 UTC (permalink / raw)
To: Burakov, Anatoly; +Cc: dev
On Thu, Mar 29, 2018 at 09:04:52AM +0100, Burakov, Anatoly wrote:
> On 27-Feb-18 2:46 PM, Olivier Matz wrote:
> > The management threads must not bother the dataplane or service cores.
> > Set the affinity of these threads accordingly.
> >
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > ---
>
> <...>
>
> > + set_affinity = 0;
> > + CPU_ZERO(&cpuset);
> > + for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
> > + if (eal_cpu_detected(lcore_id) &&
> > + rte_lcore_has_role(lcore_id, ROLE_OFF)) {
> > + CPU_SET(lcore_id, &cpuset);
> > + set_affinity = 1;
> > + }
> > + }
> > + if (set_affinity) {
> > + ret = pthread_setaffinity_np(*thread, sizeof(cpuset), &cpuset);
> > + if (ret < 0)
> > + goto fail;
> > + }
>
> Hi Olivier,
>
> Please correct me if i'm wrong here, but if all detected cores are busy
> doing something (such as would be the case if DPDK is run without a coremask
> specified), affinity is not set? Maybe set it to master lcore instead
> (perhaps unconditionally - we usually recommend not using master lcore for
> anything important - perhaps setting it to master core always would be
> better, instead of trying to find unused lcores)?
Good point and good idea, I will update the patchset.
Thanks,
Olivier
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH 0/4] fix control thread affinities
2018-02-27 14:46 ` [dpdk-dev] [PATCH 0/4] fix control thread affinities Olivier Matz
` (3 preceding siblings ...)
2018-02-27 14:46 ` [dpdk-dev] [PATCH 4/4] eal: set affinity for control threads Olivier Matz
@ 2018-03-28 12:54 ` Olivier Matz
4 siblings, 0 replies; 13+ messages in thread
From: Olivier Matz @ 2018-03-28 12:54 UTC (permalink / raw)
To: dev
Hi,
On Tue, Feb 27, 2018 at 03:46:26PM +0100, Olivier Matz wrote:
> Some parts of dpdk use their own management threads. Most of the time,
> the affinity of the thread is not properly set: it should not be scheduled
> on the dataplane cores, because interrupting them can cause packet losses.
>
> This patchset introduces a new wrapper for thread creation that does
> the job automatically, avoiding code duplication.
>
> Olivier Matz (4):
> eal: use sizeof to avoid a double use of a define
> eal: new function to create control threads
> eal: set name when creating a control thread
> eal: set affinity for control threads
>
> drivers/net/kni/Makefile | 1 +
> drivers/net/kni/rte_eth_kni.c | 3 +-
> lib/librte_eal/bsdapp/eal/eal.c | 2 +-
> lib/librte_eal/bsdapp/eal/eal_thread.c | 2 +-
> lib/librte_eal/common/eal_common_thread.c | 70 ++++++++++++++++++++++++++
> lib/librte_eal/common/include/rte_lcore.h | 26 ++++++++++
> lib/librte_eal/linuxapp/eal/eal.c | 4 +-
> lib/librte_eal/linuxapp/eal/eal_interrupts.c | 17 ++-----
> lib/librte_eal/linuxapp/eal/eal_thread.c | 2 +-
> lib/librte_eal/linuxapp/eal/eal_timer.c | 12 +----
> lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c | 10 +---
> lib/librte_eal/rte_eal_version.map | 1 +
> lib/librte_pdump/Makefile | 1 +
> lib/librte_pdump/rte_pdump.c | 13 ++---
> lib/librte_vhost/socket.c | 7 +--
> 15 files changed, 123 insertions(+), 48 deletions(-)
Any comment about this patchset?
Thanks,
Olivier
^ permalink raw reply [flat|nested] 13+ messages in thread