Skip to content

Commit

Permalink
crimson/os/seastore/object_data_handler: consider a RBM case when che…
Browse files Browse the repository at this point in the history
…cking 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 <myoungwon.oh@samsung.com>
  • Loading branch information
myoungwon committed Sep 12, 2023
1 parent 670a19f commit 73de893
Showing 1 changed file with 21 additions and 10 deletions.
31 changes: 21 additions & 10 deletions src/crimson/os/seastore/object_data_handler.cc
Expand Up @@ -576,7 +576,8 @@ struct overwrite_plan_t {
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()),
Expand All @@ -589,7 +590,7 @@ struct overwrite_plan_t {
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);
}
Expand Down Expand Up @@ -617,19 +618,31 @@ struct overwrite_plan_t {
* 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;
Expand All @@ -639,9 +652,7 @@ struct overwrite_plan_t {
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;
Expand Down Expand Up @@ -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(),
Expand Down

0 comments on commit 73de893

Please sign in to comment.