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

Max Filippov jcmvbkbc at gmail.com
Wed Jan 25 11:15:41 CET 2017


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

-- 
Thanks.
-- Max


More information about the devel mailing list