Merge pull request ceph#53130 from cyx1231st/wip-crimson-drop-mempool

mempool: avoid true sharing for the counters, with crimson

Reviewed-by: Radoslaw Zarzynski <[email protected]>
Reviewed-by: Chunmei Liu <[email protected]>
Reviewed-by: Matan Breizman <[email protected]>
Reviewed-by: Laura Flores <[email protected]>

Author: cyx1231st

src/common/mempool.cc

@@ -15,9 +15,12 @@
 #include "include/mempool.h"
 #include "include/demangle.h"
 
+#if defined(_GNU_SOURCE) && defined(WITH_SEASTAR) && !defined(WITH_ALIEN)
+#else
 // Thread local variables should save index, not &shard[index],
 // because shard[] is defined in the class
 static thread_local size_t thread_shard_index = mempool::num_shards;
+#endif
 
 // default to debug_mode off
 bool mempool::debug_mode = false;
@@ -95,9 +98,21 @@ size_t mempool::pool_t::allocated_items() const
 
 void mempool::pool_t::adjust_count(ssize_t items, ssize_t bytes)
 {
-  thread_shard_index = (thread_shard_index == num_shards) ? pick_a_shard_int() : thread_shard_index;
-  shard[thread_shard_index].items += items;
-  shard[thread_shard_index].bytes += bytes;
+#if defined(_GNU_SOURCE) && defined(WITH_SEASTAR) && !defined(WITH_ALIEN)
+  // the expected path: we alway pick the shard for a cpu core
+  // a thread is executing on.
+  const size_t shard_index = pick_a_shard_int();
+#else
+  // fallback for lack of sched_getcpu()
+  const size_t shard_index = []() {
+    if (thread_shard_index == num_shards) {
+      thread_shard_index = pick_a_shard_int();
+    }
+    return thread_shard_index;
+  }();
+#endif
+  shard[shard_index].items += items;
+  shard[shard_index].bytes += bytes;
 }
 
 void mempool::pool_t::get_stats(
@@ -113,8 +128,17 @@ void mempool::pool_t::get_stats(
     for (auto &p : type_map) {
       std::string n = ceph_demangle(p.second.type_name);
       stats_t &s = (*by_type)[n];
+#if defined(WITH_SEASTAR) && !defined(WITH_ALIEN)
+      s.bytes = 0;
+      s.items = 0;
+      for (size_t i = 0 ; i < num_shards; ++i) {
+        s.bytes += p.second.shards[i].items * p.second.item_size;
+        s.items += p.second.shards[i].items;
+      }
+#else
       s.bytes = p.second.items * p.second.item_size;
       s.items = p.second.items;
+#endif
     }
   }
 }

src/include/mempool.h

@@ -26,6 +26,10 @@
 #include <boost/container/flat_set.hpp>
 #include <boost/container/flat_map.hpp>
 
+#if defined(_GNU_SOURCE) && defined(WITH_SEASTAR) && !defined(WITH_ALIEN)
+#  include <sched.h>
+#endif
+
 #include "common/Formatter.h"
 #include "common/ceph_atomic.h"
 #include "include/ceph_assert.h"
@@ -201,6 +205,24 @@ enum {
   num_shards = 1 << num_shard_bits
 };
 
+static size_t pick_a_shard_int() {
+#if defined(_GNU_SOURCE) && defined(WITH_SEASTAR) && !defined(WITH_ALIEN)
+  // a thread local storage is actually just an approximation;
+  // what we truly want is a _cpu local storage_.
+  //
+  // on the architectures we care about sched_getcpu() is
+  // a syscall-handled-in-userspace (vdso!). it grabs the cpu
+  // id kernel exposes to a task on context switch.
+  return sched_getcpu() & ((1 << num_shard_bits) - 1);
+#else
+  // Dirt cheap, see:
+  //   https://fossies.org/dox/glibc-2.32/pthread__self_8c_source.html
+  size_t me = (size_t)pthread_self();
+  size_t i = (me >> CEPH_PAGE_SHIFT) & ((1 << num_shard_bits) - 1);
+  return i;
+#endif
+}
+
 //
 // Align shard to a cacheline.
 //
@@ -240,7 +262,18 @@ const char *get_pool_name(pool_index_t ix);
 struct type_t {
   const char *type_name;
   size_t item_size;
+#if defined(WITH_SEASTAR) && !defined(WITH_ALIEN)
+  struct type_shard_t {
+    ceph::atomic<ssize_t> items = {0}; // signed
+    char __padding[128 - sizeof(ceph::atomic<ssize_t>)];
+  } __attribute__ ((aligned (128)));
+  static_assert(sizeof(type_shard_t) == 128,
+                "type_shard_t should be cacheline-sized");
+  type_shard_t shards[num_shards];
+#else
+// XXX: consider dropping this case for classic with perf tests
   ceph::atomic<ssize_t> items = {0};  // signed
+#endif
 };
 
 struct type_info_hash {
@@ -255,6 +288,8 @@ class pool_t {
   mutable std::mutex lock;  // only used for types list
   std::unordered_map<const char *, type_t> type_map;
 
+  template<pool_index_t, typename T>
+  friend class pool_allocator;
 public:
   //
   // How much this pool consumes. O(<num_shards>)
@@ -264,19 +299,6 @@ class pool_t {
 
   void adjust_count(ssize_t items, ssize_t bytes);
 
-  static size_t pick_a_shard_int() {
-    // Dirt cheap, see:
-    //   https://fossies.org/dox/glibc-2.32/pthread__self_8c_source.html
-    size_t me = (size_t)pthread_self();
-    size_t i = (me >> CEPH_PAGE_SHIFT) & ((1 << num_shard_bits) - 1);
-    return i;
-  }
-
-  shard_t* pick_a_shard() {
-    size_t i = pick_a_shard_int();
-    return &shard[i];
-  }
-
   type_t *get_type(const std::type_info& ti, size_t size) {
     std::lock_guard<std::mutex> l(lock);
     auto p = type_map.find(ti.name());
@@ -339,34 +361,49 @@ class pool_allocator {
 
   T* allocate(size_t n, void *p = nullptr) {
     size_t total = sizeof(T) * n;
-    shard_t *shard = pool->pick_a_shard();
-    shard->bytes += total;
-    shard->items += n;
+    const auto shid = pick_a_shard_int();
+    auto& shard = pool->shard[shid];
+    shard.bytes += total;
+    shard.items += n;
     if (type) {
+#if defined(WITH_SEASTAR) && !defined(WITH_ALIEN)
+      type->shards[shid].items += n;
+#else
       type->items += n;
+#endif
     }
     T* r = reinterpret_cast<T*>(new char[total]);
     return r;
   }
 
   void deallocate(T* p, size_t n) {
     size_t total = sizeof(T) * n;
-    shard_t *shard = pool->pick_a_shard();
-    shard->bytes -= total;
-    shard->items -= n;
+    const auto shid = pick_a_shard_int();
+    auto& shard = pool->shard[shid];
+    shard.bytes -= total;
+    shard.items -= n;
     if (type) {
+#if defined(WITH_SEASTAR) && !defined(WITH_ALIEN)
+      type->shards[shid].items -= n;
+#else
       type->items -= n;
+#endif
     }
     delete[] reinterpret_cast<char*>(p);
   }
 
   T* allocate_aligned(size_t n, size_t align, void *p = nullptr) {
     size_t total = sizeof(T) * n;
-    shard_t *shard = pool->pick_a_shard();
-    shard->bytes += total;
-    shard->items += n;
+    const auto shid = pick_a_shard_int();
+    auto& shard = pool->shard[shid];
+    shard.bytes += total;
+    shard.items += n;
     if (type) {
+#if defined(WITH_SEASTAR) && !defined(WITH_ALIEN)
+      type->shards[shid].items += n;
+#else
       type->items += n;
+#endif
     }
     char *ptr;
     int rc = ::posix_memalign((void**)(void*)&ptr, align, total);
@@ -378,11 +415,16 @@ class pool_allocator {
 
   void deallocate_aligned(T* p, size_t n) {
     size_t total = sizeof(T) * n;
-    shard_t *shard = pool->pick_a_shard();
-    shard->bytes -= total;
-    shard->items -= n;
+    const auto shid = pick_a_shard_int();
+    auto& shard = pool->shard[shid];
+    shard.bytes -= total;
+    shard.items -= n;
     if (type) {
+#if defined(WITH_SEASTAR) && !defined(WITH_ALIEN)
+      type->shards[shid].items -= n;
+#else
       type->items -= n;
+#endif
     }
     aligned_free(p);
   }

src/test/test_c2c.cc

@@ -70,7 +70,7 @@ int main(int argc, const char **argv)
 	  while(1) {
 	    size_t i;
 	    if (sharding) {
-	      i = mempool::pool_t::pick_a_shard_int();
+	      i = mempool::pick_a_shard_int();
 	    } else {
 	      i = 0;
 	    }

src/test/test_mempool.cc

@@ -412,7 +412,7 @@ TEST(mempool, check_shard_select)
   for (size_t i = 0; i < samples; i++) {
     workers.push_back(
       std::thread([&](){
-          size_t i = mempool::pool_t::pick_a_shard_int();
+          size_t i = mempool::pick_a_shard_int();
           shards[i]++;
         }));
   }