crimson/os/seastore: add releasing state for segments pending close

This should fix a bug by which we might start scanning a segment a second
time as it is released and possibly even reused resulting in nonsensical
behavior.

Signed-off-by: Samuel Just <sjust@redhat.com>

diff --git a/src/crimson/os/seastore/segment_cleaner.cc b/src/crimson/os/seastore/segment_cleaner.cc
index b2da583cc643b4934362d6986335ec490c7177c9..ec477f847a6277603520480aa7cda6a200a69573 100644 (file)
--- a/src/crimson/os/seastore/segment_cleaner.cc
+++ b/src/crimson/os/seastore/segment_cleaner.cc
@@ -331,6 +331,7 @@ SegmentCleaner::do_gc_ret SegmentCleaner::do_gc(
          }).safe_then([&t, this] {
            if (scan_cursor->is_complete()) {
              t.mark_segment_to_release(scan_cursor->get_offset().segment);
+             mark_releasing(scan_cursor->get_offset().segment);
              scan_cursor.reset();
            }
            return ExtentCallbackInterface::release_segment_ertr::now();

diff --git a/src/crimson/os/seastore/segment_cleaner.h b/src/crimson/os/seastore/segment_cleaner.h
index e6297e57e2c03524baee72b555fc1b865ec8c297..d976a7f0ecc060e9413e297b4090d398b7c5683e 100644 (file)
--- a/src/crimson/os/seastore/segment_cleaner.h
+++ b/src/crimson/os/seastore/segment_cleaner.h
@@ -35,6 +35,10 @@ struct segment_info_t {
     return state == Segment::segment_state_t::CLOSED;
   }
 
+  bool is_releasing() const {
+    return state == Segment::segment_state_t::RELEASING;
+  }
+
   bool is_open() const {
     return state == Segment::segment_state_t::OPEN;
   }
@@ -665,9 +669,16 @@ private:
     segments[segment].state = Segment::segment_state_t::CLOSED;
   }
 
-  void mark_empty(segment_id_t segment) {
+  void mark_releasing(segment_id_t segment) {
     assert(segments.size() > segment);
     assert(segments[segment].is_closed());
+    segments[segment].state = Segment::segment_state_t::RELEASING;
+  }
+
+
+  void mark_empty(segment_id_t segment) {
+    assert(segments.size() > segment);
+    assert(segments[segment].is_releasing());
     assert(segments.size() > empty_segments);
     ++empty_segments;
     if (space_tracker->get_usage(segment) != 0) {

diff --git a/src/crimson/os/seastore/segment_manager.h b/src/crimson/os/seastore/segment_manager.h
index 61c6509d19f7dbc503a262d4022b518775d792a0..1f9cd90b9cc13eab23f984b3d4b318d4d37a76b0 100644 (file)
--- a/src/crimson/os/seastore/segment_manager.h
+++ b/src/crimson/os/seastore/segment_manager.h
@@ -24,7 +24,8 @@ public:
   enum class segment_state_t : uint8_t {
     EMPTY = 0,
     OPEN = 1,
-    CLOSED = 2
+    CLOSED = 2,
+    RELEASING = 3
   };
 
   /**

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index 9592ba854acfb30fe0916550dcc71267aef30dfc..13c0761f6383401443fe70fd52861ee9c043f745 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -214,8 +214,11 @@ TransactionManager::submit_transaction(
       lba_manager->complete_transaction(tref);
       auto to_release = tref.get_segment_to_release();
       if (to_release != NULL_SEG_ID) {
-       segment_cleaner->mark_segment_released(to_release);
-       return segment_manager.release(to_release);
+       return segment_manager.release(to_release
+       ).safe_then([this, to_release] {
+         segment_cleaner->mark_segment_released(to_release);
+         return SegmentManager::release_ertr::now();
+       });
       } else {
        return SegmentManager::release_ertr::now();
       }