common: add an aligned buffer with less alignment than a page

SIMD optimized erasure code computation needs aligned memory. Buffers
aligned to a page boundary are not needed though. The buffers used
for the erasure code computation are typical smaller than a page.

The typical alignment requirements SIMD operations are 16 bytes for
SSE2 and NEON and 32 bytes for AVX/AVX2.

Add new prototypes with an align argument, similar to the one enforcing
page alignment. The implementation is exactly the same, except for the
align parameter. The page alignment method are then implemented as calls
to the more generic methods.

The align parameter is an unsigned (same type as CEPH_PAGE_SIZE). The
CEPH_PAGE_MASK value ( ~(CEPH_PAGE_SIZE - 1) ) was only used as
~CEPH_PAGE_MASK, i.e. equivalent of (CEPH_PAGE_SIZE - 1) once the double
~~ is reduced. These occurrence are replaced with (align - 1). The type
of CEPH_PAGE_MASK is an unsigned long which probably because it was
~(CEPH_PAGE_SIZE). When using (align - 1) as a mask for both
CEPH_PAGE_SIZE and SIMD alignment there is no need to use an unsigned
long because there is no risk of overflowing the unsigned value.

The CYGWIN specific code is also modified but not tested.

Unit tests are added for the new methods.

Signed-off-by: Janne Grunau <j@jannau.net>
Signed-off-by: Loic Dachary <loic-201408@dachary.org>

diff --git a/src/common/buffer.cc b/src/common/buffer.cc
index fa45c61c2e2ddd7e984e147b0f066dd7f0dffb1d..9ee2bfedf34113e7e819b48ff7d63b3e127c98dd 100644 (file)
--- a/src/common/buffer.cc
+++ b/src/common/buffer.cc
@@ -230,20 +230,23 @@ static simple_spinlock_t buffer_debug_lock = SIMPLE_SPINLOCK_INITIALIZER;
   };
 
   class buffer::raw_posix_aligned : public buffer::raw {
+    unsigned align;
   public:
-    raw_posix_aligned(unsigned l) : raw(l) {
+    raw_posix_aligned(unsigned l, unsigned _align) : raw(l) {
+      align = _align;
+      assert((align >= sizeof(void *)) && (align & (align - 1)) == 0);
 #ifdef DARWIN
       data = (char *) valloc (len);
 #else
       data = 0;
-      int r = ::posix_memalign((void**)(void*)&data, CEPH_PAGE_SIZE, len);
+      int r = ::posix_memalign((void**)(void*)&data, align, len);
       if (r)
        throw bad_alloc();
 #endif /* DARWIN */
       if (!data)
        throw bad_alloc();
       inc_total_alloc(len);
-      bdout << "raw_posix_aligned " << this << " alloc " << (void *)data << " " << l << " " << buffer::get_total_alloc() << bendl;
+      bdout << "raw_posix_aligned " << this << " alloc " << (void *)data << " l=" << l << ", align=" << align << " total_alloc=" << buffer::get_total_alloc() << bendl;
     }
     ~raw_posix_aligned() {
       ::free((void*)data);
@@ -251,34 +254,36 @@ static simple_spinlock_t buffer_debug_lock = SIMPLE_SPINLOCK_INITIALIZER;
       bdout << "raw_posix_aligned " << this << " free " << (void *)data << " " << buffer::get_total_alloc() << bendl;
     }
     raw* clone_empty() {
-      return new raw_posix_aligned(len);
+      return new raw_posix_aligned(len, align);
     }
   };
 #endif
 
 #ifdef __CYGWIN__
   class buffer::raw_hack_aligned : public buffer::raw {
+    unsigned align;
     char *realdata;
   public:
-    raw_hack_aligned(unsigned l) : raw(l) {
-      realdata = new char[len+CEPH_PAGE_SIZE-1];
-      unsigned off = ((unsigned)realdata) & ~CEPH_PAGE_MASK;
+    raw_hack_aligned(unsigned l, unsigned _align) : raw(l) {
+      align = _align;
+      realdata = new char[len+align-1];
+      unsigned off = ((unsigned)realdata) & (align-1);
       if (off)
-       data = realdata + CEPH_PAGE_SIZE - off;
+       data = realdata + align - off;
       else
        data = realdata;
-      inc_total_alloc(len+CEPH_PAGE_SIZE-1);
+      inc_total_alloc(len+align-1);
       //cout << "hack aligned " << (unsigned)data
       //<< " in raw " << (unsigned)realdata
       //<< " off " << off << std::endl;
-      assert(((unsigned)data & (CEPH_PAGE_SIZE-1)) == 0);
+      assert(((unsigned)data & (align-1)) == 0);
     }
     ~raw_hack_aligned() {
       delete[] realdata;
-      dec_total_alloc(len+CEPH_PAGE_SIZE-1);
+      dec_total_alloc(len+align-1);
     }
     raw* clone_empty() {
-      return new raw_hack_aligned(len);
+      return new raw_hack_aligned(len, align);
     }
   };
 #endif
@@ -516,14 +521,17 @@ static simple_spinlock_t buffer_debug_lock = SIMPLE_SPINLOCK_INITIALIZER;
   buffer::raw* buffer::create_static(unsigned len, char *buf) {
     return new raw_static(buf, len);
   }
-  buffer::raw* buffer::create_page_aligned(unsigned len) {
+  buffer::raw* buffer::create_aligned(unsigned len, unsigned align) {
 #ifndef __CYGWIN__
     //return new raw_mmap_pages(len);
-    return new raw_posix_aligned(len);
+    return new raw_posix_aligned(len, align);
 #else
-    return new raw_hack_aligned(len);
+    return new raw_hack_aligned(len, align);
 #endif
   }
+  buffer::raw* buffer::create_page_aligned(unsigned len) {
+    return create_aligned(len, CEPH_PAGE_SIZE);
+  }
 
   buffer::raw* buffer::create_zero_copy(unsigned len, int fd, int64_t *offset) {
 #ifdef CEPH_HAVE_SPLICE
@@ -1009,22 +1017,22 @@ static simple_spinlock_t buffer_debug_lock = SIMPLE_SPINLOCK_INITIALIZER;
     return true;
   }
 
-  bool buffer::list::is_page_aligned() const
+  bool buffer::list::is_aligned(unsigned align) const
   {
     for (std::list<ptr>::const_iterator it = _buffers.begin();
         it != _buffers.end();
         ++it) 
-      if (!it->is_page_aligned())
+      if (!it->is_aligned(align))
        return false;
     return true;
   }
 
-  bool buffer::list::is_n_page_sized() const
+  bool buffer::list::is_n_align_sized(unsigned align) const
   {
     for (std::list<ptr>::const_iterator it = _buffers.begin();
         it != _buffers.end();
         ++it) 
-      if (!it->is_n_page_sized())
+      if (!it->is_n_align_sized(align))
        return false;
     return true;
   }
@@ -1074,6 +1082,16 @@ static simple_spinlock_t buffer_debug_lock = SIMPLE_SPINLOCK_INITIALIZER;
     return &(*_buffers.begin()) == &(*_buffers.rbegin());
   }
 
+  bool buffer::list::is_n_page_sized() const
+  {
+    return is_n_align_sized(CEPH_PAGE_SIZE);
+  }
+
+  bool buffer::list::is_page_aligned() const
+  {
+    return is_aligned(CEPH_PAGE_SIZE);
+  }
+
   void buffer::list::rebuild()
   {
     ptr nb;
@@ -1097,16 +1115,16 @@ static simple_spinlock_t buffer_debug_lock = SIMPLE_SPINLOCK_INITIALIZER;
     _buffers.push_back(nb);
   }
 
-void buffer::list::rebuild_page_aligned()
+void buffer::list::rebuild_aligned(unsigned align)
 {
   std::list<ptr>::iterator p = _buffers.begin();
   while (p != _buffers.end()) {
-    // keep anything that's already page sized+aligned
-    if (p->is_page_aligned() && p->is_n_page_sized()) {
+    // keep anything that's already align and sized aligned
+    if (p->is_aligned(align) && p->is_n_align_sized(align)) {
       /*cout << " segment " << (void*)p->c_str()
-            << " offset " << ((unsigned long)p->c_str() & ~CEPH_PAGE_MASK)
+            << " offset " << ((unsigned long)p->c_str() & (align - 1))
             << " length " << p->length()
-            << " " << (p->length() & ~CEPH_PAGE_MASK) << " ok" << std::endl;
+            << " " << (p->length() & (align - 1)) << " ok" << std::endl;
       */
       ++p;
       continue;
@@ -1117,26 +1135,31 @@ void buffer::list::rebuild_page_aligned()
     unsigned offset = 0;
     do {
       /*cout << " segment " << (void*)p->c_str()
-            << " offset " << ((unsigned long)p->c_str() & ~CEPH_PAGE_MASK)
-            << " length " << p->length() << " " << (p->length() & ~CEPH_PAGE_MASK)
-            << " overall offset " << offset << " " << (offset & ~CEPH_PAGE_MASK)
+             << " offset " << ((unsigned long)p->c_str() & (align - 1))
+             << " length " << p->length() << " " << (p->length() & (align - 1))
+             << " overall offset " << offset << " " << (offset & (align - 1))
             << " not ok" << std::endl;
       */
       offset += p->length();
       unaligned.push_back(*p);
       _buffers.erase(p++);
     } while (p != _buffers.end() &&
-            (!p->is_page_aligned() ||
-             !p->is_n_page_sized() ||
-             (offset & ~CEPH_PAGE_MASK)));
-    if (!(unaligned.is_contiguous() && unaligned._buffers.front().is_page_aligned())) {
-      ptr nb(buffer::create_page_aligned(unaligned._len));
+            (!p->is_aligned(align) ||
+             !p->is_n_align_sized(align) ||
+             (offset & (align-1))));
+    if (!(unaligned.is_contiguous() && unaligned._buffers.front().is_aligned(align))) {
+      ptr nb(buffer::create_aligned(unaligned._len, align));
       unaligned.rebuild(nb);
     }
     _buffers.insert(p, unaligned._buffers.front());
   }
 }
 
+void buffer::list::rebuild_page_aligned()
+{
+  rebuild_aligned(CEPH_PAGE_SIZE);
+}
+
   // sort-of-like-assignment-op
   void buffer::list::claim(list& bl)
   {

diff --git a/src/include/buffer.h b/src/include/buffer.h
index e5c1b5053584408b14634f69ca84646b07cf7a4d..3cd0a7a19a0727447bd6f9d7bf3de16a86a31674 100644 (file)
--- a/src/include/buffer.h
+++ b/src/include/buffer.h
@@ -144,6 +144,7 @@ public:
   static raw* create_malloc(unsigned len);
   static raw* claim_malloc(unsigned len, char *buf);
   static raw* create_static(unsigned len, char *buf);
+  static raw* create_aligned(unsigned len, unsigned align);
   static raw* create_page_aligned(unsigned len);
   static raw* create_zero_copy(unsigned len, int fd, int64_t *offset);
 
@@ -177,8 +178,15 @@ public:
     bool at_buffer_head() const { return _off == 0; }
     bool at_buffer_tail() const;
 
-    bool is_page_aligned() const { return ((long)c_str() & ~CEPH_PAGE_MASK) == 0; }
-    bool is_n_page_sized() const { return (length() & ~CEPH_PAGE_MASK) == 0; }
+    bool is_aligned(unsigned align) const {
+      return ((long)c_str() & (align-1)) == 0;
+    }
+    bool is_page_aligned() const { return is_aligned(CEPH_PAGE_SIZE); }
+    bool is_n_align_sized(unsigned align) const
+    {
+      return (length() & (align-1)) == 0;
+    }
+    bool is_n_page_sized() const { return is_n_align_sized(CEPH_PAGE_SIZE); }
 
     // accessors
     raw *get_raw() const { return _raw; }
@@ -344,7 +352,9 @@ public:
     bool contents_equal(buffer::list& other);
 
     bool can_zero_copy() const;
+    bool is_aligned(unsigned align) const;
     bool is_page_aligned() const;
+    bool is_n_align_sized(unsigned align) const;
     bool is_n_page_sized() const;
 
     bool is_zero() const;
@@ -382,6 +392,7 @@ public:
     bool is_contiguous();
     void rebuild();
     void rebuild(ptr& nb);
+    void rebuild_aligned(unsigned align);
     void rebuild_page_aligned();
 
     // sort-of-like-assignment-op

diff --git a/src/test/bufferlist.cc b/src/test/bufferlist.cc
index 3b379afded58bc8a7ce5b24dec4577e7f1e084ab..da4d33ff2b4d91dd15ad4a92eb3d99a8649035d5 100644 (file)
--- a/src/test/bufferlist.cc
+++ b/src/test/bufferlist.cc
@@ -1122,6 +1122,52 @@ TEST(BufferList, contents_equal) {
   ASSERT_FALSE(bl1.contents_equal(bl3)); // same length different content
 }
 
+TEST(BufferList, is_aligned) {
+  const int SIMD_ALIGN = 32;
+  {
+    bufferlist bl;
+    EXPECT_TRUE(bl.is_aligned(SIMD_ALIGN));
+  }
+  {
+    bufferlist bl;
+    bufferptr ptr(buffer::create_aligned(2, SIMD_ALIGN));
+    ptr.set_offset(1);
+    ptr.set_length(1);
+    bl.append(ptr);
+    EXPECT_FALSE(bl.is_aligned(SIMD_ALIGN));
+    bl.rebuild_aligned(SIMD_ALIGN);
+    EXPECT_TRUE(bl.is_aligned(SIMD_ALIGN));
+  }
+  {
+    bufferlist bl;
+    bufferptr ptr(buffer::create_aligned(SIMD_ALIGN + 1, SIMD_ALIGN));
+    ptr.set_offset(1);
+    ptr.set_length(SIMD_ALIGN);
+    bl.append(ptr);
+    EXPECT_FALSE(bl.is_aligned(SIMD_ALIGN));
+    bl.rebuild_aligned(SIMD_ALIGN);
+    EXPECT_TRUE(bl.is_aligned(SIMD_ALIGN));
+  }
+}
+
+TEST(BufferList, is_n_align_sized) {
+  const int SIMD_ALIGN = 32;
+  {
+    bufferlist bl;
+    EXPECT_TRUE(bl.is_n_align_sized(SIMD_ALIGN));
+  }
+  {
+    bufferlist bl;
+    bl.append_zero(1);
+    EXPECT_FALSE(bl.is_n_align_sized(SIMD_ALIGN));
+  }
+  {
+    bufferlist bl;
+    bl.append_zero(SIMD_ALIGN);
+    EXPECT_TRUE(bl.is_n_align_sized(SIMD_ALIGN));
+  }
+}
+
 TEST(BufferList, is_page_aligned) {
   {
     bufferlist bl;