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

mempool: improve dump; fix buffer accounting bugs #15403

Merged
merged 2 commits into from Jun 2, 2017

Conversation

Projects
None yet
3 participants
@liewegas
Member

liewegas commented Jun 1, 2017

No description provided.

liewegas added some commits May 31, 2017

mempool: make dump more concise
Signed-off-by: Sage Weil <sage@redhat.com>
buffer: fix some append_buffer vs mempool assignment; add tests
Signed-off-by: Sage Weil <sage@redhat.com>
@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 1, 2017

retest this please. see #15406

@liewegas

This comment has been minimized.

Member

liewegas commented Jun 1, 2017

retest this please

@@ -1768,7 +1768,7 @@ static std::atomic_flag buffer_debug_lock = ATOMIC_FLAG_INIT;
{
if (append_buffer.unused_tail_length() < prealloc) {
append_buffer = buffer::create(prealloc);
if (_mempool) {
if (_mempool >= 0) {

This comment has been minimized.

@tchaikov

tchaikov Jun 1, 2017

Contributor

yeah, it's -1 by default. and the mem pool index starts at 0. they are enum whose first element is not assigned with a number. maybe we should make this explicit. like

enum pool_index_t {
  P(none),
  DEFINE_MEMORY_POOLS_HELPER(P)
  num_pools        // Must be last.
};

and use mempool_none as the default value for _mempool? so we can compare _mempool with mempool_none instead of the (sort of) magic number?

// move existing bl
bl.reassign_to_mempool(mempool::mempool_osd);
ASSERT_EQ(items_before, mempool::buffer_anon::allocated_items());
ASSERT_EQ(bytes_before, mempool::buffer_anon::allocated_bytes());

This comment has been minimized.

@tchaikov

tchaikov Jun 1, 2017

Contributor

nit, could check if mempool::mempool_osd gets the item and bytes number.

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Jun 2, 2017

@liewegas
Something in this PR fixes a crash I'm having (on FreeBSD) since the previous changes to the buffer/mempool code, likely #15352.
So I'm all in favour for this PR. :)

@liewegas liewegas merged commit a35cd1c into ceph:master Jun 2, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@liewegas liewegas deleted the liewegas:wip-mempool-dump branch Jun 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment