]> git-server-git.apps.pok.os.sepia.ceph.com Git - ceph.git/commit
librbd/migration/QCOWFormat: don't complete read_clusters() inline 64197/head
authorIlya Dryomov <idryomov@gmail.com>
Wed, 25 Jun 2025 10:24:35 +0000 (12:24 +0200)
committerIlya Dryomov <idryomov@gmail.com>
Thu, 26 Jun 2025 06:07:22 +0000 (08:07 +0200)
commit0f92a72dff5b8015d39656bfa6d9cc4284c7dc6f
tree821d73cf65969d36d5b05b026591ea3cbd80fca4
parent994245f7a5f13fb08090aa617d9c21f9a9ba4efc
librbd/migration/QCOWFormat: don't complete read_clusters() inline

When the cluster needs to be read, the completion is posted to ASIO.
However, in the two special cases (cluster DNE and zero cluster), the
completion is completed inline at the moment.  This violates invariants
and can eventually lead to a lockup.  For example, in a scenario of
a read from a clone image whose parent is under migration:

  io::ObjectReadRequest::read_parent()
    io::util::read_parent()
      < image_lock is taken for read >
      io::ImageDispatchSpec::send()
        migration::ImageDispatch::read()
          migration::QCOWFormat::ReadRequest::send()
            ...
            migration::QCOWFormat::ReadRequest::read_clusters()
              < cluster DNE >
              migration::QCOWFormat::ReadRequest::handle_read_clusters()
                io::AioCompletion::complete()
                  io::ObjectReadRequest::copyup()
                    is_copy_on_read()
                      < image_lock is taken for read >

copyup() expects to be called with no locks held, but going through
QCOWFormat in the "cluster DNE" case essentially maintains image_lock
taken in read_parent() and then it's taken again by the same thread in
is_copy_on_read().  Under pthreads, it's not a problem:

  A thread may hold multiple concurrent read locks on rwlock (that is,
  successfully call the pthread_rwlock_rdlock() function n times). If
  so, the thread must perform matching unlocks (that is, it must call
  the pthread_rwlock_unlock() function n times).

But according to C++ standard it's undefined behavior:

  If lock_shared is called by a thread that already owns the mutex in
  any mode (exclusive or shared), the behavior is undefined.

Other, longer and more elaborate, call chains are possible too and
there it may end up being a write lock, a tripped assertion, etc.  To
avoid this, make the special cases in read_clusters() behave the same
as the main path.

Fixes: https://tracker.ceph.com/issues/71838
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit a02cc13191d5e695620791c883ff1460d2e42f57)
src/librbd/migration/QCOWFormat.cc