Skip to content

Commit

Permalink
Handle a case when TSC goes back for DistributedMutex
Browse files Browse the repository at this point in the history
Summary:
Based on the [comment][1] form the Linux kernel TSC values might be
sligthly off across different sockets:

> The regular implementation assumes that clocksource reads are globally
> monotonic. The TSC can be slightly off across sockets which can cause
> the regular delta calculation (cycles - last) to return a huge time
> jump.

[1]: https://github.com/torvalds/linux/blob/70d201a40823acba23899342d62bc2644051ad2e/arch/x86/include/asm/vdso/gettimeofday.h#L305-L308

Reviewed By: ot

Differential Revision: D52729872

fbshipit-source-id: 69c221839c745ec75a38381c046bfbdda392c175
  • Loading branch information
ilvokhin authored and facebook-github-bot committed Jan 15, 2024
1 parent 444dc79 commit 19f74a4
Showing 1 changed file with 9 additions and 8 deletions.
17 changes: 9 additions & 8 deletions folly/synchronization/DistributedMutex-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ constexpr auto kExceptionOccurred = std::uint32_t{0b1010};

// Alias for processor's time-stamp counter value to help distinguish it from
// other integers
using CpuTicks = std::int64_t;
using CpuTicks = std::uint64_t;
// The number of spins that we are allowed to do before we resort to marking a
// thread as having slept
//
Expand Down Expand Up @@ -836,7 +836,7 @@ std::uint64_t publish(
std::uint64_t spins,
CpuTicks current,
CpuTicks previous,
CpuTicks deadline,
CpuTicks elapsed,
bool& shouldPublish,
Waiter& waiter,
std::uint32_t waitMode) {
Expand Down Expand Up @@ -868,8 +868,8 @@ std::uint64_t publish(
// timestamp to force the waking thread to skip us
auto now = ((waitMode == kCombineWaiting) && !spins)
? std::numeric_limits<CpuTicks>::max()
: (current < deadline) ? current
: CpuTicks{0};
: (elapsed < kMaxSpinTime) ? current
: CpuTicks{0};
// the wait mode information is published in the bottom 8 bits of the futex
// word, the rest contains time information as computed above. Overflows are
// not really a correctness concern because time publishing is only a
Expand All @@ -889,10 +889,11 @@ bool spin(Waiter& waiter, std::uint32_t& sig, std::uint32_t mode) {
auto waitMode = (mode == kCombineUninitialized) ? kCombineWaiting : kWaiting;
auto previous = CpuTicks{0};
auto shouldPublish = false;
for (auto current = time(), deadline = current + kMaxSpinTime;;
previous = current, current = time()) {
// elapsed is unsigned and will intentionally underflows if time goes back
for (CpuTicks start = time(), current = start, elapsed = 0;;
previous = current, current = time(), elapsed = current - start) {
auto signal = publish(
spins++, current, previous, deadline, shouldPublish, waiter, waitMode);
spins++, current, previous, elapsed, shouldPublish, waiter, waitMode);

// if we got skipped, make a note of it and return if we got a skipped
// signal or a signal to wake up
Expand All @@ -907,7 +908,7 @@ bool spin(Waiter& waiter, std::uint32_t& sig, std::uint32_t mode) {

// if we are under the spin threshold, pause to allow the other
// hyperthread to run. If not, then sleep
if (current < deadline) {
if (elapsed < kMaxSpinTime) {
asm_volatile_pause();
} else {
std::this_thread::sleep_for(folly::detail::Sleeper::kMinYieldingSleep);
Expand Down

0 comments on commit 19f74a4

Please sign in to comment.