Bug 1469670

We can redirect getpid to a vDSO syscall if the kernel provides one.  I doubt that we will reintroduce the PID cache because it means that glibc needs to know about the semantics of the unshare and clone flags and reset the PID cache for the right flag combinations (an unconditional reset is impossible, unfortunately).

Your fork detection use case could also be served by a new type of mapping which is zeroed on fork (which has also been suggested for fork protection of PRNGs).

I filed bug 1469757 for the kernel side of the vDSO change.

(In reply to Lennart Poettering from comment #0)
> As I commented earlier on bug 1443976:
> 
> <snip>
> Umm, this change causes major slowdowns in various bits of systemd code,
> which assumes that getpid() is fast. libsystemd uses getpid() to detect when
> processes fork, to then politely return an error from all calls that operate
> on objects that cannot be reused after fork(). This used to be dead cheap on
> all glibc versions, but suddenly becomes excessively expensive, see this
> thread:
> 
> https://lists.freedesktop.org/archives/systemd-devel/2017-July/039195.html

Yes, with glibc 2.25 it has become expensive to do getpid() in a loop, but that is really the only operation where the latency of the getpid syscall is not hidden by other system latencies.

In your case it looks like you've had a 20-30% performance regression in this specific code because of the tight getpid() loop. We understand that this is bad, and we apologize that you will have to rewrite this to get back your performance.

There is a solution to this, and if your only interest is in detecting usage after fork(), and only fork(), then you can use pthread_atfork() to set handlers that clear or reset your own cached values.

If you do not want to link against libpthread, then we can go upstream again with our Fedora-specific patch which allows pthread_atfork without needing to link against libpthread (already in Fedora for an SELinux workaround).

This solution however does not extend to all the places where such an application cache may need invalidation.

> I find the explanation "The glibc PID cache negatively interacts with
> setting up containers and namespaces." quite weak... What is "negatively
> interacts" supposed to mean?

The most difficult issue to get right is cache invalidation. Getting the wrong value from getpid() is fatal in many cases and so conservatively we have to get it right 100% of the time in all cases.

Even more difficult the cache is thread local to ensure fast access to the stored value for the thread. Thus when entering a PID namespace the cache needs to be invalidated immediately and that requires complex machinery, to signal all threads to invalidate their caches. Such synchronization machinery might now require getpid to block in all threads while caches are invalidated, and that's also terrible for performance, complexity, and maintenance costs.

The need to get the cache invalidated correctly quickly spiralled out of control over the last 5 years, and the net result is that glibc will no longer maintain a userspace cache (we had clone issues, and detached thread issues to deal with also). 

We will pursue a kernel solution, which clears a value on fork, similar in the way that CLONE_CHILD_CLEARTID clears the TID when a thread dies. This ensures that the kernel is arbiter of when the fork is complete and can tell userspace about that with a memory operation on a registered location.

Does that answer your question?

(In reply to Carlos O'Donell from comment #2)
> We will pursue a kernel solution, which clears a value on fork, similar in
> the way that CLONE_CHILD_CLEARTID clears the TID when a thread dies.

I don't think this is the right design.  I am not even sure if fork is the only way to change the PID today, and who knows what the future will bring.

(Even CLONE_CHILD_CLEARTID turned out to be the wrong design for threads because thread exit is racy as a result, and the TID races in the current implementation are actually non-conforming.  We tried to paper this over with tgkill, but that only avoids sending signals to other processes.)

(In reply to Florian Weimer from comment #3)
> (In reply to Carlos O'Donell from comment #2)
> > We will pursue a kernel solution, which clears a value on fork, similar in
> > the way that CLONE_CHILD_CLEARTID clears the TID when a thread dies.
> 
> I don't think this is the right design.  I am not even sure if fork is the
> only way to change the PID today, and who knows what the future will bring.
> 
> (Even CLONE_CHILD_CLEARTID turned out to be the wrong design for threads
> because thread exit is racy as a result, and the TID races in the current
> implementation are actually non-conforming.  We tried to paper this over
> with tgkill, but that only avoids sending signals to other processes.)

I agree that there are problems with CLONE_CHILD_CLEARTID (ownership of the affected memory becomes a problem). I did not expect we would use exactly that mechanism, my apologies if that was implied.

There needs to be a deep discussion on how we do the notification to userspace, and when and how. It won't be solved tomorrow.

Not sure I follow, but wouldn't it suffice if getpid() caching is only turned off as soon as the first clone() or unshare() are called? These calls are pretty exotic, only a small number of programs will call them, but I am sure a much higher number of apps do benefit from the getpid() caching. I mean, there's a lot of room between "keep the getpid() cache updated correctly in all cases" and "do not do any getpid() caching" — some form of "maintain the getpid() cache for the obvious cases, turn it off only as soon as things complicated"...

I mean, maintaining a simple cache that oyu just turn off eventually isn't that hard, is it? and an atomic op for doing that in a thread-safe manner is still a ton faster than doing the full syscall all the time...

> We will pursue a kernel solution, which clears a value on fork, similar in the way that CLONE_CHILD_CLEARTID clears the TID when a thread dies. This ensures that the kernel is arbiter of when the fork is complete and can tell userspace about that with a memory operation on a registered location.

> Does that answer your question?

Not really, I would have preferred if glibc wouldn't regress on this without more consideration, and not until the new kernel infrastructure for this is in place.

> Even more difficult the cache is thread local to ensure fast access to the stored value for the thread. 

Why go directly from a thread local cache to calling the syscall? You could just convert it into an process-wide cache... that should still be quicker than the syscall..

(In reply to Florian Weimer from comment #1)

> needs to know about the semantics of the unshare and clone flags and reset
> the PID cache for the right flag combinations (an unconditional reset is
> impossible, unfortunately).

Why does "unshare()" even matter here? Last time I looked it doesn't even change the calling process' PID, but defers that for the next fork() child, where you need to invalidate anyway... What is this really about?

BTW, with this change you are breaking expressly documented behaviour

http://man7.org/linux/man-pages/man2/getpid.2.html

    "Since glibc version 2.3.4, the glibc wrapper function for
     getpid() caches PIDs, so as to avoid additional system calls when
     a process calls getpid() repeatedly."

Created attachment 1297042 [details]
Workaround for efficient systemd getpid() call

Lennart,

in order to quickly fix this pressing matter, I've attached code that I believe will fix your issue. It uses a custom caching mechanism and fits well into the systemd ecosystem.

Best regards.

(In reply to Lennart Poettering from comment #7)
> Why go directly from a thread local cache to calling the syscall? You could
> just convert it into an process-wide cache... that should still be quicker
> than the syscall..

Unfortunately the community did not find there was a good risk/reward for this kind of caching given the problems we've had with the cache over the years.

Regarding giving consideration and waiting for the kernel interface to appear, yes, in the best case it's always a benefit to wait for the kernel interface to appear. Here, there were benefits to the removal too. In the communities judgement the immediate benefits outweighed waiting for the kernel interface to be finished.

(In reply to Lennart Poettering from comment #9)
> BTW, with this change you are breaking expressly documented behaviour
> 
> http://man7.org/linux/man-pages/man2/getpid.2.html
> 
>     "Since glibc version 2.3.4, the glibc wrapper function for
>      getpid() caches PIDs, so as to avoid additional system calls when
>      a process calls getpid() repeatedly."

The Linux man pages project is not canonical documentation for glibc.
This quote is also part of the non-normative NOTES.
Therefore no explicit contract has been broken.

I have submitted a patch to the linux man-pages project 
to add a note about the change in implementation behaviour.

I have updated the glibc 2.25 release notes with more explanations for
developers (https://sourceware.org/glibc/wiki/Release/2.25#pid_cache_removal).

(In reply to Lennart Poettering from comment #8)
> (In reply to Florian Weimer from comment #1)
> 
> > needs to know about the semantics of the unshare and clone flags and reset
> > the PID cache for the right flag combinations (an unconditional reset is
> > impossible, unfortunately).
> 
> Why does "unshare()" even matter here? Last time I looked it doesn't even
> change the calling process' PID, but defers that for the next fork() child,
> where you need to invalidate anyway... What is this really about?

Is pthread_atfork() a usable solution for systemd?

> Is pthread_atfork() a usable solution for systemd?

Hmpf, the pthread_atfork() API is just fucked...

There's no way to unregister an atfork handler, is there? This means we'd have to mark libsystemd somehow as RTLD_NODELETE, so that this doesn't fall apart as soon as somebody pulls in libsystemd.so as a dlopen() dep, makes use of it for a short time and then unloads it again with dlopen(). If we can't unload our pthread_atfork() handler in that case we'd cause a bad memory access on the next fork().

What a mess!

(In reply to Lennart Poettering from comment #12)
> > Is pthread_atfork() a usable solution for systemd?
> 
> Hmpf, the pthread_atfork() API is just fucked...
> 
> There's no way to unregister an atfork handler, is there? This means we'd
> have to mark libsystemd somehow as RTLD_NODELETE, so that this doesn't fall
> apart as soon as somebody pulls in libsystemd.so as a dlopen() dep, makes
> use of it for a short time and then unloads it again with dlopen(). If we
> can't unload our pthread_atfork() handler in that case we'd cause a bad
> memory access on the next fork().
> 
> What a mess!

It can be done with -Wl,-z,nodelete.

This is obviously just a workaround, and even then I think we might be able to automatically unregister such functions at dlclose() time transparently. Apparently IBM's AIX used to do this also, but we'd have to have a bulletproof way of doing that. I'm not sure of all of the consequences involved.

Even if we fixed the unregister at dlclose() you would have to wait for such a glibc to propagate out, and so you'd need nodelete flags for your DSO until you know it's OK.

(In reply to Lennart Poettering from comment #12)
> > Is pthread_atfork() a usable solution for systemd?
> 
> Hmpf, the pthread_atfork() API is just fucked...
> 
> There's no way to unregister an atfork handler, is there? This means we'd
> have to mark libsystemd somehow as RTLD_NODELETE, so that this doesn't fall
> apart as soon as somebody pulls in libsystemd.so as a dlopen() dep, makes
> use of it for a short time and then unloads it again with dlopen(). If we
> can't unload our pthread_atfork() handler in that case we'd cause a bad
> memory access on the next fork().

glibc is supposed to do this automatically.  There's a definition of pthread_atfork in libpthread_nonshared.a which passes __dso_handle to the actual implementation.  At dlclose time, we search for handlers with a matching __dso_handle and automatically unregister them.

(In reply to Florian Weimer from comment #14)
> (In reply to Lennart Poettering from comment #12)
> > > Is pthread_atfork() a usable solution for systemd?
> > 
> > Hmpf, the pthread_atfork() API is just fucked...
> > 
> > There's no way to unregister an atfork handler, is there? This means we'd
> > have to mark libsystemd somehow as RTLD_NODELETE, so that this doesn't fall
> > apart as soon as somebody pulls in libsystemd.so as a dlopen() dep, makes
> > use of it for a short time and then unloads it again with dlopen(). If we
> > can't unload our pthread_atfork() handler in that case we'd cause a bad
> > memory access on the next fork().
> 
> glibc is supposed to do this automatically.  There's a definition of
> pthread_atfork in libpthread_nonshared.a which passes __dso_handle to the
> actual implementation.  At dlclose time, we search for handlers with a
> matching __dso_handle and automatically unregister them.

Good catch. I'd missed the registration of __unregister_atfork() via the process global destructors (hidden by a macro). So it looks like we don't have anything to fix. If you dlopen a library with atfork handler then they will be removed at unload time.

Weird undocumented automatism, but OK. I figure that'll do then. Still not happy that this semi-broke all kinds of systemd, PulseAudio, libcanberra installations already out there (and even got backported to "stable" stuff), but I'll work around this now at least in libsystemd... Thanks.

This message is a reminder that Fedora 26 is nearing its end of life.
Approximately 4 (four) weeks from now Fedora will stop maintaining
and issuing updates for Fedora 26. It is Fedora's policy to close all
bug reports from releases that are no longer maintained. At that time
this bug will be closed as EOL if it remains open with a Fedora  'version'
of '26'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version'
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not
able to fix it before Fedora 26 is end of life. If you would still like
to see this bug fixed and are able to reproduce it against a later version
of Fedora, you are encouraged  change the 'version' to a later Fedora
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's
lifetime, sometimes those efforts are overtaken by events. Often a
more recent Fedora release includes newer upstream software that fixes
bugs or makes them obsolete.

The kernel doesn't provide a getpid function in the vDSO, so there is nothing that glibc can do here.