From: Sage Weil Date: Sat, 27 Jan 2018 19:24:22 +0000 (-0600) Subject: osd/PrimaryLogPG: remove ondisk_{read,write}_lock infra X-Git-Tag: v13.0.2~222^2~23 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=c244300ef33a044ad71fea7d92d77f33b5d41851;p=ceph.git osd/PrimaryLogPG: remove ondisk_{read,write}_lock infra This is no longer needed. FileStore was the only backend doing async applies, and it now blocks until apply all on its own. Signed-off-by: Sage Weil --- diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc index a1519fc25b2..d33a1d0fd63 100644 --- a/src/osd/PrimaryLogPG.cc +++ b/src/osd/PrimaryLogPG.cc @@ -196,22 +196,6 @@ class PrimaryLogPG::C_PG_ObjectContext : public Context { } }; -class PrimaryLogPG::C_OSD_OndiskWriteUnlock : public Context { - ObjectContextRef obc, obc2, obc3; - public: - C_OSD_OndiskWriteUnlock( - ObjectContextRef o, - ObjectContextRef o2 = ObjectContextRef(), - ObjectContextRef o3 = ObjectContextRef()) : obc(o), obc2(o2), obc3(o3) {} - void finish(int r) override { - obc->ondisk_write_unlock(); - if (obc2) - obc2->ondisk_write_unlock(); - if (obc3) - obc3->ondisk_write_unlock(); - } -}; - struct OnReadComplete : public Context { PrimaryLogPG *pg; PrimaryLogPG::OpContext *opcontext; @@ -419,7 +403,6 @@ void PrimaryLogPG::on_local_recover( if (is_primary()) { if (!is_delete) { obc->obs.exists = true; - obc->ondisk_write_lock(); bool got = obc->get_recovery_read(); assert(got); @@ -427,7 +410,6 @@ void PrimaryLogPG::on_local_recover( assert(recovering.count(obc->obs.oi.soid)); recovering[obc->obs.oi.soid] = obc; obc->obs.oi = recovery_info.oi; // may have been updated above - t->register_on_applied_sync(new C_OSD_OndiskWriteUnlock(obc)); } t->register_on_applied(new C_OSD_AppliedRecoveredObject(this, obc)); @@ -3736,11 +3718,6 @@ void PrimaryLogPG::execute_ctx(OpContext *ctx) ctx->user_at_version = obc->obs.oi.user_version; dout(30) << __func__ << " user_at_version " << ctx->user_at_version << dendl; - if (op->may_read()) { - dout(10) << " taking ondisk_read_lock" << dendl; - obc->ondisk_read_lock(); - } - { #ifdef WITH_LTTNG osd_reqid_t reqid = ctx->op->get_reqid(); @@ -3759,11 +3736,6 @@ void PrimaryLogPG::execute_ctx(OpContext *ctx) reqid.name._num, reqid.tid, reqid.inc); } - if (op->may_read()) { - dout(10) << " dropping ondisk_read_lock" << dendl; - obc->ondisk_read_unlock(); - } - bool pending_async_reads = !ctx->pending_async_reads.empty(); if (result == -EINPROGRESS || pending_async_reads) { // come back later. @@ -10133,24 +10105,16 @@ void PrimaryLogPG::issue_repop(RepGather *repop, OpContext *ctx) } } - ctx->obc->ondisk_write_lock(); - ctx->op_t->add_obc(ctx->obc); if (ctx->clone_obc) { - ctx->clone_obc->ondisk_write_lock(); ctx->op_t->add_obc(ctx->clone_obc); } if (ctx->head_obc) { - ctx->head_obc->ondisk_write_lock(); ctx->op_t->add_obc(ctx->head_obc); } Context *on_all_commit = new C_OSD_RepopCommit(this, repop); Context *on_all_applied = new C_OSD_RepopApplied(this, repop); - Context *onapplied_sync = new C_OSD_OndiskWriteUnlock( - ctx->obc, - ctx->clone_obc, - ctx->head_obc); if (!(ctx->log.empty())) { assert(ctx->at_version >= projected_last_update); projected_last_update = ctx->at_version; @@ -10167,7 +10131,7 @@ void PrimaryLogPG::issue_repop(RepGather *repop, OpContext *ctx) min_last_complete_ondisk, ctx->log, ctx->updated_hset_history, - onapplied_sync, + nullptr, on_all_applied, on_all_commit, repop->rep_tid, @@ -12238,7 +12202,6 @@ uint64_t PrimaryLogPG::recover_primary(uint64_t max, ThreadPool::TPHandle &handl dout(10) << " already reverting " << soid << dendl; } else { dout(10) << " reverting " << soid << " to " << latest->prior_version << dendl; - obc->ondisk_write_lock(); obc->obs.oi.version = latest->version; ObjectStore::Transaction t; @@ -12254,7 +12217,6 @@ uint64_t PrimaryLogPG::recover_primary(uint64_t max, ThreadPool::TPHandle &handl ++active_pushes; - t.register_on_applied_sync(new C_OSD_OndiskWriteUnlock(obc)); t.register_on_applied(new C_OSD_AppliedRecoveredObject(this, obc)); t.register_on_commit(new C_OSD_CommittedPushedObject( this, @@ -12401,14 +12363,12 @@ int PrimaryLogPG::prep_object_replica_pushes( * a client write would be blocked since the object is degraded. * In almost all cases, therefore, this lock should be uncontended. */ - obc->ondisk_read_lock(); int r = pgbackend->recover_object( soid, v, ObjectContextRef(), obc, // has snapset context h); - obc->ondisk_read_unlock(); if (r < 0) { dout(0) << __func__ << " Error " << r << " on oid " << soid << dendl; primary_failed(soid); @@ -12959,14 +12919,12 @@ int PrimaryLogPG::prep_backfill_object_push( recovering.insert(make_pair(oid, obc)); // We need to take the read_lock here in order to flush in-progress writes - obc->ondisk_read_lock(); int r = pgbackend->recover_object( oid, v, ObjectContextRef(), obc, h); - obc->ondisk_read_unlock(); if (r < 0) { dout(0) << __func__ << " Error " << r << " on oid " << oid << dendl; primary_failed(oid); @@ -13727,10 +13685,8 @@ void PrimaryLogPG::agent_load_hit_sets() bufferlist bl; { - obc->ondisk_read_lock(); int r = osd->store->read(ch, ghobject_t(oid), 0, 0, bl); assert(r >= 0); - obc->ondisk_read_unlock(); } HitSetRef hs(new HitSet); bufferlist::iterator pbl = bl.begin(); diff --git a/src/osd/PrimaryLogPG.h b/src/osd/PrimaryLogPG.h index f11a391775a..435dfa93b2b 100644 --- a/src/osd/PrimaryLogPG.h +++ b/src/osd/PrimaryLogPG.h @@ -1270,7 +1270,6 @@ protected: void send_remove_op(const hobject_t& oid, eversion_t v, pg_shard_t peer); - class C_OSD_OndiskWriteUnlock; class C_OSD_AppliedRecoveredObject; class C_OSD_CommittedPushedObject; class C_OSD_AppliedRecoveredObjectReplica; diff --git a/src/osd/osd_internal_types.h b/src/osd/osd_internal_types.h index 924a8be0c37..4217767e677 100644 --- a/src/osd/osd_internal_types.h +++ b/src/osd/osd_internal_types.h @@ -46,12 +46,7 @@ struct ObjectContext { Context *destructor_callback; -private: - Mutex lock; public: - Cond cond; - int unstable_writes, readers, writers_waiting, readers_waiting; - // any entity in obs.oi.watchers MUST be in either watchers or unconnected_watchers. map, WatchRef> watchers; @@ -299,8 +294,6 @@ public: ObjectContext() : ssc(NULL), destructor_callback(0), - lock("PrimaryLogPG::ObjectContext::lock"), - unstable_writes(0), readers(0), writers_waiting(0), readers_waiting(0), blocked(false), requeue_scrub_on_unblock(false) {} ~ObjectContext() { @@ -321,42 +314,6 @@ public: return blocked; } - // do simple synchronous mutual exclusion, for now. no waitqueues or anything fancy. - void ondisk_write_lock() { - lock.Lock(); - writers_waiting++; - while (readers_waiting || readers) - cond.Wait(lock); - writers_waiting--; - unstable_writes++; - lock.Unlock(); - } - void ondisk_write_unlock() { - lock.Lock(); - assert(unstable_writes > 0); - unstable_writes--; - if (!unstable_writes && readers_waiting) - cond.Signal(); - lock.Unlock(); - } - void ondisk_read_lock() { - lock.Lock(); - readers_waiting++; - while (unstable_writes) - cond.Wait(lock); - readers_waiting--; - readers++; - lock.Unlock(); - } - void ondisk_read_unlock() { - lock.Lock(); - assert(readers > 0); - readers--; - if (!readers && writers_waiting) - cond.Signal(); - lock.Unlock(); - } - /// in-progress copyfrom ops for this object bool blocked:1; bool requeue_scrub_on_unblock:1; // true if we need to requeue scrub on unblock diff --git a/src/test/osd/types.cc b/src/test/osd/types.cc index 6491cc74c0c..b3e5dc85baa 100644 --- a/src/test/osd/types.cc +++ b/src/test/osd/types.cc @@ -1039,197 +1039,6 @@ TEST(pg_missing_t, split_into) EXPECT_TRUE(missing.is_missing(oid2)); } -class ObjectContextTest : public ::testing::Test { -protected: - - static const useconds_t DELAY_MAX = 20 * 1000 * 1000; - - class Thread_read_lock : public Thread { - public: - ObjectContext &obc; - - explicit Thread_read_lock(ObjectContext& _obc) : - obc(_obc) - { - } - - void *entry() override { - obc.ondisk_read_lock(); - return NULL; - } - }; - - class Thread_write_lock : public Thread { - public: - ObjectContext &obc; - - explicit Thread_write_lock(ObjectContext& _obc) : - obc(_obc) - { - } - - void *entry() override { - obc.ondisk_write_lock(); - return NULL; - } - }; - -}; - -TEST_F(ObjectContextTest, read_write_lock) -{ - { - ObjectContext obc; - - // - // write_lock - // write_lock - // write_unlock - // write_unlock - // - EXPECT_EQ(0, obc.writers_waiting); - EXPECT_EQ(0, obc.unstable_writes); - - obc.ondisk_write_lock(); - - EXPECT_EQ(0, obc.writers_waiting); - EXPECT_EQ(1, obc.unstable_writes); - - obc.ondisk_write_lock(); - - EXPECT_EQ(0, obc.writers_waiting); - EXPECT_EQ(2, obc.unstable_writes); - - obc.ondisk_write_unlock(); - - EXPECT_EQ(0, obc.writers_waiting); - EXPECT_EQ(1, obc.unstable_writes); - - obc.ondisk_write_unlock(); - - EXPECT_EQ(0, obc.writers_waiting); - EXPECT_EQ(0, obc.unstable_writes); - } - - useconds_t delay = 0; - - { - ObjectContext obc; - - // - // write_lock - // read_lock => wait - // write_unlock => signal - // read_unlock - // - EXPECT_EQ(0, obc.readers_waiting); - EXPECT_EQ(0, obc.readers); - EXPECT_EQ(0, obc.writers_waiting); - EXPECT_EQ(0, obc.unstable_writes); - - obc.ondisk_write_lock(); - - EXPECT_EQ(0, obc.readers_waiting); - EXPECT_EQ(0, obc.readers); - EXPECT_EQ(0, obc.writers_waiting); - EXPECT_EQ(1, obc.unstable_writes); - - Thread_read_lock t(obc); - t.create("obc_read"); - - do { - cout << "Trying (1) with delay " << delay << "us\n"; - usleep(delay); - } while (obc.readers_waiting == 0 && - ( delay = delay * 2 + 1) < DELAY_MAX); - - EXPECT_EQ(1, obc.readers_waiting); - EXPECT_EQ(0, obc.readers); - EXPECT_EQ(0, obc.writers_waiting); - EXPECT_EQ(1, obc.unstable_writes); - - obc.ondisk_write_unlock(); - - do { - cout << "Trying (2) with delay " << delay << "us\n"; - usleep(delay); - } while ((obc.readers == 0 || obc.readers_waiting == 1) && - ( delay = delay * 2 + 1) < DELAY_MAX); - EXPECT_EQ(0, obc.readers_waiting); - EXPECT_EQ(1, obc.readers); - EXPECT_EQ(0, obc.writers_waiting); - EXPECT_EQ(0, obc.unstable_writes); - - obc.ondisk_read_unlock(); - - EXPECT_EQ(0, obc.readers_waiting); - EXPECT_EQ(0, obc.readers); - EXPECT_EQ(0, obc.writers_waiting); - EXPECT_EQ(0, obc.unstable_writes); - - t.join(); - } - - { - ObjectContext obc; - - // - // read_lock - // write_lock => wait - // read_unlock => signal - // write_unlock - // - EXPECT_EQ(0, obc.readers_waiting); - EXPECT_EQ(0, obc.readers); - EXPECT_EQ(0, obc.writers_waiting); - EXPECT_EQ(0, obc.unstable_writes); - - obc.ondisk_read_lock(); - - EXPECT_EQ(0, obc.readers_waiting); - EXPECT_EQ(1, obc.readers); - EXPECT_EQ(0, obc.writers_waiting); - EXPECT_EQ(0, obc.unstable_writes); - - Thread_write_lock t(obc); - t.create("obc_write"); - - do { - cout << "Trying (3) with delay " << delay << "us\n"; - usleep(delay); - } while ((obc.writers_waiting == 0) && - ( delay = delay * 2 + 1) < DELAY_MAX); - - EXPECT_EQ(0, obc.readers_waiting); - EXPECT_EQ(1, obc.readers); - EXPECT_EQ(1, obc.writers_waiting); - EXPECT_EQ(0, obc.unstable_writes); - - obc.ondisk_read_unlock(); - - do { - cout << "Trying (4) with delay " << delay << "us\n"; - usleep(delay); - } while ((obc.unstable_writes == 0 || obc.writers_waiting == 1) && - ( delay = delay * 2 + 1) < DELAY_MAX); - - EXPECT_EQ(0, obc.readers_waiting); - EXPECT_EQ(0, obc.readers); - EXPECT_EQ(0, obc.writers_waiting); - EXPECT_EQ(1, obc.unstable_writes); - - obc.ondisk_write_unlock(); - - EXPECT_EQ(0, obc.readers_waiting); - EXPECT_EQ(0, obc.readers); - EXPECT_EQ(0, obc.writers_waiting); - EXPECT_EQ(0, obc.unstable_writes); - - t.join(); - } - -} - TEST(pg_pool_t_test, get_pg_num_divisor) { pg_pool_t p; p.set_pg_num(16);