From: Sage Weil Date: Thu, 15 Dec 2016 14:38:02 +0000 (-0500) Subject: common/TrackedOp: use explicit ref count, intrusive_ptr X-Git-Tag: v12.0.0~45^2~14 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=04fed4af909b685ccfe23bd92e052ed28021261b;p=ceph-ci.git common/TrackedOp: use explicit ref count, intrusive_ptr This is more efficient and more explicit than shared_ptr. It will also allow us to put these in intrusive lists. Note that before we used the shared_ptr OnDelete property only for the live ops and left it off for the history. Here we rename is_tracked to state and explicit note whether we are untracked, live, or history. Also, now that we #include OpRequest in osd_types.h, we have to include the .cc file (and ~OpRequest definition) in libcommon to avoid a link error on all the unit tests etc. Signed-off-by: Sage Weil --- diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index d7cf3a2b57c..35980e319a9 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -461,6 +461,7 @@ set(libcommon_files osd/OSDMap.cc common/histogram.cc osd/osd_types.cc + osd/OpRequest.cc common/blkdev.cc common/common_init.cc common/pipe.c @@ -595,6 +596,7 @@ add_library(common_utf8 STATIC common/utf8.c) if(${WITH_LTTNG}) add_subdirectory(tracing) + add_dependencies(common-objs oprequest-tp) endif(${WITH_LTTNG}) add_subdirectory(global) diff --git a/src/common/TrackedOp.cc b/src/common/TrackedOp.cc index 00f66c7cefa..52396bec37b 100644 --- a/src/common/TrackedOp.cc +++ b/src/common/TrackedOp.cc @@ -182,7 +182,7 @@ bool OpTracker::register_inflight_op(xlist::item *i) void OpTracker::unregister_inflight_op(TrackedOp *i) { // caller checks; - assert(i->is_tracked); + assert(i->state); uint32_t shard_index = i->seq % num_optracker_shards; ShardedTrackingData* sdata = sharded_in_flight_list[shard_index]; @@ -198,6 +198,7 @@ void OpTracker::unregister_inflight_op(TrackedOp *i) if (!tracking_enabled) delete i; else { + i->state = TrackedOp::STATE_HISTORY; utime_t now = ceph_clock_now(); history.insert(now, TrackedOpRef(i)); } @@ -315,7 +316,7 @@ void OpTracker::get_age_ms_histogram(pow2_hist_t *h) void OpTracker::mark_event(TrackedOp *op, const string &dest, utime_t time) { - if (!op->is_tracked) + if (!op->state) return; return _mark_event(op, dest, time); } @@ -332,20 +333,9 @@ void OpTracker::_mark_event(TrackedOp *op, const string &evt, } -void OpTracker::RemoveOnDelete::operator()(TrackedOp *op) { - if (!op->is_tracked) { - op->_unregistered(); - delete op; - return; - } - op->mark_event("done"); - tracker->unregister_inflight_op(op); - // Do not delete op, unregister_inflight_op took control -} - void TrackedOp::mark_event(const string &event) { - if (!is_tracked) + if (!state) return; utime_t now = ceph_clock_now(); @@ -360,7 +350,7 @@ void TrackedOp::mark_event(const string &event) void TrackedOp::dump(utime_t now, Formatter *f) const { // Ignore if still in the constructor - if (!is_tracked) + if (!state) return; stringstream name; _dump_op_descriptor_unlocked(name); diff --git a/src/common/TrackedOp.h b/src/common/TrackedOp.h index 5b21c5aee1f..7717df01ec7 100644 --- a/src/common/TrackedOp.h +++ b/src/common/TrackedOp.h @@ -25,7 +25,7 @@ #include class TrackedOp; -typedef ceph::shared_ptr TrackedOpRef; +typedef boost::intrusive_ptr TrackedOpRef; class OpHistory { set > arrived; @@ -54,13 +54,6 @@ public: struct ShardedTrackingData; class OpTracker { - class RemoveOnDelete { - OpTracker *tracker; - public: - explicit RemoveOnDelete(OpTracker *tracker) : tracker(tracker) {} - void operator()(TrackedOp *op); - }; - friend class RemoveOnDelete; friend class OpHistory; atomic64_t seq; vector sharded_in_flight_list; @@ -114,8 +107,7 @@ public: template typename T::Ref create_request(U params) { - typename T::Ref retval(new T(params, this), - RemoveOnDelete(this)); + typename T::Ref retval(new T(params, this)); retval->tracking_start(); return retval; } @@ -127,7 +119,8 @@ private: friend class OpTracker; xlist::item xitem; protected: - OpTracker *tracker; /// the tracker we are associated with + OpTracker *tracker; ///< the tracker we are associated with + std::atomic_int nref = {0}; ///< ref count utime_t initiated_at; list > events; /// list of events and their times @@ -136,16 +129,21 @@ protected: uint64_t seq; /// a unique value set by the OpTracker uint32_t warn_interval_multiplier; // limits output of a given op warning - // Transitions from false -> true without locks being held - atomic is_tracked; //whether in tracker and out of constructor + + enum { + STATE_UNTRACKED = 0, + STATE_LIVE, + STATE_HISTORY + }; + atomic state = {STATE_UNTRACKED}; + TrackedOp(OpTracker *_tracker, const utime_t& initiated) : xitem(this), tracker(_tracker), initiated_at(initiated), lock("TrackedOp::lock"), seq(0), - warn_interval_multiplier(1), - is_tracked(false) + warn_interval_multiplier(1) { } /// output any type-specific data you want to get when dump() is called @@ -181,7 +179,35 @@ public: void tracking_start() { if (tracker->register_inflight_op(&xitem)) { events.push_back(make_pair(initiated_at, "initiated")); - is_tracked = true; + state = STATE_LIVE; + } + } + + // ref counting via intrusive_ptr, with special behavior on final + // put for historical op tracking + friend void intrusive_ptr_add_ref(TrackedOp *o) { + ++o->nref; + } + friend void intrusive_ptr_release(TrackedOp *o) { + if (--o->nref == 0) { + switch (o->state.load()) { + case STATE_UNTRACKED: + o->_unregistered(); + delete o; + break; + + case STATE_LIVE: + o->mark_event("done"); + o->tracker->unregister_inflight_op(o); + break; + + case STATE_HISTORY: + delete o; + break; + + default: + ceph_abort(); + } } } }; diff --git a/src/mds/CInode.h b/src/mds/CInode.h index 9ce9a6a13c4..b8bd8206fd1 100644 --- a/src/mds/CInode.h +++ b/src/mds/CInode.h @@ -32,6 +32,7 @@ #include "LocalLock.h" #include "Capability.h" #include "SnapRealm.h" +#include "Mutation.h" #include #include @@ -51,8 +52,6 @@ class Session; class MClientCaps; struct ObjectOperation; class EMetaBlob; -struct MDRequestImpl; -typedef ceph::shared_ptr MDRequestRef; ostream& operator<<(ostream& out, const CInode& in); diff --git a/src/mds/Locker.h b/src/mds/Locker.h index 7c3fcd9b277..c34c7f82709 100644 --- a/src/mds/Locker.h +++ b/src/mds/Locker.h @@ -40,10 +40,8 @@ class SimpleLock; class ScatterLock; class LocalLock; -class MDCache; -typedef ceph::shared_ptr MDRequestRef; - #include "SimpleLock.h" +#include "Mutation.h" class Locker { private: diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc index 6717adb9ed1..2cd1f78d058 100644 --- a/src/mds/MDCache.cc +++ b/src/mds/MDCache.cc @@ -2213,8 +2213,7 @@ void MDCache::predirty_journal_parents(MutationRef mut, EMetaBlob *blob, } // can cast only because i'm passing nowait=true in the sole user - MDRequestRef mdmut = - ceph::static_pointer_cast(mut); + MDRequestRef mdmut = static_cast(mut.get()); if (!stop && mut->wrlocks.count(&pin->nestlock) == 0 && (!pin->versionlock.can_wrlock() || // make sure we can take versionlock, too diff --git a/src/mds/MDCache.h b/src/mds/MDCache.h index a9c41193478..bd00ffcfb76 100644 --- a/src/mds/MDCache.h +++ b/src/mds/MDCache.h @@ -31,6 +31,7 @@ #include "StrayManager.h" #include "MDSContext.h" #include "MDSMap.h" +#include "Mutation.h" #include "messages/MClientRequest.h" #include "messages/MMDSSlaveRequest.h" @@ -68,10 +69,6 @@ class MMDSFragmentNotify; class ESubtreeMap; -struct MDRequestImpl; -typedef ceph::shared_ptr MDRequestRef; -struct MDSlaveUpdate; - enum { l_mdc_first = 3000, // How many inodes currently in stray dentries diff --git a/src/mds/Migrator.cc b/src/mds/Migrator.cc index e57bffdec08..1b50c42adc0 100644 --- a/src/mds/Migrator.cc +++ b/src/mds/Migrator.cc @@ -358,7 +358,7 @@ void Migrator::export_try_cancel(CDir *dir, bool notify_peer) // drop locks if (state == EXPORT_LOCKING || state == EXPORT_DISCOVERING) { - MDRequestRef mdr = ceph::static_pointer_cast(mut); + MDRequestRef mdr = static_cast(mut.get()); assert(mdr); if (mdr->more()->waiting_on_slave.empty()) mds->mdcache->request_finish(mdr); @@ -876,8 +876,7 @@ void Migrator::handle_export_discover_ack(MExportDirDiscoverAck *m) } else { assert(it->second.state == EXPORT_DISCOVERING); // release locks to avoid deadlock - MDRequestRef mdr = ceph::static_pointer_cast(it->second.mut); + MDRequestRef mdr = static_cast(it->second.mut.get()); assert(mdr); mds->mdcache->request_finish(mdr); it->second.mut.reset(); diff --git a/src/mds/Mutation.h b/src/mds/Mutation.h index dcb2ed40dce..8711d9f50a9 100644 --- a/src/mds/Mutation.h +++ b/src/mds/Mutation.h @@ -164,7 +164,7 @@ inline ostream& operator<<(ostream &out, const MutationImpl &mut) return out; } -typedef ceph::shared_ptr MutationRef; +typedef boost::intrusive_ptr MutationRef; @@ -334,13 +334,13 @@ struct MDRequestImpl : public MutationImpl { void dump(Formatter *f) const override; // TrackedOp stuff - typedef ceph::shared_ptr Ref; + typedef boost::intrusive_ptr Ref; protected: void _dump(Formatter *f) const; void _dump_op_descriptor_unlocked(ostream& stream) const; }; -typedef ceph::shared_ptr MDRequestRef; +typedef boost::intrusive_ptr MDRequestRef; struct MDSlaveUpdate { diff --git a/src/mds/Server.h b/src/mds/Server.h index 8cea19ceceb..bf6e05bae00 100644 --- a/src/mds/Server.h +++ b/src/mds/Server.h @@ -16,6 +16,7 @@ #define CEPH_MDS_SERVER_H #include "MDSRank.h" +#include "Mutation.h" class OSDMap; class PerfCounters; @@ -28,11 +29,6 @@ class MClientRequest; class MClientReply; class MDLog; -struct MutationImpl; -struct MDRequestImpl; -typedef ceph::shared_ptr MutationRef; -typedef ceph::shared_ptr MDRequestRef; - enum { l_mdss_first = 1000, l_mdss_handle_client_request, diff --git a/src/mds/SimpleLock.h b/src/mds/SimpleLock.h index e5527bfea53..27eae7bebd4 100644 --- a/src/mds/SimpleLock.h +++ b/src/mds/SimpleLock.h @@ -16,6 +16,8 @@ #ifndef CEPH_SIMPLELOCK_H #define CEPH_SIMPLELOCK_H +#include + #include "MDSCacheObject.h" #include "MDSContext.h" @@ -42,8 +44,9 @@ inline const char *get_lock_type_name(int t) { } #include "include/memory.h" + struct MutationImpl; -typedef ceph::shared_ptr MutationRef; +typedef boost::intrusive_ptr MutationRef; extern "C" { #include "locks.h" diff --git a/src/mon/MonOpRequest.h b/src/mon/MonOpRequest.h index f34d4773507..2080b606e56 100644 --- a/src/mon/MonOpRequest.h +++ b/src/mon/MonOpRequest.h @@ -178,7 +178,7 @@ public: return (con && con->get_peer_type() & CEPH_ENTITY_TYPE_MON); } - typedef ceph::shared_ptr Ref; + typedef boost::intrusive_ptr Ref; void set_op_type(op_type_t t) { op_type = t; diff --git a/src/osd/CMakeLists.txt b/src/osd/CMakeLists.txt index c817e724980..681d9fd36ed 100644 --- a/src/osd/CMakeLists.txt +++ b/src/osd/CMakeLists.txt @@ -6,7 +6,6 @@ set(osd_srcs OSD.cc Watch.cc ClassHandler.cc - OpRequest.cc PG.cc PGLog.cc PrimaryLogPG.cc @@ -36,7 +35,7 @@ add_library(osd STATIC ${osd_srcs} $) target_link_libraries(osd ${LEVELDB_LIBRARIES} ${CMAKE_DL_LIBS} ${ALLOC_LIBS}) if(WITH_LTTNG) - add_dependencies(osd oprequest-tp osd-tp pg-tp) + add_dependencies(osd osd-tp pg-tp) endif() if(WITH_LTTNG AND WITH_EVENTTRACE) add_dependencies(osd eventtrace_tp) diff --git a/src/osd/OpRequest.h b/src/osd/OpRequest.h index d26e3972cbc..d92042e50b1 100644 --- a/src/osd/OpRequest.h +++ b/src/osd/OpRequest.h @@ -193,7 +193,7 @@ public: return reqid; } - typedef ceph::shared_ptr Ref; + typedef boost::intrusive_ptr Ref; private: void set_rmw_flags(int flags);