From 73de8937f65706c9c3dd2e36f6504196d0796b01 Mon Sep 17 00:00:00 2001 From: Myoungwon Oh Date: Fri, 25 Aug 2023 17:55:36 +0900 Subject: [PATCH] crimson/os/seastore/object_data_handler: consider a RBM case when checking if write can be merged RBM's paddr always indicates physical address, which means it doesn't have the dealayed. So, this commit adds a condition that checks if given paddr is used for ongoing write. Signed-off-by: Myoungwon Oh --- .../os/seastore/object_data_handler.cc | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/crimson/os/seastore/object_data_handler.cc b/src/crimson/os/seastore/object_data_handler.cc index a52414a336e0e..0d852696b7144 100644 --- a/src/crimson/os/seastore/object_data_handler.cc +++ b/src/crimson/os/seastore/object_data_handler.cc @@ -576,7 +576,8 @@ public: overwrite_plan_t(laddr_t offset, extent_len_t len, const lba_pin_list_t& pins, - extent_len_t block_size) : + extent_len_t block_size, + Transaction& t) : pin_begin(pins.front()->get_key()), pin_end(pins.back()->get_key() + pins.back()->get_length()), left_paddr(pins.front()->get_val()), @@ -589,7 +590,7 @@ public: right_operation(overwrite_operation_t::UNKNOWN), block_size(block_size) { validate(); - evaluate_operations(); + evaluate_operations(t); assert(left_operation != overwrite_operation_t::UNKNOWN); assert(right_operation != overwrite_operation_t::UNKNOWN); } @@ -617,19 +618,31 @@ private: * original extent into at most three parts: origin-left, part-to-be-modified * and origin-right. */ - void evaluate_operations() { + void evaluate_operations(Transaction& t) { auto actual_write_size = get_pins_size(); auto aligned_data_size = get_aligned_data_size(); auto left_ext_size = get_left_extent_size(); auto right_ext_size = get_right_extent_size(); + auto can_merge = [](Transaction& t, paddr_t paddr) { + CachedExtentRef ext; + if (paddr.is_relative() || paddr.is_delayed()) { + return true; + } else if (t.get_extent(paddr, &ext) == + Transaction::get_extent_ret::PRESENT) { + // FIXME: there is no need to lookup the cache if the pin can + // be associated with the extent state + if (ext->is_mutable()) { + return true; + } + } + return false; + }; if (left_paddr.is_zero()) { actual_write_size -= left_ext_size; left_ext_size = 0; left_operation = overwrite_operation_t::OVERWRITE_ZERO; - // FIXME: left_paddr can be absolute and pending - } else if (left_paddr.is_relative() || - left_paddr.is_delayed()) { + } else if (can_merge(t, left_paddr)) { aligned_data_size += left_ext_size; left_ext_size = 0; left_operation = overwrite_operation_t::MERGE_EXISTING; @@ -639,9 +652,7 @@ private: actual_write_size -= right_ext_size; right_ext_size = 0; right_operation = overwrite_operation_t::OVERWRITE_ZERO; - // FIXME: right_paddr can be absolute and pending - } else if (right_paddr.is_relative() || - right_paddr.is_delayed()) { + } else if (can_merge(t, right_paddr)) { aligned_data_size += right_ext_size; right_ext_size = 0; right_operation = overwrite_operation_t::MERGE_EXISTING; @@ -1148,7 +1159,7 @@ ObjectDataHandler::write_ret ObjectDataHandler::overwrite( if (bl.has_value()) { assert(bl->length() == len); } - overwrite_plan_t overwrite_plan(offset, len, _pins, ctx.tm.get_block_size()); + overwrite_plan_t overwrite_plan(offset, len, _pins, ctx.tm.get_block_size(), ctx.t); return seastar::do_with( std::move(_pins), extent_to_write_list_t(), -- 2.39.5