librbd: potential assertion failure during cache read

It's possible for a cache read from a clone to trigger a writeback if a
previous read op determined the object doesn't exist in the clone,
followed by a cached write to the non-existent clone object, followed
by another read request to the same object.  This causes the cache to
flush the pending writeback ops while not holding the owner lock.

Fixes: #13559
Backport: hammer
Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/LibrbdWriteback.cc b/src/librbd/LibrbdWriteback.cc
index 058760e6d97994b28fa7383f4d0437c6e85fe706..ac778eec5455daec3f54eabd86131aadae52e5da 100644 (file)
--- a/src/librbd/LibrbdWriteback.cc
+++ b/src/librbd/LibrbdWriteback.cc
@@ -45,15 +45,18 @@ namespace librbd {
    * @param c context to finish
    * @param l mutex to lock
    */
-  class C_Request : public Context {
+  class C_ReadRequest : public Context {
   public:
-    C_Request(CephContext *cct, Context *c, Mutex *l)
-      : m_cct(cct), m_ctx(c), m_lock(l) {}
-    virtual ~C_Request() {}
+    C_ReadRequest(CephContext *cct, Context *c, RWLock *owner_lock,
+                  Mutex *cache_lock)
+      : m_cct(cct), m_ctx(c), m_owner_lock(owner_lock),
+        m_cache_lock(cache_lock) {
+    }
     virtual void finish(int r) {
       ldout(m_cct, 20) << "aio_cb completing " << dendl;
       {
-       Mutex::Locker l(*m_lock);
+        RWLock::RLocker owner_locker(*m_owner_lock);
+        Mutex::Locker cache_locker(*m_cache_lock);
        m_ctx->complete(r);
       }
       ldout(m_cct, 20) << "aio_cb finished" << dendl;
@@ -61,7 +64,8 @@ namespace librbd {
   private:
     CephContext *m_cct;
     Context *m_ctx;
-    Mutex *m_lock;
+    RWLock *m_owner_lock;
+    Mutex *m_cache_lock;
   };
 
   class C_OrderedWrite : public Context {
@@ -105,7 +109,8 @@ namespace librbd {
                             __u32 trunc_seq, int op_flags, Context *onfinish)
   {
     // on completion, take the mutex and then call onfinish.
-    Context *req = new C_Request(m_ictx->cct, onfinish, &m_lock);
+    Context *req = new C_ReadRequest(m_ictx->cct, onfinish, &m_ictx->owner_lock,
+                                     &m_lock);
 
     {
       if (!m_ictx->object_map.object_may_exist(object_no)) {

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 727168232d36628fb695268d7e4844329d0fa94c..3c6f7409d086e7304a89471a9f2389368c92f69d 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -3778,6 +3778,8 @@ reprotect_and_return_err:
       return;
     }
 
+    RWLock::RLocker owner_locker(ictx->owner_lock);
+
     // readahead
     if (ictx->object_cacher && ictx->readahead_max_bytes > 0 &&
        !(op_flags & LIBRADOS_OP_FLAG_FADVISE_RANDOM)) {