From: Jason Dillaman <dillaman@redhat.com>
Date: Tue, 13 Oct 2020 01:34:25 +0000 (-0400)
Subject: librbd: update AioCompletion return value before evaluating pending count
X-Git-Tag: v16.1.0~831^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F37645%2Fhead;p=ceph.git

librbd: update AioCompletion return value before evaluating pending count

If the pending count is decremented before the return value is updated,
there is a possibility of two ASIO threads concurrently decrementing the
pending count down from 2 -> 1 -> 0. In the second thread (the one that
performs the final decrement from 1 -> 0), it can finalize the completion
before the first thread has had a chance to update the return value.

Fixes: https://tracker.ceph.com/issues/47847
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---

diff --git a/src/librbd/io/AioCompletion.cc b/src/librbd/io/AioCompletion.cc
index 96cc0f9f7719..c04b80770f99 100644
--- a/src/librbd/io/AioCompletion.cc
+++ b/src/librbd/io/AioCompletion.cc
@@ -213,22 +213,22 @@ void AioCompletion::set_request_count(uint32_t count) {
 
 void AioCompletion::complete_request(ssize_t r)
 {
-  uint32_t previous_pending_count = pending_count--;
-  ceph_assert(previous_pending_count > 0);
-  auto pending_count = previous_pending_count - 1;
-
   ceph_assert(ictx != nullptr);
   CephContext *cct = ictx->cct;
 
   if (r > 0) {
     rval += r;
-  } else if (r != -EEXIST) {
+  } else if (r < 0 && r != -EEXIST) {
     // might race w/ another thread setting an error code but
     // first one wins
     int zero = 0;
     error_rval.compare_exchange_strong(zero, r);
   }
 
+  uint32_t previous_pending_count = pending_count--;
+  ceph_assert(previous_pending_count > 0);
+  auto pending_count = previous_pending_count - 1;
+
   ldout(cct, 20) << "cb=" << complete_cb << ", "
                  << "pending=" << pending_count << dendl;
   if (pending_count == 0) {