From 0932d39e28cd1e106efb697612bb405584e90154 Mon Sep 17 00:00:00 2001
From: Sage Weil <sage@redhat.com>
Date: Wed, 5 Oct 2016 09:56:09 -0400
Subject: [PATCH] os/bluestore: simplify BufferSpace writing_map / list

We were tracking buffers by seq.  In reality, though, we
won't have very many buffers that have in flight IO to
the same blob at the same time, so we can get by with
just a list here.  We can rely on the fact that the
list will be sorted (seq's increase, and we put new
buffers at the end).

This saves us a memory allocation.  No change in size of
BufferSpace and Blob (std::list -> intrusive::list).

Signed-off-by: Sage Weil <sage@redhat.com>
---
 src/os/bluestore/BlueStore.cc | 37 +++++++++++++++++------------------
 src/os/bluestore/BlueStore.h  | 32 +++++++-----------------------
 2 files changed, 25 insertions(+), 44 deletions(-)

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 1e963e3e52374..dd88ac57c1f38 100644
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -1043,29 +1043,28 @@ void BlueStore::BufferSpace::finish_write(uint64_t seq)
 {
   std::lock_guard<std::recursive_mutex> l(cache->lock);
 
-  auto i = writing_map.begin();
-  while (i != writing_map.end()) {
-    if (i->first > seq)
+  auto i = writing.begin();
+  while (i != writing.end()) {
+    if (i->seq > seq) {
       break;
+    }
+    if (i->seq < seq) {
+      ++i;
+      continue;
+    }
 
-    auto l = i->second.begin();
-    while (l != i->second.end()) {
-      Buffer *b = &*l;
-      dout(20) << __func__ << " " << *b << dendl;
-      assert(b->is_writing());
+    Buffer *b = &*i;
+    dout(20) << __func__ << " " << *b << dendl;
+    assert(b->is_writing());
 
-      if (b->flags & Buffer::FLAG_NOCACHE) {
-        i->second.erase(l++);
-        buffer_map.erase(b->offset);
-      } else {
-        b->state = Buffer::STATE_CLEAN;
-        i->second.erase(l++);
-        cache->_add_buffer(b, 1, nullptr);
-      }
+    if (b->flags & Buffer::FLAG_NOCACHE) {
+      writing.erase(i++);
+      buffer_map.erase(b->offset);
+    } else {
+      b->state = Buffer::STATE_CLEAN;
+      writing.erase(i++);
+      cache->_add_buffer(b, 1, nullptr);
     }
-
-    assert(i->second.empty());
-    writing_map.erase(i++);
   }
 
   cache->_audit("finish_write end");
diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index 416821389f2d7..ca9035b0a87b7 100644
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -208,22 +208,10 @@ public:
     map<uint64_t,std::unique_ptr<Buffer>> buffer_map;
     Cache *cache;
 
-    // we use a list here instead of std::map because it uses less memory and
-    // we expect this to be very small (very few IOs in flight to the same
-    // Blob at the same time).
-    list<pair<uint64_t,state_list_t>> writing_map;
-
-    list<pair<uint64_t,state_list_t>>::iterator get_writing_map(uint64_t seq) {
-      for (auto p = writing_map.begin(); p != writing_map.end(); ++p) {
-	if (p->first == seq) {
-	  return p;
-	}
-      }
-      writing_map.push_back(make_pair(seq, state_list_t()));
-      auto p = writing_map.end();
-      --p;
-      return p;
-    }
+    // we use a bare intrusive list here instead of std::map because
+    // it uses less memory and we expect this to be very small (very
+    // few IOs in flight to the same Blob at the same time).
+    state_list_t writing;   ///< writing buffers, sorted by seq, ascending
 
     BufferSpace(Cache *c) : cache(c) {
       if (cache) {
@@ -232,7 +220,7 @@ public:
     }
     ~BufferSpace() {
       assert(buffer_map.empty());
-      assert(writing_map.empty());
+      assert(writing.empty());
       if (cache) {
 	cache->rm_blob();
       }
@@ -242,7 +230,7 @@ public:
       cache->_audit("_add_buffer start");
       buffer_map[b->offset].reset(b);
       if (b->is_writing()) {
-        get_writing_map(b->seq)->second.push_back(*b);
+        writing.push_back(*b);
       } else {
 	cache->_add_buffer(b, level, near);
       }
@@ -254,13 +242,7 @@ public:
     void _rm_buffer(map<uint64_t,std::unique_ptr<Buffer>>::iterator p) {
       cache->_audit("_rm_buffer start");
       if (p->second->is_writing()) {
-        uint64_t seq = (*p->second.get()).seq;
-        auto it = get_writing_map(seq);
-        assert(!it->second.empty());
-        it->second.erase(it->second.iterator_to(*p->second));
-        if (it->second.empty()) {
-          writing_map.erase(it);
-	}
+        writing.erase(writing.iterator_to(*p->second));
       } else {
 	cache->_rm_buffer(p->second.get());
       }
-- 
2.39.5