Skip to content

Commit

Permalink
Merge pull request #14675 from ifed01/wip-bluestore-nolock
Browse files Browse the repository at this point in the history 
os/bluestore: eliminate some excessive stuff

Reviewed-by: Sage Weil <sage@redhat.com>
  • Loading branch information
@liewegas
liewegas committed Apr 28, 2017
2 parents 0530a24 + 6b457d1 commit 2944c48
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 61 deletions.
107 changes: 49 additions & 58 deletions src/os/bluestore/BlueStore.cc
View file
Expand Up @@ -2853,13 +2853,14 @@ BlueStore::BlobRef BlueStore::ExtentMap::split_blob(

void BlueStore::Onode::flush()
{
if (flushing_count) {
if (flushing_count.load()) {
ldout(c->store->cct, 20) << __func__ << " cnt:" << flushing_count << dendl;
std::unique_lock<std::mutex> l(flush_lock);
ldout(c->store->cct, 20) << flush_txns << dendl;
while (!flush_txns.empty())
while (flushing_count.load()) {
flush_cond.wait(l);
}
}
ldout(c->store->cct, 20) << "done" << dendl;
ldout(c->store->cct, 20) << __func__ << " done" << dendl;
}

// =======================================================
Expand Down Expand Up @@ -5795,9 +5796,9 @@ void BlueStore::_reap_collections()
dout(10) << __func__ << " " << c << " " << c->cid << dendl;
if (c->onode_map.map_any([&](OnodeRef o) {
assert(!o->exists);
if (!o->flush_txns.empty()) {
if (o->flushing_count.load()) {
dout(10) << __func__ << " " << c << " " << c->cid << " " << o->oid
<< " flush_txns " << o->flush_txns << dendl;
<< " flush_txns " << o->flushing_count << dendl;
return false;
}
return true;
Expand Down Expand Up @@ -7629,18 +7630,13 @@ void BlueStore::_txc_write_nodes(TransContext *txc, KeyValueDB::Transaction t)
<< extent_part << " bytes inline extents)"
<< dendl;
t->set(PREFIX_OBJ, o->key.c_str(), o->key.size(), bl);

std::lock_guard<std::mutex> l(o->flush_lock);
o->flush_txns.insert(txc);
o->flushing_count++;
}

// objects we modified but didn't affect the onode
auto p = txc->modified_objects.begin();
while (p != txc->modified_objects.end()) {
if (txc->onodes.count(*p) == 0) {
std::lock_guard<std::mutex> l((*p)->flush_lock);
(*p)->flush_txns.insert(txc);
(*p)->flushing_count++;
++p;
} else {
Expand Down Expand Up @@ -7731,13 +7727,10 @@ void BlueStore::_txc_applied_kv(TransContext *txc)
{
for (auto ls : { &txc->onodes, &txc->modified_objects }) {
for (auto& o : *ls) {
std::lock_guard<std::mutex> l(o->flush_lock);
dout(20) << __func__ << " onode " << o << " had " << o->flush_txns
dout(20) << __func__ << " onode " << o << " had " << o->flushing_count
<< dendl;
assert(o->flush_txns.count(txc));
o->flush_txns.erase(txc);
o->flushing_count--;
if (o->flush_txns.empty()) {
if (--o->flushing_count == 0) {
std::lock_guard<std::mutex> l(o->flush_lock);
o->flush_cond.notify_all();
}
}
Expand Down Expand Up @@ -7785,41 +7778,13 @@ void BlueStore::_txc_finish(TransContext *txc)
}

OpSequencerRef osr = txc->osr;
{
std::lock_guard<std::mutex> l(osr->qlock);
txc->state = TransContext::STATE_DONE;
}

bool empty = _osr_reap_done(osr.get());
if (empty && osr->zombie) {
dout(10) << __func__ << " reaping empty zombie osr " << osr << dendl;
osr->_unregister();
}
}

void BlueStore::_txc_release_alloc(TransContext *txc)
{
// update allocator with full released set
if (!cct->_conf->bluestore_debug_no_reuse_blocks) {
dout(10) << __func__ << " " << txc << " " << txc->released << dendl;
for (interval_set<uint64_t>::iterator p = txc->released.begin();
p != txc->released.end();
++p) {
alloc->release(p.get_start(), p.get_len());
}
}

txc->allocated.clear();
txc->released.clear();
}

bool BlueStore::_osr_reap_done(OpSequencer *osr)
{
CollectionRef c;
bool empty = false;
OpSequencer::q_list_t releasing_txc;
{
std::lock_guard<std::mutex> l(osr->qlock);
dout(20) << __func__ << " osr " << osr << dendl;
txc->state = TransContext::STATE_DONE;
bool notify = false;
while (!osr->q.empty()) {
TransContext *txc = &osr->q.front();
dout(20) << __func__ << " txc " << txc << " " << txc->get_state_name()
Expand All @@ -7828,36 +7793,62 @@ bool BlueStore::_osr_reap_done(OpSequencer *osr)
if (txc->state == TransContext::STATE_PREPARE &&
deferred_aggressive) {
// for _osr_drain_preceding()
osr->qcond.notify_all();
notify = true;
}
break;
}

// release to allocator only after all preceding txc's have also
// finished any deferred writes that potentially land in these
// blocks
_txc_release_alloc(txc);

if (!c && txc->first_collection) {
c = txc->first_collection;
}

osr->q.pop_front();
txc->log_state_latency(logger, l_bluestore_state_done_lat);
delete txc;
releasing_txc.push_back(*txc);
notify = true;
}
if (notify) {
osr->qcond.notify_all();
}
if (osr->q.empty()) {
dout(20) << __func__ << " osr " << osr << " q now empty" << dendl;
empty = true;
}
}
while (!releasing_txc.empty()) {
// release to allocator only after all preceding txc's have also
// finished any deferred writes that potentially land in these
// blocks
auto txc = &releasing_txc.front();
_txc_release_alloc(txc);
releasing_txc.pop_front();
txc->log_state_latency(logger, l_bluestore_state_done_lat);
delete txc;
}

if (c) {
c->trim_cache();
}

return empty;

if (empty && osr->zombie) {
dout(10) << __func__ << " reaping empty zombie osr " << osr << dendl;
osr->_unregister();
}
}

void BlueStore::_txc_release_alloc(TransContext *txc)
{
// update allocator with full released set
if (!cct->_conf->bluestore_debug_no_reuse_blocks) {
dout(10) << __func__ << " " << txc << " " << txc->released << dendl;
for (interval_set<uint64_t>::iterator p = txc->released.begin();
p != txc->released.end();
++p) {
alloc->release(p.get_start(), p.get_len());
}
}

txc->allocated.clear();
txc->released.clear();
}

void BlueStore::_osr_drain_preceding(TransContext *txc)
Expand Down
4 changes: 1 addition & 3 deletions src/os/bluestore/BlueStore.h
View file
Expand Up @@ -988,7 +988,6 @@ class BlueStore : public ObjectStore,
std::atomic<int> flushing_count = {0};
std::mutex flush_lock; ///< protect flush_txns
std::condition_variable flush_cond; ///< wait here for uncommitted txns
set<TransContext*> flush_txns; ///< unapplied txns

Onode(Collection *c, const ghobject_t& o,
const mempool::bluestore_meta_other::string& k)
Expand Down Expand Up @@ -1635,7 +1634,7 @@ class BlueStore : public ObjectStore,
void discard() override {
// Note that we may have txc's in flight when the parent Sequencer
// goes away. Reflect this with zombie==registered==true and let
// _osr_reap_done or _osr_drain_all clean up later.
// _osr_drain_all clean up later.
assert(!zombie);
zombie = true;
parent = nullptr;
Expand Down Expand Up @@ -1961,7 +1960,6 @@ class BlueStore : public ObjectStore,
void _txc_finish(TransContext *txc);
void _txc_release_alloc(TransContext *txc);

bool _osr_reap_done(OpSequencer *osr);
void _osr_drain_preceding(TransContext *txc);
void _osr_drain_all();
void _osr_unregister_all();
Expand Down

0 comments on commit 2944c48

Please sign in to comment.