Message ID | 20180801131913.6576-3-mac@mcrowe.com |
---|---|
State | New |
Headers | show |
Series | std::future::wait_* improvements | expand |
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.
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); + if (errno == ETIMEDOUT) + return false; + if (errno == ENOSYS) + { + futex_clock_realtime_unavailable.store(true, std::memory_order_relaxed); + // Fall through to legacy implementation if the system + // call is unavailable. + } + else + return true; + } + else + return true; + } + + // We only get to here if futex_clock_realtime_unavailable was + // true or has just been set to true. struct timeval tv; gettimeofday (&tv, NULL); // Convert the absolute timeout value to a relative timeout