From 4e955f41297283798236c505c3d21bdcabb5caa0 Mon Sep 17 00:00:00 2001 From: Loic Dachary Date: Tue, 2 Dec 2014 01:07:34 +0100 Subject: [PATCH] erasure-code: enforce chunk size alignment Let say the ErasureCode::encode function is given a 4096 bytes bufferlist made of a 1249 bytes bufferptr followed by a 2847 bytes bufferptr, both properly starting on SIMD_ALIGN address. As a result the second 2048 had to be reallocated when bufferlist::substr_of gets the second 2048 buffer, the address starts at 799 bytes after the beginning of the 2847 buffer ptr and is not SIMD_ALIGN'ed. The ErasureCode::encode must enforce a size alignment based on the chunk size in addition to the memory alignment required by SIMD operations, using the bufferlist::rebuild_aligned_size_and_memory function instead of bufferlist::rebuild_aligned. http://tracker.ceph.com/issues/10211 Fixes: #10211 Signed-off-by: Loic Dachary --- src/erasure-code/ErasureCode.cc | 11 ++------ src/test/erasure-code/TestErasureCode.cc | 36 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/src/erasure-code/ErasureCode.cc b/src/erasure-code/ErasureCode.cc index 96a6b39c7d38..30dc186b594a 100644 --- a/src/erasure-code/ErasureCode.cc +++ b/src/erasure-code/ErasureCode.cc @@ -66,21 +66,14 @@ int ErasureCode::encode_prepare(const bufferlist &raw, unsigned int k = get_data_chunk_count(); unsigned int m = get_chunk_count() - k; unsigned blocksize = get_chunk_size(raw.length()); - unsigned pad_len = blocksize * k - raw.length(); unsigned padded_chunks = k - raw.length() / blocksize; bufferlist prepared = raw; - if (!prepared.is_aligned(SIMD_ALIGN)) { - // splice padded chunks off to make the rebuild faster - if (padded_chunks) - prepared.splice((k - padded_chunks) * blocksize, - padded_chunks * blocksize - pad_len); - prepared.rebuild_aligned(SIMD_ALIGN); - } - for (unsigned int i = 0; i < k - padded_chunks; i++) { bufferlist &chunk = encoded[chunk_index(i)]; chunk.substr_of(prepared, i * blocksize, blocksize); + chunk.rebuild_aligned_size_and_memory(blocksize, SIMD_ALIGN); + assert(chunk.is_contiguous()); } if (padded_chunks) { unsigned remainder = raw.length() - (k - padded_chunks) * blocksize; diff --git a/src/test/erasure-code/TestErasureCode.cc b/src/test/erasure-code/TestErasureCode.cc index c572cdc59eab..2c972610fc7a 100644 --- a/src/test/erasure-code/TestErasureCode.cc +++ b/src/test/erasure-code/TestErasureCode.cc @@ -111,6 +111,42 @@ TEST(ErasureCodeTest, encode_memory_align) ASSERT_NE(encoded[1][chunk_size / 2], 'X'); } +TEST(ErasureCodeTest, encode_misaligned_non_contiguous) +{ + int k = 3; + int m = 1; + unsigned chunk_size = ErasureCode::SIMD_ALIGN * 7; + ErasureCodeTest erasure_code(k, m, chunk_size); + + set want_to_encode; + for (unsigned int i = 0; i < erasure_code.get_chunk_count(); i++) + want_to_encode.insert(i); + string data(chunk_size, 'X'); + // create a non contiguous bufferlist where the frist and the second + // bufferptr are not size aligned although they are memory aligned + bufferlist in; + { + bufferptr ptr(buffer::create_aligned(data.length() - 1, ErasureCode::SIMD_ALIGN)); + in.append(ptr); + } + { + bufferptr ptr(buffer::create_aligned(data.length() + 1, ErasureCode::SIMD_ALIGN)); + in.append(ptr); + } + map encoded; + + ASSERT_FALSE(in.is_contiguous()); + ASSERT_TRUE(in.buffers().front().is_aligned(ErasureCode::SIMD_ALIGN)); + ASSERT_FALSE(in.buffers().front().is_n_align_sized(chunk_size)); + ASSERT_TRUE(in.buffers().back().is_aligned(ErasureCode::SIMD_ALIGN)); + ASSERT_FALSE(in.buffers().back().is_n_align_sized(chunk_size)); + ASSERT_EQ(0, erasure_code.encode(want_to_encode, in, &encoded)); + for (unsigned int i = 0; i < erasure_code.get_chunk_count(); i++) { + ASSERT_TRUE(encoded[i].is_aligned(ErasureCode::SIMD_ALIGN)); + ASSERT_TRUE(encoded[i].is_n_align_sized(chunk_size)); + } +} + int main(int argc, char **argv) { vector args; -- 2.47.3