Skip to content

Commit 0855adb

Browse files
aclamkaclamk
authored
Merge pull request ceph#54650 from pereman2/buffer_map-noptr
os/bluestore: use buffer values instead of pointers in buffer_map
2 parents ff6d095 + 8d915fe commit 0855adb

File tree

2 files changed

+113
-89
lines changed

2 files changed

+113
-89
lines changed

‎src/os/bluestore/BlueStore.cc‎

Lines changed: 57 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -585,8 +585,8 @@ void _dump_extent_map(CephContext *cct, const BlueStore::ExtentMap &em)
585585
std::lock_guard l(e.blob->shared_blob->get_cache()->lock);
586586
for (auto& i : e.blob->get_bc().buffer_map) {
587587
dout(LogLevelV) << __func__ << " 0x" << std::hex << i.first
588-
<< "~" << i.second->length << std::dec
589-
<< " " << *i.second << dendl;
588+
<< "~" << i.second.length << std::dec
589+
<< " " << i.second << dendl;
590590
}
591591
}
592592
}
@@ -1672,7 +1672,7 @@ int BlueStore::BufferSpace::_discard(BufferCacheShard* cache, uint32_t offset, u
16721672
auto i = _data_lower_bound(offset);
16731673
uint32_t end = offset + length;
16741674
while (i != buffer_map.end()) {
1675-
Buffer *b = i->second.get();
1675+
Buffer *b = &i->second;
16761676
if (b->offset >= end) {
16771677
break;
16781678
}
@@ -1687,13 +1687,9 @@ int BlueStore::BufferSpace::_discard(BufferCacheShard* cache, uint32_t offset, u
16871687
if (b->data.length()) {
16881688
bufferlist bl;
16891689
bl.substr_of(b->data, b->length - tail, tail);
1690-
Buffer *nb = new Buffer(this, b->state, b->seq, end, bl, b->flags);
1691-
nb->maybe_rebuild();
1692-
_add_buffer(cache, nb, 0, b);
1690+
_add_buffer(cache, this, Buffer(this, b->state, b->seq, end, bl, b->flags), 0, 0, b);
16931691
} else {
1694-
_add_buffer(cache, new Buffer(this, b->state, b->seq, end, tail,
1695-
b->flags),
1696-
0, b);
1692+
_add_buffer(cache, this, Buffer(this, b->state, b->seq, end, tail, b->flags), 0, 0, b);
16971693
}
16981694
if (!b->is_writing()) {
16991695
cache->_adjust_size(b, front - (int64_t)b->length);
@@ -1723,13 +1719,11 @@ int BlueStore::BufferSpace::_discard(BufferCacheShard* cache, uint32_t offset, u
17231719
if (b->data.length()) {
17241720
bufferlist bl;
17251721
bl.substr_of(b->data, b->length - keep, keep);
1726-
Buffer *nb = new Buffer(this, b->state, b->seq, end, bl, b->flags);
1727-
nb->maybe_rebuild();
1728-
_add_buffer(cache, nb, 0, b);
1722+
_add_buffer(cache, this,
1723+
Buffer(this, b->state, b->seq, end, bl, b->flags), 0, 0, b);
17291724
} else {
1730-
_add_buffer(cache, new Buffer(this, b->state, b->seq, end, keep,
1731-
b->flags),
1732-
0, b);
1725+
_add_buffer(cache, this,
1726+
Buffer(this, b->state, b->seq, end, keep, b->flags), 0, 0, b);
17331727
}
17341728
_rm_buffer(cache, i);
17351729
cache->_audit("discard end 2");
@@ -1754,9 +1748,8 @@ void BlueStore::BufferSpace::read(
17541748
{
17551749
std::lock_guard l(cache->lock);
17561750
for (auto i = _data_lower_bound(offset);
1757-
i != buffer_map.end() && offset < end && i->first < end;
1758-
++i) {
1759-
Buffer *b = i->second.get();
1751+
i != buffer_map.end() && offset < end && i->first < end; ++i) {
1752+
Buffer *b = &i->second;
17601753
ceph_assert(b->end() > offset);
17611754

17621755
bool val = false;
@@ -1856,9 +1849,10 @@ bool BlueStore::BufferSpace::_dup_writing(BufferCacheShard* cache, BufferSpace*
18561849
copied = true;
18571850
for (auto it = writing.begin(); it != writing.end(); ++it) {
18581851
Buffer& b = *it;
1859-
Buffer* to_b = new Buffer(to, b.state, b.seq, b.offset, b.data, b.flags);
1860-
ceph_assert(to_b->is_writing());
1861-
to->_add_buffer(cache, to_b, 0, nullptr);
1852+
ceph_assert(b.is_writing());
1853+
to->_add_buffer(cache, to,
1854+
Buffer(to, b.state, b.seq, b.offset, b.data, b.flags), 0,
1855+
0, nullptr);
18621856
}
18631857
}
18641858
return copied;
@@ -1872,39 +1866,42 @@ void BlueStore::BufferSpace::split(BufferCacheShard* cache, size_t pos, BlueStor
18721866

18731867
auto p = --buffer_map.end();
18741868
while (true) {
1875-
if (p->second->end() <= pos)
1876-
break;
1869+
if (p->second.end() <= pos) break;
18771870

1878-
if (p->second->offset < pos) {
1879-
ldout(cache->cct, 30) << __func__ << " cut " << *p->second << dendl;
1880-
size_t left = pos - p->second->offset;
1881-
size_t right = p->second->length - left;
1882-
if (p->second->data.length()) {
1883-
bufferlist bl;
1884-
bl.substr_of(p->second->data, left, right);
1885-
r._add_buffer(cache, new Buffer(&r, p->second->state, p->second->seq,
1886-
0, bl, p->second->flags),
1887-
0, p->second.get());
1871+
if (p->second.offset < pos) {
1872+
ldout(cache->cct, 30) << __func__ << " cut " << p->second << dendl;
1873+
size_t left = pos - p->second.offset;
1874+
size_t right = p->second.length - left;
1875+
if (p->second.data.length()) {
1876+
bufferlist bl;
1877+
bl.substr_of(p->second.data, left, right);
1878+
r._add_buffer(
1879+
cache, &r,
1880+
Buffer(&r, p->second.state, p->second.seq, 0, bl, p->second.flags),
1881+
0, 0, &p->second);
18881882
} else {
1889-
r._add_buffer(cache, new Buffer(&r, p->second->state, p->second->seq,
1890-
0, right, p->second->flags),
1891-
0, p->second.get());
1883+
r._add_buffer(cache, &r, Buffer(&r, p->second.state, p->second.seq, 0, right,
1884+
p->second.flags), 0, 0, &p->second);
18921885
}
1893-
cache->_adjust_size(p->second.get(), -right);
1894-
p->second->truncate(left);
1886+
cache->_adjust_size(&p->second, -right);
1887+
p->second.truncate(left);
18951888
break;
18961889
}
18971890

1898-
ceph_assert(p->second->end() > pos);
1899-
ldout(cache->cct, 30) << __func__ << " move " << *p->second << dendl;
1900-
if (p->second->data.length()) {
1901-
r._add_buffer(cache, new Buffer(&r, p->second->state, p->second->seq,
1902-
p->second->offset - pos, p->second->data, p->second->flags),
1903-
0, p->second.get());
1891+
ceph_assert(p->second.end() > pos);
1892+
ldout(cache->cct, 30) << __func__ << " move " << p->second << dendl;
1893+
if (p->second.data.length()) {
1894+
r._add_buffer(cache, &r,
1895+
Buffer(&r, p->second.state, p->second.seq,
1896+
p->second.offset - pos, p->second.data,
1897+
p->second.flags),
1898+
0, 0, &p->second);
19041899
} else {
1905-
r._add_buffer(cache, new Buffer(&r, p->second->state, p->second->seq,
1906-
p->second->offset - pos, p->second->length, p->second->flags),
1907-
0, p->second.get());
1900+
r._add_buffer(cache, &r,
1901+
Buffer(&r, p->second.state, p->second.seq,
1902+
p->second.offset - pos, p->second.length,
1903+
p->second.flags),
1904+
0, 0, &p->second);
19081905
}
19091906
if (p == buffer_map.begin()) {
19101907
_rm_buffer(cache, p);
@@ -1922,7 +1919,7 @@ void BlueStore::BufferSpace::split(BufferCacheShard* cache, size_t pos, BlueStor
19221919
std::ostream& operator<<(std::ostream& out, const BlueStore::BufferSpace& bc)
19231920
{
19241921
for (auto& [i, j] : bc.buffer_map) {
1925-
out << " [0x" << std::hex << i << "]=" << *j << std::dec;
1922+
out << " [0x" << std::hex << i << "]=" << j << std::dec;
19261923
}
19271924
if (!bc.writing.empty()) {
19281925
out << " writing:";
@@ -2866,9 +2863,9 @@ uint32_t BlueStore::Blob::merge_blob(CephContext* cct, Blob* blob_to_dissolve)
28662863
// move BufferSpace buffers
28672864
while(!src->bc.buffer_map.empty()) {
28682865
auto buf = src->bc.buffer_map.extract(src->bc.buffer_map.cbegin());
2869-
buf.mapped()->space = &dst->bc;
2866+
buf.mapped().space = &dst->bc;
28702867
if (dst->bc.buffer_map.count(buf.key()) == 0) {
2871-
dst->bc.buffer_map[buf.key()] = std::move(buf.mapped());
2868+
dst->bc.buffer_map.insert({buf.key(), std::move(buf.mapped())});
28722869
}
28732870
}
28742871
// move BufferSpace writing
@@ -5160,12 +5157,12 @@ void BlueStore::Collection::split_cache(
51605157

51615158
auto rehome_blob = [&](Blob* b) {
51625159
for (auto& i : b->bc.buffer_map) {
5163-
if (!i.second->is_writing()) {
5164-
ldout(store->cct, 1) << __func__ << " moving " << *i.second
5160+
if (!i.second.is_writing()) {
5161+
ldout(store->cct, 1) << __func__ << " moving " << i.second
51655162
<< dendl;
5166-
dest->cache->_move(cache, i.second.get());
5163+
dest->cache->_move(cache, &i.second);
51675164
} else {
5168-
ldout(store->cct, 1) << __func__ << " not moving " << *i.second
5165+
ldout(store->cct, 1) << __func__ << " not moving " << i.second
51695166
<< dendl;
51705167
}
51715168
}
@@ -5194,13 +5191,13 @@ void BlueStore::Collection::split_cache(
51945191
b.second->last_encoded_id = -1;
51955192
}
51965193
for (auto& e : o->extent_map.extent_map) {
5197-
cache->rm_extent();
5198-
dest->cache->add_extent();
5194+
cache->rm_extent();
5195+
dest->cache->add_extent();
51995196
Blob* tb = e.blob.get();
5200-
if (tb->last_encoded_id == -1) {
5201-
rehome_blob(tb);
5202-
tb->last_encoded_id = 0;
5203-
}
5197+
if (tb->last_encoded_id == -1) {
5198+
rehome_blob(tb);
5199+
tb->last_encoded_id = 0;
5200+
}
52045201
}
52055202
for (auto& b : o->extent_map.spanning_blob_map) {
52065203
Blob* tb = b.second.get();

‎src/os/bluestore/BlueStore.h‎

Lines changed: 56 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
#include "acconfig.h"
1919

20+
#include <tuple>
2021
#include <unistd.h>
2122

2223
#include <atomic>
@@ -32,6 +33,7 @@
3233
#include <boost/functional/hash.hpp>
3334
#include <boost/dynamic_bitset.hpp>
3435
#include <boost/circular_buffer.hpp>
36+
#include <utility>
3537

3638
#include "include/cpp-btree/btree_set.h"
3739

@@ -311,6 +313,20 @@ class BlueStore : public ObjectStore,
311313
: space(space), state(s), flags(f), seq(q), offset(o),
312314
length(b.length()), data(b) {}
313315

316+
Buffer(Buffer &&other) {
317+
std::swap(space, other.space);
318+
std::swap(state, other.state);
319+
std::swap(cache_private, other.cache_private);
320+
std::swap(flags, other.flags);
321+
std::swap(seq, other.seq);
322+
std::swap(offset, other.offset);
323+
std::swap(length, other.length);
324+
std::swap(data, other.data);
325+
std::swap(cache_age_bin, other.cache_age_bin);
326+
lru_item.swap_nodes(other.lru_item);
327+
state_item.swap_nodes(other.state_item);
328+
}
329+
314330
bool is_empty() const {
315331
return state == STATE_EMPTY;
316332
}
@@ -366,7 +382,7 @@ class BlueStore : public ObjectStore,
366382
boost::intrusive::list_member_hook<>,
367383
&Buffer::state_item> > state_list_t;
368384

369-
mempool::bluestore_cache_meta::map<uint32_t, std::unique_ptr<Buffer>>
385+
mempool::bluestore_cache_meta::map<uint32_t, Buffer>
370386
buffer_map;
371387

372388
// we use a bare intrusive list here instead of std::map because
@@ -379,56 +395,63 @@ class BlueStore : public ObjectStore,
379395
ceph_assert(writing.empty());
380396
}
381397

382-
void _add_buffer(BufferCacheShard* cache, Buffer* b, int level, Buffer* near) {
398+
void _add_buffer(BufferCacheShard *cache, BufferSpace *space, Buffer&& buffer,
399+
uint16_t cache_private, int level, Buffer *near) {
400+
auto it = buffer_map.emplace(buffer.offset, std::move(buffer));
401+
Buffer *cached_buffer = &it.first->second;
402+
cached_buffer->cache_private = cache_private;
403+
_add_buffer(cache, space, cached_buffer, level, near);
404+
}
405+
406+
void _add_buffer(BufferCacheShard *cache, BufferSpace *space,
407+
Buffer *buffer, int level, Buffer *near) {
383408
cache->_audit("_add_buffer start");
384-
buffer_map[b->offset].reset(b);
385-
if (b->is_writing()) {
409+
if (buffer->is_writing()) {
386410
// we might get already cached data for which resetting mempool is inppropriate
387411
// hence calling try_assign_to_mempool
388-
b->data.try_assign_to_mempool(mempool::mempool_bluestore_writing);
389-
if (writing.empty() || writing.rbegin()->seq <= b->seq) {
390-
writing.push_back(*b);
412+
buffer->data.try_assign_to_mempool(mempool::mempool_bluestore_writing);
413+
if (writing.empty() || writing.rbegin()->seq <= buffer->seq) {
414+
writing.push_back(*buffer);
391415
} else {
392416
auto it = writing.begin();
393-
while (it->seq < b->seq) {
417+
while (it->seq < buffer->seq) {
394418
++it;
395419
}
396420

397-
ceph_assert(it->seq >= b->seq);
421+
ceph_assert(it->seq >= buffer->seq);
398422
// note that this will insert b before it
399423
// hence the order is maintained
400-
writing.insert(it, *b);
424+
writing.insert(it, *buffer);
401425
}
402426
} else {
403-
b->data.reassign_to_mempool(mempool::mempool_bluestore_cache_data);
404-
cache->_add(b, level, near);
427+
buffer->data.reassign_to_mempool(mempool::mempool_bluestore_cache_data);
428+
cache->_add(buffer, level, near);
405429
}
406430
cache->_audit("_add_buffer end");
407431
}
408432
void _rm_buffer(BufferCacheShard* cache, Buffer *b) {
409433
_rm_buffer(cache, buffer_map.find(b->offset));
410434
}
411-
std::map<uint32_t, std::unique_ptr<Buffer>>::iterator
435+
std::map<uint32_t, Buffer>::iterator
412436
_rm_buffer(BufferCacheShard* cache,
413-
std::map<uint32_t, std::unique_ptr<Buffer>>::iterator p) {
437+
std::map<uint32_t, Buffer>::iterator p) {
414438
ceph_assert(p != buffer_map.end());
415439
cache->_audit("_rm_buffer start");
416-
if (p->second->is_writing()) {
417-
writing.erase(writing.iterator_to(*p->second));
440+
if (p->second.is_writing()) {
441+
writing.erase(writing.iterator_to(p->second));
418442
} else {
419-
cache->_rm(p->second.get());
443+
cache->_rm(&p->second);
420444
}
421445
p = buffer_map.erase(p);
422446
cache->_audit("_rm_buffer end");
423447
return p;
424448
}
425449

426-
std::map<uint32_t,std::unique_ptr<Buffer>>::iterator _data_lower_bound(
427-
uint32_t offset) {
450+
std::map<uint32_t, Buffer>::iterator _data_lower_bound(uint32_t offset) {
428451
auto i = buffer_map.lower_bound(offset);
429452
if (i != buffer_map.begin()) {
430453
--i;
431-
if (i->first + i->second->length <= offset)
454+
if (i->first + i->second.length <= offset)
432455
++i;
433456
}
434457
return i;
@@ -449,18 +472,22 @@ class BlueStore : public ObjectStore,
449472
void write(BufferCacheShard* cache, uint64_t seq, uint32_t offset, ceph::buffer::list& bl,
450473
unsigned flags) {
451474
std::lock_guard l(cache->lock);
452-
Buffer *b = new Buffer(this, Buffer::STATE_WRITING, seq, offset, bl,
453-
flags);
454-
b->cache_private = _discard(cache, offset, bl.length());
455-
_add_buffer(cache, b, (flags & Buffer::FLAG_NOCACHE) ? 0 : 1, nullptr);
475+
uint16_t cache_private = _discard(cache, offset, bl.length());
476+
_add_buffer(cache, this,
477+
Buffer(this, Buffer::STATE_WRITING, seq, offset, bl,
478+
flags),
479+
cache_private, (flags & Buffer::FLAG_NOCACHE) ? 0 : 1,
480+
nullptr);
456481
cache->_trim();
457482
}
458483
void _finish_write(BufferCacheShard* cache, uint64_t seq);
459484
void did_read(BufferCacheShard* cache, uint32_t offset, ceph::buffer::list& bl) {
460485
std::lock_guard l(cache->lock);
461-
Buffer *b = new Buffer(this, Buffer::STATE_CLEAN, 0, offset, bl);
462-
b->cache_private = _discard(cache, offset, bl.length());
463-
_add_buffer(cache, b, 1, nullptr);
486+
uint16_t cache_private = _discard(cache, offset, bl.length());
487+
_add_buffer(
488+
cache, this,
489+
Buffer(this, Buffer::STATE_CLEAN, 0, offset, bl, 0),
490+
cache_private, 1, nullptr);
464491
cache->_trim();
465492
}
466493

@@ -481,8 +508,8 @@ class BlueStore : public ObjectStore,
481508
f->open_array_section("buffers");
482509
for (auto& i : buffer_map) {
483510
f->open_object_section("buffer");
484-
ceph_assert(i.first == i.second->offset);
485-
i.second->dump(f);
511+
ceph_assert(i.first == i.second.offset);
512+
i.second.dump(f);
486513
f->close_section();
487514
}
488515
f->close_section();

0 commit comments

Comments
 (0)