From: Kefu Chai <kchai@redhat.com>
Date: Fri, 8 Jan 2021 07:55:49 +0000 (+0800)
Subject: common/Throttle: return new count in put() right away
X-Git-Tag: v16.1.0~72^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=fedaa12737d4c347b00275f3f4d323e58b4c6689;p=ceph.git

common/Throttle: return new count in put() right away

before this change, the is a racing after updating `count`, where
`count` could be updated again by another thread using this Throttle
instance. but the caller expects the value before the second mutation.

actually, the only caller which care about the return value is the
unittests in test/common/Throttle.cc. and without this change, the
ThrottleTest.get test fails randomly. almost 1 out of 5 runs fails.

after this change, the test passes after running for 256 times without
failures.

Signed-off-by: Kefu Chai <kchai@redhat.com>
---

diff --git a/src/common/Throttle.cc b/src/common/Throttle.cc
index b8b4783dce3..2e5b3459c39 100644
--- a/src/common/Throttle.cc
+++ b/src/common/Throttle.cc
@@ -228,14 +228,16 @@ int64_t Throttle::put(int64_t c)
   ceph_assert(c >= 0);
   ldout(cct, 10) << "put " << c << " (" << count.load() << " -> "
 		 << (count.load()-c) << ")" << dendl;
+  int64_t new_count;
   {
     std::lock_guard l(lock);
+    new_count = count;
     if (c) {
       if (!conds.empty())
 	conds.front().notify_one();
       // if count goes negative, we failed somewhere!
       ceph_assert(count >= c);
-      count -= c;
+      new_count = count -= c;
     }
   }
   if (logger) {
@@ -244,7 +246,7 @@ int64_t Throttle::put(int64_t c)
     logger->set(l_throttle_val, count);
   }
 
-  return count;
+  return new_count;
 }
 
 void Throttle::reset()