RepoMirrors/ceph
1
0
Fork 0
mirror of https://github.com/ceph/ceph synced 2025-01-01 08:32:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

rgw nfs: don't leak fh->mtx in lookup_handle()

Browse Source 
This change fixes a serious latent locking problem, noticed after
updating the ganesha/rgw driver invalidation after renames.

Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
This commit is contained in:
Matt Benjamin 2016-08-05 10:03:33 -04:00
parent 297220fd2a
commit d74d46170d
2 changed files with 114 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

81
src/rgw/rgw_file.cc
View File

@ -252,7 +252,7 @@ namespace rgw {
/* rgw_fh ref+ */
rgw_fh = get_rgwfh(fh);
rgw_fh->mtx.lock();
rgw_fh->mtx.lock(); /* LOCKED */
}
std::string oname = rgw_fh->relative_object_name();
@ -268,6 +268,14 @@ namespace rgw {
rgw_fh->flags |= RGWFileHandle::FLAG_DELETED;
fh_cache.remove(rgw_fh->fh.fh_hk.object, rgw_fh, cohort::lru::FLAG_NONE);
#if 1 /* XXX verify clear cache */
fh_key fhk(rgw_fh->fh.fh_hk);
LookupFHResult tfhr = lookup_fh(fhk, RGWFileHandle::FLAG_LOCKED);
RGWFileHandle* nfh = get<0>(tfhr);
assert(!nfh);
#endif
rgw_fh->mtx.unlock();
unref(rgw_fh);
@ -282,6 +290,8 @@ namespace rgw {
* succeeds */
int rc = -EINVAL;
real_time t;
std::string src_name{_src_name};
std::string dst_name{_dst_name};
@ -304,12 +314,22 @@ namespace rgw {
<< " rejecting attempt to rename directory path="
<< rgw_fh->full_object_name()
<< dendl;
rgw_fh->mtx.unlock(); /* !LOCKED */
unref(rgw_fh); /* -ref */
rc = -EPERM;
goto out;
goto unlock;
}
/* forbid renaming open files (violates intent, for now) */
if (rgw_fh->is_open()) {
ldout(get_context(), 12) << __func__
<< " rejecting attempt to rename open file path="
<< rgw_fh->full_object_name()
<< dendl;
rc = -EPERM;
goto unlock;
}
t = real_clock::now();
for (int ix : {0, 1}) {
switch (ix) {
case 0:
@ -319,8 +339,26 @@ namespace rgw {
int rc = rgwlib.get_fe()->execute_req(&req);
if ((rc != 0) ||
((rc = req.get_ret()) != 0)) {
goto out;
ldout(get_context(), 1)
<< __func__
<< " rename step 0 failed src="
<< src_fh->full_object_name() << " " << src_name
<< " dst=" << dst_fh->full_object_name()
<< " " << dst_name
<< "rc " << rc
<< dendl;
goto unlock;
}
ldout(get_context(), 12)
<< __func__
<< " rename step 0 success src="
<< src_fh->full_object_name() << " " << src_name
<< " dst=" << dst_fh->full_object_name()
<< " " << dst_name
<< " rc " << rc
<< dendl;
/* update dst change id */
dst_fh->set_times(t);
}
break;
case 1:
@ -328,14 +366,37 @@ namespace rgw {
rc = this->unlink(rgw_fh /* LOCKED */, _src_name,
RGWFileHandle::FLAG_UNLINK_THIS);
/* !LOCKED, -ref */
if (! rc)
goto out;
if (! rc) {
ldout(get_context(), 12)
<< __func__
<< " rename step 1 success src="
<< src_fh->full_object_name() << " " << src_name
<< " dst=" << dst_fh->full_object_name()
<< " " << dst_name
<< " rc " << rc
<< dendl;
/* update src change id */
src_fh->set_times(t);
} else {
ldout(get_context(), 1)
<< __func__
<< " rename step 1 failed src="
<< src_fh->full_object_name() << " " << src_name
<< " dst=" << dst_fh->full_object_name()
<< " " << dst_name
<< " rc " << rc
<< dendl;
}
}
break;
default:
abort();
} /* switch */
} /* ix */
unlock:
rgw_fh->mtx.unlock(); /* !LOCKED */
unref(rgw_fh); /* -ref */
out:
return rc;
} /* RGWLibFS::rename */
@ -1222,6 +1283,12 @@ int rgw_getattr(struct rgw_fs *rgw_fs,
RGWLibFS *fs = static_cast<RGWLibFS*>(rgw_fs->fs_private);
RGWFileHandle* rgw_fh = get_rgwfh(fh);
int rc = -(fs->getattr(rgw_fh, st));
if (rc != 0) {
// do it again
rc = -(fs->getattr(rgw_fh, st));
}
return -(fs->getattr(rgw_fh, st));
}

42
src/rgw/rgw_file.h
View File

@ -153,6 +153,7 @@ namespace rgw {
{
struct rgw_file_handle fh;
std::mutex mtx;
RGWLibFS* fs;
RGWFileHandle* bucket;
RGWFileHandle* parent;
@ -232,6 +233,7 @@ namespace rgw {
static constexpr uint32_t FLAG_LOCK = 0x0040;
static constexpr uint32_t FLAG_DELETED = 0x0080;
static constexpr uint32_t FLAG_UNLINK_THIS = 0x0100;
static constexpr uint32_t FLAG_LOCKED = 0x0200;
#define CREATE_FLAGS(x) \
((x) & ~(RGWFileHandle::FLAG_CREATE|RGWFileHandle::FLAG_LOCK))
@ -811,6 +813,41 @@ namespace rgw {
return ret;
} /* authorize */
/* find RGWFileHandle by id */
LookupFHResult lookup_fh(const fh_key& fhk,
const uint32_t flags = RGWFileHandle::FLAG_NONE) {
using std::get;
// cast int32_t(RGWFileHandle::FLAG_NONE) due to strictness of Clang
// the cast transfers a lvalue into a rvalue in the ctor
// check the commit message for the full details
LookupFHResult fhr { nullptr, uint32_t(RGWFileHandle::FLAG_NONE) };
RGWFileHandle::FHCache::Latch lat;
retry:
RGWFileHandle* fh =
fh_cache.find_latch(fhk.fh_hk.object /* partition selector*/,
fhk /* key */, lat /* serializer */,
RGWFileHandle::FHCache::FLAG_LOCK);
/* LATCHED */
if (fh) {
fh->mtx.lock(); // XXX !RAII because may-return-LOCKED
/* need initial ref from LRU (fast path) */
if (! fh_lru.ref(fh, cohort::lru::FLAG_INITIAL)) {
lat.lock->unlock();
fh->mtx.unlock();
goto retry; /* !LATCHED */
}
/* LATCHED, LOCKED */
if (! (flags & RGWFileHandle::FLAG_LOCK))
fh->mtx.unlock(); /* ! LOCKED */
}
lat.lock->unlock(); /* !LATCHED */
get<0>(fhr) = fh;
return fhr;
} /* lookup_fh(const fh_key&) */
/* find or create an RGWFileHandle */
LookupFHResult lookup_fh(RGWFileHandle* parent, const char *name,
const uint32_t flags = RGWFileHandle::FLAG_NONE) {
@ -887,7 +924,7 @@ namespace rgw {
out:
get<0>(fhr) = fh;
return fhr;
}
} /* lookup_fh(RGWFileHandle*, const char *, const uint32_t) */
inline void unref(RGWFileHandle* fh) {
(void) fh_lru.unref(fh, cohort::lru::FLAG_NONE);
@ -953,7 +990,7 @@ namespace rgw {
if (fh->flags & RGWFileHandle::FLAG_DELETED) {
/* for now, delay briefly and retry */
lat.lock->unlock();
fh->mtx.unlock();
fh->mtx.unlock(); /* !LOCKED */
std::this_thread::sleep_for(std::chrono::milliseconds(20));
goto retry; /* !LATCHED */
}
@ -965,6 +1002,7 @@ namespace rgw {
/* LATCHED */
out:
lat.lock->unlock(); /* !LATCHED */
fh->mtx.unlock(); /* !LOCKED */
return fh;
}