mirror of
https://github.com/ceph/ceph
synced 2025-02-15 06:48:22 +00:00
rgw_file: alternate fix deadlock on lru eviction
This change is an alternate fix for two problems found and fixed by Yao Zongyou <yaozongyou@vip.qq.com>. The deadlock can be avoided just by not taking it in the recycle case, which invariantly holds the lock. The invalidation of the insert iterator by the recyle-path unlink we'd like to handle as a condition in order to preserve the cached insertion point optimization we get in the common case. (The original behavior was, indeed, incorrect.) Based on feedback from Yao, removed the RGWFileHandle dtor version of the unlink check, which I think happened twice. Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
This commit is contained in:
parent
2cfd5fece9
commit
3cf0880f86
@ -32,6 +32,7 @@ namespace cohort {
|
||||
/* public flag values */
|
||||
constexpr uint32_t FLAG_NONE = 0x0000;
|
||||
constexpr uint32_t FLAG_INITIAL = 0x0001;
|
||||
constexpr uint32_t FLAG_RECYCLE = 0x0002;
|
||||
|
||||
enum class Edge : std::uint8_t
|
||||
{
|
||||
@ -232,12 +233,14 @@ namespace cohort {
|
||||
delete tdo;
|
||||
} /* unref */
|
||||
|
||||
Object* insert(ObjectFactory* fac, Edge edge, uint32_t flags) {
|
||||
Object* insert(ObjectFactory* fac, Edge edge, uint32_t& flags) {
|
||||
/* use supplied functor to re-use an evicted object, or
|
||||
* allocate a new one of the descendant type */
|
||||
Object* o = evict_block();
|
||||
if (o)
|
||||
if (o) {
|
||||
fac->recycle(o); /* recycle existing object */
|
||||
flags |= FLAG_RECYCLE;
|
||||
}
|
||||
else
|
||||
o = fac->alloc(); /* get a new one */
|
||||
|
||||
|
@ -954,10 +954,6 @@ namespace rgw {
|
||||
}
|
||||
|
||||
RGWFileHandle::~RGWFileHandle() {
|
||||
/* in the non-delete case, handle may still be in handle table */
|
||||
if (fh_hook.is_linked()) {
|
||||
fs->fh_cache.remove(fh.fh_hk.object, this, FHCache::FLAG_LOCK);
|
||||
}
|
||||
/* cond-unref parent */
|
||||
if (parent && (! parent->is_mount())) {
|
||||
/* safe because if parent->unref causes its deletion,
|
||||
@ -1007,9 +1003,11 @@ namespace rgw {
|
||||
lsubdout(fs->get_context(), rgw, 17)
|
||||
<< __func__ << " " << *this
|
||||
<< dendl;
|
||||
/* remove if still in fh_cache */
|
||||
/* in the non-delete case, handle may still be in handle table */
|
||||
if (fh_hook.is_linked()) {
|
||||
fs->fh_cache.remove(fh.fh_hk.object, this, FHCache::FLAG_LOCK);
|
||||
/* in this case, we are being called from a context which holds
|
||||
* the partition lock */
|
||||
fs->fh_cache.remove(fh.fh_hk.object, this, FHCache::FLAG_NONE);
|
||||
}
|
||||
return true;
|
||||
} /* RGWFileHandle::reclaim */
|
||||
|
@ -1061,16 +1061,25 @@ namespace rgw {
|
||||
/* make or re-use handle */
|
||||
RGWFileHandle::Factory prototype(this, parent, fhk,
|
||||
obj_name, CREATE_FLAGS(flags));
|
||||
uint32_t iflags{cohort::lru::FLAG_INITIAL};
|
||||
fh = static_cast<RGWFileHandle*>(
|
||||
fh_lru.insert(&prototype,
|
||||
cohort::lru::Edge::MRU,
|
||||
cohort::lru::FLAG_INITIAL));
|
||||
iflags));
|
||||
if (fh) {
|
||||
/* lock fh (LATCHED) */
|
||||
if (flags & RGWFileHandle::FLAG_LOCK)
|
||||
fh->mtx.lock();
|
||||
/* inserts, releasing latch */
|
||||
fh_cache.insert_latched(fh, lat, RGWFileHandle::FHCache::FLAG_UNLOCK);
|
||||
if (likely(! (iflags & cohort::lru::FLAG_RECYCLE))) {
|
||||
/* inserts at cached insert iterator, releasing latch */
|
||||
fh_cache.insert_latched(
|
||||
fh, lat, RGWFileHandle::FHCache::FLAG_UNLOCK);
|
||||
} else {
|
||||
/* recycle step invalidates Latch */
|
||||
fh_cache.insert(
|
||||
fhk.fh_hk.object, fh, RGWFileHandle::FHCache::FLAG_NONE);
|
||||
lat.lock->unlock(); /* !LATCHED */
|
||||
}
|
||||
get<1>(fhr) |= RGWFileHandle::FLAG_CREATE;
|
||||
/* ref parent (non-initial ref cannot fail on valid object) */
|
||||
if (! parent->is_mount()) {
|
||||
|
Loading…
Reference in New Issue
Block a user