Skip to content
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.

Sign up for GitHub

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

Jump to bottom

os/bluestore: remove cmake warning from extent alloc functions #10492

Merged
merged 1 commit into from Aug 2, 2016
Merged

os/bluestore: remove cmake warning from extent alloc functions #10492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/os/bluestore/Allocator.h
View file
Open in desktop
Expand Up @@ -43,9 +43,8 @@ class Allocator {
uint64_t max_alloc_size, int64_t hint,
std::vector<AllocExtent> *extents, int *count) = 0;

virtual int alloc_extents(uint64_t want_size, uint64_t alloc_unit,
int64_t hint, std::vector<AllocExtent> *extents,
int *count) {
int alloc_extents(uint64_t want_size, uint64_t alloc_unit,
int64_t hint, std::vector<AllocExtent> *extents, int *count) {
return alloc_extents(want_size, alloc_unit, want_size, hint, extents, count);
}

Expand Down
5 changes: 1 addition & 4 deletions src/os/bluestore/BitAllocator.h
View file
Open in desktop
Expand Up @@ -194,10 +194,6 @@ class BitMapArea {
static int get_level(int64_t total_blocks);
static int64_t get_level_factor(int level);
virtual bool is_allocated(int64_t start_block, int64_t num_blocks) = 0;
virtual bool is_allocated(ExtentList *blocks, int64_t num_blocks, int blk_off) {
debug_assert(0);
return true;
}
virtual bool is_exhausted() = 0;
virtual bool child_check_n_lock(BitMapArea *child, int64_t required) {
debug_assert(0);
Expand Down Expand Up @@ -364,6 +360,7 @@ class BitMapZone: public BitMapArea{
}

int64_t alloc_blocks(int64_t num_blocks, int64_t *start_block);
using BitMapArea::alloc_blocks_dis;
Copy link
Contributor

@ifed01 ifed01 Jul 29, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have two remarks regarding alloc_blocks_dis method(s):
The first one - I suggest to avoid overloading virtuals at all. Hence one can rename alloc_blocks_dis( bool wait...) to something like alloc_blocks_dis_wait..
The second - Probably I missed something but I'm unable to find any use of this bool wait parameter in alloc_blocks[_dis[_int]] methods. I can see they are passed between these methods but don't see real wait var handling. Could you please point out that to me. And if my finding is correct probably one can simplify these methods and get rid of some of them...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait flag is not used currently as it is for a mode where we want to search for blocks without jumping a zone if another thread is already allocating in that zone. So there are three modes a. ) where we keep searching and jump zones that are busy, b.) wait for the busy zone on the way to search, but still multiple threads can be active. c.) everything goes serial. I am sure this will be useful when we implement hint or at least we should not remove it till we are more clear on its usage.

int64_t alloc_blocks_dis(int64_t num_blocks,
int64_t blk_off, ExtentList *block_list);
void set_blocks_used(int64_t start_block, int64_t num_blocks);
Expand Down