From 364563aac979fdf5ccbb6c588051d097a26bc594 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Mon, 16 Mar 2015 11:04:22 -0400 Subject: [PATCH] librbd: acquire cache_lock before refreshing parent cache_lock needs to be acquired before snap_lock to avoid the potential for deadlock. Fixes: #5488 Signed-off-by: Jason Dillaman --- src/librbd/ImageCtx.cc | 2 -- src/librbd/internal.cc | 18 ++++++++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc index 8fb8e37129c91..1295d421c2353 100644 --- a/src/librbd/ImageCtx.cc +++ b/src/librbd/ImageCtx.cc @@ -603,9 +603,7 @@ namespace librbd { void ImageCtx::clear_nonexistence_cache() { if (!object_cacher) return; - cache_lock.Lock(); object_cacher->clear_nonexistence(object_set); - cache_lock.Unlock(); } int ImageCtx::register_watch() { diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc index 598d515cde4a4..6ca8941214ac6 100644 --- a/src/librbd/internal.cc +++ b/src/librbd/internal.cc @@ -1286,11 +1286,13 @@ reprotect_and_return_err: return r; } + ictx->parent->cache_lock.Lock(); ictx->parent->snap_lock.get_write(); r = ictx->parent->get_snap_name(parent_snap_id, &ictx->parent->snap_name); if (r < 0) { lderr(ictx->cct) << "parent snapshot does not exist" << dendl; ictx->parent->snap_lock.put_write(); + ictx->parent->cache_lock.Unlock(); close_image(ictx->parent); ictx->parent = NULL; return r; @@ -1304,12 +1306,14 @@ reprotect_and_return_err: << ictx->parent->snap_name << dendl; ictx->parent->parent_lock.put_write(); ictx->parent->snap_lock.put_write(); + ictx->parent->cache_lock.Unlock(); close_image(ictx->parent); ictx->parent = NULL; return r; } ictx->parent->parent_lock.put_write(); ictx->parent->snap_lock.put_write(); + ictx->parent->cache_lock.Unlock(); return 0; } @@ -1707,10 +1711,11 @@ reprotect_and_return_err: vector snap_parents; vector snap_protection; { - RWLock::WLocker l(ictx->snap_lock); + Mutex::Locker cache_locker(ictx->cache_lock); + RWLock::WLocker snap_locker(ictx->snap_lock); { - int r; - RWLock::WLocker l2(ictx->parent_lock); + int r; + RWLock::WLocker parent_locker(ictx->parent_lock); ictx->lockers.clear(); if (ictx->old_format) { r = read_header(ictx->md_ctx, ictx->header_oid, &ictx->header, NULL); @@ -1836,7 +1841,7 @@ reprotect_and_return_err: } ictx->data_ctx.selfmanaged_snap_set_write_ctx(ictx->snapc.seq, ictx->snaps); - } // release snap_lock + } // release snap_lock and cache_lock if (new_snap) { _flush(ictx); @@ -2068,8 +2073,9 @@ reprotect_and_return_err: int _snap_set(ImageCtx *ictx, const char *snap_name) { - RWLock::WLocker l1(ictx->snap_lock); - RWLock::WLocker l2(ictx->parent_lock); + Mutex::Locker cache_locker(ictx->cache_lock); + RWLock::WLocker snap_locker(ictx->snap_lock); + RWLock::WLocker parent_locker(ictx->parent_lock); int r; if ((snap_name != NULL) && (strlen(snap_name) != 0)) { r = ictx->snap_set(snap_name); -- 2.39.5