From 464d8b58869b94c8a6ce85dcaee169296fc9ee13 Mon Sep 17 00:00:00 2001 From: Zhiwei Dai Date: Wed, 8 Dec 2021 15:06:59 +0800 Subject: [PATCH] fix librgw lru lock nit modified: src/common/cohort_lru.h modified: src/rgw/rgw_file.cc modified: src/rgw/rgw_file.h Signed-off-by: Dai Zhiwei --- src/common/cohort_lru.h | 13 +++++++------ src/rgw/rgw_file.cc | 16 +++++++++++++++- src/rgw/rgw_file.h | 16 +++++++++++++++- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/common/cohort_lru.h b/src/common/cohort_lru.h index 1dafe33..b144977 100644 --- a/src/common/cohort_lru.h +++ b/src/common/cohort_lru.h @@ -74,6 +74,8 @@ namespace cohort { virtual bool reclaim(const ObjectFactory* newobj_fac) = 0; + virtual bool remove() = 0; + virtual ~Object() {} private: @@ -139,15 +141,15 @@ namespace cohort { for (int ix = 0; ix < n_lanes; ++ix, lane_ix = next_evict_lane()) { Lane& lane = qlane[lane_ix]; - lane.lock.lock(); + std::unique_lock lane_lock{lane.lock}; /* if object at LRU has refcnt==1, it may be reclaimable */ Object* o = &(lane.q.back()); if (can_reclaim(o)) { ++(o->lru_refcnt); o->lru_flags |= FLAG_EVICTING; - lane.lock.unlock(); + lane_lock.unlock(); if (o->reclaim(newobj_fac)) { - lane.lock.lock(); + lane_lock.lock(); --(o->lru_refcnt); /* assertions that o state has not changed across * relock */ @@ -156,16 +158,13 @@ namespace cohort { Object::Queue::iterator it = Object::Queue::s_iterator_to(*o); lane.q.erase(it); - lane.lock.unlock(); return o; } else { - // XXX can't make unreachable (means what?) --(o->lru_refcnt); o->lru_flags &= ~FLAG_EVICTING; /* unlock in next block */ } } /* can_reclaim(o) */ - lane.lock.unlock(); } /* each lane */ return nullptr; } /* evict_block */ @@ -210,6 +209,7 @@ namespace cohort { Object::Queue::s_iterator_to(*o); lane.q.erase(it); tdo = o; + tdo->remove(); } lane.lock.unlock(); } else if (unlikely(refcnt == SENTINEL_REFCNT)) { @@ -224,6 +224,7 @@ namespace cohort { /* hiwat check */ if (lane.q.size() > lane_hiwat) { tdo = o; + tdo->remove(); } else { lane.q.push_back(*o); } diff --git a/src/rgw/rgw_file.cc b/src/rgw/rgw_file.cc index 6ac5738..8b01e6f 100644 --- a/src/rgw/rgw_file.cc +++ b/src/rgw/rgw_file.cc @@ -1157,7 +1157,7 @@ namespace rgw { * no unsafe iterators reaching it either--n.b., this constraint * is binding oncode which may in future attempt to e.g., * cause the eviction of objects in LRU order */ - (void) get_fs()->unref(parent); + (void) get_fs()->unref(parent, RGWFileHandle::FLAG_NONE); } } @@ -1223,6 +1223,20 @@ namespace rgw { return true; } /* RGWFileHandle::reclaim */ + bool RGWFileHandle::remove() { + lsubdout(fs->get_context(), rgw, 17) + << __func__ << " " << *this + << dendl; + /* in the non-delete case, handle may still be in handle table */ + if (fh_hook.is_linked()) { + /* in this case, we are being called from a context which holds + * the partition lock */ + //this->flags |= RGWFileHandle::FLAG_DELETED; + fs->fh_cache.remove(fh.fh_hk.object, this, FHCache::FLAG_NONE); + } + return true; + } /* RGWFileHandle::remove */ + bool RGWFileHandle::has_children() const { if (unlikely(! is_dir())) diff --git a/src/rgw/rgw_file.h b/src/rgw/rgw_file.h index 46565fb..d5ae2e5 100644 --- a/src/rgw/rgw_file.h +++ b/src/rgw/rgw_file.h @@ -723,6 +723,8 @@ namespace rgw { bool reclaim(const cohort::lru::ObjectFactory* newobj_fac) override; + bool remove() override; + typedef cohort::lru::LRU FhLRU; struct FhLT @@ -1159,9 +1161,21 @@ namespace rgw { return fhr; } /* lookup_fh(RGWFileHandle*, const char *, const uint32_t) */ - inline void unref(RGWFileHandle* fh) { + inline void unref(RGWFileHandle* fh, uint32_t flags = RGWFileHandle::FLAG_LOCK) { if (likely(! fh->is_mount())) { + RGWFileHandle::FHCache::Latch lat; + lat.p = &(fh->fs->fh_cache.partition_of_scalar(fh->fh.fh_hk.object)); + + if (flags & RGWFileHandle::FLAG_LOCK) { + lat.lock = &lat.p->lock; + lat.lock->lock(); + } + (void) fh_lru.unref(fh, cohort::lru::FLAG_NONE); + + if ( flags & RGWFileHandle::FLAG_LOCK) { + lat.lock->unlock(); + } } } -- 2.27.0