Project

General

Profile

Bug #6614

unittest_bufferlist failed

Added by huang jun over 10 years ago. Updated over 10 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
common
Target version:
% Done:

0%

Source:
other
Tags:
Backport:
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Pull request ID:
Crash signature (v1):
Crash signature (v2):

Description

work in progress
Run "make check", it reports bufferlist test failed.

test/bufferlist.cc:978: Failure
Value of: bl.is_page_aligned()
  Actual: true
Expected: false
[  FAILED  ] BufferList.is_page_aligned (0 ms)

logfile.txt View (214 KB) huang jun, 10/22/2013 11:22 PM

Associated revisions

Revision 66a9fbe2 (diff)
Added by Loïc Dachary over 10 years ago

common: rebuild_page_aligned sometimes rebuilds unaligned

rebuild_page_aligned relies on rebuild to create memory that is aligned
according to list::is_page_aligned(). However, when the bufferlist only
contains a single ptr and that its size is not list::is_n_page_size(),
rebuild will not create the expected alligned bufferlist.

The allocation of the ptr is moved out of rebuild which is now given the
ptr as an argument. The rebuild_page_aligned function always require an
aligned ptr with buffer::create_page_aligned(_len) for consistency.

The test

bufferlist bl;
bufferptr ptr(buffer::create_page_aligned(2));
ptr.set_offset(1);
ptr.set_length(1);
bl.append(ptr);
EXPECT_FALSE(bl.is_page_aligned());
bl.rebuild_page_aligned();
EXPECT_FALSE(bl.is_page_aligned());

demonstrated the problem. It was assumed to be a feature but should have
been identified as a bug. The last ligne is replaced with

EXPECT_TRUE(bl.is_page_aligned());

Most tests related to is_page_aligned() wrongfully assumed that

bufferptr ptr(2);

is never page aligned. Most of the time it is not but sometime it is
when the pointer address is by chance on a CEPH_PAGE_SIZE boundary,
which triggered #6614. Non aligned ptr are created as follows instead:

bufferptr ptr(buffer::create_page_aligned(2));
ptr.set_offset(1);
ptr.set_length(1);

http://tracker.ceph.com/issues/6614 fixes: #6614

Signed-off-by: Loic Dachary <>

Revision 1d7a228d (diff)
Added by Loïc Dachary over 9 years ago

common: rebuild_page_aligned sometimes rebuilds unaligned

rebuild_page_aligned relies on rebuild to create memory that is aligned
according to list::is_page_aligned(). However, when the bufferlist only
contains a single ptr and that its size is not list::is_n_page_size(),
rebuild will not create the expected alligned bufferlist.

The allocation of the ptr is moved out of rebuild which is now given the
ptr as an argument. The rebuild_page_aligned function always require an
aligned ptr with buffer::create_page_aligned(_len) for consistency.

The test

bufferlist bl;
bufferptr ptr(buffer::create_page_aligned(2));
ptr.set_offset(1);
ptr.set_length(1);
bl.append(ptr);
EXPECT_FALSE(bl.is_page_aligned());
bl.rebuild_page_aligned();
EXPECT_FALSE(bl.is_page_aligned());

demonstrated the problem. It was assumed to be a feature but should have
been identified as a bug. The last ligne is replaced with

EXPECT_TRUE(bl.is_page_aligned());

Most tests related to is_page_aligned() wrongfully assumed that

bufferptr ptr(2);

is never page aligned. Most of the time it is not but sometime it is
when the pointer address is by chance on a CEPH_PAGE_SIZE boundary,
which triggered #6614. Non aligned ptr are created as follows instead:

bufferptr ptr(buffer::create_page_aligned(2));
ptr.set_offset(1);
ptr.set_length(1);

http://tracker.ceph.com/issues/6614 fixes: #6614

Signed-off-by: Loic Dachary <>
(cherry picked from commit 66a9fbe2c7ba59b7cd034c17865adce3432cd2cb)

History

#1 Updated by Zheng Yan over 10 years ago

  • Project changed from CephFS to Ceph

#2 Updated by Greg Farnum over 10 years ago

Where are you running this, and where are you getting the source (I can't find that commit)? Our gitbuilders (http://ceph.com/gitbuilder.cgi) seem to be doing this test successfully.

#3 Updated by Josh Durgin over 10 years ago

We talked on irc last night, this was on CentOS 6.4. The deb/rpm gitbuilders aren't running make check - we should add that so we catch this kind of thing sooner.

#4 Updated by Loïc Dachary over 10 years ago

  • Description updated (diff)
  • Category set to common
  • Status changed from New to In Progress
  • Assignee set to Loïc Dachary
  • Target version set to v0.72 Emperor

The error originaly reported is not an actual error, it is the output of an assert that was expected to happen as part of the test. To look for errors check for [ FAILED ] instead. I will replace this error message with the one that matters.

below is the assert info.

common/buffer.cc: In function 'void ceph::buffer::list::zero(unsigned int, unsigned int)' thread 7f3a034f3720 time 2013-10-23 14:13:59
.522966
common/buffer.cc: 778: FAILED assert(o+l <= len)
ceph version 0.71-250-g4c8be79 (4c8be795d6bf3a52e9ac242f9263364ec59a9efd)
1: ./unittest_bufferlist() [0x489426]
2: (BufferList_zero_Test::TestBody()+0xf9f) [0x45b99f]
3: (testing::Test::Run()+0xaa) [0x47d05a]
4: (testing::internal::TestInfoImpl::Run()+0x100) [0x47d160]
5: (testing::TestCase::Run()+0xbd) [0x47d22d]
6: (testing::internal::UnitTestImpl::RunAllTests()+0x214) [0x47d494]
7: (main()+0x3a) [0x485bea]
8: (_libc_start_main()+0xfd) [0x3a5981ecdd]
9: ./unittest_bufferlist() [0x433919]
NOTE: a copy of the executable, or `objdump -rdS <executable>` is needed to interpret this.

#5 Updated by Loïc Dachary over 10 years ago

Compiled against 289b7903407ce1b34f1afe9e0c769093c14d0ba9 on centos-6.4 the test completes successfully. But the test are wrongfully assuming an allocated buffer will be page aligned. It's probably the case most of the time but not always, which leads to a rare failure. Fixing this.

#6 Updated by Loïc Dachary over 10 years ago

  • Description updated (diff)

#7 Updated by huang jun over 10 years ago

Test ”make check“ succeed now.

#8 Updated by Loïc Dachary over 10 years ago

  • Status changed from In Progress to Resolved

Good to know, thanks for the feedback :-)

Also available in: Atom PDF