|
The fix for bug #15368 was unnecessarily Linux-specific. To recap,
POSIX specifies raise to be async-signal-safe, but also specifies it
to be equivalent to pthread_kill(pthread_self(), sig), which is not
an async-signal-safe sequence of operations; a signal handler could
run in between pthread_self and pthread_kill, and do something (such
as calling fork, which is also async-signal-safe) that would invalidate
the thread descriptor. This is even true in the hypothetical case of
a port that doesn't implement multithreading: kill(getpid(), sig) will
fire the signal twice if a signal handler runs in between, calls fork,
and then returns on both sides of the fork. I don't see anything in
the standards to forbid that.
The Linux-specific fix was to override the definitions of raise in
both libpthread and libc to the same unitary function that blocks
signals, retrieves TID and PID directly from the kernel, calls tgkill,
and only then unblocks signals. This patch generalizes that to any
port: pthread_kill is moved from libpthread to libc, with a forwarding
stub left behind. The definition of raise in libpthread is also
replaced with a forwarding stub. The Linux-specific definition of
raise is deleted; those ports will now use sysdeps/pthread/raise.c,
which blocks signals first, then calls pthread_self and pthread_kill,
and then unblocks signals. Similarly, sysdeps/posix/raise.c (which
would be used on a port that didn't implement multithreading) blocks
signals, calls getpid and kill, and then unblocks signals. Thus,
ports need only implement the primitives correctly and do not need to
worry about making raise async-signal-safe.
The only wrinkle was that up till now, we did not bother initializing
the ->tid field of the initial thread's descriptor unless libpthread
was loaded; now that raise calls pthread_kill even in a single-
threaded environment, that won't fly. This is abstractly easy to fix;
the tricky part was figuring out _where_ to put the calls (two of
them, as it happens) to __pthread_initialize_pids, and I'd appreciate
careful eyes on those changes.
You might be wondering why it's safe to rely on the TID in the thread
descriptor, rather than calling gettid directly. Since all signals
are blocked from before calling pthread_self until after pthread_kill
uses the TID to call tgkill, the question is whether some _other_
thread could do something that would invalidate the calling thread's
descriptor, and I believe there is no such thing.
While I was at it I fixed another bug: raise was returning an error
code on failure (like pthread_kill does) instead of setting errno as
specified. This is user-visible but I don't think it's worth recording
as a fixed bug, nobody bothers checking whether raise failed anyway.
* nptl/pt-raise.c
* sysdeps/unix/sysv/linux/pt-raise.c
* sysdeps/unix/sysv/linux/raise.c:
Remove file.
* sysdeps/unix/sysv/linux/pthread_kill.c: Use __is_internal_signal
to check for forbidden signals. Use INTERNAL_SYSCALL_CALL to call
getpid. Provide __libc_pthread_kill, with __pthread_kill as
strong alias and pthread_kill as weak alias.
* sysdeps/posix/raise.c: Block signals around the calls to
__getpid and __kill. Provide __libc_raise, with raise as strong
alias, libc_hidden_def for raise, and gsignal as weak alias.
* sysdeps/pthread/raise.c: New file. Implement by blocking
signals, calling pthread_self and pthread_kill, and then
unblocking signals again. Provide same symbols as above.
* sysdeps/generic/internal-signals.h: Define all of the same
functions that sysdeps/unix/sysv/linux/internal-signals.h does,
with sensible default definitions.
* sysdeps/unix/sysv/linux/internal-signals.h: Clarify comments.
* nptl/pthread_kill.c: Define __libc_pthread_kill, with
__pthread_kill as strong alias and pthread_kill as weak alias.
* nptl/pthread_self.c: Define __pthread_self, with
pthread_self as weak alias.
* signal/raise.c: Define __libc_raise, with raise as strong alias,
libc_hidden_def for raise, and gsignal as weak alias.
* nptl/Makefile: Move pthread_kill from libpthread-routines to
routines. Remove pt-raise from libpthread-routines.
* nptl/Versions (libc/GLIBC_2.28): Add pthread_kill.
(libc/GLIBC_PRIVATE): Add __libc_pthread_kill and __libc_raise.
* sysdeps/generic/pt-compat-stubs.S: Add stubs for raise and
pthread_kill.
* nptl/nptl-init.c (__pthread_initialize_minimal_internal):
Don't call __pthread_initialize_pids here.
* csu/libc-tls.c (__libc_setup_tls):
Call __pthread_initialize_pids after all other setup.
* elf/rtld.c (init_tls): Likewise.
* include/pthreadP.h: New forwarder.
* include/pthread.h: Add multiple inclusion guard. Declare
__pthread_self.
* include/signal.h: Declare __pthread_kill.
* sysdeps/**/libc.abilist (GLIBC_2.28): Add pthread_kill.
|
|
This patch remove the PID cache and usage in current GLIBC code. Current
usage is mainly used a performance optimization to avoid the syscall,
however it adds some issues:
- The exposed clone syscall will try to set pid/tid to make the new
thread somewhat compatible with current GLIBC assumptions. This cause
a set of issue with new workloads and usecases (such as BZ#17214 and
[1]) as well for new internal usage of clone to optimize other algorithms
(such as clone plus CLONE_VM for posix_spawn, BZ#19957).
- The caching complexity also added some bugs in the past [2] [3] and
requires more effort of each port to handle such requirements (for
both clone and vfork implementation).
- Caching performance gain in mainly on getpid and some specific
code paths. The getpid performance leverage is questionable [4],
either by the idea of getpid being a hotspot as for the getpid
implementation itself (if it is indeed a justifiable hotspot a
vDSO symbol could let to a much more simpler solution).
Other usage is mainly for non usual code paths, such as pthread
cancellation signal and handling.
For thread creation (on stack allocation) the code simplification in fact
adds some performance gain due the no need of transverse the stack cache
and invalidate each element pid.
Other thread usages will require a direct getpid syscall, such as
cancellation/setxid signal, thread cancellation, thread fail path (at
create_thread), and thread signal (pthread_kill and pthread_sigqueue).
However these are hardly usual hotspots and I think adding a syscall is
justifiable.
It also simplifies both the clone and vfork arch-specific implementation.
And by review each fork implementation there are some discrepancies that
this patch also solves:
- microblaze clone/vfork does not set/reset the pid/tid field
- hppa uses the default vfork implementation that fallback to fork.
Since vfork is deprecated I do not think we should bother with it.
The patch also removes the TID caching in clone. My understanding for
such semantic is try provide some pthread usage after a user program
issue clone directly (as done by thread creation with CLONE_PARENT_SETTID
and pthread tid member). However, as stated before in multiple discussions
threads, GLIBC provides clone syscalls without further supporting all this
semantics.
I ran a full make check on x86_64, x32, i686, armhf, aarch64, and powerpc64le.
For sparc32, sparc64, and mips I ran the basic fork and vfork tests from
posix/ folder (on a qemu system). So it would require further testing
on alpha, hppa, ia64, m68k, nios2, s390, sh, and tile (I excluded microblaze
because it is already implementing the patch semantic regarding clone/vfork).
[1] https://codereview.chromium.org/800183004/
[2] https://sourceware.org/ml/libc-alpha/2006-07/msg00123.html
[3] https://sourceware.org/bugzilla/show_bug.cgi?id=15368
[4] http://yarchive.net/comp/linux/getpid_caching.html
* sysdeps/nptl/fork.c (__libc_fork): Remove pid cache setting.
* nptl/allocatestack.c (allocate_stack): Likewise.
(__reclaim_stacks): Likewise.
(setxid_signal_thread): Obtain pid through syscall.
* nptl/nptl-init.c (sigcancel_handler): Likewise.
(sighandle_setxid): Likewise.
* nptl/pthread_cancel.c (pthread_cancel): Likewise.
* sysdeps/unix/sysv/linux/pthread_kill.c (__pthread_kill): Likewise.
* sysdeps/unix/sysv/linux/pthread_sigqueue.c (pthread_sigqueue):
Likewise.
* sysdeps/unix/sysv/linux/createthread.c (create_thread): Likewise.
* sysdeps/unix/sysv/linux/getpid.c: Remove file.
* nptl/descr.h (struct pthread): Change comment about pid value.
* nptl/pthread_getattr_np.c (pthread_getattr_np): Remove thread
pid assert.
* sysdeps/unix/sysv/linux/pthread-pids.h (__pthread_initialize_pids):
Do not set pid value.
* nptl_db/td_ta_thr_iter.c (iterate_thread_list): Remove thread
pid cache check.
* nptl_db/td_thr_validate.c (td_thr_validate): Likewise.
* sysdeps/aarch64/nptl/tcb-offsets.sym: Remove pid offset.
* sysdeps/alpha/nptl/tcb-offsets.sym: Likewise.
* sysdeps/arm/nptl/tcb-offsets.sym: Likewise.
* sysdeps/hppa/nptl/tcb-offsets.sym: Likewise.
* sysdeps/i386/nptl/tcb-offsets.sym: Likewise.
* sysdeps/ia64/nptl/tcb-offsets.sym: Likewise.
* sysdeps/m68k/nptl/tcb-offsets.sym: Likewise.
* sysdeps/microblaze/nptl/tcb-offsets.sym: Likewise.
* sysdeps/mips/nptl/tcb-offsets.sym: Likewise.
* sysdeps/nios2/nptl/tcb-offsets.sym: Likewise.
* sysdeps/powerpc/nptl/tcb-offsets.sym: Likewise.
* sysdeps/s390/nptl/tcb-offsets.sym: Likewise.
* sysdeps/sh/nptl/tcb-offsets.sym: Likewise.
* sysdeps/sparc/nptl/tcb-offsets.sym: Likewise.
* sysdeps/tile/nptl/tcb-offsets.sym: Likewise.
* sysdeps/x86_64/nptl/tcb-offsets.sym: Likewise.
* sysdeps/unix/sysv/linux/aarch64/clone.S: Remove pid and tid caching.
* sysdeps/unix/sysv/linux/alpha/clone.S: Likewise.
* sysdeps/unix/sysv/linux/arm/clone.S: Likewise.
* sysdeps/unix/sysv/linux/hppa/clone.S: Likewise.
* sysdeps/unix/sysv/linux/i386/clone.S: Likewise.
* sysdeps/unix/sysv/linux/ia64/clone2.S: Likewise.
* sysdeps/unix/sysv/linux/mips/clone.S: Likewise.
* sysdeps/unix/sysv/linux/nios2/clone.S: Likewise.
* sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S: Likewise.
* sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S: Likewise.
* sysdeps/unix/sysv/linux/s390/s390-32/clone.S: Likewise.
* sysdeps/unix/sysv/linux/s390/s390-64/clone.S: Likewise.
* sysdeps/unix/sysv/linux/sh/clone.S: Likewise.
* sysdeps/unix/sysv/linux/sparc/sparc32/clone.S: Likewise.
* sysdeps/unix/sysv/linux/sparc/sparc64/clone.S: Likewise.
* sysdeps/unix/sysv/linux/tile/clone.S: Likewise.
* sysdeps/unix/sysv/linux/x86_64/clone.S: Likewise.
* sysdeps/unix/sysv/linux/aarch64/vfork.S: Remove pid set and reset.
* sysdeps/unix/sysv/linux/alpha/vfork.S: Likewise.
* sysdeps/unix/sysv/linux/arm/vfork.S: Likewise.
* sysdeps/unix/sysv/linux/i386/vfork.S: Likewise.
* sysdeps/unix/sysv/linux/ia64/vfork.S: Likewise.
* sysdeps/unix/sysv/linux/m68k/clone.S: Likewise.
* sysdeps/unix/sysv/linux/m68k/vfork.S: Likewise.
* sysdeps/unix/sysv/linux/mips/vfork.S: Likewise.
* sysdeps/unix/sysv/linux/nios2/vfork.S: Likewise.
* sysdeps/unix/sysv/linux/powerpc/powerpc32/vfork.S: Likewise.
* sysdeps/unix/sysv/linux/powerpc/powerpc64/vfork.S: Likewise.
* sysdeps/unix/sysv/linux/s390/s390-32/vfork.S: Likewise.
* sysdeps/unix/sysv/linux/s390/s390-64/vfork.S: Likewise.
* sysdeps/unix/sysv/linux/sh/vfork.S: Likewise.
* sysdeps/unix/sysv/linux/sparc/sparc32/vfork.S: Likewise.
* sysdeps/unix/sysv/linux/sparc/sparc64/vfork.S: Likewise.
* sysdeps/unix/sysv/linux/tile/vfork.S: Likewise.
* sysdeps/unix/sysv/linux/x86_64/vfork.S: Likewise.
* sysdeps/unix/sysv/linux/tst-clone2.c (f): Remove direct pthread
struct access.
(clone_test): Remove function.
(do_test): Rewrite to take in consideration pid is not cached anymore.
|
