From 19f74a48a3d8f19d9af0a39c6ef58eece6470cea Mon Sep 17 00:00:00 2001
From: Dmitry Ilvokhin <ilvokhin@meta.com>
Date: Mon, 15 Jan 2024 04:41:12 -0800
Subject: [PATCH] Handle a case when TSC goes back for `DistributedMutex`

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
---
 folly/synchronization/DistributedMutex-inl.h | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/folly/synchronization/DistributedMutex-inl.h b/folly/synchronization/DistributedMutex-inl.h
index 51a8e6cbd99..fcfea533e12 100644
--- a/folly/synchronization/DistributedMutex-inl.h
+++ b/folly/synchronization/DistributedMutex-inl.h
@@ -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
 //
@@ -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) {
@@ -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
@@ -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
@@ -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);