diff mbox

[dpdk-dev] Revert "eal: set name to threads"

Message ID 1447932137-16886-1-git-send-email-ferruh.yigit@intel.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Ferruh Yigit Nov. 19, 2015, 11:22 a.m. UTC
This reverts commit 67b6d3039e9edbc4624c878c6930be5e126e8b58.

Reverted patch uses pthread_setname_np() function, this function added
into glibc in version 2.12 and cause a compile error in older glibc
versions:
error: implicit declaration of function "pthread_setname_np"

Main purpose of reverted patch is to name threads, without
pthread_setname_np() function, patch does not mean much, so reverting
patch for sake of compatibility with older glibc versions.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 examples/tep_termination/Makefile                  |  1 -
 examples/tep_termination/main.c                    | 12 ++----------
 examples/vhost/Makefile                            |  1 -
 examples/vhost/main.c                              | 16 ++--------------
 examples/vhost_xen/Makefile                        |  1 -
 examples/vhost_xen/main.c                          | 16 ++--------------
 lib/librte_eal/bsdapp/eal/eal.c                    |  6 ------
 lib/librte_eal/common/include/rte_eal.h            |  3 ---
 lib/librte_eal/linuxapp/eal/Makefile               |  2 --
 lib/librte_eal/linuxapp/eal/eal.c                  | 10 ----------
 lib/librte_eal/linuxapp/eal/eal_interrupts.c       | 15 ++-------------
 lib/librte_eal/linuxapp/eal/eal_pci_vfio_mp_sync.c | 15 ++-------------
 lib/librte_eal/linuxapp/eal/eal_timer.c            | 12 +-----------
 13 files changed, 11 insertions(+), 99 deletions(-)

Comments

Panu Matilainen Nov. 19, 2015, 11:49 a.m. UTC | #1
On 11/19/2015 01:22 PM, Ferruh Yigit wrote:
> This reverts commit 67b6d3039e9edbc4624c878c6930be5e126e8b58.
>
> Reverted patch uses pthread_setname_np() function, this function added
> into glibc in version 2.12 and cause a compile error in older glibc
> versions:
> error: implicit declaration of function "pthread_setname_np"
>
> Main purpose of reverted patch is to name threads, without
> pthread_setname_np() function, patch does not mean much, so reverting
> patch for sake of compatibility with older glibc versions.
>

Debuggability is important too. Rather than revert, why not wrap it in 
rte_thread_setname() or such and just make it a no-op with glibc 
versions where pthread_setname_np() is not available?

	- Panu -
Thomas Monjalon Nov. 19, 2015, 12:32 p.m. UTC | #2
2015-11-19 13:49, Panu Matilainen:
> On 11/19/2015 01:22 PM, Ferruh Yigit wrote:
> > This reverts commit 67b6d3039e9edbc4624c878c6930be5e126e8b58.
> >
> > Reverted patch uses pthread_setname_np() function, this function added
> > into glibc in version 2.12 and cause a compile error in older glibc
> > versions:
> > error: implicit declaration of function "pthread_setname_np"
> >
> > Main purpose of reverted patch is to name threads, without
> > pthread_setname_np() function, patch does not mean much, so reverting
> > patch for sake of compatibility with older glibc versions.
> 
> Debuggability is important too. Rather than revert, why not wrap it in 
> rte_thread_setname() or such and just make it a no-op with glibc 
> versions where pthread_setname_np() is not available?

+1
diff mbox

Patch

diff --git a/examples/tep_termination/Makefile b/examples/tep_termination/Makefile
index 448e618..c37e50f 100644
--- a/examples/tep_termination/Makefile
+++ b/examples/tep_termination/Makefile
@@ -51,6 +51,5 @@  SRCS-y := main.c vxlan_setup.c vxlan.c
 
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
-CFLAGS += -D_GNU_SOURCE
 
 include $(RTE_SDK)/mk/rte.extapp.mk
diff --git a/examples/tep_termination/main.c b/examples/tep_termination/main.c
index 2b67e64..bd77617 100644
--- a/examples/tep_termination/main.c
+++ b/examples/tep_termination/main.c
@@ -1167,7 +1167,6 @@  main(int argc, char *argv[])
 	uint8_t portid;
 	uint16_t queue_id;
 	static pthread_t tid;
-	char thread_name[RTE_MAX_THREAD_NAME_LEN];
 
 	/* init EAL */
 	ret = rte_eal_init(argc, argv);
@@ -1244,15 +1243,8 @@  main(int argc, char *argv[])
 	memset(&dev_statistics, 0, sizeof(dev_statistics));
 
 	/* Enable stats if the user option is set. */
-	if (enable_stats) {
-		ret = pthread_create(&tid, NULL, (void *)print_stats, NULL);
-		if (ret != 0)
-			rte_exit(EXIT_FAILURE, "Cannot create print-stats thread\n");
-		snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "print-stats");
-		ret = pthread_setname_np(tid, thread_name);
-		if (ret != 0)
-			RTE_LOG(ERR, VHOST_CONFIG, "Cannot set print-stats name\n");
-	}
+	if (enable_stats)
+		pthread_create(&tid, NULL, (void *)print_stats, NULL);
 
 	/* Launch all data cores. */
 	RTE_LCORE_FOREACH_SLAVE(lcore_id) {
diff --git a/examples/vhost/Makefile b/examples/vhost/Makefile
index e95c68a..c269466 100644
--- a/examples/vhost/Makefile
+++ b/examples/vhost/Makefile
@@ -52,7 +52,6 @@  SRCS-y := main.c
 
 CFLAGS += -O2 -D_FILE_OFFSET_BITS=64
 CFLAGS += $(WERROR_FLAGS)
-CFLAGS += -D_GNU_SOURCE
 
 include $(RTE_SDK)/mk/rte.extapp.mk
 
diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index c081b18..9eac2d0 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -2896,7 +2896,6 @@  main(int argc, char *argv[])
 	uint8_t portid;
 	uint16_t queue_id;
 	static pthread_t tid;
-	char thread_name[RTE_MAX_THREAD_NAME_LEN];
 
 	signal(SIGINT, sigint_handler);
 
@@ -3019,19 +3018,8 @@  main(int argc, char *argv[])
 	memset(&dev_statistics, 0, sizeof(dev_statistics));
 
 	/* Enable stats if the user option is set. */
-	if (enable_stats) {
-		ret = pthread_create(&tid, NULL, (void *)print_stats, NULL);
-		if (ret != 0)
-			rte_exit(EXIT_FAILURE,
-				"Cannot create print-stats thread\n");
-
-		/* Set thread_name for aid in debugging.  */
-		snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "print-stats");
-		ret = pthread_setname_np(tid, thread_name);
-		if (ret != 0)
-			RTE_LOG(ERR, VHOST_CONFIG,
-				"Cannot set print-stats name\n");
-	}
+	if (enable_stats)
+		pthread_create(&tid, NULL, (void*)print_stats, NULL );
 
 	/* Launch all data cores. */
 	if (zero_copy == 0) {
diff --git a/examples/vhost_xen/Makefile b/examples/vhost_xen/Makefile
index 47e1489..e6fa1a1 100644
--- a/examples/vhost_xen/Makefile
+++ b/examples/vhost_xen/Makefile
@@ -46,7 +46,6 @@  SRCS-y := main.c vhost_monitor.c xenstore_parse.c
 
 CFLAGS += -O2 -I/usr/local/include -D_FILE_OFFSET_BITS=64 -Wno-unused-parameter
 CFLAGS += $(WERROR_FLAGS)
-CFLAGS += -D_GNU_SOURCE
 LDFLAGS += -lxenstore
 
 include $(RTE_SDK)/mk/rte.extapp.mk
diff --git a/examples/vhost_xen/main.c b/examples/vhost_xen/main.c
index 3fcc138..5d20700 100644
--- a/examples/vhost_xen/main.c
+++ b/examples/vhost_xen/main.c
@@ -1432,7 +1432,6 @@  main(int argc, char *argv[])
 	int ret;
 	uint8_t portid;
 	static pthread_t tid;
-	char thread_name[RTE_MAX_THREAD_NAME_LEN];
 
 	/* init EAL */
 	ret = rte_eal_init(argc, argv);
@@ -1502,19 +1501,8 @@  main(int argc, char *argv[])
 	memset(&dev_statistics, 0, sizeof(dev_statistics));
 
 	/* Enable stats if the user option is set. */
-	if (enable_stats) {
-		ret = pthread_create(&tid, NULL, (void *)print_stats, NULL);
-		if (ret != 0)
-			rte_exit(EXIT_FAILURE,
-				"Cannot create print-stats thread\n");
-
-		/* Set thread_name for aid in debugging. */
-		snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "print-xen-stats");
-		ret = pthread_setname_np(tid, thread_name);
-		if (ret != 0)
-			RTE_LOG(ERR, VHOST_CONFIG,
-				"Cannot set print-stats name\n");
-	}
+	if (enable_stats)
+		pthread_create(&tid, NULL, (void*)print_stats, NULL );
 
 	/* Launch all data cores. */
 	RTE_LCORE_FOREACH_SLAVE(lcore_id) {
diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index a34e61d..8e765c6 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -494,7 +494,6 @@  rte_eal_init(int argc, char **argv)
 	pthread_t thread_id;
 	static rte_atomic32_t run_once = RTE_ATOMIC32_INIT(0);
 	char cpuset[RTE_CPU_AFFINITY_STR_LEN];
-	char thread_name[RTE_MAX_THREAD_NAME_LEN];
 
 	if (!rte_atomic32_test_and_set(&run_once))
 		return -1;
@@ -603,11 +602,6 @@  rte_eal_init(int argc, char **argv)
 				     eal_thread_loop, NULL);
 		if (ret != 0)
 			rte_panic("Cannot create thread\n");
-
-		/* Set thread_name for aid in debugging. */
-		snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN,
-				"lcore-slave-%d", i);
-		pthread_set_name_np(lcore_config[i].thread_id, thread_name);
 	}
 
 	/*
diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
index d2816a8..f36a792 100644
--- a/lib/librte_eal/common/include/rte_eal.h
+++ b/lib/librte_eal/common/include/rte_eal.h
@@ -51,9 +51,6 @@  extern "C" {
 
 #define RTE_MAGIC 19820526 /**< Magic number written by the main partition when ready. */
 
-/* Maximum thread_name length. */
-#define RTE_MAX_THREAD_NAME_LEN 16
-
 /**
  * The lcore role (used in RTE or not).
  */
diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
index 7e36b86..d62196e 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -93,8 +93,6 @@  SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += malloc_heap.c
 
 CFLAGS_eal.o := -D_GNU_SOURCE
 CFLAGS_eal_interrupts.o := -D_GNU_SOURCE
-CFLAGS_eal_pci_vfio_mp_sync.o := -D_GNU_SOURCE
-CFLAGS_eal_timer.o := -D_GNU_SOURCE
 CFLAGS_eal_lcore.o := -D_GNU_SOURCE
 CFLAGS_eal_thread.o := -D_GNU_SOURCE
 CFLAGS_eal_log.o := -D_GNU_SOURCE
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 06536f2..274ea20 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -729,7 +729,6 @@  rte_eal_init(int argc, char **argv)
 	static rte_atomic32_t run_once = RTE_ATOMIC32_INIT(0);
 	const char *logid;
 	char cpuset[RTE_CPU_AFFINITY_STR_LEN];
-	char thread_name[RTE_MAX_THREAD_NAME_LEN];
 
 	if (!rte_atomic32_test_and_set(&run_once))
 		return -1;
@@ -855,15 +854,6 @@  rte_eal_init(int argc, char **argv)
 				     eal_thread_loop, NULL);
 		if (ret != 0)
 			rte_panic("Cannot create thread\n");
-
-		/* Set thread_name for aid in debugging. */
-		snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN,
-			"lcore-slave-%d", i);
-		ret = pthread_setname_np(lcore_config[i].thread_id,
-						thread_name);
-		if (ret != 0)
-			RTE_LOG(ERR, EAL,
-				"Cannot set name for lcore thread\n");
 	}
 
 	/*
diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index 95beb4c..e0e62e8 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -68,7 +68,6 @@ 
 
 #include "eal_private.h"
 #include "eal_vfio.h"
-#include "eal_thread.h"
 
 #define EAL_INTR_EPOLL_WAIT_FOREVER (-1)
 #define NB_OTHER_INTR               1
@@ -864,8 +863,7 @@  eal_intr_thread_main(__rte_unused void *arg)
 int
 rte_eal_intr_init(void)
 {
-	int ret = 0, ret_1 = 0;
-	char thread_name[RTE_MAX_THREAD_NAME_LEN];
+	int ret = 0;
 
 	/* init the global interrupt source head */
 	TAILQ_INIT(&intr_sources);
@@ -880,18 +878,9 @@  rte_eal_intr_init(void)
 	/* create the host thread to wait/handle the interrupt */
 	ret = pthread_create(&intr_thread, NULL,
 			eal_intr_thread_main, NULL);
-	if (ret != 0) {
+	if (ret != 0)
 		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 = pthread_setname_np(intr_thread, thread_name);
-		if (ret_1 != 0)
-			RTE_LOG(ERR, EAL,
-			"Failed to set thread name for interrupt handling\n");
-	}
 
 	return -ret;
 }
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio_mp_sync.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio_mp_sync.c
index 277565d..fec7080 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio_mp_sync.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio_mp_sync.c
@@ -34,7 +34,6 @@ 
 #include <string.h>
 #include <fcntl.h>
 #include <sys/socket.h>
-#include <pthread.h>
 
 /* sys/un.h with __USE_MISC uses strlen, which is unsafe */
 #ifdef __USE_MISC
@@ -55,7 +54,6 @@ 
 
 #include "eal_filesystem.h"
 #include "eal_pci_init.h"
-#include "eal_thread.h"
 
 /**
  * @file
@@ -376,7 +374,6 @@  int
 pci_vfio_mp_sync_setup(void)
 {
 	int ret;
-	char thread_name[RTE_MAX_THREAD_NAME_LEN];
 
 	if (vfio_mp_sync_socket_setup() < 0) {
 		RTE_LOG(ERR, EAL, "Failed to set up local socket!\n");
@@ -386,19 +383,11 @@  pci_vfio_mp_sync_setup(void)
 	ret = pthread_create(&socket_thread, NULL,
 			pci_vfio_mp_sync_thread, NULL);
 	if (ret) {
-		RTE_LOG(ERR, EAL,
-			"Failed to create thread for communication with secondary processes!\n");
+		RTE_LOG(ERR, EAL, "Failed to create thread for communication with "
+				"secondary processes!\n");
 		close(mp_socket_fd);
 		return -1;
 	}
-
-	/* Set thread_name for aid in debugging. */
-	snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "pci-vfio-sync");
-	ret = pthread_setname_np(socket_thread, thread_name);
-	if (ret)
-		RTE_LOG(ERR, EAL,
-			"Failed to set thread name for secondary processes!\n");
-
 	return 0;
 }
 
diff --git a/lib/librte_eal/linuxapp/eal/eal_timer.c b/lib/librte_eal/linuxapp/eal/eal_timer.c
index e0642de..76a8a65 100644
--- a/lib/librte_eal/linuxapp/eal/eal_timer.c
+++ b/lib/librte_eal/linuxapp/eal/eal_timer.c
@@ -166,7 +166,6 @@  int
 rte_eal_hpet_init(int make_default)
 {
 	int fd, ret;
-	char thread_name[RTE_MAX_THREAD_NAME_LEN];
 
 	if (internal_config.no_hpet) {
 		RTE_LOG(NOTICE, EAL, "HPET is disabled\n");
@@ -209,21 +208,12 @@  rte_eal_hpet_init(int make_default)
 	 * msb (hpet is 32 bits by default under linux) */
 	ret = pthread_create(&msb_inc_thread_id, NULL,
 			(void *(*)(void *))hpet_msb_inc, NULL);
-	if (ret != 0) {
+	if (ret < 0) {
 		RTE_LOG(ERR, EAL, "ERROR: Cannot create HPET timer thread!\n");
 		internal_config.no_hpet = 1;
 		return -1;
 	}
 
-	/*
-	 * Set thread_name for aid in debugging.
-	 */
-	snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "hpet-msb-inc");
-	ret = pthread_setname_np(msb_inc_thread_id, thread_name);
-	if (ret != 0)
-		RTE_LOG(ERR, EAL,
-			"ERROR: Cannot set HPET timer thread name!\n");
-
 	if (make_default)
 		eal_timer_source = EAL_TIMER_HPET;
 	return 0;