Compile errors with libc < 2.12

Compile errors with libc < 2.12

[Ferruh Yigit]

When libc version is < 2.12 there are two compile errors, both introduced in DPDK2.2, more details below.

We can solve issue in a few ways:
1- Ignore errors, this depends on if any user has this version. glibc 2.12 released around 2010, still may have distros with libc < 2.12, at least SUSE11 does.
2- Make DPDK compatible with older glibc versions. 
3- Use a compile option to wrap functionality that fails

I prefer option 2,
option 1 may break DPDK for some users, and option 3 adds new complexity and dependency to maintain.

Please advise how to proceed? 



Compile issues:
1- pthread_setname_np() 
Error msg: error: implicit declaration of function ‘pthread_setname_np’
Reason: pthread_setname_np() introduced in glibc 2.12
Commit: eal: set name to threads (commit 67b6d3039e9edbc4624c878c6930be5e126e8b58)
Introduced in: DPDK2.2
Comment: This is mostly for debug, can be reverted without functionality loss


2- CLOCK_MONOTONIC_RAW macro
Error msg: error: identifier "CLOCK_MONOTONIC_RAW" is undefined
Reason: CLOCK_MONOTONIC_RAW introduced in glibc 2.12
Commit eal/linux: make alarm not affected by system time jump (commit d08d304508a8a8caf255baf622ab65db1fec952c)
Introduced in: DPDK2.2
Comment: Can keep functionality but may use supported "CLOCK_MONOTONIC" instead of "CLOCK_MONOTONIC_RAW"


Thanks,
ferruh

Re: Compile errors with libc < 2.12

[Thomas Monjalon]

2015-11-18 13:32, Ferruh Yigit:
> When libc version is < 2.12 there are two compile errors, both introduced in DPDK2.2, more details below.
> 
> We can solve issue in a few ways:
> 1- Ignore errors, this depends on if any user has this version. glibc 2.12 released around 2010, still may have distros with libc < 2.12, at least SUSE11 does.
> 2- Make DPDK compatible with older glibc versions. 
> 3- Use a compile option to wrap functionality that fails
> 
> I prefer option 2,
> option 1 may break DPDK for some users, and option 3 adds new complexity and dependency to maintain.
> 
> Please advise how to proceed? 

Yes, please try the option 2.
Having a degraded mode for old libc is acceptable. So you can use some stubs.
Thanks

[PATCH] Revert "eal: set name to threads"

[Ferruh Yigit]

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(-)

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;
-- 
2.5.0

[PATCH] eal: fix compile error for old glibc caused by CLOCK_MONOTONIC_RAW

[Ferruh Yigit]

Fixes: d08d304508a8 ("eal/linux: make alarm not affected by system time
jump")

CLOCK_MONOTONIC_RAW added in glibc 2.12, using this define in older
glibc versions cause compile error:
'error: identifier "CLOCK_MONOTONIC_RAW" is undefined'

This patch replaces "CLOCK_MONOTONIC_RAW" with "CLOCK_MONOTONIC" for
older glibc versions, versions that support "CLOCK_MONOTONIC_RAW"
will keep using this clock type.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_alarm.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_alarm.c b/lib/librte_eal/linuxapp/eal/eal_alarm.c
index ff57323..8b042ab 100644
--- a/lib/librte_eal/linuxapp/eal/eal_alarm.c
+++ b/lib/librte_eal/linuxapp/eal/eal_alarm.c
@@ -63,6 +63,12 @@
 #define MS_PER_S 1000
 #define US_PER_S (US_PER_MS * MS_PER_S)
 
+#ifdef CLOCK_MONOTONIC_RAW /* Defined in glibc bits/time.h */
+#define CLOCK_TYPE_ID CLOCK_MONOTONIC_RAW
+#else
+#define CLOCK_TYPE_ID CLOCK_MONOTONIC
+#endif
+
 struct alarm_entry {
 	LIST_ENTRY(alarm_entry) next;
 	struct timeval time;
@@ -104,7 +110,7 @@ eal_alarm_callback(struct rte_intr_handle *hdl __rte_unused,
 
 	rte_spinlock_lock(&alarm_list_lk);
 	while ((ap = LIST_FIRST(&alarm_list)) !=NULL &&
-			clock_gettime(CLOCK_MONOTONIC_RAW, &now) == 0 &&
+			clock_gettime(CLOCK_TYPE_ID, &now) == 0 &&
 			(ap->time.tv_sec < now.tv_sec || (ap->time.tv_sec == now.tv_sec &&
 						(ap->time.tv_usec * NS_PER_US) <= now.tv_nsec))) {
 		ap->executing = 1;
@@ -152,7 +158,7 @@ rte_eal_alarm_set(uint64_t us, rte_eal_alarm_callback cb_fn, void *cb_arg)
 		return -ENOMEM;
 
 	/* use current time to calculate absolute time of alarm */
-	clock_gettime(CLOCK_MONOTONIC_RAW, &now);
+	clock_gettime(CLOCK_TYPE_ID, &now);
 
 	new_alarm->cb_fn = cb_fn;
 	new_alarm->cb_arg = cb_arg;
-- 
2.5.0

Re: [PATCH] Revert "eal: set name to threads"

[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?

	- Panu -

Re: [PATCH] Revert "eal: set name to threads"

[Thomas Monjalon]

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

Re: [PATCH] Revert "eal: set name to threads"

[Ferruh Yigit]

On Thu, Nov 19, 2015 at 01:32:44PM +0100, Thomas Monjalon wrote:
> 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

Which means adding compile time glibc version check which I was trying to avoid,
I believe we should not add glibc version dependencies unless feature is really required,
but sure I can implement that way, will send v2.

Thanks,
ferruh

Re: [PATCH] Revert "eal: set name to threads"

[Ferruh Yigit]

On Thu, Nov 19, 2015 at 01:39:24PM +0000, Ferruh Yigit wrote:
> On Thu, Nov 19, 2015 at 01:32:44PM +0100, Thomas Monjalon wrote:
> > 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
> 
> Which means adding compile time glibc version check which I was trying to avoid,
> I believe we should not add glibc version dependencies unless feature is really required,
> but sure I can implement that way, will send v2.
> 
I tried defining weak symbol within DPDK, and library already has strong version, this was nice try, thanks to Sergio,
but this also does not work because how linker works, weak symbol in binary overrides the strong in shared library..

[PATCH] eal: fix compile error for old glibc caused by pthread_setname_np()

[Ferruh Yigit]

Fixes: 67b6d3039e9e ("eal: set name to threads")

pthread_setname_np() function added in glibc 2.12, using this function
in older glibc versions cause compile error:
error: implicit declaration of function "pthread_setname_np"

This patch adds "rte_thread_setname" macro and set it according
glibc >= 2.12 check, thread naming disabled for older glibc versions,
glibc versions that support "pthread_setname_np" will keep using this
function.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 examples/tep_termination/main.c                    | 2 +-
 examples/vhost/main.c                              | 2 +-
 examples/vhost_xen/main.c                          | 2 +-
 lib/librte_eal/common/eal_thread.h                 | 6 ++++++
 lib/librte_eal/linuxapp/eal/eal.c                  | 2 +-
 lib/librte_eal/linuxapp/eal/eal_interrupts.c       | 2 +-
 lib/librte_eal/linuxapp/eal/eal_pci_vfio_mp_sync.c | 2 +-
 lib/librte_eal/linuxapp/eal/eal_timer.c            | 2 +-
 8 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/examples/tep_termination/main.c b/examples/tep_termination/main.c
index 2b67e64..f97d552 100644
--- a/examples/tep_termination/main.c
+++ b/examples/tep_termination/main.c
@@ -1249,7 +1249,7 @@ main(int argc, char *argv[])
 		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);
+		ret = rte_thread_setname(tid, thread_name);
 		if (ret != 0)
 			RTE_LOG(ERR, VHOST_CONFIG, "Cannot set print-stats name\n");
 	}
diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index c081b18..9bfda6d 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -3027,7 +3027,7 @@ main(int argc, char *argv[])
 
 		/* Set thread_name for aid in debugging.  */
 		snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "print-stats");
-		ret = pthread_setname_np(tid, thread_name);
+		ret = rte_thread_setname(tid, thread_name);
 		if (ret != 0)
 			RTE_LOG(ERR, VHOST_CONFIG,
 				"Cannot set print-stats name\n");
diff --git a/examples/vhost_xen/main.c b/examples/vhost_xen/main.c
index 3fcc138..ca725f2 100644
--- a/examples/vhost_xen/main.c
+++ b/examples/vhost_xen/main.c
@@ -1510,7 +1510,7 @@ main(int argc, char *argv[])
 
 		/* 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);
+		ret = rte_thread_setname(tid, thread_name);
 		if (ret != 0)
 			RTE_LOG(ERR, VHOST_CONFIG,
 				"Cannot set print-stats name\n");
diff --git a/lib/librte_eal/common/eal_thread.h b/lib/librte_eal/common/eal_thread.h
index e4e76b9..58e410d 100644
--- a/lib/librte_eal/common/eal_thread.h
+++ b/lib/librte_eal/common/eal_thread.h
@@ -36,6 +36,12 @@
 
 #include <rte_lcore.h>
 
+#if __GLIBC_PREREQ(2, 12)
+#define rte_thread_setname(...) pthread_setname_np(__VA_ARGS__)
+#else
+#define rte_thread_setname(...) 0
+#endif
+
 /**
  * basic loop of thread, called for each thread by eal_init().
  *
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 06536f2..635ec36 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -859,7 +859,7 @@ rte_eal_init(int argc, char **argv)
 		/* 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,
+		ret = rte_thread_setname(lcore_config[i].thread_id,
 						thread_name);
 		if (ret != 0)
 			RTE_LOG(ERR, EAL,
diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index 95beb4c..470d6a1 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -887,7 +887,7 @@ rte_eal_intr_init(void)
 		/* 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);
+		ret_1 = rte_thread_setname(intr_thread, thread_name);
 		if (ret_1 != 0)
 			RTE_LOG(ERR, EAL,
 			"Failed to set thread name for interrupt handling\n");
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..d9188fd 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
@@ -394,7 +394,7 @@ pci_vfio_mp_sync_setup(void)
 
 	/* 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);
+	ret = rte_thread_setname(socket_thread, thread_name);
 	if (ret)
 		RTE_LOG(ERR, EAL,
 			"Failed to set thread name for secondary processes!\n");
diff --git a/lib/librte_eal/linuxapp/eal/eal_timer.c b/lib/librte_eal/linuxapp/eal/eal_timer.c
index e0642de..9ceff33 100644
--- a/lib/librte_eal/linuxapp/eal/eal_timer.c
+++ b/lib/librte_eal/linuxapp/eal/eal_timer.c
@@ -219,7 +219,7 @@ rte_eal_hpet_init(int make_default)
 	 * 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);
+	ret = rte_thread_setname(msb_inc_thread_id, thread_name);
 	if (ret != 0)
 		RTE_LOG(ERR, EAL,
 			"ERROR: Cannot set HPET timer thread name!\n");
-- 
2.5.0

Re: [PATCH] eal: fix compile error for old glibc caused by pthread_setname_np()#

[Bruce Richardson]

On Thu, Nov 19, 2015 at 05:44:26PM +0000, Ferruh Yigit wrote:
> Fixes: 67b6d3039e9e ("eal: set name to threads")
> 
> pthread_setname_np() function added in glibc 2.12, using this function
> in older glibc versions cause compile error:
> error: implicit declaration of function "pthread_setname_np"
> 
> This patch adds "rte_thread_setname" macro and set it according
> glibc >= 2.12 check, thread naming disabled for older glibc versions,
> glibc versions that support "pthread_setname_np" will keep using this
> function.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
>  examples/tep_termination/main.c                    | 2 +-
>  examples/vhost/main.c                              | 2 +-
>  examples/vhost_xen/main.c                          | 2 +-
>  lib/librte_eal/common/eal_thread.h                 | 6 ++++++
>  lib/librte_eal/linuxapp/eal/eal.c                  | 2 +-
>  lib/librte_eal/linuxapp/eal/eal_interrupts.c       | 2 +-
>  lib/librte_eal/linuxapp/eal/eal_pci_vfio_mp_sync.c | 2 +-
>  lib/librte_eal/linuxapp/eal/eal_timer.c            | 2 +-
>  8 files changed, 13 insertions(+), 7 deletions(-)
>
I only see changes to linux files above. Does this not also have an implication
for bsd too?

	/Bruce

Re: [PATCH] eal: fix compile error for old glibc caused by CLOCK_MONOTONIC_RAW

[Thomas Monjalon]

2015-11-19 11:23, Ferruh Yigit:
> Fixes: d08d304508a8 ("eal/linux: make alarm not affected by system time
> jump")
> 
> CLOCK_MONOTONIC_RAW added in glibc 2.12, using this define in older
> glibc versions cause compile error:
> 'error: identifier "CLOCK_MONOTONIC_RAW" is undefined'
> 
> This patch replaces "CLOCK_MONOTONIC_RAW" with "CLOCK_MONOTONIC" for
> older glibc versions, versions that support "CLOCK_MONOTONIC_RAW"
> will keep using this clock type.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied, thanks

Re: [PATCH] eal: fix compile error for old glibc caused by pthread_setname_np()#

[Ferruh Yigit]

On Fri, Nov 20, 2015 at 12:21:05PM +0000, Bruce Richardson wrote:
> On Thu, Nov 19, 2015 at 05:44:26PM +0000, Ferruh Yigit wrote:
> > Fixes: 67b6d3039e9e ("eal: set name to threads")
> > 
> > pthread_setname_np() function added in glibc 2.12, using this function
> > in older glibc versions cause compile error:
> > error: implicit declaration of function "pthread_setname_np"
> > 
> > This patch adds "rte_thread_setname" macro and set it according
> > glibc >= 2.12 check, thread naming disabled for older glibc versions,
> > glibc versions that support "pthread_setname_np" will keep using this
> > function.
> > 
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > ---
> >  examples/tep_termination/main.c                    | 2 +-
> >  examples/vhost/main.c                              | 2 +-
> >  examples/vhost_xen/main.c                          | 2 +-
> >  lib/librte_eal/common/eal_thread.h                 | 6 ++++++
> >  lib/librte_eal/linuxapp/eal/eal.c                  | 2 +-
> >  lib/librte_eal/linuxapp/eal/eal_interrupts.c       | 2 +-
> >  lib/librte_eal/linuxapp/eal/eal_pci_vfio_mp_sync.c | 2 +-
> >  lib/librte_eal/linuxapp/eal/eal_timer.c            | 2 +-
> >  8 files changed, 13 insertions(+), 7 deletions(-)
> >
> I only see changes to linux files above. Does this not also have an implication
> for bsd too?
> 
BSD is not effected from this defect since it doesn't use glibc, 
equivalent BSD libc function pthread_set_name_np() is 10+ years old...

thanks,
ferruh

Re: [PATCH] eal: fix compile error for old glibc caused by pthread_setname_np()

[Thomas Monjalon]

2015-11-19 17:44, Ferruh Yigit:
>  examples/tep_termination/main.c                    | 2 +-
>  examples/vhost/main.c                              | 2 +-
>  examples/vhost_xen/main.c                          | 2 +-
>  lib/librte_eal/common/eal_thread.h                 | 6 ++++++

It cannot compile because eal_thread.h is not part of the public API
so not included in the examples.
Please could you move it in lib/librte_eal/common/include?
An idea of the filename it should be?

Re: [PATCH] eal: fix compile error for old glibc caused by pthread_setname_np()

[Ferruh Yigit]

On Tue, Nov 24, 2015 at 06:54:15PM +0100, Thomas Monjalon wrote:
> 2015-11-19 17:44, Ferruh Yigit:
> >  examples/tep_termination/main.c                    | 2 +-
> >  examples/vhost/main.c                              | 2 +-
> >  examples/vhost_xen/main.c                          | 2 +-
> >  lib/librte_eal/common/eal_thread.h                 | 6 ++++++
> 
> It cannot compile because eal_thread.h is not part of the public API
> so not included in the examples.

My bad.

> Please could you move it in lib/librte_eal/common/include?
> An idea of the filename it should be?

Is rte_lcore.h good? It already has rte_thread_* APIs in it.

ferruh

[PATCH v2] eal: fix compile error for old glibc caused by pthread_setname_np()

[Ferruh Yigit]

Fixes: 67b6d3039e9e ("eal: set name to threads")

pthread_setname_np() function added in glibc 2.12, using this function
in older glibc versions cause compile error:
error: implicit declaration of function "pthread_setname_np"

This patch adds "rte_thread_setname" macro and set it according
glibc >= 2.12 check, thread naming disabled for older glibc versions,
glibc versions that support "pthread_setname_np" will keep using this
function.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 examples/tep_termination/main.c                    |  2 +-
 examples/vhost/main.c                              |  2 +-
 examples/vhost_xen/main.c                          |  2 +-
 lib/librte_eal/common/include/rte_lcore.h          | 20 ++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal.c                  |  2 +-
 lib/librte_eal/linuxapp/eal/eal_interrupts.c       |  2 +-
 lib/librte_eal/linuxapp/eal/eal_pci_vfio_mp_sync.c |  2 +-
 lib/librte_eal/linuxapp/eal/eal_timer.c            |  2 +-
 8 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/examples/tep_termination/main.c b/examples/tep_termination/main.c
index 2b67e64..f97d552 100644
--- a/examples/tep_termination/main.c
+++ b/examples/tep_termination/main.c
@@ -1249,7 +1249,7 @@ main(int argc, char *argv[])
 		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);
+		ret = rte_thread_setname(tid, thread_name);
 		if (ret != 0)
 			RTE_LOG(ERR, VHOST_CONFIG, "Cannot set print-stats name\n");
 	}
diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index c081b18..9bfda6d 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -3027,7 +3027,7 @@ main(int argc, char *argv[])
 
 		/* Set thread_name for aid in debugging.  */
 		snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "print-stats");
-		ret = pthread_setname_np(tid, thread_name);
+		ret = rte_thread_setname(tid, thread_name);
 		if (ret != 0)
 			RTE_LOG(ERR, VHOST_CONFIG,
 				"Cannot set print-stats name\n");
diff --git a/examples/vhost_xen/main.c b/examples/vhost_xen/main.c
index 3fcc138..ca725f2 100644
--- a/examples/vhost_xen/main.c
+++ b/examples/vhost_xen/main.c
@@ -1510,7 +1510,7 @@ main(int argc, char *argv[])
 
 		/* 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);
+		ret = rte_thread_setname(tid, thread_name);
 		if (ret != 0)
 			RTE_LOG(ERR, VHOST_CONFIG,
 				"Cannot set print-stats name\n");
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index e03264e..25460b9 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -247,6 +247,26 @@ int rte_thread_set_affinity(rte_cpuset_t *cpusetp);
  */
 void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
 
+/**
+ * Set thread names.
+ *
+ * Macro to wrap `pthread_setname_np()` with a glibc version check.
+ * Only glibc >= 2.12 supports this feature.
+ *
+ * This macro only used for Linux, BSD does direct libc call.
+ * BSD libc version of function is `pthread_set_name_np()`.
+ */
+#if defined(__DOXYGEN__)
+#define rte_thread_setname(...) pthread_setname_np(__VA_ARGS__)
+#endif
+
+#if defined(__GLIBC__) && defined(__GLIBC_PREREQ)
+#if __GLIBC_PREREQ(2, 12)
+#define rte_thread_setname(...) pthread_setname_np(__VA_ARGS__)
+#else
+#define rte_thread_setname(...) 0
+#endif
+#endif
 
 #ifdef __cplusplus
 }
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 06536f2..635ec36 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -859,7 +859,7 @@ rte_eal_init(int argc, char **argv)
 		/* 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,
+		ret = rte_thread_setname(lcore_config[i].thread_id,
 						thread_name);
 		if (ret != 0)
 			RTE_LOG(ERR, EAL,
diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index 95beb4c..470d6a1 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -887,7 +887,7 @@ rte_eal_intr_init(void)
 		/* 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);
+		ret_1 = rte_thread_setname(intr_thread, thread_name);
 		if (ret_1 != 0)
 			RTE_LOG(ERR, EAL,
 			"Failed to set thread name for interrupt handling\n");
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..d9188fd 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
@@ -394,7 +394,7 @@ pci_vfio_mp_sync_setup(void)
 
 	/* 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);
+	ret = rte_thread_setname(socket_thread, thread_name);
 	if (ret)
 		RTE_LOG(ERR, EAL,
 			"Failed to set thread name for secondary processes!\n");
diff --git a/lib/librte_eal/linuxapp/eal/eal_timer.c b/lib/librte_eal/linuxapp/eal/eal_timer.c
index e0642de..9ceff33 100644
--- a/lib/librte_eal/linuxapp/eal/eal_timer.c
+++ b/lib/librte_eal/linuxapp/eal/eal_timer.c
@@ -219,7 +219,7 @@ rte_eal_hpet_init(int make_default)
 	 * 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);
+	ret = rte_thread_setname(msb_inc_thread_id, thread_name);
 	if (ret != 0)
 		RTE_LOG(ERR, EAL,
 			"ERROR: Cannot set HPET timer thread name!\n");
-- 
2.5.0

Re: [PATCH v2] eal: fix compile error for old glibc caused by pthread_setname_np()

[Thomas Monjalon]

2015-11-25 11:13, Ferruh Yigit:
> +/**
> + * Set thread names.
> + *
> + * Macro to wrap `pthread_setname_np()` with a glibc version check.
> + * Only glibc >= 2.12 supports this feature.
> + *
> + * This macro only used for Linux, BSD does direct libc call.
> + * BSD libc version of function is `pthread_set_name_np()`.
> + */
> +#if defined(__DOXYGEN__)
> +#define rte_thread_setname(...) pthread_setname_np(__VA_ARGS__)
> +#endif
> +
> +#if defined(__GLIBC__) && defined(__GLIBC_PREREQ)
> +#if __GLIBC_PREREQ(2, 12)
> +#define rte_thread_setname(...) pthread_setname_np(__VA_ARGS__)
> +#else
> +#define rte_thread_setname(...) 0
> +#endif
> +#endif

OK it is a first (and important) fix.
EAL is an abstraction for Linux and FreeBSD, so ideally another patch would
make rte_thread_setname working for BSD too.

Re: [PATCH v2] eal: fix compile error for old glibc caused by pthread_setname_np()

[Ferruh Yigit]

On Wed, Nov 25, 2015 at 12:18:02PM +0100, Thomas Monjalon wrote:
> 2015-11-25 11:13, Ferruh Yigit:
> > +/**
> > + * Set thread names.
> > + *
> > + * Macro to wrap `pthread_setname_np()` with a glibc version check.
> > + * Only glibc >= 2.12 supports this feature.
> > + *
> > + * This macro only used for Linux, BSD does direct libc call.
> > + * BSD libc version of function is `pthread_set_name_np()`.
> > + */
> > +#if defined(__DOXYGEN__)
> > +#define rte_thread_setname(...) pthread_setname_np(__VA_ARGS__)
> > +#endif
> > +
> > +#if defined(__GLIBC__) && defined(__GLIBC_PREREQ)
> > +#if __GLIBC_PREREQ(2, 12)
> > +#define rte_thread_setname(...) pthread_setname_np(__VA_ARGS__)
> > +#else
> > +#define rte_thread_setname(...) 0
> > +#endif
> > +#endif
> 
> OK it is a first (and important) fix.
> EAL is an abstraction for Linux and FreeBSD, so ideally another patch would
> make rte_thread_setname working for BSD too.

I wasn't sure to do that update, since BSD has nothing with glibc versions.
Do you want me amend this patch to update BSD usage to rte_thread_setname?

thanks,
ferruh

Re: [PATCH v2] eal: fix compile error for old glibc caused by pthread_setname_np()

[Thomas Monjalon]

2015-11-25 11:24, Ferruh Yigit:
> On Wed, Nov 25, 2015 at 12:18:02PM +0100, Thomas Monjalon wrote:
> > 2015-11-25 11:13, Ferruh Yigit:
> > > +/**
> > > + * Set thread names.
> > > + *
> > > + * Macro to wrap `pthread_setname_np()` with a glibc version check.
> > > + * Only glibc >= 2.12 supports this feature.
> > > + *
> > > + * This macro only used for Linux, BSD does direct libc call.
> > > + * BSD libc version of function is `pthread_set_name_np()`.
> > > + */
> > > +#if defined(__DOXYGEN__)
> > > +#define rte_thread_setname(...) pthread_setname_np(__VA_ARGS__)
> > > +#endif
> > > +
> > > +#if defined(__GLIBC__) && defined(__GLIBC_PREREQ)
> > > +#if __GLIBC_PREREQ(2, 12)
> > > +#define rte_thread_setname(...) pthread_setname_np(__VA_ARGS__)
> > > +#else
> > > +#define rte_thread_setname(...) 0
> > > +#endif
> > > +#endif
> > 
> > OK it is a first (and important) fix.
> > EAL is an abstraction for Linux and FreeBSD, so ideally another patch would
> > make rte_thread_setname working for BSD too.
> 
> I wasn't sure to do that update, since BSD has nothing with glibc versions.
> Do you want me amend this patch to update BSD usage to rte_thread_setname?

No, for 2.2, you have fixed the glibc issue, that's enough.
BSD support can be another path as it is not a real issue currently.

Re: [PATCH v2] eal: fix compile error for old glibc caused by pthread_setname_np()

[Thomas Monjalon]

2015-11-25 11:13, Ferruh Yigit:
> Fixes: 67b6d3039e9e ("eal: set name to threads")
> 
> pthread_setname_np() function added in glibc 2.12, using this function
> in older glibc versions cause compile error:
> error: implicit declaration of function "pthread_setname_np"
> 
> This patch adds "rte_thread_setname" macro and set it according
> glibc >= 2.12 check, thread naming disabled for older glibc versions,
> glibc versions that support "pthread_setname_np" will keep using this
> function.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied as eal/linux fix, thanks

Re: [PATCH v2] eal: fix compile error for old glibc caused by pthread_setname_np()

[Roger B. Melton]

> +/**
> + * Set thread names.
> + *
> + * Macro to wrap `pthread_setname_np()` with a glibc version check.
> + * Only glibc >= 2.12 supports this feature.
> + *
> + * This macro only used for Linux, BSD does direct libc call.
> + * BSD libc version of function is `pthread_set_name_np()`.
> + */
> +#if defined(__DOXYGEN__)
> +#define rte_thread_setname(...) pthread_setname_np(__VA_ARGS__)
> +#endif
> +
> +#if defined(__GLIBC__) && defined(__GLIBC_PREREQ)
> +#if __GLIBC_PREREQ(2, 12)
> +#define rte_thread_setname(...) pthread_setname_np(__VA_ARGS__)
> +#else
> +#define rte_thread_setname(...) 0
> +#endif
> +#endif

Have you thought about a way to set thread name when glibc < 2.12.  I 
also ran into the problem recently and played around with prctl() 
(Linux) to set thread (process) name.  e.g.

    ret = prctl(PR_SET_NAME,<thread_name>,0,0,0);


There are 2 issues I think:

    1) The semantics are different than prthread_setname_np().  With
    pthread_setname_np() a name can be assigned to any thread, with
    prctl() the name is assigned to the active thread.  That would mean
    that rather than rte_eal_init(), rte_eal_intr_init() could not
    assign thread names.  Rather the threads would have to name themselves.

    2) I think BSD lacks prctl(), but some (not all?) BSD
    implementations have setproctitle() to do the same thing.


It might be too late for 2.2, but something to think about for the future.

Regards,
Roger

Re: [PATCH v2] eal: fix compile error for old glibc caused by pthread_setname_np()

[Thomas Monjalon]

2015-11-25 08:51, Roger B. Melton:
> Have you thought about a way to set thread name when glibc < 2.12.  I 
> also ran into the problem recently and played around with prctl() 
> (Linux) to set thread (process) name.  e.g.
> 
>     ret = prctl(PR_SET_NAME,<thread_name>,0,0,0);
> 
> 
> There are 2 issues I think:
> 
>     1) The semantics are different than prthread_setname_np().  With
>     pthread_setname_np() a name can be assigned to any thread, with
>     prctl() the name is assigned to the active thread.  That would mean
>     that rather than rte_eal_init(), rte_eal_intr_init() could not
>     assign thread names.  Rather the threads would have to name themselves.
> 
>     2) I think BSD lacks prctl(), but some (not all?) BSD
>     implementations have setproctitle() to do the same thing.
> 
> 
> It might be too late for 2.2, but something to think about for the future.

I don't think this feature is important enough to deal with old environments
and to risk some complicated bugs.
Do you think it deserves more tricks?

Re: [PATCH v2] eal: fix compile error for old glibc caused by pthread_setname_np()

[Roger B. Melton]

On 11/25/15 9:03 AM, Thomas Monjalon wrote:
> 2015-11-25 08:51, Roger B. Melton:
>> Have you thought about a way to set thread name when glibc < 2.12.  I
>> also ran into the problem recently and played around with prctl()
>> (Linux) to set thread (process) name.  e.g.
>>
>>      ret = prctl(PR_SET_NAME,<thread_name>,0,0,0);
>>
>>
>> There are 2 issues I think:
>>
>>      1) The semantics are different than prthread_setname_np().  With
>>      pthread_setname_np() a name can be assigned to any thread, with
>>      prctl() the name is assigned to the active thread.  That would mean
>>      that rather than rte_eal_init(), rte_eal_intr_init() could not
>>      assign thread names.  Rather the threads would have to name themselves.
>>
>>      2) I think BSD lacks prctl(), but some (not all?) BSD
>>      implementations have setproctitle() to do the same thing.
>>
>>
>> It might be too late for 2.2, but something to think about for the future.
> I don't think this feature is important enough to deal with old environments
> and to risk some complicated bugs.
> Do you think it deserves more tricks?
> .
>
I agree with you Thomas.  While I am one of those living in an old 
environment, I believe that the complications of the tricks out weight 
the debug benefit.  However there may be other in the community who have 
a different view, so I thought I would at least suggest that there are 
alternatives.

Thanks,
-Roger