From c148f97ee44cbd4aa7d292c9b7754a29227e276f Mon Sep 17 00:00:00 2001 From: Vitaliy Filippov Date: Sun, 31 Aug 2025 15:41:59 +0300 Subject: [PATCH] Fix checksum validation in !inmemory_journal mode --- src/blockstore/blockstore_flush.cpp | 21 ++++++++----- src/blockstore/blockstore_flush.h | 2 +- src/blockstore/blockstore_heap.cpp | 5 ++-- src/blockstore/blockstore_read.cpp | 46 ++++++++++++++--------------- src/blockstore/blockstore_write.cpp | 6 ++-- 5 files changed, 43 insertions(+), 37 deletions(-) diff --git a/src/blockstore/blockstore_flush.cpp b/src/blockstore/blockstore_flush.cpp index 080f47cc..3ede5c5c 100644 --- a/src/blockstore/blockstore_flush.cpp +++ b/src/blockstore/blockstore_flush.cpp @@ -302,11 +302,13 @@ resume_8: for (i = 0; i < read_vec.size(); i++) { if ((read_vec[i].copy_flags & COPY_BUF_JOURNAL) && - !(read_vec[i].copy_flags & COPY_BUF_COALESCED)) + !(read_vec[i].copy_flags & COPY_BUF_COALESCED) || + (read_vec[i].copy_flags & COPY_BUF_PADDED)) // FIXME Shit, simplify these flags { assert(read_vec[i].buf); await_sqe(9); - data->iov = (struct iovec){ read_vec[i].buf, (size_t)read_vec[i].len }; + data->iov = (struct iovec){ read_vec[i].buf + (read_vec[i].copy_flags & COPY_BUF_PADDED + ? read_vec[i].offset - read_vec[i].disk_offset : 0), (size_t)read_vec[i].len }; data->callback = simple_callback_w; io_uring_prep_writev(sqe, bs->dsk.data_fd, &data->iov, 1, bs->dsk.data_offset + clean_loc + read_vec[i].offset); wait_count++; @@ -410,7 +412,8 @@ void journal_flusher_co::fill_partial_checksum_blocks() .copy_flags = COPY_BUF_DATA | copy_flags, .offset = blk_begin, .len = blk_end - blk_begin, - .disk_offset = end_wr->location + blk_begin, + .disk_loc = end_wr->location, + .disk_offset = blk_begin, .disk_len = blk_end - blk_begin, .buf = (uint8_t*)memalign_or_die(MEM_ALIGNMENT, blk_end - blk_begin), .wr_lsn = end_wr->lsn, @@ -421,6 +424,8 @@ void journal_flusher_co::fill_partial_checksum_blocks() .copy_flags = COPY_BUF_JOURNAL|COPY_BUF_COALESCED, .offset = hole_start, .len = hole_end - hole_start, + .disk_offset = hole_start, + .disk_len = hole_end - hole_start, .buf = vec.buf + hole_start - vec.offset, }); vec_pos++; @@ -461,16 +466,16 @@ int journal_flusher_co::check_and_punch_checksums() wr = wr->next(); assert(wr); uint32_t *csums = (uint32_t*)(wr->get_checksums(bs->heap) - + (vec.offset/bs->dsk.csum_block_size)*(bs->dsk.data_csum_type & 0xFF) + + (vec.disk_offset/bs->dsk.csum_block_size)*(bs->dsk.data_csum_type & 0xFF) - ((wr->type() == BS_HEAP_BIG_WRITE) ? 0 : (wr->offset/bs->dsk.csum_block_size)*(bs->dsk.data_csum_type & 0xFF))); bs->heap->calc_block_checksums( - csums, vec.buf, wr->get_int_bitmap(bs->heap), vec.offset, vec.offset+vec.len, false, + csums, vec.buf, wr->get_int_bitmap(bs->heap), vec.disk_offset, vec.disk_offset+vec.disk_len, false, [&](uint32_t mismatch_pos, uint32_t expected_csum, uint32_t real_csum) { printf("Checksum mismatch during compaction in object %jx:%jx v%ju, offset 0x%x in %s area at offset 0x%jx: got %08x, expected %08x\n", cur_oid.inode, cur_oid.stripe, wr->version, mismatch_pos, (vec.copy_flags & COPY_BUF_JOURNAL ? "buffer" : "data"), - vec.disk_offset, real_csum, expected_csum); + vec.disk_loc+vec.disk_offset, real_csum, expected_csum); csum_ok = false; } ); @@ -586,7 +591,7 @@ bool journal_flusher_co::calc_block_checksums() if (read_vec[i].offset+read_vec[i].len > start) { len = read_vec[i].offset+read_vec[i].len-start; - return read_vec[i].buf + start-read_vec[i].offset; + return read_vec[i].buf + start-read_vec[i].disk_offset; } } return (uint8_t*)NULL; @@ -644,7 +649,7 @@ bool journal_flusher_co::read_buffered(int wait_base) sqe, (vec.copy_flags & COPY_BUF_JOURNAL) ? bs->dsk.journal_fd : bs->dsk.data_fd, &data->iov, 1, - ((vec.copy_flags & COPY_BUF_JOURNAL) ? bs->dsk.journal_offset : bs->dsk.data_offset) + vec.disk_offset + ((vec.copy_flags & COPY_BUF_JOURNAL) ? bs->dsk.journal_offset : bs->dsk.data_offset) + vec.disk_loc + vec.disk_offset ); data->callback = simple_callback_r; } diff --git a/src/blockstore/blockstore_flush.h b/src/blockstore/blockstore_flush.h index dc332fd4..3ed9d2ff 100644 --- a/src/blockstore/blockstore_flush.h +++ b/src/blockstore/blockstore_flush.h @@ -4,7 +4,7 @@ struct copy_buffer_t { uint32_t copy_flags; - uint64_t offset, len, disk_offset, disk_len; + uint64_t offset, len, disk_loc, disk_offset, disk_len; uint8_t *buf; uint64_t wr_lsn; }; diff --git a/src/blockstore/blockstore_heap.cpp b/src/blockstore/blockstore_heap.cpp index ee53b8d5..5864e5f1 100644 --- a/src/blockstore/blockstore_heap.cpp +++ b/src/blockstore/blockstore_heap.cpp @@ -661,9 +661,10 @@ bool blockstore_heap_t::calc_block_checksums(uint32_t *block_csums, uint8_t *bit bool isset = false; while (pos < end) { - uint32_t prev = pos; + uint32_t blk_start = pos; if (bitmap) { + uint32_t prev = pos; while (pos < end && pos < block_end) { while (pos < end && pos < block_end && !(bitmap[pos/dsk->bitmap_granularity/8] & (1 << ((pos/dsk->bitmap_granularity) % 8)))) @@ -696,7 +697,7 @@ bool blockstore_heap_t::calc_block_checksums(uint32_t *block_csums, uint8_t *bit { if (bad_block_cb) { - bad_block_cb(prev, *block_csums, block_crc); + bad_block_cb(blk_start, *block_csums, block_crc); res = false; } else diff --git a/src/blockstore/blockstore_read.cpp b/src/blockstore/blockstore_read.cpp index b7a80bc7..a6533563 100644 --- a/src/blockstore/blockstore_read.cpp +++ b/src/blockstore/blockstore_read.cpp @@ -89,7 +89,7 @@ int blockstore_impl_t::fulfill_read(blockstore_op_t *op) } else if ((vec.copy_flags & COPY_BUF_JOURNAL) && dsk.inmemory_journal) { - memcpy(op->buf + vec.offset - op->offset, buffer_area + vec.disk_offset, vec.len); + memcpy(op->buf + vec.offset - op->offset, buffer_area + vec.disk_loc + vec.disk_offset, vec.len); } else { @@ -100,7 +100,7 @@ int blockstore_impl_t::fulfill_read(blockstore_op_t *op) sqe, (vec.copy_flags & COPY_BUF_JOURNAL) ? dsk.journal_fd : dsk.data_fd, &data->iov, 1, - ((vec.copy_flags & COPY_BUF_JOURNAL) ? dsk.journal_offset : dsk.data_offset) + vec.disk_offset + ((vec.copy_flags & COPY_BUF_JOURNAL) ? dsk.journal_offset : dsk.data_offset) + vec.disk_loc + vec.disk_offset ); data->callback = [this, op](ring_data_t *data) { handle_read_event(data, op); }; } @@ -185,7 +185,8 @@ uint32_t blockstore_impl_t::prepare_read_simple(std::vector & rea .copy_flags = COPY_BUF_JOURNAL | COPY_BUF_SKIP_CSUM, .offset = start, .len = end-start, - .disk_offset = wr->location + start - wr->offset, + .disk_loc = wr->location - wr->offset, + .disk_offset = start, .disk_len = end-start, .buf = buffer_area + wr->location + start - wr->offset, .wr_lsn = wr->lsn, @@ -223,8 +224,8 @@ uint32_t blockstore_impl_t::prepare_read_simple(std::vector & rea { // one or two partial blocks plus any number of full blocks // i.e. [..XX][XXXX][X...] - uint32_t full_start = (blk_start != start ? blk_start+dsk.csum_block_size : blk_start); - uint32_t full_end = (blk_end != end ? blk_end-dsk.csum_block_size : blk_end); + uint32_t full_start = (blk_start != start ? (blk_start/dsk.csum_block_size+1)*dsk.csum_block_size : blk_start); + uint32_t full_end = (blk_end != end ? (blk_end % dsk.csum_block_size ? blk_end-blk_end%dsk.csum_block_size : blk_end - dsk.csum_block_size) : blk_end); if (blk_start != start) // starting padded block prepare_disk_read(read_vec, pos++, obj, wr, blk_start, full_start, start, full_start, skip_csum); if (full_end > full_start) // full non-padded blocks @@ -246,7 +247,8 @@ void blockstore_impl_t::prepare_disk_read(std::vector & read_vec, .copy_flags = (wr->type() == BS_HEAP_SMALL_WRITE ? COPY_BUF_JOURNAL : COPY_BUF_DATA) | copy_flags, .offset = start, .len = end-start, - .disk_offset = (wr->type() == BS_HEAP_INTENT_WRITE ? wr->next()->location : wr->location) + blk_start, + .disk_loc = (wr->type() == BS_HEAP_INTENT_WRITE ? wr->next()->location : wr->location - (wr->type() == BS_HEAP_SMALL_WRITE ? wr->offset : 0)), + .disk_offset = blk_start, .disk_len = blk_end - blk_start, .wr_lsn = wr->lsn, }; @@ -254,16 +256,19 @@ void blockstore_impl_t::prepare_disk_read(std::vector & read_vec, { assert(!(copy_flags & COPY_BUF_CSUM_FILL)); vec.copy_flags |= COPY_BUF_PADDED; - if (pos > 0 && read_vec.size() >= pos && read_vec[pos-1].copy_flags == vec.copy_flags && - read_vec[pos-1].offset <= blk_start && read_vec[pos-1].offset+read_vec[pos-1].len >= blk_end) + if (pos > 0 && read_vec.size() >= pos && + read_vec[pos-1].copy_flags == vec.copy_flags && + read_vec[pos-1].wr_lsn == vec.wr_lsn && + read_vec[pos-1].disk_offset <= vec.disk_offset && + read_vec[pos-1].disk_offset+read_vec[pos-1].disk_len >= blk_end) { // This is the same block as the previous one, we can read it only once vec.copy_flags |= COPY_BUF_COALESCED; - vec.buf = read_vec[pos-1].buf + blk_start - read_vec[pos-1].offset; + vec.buf = read_vec[pos-1].buf + vec.disk_offset - read_vec[pos-1].disk_offset; } else { - vec.buf = (uint8_t*)memalign_or_die(MEM_ALIGNMENT, blk_end-blk_start); + vec.buf = (uint8_t*)memalign_or_die(MEM_ALIGNMENT, vec.disk_len); } } read_vec.insert(read_vec.begin() + pos, vec); @@ -351,32 +356,25 @@ bool blockstore_impl_t::verify_read_checksums(blockstore_op_t *op) { if (vec.copy_flags & COPY_BUF_ZERO) continue; + if (vec.copy_flags & COPY_BUF_PADDED) + memcpy(op->buf + vec.offset - op->offset, vec.buf + vec.offset - vec.disk_offset, vec.len); + if (vec.copy_flags & (COPY_BUF_COALESCED|COPY_BUF_SKIP_CSUM)) + continue; heap_write_t *wr = obj->get_writes(); while (wr && wr->lsn != vec.wr_lsn) wr = wr->next(); assert(wr); - uint32_t blk_start = vec.offset, blk_end = vec.offset + vec.len; - if (vec.copy_flags & COPY_BUF_PADDED) - { - blk_start = (blk_start/dsk.csum_block_size) * dsk.csum_block_size; - blk_start = blk_start < wr->offset ? wr->offset : blk_start; - blk_end = ((blk_end-1) / dsk.csum_block_size + 1) * dsk.csum_block_size; - blk_end = blk_end > wr->offset+wr->len ? wr->offset+wr->len : blk_end; - memcpy(op->buf + vec.offset - op->offset, vec.buf + vec.offset - blk_start, vec.len); - } - if (vec.copy_flags & (COPY_BUF_COALESCED|COPY_BUF_SKIP_CSUM)) - continue; uint8_t *buf = vec.buf ? vec.buf : (op->buf + vec.offset - op->offset); uint32_t *csums = (uint32_t*)(wr->get_checksums(heap) - + (blk_start/dsk.csum_block_size)*(dsk.data_csum_type & 0xFF) + + (vec.disk_offset/dsk.csum_block_size)*(dsk.data_csum_type & 0xFF) - ((wr->type() == BS_HEAP_BIG_WRITE) ? 0 : (wr->offset/dsk.csum_block_size)*(dsk.data_csum_type & 0xFF))); if (!heap->calc_block_checksums(csums, buf, wr->get_int_bitmap(heap), - blk_start, blk_end, false, [&](uint32_t mismatch_pos, uint32_t expected_csum, uint32_t real_csum) + vec.disk_offset, vec.disk_offset+vec.disk_len, false, [&](uint32_t mismatch_pos, uint32_t expected_csum, uint32_t real_csum) { printf( "Checksum mismatch in object %jx:%jx v%ju, offset 0x%x in %s area at offset 0x%jx: %08x expected vs %08x actual\n", op->oid.inode, op->oid.stripe, op->version, mismatch_pos, - (vec.copy_flags & COPY_BUF_JOURNAL) ? "buffer" : "data", vec.disk_offset, + (vec.copy_flags & COPY_BUF_JOURNAL) ? "buffer" : "data", vec.disk_loc + vec.disk_offset, expected_csum, real_csum ); })) diff --git a/src/blockstore/blockstore_write.cpp b/src/blockstore/blockstore_write.cpp index c7f2097f..23e504f6 100644 --- a/src/blockstore/blockstore_write.cpp +++ b/src/blockstore/blockstore_write.cpp @@ -242,8 +242,10 @@ process_intent: if (op->len > 0) { // Prepare buffered data write - assert(dsk.inmemory_journal); - memcpy((uint8_t*)buffer_area + loc, op->buf, op->len); + if (dsk.inmemory_journal) + { + memcpy((uint8_t*)buffer_area + loc, op->buf, op->len); + } BS_SUBMIT_GET_SQE(sqe2, data2); data2->iov = (struct iovec){ op->buf, op->len }; data2->callback = [this, op](ring_data_t *data) { handle_write_event(data, op); };