From 2b8a9e3f90148c424dde96b78c498a3344caee40 Mon Sep 17 00:00:00 2001 From: Vitaliy Filippov Date: Sun, 14 Sep 2025 02:18:42 +0300 Subject: [PATCH] Add entry_type to heap_object_t too This is required to: 1) later inline the last "big_write" entry into the object to slightly reduce memory usage 2) eliminate an ugly hack where entry type is determined by its size 3) make the storage scheme extensible i.e. when adding new entry types --- src/blockstore/blockstore_heap.cpp | 51 ++++++++++++++++++++---------- src/blockstore/blockstore_heap.h | 10 +++--- src/test/test_heap.cpp | 12 ++++--- 3 files changed, 49 insertions(+), 24 deletions(-) diff --git a/src/blockstore/blockstore_heap.cpp b/src/blockstore/blockstore_heap.cpp index d19b950f..3f2042d1 100644 --- a/src/blockstore/blockstore_heap.cpp +++ b/src/blockstore/blockstore_heap.cpp @@ -22,6 +22,8 @@ #define MIN_ALLOC (sizeof(heap_object_t)+sizeof(heap_write_t)) +static constexpr uint32_t heap_entry_type_pos = 4; + heap_write_t *heap_write_t::next() { return (next_pos ? (heap_write_t*)((uint8_t*)this + next_pos) : NULL); @@ -169,7 +171,6 @@ blockstore_heap_t::blockstore_heap_t(blockstore_disk_t *dsk, uint8_t *buffer_are assert(dsk->meta_block_size < 32768); assert(dsk->meta_area_size > 0); assert(dsk->journal_len > 0); - assert(sizeof(heap_object_t) < sizeof(heap_write_t)); meta_alloc = new multilist_index_t(meta_block_count, 1 + dsk->meta_block_size/MIN_ALLOC, 0); block_info.resize(meta_block_count); data_alloc = new allocator_t(dsk->block_count); @@ -277,9 +278,28 @@ void blockstore_heap_t::read_blocks(uint64_t disk_offset, uint64_t disk_size, ui } break; } - if (region_marker < sizeof(heap_object_t)) + const uint8_t entry_type = data[heap_entry_type_pos]; + if (entry_type != BS_HEAP_OBJECT) { - fprintf(stderr, "Warning: Entry is too small in metadata block %u at %u (%u < min %ju bytes), skipping\n", + // Write entry (probably) (or garbage) + if ((entry_type & BS_HEAP_TYPE) < BS_HEAP_OBJECT || + (entry_type & BS_HEAP_TYPE) > BS_HEAP_INTENT_WRITE || + (entry_type & ~(BS_HEAP_TYPE|BS_HEAP_STABLE))) + { + fprintf(stderr, "Warning: Entry of unknown type %u in metadata block %u at %u, skipping\n", + entry_type, block_num, block_offset); + } + auto offset_it = offsets_seen.find(block_offset+region_marker); + if (offset_it == offsets_seen.end()) + { + offsets_seen[block_offset+region_marker] = (verify_offset_t){ .start = block_offset, .handled = false }; + } + block_offset += region_marker; + continue; + } + if (region_marker != sizeof(heap_object_t)) + { + fprintf(stderr, "Warning: Object entry has invalid size in metadata block %u at %u (%u != %ju bytes), skipping\n", block_num, block_offset, region_marker, sizeof(heap_object_t)); skip_corrupted: if (abort_on_corruption) @@ -301,17 +321,6 @@ skip_unseen: block_offset += (region_marker & ~FREE_SPACE_BIT); continue; } - if (region_marker != sizeof(heap_object_t)) - { - // Write entry (probably) (or garbage) - auto offset_it = offsets_seen.find(block_offset+region_marker); - if (offset_it == offsets_seen.end()) - { - offsets_seen[block_offset+region_marker] = (verify_offset_t){ .start = block_offset, .handled = false }; - } - block_offset += region_marker; - continue; - } heap_object_t *obj = (heap_object_t *)data; { auto offset_it = offsets_seen.upper_bound(block_offset); @@ -352,6 +361,14 @@ skip_unseen: for (auto wr = obj->get_writes(); wr; wr = wr->next(), wr_i++) { uint32_t wr_pos = ((uint8_t*)wr - buf - buf_offset); + if ((wr->entry_type & BS_HEAP_TYPE) < BS_HEAP_SMALL_WRITE || + (wr->entry_type & BS_HEAP_TYPE) > BS_HEAP_INTENT_WRITE || + (wr->entry_type & ~(BS_HEAP_TYPE|BS_HEAP_STABLE))) + { + fprintf(stderr, "Warning: Object %jx:%jx in metadata block %u at %u list entry #%u at %u type %u is invalid, skipping object\n", + obj->inode, obj->stripe, block_num, block_offset, wr_i, wr_pos, entry_type); + goto skip_corrupted; + } if (wr->size != wr->get_size(this)) { fprintf(stderr, "Warning: Object %jx:%jx in metadata block %u at %u list entry #%u at %u size is invalid: %u instead of %u, skipping object\n", @@ -1127,7 +1144,7 @@ void blockstore_heap_t::defragment_block(uint32_t block_num) old += (region_marker & ~FREE_SPACE_BIT); continue; } - if (region_marker != sizeof(heap_object_t)) + if (old[heap_entry_type_pos] != BS_HEAP_OBJECT) { // heap_write_t, skip old += region_marker; @@ -1135,6 +1152,7 @@ void blockstore_heap_t::defragment_block(uint32_t block_num) } // object header heap_object_t *obj = (heap_object_t *)old; + assert(obj->size == sizeof(heap_object_t)); heap_object_t *new_obj = (heap_object_t *)cur; memcpy(cur, obj, sizeof(heap_object_t)); new_obj->write_pos = sizeof(heap_object_t); @@ -1244,7 +1262,7 @@ uint32_t blockstore_heap_t::block_has_compactable(uint8_t *data) uint16_t region_marker = *((uint16_t*)data); assert(region_marker); if (!(region_marker & FREE_SPACE_BIT) && - region_marker > sizeof(heap_object_t)) + data[heap_entry_type_pos] != BS_HEAP_OBJECT) { heap_write_t *wr = (heap_write_t*)data; if (wr->entry_type == (BS_HEAP_SMALL_WRITE|BS_HEAP_STABLE) || @@ -1335,6 +1353,7 @@ int blockstore_heap_t::add_object(object_id oid, heap_write_t *wr, uint32_t *mod // Fill the object entry new_obj->size = sizeof(heap_object_t); new_obj->write_pos = sizeof(heap_object_t); + new_obj->entry_type = BS_HEAP_OBJECT; new_obj->inode = oid.inode; new_obj->stripe = oid.stripe; heap_write_t *new_wr = new_obj->get_writes(); diff --git a/src/blockstore/blockstore_heap.h b/src/blockstore/blockstore_heap.h index d2ab7745..7b35d78b 100644 --- a/src/blockstore/blockstore_heap.h +++ b/src/blockstore/blockstore_heap.h @@ -22,10 +22,11 @@ struct pool_shard_settings_t }; #define BS_HEAP_TYPE 7 -#define BS_HEAP_SMALL_WRITE 1 -#define BS_HEAP_BIG_WRITE 2 -#define BS_HEAP_TOMBSTONE 3 -#define BS_HEAP_INTENT_WRITE 4 +#define BS_HEAP_OBJECT 1 +#define BS_HEAP_SMALL_WRITE 2 +#define BS_HEAP_BIG_WRITE 3 +#define BS_HEAP_TOMBSTONE 4 +#define BS_HEAP_INTENT_WRITE 5 #define BS_HEAP_STABLE 8 class blockstore_heap_t; @@ -68,6 +69,7 @@ struct __attribute__((__packed__)) heap_object_t // linked list of write entries... // newest entries are stored first to simplify scanning int16_t write_pos = 0; + uint8_t entry_type = 0; // BS_HEAP_* uint32_t crc32c = 0; uint64_t inode = 0; uint64_t stripe = 0; diff --git a/src/test/test_heap.cpp b/src/test/test_heap.cpp index 39e7b7b4..ebbfadd9 100644 --- a/src/test/test_heap.cpp +++ b/src/test/test_heap.cpp @@ -295,7 +295,7 @@ void test_compact_block() uint32_t big_write_size = (sizeof(heap_object_t) + sizeof(heap_write_t) + 2*dsk.clean_entry_bitmap_size + dsk.data_block_size/dsk.csum_block_size*4); uint32_t small_write_size = (sizeof(heap_write_t) + dsk.clean_entry_bitmap_size + 4); - assert(big_write_size == 197); + assert(big_write_size == 198); assert(small_write_size == 45); uint32_t nwr = dsk.meta_block_size/(big_write_size+small_write_size); @@ -319,7 +319,8 @@ void test_compact_block() _test_big_write(heap, dsk, 1, nwr*2*0x20000, 1, nwr*2*0x20000); _test_big_write(heap, dsk, 1, (nwr*2+1)*0x20000, 1, (nwr*2+1)*0x20000); _test_big_write(heap, dsk, 1, (nwr*2+2)*0x20000, 1, (nwr*2+2)*0x20000); - assert(count_free_fragments(heap, dsk, 0) == 1); + assert(count_free_fragments(heap, dsk, 0) == nwr+1); + assert(count_free_fragments(heap, dsk, 1) == 1); } printf("OK test_compact_block\n"); @@ -842,6 +843,7 @@ void _test_invalid_data_setup(blockstore_disk_t & dsk, std::vector & bu tmp.resize(dsk.meta_block_size*2); heap_object_t *obj = (heap_object_t*)tmp.data(); + obj->entry_type = BS_HEAP_OBJECT; obj->size = sizeof(heap_object_t); obj->inode = INODE_WITH_POOL(1, 1); obj->write_pos = sizeof(heap_object_t); @@ -854,6 +856,7 @@ void _test_invalid_data_setup(blockstore_disk_t & dsk, std::vector & bu obj->crc32c = obj->calc_crc32c(); obj = (heap_object_t*)((uint8_t*)wr + wr->size); + obj->entry_type = BS_HEAP_OBJECT; obj->size = sizeof(heap_object_t); obj->inode = INODE_WITH_POOL(1, 3); obj->stripe = 0; @@ -866,6 +869,7 @@ void _test_invalid_data_setup(blockstore_disk_t & dsk, std::vector & bu obj->crc32c = obj->calc_crc32c(); obj = (heap_object_t*)(tmp.data() + dsk.meta_block_size); + obj->entry_type = BS_HEAP_OBJECT; obj->size = sizeof(heap_object_t); obj->inode = INODE_WITH_POOL(1, 2); obj->write_pos = sizeof(heap_object_t); @@ -1296,7 +1300,7 @@ void test_full_alloc() uint32_t big_write_size = (sizeof(heap_object_t) + sizeof(heap_write_t) + 2*dsk.clean_entry_bitmap_size + dsk.data_block_size/dsk.csum_block_size*4); uint32_t small_write_size = (sizeof(heap_write_t) + dsk.clean_entry_bitmap_size + 4); - assert(big_write_size == 197); + assert(big_write_size == 198); assert(small_write_size == 45); uint32_t b_4s = (big_write_size + 4*small_write_size); // 377 uint32_t epb = (4096-800+b_4s-1)/b_4s; // entries per block @@ -1597,7 +1601,7 @@ void test_move() uint32_t big_write_size = (sizeof(heap_object_t) + sizeof(heap_write_t) + 2*dsk.clean_entry_bitmap_size + dsk.data_block_size/dsk.csum_block_size*4); uint32_t small_write_size = (sizeof(heap_write_t) + dsk.clean_entry_bitmap_size + 4); - assert(big_write_size == 197); + assert(big_write_size == 198); assert(small_write_size == 45); // Fill block 1 almost completely with unstable small writes