diff mbox series

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

Message ID 20180801131913.6576-3-mac@mcrowe.com
State New
Headers show
Series std::future::wait_* improvements | expand

Commit Message

Mike Crowe Aug. 1, 2018, 1:19 p.m. UTC
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(+)

--
2.11.0

BrightSign considers your privacy to be very important. The emails you send to us will be protected and secured. Furthermore, we will only use your email and contact information for the reasons you sent them to us and for tracking how effectively we respond to your requests.

Comments

Jonathan Wakely Aug. 1, 2018, 7:57 p.m. UTC | #1
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 mbox series

Patch

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