New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
os/bluestore: make zone/span size of bitmap-allocator configurable #10040
Changes from all commits
b7d09ed
0c86d5e
6147ea0
39458dd
af82dbc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ | |
* of the interfaces defined in BitMapArea. | ||
*/ | ||
|
||
#include "common/dout.h" | ||
#include "BitAllocator.h" | ||
#include <assert.h> | ||
#include <math.h> | ||
|
@@ -26,7 +27,7 @@ | |
|
||
int64_t BitMapAreaLeaf::count = 0; | ||
int64_t BitMapZone::count = 0; | ||
int64_t BitMapZone::total_blocks = BITMAP_SPAN_SIZE; | ||
int64_t BitMapZone::total_blocks = 0; | ||
|
||
/* | ||
* BmapEntityList functions. | ||
|
@@ -345,6 +346,7 @@ int BmapEntry::find_any_free_bits(int start_offset, int64_t num_blocks, | |
void BitMapZone::init(int64_t zone_num, int64_t total_blocks, bool def) | ||
{ | ||
m_area_index = zone_num; | ||
BitMapZone::total_blocks = total_blocks; | ||
debug_assert(size() > 0); | ||
m_type = ZONE; | ||
|
||
|
@@ -417,11 +419,7 @@ BitMapZone::~BitMapZone() | |
bool BitMapZone::is_exhausted() | ||
{ | ||
debug_assert(check_locked()); | ||
if (get_used_blocks() == size()) { | ||
return true; | ||
} else { | ||
return false; | ||
} | ||
return get_used_blocks() == size(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks good |
||
} | ||
|
||
bool BitMapZone::is_allocated(int64_t start_block, int64_t num_blocks) | ||
|
@@ -521,10 +519,7 @@ void BitMapZone::lock_excl() | |
|
||
bool BitMapZone::lock_excl_try() | ||
{ | ||
if (m_lock.try_lock()) { | ||
return true; | ||
} | ||
return false; | ||
return m_lock.try_lock(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks good |
||
} | ||
|
||
void BitMapZone::unlock() | ||
|
@@ -634,9 +629,14 @@ int64_t BitMapZone::alloc_blocks_dis(int64_t num_blocks, int64_t zone_blk_off, i | |
/* | ||
* BitMapArea Leaf and non-Leaf functions. | ||
*/ | ||
int64_t BitMapArea::get_zone_size() | ||
{ | ||
return g_conf->bluestore_bitmapallocator_blocks_per_zone; | ||
} | ||
|
||
int64_t BitMapArea::get_span_size() | ||
{ | ||
return BITMAP_SPAN_SIZE; | ||
return g_conf->bluestore_bitmapallocator_span_size; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks good. You can do same for g_conf->bluestore_bitmapallocator_blocks_per_zone instead of accessing this g_conf member at places, having a static function will look cleaner. |
||
} | ||
|
||
bmap_area_type_t BitMapArea::level_to_type(int level) | ||
|
@@ -653,15 +653,34 @@ bmap_area_type_t BitMapArea::level_to_type(int level) | |
int BitMapArea::get_level(int64_t total_blocks) | ||
{ | ||
int level = 1; | ||
int64_t span_size = BitMapArea::get_span_size(); | ||
int64_t spans = span_size * span_size; | ||
int64_t zone_size_block = get_zone_size(); | ||
int64_t span_size = get_span_size(); | ||
int64_t spans = zone_size_block * span_size; | ||
while (spans < total_blocks) { | ||
spans *= span_size; | ||
level++; | ||
} | ||
return level; | ||
} | ||
|
||
int64_t BitMapArea::get_level_factor(int level) | ||
{ | ||
debug_assert(level > 0); | ||
|
||
int64_t zone_size = get_zone_size(); | ||
if (level == 1) { | ||
return zone_size; | ||
} | ||
|
||
int64_t level_factor = zone_size; | ||
int64_t span_size = get_span_size(); | ||
while (--level) { | ||
level_factor *= span_size; | ||
} | ||
|
||
return level_factor; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks good, please make it static and access same in test file, declaring new function there is not required? |
||
|
||
int64_t BitMapArea::get_index() | ||
{ | ||
return m_area_index; | ||
|
@@ -697,7 +716,7 @@ void BitMapAreaIN::init(int64_t total_blocks, int64_t area_idx, bool def) | |
debug_assert(!(total_blocks % BmapEntry::size())); | ||
|
||
init_common(total_blocks, area_idx, def); | ||
int64_t level_factor = pow(BitMapArea::get_span_size(), m_level); | ||
int64_t level_factor = BitMapArea::get_level_factor(m_level); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks good |
||
|
||
num_child = (total_blocks + level_factor - 1) / level_factor; | ||
debug_assert(num_child < std::numeric_limits<int16_t>::max()); | ||
|
@@ -767,10 +786,7 @@ void BitMapAreaIN::child_unlock(BitMapArea *child) | |
|
||
bool BitMapAreaIN::is_exhausted() | ||
{ | ||
if (get_used_blocks() == size()) { | ||
return true; | ||
} | ||
return false; | ||
return get_used_blocks() == size(); | ||
} | ||
|
||
int64_t BitMapAreaIN::add_used_blocks(int64_t blks) | ||
|
@@ -930,7 +946,7 @@ int64_t BitMapAreaIN::alloc_blocks_dis_int(bool wait, int64_t num_blocks, | |
} | ||
|
||
blk_off = child->get_index() * m_child_size_blocks + area_blk_off; | ||
allocated += child->alloc_blocks_dis(wait, num_blocks, | ||
allocated += child->alloc_blocks_dis(wait, num_blocks - allocated, | ||
blk_off, &block_list[allocated]); | ||
child_unlock(child); | ||
if (allocated == num_blocks) { | ||
|
@@ -1054,11 +1070,15 @@ void BitMapAreaLeaf::init(int64_t total_blocks, int64_t area_idx, | |
debug_assert(!(total_blocks % BmapEntry::size())); | ||
|
||
init_common(total_blocks, area_idx, def); | ||
num_child = total_blocks / pow(BitMapArea::get_span_size(), m_level); | ||
debug_assert(m_level == 1); | ||
int zone_size_block = get_zone_size(); | ||
debug_assert(zone_size_block > 0); | ||
num_child = (total_blocks + zone_size_block - 1) / zone_size_block; | ||
debug_assert(num_child); | ||
m_child_size_blocks = total_blocks / num_child; | ||
|
||
debug_assert(m_level == 1); | ||
BitMapArea **children = new BitMapArea*[num_child]; | ||
BitMapArea **children = new BitMapArea*[num_child]; | ||
for (int i = 0; i < num_child; i++) { | ||
children[i] = new BitMapZone(m_child_size_blocks, i, def); | ||
} | ||
|
@@ -1154,7 +1174,8 @@ int64_t BitMapAreaLeaf::alloc_blocks_dis_int(bool wait, int64_t num_blocks, | |
} | ||
|
||
blk_off = child->get_index() * m_child_size_blocks + area_blk_off; | ||
allocated += child->alloc_blocks_dis(num_blocks, blk_off, &block_list[allocated]); | ||
allocated += child->alloc_blocks_dis(num_blocks - allocated, | ||
blk_off, &block_list[allocated]); | ||
child_unlock(child); | ||
if (allocated == num_blocks) { | ||
break; | ||
|
@@ -1358,7 +1379,7 @@ bool BitAllocator::check_input(int64_t num_blocks) | |
return false; | ||
} | ||
|
||
if (num_blocks > BitMapArea::get_span_size()) { | ||
if (num_blocks > get_zone_size()) { | ||
return false; | ||
} | ||
return true; | ||
|
@@ -1543,7 +1564,8 @@ int64_t BitAllocator::alloc_blocks_dis(int64_t num_blocks, int64_t *block_list) | |
} | ||
|
||
while (scans && allocated < num_blocks) { | ||
allocated += alloc_blocks_dis_int(false, num_blocks, blk_off, &block_list[allocated]); | ||
allocated += alloc_blocks_dis_int(false, num_blocks - allocated, | ||
blk_off, &block_list[allocated]); | ||
scans--; | ||
} | ||
|
||
|
@@ -1557,7 +1579,8 @@ int64_t BitAllocator::alloc_blocks_dis(int64_t num_blocks, int64_t *block_list) | |
unlock(); | ||
lock_excl(); | ||
serial_lock(); | ||
allocated += alloc_blocks_dis_int(false, num_blocks, blk_off, &block_list[allocated]); | ||
allocated += alloc_blocks_dis_int(false, num_blocks - allocated, | ||
blk_off, &block_list[allocated]); | ||
if (is_stats_on()) { | ||
m_stats->add_serial_scans(1); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,23 @@ BitMapAllocator::BitMapAllocator(int64_t device_size, int64_t block_size) | |
: m_num_uncommitted(0), | ||
m_num_committing(0) | ||
{ | ||
int64_t zone_size_blks = 1024; // Change it later | ||
int64_t zone_size_blks = g_conf->bluestore_bitmapallocator_blocks_per_zone; | ||
assert((zone_size_blks & (zone_size_blks - 1)) == 0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks good |
||
if (zone_size_blks & (zone_size_blks - 1)) { | ||
derr << __func__ << " zone_size " << zone_size_blks | ||
<< " not power of 2 aligned!" | ||
<< dendl; | ||
return; | ||
} | ||
|
||
int64_t span_size = g_conf->bluestore_bitmapallocator_span_size; | ||
assert((span_size & (span_size - 1)) == 0); | ||
if (span_size & (span_size - 1)) { | ||
derr << __func__ << " span_size " << span_size | ||
<< " not power of 2 aligned!" | ||
<< dendl; | ||
return; | ||
} | ||
|
||
m_block_size = block_size; | ||
m_bit_alloc = new BitAllocator(device_size / block_size, | ||
|
@@ -89,6 +105,8 @@ int BitMapAllocator::allocate( | |
uint64_t want_size, uint64_t alloc_unit, int64_t hint, | ||
uint64_t *offset, uint32_t *length) | ||
{ | ||
if (want_size == 0) | ||
return 0; | ||
|
||
assert(!(alloc_unit % m_block_size)); | ||
int64_t nblks = (want_size + m_block_size - 1) / m_block_size; | ||
|
@@ -136,6 +154,7 @@ int BitMapAllocator::release( | |
|
||
uint64_t BitMapAllocator::get_free() | ||
{ | ||
assert(m_bit_alloc->size() >= m_bit_alloc->get_used_blocks()); | ||
return (( | ||
m_bit_alloc->size() - m_bit_alloc->get_used_blocks()) * | ||
m_block_size); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good