Skip to content

Commit

Permalink
os/bluestore/bluefs: Add tags and comments for locks
Browse files Browse the repository at this point in the history
Add function tags and comments related to locks.
Fixed locking graph documentation.

Signed-off-by: Adam Kupczyk <akupczyk@redhat.com>
  • Loading branch information
aclamk committed Jan 4, 2022
1 parent fd9ef8b commit a091ea7
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 35 deletions.
45 changes: 23 additions & 22 deletions src/os/bluestore/BlueFS.cc
Expand Up @@ -2291,13 +2291,13 @@ uint64_t BlueFS::_estimate_log_size_N()
return round_up_to(size, super.block_size);
}

void BlueFS::compact_log()
void BlueFS::compact_log()/*_LNF_LD_NF_D*/
{
if (!cct->_conf->bluefs_replay_recovery_disable_compact) {
if (cct->_conf->bluefs_compact_log_sync) {
_compact_log_sync_LNF_LD();
} else {
_compact_log_async_LD_NF_D();
_compact_log_async_LD_LNF_D();
}
}
}
Expand Down Expand Up @@ -2551,7 +2551,7 @@ void BlueFS::_rewrite_log_and_layout_sync_LNF_LD(bool allocate_with_fallback,
* 8. Release the old log space. Clean up.
*/

void BlueFS::_compact_log_async_LD_NF_D() //also locks FW for new_writer
void BlueFS::_compact_log_async_LD_LNF_D() //also locks FW for new_writer
{
dout(10) << __func__ << dendl;
// only one compaction allowed at one time
Expand Down Expand Up @@ -3080,7 +3080,8 @@ int BlueFS::_signal_dirty_to_log_D(FileWriter *h)
return 0;
}

void BlueFS::flush_range(FileWriter *h, uint64_t offset, uint64_t length) {
void BlueFS::flush_range(FileWriter *h, uint64_t offset, uint64_t length)/*_WF*/
{
std::unique_lock hl(h->lock);
_flush_range_F(h, offset, length);
}
Expand Down Expand Up @@ -3255,7 +3256,7 @@ void BlueFS::_wait_for_aio(FileWriter *h)
}
#endif

void BlueFS::append_try_flush(FileWriter *h, const char* buf, size_t len)
void BlueFS::append_try_flush(FileWriter *h, const char* buf, size_t len)/*_WF_LNF_NF_LD_D*/
{
bool flushed_sum = false;
{
Expand Down Expand Up @@ -3287,7 +3288,7 @@ void BlueFS::append_try_flush(FileWriter *h, const char* buf, size_t len)
}
}

void BlueFS::flush(FileWriter *h, bool force)
void BlueFS::flush(FileWriter *h, bool force)/*_WF_LNF_NF_LD_D*/
{
bool flushed = false;
int r;
Expand Down Expand Up @@ -3352,7 +3353,7 @@ int BlueFS::_flush_special(FileWriter *h)
return _flush_data(h, offset, length, false);
}

int BlueFS::truncate(FileWriter *h, uint64_t offset)
int BlueFS::truncate(FileWriter *h, uint64_t offset)/*_WF_L*/
{
std::lock_guard hl(h->lock);
dout(10) << __func__ << " 0x" << std::hex << offset << std::dec
Expand Down Expand Up @@ -3394,7 +3395,7 @@ int BlueFS::truncate(FileWriter *h, uint64_t offset)
return 0;
}

int BlueFS::fsync(FileWriter *h)
int BlueFS::fsync(FileWriter *h)/*_WF_WD_WLD_WLNF_WNF*/
{
std::unique_lock hl(h->lock);
uint64_t old_dirty_seq = 0;
Expand Down Expand Up @@ -3576,7 +3577,7 @@ int BlueFS::_allocate(uint8_t id, uint64_t len,
return 0;
}

int BlueFS::preallocate(FileRef f, uint64_t off, uint64_t len)
int BlueFS::preallocate(FileRef f, uint64_t off, uint64_t len)/*_LF*/
{
std::lock_guard ll(log.lock);
std::lock_guard fl(f->lock);
Expand Down Expand Up @@ -3604,7 +3605,7 @@ int BlueFS::preallocate(FileRef f, uint64_t off, uint64_t len)
return 0;
}

void BlueFS::sync_metadata(bool avoid_compact)
void BlueFS::sync_metadata(bool avoid_compact)/*_LNF_NF_LD_D*/
{
bool can_skip_flush;
{
Expand Down Expand Up @@ -3636,7 +3637,7 @@ void BlueFS::_maybe_compact_log_LNF_NF_LD_D()
if (cct->_conf->bluefs_compact_log_sync) {
_compact_log_sync_LNF_LD();
} else {
_compact_log_async_LD_NF_D();
_compact_log_async_LD_LNF_D();
}
}
}
Expand All @@ -3645,7 +3646,7 @@ int BlueFS::open_for_write(
std::string_view dirname,
std::string_view filename,
FileWriter **h,
bool overwrite)
bool overwrite)/*_N_LD*/
{
FileRef file;
bool create = false;
Expand Down Expand Up @@ -3799,7 +3800,7 @@ int BlueFS::open_for_read(
std::string_view dirname,
std::string_view filename,
FileReader **h,
bool random)
bool random)/*_N*/
{
std::lock_guard nl(nodes.lock);
dout(10) << __func__ << " " << dirname << "/" << filename
Expand Down Expand Up @@ -3828,7 +3829,7 @@ int BlueFS::open_for_read(

int BlueFS::rename(
std::string_view old_dirname, std::string_view old_filename,
std::string_view new_dirname, std::string_view new_filename)
std::string_view new_dirname, std::string_view new_filename)/*_LND*/
{
std::lock_guard ll(log.lock);
std::lock_guard nl(nodes.lock);
Expand Down Expand Up @@ -3876,7 +3877,7 @@ int BlueFS::rename(
return 0;
}

int BlueFS::mkdir(std::string_view dirname)
int BlueFS::mkdir(std::string_view dirname)/*_LN*/
{
std::lock_guard ll(log.lock);
std::lock_guard nl(nodes.lock);
Expand All @@ -3891,7 +3892,7 @@ int BlueFS::mkdir(std::string_view dirname)
return 0;
}

int BlueFS::rmdir(std::string_view dirname)
int BlueFS::rmdir(std::string_view dirname)/*_LN*/
{
std::lock_guard ll(log.lock);
std::lock_guard nl(nodes.lock);
Expand All @@ -3911,7 +3912,7 @@ int BlueFS::rmdir(std::string_view dirname)
return 0;
}

bool BlueFS::dir_exists(std::string_view dirname)
bool BlueFS::dir_exists(std::string_view dirname)/*_N*/
{
std::lock_guard nl(nodes.lock);
map<string,DirRef>::iterator p = nodes.dir_map.find(dirname);
Expand All @@ -3921,7 +3922,7 @@ bool BlueFS::dir_exists(std::string_view dirname)
}

int BlueFS::stat(std::string_view dirname, std::string_view filename,
uint64_t *size, utime_t *mtime)
uint64_t *size, utime_t *mtime)/*_N*/
{
std::lock_guard nl(nodes.lock);
dout(10) << __func__ << " " << dirname << "/" << filename << dendl;
Expand Down Expand Up @@ -3949,7 +3950,7 @@ int BlueFS::stat(std::string_view dirname, std::string_view filename,
}

int BlueFS::lock_file(std::string_view dirname, std::string_view filename,
FileLock **plock)
FileLock **plock)/*_LN*/
{
std::lock_guard ll(log.lock);
std::lock_guard nl(nodes.lock);
Expand Down Expand Up @@ -3989,7 +3990,7 @@ int BlueFS::lock_file(std::string_view dirname, std::string_view filename,
return 0;
}

int BlueFS::unlock_file(FileLock *fl)
int BlueFS::unlock_file(FileLock *fl)/*_N*/
{
std::lock_guard nl(nodes.lock);
dout(10) << __func__ << " " << fl << " on " << fl->file->fnode << dendl;
Expand All @@ -3999,7 +4000,7 @@ int BlueFS::unlock_file(FileLock *fl)
return 0;
}

int BlueFS::readdir(std::string_view dirname, vector<string> *ls)
int BlueFS::readdir(std::string_view dirname, vector<string> *ls)/*_N*/
{
// dirname may contain a trailing /
if (!dirname.empty() && dirname.back() == '/') {
Expand Down Expand Up @@ -4031,7 +4032,7 @@ int BlueFS::readdir(std::string_view dirname, vector<string> *ls)
return 0;
}

int BlueFS::unlink(std::string_view dirname, std::string_view filename)
int BlueFS::unlink(std::string_view dirname, std::string_view filename)/*_LND*/
{
std::lock_guard ll(log.lock);
std::lock_guard nl(nodes.lock);
Expand Down
27 changes: 14 additions & 13 deletions src/os/bluestore/BlueFS.h
Expand Up @@ -458,7 +458,7 @@ class BlueFS {
uint64_t capture_before_seq);

void _compact_log_sync_LNF_LD();
void _compact_log_async_LD_NF_D();
void _compact_log_async_LD_LNF_D();

void _rewrite_log_and_layout_sync_LNF_LD(bool allocate_with_fallback,
int super_dev,
Expand Down Expand Up @@ -590,8 +590,6 @@ class BlueFS {

/// sync any uncommitted state to disk
void sync_metadata(bool avoid_compact);
/// test and compact log, if necessary
void _maybe_compact_log_LNF_NF_LD_D();

void set_volume_selector(BlueFSVolumeSelector* s) {
vselector.reset(s);
Expand Down Expand Up @@ -635,11 +633,6 @@ class BlueFS {
void invalidate_cache(FileRef f, uint64_t offset, uint64_t len);
int preallocate(FileRef f, uint64_t offset, uint64_t len);
int truncate(FileWriter *h, uint64_t offset);
int _do_replay_recovery_read(FileReader *log,
size_t log_pos,
size_t read_offset,
size_t read_len,
bufferlist* bl);

size_t probe_alloc_avail(int dev, uint64_t alloc_size);

Expand All @@ -660,6 +653,14 @@ class BlueFS {
int _bdev_read(uint8_t ndev, uint64_t off, uint64_t len,
ceph::buffer::list* pbl, IOContext* ioc, bool buffered);
int _bdev_read_random(uint8_t ndev, uint64_t off, uint64_t len, char* buf, bool buffered);

/// test and compact log, if necessary
void _maybe_compact_log_LNF_NF_LD_D();
int _do_replay_recovery_read(FileReader *log,
size_t log_pos,
size_t read_offset,
size_t read_len,
bufferlist* bl);
};

class OriginalVolumeSelector : public BlueFSVolumeSelector {
Expand Down Expand Up @@ -714,15 +715,15 @@ class FitToFastVolumeSelector : public OriginalVolumeSelector {
* Vertices - Locks. Edges (directed) - locking progression.
* Edge A->B exist if last taken lock was A and next taken lock is B.
*
* Column represents last lock taken.
* Row represents next lock taken.
* Row represents last lock taken.
* Column represents next lock taken.
*
* > | W | L | D | N | F
* > | W | L | N | D | F
* -------------|---|---|---|---|---
* FileWriter W | | > | > | | >
* FileWriter W | | > | > | > | >
* log L | | > | > | >
* nodes N | | > | >
* dirty D | | | >
* nodes N | | >
* File F |
*
* Claim: Deadlock is possible IFF graph contains cycles.
Expand Down

0 comments on commit a091ea7

Please sign in to comment.