Skip to content

Commit 7f36010

Browse files
committed
os/bluestore: Fix BlueFS::truncate()
In `struct bluefs_fnode_t` there is a vector `extents` and the vector `extents_index` that is a log2 seek cache. Until modifications to truncate() we never removed extents from files. Modified truncate() did not update extents_index. For example 10 extents long files when truncated to 0 will have: 0 extents, 10 extents_index. After writing some data to file: 1 extents, 11 extents_index. Now, `bluefs_fnode_t::seek` will binary search extents_index, lets say it located seek at item ceph#3. It will then jump up from #0 extent (that exists) to ceph#3 extent which does not exist at. The worst part is that code is now broken, as ceph#3 != extent.end(). There are 3 parts of the fix: 1) assert in `bluefs_fnode_t::seek` to protect against jumping outside extents 2) code in BlueFS::truncate to sync up `extents_index` with `extents` 3) dampening down assert in _replay to give a way out of cases where incorrect "offset 12345" (12345 is file size) instead of "offset 20000" (allocations occupied) was written to log. Fixes: https://tracker.ceph.com/issues/69481 Signed-off-by: Adam Kupczyk <[email protected]>
1 parent f2b5e2f commit 7f36010

File tree

3 files changed

+7
-2
lines changed

3 files changed

+7
-2
lines changed

src/os/bluestore/BlueFS.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1706,7 +1706,8 @@ int BlueFS::_replay(bool noop, bool to_stdout)
17061706
<< " fnode=" << fnode
17071707
<< " delta=" << delta
17081708
<< dendl;
1709-
ceph_assert(delta.offset == fnode.allocated);
1709+
// be leanient, if there is no extents just produce error message
1710+
ceph_assert(delta.offset == fnode.allocated || delta.extents.empty());
17101711
}
17111712
if (cct->_conf->bluefs_log_replay_check_allocations) {
17121713
int r = _check_allocations(fnode,
@@ -3830,6 +3831,7 @@ int BlueFS::truncate(FileWriter *h, uint64_t offset)/*_WF_L*/
38303831
fnode.size = offset;
38313832
fnode.allocated = new_allocated;
38323833
fnode.reset_delta();
3834+
fnode.recalc_allocated();
38333835
log.t.op_file_update(fnode);
38343836
// sad, but is_dirty must be set to signal flushing of the log
38353837
h->file->is_dirty = true;

src/os/bluestore/bluefs_types.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,9 @@ mempool::bluefs::vector<bluefs_extent_t>::iterator bluefs_fnode_t::seek(
154154
assert(it != extents_index.begin());
155155
--it;
156156
assert(offset >= *it);
157-
p += it - extents_index.begin();
157+
uint32_t skip = it - extents_index.begin();
158+
ceph_assert(skip <= extents.size());
159+
p += skip;
158160
offset -= *it;
159161
}
160162

src/os/bluestore/bluefs_types.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ struct bluefs_fnode_t {
8989
void recalc_allocated() {
9090
allocated = 0;
9191
extents_index.reserve(extents.size());
92+
extents_index.clear();
9293
for (auto& p : extents) {
9394
extents_index.emplace_back(allocated);
9495
allocated += p.length;

0 commit comments

Comments
 (0)