os/bluestore: remove cmake warning from extent alloc functions #10492
Conversation
|
LGTM |
|
@xiexingguo thanks for quick review :) |
| @@ -322,6 +322,8 @@ class BitMapZone: public BitMapArea{ | |||
| static int64_t total_blocks; | |||
| static void incr_count() { count++;} | |||
| static int64_t get_total_blocks() {return total_blocks;} | |||
|
|
|||
| using BitMapArea::is_allocated; | |||
There was a problem hiding this comment.
IMHO one can simply remove
virtual bool BitMapArea::is_allocated(ExtentList *blocks, int64_t num_blocks, int blk_off)
to fix the warning. Looks like it's not used..
| @@ -364,6 +366,7 @@ class BitMapZone: public BitMapArea{ | |||
| } | |||
|
|
|||
| int64_t alloc_blocks(int64_t num_blocks, int64_t *start_block); | |||
| using BitMapArea::alloc_blocks_dis; | |||
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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.
|
@chhabaramesh thanks for looking at this. i recently created a tracker issue for it at http://tracker.ceph.com/issues/16766. if this makes those warnings go away, can you please add this to your commit message? |
|
@cbodley sure, I will add tracker number in commit message. |
chhabaramesh
force-pushed
the
master
branch
2 times, most recently
from
5ea4f1a
to
edcf257
Compare
|
@ifed01 please merge this. |
|
@ifed01 anything pending on my side to merge this? |
|
@chhabaramesh please remove additional 'using BitMapArea::is_allocated;' |
chhabaramesh
force-pushed
the
master
branch
2 times, most recently
from
350aa91
to
c457603
Compare
tracker link: http://tracker.ceph.com/issues/16766 Signed-off-by: Ramesh Chander <Ramesh.Chander@sandisk.com>
Removing cmake related warning from alloc extents functions
Signed-off-by: Ramesh Chander Ramesh.Chander@sandisk.com