From: Xuehan Xu <xxhdx1985126@gmail.com>
Date: Tue, 12 Oct 2021 01:55:21 +0000 (+0800)
Subject: crimson/os/seastore: set ExtentPlacementManager::allocated_to before rolling segments
X-Git-Tag: v17.1.0~710^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=460f6ca0478c8320247f96a8d308197a0d9c432e;p=ceph-ci.git

crimson/os/seastore: set ExtentPlacementManager::allocated_to before rolling segments

There are circumstances in which a transaction that are supposed to roll the current segment
is invalidated after it finished writing and before it rolls the segment. If we don't set
ExtentPlacementManager::allocated_to in this situation, another transaction can try to write
to the old "allocated_to" position, which would cause an invalid write error

Fixes: https://tracker.ceph.com/issues/52894
Signed-off-by: Xuehan Xu <xxhdx1985126@gmail.com>
---

diff --git a/src/crimson/os/seastore/extent_placement_manager.cc b/src/crimson/os/seastore/extent_placement_manager.cc
index 48a89ea273a..f3670f3c2ea 100644
--- a/src/crimson/os/seastore/extent_placement_manager.cc
+++ b/src/crimson/os/seastore/extent_placement_manager.cc
@@ -69,9 +69,10 @@ SegmentedAllocator::Writer::finish_write(
 SegmentedAllocator::Writer::write_iertr::future<>
 SegmentedAllocator::Writer::_write(
   Transaction& t,
-  ool_record_t& record,
-  const record_size_t& record_size)
+  ool_record_t& record)
 {
+  record_size_t record_size = record.get_encoded_record_length();
+  allocated_to += record_size.mdlength + record_size.dlength;
   bufferlist bl = record.encode(
       record_size,
       current_segment->segment->get_segment_id(),
@@ -165,9 +166,8 @@ SegmentedAllocator::Writer::write(
                   num_extents,
                   current_segment->segment->get_segment_id(),
                   allocated_to);
-                auto rsize = record.get_encoded_record_length();
                 return (num_extents ?
-                        _write(t, record, rsize) :
+                        _write(t, record) :
                         write_iertr::now()
                 ).si_then([this]() mutable {
                   return roll_segment(false);
@@ -179,7 +179,6 @@ SegmentedAllocator::Writer::write(
               add_extent_to_write(record, extent);
               it = extents.erase(it);
             }
-            record_size_t rsize = record.get_encoded_record_length();
 
             DEBUGT(
               "writing {} extents to segment {} at {}",
@@ -187,8 +186,7 @@ SegmentedAllocator::Writer::write(
               record.get_num_extents(),
               current_segment->segment->get_segment_id(),
               allocated_to);
-            allocated_to += rsize.mdlength + rsize.dlength;
-            return _write(t, record, rsize);
+            return _write(t, record);
           }
         ).si_then([]()
           -> write_iertr::future<seastar::stop_iteration> {
diff --git a/src/crimson/os/seastore/extent_placement_manager.h b/src/crimson/os/seastore/extent_placement_manager.h
index d9a15ea5577..690d817c2ed 100644
--- a/src/crimson/os/seastore/extent_placement_manager.h
+++ b/src/crimson/os/seastore/extent_placement_manager.h
@@ -217,8 +217,7 @@ class SegmentedAllocator : public ExtentAllocator {
 
     write_iertr::future<> _write(
       Transaction& t,
-      ool_record_t& record,
-      const record_size_t& record_size);
+      ool_record_t& record);
 
     using roll_segment_ertr = crimson::errorator<
       crimson::ct_error::input_output_error>;