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 <loic@dachary.org>

diff --git a/src/erasure-code/ErasureCode.cc b/src/erasure-code/ErasureCode.cc
index 96a6b39c7d3830e699dd74061afc4d6536f57f22..30dc186b594a7bdea0e1190ac55e8e2c800b4d34 100644 (file)
--- 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 c572cdc59eabd8e2b52e468ac04c2e7ce78ce570..2c972610fc7a82d7b57736f9ce0bca70899a36ec 100644 (file)
--- 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<int> 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<int, bufferlist> 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<const char*> args;