Commit c23acb9b by Derek Mauro Committed by Copybara-Service

Synchronization: Consolidate the logic for whether steady clocks are supported

for relative timeouts

PiperOrigin-RevId: 523789416
Change-Id: Ide4cfdcae9ea7bffca3355c80ea9c8833a9536e6
parent 32d314d0
...@@ -61,7 +61,9 @@ cc_library( ...@@ -61,7 +61,9 @@ cc_library(
"//absl/synchronization:__pkg__", "//absl/synchronization:__pkg__",
], ],
deps = [ deps = [
"//absl/base",
"//absl/base:config", "//absl/base:config",
"//absl/base:core_headers",
"//absl/base:raw_logging_internal", "//absl/base:raw_logging_internal",
"//absl/time", "//absl/time",
], ],
...@@ -75,6 +77,7 @@ cc_test( ...@@ -75,6 +77,7 @@ cc_test(
deps = [ deps = [
":kernel_timeout_internal", ":kernel_timeout_internal",
"//absl/base:config", "//absl/base:config",
"//absl/random",
"//absl/time", "//absl/time",
"@com_google_googletest//:gtest_main", "@com_google_googletest//:gtest_main",
], ],
...@@ -348,6 +351,7 @@ cc_test( ...@@ -348,6 +351,7 @@ cc_test(
":kernel_timeout_internal", ":kernel_timeout_internal",
":synchronization", ":synchronization",
":thread_pool", ":thread_pool",
"//absl/random",
"//absl/time", "//absl/time",
"@com_google_googletest//:gtest_main", "@com_google_googletest//:gtest_main",
], ],
......
...@@ -44,7 +44,9 @@ absl_cc_library( ...@@ -44,7 +44,9 @@ absl_cc_library(
COPTS COPTS
${ABSL_DEFAULT_COPTS} ${ABSL_DEFAULT_COPTS}
DEPS DEPS
absl::base
absl::config absl::config
absl::core_headers
absl::raw_logging_internal absl::raw_logging_internal
absl::time absl::time
) )
...@@ -59,6 +61,7 @@ absl_cc_test( ...@@ -59,6 +61,7 @@ absl_cc_test(
DEPS DEPS
absl::kernel_timeout_internal absl::kernel_timeout_internal
absl::config absl::config
absl::random_random
absl::time absl::time
GTest::gmock_main GTest::gmock_main
) )
...@@ -257,6 +260,7 @@ absl_cc_test( ...@@ -257,6 +260,7 @@ absl_cc_test(
${ABSL_TEST_COPTS} ${ABSL_TEST_COPTS}
DEPS DEPS
absl::kernel_timeout_internal absl::kernel_timeout_internal
absl::random_random
absl::synchronization absl::synchronization
absl::thread_pool absl::thread_pool
absl::time absl::time
......
...@@ -83,31 +83,6 @@ namespace synchronization_internal { ...@@ -83,31 +83,6 @@ namespace synchronization_internal {
class FutexImpl { class FutexImpl {
public: public:
// Atomically check that `*v == val`, and if it is, then sleep until the
// timeout `t` has been reached, or until woken by `Wake()`.
static int WaitUntil(std::atomic<int32_t>* v, int32_t val,
KernelTimeout t) {
// Monotonic waits are disabled for production builds because go/btm
// requires synchronized clocks.
// TODO(b/160682823): Find a way to enable this when BTM is not enabled
// since production builds don't always run on Borg.
#if defined(CLOCK_MONOTONIC) && !defined(__GOOGLE_GRTE_VERSION__)
constexpr bool kRelativeTimeoutSupported = true;
#else
constexpr bool kRelativeTimeoutSupported = false;
#endif
if (!t.has_timeout()) {
return Wait(v, val);
} else if (kRelativeTimeoutSupported && t.is_relative_timeout()) {
auto rel_timespec = t.MakeRelativeTimespec();
return WaitRelativeTimeout(v, val, &rel_timespec);
} else {
auto abs_timespec = t.MakeAbsTimespec();
return WaitAbsoluteTimeout(v, val, &abs_timespec);
}
}
// Atomically check that `*v == val`, and if it is, then sleep until the until // Atomically check that `*v == val`, and if it is, then sleep until the until
// woken by `Wake()`. // woken by `Wake()`.
static int Wait(std::atomic<int32_t>* v, int32_t val) { static int Wait(std::atomic<int32_t>* v, int32_t val) {
......
...@@ -35,6 +35,28 @@ namespace synchronization_internal { ...@@ -35,6 +35,28 @@ namespace synchronization_internal {
constexpr char FutexWaiter::kName[]; constexpr char FutexWaiter::kName[];
#endif #endif
int FutexWaiter::WaitUntil(std::atomic<int32_t>* v, int32_t val,
KernelTimeout t) {
#ifdef CLOCK_MONOTONIC
constexpr bool kHasClockMonotonic = true;
#else
constexpr bool kHasClockMonotonic = false;
#endif
// We can't call Futex::WaitUntil() here because the prodkernel implementation
// does not know about KernelTimeout::SupportsSteadyClock().
if (!t.has_timeout()) {
return Futex::Wait(v, val);
} else if (kHasClockMonotonic && KernelTimeout::SupportsSteadyClock() &&
t.is_relative_timeout()) {
auto rel_timespec = t.MakeRelativeTimespec();
return Futex::WaitRelativeTimeout(v, val, &rel_timespec);
} else {
auto abs_timespec = t.MakeAbsTimespec();
return Futex::WaitAbsoluteTimeout(v, val, &abs_timespec);
}
}
bool FutexWaiter::Wait(KernelTimeout t) { bool FutexWaiter::Wait(KernelTimeout t) {
// Loop until we can atomically decrement futex from a positive // Loop until we can atomically decrement futex from a positive
// value, waiting on a futex while we believe it is zero. // value, waiting on a futex while we believe it is zero.
...@@ -54,7 +76,7 @@ bool FutexWaiter::Wait(KernelTimeout t) { ...@@ -54,7 +76,7 @@ bool FutexWaiter::Wait(KernelTimeout t) {
} }
if (!first_pass) MaybeBecomeIdle(); if (!first_pass) MaybeBecomeIdle();
const int err = Futex::WaitUntil(&futex_, 0, t); const int err = WaitUntil(&futex_, 0, t);
if (err != 0) { if (err != 0) {
if (err == -EINTR || err == -EWOULDBLOCK) { if (err == -EINTR || err == -EWOULDBLOCK) {
// Do nothing, the loop will retry. // Do nothing, the loop will retry.
......
...@@ -43,6 +43,11 @@ class FutexWaiter : public WaiterCrtp<FutexWaiter> { ...@@ -43,6 +43,11 @@ class FutexWaiter : public WaiterCrtp<FutexWaiter> {
static constexpr char kName[] = "FutexWaiter"; static constexpr char kName[] = "FutexWaiter";
private: private:
// Atomically check that `*v == val`, and if it is, then sleep until the
// timeout `t` has been reached, or until woken by `Wake()`.
static int WaitUntil(std::atomic<int32_t>* v, int32_t val,
KernelTimeout t);
// Futexes are defined by specification to be 32-bits. // Futexes are defined by specification to be 32-bits.
// Thus std::atomic<int32_t> must be just an int32_t with lockfree methods. // Thus std::atomic<int32_t> must be just an int32_t with lockfree methods.
std::atomic<int32_t> futex_; std::atomic<int32_t> futex_;
......
...@@ -21,9 +21,13 @@ ...@@ -21,9 +21,13 @@
#include <algorithm> #include <algorithm>
#include <chrono> // NOLINT(build/c++11) #include <chrono> // NOLINT(build/c++11)
#include <cstdint> #include <cstdint>
#include <cstdlib>
#include <cstring>
#include <ctime> #include <ctime>
#include <limits> #include <limits>
#include "absl/base/attributes.h"
#include "absl/base/call_once.h"
#include "absl/base/config.h" #include "absl/base/config.h"
#include "absl/time/time.h" #include "absl/time/time.h"
...@@ -37,15 +41,12 @@ constexpr int64_t KernelTimeout::kMaxNanos; ...@@ -37,15 +41,12 @@ constexpr int64_t KernelTimeout::kMaxNanos;
#endif #endif
int64_t KernelTimeout::SteadyClockNow() { int64_t KernelTimeout::SteadyClockNow() {
#ifdef __GOOGLE_GRTE_VERSION__ if (!SupportsSteadyClock()) {
// go/btm requires synchronized clocks, so we have to use the system
// clock.
return absl::GetCurrentTimeNanos(); return absl::GetCurrentTimeNanos();
#else }
return std::chrono::duration_cast<std::chrono::nanoseconds>( return std::chrono::duration_cast<std::chrono::nanoseconds>(
std::chrono::steady_clock::now().time_since_epoch()) std::chrono::steady_clock::now().time_since_epoch())
.count(); .count();
#endif
} }
KernelTimeout::KernelTimeout(absl::Time t) { KernelTimeout::KernelTimeout(absl::Time t) {
......
...@@ -128,6 +128,11 @@ class KernelTimeout { ...@@ -128,6 +128,11 @@ class KernelTimeout {
// case of a spurious wakeup. // case of a spurious wakeup.
std::chrono::nanoseconds ToChronoDuration() const; std::chrono::nanoseconds ToChronoDuration() const;
// Returns true if steady (aka monotonic) clocks are supported by the system.
// This method exists because go/btm requires synchronized clocks, and
// thus requires we use the system (aka walltime) clock.
static constexpr bool SupportsSteadyClock() { return true; }
private: private:
// Returns the current time, expressed as a count of nanoseconds since the // Returns the current time, expressed as a count of nanoseconds since the
// epoch used by an arbitrary clock. The implementation tries to use a steady // epoch used by an arbitrary clock. The implementation tries to use a steady
......
...@@ -14,14 +14,34 @@ ...@@ -14,14 +14,34 @@
#include "absl/synchronization/internal/kernel_timeout.h" #include "absl/synchronization/internal/kernel_timeout.h"
#include <ctime>
#include <chrono> // NOLINT(build/c++11) #include <chrono> // NOLINT(build/c++11)
#include <limits> #include <limits>
#include "absl/random/random.h"
#include "gtest/gtest.h" #include "gtest/gtest.h"
#include "absl/base/config.h" #include "absl/base/config.h"
#include "absl/time/clock.h" #include "absl/time/clock.h"
#include "absl/time/time.h" #include "absl/time/time.h"
// Test go/btm support by randomizing the value clock_gettime() for
// CLOCK_MONOTONIC. This works by overriding a weak symbol in glibc.
// We should be resistant to this randomization when !SupportsSteadyClock().
#ifdef __GOOGLE_GRTE_VERSION__
extern "C" int __clock_gettime(clockid_t c, struct timespec* ts);
extern "C" int clock_gettime(clockid_t c, struct timespec* ts) {
if (c == CLOCK_MONOTONIC &&
!absl::synchronization_internal::KernelTimeout::SupportsSteadyClock()) {
absl::SharedBitGen gen;
ts->tv_sec = absl::Uniform(gen, 0, 1'000'000'000);
ts->tv_nsec = absl::Uniform(gen, 0, 1'000'000'000);
return 0;
}
return __clock_gettime(c, ts);
}
#endif // __GOOGLE_GRTE_VERSION__
namespace { namespace {
#if defined(ABSL_HAVE_ADDRESS_SANITIZER) || \ #if defined(ABSL_HAVE_ADDRESS_SANITIZER) || \
......
...@@ -88,14 +88,8 @@ PthreadWaiter::PthreadWaiter() : waiter_count_(0), wakeup_count_(0) { ...@@ -88,14 +88,8 @@ PthreadWaiter::PthreadWaiter() : waiter_count_(0), wakeup_count_(0) {
// KernelTimeout requested. The return value is the same as the return // KernelTimeout requested. The return value is the same as the return
// value of pthread_cond_timedwait(). // value of pthread_cond_timedwait().
int PthreadWaiter::TimedWait(KernelTimeout t) { int PthreadWaiter::TimedWait(KernelTimeout t) {
#ifndef __GOOGLE_GRTE_VERSION__
constexpr bool kRelativeTimeoutSupported = true;
#else
constexpr bool kRelativeTimeoutSupported = false;
#endif
assert(t.has_timeout()); assert(t.has_timeout());
if (kRelativeTimeoutSupported && t.is_relative_timeout()) { if (KernelTimeout::SupportsSteadyClock() && t.is_relative_timeout()) {
#ifdef ABSL_INTERNAL_HAS_PTHREAD_COND_TIMEDWAIT_RELATIVE_NP #ifdef ABSL_INTERNAL_HAS_PTHREAD_COND_TIMEDWAIT_RELATIVE_NP
const auto rel_timeout = t.MakeRelativeTimespec(); const auto rel_timeout = t.MakeRelativeTimespec();
return pthread_cond_timedwait_relative_np(&cv_, &mu_, &rel_timeout); return pthread_cond_timedwait_relative_np(&cv_, &mu_, &rel_timeout);
......
...@@ -53,13 +53,7 @@ SemWaiter::SemWaiter() : wakeups_(0) { ...@@ -53,13 +53,7 @@ SemWaiter::SemWaiter() : wakeups_(0) {
// KernelTimeout requested. The return value is the same as a call to the return // KernelTimeout requested. The return value is the same as a call to the return
// value to a call to sem_timedwait(). // value to a call to sem_timedwait().
int SemWaiter::TimedWait(KernelTimeout t) { int SemWaiter::TimedWait(KernelTimeout t) {
#ifndef __GOOGLE_GRTE_VERSION__ if (KernelTimeout::SupportsSteadyClock() && t.is_relative_timeout()) {
constexpr bool kRelativeTimeoutSupported = true;
#else
constexpr bool kRelativeTimeoutSupported = false;
#endif
if (kRelativeTimeoutSupported && t.is_relative_timeout()) {
#if defined(ABSL_INTERNAL_HAVE_SEM_CLOCKWAIT) && defined(CLOCK_MONOTONIC) #if defined(ABSL_INTERNAL_HAVE_SEM_CLOCKWAIT) && defined(CLOCK_MONOTONIC)
const auto abs_clock_timeout = t.MakeClockAbsoluteTimespec(CLOCK_MONOTONIC); const auto abs_clock_timeout = t.MakeClockAbsoluteTimespec(CLOCK_MONOTONIC);
return sem_clockwait(&sem_, CLOCK_MONOTONIC, &abs_clock_timeout); return sem_clockwait(&sem_, CLOCK_MONOTONIC, &abs_clock_timeout);
......
...@@ -50,7 +50,7 @@ bool StdcppWaiter::Wait(KernelTimeout t) { ...@@ -50,7 +50,7 @@ bool StdcppWaiter::Wait(KernelTimeout t) {
if (!t.has_timeout()) { if (!t.has_timeout()) {
cv_.wait(lock); cv_.wait(lock);
} else { } else {
auto wait_result = t.is_relative_timeout() auto wait_result = t.SupportsSteadyClock() && t.is_relative_timeout()
? cv_.wait_for(lock, t.ToChronoDuration()) ? cv_.wait_for(lock, t.ToChronoDuration())
: cv_.wait_until(lock, t.ToChronoTimePoint()); : cv_.wait_until(lock, t.ToChronoTimePoint());
if (wait_result == std::cv_status::timeout) { if (wait_result == std::cv_status::timeout) {
......
...@@ -14,9 +14,11 @@ ...@@ -14,9 +14,11 @@
#include "absl/synchronization/internal/waiter.h" #include "absl/synchronization/internal/waiter.h"
#include <ctime>
#include <iostream> #include <iostream>
#include <ostream> #include <ostream>
#include "absl/random/random.h"
#include "absl/synchronization/internal/create_thread_identity.h" #include "absl/synchronization/internal/create_thread_identity.h"
#include "absl/synchronization/internal/futex_waiter.h" #include "absl/synchronization/internal/futex_waiter.h"
#include "absl/synchronization/internal/kernel_timeout.h" #include "absl/synchronization/internal/kernel_timeout.h"
...@@ -29,6 +31,24 @@ ...@@ -29,6 +31,24 @@
#include "absl/time/time.h" #include "absl/time/time.h"
#include "gtest/gtest.h" #include "gtest/gtest.h"
// Test go/btm support by randomizing the value clock_gettime() for
// CLOCK_MONOTONIC. This works by overriding a weak symbol in glibc.
// We should be resistant to this randomization when !SupportsSteadyClock().
#ifdef __GOOGLE_GRTE_VERSION__
extern "C" int __clock_gettime(clockid_t c, struct timespec* ts);
extern "C" int clock_gettime(clockid_t c, struct timespec* ts) {
if (c == CLOCK_MONOTONIC &&
!absl::synchronization_internal::KernelTimeout::SupportsSteadyClock()) {
absl::SharedBitGen gen;
ts->tv_sec = absl::Uniform(gen, 0, 1'000'000'000);
ts->tv_nsec = absl::Uniform(gen, 0, 1'000'000'000);
return 0;
}
return __clock_gettime(c, ts);
}
#endif // __GOOGLE_GRTE_VERSION__
namespace { namespace {
TEST(Waiter, PrintPlatformImplementation) { TEST(Waiter, PrintPlatformImplementation) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment