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.

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: Hint based allocation in bitmap Allocator #10978

Merged
merged 3 commits into from Sep 7, 2016

Conversation

chhabaramesh
Copy link
Contributor

This pull request has following changes:

  1. Hint based allocation in BitMapAllocator.
  2. Unit Test cases related to that.
  3. Cleanup of wrap argument that is no more in use.

@chhabaramesh
Copy link
Contributor Author

@liewegas , Please review this.

@@ -187,6 +187,49 @@ TEST_P(AllocTest, test_alloc_failure)
}
}

TEST_P(AllocTest, test_alloc_hint_bmap)
{
if (GetParam() == std::string("stupid")) {
Copy link
Member

Choose a reason for hiding this comment

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

!= std::string("bitmap") ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The stupid allocator was not passing all new hint test cases. So added this to bypass it for stupid allocator.

@liewegas
Copy link
Member

liewegas commented Sep 6, 2016

@chhabaramesh I haven't been following bitmap allocator internals.. is there someone else that can review?

@chhabaramesh
Copy link
Contributor Author

@liewegas , I can explain you it to you in short call ? Or Maybe @ifed01 or Manavalan can review?

But this change is quite simple, just makes sure that search of bit start from a given hint.

@liewegas
Copy link
Member

liewegas commented Sep 6, 2016

I see where the hint is used, but I don't see where the search loops around to the beginning if it doesn't find anything > hint (and, ideally, only searches back up to the start point).

@chhabaramesh
Copy link
Contributor Author

chhabaramesh commented Sep 6, 2016

@liewegas The iterator BmapEntityListIter takes a wrap argument and that is true only for children of root nodes.

1555 int64_t BitAllocator::alloc_blocks_dis_int(bool wait, int64_t num_blocks,
1556            int64_t hint, int64_t area_blk_off, ExtentList *block_list)
1557 {
1558   return alloc_blocks_dis_int_work(wait, true, num_blocks, hint,
1559                      area_blk_off, block_list);
1560 }

At all other levels it should be false. So it will start at a child at root level and wrap around and end at same child.

I think there is a type bug at:

 959 int64_t BitMapAreaIN::alloc_blocks_dis_int(bool wait, int64_t num_blocks,
 960            int64_t hint, int64_t area_blk_off, ExtentList *block_list)
 961 {
 962   return alloc_blocks_dis_int_work(wait, true, num_blocks, hint,
 963                      area_blk_off, block_list);
 964 }

The wrap ( second ) argument must be false here :(.

@chhabaramesh chhabaramesh force-pushed the extent_alloc branch 2 times, most recently from b094f61 to ab505af Compare September 6, 2016 16:41
Signed-off-by: Ramesh Chander <Ramesh.Chander@sandisk.com>
Signed-off-by: Ramesh Chander <Ramesh.Chander@sandisk.com>
Signed-off-by: Ramesh Chander <Ramesh.Chander@sandisk.com>
@chhabaramesh
Copy link
Contributor Author

@liewegas , Fixed the typo bug mentioned above and updated. Please check again.

@liewegas liewegas merged commit f8abb62 into ceph:master Sep 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants