From ad9f6193707fc91f7e9ec624ffafe9468c985156 Mon Sep 17 00:00:00 2001
From: Vitaliy Filippov <vitalif@yourcmc.ru>
Date: Sun, 28 Mar 2021 00:16:12 +0300
Subject: [PATCH] Skip double allocs when reading journal

---
 src/allocator.cpp           | 15 +++++++++++++++
 src/allocator.h             |  1 +
 src/blockstore_init.cpp     | 38 +++++++++++++++++++++++++++++--------
 src/blockstore_init.h       |  1 +
 src/blockstore_rollback.cpp |  3 ++-
 src/test_allocator.cpp      |  8 ++++++++
 6 files changed, 57 insertions(+), 9 deletions(-)

diff --git a/src/allocator.cpp b/src/allocator.cpp
index 39b8a7b9..2893a56b 100644
--- a/src/allocator.cpp
+++ b/src/allocator.cpp
@@ -37,6 +37,21 @@ allocator::~allocator()
     delete[] mask;
 }
 
+bool allocator::get(uint64_t addr)
+{
+    if (addr >= size)
+    {
+        return false;
+    }
+    uint64_t p2 = 1, offset = 0;
+    while (p2 * 64 < size)
+    {
+        offset += p2;
+        p2 = p2 * 64;
+    }
+    return ((mask[offset + addr/64] >> (addr % 64)) & 1);
+}
+
 void allocator::set(uint64_t addr, bool value)
 {
     if (addr >= size)
diff --git a/src/allocator.h b/src/allocator.h
index 58b6808d..1bc5d444 100644
--- a/src/allocator.h
+++ b/src/allocator.h
@@ -16,6 +16,7 @@ class allocator
 public:
     allocator(uint64_t blocks);
     ~allocator();
+    bool get(uint64_t addr);
     void set(uint64_t addr, bool value);
     uint64_t find_free();
     uint64_t get_free_count();
diff --git a/src/blockstore_init.cpp b/src/blockstore_init.cpp
index 5faf4c2d..2a1be17e 100644
--- a/src/blockstore_init.cpp
+++ b/src/blockstore_init.cpp
@@ -402,6 +402,18 @@ resume_1:
             }
         }
     }
+    for (auto ov: double_allocs)
+    {
+        auto dirty_it = bs->dirty_db.find(ov);
+        if (dirty_it != bs->dirty_db.end() &&
+            IS_BIG_WRITE(dirty_it->second.state) &&
+            dirty_it->second.location == UINT64_MAX)
+        {
+            printf("Fatal error (bug): %lx:%lx v%lu big_write journal_entry was allocated over another object\n",
+                dirty_it->first.oid.inode, dirty_it->first.oid.stripe, dirty_it->first.version);
+            exit(1);
+        }
+    }
     bs->flusher->mark_trim_possible();
     bs->journal.dirty_start = bs->journal.next_free;
     printf(
@@ -597,22 +609,32 @@ int blockstore_init_journal::handle_journal_part(void *buf, uint64_t done_pos, u
                         .oid = je->big_write.oid,
                         .version = je->big_write.version,
                     };
-                    bs->dirty_db.emplace(ov, (dirty_entry){
+                    auto dirty_it = bs->dirty_db.emplace(ov, (dirty_entry){
                         .state = (BS_ST_BIG_WRITE | BS_ST_SYNCED),
                         .flags = 0,
                         .location = je->big_write.location,
                         .offset = je->big_write.offset,
                         .len = je->big_write.len,
                         .journal_sector = proc_pos,
-                    });
+                    }).first;
+                    if (bs->data_alloc->get(je->big_write.location >> bs->block_order))
+                    {
+                        // This is probably a big_write that's already flushed and freed, but it may
+                        // also indicate a bug. So we remember such entries and recheck them afterwards
+                        dirty_it->second.location = UINT64_MAX;
+                        double_allocs.push_back(ov);
+                    }
+                    else
+                    {
 #ifdef BLOCKSTORE_DEBUG
-                    printf(
-                        "Allocate block (journal) %lu: %lx:%lx v%lu\n",
-                        je->big_write.location >> bs->block_order,
-                        ov.oid.inode, ov.oid.stripe, ov.version
-                    );
+                        printf(
+                            "Allocate block (journal) %lu: %lx:%lx v%lu\n",
+                            je->big_write.location >> bs->block_order,
+                            ov.oid.inode, ov.oid.stripe, ov.version
+                        );
 #endif
-                    bs->data_alloc->set(je->big_write.location >> bs->block_order, true);
+                        bs->data_alloc->set(je->big_write.location >> bs->block_order, true);
+                    }
                     bs->journal.used_sectors[proc_pos]++;
 #ifdef BLOCKSTORE_DEBUG
                     printf(
diff --git a/src/blockstore_init.h b/src/blockstore_init.h
index ad8f9970..cce032da 100644
--- a/src/blockstore_init.h
+++ b/src/blockstore_init.h
@@ -36,6 +36,7 @@ class blockstore_init_journal
     bool started = false;
     uint64_t next_free;
     std::vector<bs_init_journal_done> done;
+    std::vector<obj_ver_id> double_allocs;
     uint64_t journal_pos = 0;
     uint64_t continue_pos = 0;
     void *init_write_buf = NULL;
diff --git a/src/blockstore_rollback.cpp b/src/blockstore_rollback.cpp
index 0de305d9..d5037b09 100644
--- a/src/blockstore_rollback.cpp
+++ b/src/blockstore_rollback.cpp
@@ -248,7 +248,8 @@ void blockstore_impl_t::erase_dirty(blockstore_dirty_db_t::iterator dirty_start,
     }
     while (1)
     {
-        if (IS_BIG_WRITE(dirty_it->second.state) && dirty_it->second.location != clean_loc)
+        if (IS_BIG_WRITE(dirty_it->second.state) && dirty_it->second.location != clean_loc &&
+            dirty_it->second.location != UINT64_MAX)
         {
 #ifdef BLOCKSTORE_DEBUG
             printf("Free block %lu from %lx:%lx v%lu\n", dirty_it->second.location >> block_order,
diff --git a/src/test_allocator.cpp b/src/test_allocator.cpp
index 71d9fca5..a7651b32 100644
--- a/src/test_allocator.cpp
+++ b/src/test_allocator.cpp
@@ -20,7 +20,15 @@ void alloc_all(int size)
         {
             printf("incorrect block allocated: expected %d, got %lu\n", i, x);
         }
+        if (a->get(x))
+        {
+            printf("not free before set at %d\n", i);
+        }
         a->set(x, true);
+        if (!a->get(x))
+        {
+            printf("free after set at %d\n", i);
+        }
     }
     uint64_t x = a->find_free();
     if (x != UINT64_MAX)