From: Yan, Zheng Date: Mon, 26 Aug 2019 12:24:22 +0000 (+0800) Subject: mds: cleanup dirty snap caps tracking X-Git-Tag: v15.1.0~1423^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=6acd9e8b3558bf5650302e90ba6b546657cbe68e;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" --- diff --git a/src/mds/CInode.h b/src/mds/CInode.h index f2b23a48645..d866b7622b2 100644 --- a/src/mds/CInode.h +++ b/src/mds/CInode.h @@ -934,7 +934,7 @@ class CInode : public MDSCacheObject, public InodeStoreBase, public Counter::item dirty_rstat_item; - 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; // LogSegment lists i (may) belong to diff --git a/src/mds/Locker.cc b/src/mds/Locker.cc index 910b048a007..7515b6a76fc 100644 --- a/src/mds/Locker.cc +++ b/src/mds/Locker.cc @@ -1863,30 +1863,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 722bb6d065c..039ce2e6253 100644 --- a/src/mds/MDCache.cc +++ b/src/mds/MDCache.cc @@ -1471,17 +1471,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 */ } } } @@ -1491,22 +1490,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; } @@ -1520,23 +1518,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 @@ -1550,6 +1533,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; } @@ -5487,17 +5483,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); } }