Re: [uclibc-ng-devel] [PATCH] remove PID caching - preview

Hi Waldemar,

On Wed, Jan 25, 2017 at 07:02:53AM +0100, Waldemar Brodkorb wrote:

I would like to suggest following changeset, any opinions?

Follow GNU C Library from c579f48edba88380635ab98cb612030e3ed8691e and remove the PID caching. These simplifies the architecture specific assembly code. The run of the test suite found no regressions, it even solves some of the test failures for x86/x86_64/sparc.

Signed-off-by: Waldemar Brodkorb wbx@openadk.org

Looks good to me, but there's a couple of questions below.

[...]

diff --git a/libpthread/nptl/descr.h b/libpthread/nptl/descr.h index 3fda681..30a4f4b 100644 --- a/libpthread/nptl/descr.h +++ b/libpthread/nptl/descr.h @@ -153,7 +153,7 @@ struct pthread pid_t tid;

/* Process ID - thread group ID in kernel speak. */

• pid_t pid;

• pid_t pid_unused;

Why is it left here?

/* List of robust mutexes the thread is holding. */ #ifdef __PTHREAD_MUTEX_HAVE_PREV

[...]

diff --git a/libpthread/nptl/sysdeps/unix/sysv/linux/raise.c b/libpthread/nptl/sysdeps/unix/sysv/linux/raise.c index 6b15938..b168c15 100644 --- a/libpthread/nptl/sysdeps/unix/sysv/linux/raise.c +++ b/libpthread/nptl/sysdeps/unix/sysv/linux/raise.c @@ -26,31 +26,27 @@ int raise (int sig) {

• struct pthread *pd = THREAD_SELF;
• pid_t pid = THREAD_GETMEM (pd, pid);
• pid_t selftid = THREAD_GETMEM (pd, tid);
• if (selftid == 0)
• {

•  /* This system call is not supposed to fail.  */
  

-#ifdef INTERNAL_SYSCALL

•  INTERNAL_SYSCALL_DECL (err);
  

•  selftid = INTERNAL_SYSCALL (gettid, err, 0);
  

-#else

•  selftid = INLINE_SYSCALL (gettid, 0);
  

-#endif

•  THREAD_SETMEM (pd, tid, selftid);
  

•  /* We do not set the PID field in the TID here since we might be
  

• called from a signal handler while the thread executes fork.  */
  

•  pid = selftid;
  

• }
• else
• /* raise is an async-safe function. It could be called while the

•   fork/vfork function temporarily invalidated the PID field.  Adjust for
  

•   that.  */
  

• if (__builtin_expect (pid <= 0, 0))

•  pid = (pid & INT_MAX) == 0 ? selftid : -pid;
  

• return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);

• /* rt_sigprocmask may fail if:

• 1. sigsetsize != sizeof (sigset_t) (EINVAL)
  

• 2. a failure in copy from/to user space (EFAULT)
  

• 3. an invalid 'how' operation (EINVAL)
  

• The first case is already handle in glibc syscall call by using the arch
  

• defined _NSIG.  Second case is handled by using a stack allocated mask.
  

• The last one should be handled by the block/unblock functions.  */
  

• //sigset_t set;
• //__libc_signal_block_app (&set);

Why adding commented out code instead of using e.g. sigprocmask?

• INTERNAL_SYSCALL_DECL (err);
• pid_t pid = INTERNAL_SYSCALL (getpid, err, 0);
• pid_t tid = INTERNAL_SYSCALL (gettid, err, 0);
• int ret = INLINE_SYSCALL (tgkill, 3, pid, tid, sig);
• //__libc_signal_restore_set (&set);
• return ret;

} libc_hidden_def (raise)