From 275aaf406dc6245f17433c33bdd86fac8395cde8 Mon Sep 17 00:00:00 2001 From: Igor Fedotov Date: Fri, 15 Jan 2016 15:20:23 +0300 Subject: [PATCH] Fixing both the subissues from #14383 by using intrusive_ptr instead of Dir* in BlueFS::dir_map Signed-off-by: Igor Fedotov --- src/os/bluestore/BlueFS.cc | 51 ++++++++++++++++++-------------------- src/os/bluestore/BlueFS.h | 12 +++++++-- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc index d32d7d6f83f45..520ac77019db3 100644 --- a/src/os/bluestore/BlueFS.cc +++ b/src/os/bluestore/BlueFS.cc @@ -284,9 +284,7 @@ void BlueFS::umount() block_all.clear(); _stop_alloc(); file_map.clear(); - for (auto& p : dir_map) { - delete p.second; - } + dir_map.clear(); super = bluefs_super_t(); log_t.clear(); } @@ -490,7 +488,7 @@ int BlueFS::_replay() << dendl; FileRef file = _get_file(ino); assert(file->fnode.ino); - map::iterator q = dir_map.find(dirname); + map::iterator q = dir_map.find(dirname); assert(q != dir_map.end()); map::iterator r = q->second->file_map.find(filename); assert(r == q->second->file_map.end()); @@ -506,7 +504,7 @@ int BlueFS::_replay() ::decode(filename, p); dout(20) << __func__ << " " << pos << ": op_dir_unlink " << " " << dirname << "/" << filename << dendl; - map::iterator q = dir_map.find(dirname); + map::iterator q = dir_map.find(dirname); assert(q != dir_map.end()); map::iterator r = q->second->file_map.find(filename); assert(r != q->second->file_map.end()); @@ -521,7 +519,7 @@ int BlueFS::_replay() ::decode(dirname, p); dout(20) << __func__ << " " << pos << ": op_dir_create " << dirname << dendl; - map::iterator q = dir_map.find(dirname); + map::iterator q = dir_map.find(dirname); assert(q == dir_map.end()); dir_map[dirname] = new Dir; } @@ -533,10 +531,9 @@ int BlueFS::_replay() ::decode(dirname, p); dout(20) << __func__ << " " << pos << ": op_dir_remove " << dirname << dendl; - map::iterator q = dir_map.find(dirname); + map::iterator q = dir_map.find(dirname); assert(q != dir_map.end()); assert(q->second->file_map.empty()); - delete q->second; dir_map.erase(q); } break; @@ -1235,8 +1232,8 @@ int BlueFS::open_for_write( { Mutex::Locker l(lock); dout(10) << __func__ << " " << dirname << "/" << filename << dendl; - map::iterator p = dir_map.find(dirname); - Dir *dir; + map::iterator p = dir_map.find(dirname); + DirRef dir; if (p == dir_map.end()) { // implicitly create the dir dout(20) << __func__ << " dir " << dirname @@ -1322,12 +1319,12 @@ int BlueFS::open_for_read( Mutex::Locker l(lock); dout(10) << __func__ << " " << dirname << "/" << filename << (random ? " (random)":" (sequential)") << dendl; - map::iterator p = dir_map.find(dirname); + map::iterator p = dir_map.find(dirname); if (p == dir_map.end()) { dout(20) << __func__ << " dir " << dirname << " not found" << dendl; return -ENOENT; } - Dir *dir = p->second; + DirRef dir = p->second; map::iterator q = dir->file_map.find(filename); if (q == dir->file_map.end()) { @@ -1351,12 +1348,12 @@ int BlueFS::rename( Mutex::Locker l(lock); dout(10) << __func__ << " " << old_dirname << "/" << old_filename << " -> " << new_dirname << "/" << new_filename << dendl; - map::iterator p = dir_map.find(old_dirname); + map::iterator p = dir_map.find(old_dirname); if (p == dir_map.end()) { dout(20) << __func__ << " dir " << old_dirname << " not found" << dendl; return -ENOENT; } - Dir *old_dir = p->second; + DirRef old_dir = p->second; map::iterator q = old_dir->file_map.find(old_filename); if (q == old_dir->file_map.end()) { dout(20) << __func__ << " dir " << old_dirname << " (" << old_dir @@ -1371,7 +1368,7 @@ int BlueFS::rename( dout(20) << __func__ << " dir " << new_dirname << " not found" << dendl; return -ENOENT; } - Dir *new_dir = p->second; + DirRef new_dir = p->second; q = new_dir->file_map.find(new_filename); if (q != new_dir->file_map.end()) { dout(20) << __func__ << " dir " << new_dirname << " (" << old_dir @@ -1397,7 +1394,7 @@ int BlueFS::mkdir(const string& dirname) { Mutex::Locker l(lock); dout(10) << __func__ << " " << dirname << dendl; - map::iterator p = dir_map.find(dirname); + map::iterator p = dir_map.find(dirname); if (p != dir_map.end()) { dout(20) << __func__ << " dir " << dirname << " exists" << dendl; return -EEXIST; @@ -1411,12 +1408,12 @@ int BlueFS::rmdir(const string& dirname) { Mutex::Locker l(lock); dout(10) << __func__ << " " << dirname << dendl; - map::iterator p = dir_map.find(dirname); + map::iterator p = dir_map.find(dirname); if (p == dir_map.end()) { dout(20) << __func__ << " dir " << dirname << " does not exist" << dendl; return -ENOENT; } - Dir *dir = p->second; + DirRef dir = p->second; if (!dir->file_map.empty()) { dout(20) << __func__ << " dir " << dirname << " not empty" << dendl; return -ENOTEMPTY; @@ -1429,7 +1426,7 @@ int BlueFS::rmdir(const string& dirname) bool BlueFS::dir_exists(const string& dirname) { Mutex::Locker l(lock); - map::iterator p = dir_map.find(dirname); + map::iterator p = dir_map.find(dirname); bool exists = p != dir_map.end(); dout(10) << __func__ << " " << dirname << " = " << (int)exists << dendl; return exists; @@ -1440,12 +1437,12 @@ int BlueFS::stat(const string& dirname, const string& filename, { Mutex::Locker l(lock); dout(10) << __func__ << " " << dirname << "/" << filename << dendl; - map::iterator p = dir_map.find(dirname); + map::iterator p = dir_map.find(dirname); if (p == dir_map.end()) { dout(20) << __func__ << " dir " << dirname << " not found" << dendl; return -ENOENT; } - Dir *dir = p->second; + DirRef dir = p->second; map::iterator q = dir->file_map.find(filename); if (q == dir->file_map.end()) { dout(20) << __func__ << " dir " << dirname << " (" << dir @@ -1468,12 +1465,12 @@ int BlueFS::lock_file(const string& dirname, const string& filename, { Mutex::Locker l(lock); dout(10) << __func__ << " " << dirname << "/" << filename << dendl; - map::iterator p = dir_map.find(dirname); + map::iterator p = dir_map.find(dirname); if (p == dir_map.end()) { dout(20) << __func__ << " dir " << dirname << " not found" << dendl; return -ENOENT; } - Dir *dir = p->second; + DirRef dir = p->second; map::iterator q = dir->file_map.find(filename); File *file; if (q == dir->file_map.end()) { @@ -1524,12 +1521,12 @@ int BlueFS::readdir(const string& dirname, vector *ls) } } else { // list files in dir - map::iterator p = dir_map.find(dirname); + map::iterator p = dir_map.find(dirname); if (p == dir_map.end()) { dout(20) << __func__ << " dir " << dirname << " not found" << dendl; return -ENOENT; } - Dir *dir = p->second; + DirRef dir = p->second; ls->reserve(dir->file_map.size() + 2); for (auto& q : dir->file_map) { ls->push_back(q.first); @@ -1544,12 +1541,12 @@ int BlueFS::unlink(const string& dirname, const string& filename) { Mutex::Locker l(lock); dout(10) << __func__ << " " << dirname << "/" << filename << dendl; - map::iterator p = dir_map.find(dirname); + map::iterator p = dir_map.find(dirname); if (p == dir_map.end()) { dout(20) << __func__ << " dir " << dirname << " not found" << dendl; return -ENOENT; } - Dir *dir = p->second; + DirRef dir = p->second; map::iterator q = dir->file_map.find(filename); if (q == dir->file_map.end()) { dout(20) << __func__ << " file " << dirname << "/" << filename diff --git a/src/os/bluestore/BlueFS.h b/src/os/bluestore/BlueFS.h index be1f88dea88a2..474a63a97b682 100644 --- a/src/os/bluestore/BlueFS.h +++ b/src/os/bluestore/BlueFS.h @@ -57,9 +57,17 @@ public: boost::intrusive::list_member_hook<>, &File::dirty_item> > dirty_file_list_t; - struct Dir { + struct Dir : public RefCountedObject { map file_map; + + friend void intrusive_ptr_add_ref(Dir *d) { + d->get(); + } + friend void intrusive_ptr_release(Dir *d) { + d->put(); + } }; + typedef boost::intrusive_ptr DirRef; struct FileWriter { FileRef file; @@ -152,7 +160,7 @@ private: Cond cond; // cache - map dir_map; ///< dirname -> Dir + map dir_map; ///< dirname -> Dir ceph::unordered_map file_map; ///< ino -> File dirty_file_list_t dirty_files; ///< list of dirty files -- 2.39.5