Fixing both the subissues from #14383 by using intrusive_ptr instead of Dir* in BlueF...

Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>

diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc
index d32d7d6f83f4575c0631a7733a3907a85b11205a..520ac77019db38d9b9b935545e912c85533de5d2 100644 (file)
--- 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<string,Dir*>::iterator q = dir_map.find(dirname);
+         map<string,DirRef>::iterator q = dir_map.find(dirname);
          assert(q != dir_map.end());
          map<string,FileRef>::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<string,Dir*>::iterator q = dir_map.find(dirname);
+         map<string,DirRef>::iterator q = dir_map.find(dirname);
          assert(q != dir_map.end());
          map<string,FileRef>::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<string,Dir*>::iterator q = dir_map.find(dirname);
+         map<string,DirRef>::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<string,Dir*>::iterator q = dir_map.find(dirname);
+         map<string,DirRef>::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<string,Dir*>::iterator p = dir_map.find(dirname);
-  Dir *dir;
+  map<string,DirRef>::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<string,Dir*>::iterator p = dir_map.find(dirname);
+  map<string,DirRef>::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<string,FileRef>::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<string,Dir*>::iterator p = dir_map.find(old_dirname);
+  map<string,DirRef>::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<string,FileRef>::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<string,Dir*>::iterator p = dir_map.find(dirname);
+  map<string,DirRef>::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<string,Dir*>::iterator p = dir_map.find(dirname);
+  map<string,DirRef>::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<string,Dir*>::iterator p = dir_map.find(dirname);
+  map<string,DirRef>::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<string,Dir*>::iterator p = dir_map.find(dirname);
+  map<string,DirRef>::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<string,FileRef>::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<string,Dir*>::iterator p = dir_map.find(dirname);
+  map<string,DirRef>::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<string,FileRef>::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<string> *ls)
     }
   } else {
     // list files in dir
-    map<string,Dir*>::iterator p = dir_map.find(dirname);
+    map<string,DirRef>::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<string,Dir*>::iterator p = dir_map.find(dirname);
+  map<string,DirRef>::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<string,FileRef>::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 be1f88dea88a2633b5bf0ea1a8dd35be6a6dc8d4..474a63a97b682e0be6a0aa7c47fe62d53b97a349 100644 (file)
--- 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<string,FileRef> 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<Dir> DirRef;
 
   struct FileWriter {
     FileRef file;
@@ -152,7 +160,7 @@ private:
   Cond cond;
 
   // cache
-  map<string, Dir*> dir_map;                      ///< dirname -> Dir
+  map<string, DirRef> dir_map;                    ///< dirname -> Dir
   ceph::unordered_map<uint64_t,FileRef> file_map; ///< ino -> File
   dirty_file_list_t dirty_files;                  ///< list of dirty files