From 3768ef692fc47b629b3ac7a9dd09762172fbe646 Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Mon, 8 Mar 2021 18:09:15 -0800 Subject: [PATCH] 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 --- src/crimson/os/seastore/segment_cleaner.cc | 1 + src/crimson/os/seastore/segment_cleaner.h | 13 ++++++++++++- src/crimson/os/seastore/segment_manager.h | 3 ++- src/crimson/os/seastore/transaction_manager.cc | 7 +++++-- 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/crimson/os/seastore/segment_cleaner.cc b/src/crimson/os/seastore/segment_cleaner.cc index b2da583cc64..ec477f847a6 100644 --- 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 e6297e57e2c..d976a7f0ecc 100644 --- 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 61c6509d19f..1f9cd90b9cc 100644 --- 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 9592ba854ac..13c0761f638 100644 --- 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(); } -- 2.39.5