[PATCHv3 2/6] libstdc++ futex: Use FUTEX_CLOCK_REALTIME for wait

[Jonathan Wakely]

On 01/08/18 14:19 +0100, Mike Crowe wrote:
>The futex system call supports waiting for an absolute time if
>FUTEX_WAIT_BITSET is used rather than FUTEX_WAIT. Doing so provides two
>benefits:
>
>1. The call to gettimeofday is not required in order to calculate a
>   relative timeout.
>
>2. If someone changes the system clock during the wait then the futex
>   timeout will correctly expire earlier or later. Currently that only
>   happens if the clock is changed prior to the call to gettimeofday.
>
>According to futex(2), support for FUTEX_CLOCK_REALTIME was added in the
>v2.6.28 Linux kernel and FUTEX_WAIT_BITSET was added in v2.6.25. To ensure
>that the code still works correctly with earlier kernel versions, an ENOSYS
>error from futex[1] results in the futex_clock_realtime_unavailable being
>flag being set. This flag is used to avoid the unnecessary unsupported
>futex call in the future and to fall back to the previous gettimeofday and
>relative time implementation.
>
>glibc applied an equivalent switch in pthread_cond_timedwait to use
>FUTEX_CLOCK_REALTIME and FUTEX_WAIT_BITSET rather than FUTEX_WAIT for
>glibc-2.10 back in 2009. See glibc:cbd8aeb836c8061c23a5e00419e0fb25a34abee7
>
>The futex_clock_realtime_unavailable flag is accessed using
>std::memory_order_relaxed to stop it becoming a bottleneck. If the first
>two calls to _M_futex_wait_until happen to happen simultaneously then the
>only consequence is that both will try to use FUTEX_CLOCK_REALTIME, both
>risk discovering that it doesn't work and, if so, both set the flag.
>
>[1] This is how glibc's nptl-init.c determines whether these flags are
>    supported.
>---
> libstdc++-v3/src/c++11/futex.cc | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
>diff --git a/libstdc++-v3/src/c++11/futex.cc b/libstdc++-v3/src/c++11/futex.cc
>index 278a5a80902..72062a4285e 100644
>--- a/libstdc++-v3/src/c++11/futex.cc
>+++ b/libstdc++-v3/src/c++11/futex.cc
>@@ -35,8 +35,16 @@
>
> // Constants for the wait/wake futex syscall operations
> const unsigned futex_wait_op = 0;
>+const unsigned futex_wait_bitset_op = 9;
>+const unsigned futex_clock_realtime_flag = 256;
>+const unsigned futex_bitset_match_any = ~0;
> const unsigned futex_wake_op = 1;
>
>+namespace
>+{
>+  std::atomic<bool> futex_clock_realtime_unavailable;
>+}
>+
> namespace std _GLIBCXX_VISIBILITY(default)
> {
> _GLIBCXX_BEGIN_NAMESPACE_VERSION
>@@ -58,6 +66,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       }
>     else
>       {
>+       if (!futex_clock_realtime_unavailable.load(std::memory_order_relaxed))
>+         {
>+           struct timespec rt;
>+           rt.tv_sec = __s.count();
>+           rt.tv_nsec = __ns.count();
>+           if (syscall (SYS_futex, __addr, futex_wait_bitset_op | futex_clock_realtime_flag, __val, &rt, nullptr, futex_bitset_match_any) == -1)
>+             {
>+               _GLIBCXX_DEBUG_ASSERT(errno == EINTR || errno == EAGAIN
>+                                     || errno == ETIMEDOUT || errno == ENOSYS);

I've just replaced the use of _GLIBCXX_DEBUG_ASSERT in that file with
__glibcxx_assert, because the former is only used when Debug Mode is
active, and we'll never build libstdc++.so with Debug Mode.

The latter is enabled by -D_GLIBCXX_ASSERTIONS, which I've just added
to the default flags for the --enable-libstdcxx-debug option (which
builds a second copy of libstdc++.so built without optimisations).

I can make the corresponding changes to your patches, so this is just
FYI.