add pthread_set_name_np support

add pthread_set_name_np support

[Timo Myyrä]

Hi,

While browsing the emacs code I noticed that pthread_set_name_np is not
supported by emacs currently. Here's simple diff to add it.
I'm not that well versed in autoconf, probably should check pthread_set_name_np
within the pthread_setname_np block so both won't get enabled at the same time.

Also I'm not sure if name should be padded, quickly looking OpenBSD sources
didn't indicate that padding would be required. LLVM code base seems to pad name
argument to max 16 chars on FreeBSD and 32 on OpenBSD.

Thoughts?

Timo


diff --git a/configure.ac b/configure.ac
index b1b8c846e1..cfc642f72e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -4187,7 +4187,8 @@ AC_DEFUN
 sendto recvfrom getsockname getifaddrs freeifaddrs \
 gai_strerror sync \
 getpwent endpwent getgrent endgrent \
-cfmakeraw cfsetspeed __executable_start log2 pthread_setname_np)
+cfmakeraw cfsetspeed __executable_start log2 pthread_setname_np \
+pthread_set_name_np)
 LIBS=$OLD_LIBS
 
 if test "$ac_cv_func_pthread_setname_np" = "yes"; then
@@ -4222,6 +4223,23 @@ AC_DEFUN
   fi
 fi
 
+if test "$ac_cv_func_pthread_set_name_np" = "yes"; then
+  AC_CACHE_CHECK(
+   [whether pthread_set_name_np is supported],
+   [emacs_cv_pthread_set_name_np],
+   [AC_COMPILE_IFELSE(
+     [AC_LANG_PROGRAM(
+       [[#include <pthread.h>][#incule <pthread_np.h>]],
+       [[pthread_setname_np (1, "a");]])],
+     [emacs_cv_pthread_set_name_np=yes],
+     [emacs_cv_pthread_set_name_np=no])])
+  if test "$emacs_cv_pthread_set_name_np" = "yes"; then
+    AC_DEFINE(
+      HAVE_PTHREAD_SET_NAME_NP, 1,
+      [Define to 1 if pthread_set_name_np is supported.])
+  fi
+fi
+
 dnl No need to check for posix_memalign if aligned_alloc works.
 AC_CHECK_FUNCS([aligned_alloc posix_memalign], [break])
 AC_CHECK_DECLS([aligned_alloc], [], [], [[#include <stdlib.h>]])
diff --git a/src/systhread.c b/src/systhread.c
index 0d600d6895..3087f18d4c 100644
--- a/src/systhread.c
+++ b/src/systhread.c
@@ -26,6 +26,10 @@ Copyright (C) 2012-2020 Free Software Foundation, Inc.
 #include "nsterm.h"
 #endif
 
+#ifdef HAVE_PTHREAD_SET_NAME_NP
+#include "pthread_np.h"
+#endif
+
 #ifndef THREADS_ENABLED
 
 void
@@ -222,6 +226,9 @@ #define TASK_COMM_LEN 16
   pthread_setname_np (pthread_self (), p_name);
 # endif
 #endif
+#ifdef HAVE_PTHREAD_SET_NAME_NP
+  pthread_set_name_np (pthread_self (), name);
+#endif
 }
 
 bool

Re: add pthread_set_name_np support

[Eli Zaretskii]

> From: timo.myyra@bittivirhe.fi (Timo Myyrä)
> Date: Sat, 27 Jun 2020 13:49:24 +0300
> 
> While browsing the emacs code I noticed that pthread_set_name_np is not
> supported by emacs currently. Here's simple diff to add it.
> I'm not that well versed in autoconf, probably should check pthread_set_name_np
> within the pthread_setname_np block so both won't get enabled at the same time.
> 
> Also I'm not sure if name should be padded, quickly looking OpenBSD sources
> didn't indicate that padding would be required. LLVM code base seems to pad name
> argument to max 16 chars on FreeBSD and 32 on OpenBSD.
> 
> Thoughts?

Thanks, we already have support for pthread_setname_np in what is soon
going to be released as Emacs 27.1.  Please take a look at the
emacs-27 branch of the Emacs Git repository.

Re: add pthread_set_name_np support

[Timo Myyrä]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: timo.myyra@bittivirhe.fi (Timo Myyrä)
>> Date: Sat, 27 Jun 2020 13:49:24 +0300
>> 
>> While browsing the emacs code I noticed that pthread_set_name_np is not
>> supported by emacs currently. Here's simple diff to add it.
>> I'm not that well versed in autoconf, probably should check pthread_set_name_np
>> within the pthread_setname_np block so both won't get enabled at the same time.
>> 
>> Also I'm not sure if name should be padded, quickly looking OpenBSD sources
>> didn't indicate that padding would be required. LLVM code base seems to pad name
>> argument to max 16 chars on FreeBSD and 32 on OpenBSD.
>> 
>> Thoughts?
>
> Thanks, we already have support for pthread_setname_np in what is soon
> going to be released as Emacs 27.1.  Please take a look at the
> emacs-27 branch of the Emacs Git repository.

Hi,

OpenBSD and seems that FreeBSD don't have pthread_setname_np, they use
pthread_set_name_np instead.

But I got feedback that previous diff had typos so here is a better diff.
I included the padding of max process name though it doesn't seem necessary on OpenBSD.


Timo


diff --git a/configure.ac b/configure.ac
index b1b8c846e1..f198894e02 100644
--- a/configure.ac
+++ b/configure.ac
@@ -4187,7 +4187,8 @@ AC_DEFUN
 sendto recvfrom getsockname getifaddrs freeifaddrs \
 gai_strerror sync \
 getpwent endpwent getgrent endgrent \
-cfmakeraw cfsetspeed __executable_start log2 pthread_setname_np)
+cfmakeraw cfsetspeed __executable_start log2 pthread_setname_np \
+pthread_set_name_np)
 LIBS=$OLD_LIBS
 
 if test "$ac_cv_func_pthread_setname_np" = "yes"; then
@@ -4222,6 +4223,23 @@ AC_DEFUN
   fi
 fi
 
+if test "$ac_cv_func_pthread_set_name_np" = "yes"; then
+  AC_CACHE_CHECK(
+   [whether pthread_set_name_np is supported],
+   [emacs_cv_pthread_set_name_np],
+   [AC_COMPILE_IFELSE(
+     [AC_LANG_PROGRAM(
+       [[#include <pthread.h>][#include <pthread_np.h>]],
+       [[pthread_setname_np (1, "a");]])],
+     [emacs_cv_pthread_set_name_np=yes],
+     [emacs_cv_pthread_set_name_np=no])])
+  if test "$emacs_cv_pthread_set_name_np" = "yes"; then
+    AC_DEFINE(
+      HAVE_PTHREAD_SET_NAME_NP, 1,
+      [Define to 1 if pthread_set_name_np is supported.])
+  fi
+fi
+
 dnl No need to check for posix_memalign if aligned_alloc works.
 AC_CHECK_FUNCS([aligned_alloc posix_memalign], [break])
 AC_CHECK_DECLS([aligned_alloc], [], [], [[#include <stdlib.h>]])
diff --git a/src/systhread.c b/src/systhread.c
index 0d600d6895..57005bacc3 100644
--- a/src/systhread.c
+++ b/src/systhread.c
@@ -26,6 +26,10 @@ Copyright (C) 2012-2020 Free Software Foundation, Inc.
 #include "nsterm.h"
 #endif
 
+#ifdef HAVE_PTHREAD_SET_NAME_NP
+#include <pthread_np.h>
+#endif
+
 #ifndef THREADS_ENABLED
 
 void
@@ -206,7 +210,7 @@ sys_thread_equal (sys_thread_t t, sys_thread_t u)
 void
 sys_thread_set_name (const char *name)
 {
-#ifdef HAVE_PTHREAD_SETNAME_NP
+#if defined HAVE_PTHREAD_SETNAME_NP || defined HAVE_PTHREAD_SET_NAME_NP
   /* We need to truncate here otherwise pthread_setname_np
      fails to set the name.  TASK_COMM_LEN is what the length
      is called in the Linux kernel headers (Bug#38632).  */
@@ -218,10 +222,13 @@ #define TASK_COMM_LEN 16
   pthread_setname_np (p_name);
 # elif defined HAVE_PTHREAD_SETNAME_NP_3ARG
   pthread_setname_np (pthread_self (), "%s", p_name);
+# elif HAVE_PTHREAD_SET_NAME_NP
+  pthread_set_name_np (pthread_self (), p_name);
 # else
   pthread_setname_np (pthread_self (), p_name);
 # endif
 #endif
+
 }
 
 bool

Re: add pthread_set_name_np support

[Mattias Engdegård]

28 juni 2020 kl. 07.07 skrev Timo Myyrä <timo.myyra@bittivirhe.fi>:

> OpenBSD removed /proc interface ages ago. I tried to look if htop / top would
> list thread names but I didn't succeed in them. 

Very well -- I'm not sure if the thread name even makes it into the kernel, but at that point it's no longer our problem.
Thank you, pushed to master. Any serious error in the final patch is mine.

Re: add pthread_set_name_np support

[Robert Pluim]

>>>>> On Sat, 27 Jun 2020 15:51:50 +0300, timo.myyra@bittivirhe.fi (Timo Myyrä) said:

    Timo> diff --git a/configure.ac b/configure.ac
    Timo> index b1b8c846e1..f198894e02 100644
    Timo> --- a/configure.ac
    Timo> +++ b/configure.ac
    Timo> @@ -4187,7 +4187,8 @@ AC_DEFUN
    Timo>  sendto recvfrom getsockname getifaddrs freeifaddrs \
    Timo>  gai_strerror sync \
    Timo>  getpwent endpwent getgrent endgrent \
    Timo> -cfmakeraw cfsetspeed __executable_start log2 pthread_setname_np)
    Timo> +cfmakeraw cfsetspeed __executable_start log2 pthread_setname_np \
    Timo> +pthread_set_name_np)
    Timo>  LIBS=$OLD_LIBS
 
    Timo>  if test "$ac_cv_func_pthread_setname_np" = "yes"; then
    Timo> @@ -4222,6 +4223,23 @@ AC_DEFUN
    Timo>    fi
    Timo>  fi

Iʼm confused; why do you need the explicit check for
pthread_set_name_np when the AC_CHECK_FUNCS call above already checks
for it (and sets HAVE_PTHREAD_SET_NAME_NP if successful).

    Timo> +if test "$ac_cv_func_pthread_set_name_np" = "yes"; then
    Timo> +  AC_CACHE_CHECK(
    Timo> +   [whether pthread_set_name_np is supported],
    Timo> +   [emacs_cv_pthread_set_name_np],
    Timo> +   [AC_COMPILE_IFELSE(
    Timo> +     [AC_LANG_PROGRAM(
    Timo> +       [[#include <pthread.h>][#include <pthread_np.h>]],
    Timo> +       [[pthread_setname_np (1, "a");]])],
    Timo> +     [emacs_cv_pthread_set_name_np=yes],
    Timo> +     [emacs_cv_pthread_set_name_np=no])])
    Timo> +  if test "$emacs_cv_pthread_set_name_np" = "yes"; then
    Timo> +    AC_DEFINE(
    Timo> +      HAVE_PTHREAD_SET_NAME_NP, 1,
    Timo> +      [Define to 1 if pthread_set_name_np is supported.])
    Timo> +  fi
    Timo> +fi
    Timo> +

Re: add pthread_set_name_np support

[Philipp Stephani]

Am Sa., 27. Juni 2020 um 13:10 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > From: timo.myyra@bittivirhe.fi (Timo Myyrä)
> > Date: Sat, 27 Jun 2020 13:49:24 +0300
> >
> > While browsing the emacs code I noticed that pthread_set_name_np is not
> > supported by emacs currently. Here's simple diff to add it.
> > I'm not that well versed in autoconf, probably should check pthread_set_name_np
> > within the pthread_setname_np block so both won't get enabled at the same time.
> >
> > Also I'm not sure if name should be padded, quickly looking OpenBSD sources
> > didn't indicate that padding would be required. LLVM code base seems to pad name
> > argument to max 16 chars on FreeBSD and 32 on OpenBSD.
> >
> > Thoughts?
>
> Thanks, we already have support for pthread_setname_np in what is soon
> going to be released as Emacs 27.1.  Please take a look at the
> emacs-27 branch of the Emacs Git repository.
>

This is about pthread_set_name_np, not pthread_setname_np (note the underscore).