Skip to content

os/bluestore: make assert conditional with macro for allocator#11014

Merged
liewegas merged 2 commits intoceph:masterfrom
chhabaramesh:extent_alloc
Sep 14, 2016
Merged

os/bluestore: make assert conditional with macro for allocator#11014
liewegas merged 2 commits intoceph:masterfrom
chhabaramesh:extent_alloc

Conversation

@chhabaramesh
Copy link
Contributor

Change makes the is_allocated related code conditional with a macro.
Rest of code split with different assert so that basic check are still performed.

Signed-off-by: Ramesh Chander Ramesh.Chander@sandisk.com


static inline void noop(bool var)
{
}
Copy link
Member

Choose a reason for hiding this comment

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

This might not get optimized out. Better to do a simple

#define alloc_dbg_assert(x)

Copy link
Member

@liewegas liewegas Sep 8, 2016

Choose a reason for hiding this comment

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

Actually system assert.h does

#if defined __cplusplus && __GNUC_PREREQ (2,95)
# define __ASSERT_VOID_CAST static_cast<void>
#else
# define __ASSERT_VOID_CAST (void)
#endif

# define assert(expr)       (__ASSERT_VOID_CAST (0))

which is probably a bit better.

Copy link
Contributor Author

@chhabaramesh chhabaramesh Sep 8, 2016

Choose a reason for hiding this comment

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

@liewegas , changed dummy function to what you suggested from assert.h. Please have a look now. I also wanted to define this macro BIT_ALLOCATOR_DEBUG for test case compilation but could not make it work :(.

Signed-off-by: Ramesh Chander <Ramesh.Chander@sandisk.com>
if (min_min_alloc_size > 0) {
derr << "warning: bluestore_min_alloc_size value decreased from "
<< min_min_alloc_size << " to " << new_val << "."
<< " Decreased value could have perofrmanc impact." << dendl;
Copy link
Contributor

Choose a reason for hiding this comment

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

typo

@chhabaramesh chhabaramesh force-pushed the extent_alloc branch 2 times, most recently from 2d652db to 284f4d7 Compare September 9, 2016 15:24
@chhabaramesh
Copy link
Contributor Author

@ifed01 @liewegas , fixed the type on message. Please have a look now.

// Min min_alloc_size
{
bufferlist bl;
min_min_alloc_size = 0;
Copy link
Member

Choose a reason for hiding this comment

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

= min_alloc_size

for base case (after mkfs). or when someone upgrades.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is always set after call to _save_min_min_alloc_size. Taking value 0 as argument to this function identifies the condition where we always need to save the min_min_alloc_size.
The min_alloc_size is set before call to this function. So the cases:

  1. min_min_alloc_size : 0, then we need to save current min_alloc_size as min_min_alloc_size. That handles upgrade and mkfs cases where there is no older value.
  2. min_min_alloc_size < min_alloc_size, we need to put message and do nothing in this case.
  3. min_min_alloc_size > min_alloc_size, we need to put message and save new value.

In all cases at end of this function, min_min_alloc_size == min_alloc_size.

I think it handles all cases ?

@chhabaramesh chhabaramesh force-pushed the extent_alloc branch 2 times, most recently from 6f0b0aa to f82b320 Compare September 13, 2016 06:46
Signed-off-by: Ramesh Chander <Ramesh.Chander@sandisk.com>
@chhabaramesh
Copy link
Contributor Author

@liewegas , a gentle reminder to have a look now and merge if looks ok.

@liewegas liewegas merged commit d3dac2a into ceph:master Sep 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants