From: Yan, Zheng Date: Mon, 26 Aug 2019 12:24:22 +0000 (+0800) Subject: mds: cleanup dirty snap caps tracking X-Git-Tag: v14.2.5~44^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=7ebc5c2da9a55e32bae95e9af0ecd600b6eba39a;p=ceph.git mds: cleanup dirty snap caps tracking For individual client, track dirty snap caps as whole. This fixes an infinite loop in Locker::file_update_finish and closes a race case. Fixes: https://tracker.ceph.com/issues/41434 Signed-off-by: "Yan, Zheng" (cherry picked from commit 6acd9e8b3558bf5650302e90ba6b546657cbe68e) Conflicts: src/mds/CInode.h --- diff --git a/src/mds/CInode.h b/src/mds/CInode.h index 4260ef5767b..27c946b0f56 100644 --- a/src/mds/CInode.h +++ b/src/mds/CInode.h @@ -627,7 +627,7 @@ protected: int num_caps_wanted = 0; public: - mempool::mds_co::compact_map > client_snap_caps; // [auth] [snap] dirty metadata we still need from the head + mempool::mds_co::set client_snap_caps; mempool::mds_co::compact_map > client_need_snapflush; void add_need_snapflush(CInode *snapin, snapid_t snapid, client_t client); diff --git a/src/mds/Locker.cc b/src/mds/Locker.cc index 639656adc1d..fcd7deb93cd 100644 --- a/src/mds/Locker.cc +++ b/src/mds/Locker.cc @@ -1858,30 +1858,15 @@ void Locker::file_update_finish(CInode *in, MutationRef& mut, unsigned flags, } else if ((flags & UPDATE_SNAPFLUSH) && !in->client_snap_caps.empty()) { dout(10) << " client_snap_caps " << in->client_snap_caps << dendl; // check for snap writeback completion - bool gather = false; - auto p = in->client_snap_caps.begin(); - while (p != in->client_snap_caps.end()) { - auto q = p->second.find(client); - if (q != p->second.end()) { - SimpleLock *lock = in->get_lock(p->first); + in->client_snap_caps.erase(client); + if (in->client_snap_caps.empty()) { + for (int i = 0; i < num_cinode_locks; i++) { + SimpleLock *lock = in->get_lock(cinode_lock_info[i].lock); ceph_assert(lock); - dout(10) << " completing client_snap_caps for " << ccap_string(p->first) - << " lock " << *lock << " on " << *in << dendl; lock->put_wrlock(); - - p->second.erase(q); - if (p->second.empty()) { - gather = true; - in->client_snap_caps.erase(p++); - } else - ++p; - } - } - if (gather) { - if (in->client_snap_caps.empty()) { - in->item_open_file.remove_myself(); - in->item_caps.remove_myself(); } + in->item_open_file.remove_myself(); + in->item_caps.remove_myself(); eval_cap_gather(in, &need_issue); } } diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc index 5ef98eee3a8..61fdfea2284 100644 --- a/src/mds/MDCache.cc +++ b/src/mds/MDCache.cc @@ -1476,17 +1476,16 @@ CInode *MDCache::cow_inode(CInode *in, snapid_t last) auto ret = head_in->split_need_snapflush(oldin, in); if (ret.first) { oldin->client_snap_caps = in->client_snap_caps; - for (const auto &p : oldin->client_snap_caps) { - SimpleLock *lock = oldin->get_lock(p.first); - ceph_assert(lock); - for (const auto &q : p.second) { + if (!oldin->client_snap_caps.empty()) { + for (int i = 0; i < num_cinode_locks; i++) { + SimpleLock *lock = oldin->get_lock(cinode_lock_info[i].lock); + ceph_assert(lock); if (lock->get_state() != LOCK_SNAP_SYNC) { ceph_assert(lock->is_stable()); lock->set_state(LOCK_SNAP_SYNC); // gathering oldin->auth_pin(lock); } lock->get_wrlock(true); - (void)q; /* unused */ } } } @@ -1496,22 +1495,21 @@ CInode *MDCache::cow_inode(CInode *in, snapid_t last) in->item_open_file.remove_myself(); in->item_caps.remove_myself(); - MDSContext::vec finished; - for (const auto &p : client_snap_caps) { - SimpleLock *lock = in->get_lock(p.first); - ceph_assert(lock); - ceph_assert(lock->get_state() == LOCK_SNAP_SYNC); // gathering - for (const auto &q : p.second) { + if (!client_snap_caps.empty()) { + MDSContext::vec finished; + for (int i = 0; i < num_cinode_locks; i++) { + SimpleLock *lock = in->get_lock(cinode_lock_info[i].lock); + ceph_assert(lock); + ceph_assert(lock->get_state() == LOCK_SNAP_SYNC); // gathering lock->put_wrlock(); - (void)q; /* unused */ - } - if (!lock->get_num_wrlocks()) { - lock->set_state(LOCK_SYNC); - lock->take_waiting(SimpleLock::WAIT_STABLE|SimpleLock::WAIT_RD, finished); - in->auth_unpin(lock); + if (!lock->get_num_wrlocks()) { + lock->set_state(LOCK_SYNC); + lock->take_waiting(SimpleLock::WAIT_STABLE|SimpleLock::WAIT_RD, finished); + in->auth_unpin(lock); + } } + mds->queue_waiters(finished); } - mds->queue_waiters(finished); } return oldin; } @@ -1525,23 +1523,8 @@ CInode *MDCache::cow_inode(CInode *in, snapid_t last) int issued = cap->need_snapflush() ? CEPH_CAP_ANY_WR : cap->issued(); if ((issued & CEPH_CAP_ANY_WR) && cap->client_follows < last) { - // note in oldin - for (int i = 0; i < num_cinode_locks; i++) { - if (issued & cinode_lock_info[i].wr_caps) { - int lockid = cinode_lock_info[i].lock; - SimpleLock *lock = oldin->get_lock(lockid); - ceph_assert(lock); - if (lock->get_state() != LOCK_SNAP_SYNC) { - ceph_assert(lock->is_stable()); - lock->set_state(LOCK_SNAP_SYNC); // gathering - oldin->auth_pin(lock); - } - lock->get_wrlock(true); - oldin->client_snap_caps[lockid].insert(client); - dout(10) << " client." << client << " cap " << ccap_string(issued & cinode_lock_info[i].wr_caps) - << " wrlock lock " << *lock << " on " << *oldin << dendl; - } - } + dout(10) << " client." << client << " cap " << ccap_string(issued) << dendl; + oldin->client_snap_caps.insert(client); cap->client_follows = last; // we need snapflushes for any intervening snaps @@ -1555,6 +1538,19 @@ CInode *MDCache::cow_inode(CInode *in, snapid_t last) dout(10) << " ignoring client." << client << " cap follows " << cap->client_follows << dendl; } } + + if (!oldin->client_snap_caps.empty()) { + for (int i = 0; i < num_cinode_locks; i++) { + SimpleLock *lock = oldin->get_lock(cinode_lock_info[i].lock); + ceph_assert(lock); + if (lock->get_state() != LOCK_SNAP_SYNC) { + ceph_assert(lock->is_stable()); + lock->set_state(LOCK_SNAP_SYNC); // gathering + oldin->auth_pin(lock); + } + lock->get_wrlock(true); + } + } } return oldin; } @@ -5517,17 +5513,17 @@ void MDCache::rebuild_need_snapflush(CInode *head_in, SnapRealm *realm, dout(10) << " need snapflush from client." << client << " on " << *in << dendl; - /* TODO: we can check the reconnected/flushing caps to find - * which locks need gathering */ - for (int i = 0; i < num_cinode_locks; i++) { - int lockid = cinode_lock_info[i].lock; - SimpleLock *lock = in->get_lock(lockid); - ceph_assert(lock); - in->client_snap_caps[lockid].insert(client); - in->auth_pin(lock); - lock->set_state(LOCK_SNAP_SYNC); - lock->get_wrlock(true); + if (in->client_snap_caps.empty()) { + for (int i = 0; i < num_cinode_locks; i++) { + int lockid = cinode_lock_info[i].lock; + SimpleLock *lock = in->get_lock(lockid); + ceph_assert(lock); + in->auth_pin(lock); + lock->set_state(LOCK_SNAP_SYNC); + lock->get_wrlock(true); + } } + in->client_snap_caps.insert(client); mds->locker->mark_need_snapflush_inode(in); } }