Skip to content

Commit

Permalink
os/bluestore/bluefs: Fix improper vselector tracking in _flush_special()
Browse files Browse the repository at this point in the history
Moves vselector size tracking outside _flush_special().
Function _compact_log_async...() updated sizes twice.
Problem could not be solved by making second modification of size just update,
as it will possibly disrupt vselector consistency check (_vselector_check()).
Feature to track vselector consistency relies on the fact that either log.lock or nodes.lock
are taken when the check is performed. Which is not true for _compact_log_async...().

Now _flush_special does not update vselector sizes by itself but leaves the update to
the caller.

Fixes: https://tracker.ceph.com/issues/54248

Signed-off-by: Adam Kupczyk <akupczyk@redhat.com>
  • Loading branch information
aclamk committed Feb 14, 2022
1 parent e883dc3 commit 4bc0f61
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 11 deletions.
20 changes: 10 additions & 10 deletions src/os/bluestore/BlueFS.cc
Expand Up @@ -2495,8 +2495,8 @@ void BlueFS::_rewrite_log_and_layout_sync_LNF_LD(bool allocate_with_fallback,

log.writer = _create_writer(log_file);
log.writer->append(bl);
r = _flush_special(log.writer);
ceph_assert(r == 0);
uint64_t new_data = _flush_special(log.writer);
vselector->add_usage(log_file->vselector_hint, new_data);
#ifdef HAVE_LIBAIO
if (!cct->_conf->bluefs_sync_write) {
list<aio_t> completed_ios;
Expand Down Expand Up @@ -2648,8 +2648,7 @@ void BlueFS::_compact_log_async_LD_LNF_D() //also locks FW for new_writer

new_log_writer->append(bl);
// 3. flush
r = _flush_special(new_log_writer);
ceph_assert(r == 0);
_flush_special(new_log_writer);

// 4. wait
_flush_bdev(new_log_writer);
Expand Down Expand Up @@ -2863,8 +2862,8 @@ void BlueFS::_flush_and_sync_log_core(int64_t runway)
log.t.clear();
log.t.seq = log.seq_live;

int r = _flush_special(log.writer);
ceph_assert(r == 0);
uint64_t new_data = _flush_special(log.writer);
vselector->add_usage(log.writer->file->vselector_hint, new_data);
}

// Clears dirty.files up to (including) seq_stable.
Expand Down Expand Up @@ -3341,18 +3340,19 @@ int BlueFS::_flush_F(FileWriter *h, bool force, bool *flushed)
// we do not need to dirty the log file (or it's compacting
// replacement) when the file size changes because replay is
// smart enough to discover it on its own.
int BlueFS::_flush_special(FileWriter *h)
uint64_t BlueFS::_flush_special(FileWriter *h)
{
ceph_assert(h->file->fnode.ino <= 1);
uint64_t length = h->get_buffer_length();
uint64_t offset = h->pos;
uint64_t new_data = 0;
ceph_assert(length + offset <= h->file->fnode.get_allocated());
if (h->file->fnode.size < offset + length) {
vselector->sub_usage(h->file->vselector_hint, h->file->fnode.size);
new_data = offset + length - h->file->fnode.size;
h->file->fnode.size = offset + length;
vselector->add_usage(h->file->vselector_hint, h->file->fnode.size);
}
return _flush_data(h, offset, length, false);
_flush_data(h, offset, length, false);
return new_data;
}

int BlueFS::truncate(FileWriter *h, uint64_t offset)/*_WF_L*/
Expand Down
2 changes: 1 addition & 1 deletion src/os/bluestore/BlueFS.h
Expand Up @@ -427,7 +427,7 @@ class BlueFS {
int _flush_range_F(FileWriter *h, uint64_t offset, uint64_t length);
int _flush_data(FileWriter *h, uint64_t offset, uint64_t length, bool buffered);
int _flush_F(FileWriter *h, bool force, bool *flushed = nullptr);
int _flush_special(FileWriter *h);
uint64_t _flush_special(FileWriter *h);
int _fsync(FileWriter *h);

#ifdef HAVE_LIBAIO
Expand Down

0 comments on commit 4bc0f61

Please sign in to comment.