Bug #6614
unittest_bufferlist failed
0%
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)
Associated revisions
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 <loic@dachary.org>
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 <loic@dachary.org>
(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 :-)