Skip to content

Commit 0e43e69

Browse files
committed
os/bluestore: Fix Blob::copy_extents function
The function had an error, that it assumed its smallest operational block is allocation unit. In reality, the smallest block is min_release_size, that is a max(allocation size, checksum size). In some wierd cases single min_release_size block could be composed of *disjointed* allocation on the disk. This is something that bitmap allocator does often. Blob::copy_extents could not handle it properly, but luckily, it asserted when faced with that data. New fixed and improved version is able to handle this case too. Signed-off-by: Adam Kupczyk <[email protected]>
1 parent 725b583 commit 0e43e69

File tree

1 file changed

+53
-22
lines changed

1 file changed

+53
-22
lines changed

src/os/bluestore/BlueStore.cc

Lines changed: 53 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2534,21 +2534,60 @@ void BlueStore::Blob::copy_extents(
25342534
CephContext* cct, const Blob& from, uint32_t start,
25352535
uint32_t pre_len, uint32_t main_len, uint32_t post_len)
25362536
{
2537-
constexpr uint64_t invalid = bluestore_pextent_t::INVALID_OFFSET;
2538-
auto at = [&](const PExtentVector& e, uint32_t pos, uint32_t len) -> uint64_t {
2539-
auto it = e.begin();
2540-
while (it != e.end() && pos >= it->length) {
2541-
pos -= it->length;
2542-
++it;
2537+
// There are 2 valid states:
2538+
// 1) `to` is not defined on [pos~len] range
2539+
// (need to copy this region - return true)
2540+
// 2) `from` and `to` are exact on [pos~len] range
2541+
// (no need to copy region - return false)
2542+
// Otherwise just assert.
2543+
auto check_sane_need_copy = [&](
2544+
const PExtentVector& from,
2545+
const PExtentVector& to,
2546+
uint32_t pos, uint32_t len) -> bool
2547+
{
2548+
uint32_t pto = pos;
2549+
auto ito = to.begin();
2550+
while (ito != to.end() && pto >= ito->length) {
2551+
pto -= ito->length;
2552+
++ito;
25432553
}
2544-
if (it == e.end()) {
2545-
return invalid;
2554+
if (ito == to.end()) return true; // case 1 - obviously empty
2555+
if (!ito->is_valid()) {
2556+
// now sanity check that all the rest is invalid too
2557+
pto += len;
2558+
while (ito != to.end() && pto >= ito->length) {
2559+
ceph_assert(!ito->is_valid());
2560+
pto -= ito->length;
2561+
++ito;
2562+
}
2563+
return true;
25462564
}
2547-
if (!it->is_valid()) {
2548-
return invalid;
2565+
uint32_t pfrom = pos;
2566+
auto ifrom = from.begin();
2567+
while (ifrom != from.end() && pfrom >= ifrom->length) {
2568+
pfrom -= ifrom->length;
2569+
++ifrom;
2570+
}
2571+
ceph_assert(ifrom != from.end());
2572+
ceph_assert(ifrom->is_valid());
2573+
// here we require from and to be the same
2574+
while (len > 0) {
2575+
ceph_assert(ifrom->offset + pfrom == ito->offset + pto);
2576+
uint32_t jump = std::min(len, ifrom->length - pfrom);
2577+
jump = std::min(jump, ito->length - pto);
2578+
pfrom += jump;
2579+
if (pfrom == ifrom->length) {
2580+
pfrom = 0;
2581+
++ifrom;
2582+
}
2583+
pto += jump;
2584+
if (pto == ito->length) {
2585+
pto = 0;
2586+
++ito;
2587+
}
2588+
len -= jump;
25492589
}
2550-
ceph_assert(pos + len <= it->length); // post_len should be single au, and we do not split
2551-
return it->offset + pos;
2590+
return false;
25522591
};
25532592
const PExtentVector& exfrom = from.blob.get_extents();
25542593
PExtentVector& exto = blob.dirty_extents();
@@ -2557,24 +2596,16 @@ void BlueStore::Blob::copy_extents(
25572596

25582597
// the extents that cover same area must be the same
25592598
if (pre_len > 0) {
2560-
uint64_t au_from = at(exfrom, start, pre_len);
2561-
ceph_assert(au_from != bluestore_pextent_t::INVALID_OFFSET);
2562-
uint64_t au_to = at(exto, start, pre_len);
2563-
if (au_to == bluestore_pextent_t::INVALID_OFFSET) {
2599+
if (check_sane_need_copy(exfrom, exto, start, pre_len)) {
25642600
main_len += pre_len; // also copy pre_len
25652601
} else {
2566-
ceph_assert(au_from == au_to);
25672602
start += pre_len; // skip, already there
25682603
}
25692604
}
25702605
if (post_len > 0) {
2571-
uint64_t au_from = at(exfrom, start + main_len, post_len);
2572-
ceph_assert(au_from != bluestore_pextent_t::INVALID_OFFSET);
2573-
uint64_t au_to = at(exto, start + main_len, post_len);
2574-
if (au_to == bluestore_pextent_t::INVALID_OFFSET) {
2606+
if (check_sane_need_copy(exfrom, exto, start + main_len, post_len)) {
25752607
main_len += post_len; // also copy post_len
25762608
} else {
2577-
ceph_assert(au_from == au_to);
25782609
// skip, already there
25792610
}
25802611
}

0 commit comments

Comments
 (0)