[0/5] Make std::future::wait_* use std::chrono::steady_clock when required

Message ID 20180107205532.13138-1-mac@mcrowe.com
Headers show
Series
  • Make std::future::wait_* use std::chrono::steady_clock when required
Related show

Message

Mike Crowe Jan. 7, 2018, 8:55 p.m.
This patch series was originally submitted back in September at
https://gcc.gnu.org/ml/libstdc++/2017-09/msg00083.html which ended up
as https://patchwork.ozlabs.org/cover/817379/ . The patches received
no comments at all, which may mean that they are perfect or that they
are so bad that no-one knew where to start with criticising them. It
would be good to know which. :)

The patches are unchanged from last time, but I have corrected a typo
in one of the commit messages. The original cover message follows.

This is a first attempt to make std::future::wait_until and
std::future::wait_for make correct use of
std::chrono::steady_clock/CLOCK_MONOTONIC. It also makes
std::future::wait_until react to changes to CLOCK_REALTIME during the
wait, but only when passed a std::chrono::system_clock time point.

Prior to these patches, the situation is similar to that described at
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41861 for
std::condition_variable, but with the benefit that this time the bug can be
fixed entirely within libstdc++.

(I've omitted the ChangeLog entries for now, since I don't believe that
this is ready to be accepted in its current form.)

Mike Crowe (5): Improve libstdc++-v3 async test libstdc++ futex: Use
  FUTEX_CLOCK_REALTIME for wait libstdc++ futex: Support waiting on
  std::chrono::steady_clock directly libstdc++ atomic_futex: Use
  std::chrono::steady_clock as reference clock Extra async tests, not
  for merging

 libstdc++-v3/include/bits/atomic_futex.h         |  74 ++++++++++++++-
 libstdc++-v3/src/c++11/futex.cc                  |  48 +++++++---
 libstdc++-v3/testsuite/30_threads/async/async.cc | 112 +++++++++++++++++++++++
 3 files changed, 217 insertions(+), 17 deletions(-)

Comments

Jonathan Wakely Jan. 9, 2018, 2:43 p.m. | #1
On 07/01/18 20:55 +0000, Mike Crowe wrote:
>This patch series was originally submitted back in September at
>https://gcc.gnu.org/ml/libstdc++/2017-09/msg00083.html which ended up
>as https://patchwork.ozlabs.org/cover/817379/ . The patches received
>no comments at all, which may mean that they are perfect or that they
>are so bad that no-one knew where to start with criticising them. It
>would be good to know which. :)

Sorry for the lack of review. Although not perfect I think the patches
look good, but I didn't feel entirely confident reviewing them at the
time, and let them slip through the cracks (we could do with more
reviewers for libstdc++ patches).

I'm taking another look now, and have asked the original author of the
__atomic_futex_unsigned code to look too, so thanks for the ping.
Torvald Riegel Jan. 13, 2018, 3:29 p.m. | #2
On Sun, 2018-01-07 at 20:55 +0000, Mike Crowe wrote:
> This patch series was originally submitted back in September at
> https://gcc.gnu.org/ml/libstdc++/2017-09/msg00083.html which ended up
> as https://patchwork.ozlabs.org/cover/817379/ . The patches received
> no comments at all, which may mean that they are perfect or that they
> are so bad that no-one knew where to start with criticising them. It
> would be good to know which. :)
> 
> The patches are unchanged from last time, but I have corrected a typo
> in one of the commit messages. The original cover message follows.
> 
> This is a first attempt to make std::future::wait_until and
> std::future::wait_for make correct use of
> std::chrono::steady_clock/CLOCK_MONOTONIC. It also makes
> std::future::wait_until react to changes to CLOCK_REALTIME during the
> wait, but only when passed a std::chrono::system_clock time point.

I have comments on the design.

First, I don't think we should not change
__atomic_futex_unsigned_base::_M_futex_wait_until, as there's a risk
that we'll change behavior of existing applications that work as
expected.
Instead, ISTM we should additionally expose the two options we have at
the level of futexes:
* Relative timeout using CLOCK_MONOTONIC
* Absolute timeout using CLOCK_REALTIME (which will fall back to the
former on old kernels, which is fine I think).

Then we do the following translations from functions that programs would
call to the new futex functions:

1) wait_for is a loop in which we load the current time from the steady
clock, then call the relative futex wait, and if that returns for a
spurious reason (ie, neither timeout nor is the expected value present),
we reduce the prior relative amount by the difference between the time
before the futex wait and the current time.

2) wait_until using the steady clock is a loop similar to wait_for, just
that we additionally compute the initial relative timeout.

3) wait_until using the system clock is a loop that uses
absolute-timeout futex wait.

4) For wait_until using an unknown clock, I'd say that synching to the
system clock is the right approach.  Using wait_until indicates that the
programmer wanted to have a point in time, not a duration.

Does this work for you?

If so, could you provide a revised patch that uses this approach and
includes this approach in the documentation?
(Sorry for the lack of comments in the current code).
Mike Crowe Jan. 14, 2018, 4:08 p.m. | #3
Hi Torvald,

Thanks for reviewing this change.

On Saturday 13 January 2018 at 16:29:57 +0100, Torvald Riegel wrote:
> On Sun, 2018-01-07 at 20:55 +0000, Mike Crowe wrote:
> > This is a first attempt to make std::future::wait_until and
> > std::future::wait_for make correct use of
> > std::chrono::steady_clock/CLOCK_MONOTONIC. It also makes
> > std::future::wait_until react to changes to CLOCK_REALTIME during the
> > wait, but only when passed a std::chrono::system_clock time point.
> 
> I have comments on the design.
> 
> First, I don't think we should not change
> __atomic_futex_unsigned_base::_M_futex_wait_until, as there's a risk
> that we'll change behavior of existing applications that work as
> expected.

I assume you mean "I don't think we should change" or "I think we should
not change"... :-)

The only way I can see that behaviour will change for existing programs is
when the system clock changes (i.e. when someone calls settimeofday.) In
the existing code, the maximum wait time is fixed once gettimeofday is
called to calculate the relative timeout. When using FUTEX_CLOCK_REALTIME,
the maximum wait can change based on changes to the system clock after that
point. It appears that glibc made this transition successfully and
currently uses FUTEX_CLOCK_REALTIME. I think that the new behaviour is
better than the old behaviour.

Or perhaps I've missed another possibility. Did you have another risk in
mind?

> Instead, ISTM we should additionally expose the two options we have at
> the level of futexes:
> * Relative timeout using CLOCK_MONOTONIC
> * Absolute timeout using CLOCK_REALTIME (which will fall back to the
> former on old kernels, which is fine I think).
> 
> Then we do the following translations from functions that programs would
> call to the new futex functions:
> 
> 1) wait_for is a loop in which we load the current time from the steady
> clock, then call the relative futex wait, and if that returns for a
> spurious reason (ie, neither timeout nor is the expected value present),
> we reduce the prior relative amount by the difference between the time
> before the futex wait and the current time.

If we're going to loop on a relative timeout it sounds safer to convert it
to an absolute (steady clock) timeout. That way we won't risk increasing
the timeout if the scheduler decides not to run us at an inopportune moment
between waits. _M_load_when_equal_for already does this.

_M_load_and_test_until already has a loop for spurious wakeup. I think that
it makes sense to only loop at one level. That loop relies on the timeout
being absolute, which is why my _M_load_and_test_until_steady also uses an
absolute timeout.

> 2) wait_until using the steady clock is a loop similar to wait_for, just
> that we additionally compute the initial relative timeout.

Clearly an absolute wait can be implemented in terms of a relative one and
vice-versa, but at least in my attempts to write them I find the code
easier to understand (and therefore get right) if the fundamental wait is
the absolute one and the relative one is implemented on top of it.

> 3) wait_until using the system clock is a loop that uses
> absolute-timeout futex wait.
> 
> 4) For wait_until using an unknown clock, I'd say that synching to the
> system clock is the right approach.  Using wait_until indicates that the
> programmer wanted to have a point in time, not a duration.

For my embedded and desktop point of view, the system clock should not be
trusted, can suddenly change in any direction and doesn't necessarily help
identify a point in real time. If we assume that the non-standard clock is
advancing steadily too, then steady_clock is a better match than
system_clock.

If you have a machine that has its system clock locked with PTP to an
atomic clock then you might think the opposite. However, even in that
situation you're reliant on steady_clock being reliable enough for short
periods of time anyway, because that shares the same local clock as
system_time.

> Does this work for you?

Not yet, but maybe there's parts that I don't fully understand the
reasoning behind.

> If so, could you provide a revised patch that uses this approach and
> includes this approach in the documentation?
> (Sorry for the lack of comments in the current code).

I'm definitely willing to improve the current code rather than just add to
it.

Mike.
Mike Crowe Jan. 14, 2018, 8:44 p.m. | #4
On Sunday 14 January 2018 at 16:08:09 +0000, Mike Crowe wrote:
> Hi Torvald,
> 
> Thanks for reviewing this change.
> 
> On Saturday 13 January 2018 at 16:29:57 +0100, Torvald Riegel wrote:
> > On Sun, 2018-01-07 at 20:55 +0000, Mike Crowe wrote:
> > > This is a first attempt to make std::future::wait_until and
> > > std::future::wait_for make correct use of
> > > std::chrono::steady_clock/CLOCK_MONOTONIC. It also makes
> > > std::future::wait_until react to changes to CLOCK_REALTIME during the
> > > wait, but only when passed a std::chrono::system_clock time point.
> > 
> > I have comments on the design.
> > 
> > First, I don't think we should not change
> > __atomic_futex_unsigned_base::_M_futex_wait_until, as there's a risk
> > that we'll change behavior of existing applications that work as
> > expected.
> 
> I assume you mean "I don't think we should change" or "I think we should
> not change"... :-)
> 
> The only way I can see that behaviour will change for existing programs is
> when the system clock changes (i.e. when someone calls settimeofday.) In
> the existing code, the maximum wait time is fixed once gettimeofday is
> called to calculate the relative timeout. When using FUTEX_CLOCK_REALTIME,
> the maximum wait can change based on changes to the system clock after that
> point. It appears that glibc made this transition successfully and
> currently uses FUTEX_CLOCK_REALTIME. I think that the new behaviour is
> better than the old behaviour.
> 
> Or perhaps I've missed another possibility. Did you have another risk in
> mind?
> 
> > Instead, ISTM we should additionally expose the two options we have at
> > the level of futexes:
> > * Relative timeout using CLOCK_MONOTONIC
> > * Absolute timeout using CLOCK_REALTIME (which will fall back to the
> > former on old kernels, which is fine I think).
> > 
> > Then we do the following translations from functions that programs would
> > call to the new futex functions:
> > 
> > 1) wait_for is a loop in which we load the current time from the steady
> > clock, then call the relative futex wait, and if that returns for a
> > spurious reason (ie, neither timeout nor is the expected value present),
> > we reduce the prior relative amount by the difference between the time
> > before the futex wait and the current time.
> 
> If we're going to loop on a relative timeout it sounds safer to convert it
> to an absolute (steady clock) timeout. That way we won't risk increasing
> the timeout if the scheduler decides not to run us at an inopportune moment
> between waits. _M_load_when_equal_for already does this.
> 
> _M_load_and_test_until already has a loop for spurious wakeup. I think that
> it makes sense to only loop at one level. That loop relies on the timeout
> being absolute, which is why my _M_load_and_test_until_steady also uses an
> absolute timeout.
> 
> > 2) wait_until using the steady clock is a loop similar to wait_for, just
> > that we additionally compute the initial relative timeout.
> 
> Clearly an absolute wait can be implemented in terms of a relative one and
> vice-versa, but at least in my attempts to write them I find the code
> easier to understand (and therefore get right) if the fundamental wait is
> the absolute one and the relative one is implemented on top of it.

I had a quick go at implementing at least the first part of your design, as
I understood it. (I've kept the loops inside atomic_futex_unsigned - and I
think that you wanted to move them out to the client code.) I've not tested
it much.

I think that this implementation of _M_load_and_test_for is rather more
error-prone than my previous _M_load_and_test_until_steady. That's probably
partly because the type-safe duration has already been separated into seconds
and nanoseconds. It would be nice to push this separation as deeply as
possible in the code, but I'm afraid that would break ABI compatibility.

Thanks.

Mike.

--8<--

diff --git a/libstdc++-v3/include/bits/atomic_futex.h b/libstdc++-v3/include/bits/atomic_futex.h
index ad9437da4e2..fa4a4382c79 100644
--- a/libstdc++-v3/include/bits/atomic_futex.h
+++ b/libstdc++-v3/include/bits/atomic_futex.h
@@ -57,6 +57,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     _M_futex_wait_until(unsigned *__addr, unsigned __val, bool __has_timeout,
 	chrono::seconds __s, chrono::nanoseconds __ns);

+    // Returns false iff a timeout occurred.
+    bool
+    _M_futex_wait_for(unsigned *__addr, unsigned __val, bool __has_timeout,
+	chrono::seconds __s, chrono::nanoseconds __ns);
+
     // This can be executed after the object has been destroyed.
     static void _M_futex_notify_all(unsigned* __addr);
   };
@@ -110,6 +115,40 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	}
     }

+    // If a timeout occurs, returns a current value after the timeout;
+    // otherwise, returns the operand's value if equal is true or a different
+    // value if equal is false.
+    // The assumed value is the caller's assumption about the current value
+    // when making the call.
+    unsigned
+    _M_load_and_test_for(unsigned __assumed, unsigned __operand,
+	bool __equal, memory_order __mo, bool __has_timeout,
+	chrono::seconds __s, chrono::nanoseconds __ns)
+    {
+      auto __end_time = chrono::steady_clock::now() + chrono::seconds(__s) + chrono::nanoseconds(__ns);
+      for(;;)
+	{
+	  // Don't bother checking the value again because we expect the caller
+	  // to have done it recently.
+	  // memory_order_relaxed is sufficient because we can rely on just the
+	  // modification order (store_notify uses an atomic RMW operation too),
+	  // and the futex syscalls synchronize between themselves.
+	  _M_data.fetch_or(_Waiter_bit, memory_order_relaxed);
+	  bool __ret = _M_futex_wait_for((unsigned*)(void*)&_M_data,
+					   __assumed | _Waiter_bit,
+					   __has_timeout, __s, __ns);
+	  // Fetch the current value after waiting (clears _Waiter_bit).
+	  __assumed = _M_load(__mo);
+	  if (!__ret || ((__operand == __assumed) == __equal))
+	    return __assumed;
+
+	  const auto __remaining = __end_time - chrono::steady_clock::now();
+	  __s = chrono::duration_cast<chrono::seconds>(__remaining);
+	  __ns = chrono::duration_cast<chrono::nanoseconds>(__remaining - __s);
+	}
+      return __assumed;
+    }
+
     // Returns the operand's value if equal is true or a different value if
     // equal is false.
     // The assumed value is the caller's assumption about the current value
@@ -168,8 +207,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _M_load_when_equal_for(unsigned __val, memory_order __mo,
 	  const chrono::duration<_Rep, _Period>& __rtime)
       {
-	return _M_load_when_equal_until(__val, __mo,
-					__clock_t::now() + __rtime);
+	unsigned __i = _M_load(__mo);
+	if ((__i & ~_Waiter_bit) == __val)
+	  return true;
+	auto __s = chrono::duration_cast<chrono::seconds>(__rtime);
+	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__rtime - __s);
+	__i = _M_load_and_test_for(__i, __val, true, __mo, true, __s, __ns);
+	return (__i & ~_Waiter_bit) == __val;
       }

     // Returns false iff a timeout occurred.
diff --git a/libstdc++-v3/src/c++11/futex.cc b/libstdc++-v3/src/c++11/futex.cc
index f5000aa77b3..1b5fe480209 100644
--- a/libstdc++-v3/src/c++11/futex.cc
+++ b/libstdc++-v3/src/c++11/futex.cc
@@ -84,6 +84,38 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       }
   }

+  bool
+  __atomic_futex_unsigned_base::_M_futex_wait_for(unsigned *__addr,
+      unsigned __val,
+      bool __has_timeout, chrono::seconds __s, chrono::nanoseconds __ns)
+  {
+    if (!__has_timeout)
+      {
+	// Ignore whether we actually succeeded to block because at worst,
+	// we will fall back to spin-waiting.  The only thing we could do
+	// here on errors is abort.
+	int ret __attribute__((unused));
+	ret = syscall (SYS_futex, __addr, futex_wait_op, __val, nullptr);
+	_GLIBCXX_DEBUG_ASSERT(ret == 0 || errno == EINTR || errno == EAGAIN);
+	return true;
+      }
+    else
+      {
+	struct timespec rt;
+	rt.tv_sec = __s.count();
+	rt.tv_nsec = __ns.count();
+
+	if (syscall (SYS_futex, __addr, futex_wait_op, __val, &rt) == -1)
+	  {
+	    _GLIBCXX_DEBUG_ASSERT(errno == EINTR || errno == EAGAIN
+				  || errno == ETIMEDOUT);
+	    if (errno == ETIMEDOUT)
+	      return false;
+	  }
+	return true;
+      }
+  }
+
   void
   __atomic_futex_unsigned_base::_M_futex_notify_all(unsigned* __addr)
   {
--
2.11.0
Mike Crowe Feb. 14, 2018, 4:32 p.m. | #5
On Sunday 14 January 2018 at 20:44:10 +0000, Mike Crowe wrote:
> On Sunday 14 January 2018 at 16:08:09 +0000, Mike Crowe wrote:
> > Hi Torvald,
> > 
> > Thanks for reviewing this change.
> > 
> > On Saturday 13 January 2018 at 16:29:57 +0100, Torvald Riegel wrote:
> > > On Sun, 2018-01-07 at 20:55 +0000, Mike Crowe wrote:
> > > > This is a first attempt to make std::future::wait_until and
> > > > std::future::wait_for make correct use of
> > > > std::chrono::steady_clock/CLOCK_MONOTONIC. It also makes
> > > > std::future::wait_until react to changes to CLOCK_REALTIME during the
> > > > wait, but only when passed a std::chrono::system_clock time point.
> > > 
> > > I have comments on the design.
> > > 
> > > First, I don't think we should not change
> > > __atomic_futex_unsigned_base::_M_futex_wait_until, as there's a risk
> > > that we'll change behavior of existing applications that work as
> > > expected.
> > 
> > I assume you mean "I don't think we should change" or "I think we should
> > not change"... :-)
> > 
> > The only way I can see that behaviour will change for existing programs is
> > when the system clock changes (i.e. when someone calls settimeofday.) In
> > the existing code, the maximum wait time is fixed once gettimeofday is
> > called to calculate the relative timeout. When using FUTEX_CLOCK_REALTIME,
> > the maximum wait can change based on changes to the system clock after that
> > point. It appears that glibc made this transition successfully and
> > currently uses FUTEX_CLOCK_REALTIME. I think that the new behaviour is
> > better than the old behaviour.
> > 
> > Or perhaps I've missed another possibility. Did you have another risk in
> > mind?
> > 
> > > Instead, ISTM we should additionally expose the two options we have at
> > > the level of futexes:
> > > * Relative timeout using CLOCK_MONOTONIC
> > > * Absolute timeout using CLOCK_REALTIME (which will fall back to the
> > > former on old kernels, which is fine I think).
> > > 
> > > Then we do the following translations from functions that programs would
> > > call to the new futex functions:
> > > 
> > > 1) wait_for is a loop in which we load the current time from the steady
> > > clock, then call the relative futex wait, and if that returns for a
> > > spurious reason (ie, neither timeout nor is the expected value present),
> > > we reduce the prior relative amount by the difference between the time
> > > before the futex wait and the current time.
> > 
> > If we're going to loop on a relative timeout it sounds safer to convert it
> > to an absolute (steady clock) timeout. That way we won't risk increasing
> > the timeout if the scheduler decides not to run us at an inopportune moment
> > between waits. _M_load_when_equal_for already does this.
> > 
> > _M_load_and_test_until already has a loop for spurious wakeup. I think that
> > it makes sense to only loop at one level. That loop relies on the timeout
> > being absolute, which is why my _M_load_and_test_until_steady also uses an
> > absolute timeout.
> > 
> > > 2) wait_until using the steady clock is a loop similar to wait_for, just
> > > that we additionally compute the initial relative timeout.
> > 
> > Clearly an absolute wait can be implemented in terms of a relative one and
> > vice-versa, but at least in my attempts to write them I find the code
> > easier to understand (and therefore get right) if the fundamental wait is
> > the absolute one and the relative one is implemented on top of it.
> 
> I had a quick go at implementing at least the first part of your design, as
> I understood it. (I've kept the loops inside atomic_futex_unsigned - and I
> think that you wanted to move them out to the client code.) I've not tested
> it much.
> 
> I think that this implementation of _M_load_and_test_for is rather more
> error-prone than my previous _M_load_and_test_until_steady. That's probably
> partly because the type-safe duration has already been separated into seconds
> and nanoseconds. It would be nice to push this separation as deeply as
> possible in the code, but I'm afraid that would break ABI compatibility.
> 
> Thanks.
> 
> Mike.
> 
> --8<--
> 
> diff --git a/libstdc++-v3/include/bits/atomic_futex.h b/libstdc++-v3/include/bits/atomic_futex.h
> index ad9437da4e2..fa4a4382c79 100644
> --- a/libstdc++-v3/include/bits/atomic_futex.h
> +++ b/libstdc++-v3/include/bits/atomic_futex.h
> @@ -57,6 +57,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>      _M_futex_wait_until(unsigned *__addr, unsigned __val, bool __has_timeout,
>  	chrono::seconds __s, chrono::nanoseconds __ns);
> 
> +    // Returns false iff a timeout occurred.
> +    bool
> +    _M_futex_wait_for(unsigned *__addr, unsigned __val, bool __has_timeout,
> +	chrono::seconds __s, chrono::nanoseconds __ns);
> +
>      // This can be executed after the object has been destroyed.
>      static void _M_futex_notify_all(unsigned* __addr);
>    };
> @@ -110,6 +115,40 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  	}
>      }
> 
> +    // If a timeout occurs, returns a current value after the timeout;
> +    // otherwise, returns the operand's value if equal is true or a different
> +    // value if equal is false.
> +    // The assumed value is the caller's assumption about the current value
> +    // when making the call.
> +    unsigned
> +    _M_load_and_test_for(unsigned __assumed, unsigned __operand,
> +	bool __equal, memory_order __mo, bool __has_timeout,
> +	chrono::seconds __s, chrono::nanoseconds __ns)
> +    {
> +      auto __end_time = chrono::steady_clock::now() + chrono::seconds(__s) + chrono::nanoseconds(__ns);
> +      for(;;)
> +	{
> +	  // Don't bother checking the value again because we expect the caller
> +	  // to have done it recently.
> +	  // memory_order_relaxed is sufficient because we can rely on just the
> +	  // modification order (store_notify uses an atomic RMW operation too),
> +	  // and the futex syscalls synchronize between themselves.
> +	  _M_data.fetch_or(_Waiter_bit, memory_order_relaxed);
> +	  bool __ret = _M_futex_wait_for((unsigned*)(void*)&_M_data,
> +					   __assumed | _Waiter_bit,
> +					   __has_timeout, __s, __ns);
> +	  // Fetch the current value after waiting (clears _Waiter_bit).
> +	  __assumed = _M_load(__mo);
> +	  if (!__ret || ((__operand == __assumed) == __equal))
> +	    return __assumed;
> +
> +	  const auto __remaining = __end_time - chrono::steady_clock::now();
> +	  __s = chrono::duration_cast<chrono::seconds>(__remaining);
> +	  __ns = chrono::duration_cast<chrono::nanoseconds>(__remaining - __s);
> +	}
> +      return __assumed;
> +    }
> +
>      // Returns the operand's value if equal is true or a different value if
>      // equal is false.
>      // The assumed value is the caller's assumption about the current value
> @@ -168,8 +207,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        _M_load_when_equal_for(unsigned __val, memory_order __mo,
>  	  const chrono::duration<_Rep, _Period>& __rtime)
>        {
> -	return _M_load_when_equal_until(__val, __mo,
> -					__clock_t::now() + __rtime);
> +	unsigned __i = _M_load(__mo);
> +	if ((__i & ~_Waiter_bit) == __val)
> +	  return true;
> +	auto __s = chrono::duration_cast<chrono::seconds>(__rtime);
> +	auto __ns = chrono::duration_cast<chrono::nanoseconds>(__rtime - __s);
> +	__i = _M_load_and_test_for(__i, __val, true, __mo, true, __s, __ns);
> +	return (__i & ~_Waiter_bit) == __val;
>        }
> 
>      // Returns false iff a timeout occurred.
> diff --git a/libstdc++-v3/src/c++11/futex.cc b/libstdc++-v3/src/c++11/futex.cc
> index f5000aa77b3..1b5fe480209 100644
> --- a/libstdc++-v3/src/c++11/futex.cc
> +++ b/libstdc++-v3/src/c++11/futex.cc
> @@ -84,6 +84,38 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        }
>    }
> 
> +  bool
> +  __atomic_futex_unsigned_base::_M_futex_wait_for(unsigned *__addr,
> +      unsigned __val,
> +      bool __has_timeout, chrono::seconds __s, chrono::nanoseconds __ns)
> +  {
> +    if (!__has_timeout)
> +      {
> +	// Ignore whether we actually succeeded to block because at worst,
> +	// we will fall back to spin-waiting.  The only thing we could do
> +	// here on errors is abort.
> +	int ret __attribute__((unused));
> +	ret = syscall (SYS_futex, __addr, futex_wait_op, __val, nullptr);
> +	_GLIBCXX_DEBUG_ASSERT(ret == 0 || errno == EINTR || errno == EAGAIN);
> +	return true;
> +      }
> +    else
> +      {
> +	struct timespec rt;
> +	rt.tv_sec = __s.count();
> +	rt.tv_nsec = __ns.count();
> +
> +	if (syscall (SYS_futex, __addr, futex_wait_op, __val, &rt) == -1)
> +	  {
> +	    _GLIBCXX_DEBUG_ASSERT(errno == EINTR || errno == EAGAIN
> +				  || errno == ETIMEDOUT);
> +	    if (errno == ETIMEDOUT)
> +	      return false;
> +	  }
> +	return true;
> +      }
> +  }
> +
>    void
>    __atomic_futex_unsigned_base::_M_futex_notify_all(unsigned* __addr)
>    {
> --
> 2.11.0
> 

Hi Torvald and Jonathan,

Do you have any comments on my replies?

Thanks.

Mike.