os/bluestore: minor improvement

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

diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc
index 5752eb407cc909fb397ebbc6ec29f2692c1671f5..6c95d845d7d0d4e0c6250fdf5c17afdf30d35ab7 100644 (file)
--- a/src/os/bluestore/BlueFS.cc
+++ b/src/os/bluestore/BlueFS.cc
@@ -170,16 +170,14 @@ private:
   }
 };
 
-BlueFS::BlueFS(CephContext* cct,
-               bluefs_shared_alloc_context_t* _shared_alloc)
+BlueFS::BlueFS(CephContext* cct)
   : cct(cct),
     bdev(MAX_BDEV),
     ioc(MAX_BDEV),
     block_reserved(MAX_BDEV),
     alloc(MAX_BDEV),
     alloc_size(MAX_BDEV, 0),
-    pending_release(MAX_BDEV),
-    shared_alloc(_shared_alloc)
+    pending_release(MAX_BDEV)
 {
   discard_cb[BDEV_WAL] = wal_discard_cb;
   discard_cb[BDEV_DB] = db_discard_cb;
@@ -317,7 +315,7 @@ void BlueFS::_update_logger_stats()
 
 int BlueFS::add_block_device(unsigned id, const string& path, bool trim,
                              uint64_t reserved,
-                             bool shared_with_bluestore)
+                             bluefs_shared_alloc_context_t* _shared_alloc)
 {
   dout(10) << __func__ << " bdev " << id << " path " << path << " "
            << reserved << dendl;
@@ -326,7 +324,7 @@ int BlueFS::add_block_device(unsigned id, const string& path, bool trim,
   BlockDevice *b = BlockDevice::create(cct, path, NULL, NULL,
                                       discard_cb[id], static_cast<void*>(this));
   block_reserved[id] = reserved;
-  if (shared_with_bluestore) {
+  if (_shared_alloc) {
     b->set_no_exclusive_lock();
   }
   int r = b->open(path);
@@ -342,8 +340,9 @@ int BlueFS::add_block_device(unsigned id, const string& path, bool trim,
          << " size " << byte_u_t(b->get_size()) << dendl;
   bdev[id] = b;
   ioc[id] = new IOContext(cct, NULL);
-  if (shared_with_bluestore) {
-    ceph_assert(shared_alloc); // to be set in ctor before
+  if (_shared_alloc) {
+    ceph_assert(!shared_alloc);
+    shared_alloc = _shared_alloc;
     alloc[id] = shared_alloc->a;
     shared_alloc_id = id;
   }
@@ -453,11 +452,10 @@ int BlueFS::get_block_extents(unsigned id, interval_set<uint64_t> *extents)
 {
   std::lock_guard l(lock);
   dout(10) << __func__ << " bdev " << id << dendl;
-  if (id >= alloc.size())
-    return -EINVAL;
+  ceph_assert(id < alloc.size());
   for (auto& p : file_map) {
     for (auto& q : p.second->fnode.extents) {
-      if (q.bdev == id && alloc[q.bdev] == shared_alloc->a) {
+      if (q.bdev == id) {
         extents->insert(q.offset, q.length);
       }
     }
@@ -600,7 +598,6 @@ int BlueFS::mount()
 {
   dout(1) << __func__ << dendl;
 
-  bool shared_alloc_ready = shared_alloc && shared_alloc->a;
   int r = _open_super();
   if (r < 0) {
     derr << __func__ << " failed to open super: " << cpp_strerror(r) << dendl;
@@ -630,25 +627,24 @@ int BlueFS::mount()
   for (auto& p : file_map) {
     dout(30) << __func__ << " noting alloc for " << p.second->fnode << dendl;
     for (auto& q : p.second->fnode.extents) {
-      if (is_shared_alloc(q.bdev)) {
-        // we might have still uninitialized shared_alloc at this point
-        // just bypass initialization then
-        if (shared_alloc_ready && shared_alloc->need_init) {
-          ceph_assert(shared_alloc->a);
-          alloc[q.bdev]->init_rm_free(q.offset, q.length);
-          shared_alloc->bluefs_used += q.length;
-        }
-      } else {
+      bool is_shared = is_shared_alloc(q.bdev);
+      ceph_assert(!is_shared || (is_shared && shared_alloc));
+      if (is_shared && shared_alloc->need_init && shared_alloc->a) {
+        shared_alloc->bluefs_used += q.length;
+        alloc[q.bdev]->init_rm_free(q.offset, q.length);
+      } else if (!is_shared) {
         alloc[q.bdev]->init_rm_free(q.offset, q.length);
       }
     }
   }
-  if (shared_alloc_ready) {
+  if (shared_alloc) {
     shared_alloc->need_init = false;
+    dout(1) << __func__ << " shared_bdev_used = "
+            << shared_alloc->bluefs_used << dendl;
+  } else {
+    dout(1) << __func__ << " shared bdev not used"
+            << dendl;
   }
-  dout(1) << __func__ << " shared_bdev_used = "
-          << (shared_alloc_ready ? (int64_t)shared_alloc->bluefs_used : -1)
-          << dendl;
 
   // set up the log for future writes
   log_writer = _create_writer(_get_file(1));

diff --git a/src/os/bluestore/BlueFS.h b/src/os/bluestore/BlueFS.h
index b14b94b911d7cf1c2fcd3365b94dac648a66d495..d7ac37488deba7037c35192aeda663c5f7755dcb 100644 (file)
--- a/src/os/bluestore/BlueFS.h
+++ b/src/os/bluestore/BlueFS.h
@@ -324,6 +324,7 @@ private:
   BlockDevice::aio_callback_t discard_cb[3]; //discard callbacks for each dev
 
   std::unique_ptr<BlueFSVolumeSelector> vselector;
+
   bluefs_shared_alloc_context_t* shared_alloc = nullptr;
   unsigned shared_alloc_id = unsigned(-1);
   inline bool is_shared_alloc(unsigned id) const {
@@ -438,7 +439,7 @@ private:
   }
 
 public:
-  BlueFS(CephContext* cct, bluefs_shared_alloc_context_t* _shared_alloc);
+  BlueFS(CephContext* cct);
   ~BlueFS();
 
   // the super is always stored on bdev 0
@@ -533,7 +534,7 @@ public:
 
   int add_block_device(unsigned bdev, const std::string& path, bool trim,
                        uint64_t reserved,
-                      bool shared_with_bluestore = false);
+                      bluefs_shared_alloc_context_t* _shared_alloc = nullptr);
   bool bdev_support_label(unsigned id);
   uint64_t get_block_device_size(unsigned bdev) const;
 

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 61854c183e36f2bc3d5c3a252a558abcb33ca576..4a3d2c54ee6cc4a475bae69cef0e5ea01768f4ee 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -5055,7 +5055,7 @@ int BlueStore::_write_out_fm_meta(uint64_t target_size)
   return r;
 }
 
-int BlueStore::_open_alloc()
+int BlueStore::_create_alloc()
 {
   ceph_assert(shared_alloc.a == NULL);
   ceph_assert(bdev->get_size());
@@ -5069,15 +5069,25 @@ int BlueStore::_open_alloc()
   }
 
   shared_alloc.set(Allocator::create(cct, cct->_conf->bluestore_allocator,
-                            bdev->get_size(),
-                            alloc_size, "block"));
+    bdev->get_size(),
+    alloc_size, "block"));
 
   if (!shared_alloc.a) {
-    lderr(cct) << __func__ << " Allocator::unknown alloc type "
-               << cct->_conf->bluestore_allocator
-               << dendl;
+    lderr(cct) << __func__ << "Failed to create allocator:: "
+      << cct->_conf->bluestore_allocator
+      << dendl;
     return -EINVAL;
   }
+  return 0;
+}
+
+int BlueStore::_init_alloc()
+{
+  int r = _create_alloc();
+  if (r < 0) {
+    return r;
+  }
+  ceph_assert(shared_alloc.a != NULL);
 
   if (bdev->is_smr()) {
     shared_alloc.a->set_zone_states(fm->get_zone_states(db));
@@ -5274,7 +5284,7 @@ bool BlueStore::test_mount_in_use()
 int BlueStore::_minimal_open_bluefs(bool create)
 {
   int r;
-  bluefs = new BlueFS(cct, &shared_alloc);
+  bluefs = new BlueFS(cct);
 
   string bfn;
   struct stat st;
@@ -5322,7 +5332,7 @@ int BlueStore::_minimal_open_bluefs(bool create)
   // never trim here
   r = bluefs->add_block_device(bluefs_layout.shared_bdev, bfn, false,
                                0, // no need to provide valid 'reserved' for shared dev
-                               true);
+                               &shared_alloc);
   if (r < 0) {
     derr << __func__ << " add block device(" << bfn << ") returned: "
          << cpp_strerror(r) << dendl;
@@ -5497,7 +5507,6 @@ int BlueStore::_open_db_and_around(bool read_only, bool to_repair)
 
   // open in read-only first to read FM list and init allocator
   // as they might be needed for some BlueFS procedures
-
   r = _open_db(false, false, true);
   if (r < 0)
     goto out_bdev;
@@ -5511,7 +5520,7 @@ int BlueStore::_open_db_and_around(bool read_only, bool to_repair)
   if (r < 0)
     goto out_db;
 
-  r = _open_alloc();
+  r = _init_alloc();
   if (r < 0)
     goto out_fm;
 
@@ -5525,11 +5534,13 @@ int BlueStore::_open_db_and_around(bool read_only, bool to_repair)
 
   r = _open_db(false, to_repair, read_only);
   if (r < 0) {
-    goto out_fm;
+    goto out_alloc;
   }
   return 0;
 
- out_fm:
+out_alloc:
+  _close_alloc();
+out_fm:
   _close_fm();
  out_db:
   _close_db(read_only);
@@ -5545,8 +5556,8 @@ int BlueStore::_open_db_and_around(bool read_only, bool to_repair)
 void BlueStore::_close_db_and_around(bool read_only)
 {
   _close_db(read_only);
-  _close_alloc();
   _close_fm();
+  _close_alloc();
   _close_bdev();
   _close_fsid();
   _close_path();
@@ -6119,28 +6130,18 @@ int BlueStore::mkfs()
     goto out_close_bdev;
   }
 
-  uint64_t alloc_size;
-  alloc_size = min_alloc_size;
-  if (bdev->is_smr()) {
-    int r = _zoned_check_config_settings();
-    if (r < 0)
-      return r;
-    alloc_size = _zoned_piggyback_device_parameters_onto(alloc_size);
-  }
-  shared_alloc.set(Allocator::create(cct, cct->_conf->bluestore_allocator,
-    bdev->get_size(),
-    alloc_size, "block"));
-  if (!shared_alloc.a) {
-    r = -EINVAL;
+  r = _create_alloc();
+  if (r < 0) {
     goto out_close_bdev;
   }
+
   reserved = _get_ondisk_reserved();
   shared_alloc.a->init_add_free(reserved,
     p2align(bdev->get_size(), min_alloc_size) - reserved);
 
   r = _open_db(true);
   if (r < 0)
-    goto out_close_bdev;
+    goto out_close_alloc;
 
   {
     KeyValueDB::Transaction t = db->get_transaction();
@@ -6189,9 +6190,9 @@ int BlueStore::mkfs()
   _close_fm();
  out_close_db:
   _close_db(false);
+ out_close_alloc:
+  _close_alloc();
  out_close_bdev:
-  delete shared_alloc.a;
-  shared_alloc.reset();
   _close_bdev();
  out_close_fsid:
   _close_fsid();

diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index 27ab942316b3718dc000e34eb16f2b5c18b4aa1d..9bdf6f976b2b03154230cf60583ef19837cc7fa3 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -2380,7 +2380,8 @@ private:
   int _open_fm(KeyValueDB::Transaction t, bool read_only);
   void _close_fm();
   int _write_out_fm_meta(uint64_t target_size);
-  int _open_alloc();
+  int _create_alloc();
+  int _init_alloc();
   void _close_alloc();
   int _open_collections();
   void _fsck_collections(int64_t* errors);

diff --git a/src/os/bluestore/bluestore_tool.cc b/src/os/bluestore/bluestore_tool.cc
index 899d17a6bfe26a7872b8e11d03a90671142988ba..9497232dc1af85d6d4a19d34503456039833a596 100644 (file)
--- a/src/os/bluestore/bluestore_tool.cc
+++ b/src/os/bluestore/bluestore_tool.cc
@@ -161,6 +161,9 @@ void add_devices(
       cout << " -> " << target_path;
     }
     cout << std::endl;
+
+    // We provide no shared allocator which prevents bluefs to operate in R/W mode.
+    // Read-only mode isn't strictly enforced though
     int r = fs->add_block_device(e.second, e.first, false, 0); // 'reserved' is fake
     if (r < 0) {
       cerr << "unable to open " << e.first << ": " << cpp_strerror(r) << std::endl;
@@ -175,9 +178,7 @@ BlueFS *open_bluefs_readonly(
   const vector<string>& devs)
 {
   validate_path(cct, path, true);
-  // We provide no shared allocator which prevents bluefs to operate in R/W mode.
-  // Read-only mode isn't strictly enforced though
-  BlueFS *fs = new BlueFS(cct, nullptr);
+  BlueFS *fs = new BlueFS(cct);
 
   add_devices(fs, cct, devs);
 
@@ -196,9 +197,7 @@ void log_dump(
   const vector<string>& devs)
 {
   validate_path(cct, path, true);
-  // We provide no shared allocator which prevents bluefs to operate in R/W mode.
-  // Read-only mode isn't strictly enforced though
-  BlueFS *fs = new BlueFS(cct, nullptr);
+  BlueFS *fs = new BlueFS(cct);
 
   add_devices(fs, cct, devs);
   int r = fs->log_dump();

diff --git a/src/test/objectstore/test_bluefs.cc b/src/test/objectstore/test_bluefs.cc
index add3f86c62c1033d6d2212c9952fa4188f0867de..4bbd2e6dd04f6109c16bd0c575e7b614cbc16b7c 100644 (file)
--- a/src/test/objectstore/test_bluefs.cc
+++ b/src/test/objectstore/test_bluefs.cc
@@ -89,7 +89,7 @@ TEST(BlueFS, mkfs) {
   uint64_t size = 1048576 * 128;
   TempBdev bdev{size};
   uuid_d fsid;
-  BlueFS fs(g_ceph_context, nullptr);
+  BlueFS fs(g_ceph_context);
   ASSERT_EQ(0, fs.add_block_device(BlueFS::BDEV_DB, bdev.path, false, 1048576));
   ASSERT_EQ(0, fs.mkfs(fsid, { BlueFS::BDEV_DB, false, false }));
 }
@@ -97,7 +97,7 @@ TEST(BlueFS, mkfs) {
 TEST(BlueFS, mkfs_mount) {
   uint64_t size = 1048576 * 128;
   TempBdev bdev{size};
-  BlueFS fs(g_ceph_context, nullptr);
+  BlueFS fs(g_ceph_context);
   ASSERT_EQ(0, fs.add_block_device(BlueFS::BDEV_DB, bdev.path, false, 1048576));
   uuid_d fsid;
   ASSERT_EQ(0, fs.mkfs(fsid, { BlueFS::BDEV_DB, false, false }));
@@ -111,7 +111,7 @@ TEST(BlueFS, mkfs_mount) {
 TEST(BlueFS, write_read) {
   uint64_t size = 1048576 * 128;
   TempBdev bdev{size};
-  BlueFS fs(g_ceph_context, nullptr);
+  BlueFS fs(g_ceph_context);
   ASSERT_EQ(0, fs.add_block_device(BlueFS::BDEV_DB, bdev.path, false, 1048576));
   uuid_d fsid;
   ASSERT_EQ(0, fs.mkfs(fsid, { BlueFS::BDEV_DB, false, false }));
@@ -141,7 +141,7 @@ TEST(BlueFS, write_read) {
 TEST(BlueFS, small_appends) {
   uint64_t size = 1048576 * 128;
   TempBdev bdev{size};
-  BlueFS fs(g_ceph_context, nullptr);
+  BlueFS fs(g_ceph_context);
   ASSERT_EQ(0, fs.add_block_device(BlueFS::BDEV_DB, bdev.path, false, 1048576));
   uuid_d fsid;
   ASSERT_EQ(0, fs.mkfs(fsid, { BlueFS::BDEV_DB, false, false }));
@@ -173,7 +173,7 @@ TEST(BlueFS, very_large_write) {
   // we'll write a ~5G file, so allocate more than that for the whole fs
   uint64_t size = 1048576 * 1024 * 8ull;
   TempBdev bdev{size};
-  BlueFS fs(g_ceph_context, nullptr);
+  BlueFS fs(g_ceph_context);
 
   bool old = g_ceph_context->_conf.get_val<bool>("bluefs_buffered_io");
   g_ceph_context->_conf.set_val("bluefs_buffered_io", "false");
@@ -363,7 +363,7 @@ TEST(BlueFS, test_flush_1) {
     "65536");
   g_ceph_context->_conf.apply_changes(nullptr);
 
-  BlueFS fs(g_ceph_context, nullptr);
+  BlueFS fs(g_ceph_context);
   ASSERT_EQ(0, fs.add_block_device(BlueFS::BDEV_DB, bdev.path, false, 1048576));
   uuid_d fsid;
   ASSERT_EQ(0, fs.mkfs(fsid, { BlueFS::BDEV_DB, false, false }));
@@ -397,7 +397,7 @@ TEST(BlueFS, test_flush_2) {
     "65536");
   g_ceph_context->_conf.apply_changes(nullptr);
 
-  BlueFS fs(g_ceph_context, nullptr);
+  BlueFS fs(g_ceph_context);
   ASSERT_EQ(0, fs.add_block_device(BlueFS::BDEV_DB, bdev.path, false, 1048576));
   uuid_d fsid;
   ASSERT_EQ(0, fs.mkfs(fsid, { BlueFS::BDEV_DB, false, false }));
@@ -424,7 +424,7 @@ TEST(BlueFS, test_flush_3) {
     "65536");
   g_ceph_context->_conf.apply_changes(nullptr);
 
-  BlueFS fs(g_ceph_context, nullptr);
+  BlueFS fs(g_ceph_context);
   ASSERT_EQ(0, fs.add_block_device(BlueFS::BDEV_DB, bdev.path, false, 1048576));
   uuid_d fsid;
   ASSERT_EQ(0, fs.mkfs(fsid, { BlueFS::BDEV_DB, false, false }));
@@ -457,7 +457,7 @@ TEST(BlueFS, test_simple_compaction_sync) {
   uint64_t size = 1048576 * 128;
   TempBdev bdev{size};
 
-  BlueFS fs(g_ceph_context, nullptr);
+  BlueFS fs(g_ceph_context);
   ASSERT_EQ(0, fs.add_block_device(BlueFS::BDEV_DB, bdev.path, false, 1048576));
   uuid_d fsid;
   ASSERT_EQ(0, fs.mkfs(fsid, { BlueFS::BDEV_DB, false, false }));
@@ -509,7 +509,7 @@ TEST(BlueFS, test_simple_compaction_async) {
   uint64_t size = 1048576 * 128;
   TempBdev bdev{size};
 
-  BlueFS fs(g_ceph_context, nullptr);
+  BlueFS fs(g_ceph_context);
   ASSERT_EQ(0, fs.add_block_device(BlueFS::BDEV_DB, bdev.path, false, 1048576));
   uuid_d fsid;
   ASSERT_EQ(0, fs.mkfs(fsid, { BlueFS::BDEV_DB, false, false }));
@@ -564,7 +564,7 @@ TEST(BlueFS, test_compaction_sync) {
     "bluefs_compact_log_sync",
     "true");
 
-  BlueFS fs(g_ceph_context, nullptr);
+  BlueFS fs(g_ceph_context);
   ASSERT_EQ(0, fs.add_block_device(BlueFS::BDEV_DB, bdev.path, false, 1048576));
   uuid_d fsid;
   ASSERT_EQ(0, fs.mkfs(fsid, { BlueFS::BDEV_DB, false, false }));
@@ -601,7 +601,7 @@ TEST(BlueFS, test_compaction_async) {
     "bluefs_compact_log_sync",
     "false");
 
-  BlueFS fs(g_ceph_context, nullptr);
+  BlueFS fs(g_ceph_context);
   ASSERT_EQ(0, fs.add_block_device(BlueFS::BDEV_DB, bdev.path, false, 1048576));
   uuid_d fsid;
   ASSERT_EQ(0, fs.mkfs(fsid, { BlueFS::BDEV_DB, false, false }));
@@ -638,7 +638,7 @@ TEST(BlueFS, test_replay) {
     "bluefs_compact_log_sync",
     "false");
 
-  BlueFS fs(g_ceph_context, nullptr);
+  BlueFS fs(g_ceph_context);
   ASSERT_EQ(0, fs.add_block_device(BlueFS::BDEV_DB, bdev.path, false, 1048576));
   uuid_d fsid;
   ASSERT_EQ(0, fs.mkfs(fsid, { BlueFS::BDEV_DB, false, false }));
@@ -683,7 +683,7 @@ TEST(BlueFS, test_replay_growth) {
   conf.SetVal("bluefs_sync_write", "true");
   conf.ApplyChanges();
 
-  BlueFS fs(g_ceph_context, nullptr);
+  BlueFS fs(g_ceph_context);
   ASSERT_EQ(0, fs.add_block_device(BlueFS::BDEV_DB, bdev.path, false, 1048576));
   uuid_d fsid;
   ASSERT_EQ(0, fs.mkfs(fsid, { BlueFS::BDEV_DB, false, false }));