RepoMirrors/ceph
1
0
Fork 0
mirror of https://github.com/ceph/ceph synced 2024-12-29 15:03:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

librbd: possible deadlock with synchronous maintenance operations

Browse Source 
Fixes: http://tracker.ceph.com/issues/22120
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
This commit is contained in:
Jason Dillaman 2017-11-13 13:28:06 -05:00
parent 8fa17e8ad6
commit 90b7ecd8a8
3 changed files with 82 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

153
src/librbd/Operations.cc
View File

@ -539,9 +539,11 @@ int Operations<I>::rename(const char *dstname) {
return r;
}
} else {
RWLock::RLocker owner_lock(m_image_ctx.owner_lock);
C_SaferCond cond_ctx;
execute_rename(dstname, &cond_ctx);
{
RWLock::RLocker owner_lock(m_image_ctx.owner_lock);
execute_rename(dstname, &cond_ctx);
}
r = cond_ctx.wait();
if (r < 0) {
@ -759,36 +761,35 @@ int Operations<I>::snap_rollback(const cls::rbd::SnapshotNamespace& snap_namespa
if (r < 0)
return r;
RWLock::RLocker owner_locker(m_image_ctx.owner_lock);
C_SaferCond cond_ctx;
{
// need to drop snap_lock before invalidating cache
RWLock::RLocker snap_locker(m_image_ctx.snap_lock);
if (!m_image_ctx.snap_exists) {
return -ENOENT;
RWLock::RLocker owner_locker(m_image_ctx.owner_lock);
{
// need to drop snap_lock before invalidating cache
RWLock::RLocker snap_locker(m_image_ctx.snap_lock);
if (!m_image_ctx.snap_exists) {
return -ENOENT;
}
if (m_image_ctx.snap_id != CEPH_NOSNAP || m_image_ctx.read_only) {
return -EROFS;
}
uint64_t snap_id = m_image_ctx.get_snap_id(snap_namespace, snap_name);
if (snap_id == CEPH_NOSNAP) {
lderr(cct) << "No such snapshot found." << dendl;
return -ENOENT;
}
}
if (m_image_ctx.snap_id != CEPH_NOSNAP || m_image_ctx.read_only) {
r = prepare_image_update(false);
if (r < 0) {
return -EROFS;
}
uint64_t snap_id = m_image_ctx.get_snap_id(snap_namespace, snap_name);
if (snap_id == CEPH_NOSNAP) {
lderr(cct) << "No such snapshot found." << dendl;
return -ENOENT;
}
execute_snap_rollback(snap_namespace, snap_name, prog_ctx, &cond_ctx);
}
r = prepare_image_update();
if (r < 0) {
return -EROFS;
}
if (m_image_ctx.exclusive_lock != nullptr &&
!m_image_ctx.exclusive_lock->is_lock_owner()) {
return -EROFS;
}
C_SaferCond cond_ctx;
execute_snap_rollback(snap_namespace, snap_name, prog_ctx, &cond_ctx);
r = cond_ctx.wait();
if (r < 0) {
return r;
@ -975,9 +976,11 @@ int Operations<I>::snap_rename(const char *srcname, const char *dstname) {
return r;
}
} else {
RWLock::RLocker owner_lock(m_image_ctx.owner_lock);
C_SaferCond cond_ctx;
execute_snap_rename(snap_id, dstname, &cond_ctx);
{
RWLock::RLocker owner_lock(m_image_ctx.owner_lock);
execute_snap_rename(snap_id, dstname, &cond_ctx);
}
r = cond_ctx.wait();
if (r < 0) {
@ -1067,9 +1070,11 @@ int Operations<I>::snap_protect(const cls::rbd::SnapshotNamespace& snap_namespac
return r;
}
} else {
RWLock::RLocker owner_lock(m_image_ctx.owner_lock);
C_SaferCond cond_ctx;
execute_snap_protect(snap_namespace, snap_name, &cond_ctx);
{
RWLock::RLocker owner_lock(m_image_ctx.owner_lock);
execute_snap_protect(snap_namespace, snap_name, &cond_ctx);
}
r = cond_ctx.wait();
if (r < 0) {
@ -1155,9 +1160,11 @@ int Operations<I>::snap_unprotect(const cls::rbd::SnapshotNamespace& snap_namesp
return r;
}
} else {
RWLock::RLocker owner_lock(m_image_ctx.owner_lock);
C_SaferCond cond_ctx;
execute_snap_unprotect(snap_namespace, snap_name, &cond_ctx);
{
RWLock::RLocker owner_lock(m_image_ctx.owner_lock);
execute_snap_unprotect(snap_namespace, snap_name, &cond_ctx);
}
r = cond_ctx.wait();
if (r < 0) {
@ -1216,25 +1223,18 @@ int Operations<I>::snap_set_limit(uint64_t limit) {
return r;
}
C_SaferCond limit_ctx;
{
RWLock::RLocker owner_lock(m_image_ctx.owner_lock);
C_SaferCond limit_ctx;
if (m_image_ctx.exclusive_lock != nullptr &&
!m_image_ctx.exclusive_lock->is_lock_owner()) {
C_SaferCond lock_ctx;
m_image_ctx.exclusive_lock->acquire_lock(&lock_ctx);
r = lock_ctx.wait();
if (r < 0) {
return r;
}
r = prepare_image_update(true);
if (r < 0) {
return r;
}
execute_snap_set_limit(limit, &limit_ctx);
r = limit_ctx.wait();
}
r = limit_ctx.wait();
return r;
}
@ -1372,25 +1372,18 @@ int Operations<I>::metadata_set(const std::string &key,
return -EROFS;
}
C_SaferCond metadata_ctx;
{
RWLock::RLocker owner_lock(m_image_ctx.owner_lock);
C_SaferCond metadata_ctx;
if (m_image_ctx.exclusive_lock != nullptr &&
!m_image_ctx.exclusive_lock->is_lock_owner()) {
C_SaferCond lock_ctx;
m_image_ctx.exclusive_lock->acquire_lock(&lock_ctx);
r = lock_ctx.wait();
if (r < 0) {
return r;
}
r = prepare_image_update(true);
if (r < 0) {
return r;
}
execute_metadata_set(key, value, &metadata_ctx);
r = metadata_ctx.wait();
}
r = metadata_ctx.wait();
if (config_override && r >= 0) {
// apply new config key immediately
r = m_image_ctx.state->refresh_if_required();
@ -1439,25 +1432,19 @@ int Operations<I>::metadata_remove(const std::string &key) {
if(r < 0)
return r;
C_SaferCond metadata_ctx;
{
RWLock::RLocker owner_lock(m_image_ctx.owner_lock);
C_SaferCond metadata_ctx;
if (m_image_ctx.exclusive_lock != nullptr &&
!m_image_ctx.exclusive_lock->is_lock_owner()) {
C_SaferCond lock_ctx;
m_image_ctx.exclusive_lock->acquire_lock(&lock_ctx);
r = lock_ctx.wait();
if (r < 0) {
return r;
}
r = prepare_image_update(true);
if (r < 0) {
return r;
}
execute_metadata_remove(key, &metadata_ctx);
r = metadata_ctx.wait();
}
r = metadata_ctx.wait();
std::string config_key;
if (util::is_metadata_config_override(key, &config_key) && r >= 0) {
// apply new config key immediately
@ -1483,34 +1470,52 @@ void Operations<I>::execute_metadata_remove(const std::string &key,
}
template <typename I>
int Operations<I>::prepare_image_update() {
int Operations<I>::prepare_image_update(bool request_lock) {
assert(m_image_ctx.owner_lock.is_locked() &&
!m_image_ctx.owner_lock.is_wlocked());
if (m_image_ctx.image_watcher == NULL) {
if (m_image_ctx.image_watcher == nullptr) {
return -EROFS;
}
// need to upgrade to a write lock
bool trying_lock = false;
C_SaferCond ctx;
m_image_ctx.owner_lock.put_read();
bool attempting_lock = false;
{
RWLock::WLocker owner_locker(m_image_ctx.owner_lock);
if (m_image_ctx.exclusive_lock != nullptr &&
(!m_image_ctx.exclusive_lock->is_lock_owner() ||
!m_image_ctx.exclusive_lock->accept_requests())) {
m_image_ctx.exclusive_lock->try_acquire_lock(&ctx);
trying_lock = true;
attempting_lock = true;
m_image_ctx.exclusive_lock->block_requests(0);
if (request_lock) {
m_image_ctx.exclusive_lock->acquire_lock(&ctx);
} else {
m_image_ctx.exclusive_lock->try_acquire_lock(&ctx);
}
}
}
int r = 0;
if (trying_lock) {
if (attempting_lock) {
r = ctx.wait();
}
m_image_ctx.owner_lock.get_read();
return r;
m_image_ctx.owner_lock.get_read();
if (attempting_lock && m_image_ctx.exclusive_lock != nullptr) {
m_image_ctx.exclusive_lock->unblock_requests();
}
if (r < 0) {
return r;
} else if (m_image_ctx.exclusive_lock != nullptr &&
!m_image_ctx.exclusive_lock->is_lock_owner()) {
return -EROFS;
}
return 0;
}
template <typename I>

2
src/librbd/Operations.h
View File

@ -101,7 +101,7 @@ public:
int metadata_remove(const std::string &key);
void execute_metadata_remove(const std::string &key, Context *on_finish);
int prepare_image_update();
int prepare_image_update(bool request_lock);
private:
ImageCtxT &m_image_ctx;

5
src/librbd/internal.cc
View File

@ -1349,9 +1349,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
image_id = ictx->id;
ictx->owner_lock.get_read();
if (ictx->exclusive_lock != nullptr) {
r = ictx->operations->prepare_image_update();
if (r < 0 || (ictx->exclusive_lock != nullptr &&
!ictx->exclusive_lock->is_lock_owner())) {
r = ictx->operations->prepare_image_update(false);
if (r < 0) {
lderr(cct) << "cannot obtain exclusive lock - not removing" << dendl;
ictx->owner_lock.put_read();
ictx->state->close();