]> git.apps.os.sepia.ceph.com Git - ceph-ci.git/commitdiff
common/Throttle: return new count in put() right away
authorKefu Chai <kchai@redhat.com>
Fri, 8 Jan 2021 07:55:49 +0000 (15:55 +0800)
committerKefu Chai <kchai@redhat.com>
Fri, 8 Jan 2021 08:01:48 +0000 (16:01 +0800)
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>
src/common/Throttle.cc

index b8b4783dce3050b800e1620468746995a88f1929..2e5b3459c39180b55cd717d61e38539e43bf39e2 100644 (file)
@@ -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()