seastore/omap_manager/btree: change omap manager funcs to coroutines

This commit changes funcs in BTree OMap manager to coroutines. Apart
from cleaner code that's easier to follow this is done to fix ASan
heap-use-after-free asserts.

Example QA job with the error: https://pulpito.ceph.com/shraddhaag-2026-04-20_07:04:25-crimson-rados-main-distro-debug-trial/164374/
Signed-off-by: Shraddha Agrawal <shraddha.agrawal000@gmail.com>

diff --git a/src/crimson/os/seastore/omap_manager.h b/src/crimson/os/seastore/omap_manager.h
index ee2ed7624c203ad0abb1f3f316263c08b435905c..8f75fc6eec2db239546435fe9debe31aa98d0a54 100644 (file)
--- a/src/crimson/os/seastore/omap_manager.h
+++ b/src/crimson/os/seastore/omap_manager.h
@@ -75,7 +75,7 @@ public:
   virtual omap_get_value_ret omap_get_value(
     const omap_root_t &omap_root,
     Transaction &t,
-    const std::string &key) = 0;
+    std::string key) = 0;
 
   /**
    * set key value mapping in omap
@@ -91,15 +91,15 @@ public:
   virtual omap_set_key_ret omap_set_key(
     omap_root_t &omap_root,
     Transaction &t,
-    const std::string &key,
-    const ceph::bufferlist &value) = 0;
+    std::string key,
+    ceph::bufferlist value) = 0;
 
   using omap_set_keys_iertr = omap_set_key_iertr;
   using omap_set_keys_ret = omap_set_keys_iertr::future<>;
   virtual omap_set_keys_ret omap_set_keys(
     omap_root_t &omap_root,
     Transaction &t,
-    std::map<std::string, ceph::bufferlist>&& keys) = 0;
+    std::map<std::string, ceph::bufferlist> keys) = 0;
 
   /**
    * remove key value mapping in omap tree
@@ -113,14 +113,14 @@ public:
   virtual omap_rm_key_ret omap_rm_key(
     omap_root_t &omap_root,
     Transaction &t,
-    const std::string &key) = 0;
+    std::string key) = 0;
 
   using omap_rm_keys_iertr = base_iertr;
   using omap_rm_keys_ret = omap_rm_keys_iertr::future<>;
   virtual omap_rm_keys_ret omap_rm_keys(
     omap_root_t& root,
     Transaction& t,
-    std::set<std::string>& keys) = 0;
+    std::set<std::string> keys) = 0;
 
   /**
    * omap_iterate

diff --git a/src/crimson/os/seastore/omap_manager/btree/btree_omap_manager.cc b/src/crimson/os/seastore/omap_manager/btree/btree_omap_manager.cc
index ef5ed822185d28607a28ccd71b001becd8fd5123..5e004597111f6b12b41d82ba35623689043619b6 100644 (file)
--- a/src/crimson/os/seastore/omap_manager/btree/btree_omap_manager.cc
+++ b/src/crimson/os/seastore/omap_manager/btree/btree_omap_manager.cc
@@ -130,117 +130,79 @@ BtreeOMapManager::omap_get_value_ret
 BtreeOMapManager::omap_get_value(
   const omap_root_t &omap_root,
   Transaction &t,
-  const std::string &key)
+  std::string key)
 {
   LOG_PREFIX(BtreeOMapManager::omap_get_value);
   DEBUGT("key={}", t, key);
-  return get_omap_root(
-    get_omap_context(t, omap_root),
-    omap_root
-  ).si_then([this, &t, &key, &omap_root](auto&& extent) {
-    return extent->get_value(
-      get_omap_context(t, omap_root), key);
-  }).si_then([](auto &&e) {
-    return omap_get_value_ret(
-        interruptible::ready_future_marker{},
-        std::move(e));
-  });
+  auto extent = co_await get_omap_root(get_omap_context(t, omap_root), omap_root); 
+  co_return co_await extent->get_value(get_omap_context(t, omap_root), key);
 }
 
 BtreeOMapManager::omap_set_keys_ret
 BtreeOMapManager::omap_set_keys(
   omap_root_t &omap_root,
   Transaction &t,
-  std::map<std::string, ceph::bufferlist>&& keys)
+  std::map<std::string, ceph::bufferlist> keys)
 {
-  return seastar::do_with(std::move(keys), [&, this](auto& keys) {
-    return trans_intr::do_for_each(
-      keys.begin(),
-      keys.end(),
-      [&, this](auto &p) {
-      return omap_set_key(omap_root, t, p.first, p.second);
-    });
-  });
+  LOG_PREFIX(BtreeOMapManager::omap_set_keys);
+  for (auto &p: keys) {
+    DEBUGT("set key={}", t, p.first);
+    co_await omap_set_key(omap_root, t, p.first, p.second);
+  }
 }
 
 BtreeOMapManager::omap_set_key_ret
 BtreeOMapManager::omap_set_key(
   omap_root_t &omap_root,
   Transaction &t,
-  const std::string &key,
-  const ceph::bufferlist &value)
+  std::string key,
+  ceph::bufferlist value)
 {
   LOG_PREFIX(BtreeOMapManager::omap_set_key);
   DEBUGT("{} -> 0x{:x} value", t, key, value.length());
-  // #FIXME: heap buffer overflow during logging if value is long (e.g. 1020B)
+  // #FIXME: heap buffer overflow during logging if value is long (e.g. 1020B) 
   // https://tracker.ceph.com/issues/71524
   // DEBUGT("{} -> {}", t, key, value);
-  return get_omap_root(
-    get_omap_context(t, omap_root),
-    omap_root
-  ).si_then([this, &t, &key, &value, &omap_root](auto root) {
-    return root->insert(get_omap_context(
-      t, omap_root), key, value);
-  }).si_then([this, &omap_root, &t](auto mresult) -> omap_set_key_ret {
-    if (mresult.status == mutation_status_t::SUCCESS)
-      return seastar::now();
-    else if (mresult.status == mutation_status_t::WAS_SPLIT)
-      return handle_root_split(
-       get_omap_context(t, omap_root), omap_root, mresult);
-    else
-      return seastar::now();
-  });
+  auto root = co_await get_omap_root(get_omap_context(t, omap_root), omap_root);
+  auto mresult = co_await root->insert(get_omap_context(t, omap_root), key, value);
+  if (mresult.status == mutation_status_t::WAS_SPLIT) {
+    co_await handle_root_split(get_omap_context(t, omap_root), omap_root, mresult);
+  }
 }
 
 BtreeOMapManager::omap_rm_key_ret
 BtreeOMapManager::omap_rm_key(
   omap_root_t &omap_root,
   Transaction &t,
-  const std::string &key)
+  std::string key)
 {
   LOG_PREFIX(BtreeOMapManager::omap_rm_key);
   DEBUGT("{}", t, key);
-  return get_omap_root(
-    get_omap_context(t, omap_root),
-    omap_root
-  ).si_then([this, &t, &key, &omap_root](auto root) {
-    return root->rm_key(get_omap_context(t, omap_root), key);
-  }).si_then([this, &omap_root, &t](auto mresult) -> omap_rm_key_ret {
-    if (mresult.status == mutation_status_t::SUCCESS) {
-      return seastar::now();
-    } else if (mresult.status == mutation_status_t::WAS_SPLIT) {
-      return handle_root_split(
-        get_omap_context(t, omap_root), omap_root, mresult);
-    } else if (mresult.status == mutation_status_t::NEED_MERGE) {
-      auto root = *(mresult.need_merge);
-      if (root->get_node_size() == 1 && omap_root.depth != 1) {
-        return handle_root_merge(
-          get_omap_context(t, omap_root), omap_root, mresult);
-      } else {
-        return seastar::now(); 
-      }
-    } else {
-      return seastar::now();
+  auto root = co_await get_omap_root(get_omap_context(t, omap_root), omap_root);
+  auto mresult = co_await root->rm_key(get_omap_context(t, omap_root), key);
+  
+  if (mresult.status == mutation_status_t::WAS_SPLIT) {
+    co_await handle_root_split(get_omap_context(t, omap_root), omap_root, mresult);
+  } else if (mresult.status == mutation_status_t::NEED_MERGE) {
+    auto root = *(mresult.need_merge);
+    if (root->get_node_size() == 1 && omap_root.depth != 1) {
+      co_await handle_root_merge(get_omap_context(t, omap_root), omap_root, mresult);
     }
-  });
+  }
 }
 
 BtreeOMapManager::omap_rm_keys_ret
 BtreeOMapManager::omap_rm_keys(
   omap_root_t &omap_root,
   Transaction &t,
-  std::set<std::string>& keys)
+  std::set<std::string> keys)
 {
+  LOG_PREFIX(BtreeOMapManager::omap_rm_keys);
   auto type = omap_root.get_type();
-  return trans_intr::do_for_each(
-    keys.begin(),
-    keys.end(),
-    [&t, &omap_root, type, this](auto &p)
-  {
-    LOG_PREFIX(BtreeOMapManager::omap_rm_keys);
+  for (auto &p: keys) {
     DEBUGT("{} remove key={} ...", t, type, p);
-    return omap_rm_key(omap_root, t, p);
-  });
+    co_await omap_rm_key(omap_root, t, p);
+  }
 }
 
 BtreeOMapManager::omap_rm_key_range_ret

diff --git a/src/crimson/os/seastore/omap_manager/btree/btree_omap_manager.h b/src/crimson/os/seastore/omap_manager/btree/btree_omap_manager.h
index aa9231acf621c57f1e4a42f597876ec03262b19e..bb2505cc9d17be3d92ce8b7728184371248d71a8 100644 (file)
--- a/src/crimson/os/seastore/omap_manager/btree/btree_omap_manager.h
+++ b/src/crimson/os/seastore/omap_manager/btree/btree_omap_manager.h
@@ -72,22 +72,22 @@ public:
   omap_get_value_ret omap_get_value(
     const omap_root_t &omap_root,
     Transaction &t,
-    const std::string &key) final;
+    std::string key) final;
 
   omap_set_key_ret omap_set_key(
     omap_root_t &omap_root,
     Transaction &t,
-    const std::string &key, const ceph::bufferlist &value) final;
+    std::string key, ceph::bufferlist value) final;
 
   omap_set_keys_ret omap_set_keys(
     omap_root_t &omap_root,
     Transaction &t,
-    std::map<std::string, ceph::bufferlist>&& keys) final;
+    std::map<std::string, ceph::bufferlist> keys) final;
 
   omap_rm_key_ret omap_rm_key(
     omap_root_t &omap_root,
     Transaction &t,
-    const std::string &key) final;
+    std::string key) final;
 
   omap_rm_key_range_ret omap_rm_key_range(
     omap_root_t &omap_root,
@@ -115,7 +115,7 @@ public:
   omap_rm_keys_ret omap_rm_keys(
     omap_root_t &omap_root,
     Transaction &t,
-    std::set<std::string>& keys) final;
+    std::set<std::string> keys) final;
 };
 using BtreeOMapManagerRef = std::unique_ptr<BtreeOMapManager>;
 

diff --git a/src/crimson/os/seastore/omap_manager/log/log_manager.cc b/src/crimson/os/seastore/omap_manager/log/log_manager.cc
index 59e182c070948b37578da817eb7ac2b5d86fea97..c2833e06b996a4c62a5da75d0e99e0ac9a8c97ce 100644 (file)
--- a/src/crimson/os/seastore/omap_manager/log/log_manager.cc
+++ b/src/crimson/os/seastore/omap_manager/log/log_manager.cc
@@ -55,13 +55,12 @@ LogManager::initialize_omap(Transaction &t, laddr_t hint, omap_type_t omap_type)
 LogManager::omap_set_keys_ret
 LogManager::omap_set_keys(
   omap_root_t &log_root,
-  Transaction &t, std::map<std::string, ceph::bufferlist>&& _kvs) 
+  Transaction &t, std::map<std::string, ceph::bufferlist> kvs)
 {
   LOG_PREFIX(LogManager::omap_set_keys);
-  DEBUGT("enter kv size {}", t, _kvs.size());
+  DEBUGT("enter kv size {}", t, kvs.size());
   assert(log_root.get_type() == omap_type_t::LOG);
 
-  auto kvs = std::move(_kvs);
   auto ext = co_await log_load_extent<LogNode>(
     t, log_root.addr, BEGIN_KEY, END_KEY);
   ceph_assert(ext);
@@ -242,7 +241,7 @@ LogManager::omap_set_key_ret
 LogManager::omap_set_key(
   omap_root_t &log_root,
   Transaction &t,
-  const std::string &key, const ceph::bufferlist &value) 
+  std::string key, ceph::bufferlist value)
 {
   LOG_PREFIX(LogManager::omap_set_key);
   DEBUGT("enter k={}", t, key);
@@ -401,7 +400,7 @@ LogManager::log_load_extent(
 
 LogManager::omap_get_value_ret
 LogManager::omap_get_value(
-  const omap_root_t &log_root, Transaction &t, const std::string &key)
+  const omap_root_t &log_root, Transaction &t, std::string key)
 {
   LOG_PREFIX(LogManager::omap_get_value);
   DEBUGT("key={}", t, key);
@@ -673,7 +672,7 @@ LogManager::omap_rm_key_ret
 LogManager::omap_rm_key(
   omap_root_t &log_root,
   Transaction &t,
-  const std::string &key)
+  std::string key)
 {
   LOG_PREFIX(LogManager::omap_rm_key);
   DEBUGT("key={}", t, key);
@@ -734,7 +733,7 @@ LogManager::omap_rm_keys_ret
 LogManager::omap_rm_keys(
   omap_root_t& log_root,
   Transaction& t,
-  std::set<std::string>& keys)
+  std::set<std::string> keys)
 {
   LOG_PREFIX(LogManager::omap_rm_keys);
   DEBUGT("key size={}", t, keys.size());

diff --git a/src/crimson/os/seastore/omap_manager/log/log_manager.h b/src/crimson/os/seastore/omap_manager/log/log_manager.h
index b073611ba94897ded20ff059e938785d737b854c..10a8c52ff3cbc643878a3e11c12054b93a08ffbf 100644 (file)
--- a/src/crimson/os/seastore/omap_manager/log/log_manager.h
+++ b/src/crimson/os/seastore/omap_manager/log/log_manager.h
@@ -71,14 +71,14 @@ public:
    * @param _kvs   Batch of keys to set
    */
   omap_set_keys_ret omap_set_keys(omap_root_t &log_root,
-    Transaction &t, std::map<std::string, ceph::bufferlist>&& _kvs) final;
+    Transaction &t, std::map<std::string, ceph::bufferlist> kvs) final;
 
   // see omap_set_keys
   omap_set_key_ret omap_set_key(
     omap_root_t &log_root,
     Transaction &t,
-    const std::string &key,
-    const ceph::bufferlist &value) final;
+    std::string key,
+    ceph::bufferlist value) final;
 
   /**
    * omap_get_value
@@ -92,7 +92,7 @@ public:
    */
   omap_get_value_ret
   omap_get_value(const omap_root_t &log_root, Transaction &t,
-    const std::string &key) final;
+    std::string key) final;
 
   /**
    * omap_list
@@ -151,13 +151,13 @@ public:
   omap_rm_key_ret omap_rm_key(
     omap_root_t &log_root,
     Transaction &t,
-    const std::string &key) final;
+    std::string key) final;
 
 
   omap_rm_keys_ret omap_rm_keys(
     omap_root_t &omap_root,
     Transaction &t,
-    std::set<std::string>& keys) final;
+    std::set<std::string> keys) final;
 
   /**
    * omap_clear