From a17a82c6f425f51cf94f792146463bf5aafcf4c8 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Mon, 8 May 2023 12:47:42 +0000 Subject: [PATCH] common: use signedspan for monotonic ceph clocks The monotonic clocks are commonly used for measuring time deltas, which can be negative. ceph::mono_clock and ceph::coarse_mono_clock currently use unsigned duration types [1]. The difference operators are overloaded in order to ensure that the result is signed [2][3]. However, we still have issues when unsigned timespans are compared. For example, std::condition::wait_for can hang indefinitely due to underflows [4][5]. It ends up using our unsigned type for a negative timespan, which is then compared to std::chrono::duration::zero. In order to avoid such problems, we'll simply use a signed type for monotonic clock durations. With signed timespans, we can no longer assume that time_point::zero() is equal to time_point::min(), so we're updating it accodingly. [1] https://github.com/ceph/ceph/blob/4040f12347a5f48520f8ff2f83065b9ee3a36f68/src/common/ceph_time.h#L285 [2] https://github.com/ceph/ceph/blob/4040f12347a5f48520f8ff2f83065b9ee3a36f68/src/common/ceph_time.h#L345-L380 [3] https://github.com/ceph/ceph/blob/4040f12347a5f48520f8ff2f83065b9ee3a36f68/src/common/ceph_time.h#L466-L487 [4] https://github.com/llvm/llvm-project/blob/91cff8a71872cf49f0c5c9e5510f8065bfefa3c3/libcxx/include/__condition_variable/condition_variable.h#L178 [5] https://github.com/llvm/llvm-project/blob/91cff8a71872cf49f0c5c9e5510f8065bfefa3c3/libcxx/include/__condition_variable/condition_variable.h#L193 Signed-off-by: Lucian Petrut --- src/SimpleRADOSStriper.h | 2 +- src/client/Client.cc | 3 +-- src/common/ceph_time.h | 26 +++++++++---------- src/common/ceph_timer.h | 2 +- .../cache/ObjectCacherObjectDispatch.cc | 2 +- src/mds/MDCache.cc | 6 ++--- src/mds/MDCache.h | 4 +-- src/mon/MgrMonitor.cc | 2 +- src/test/osdc/object_cacher_stress.cc | 8 +++--- 9 files changed, 27 insertions(+), 28 deletions(-) diff --git a/src/SimpleRADOSStriper.h b/src/SimpleRADOSStriper.h index ab60b4dd25958..1ae0a67ca2e59 100644 --- a/src/SimpleRADOSStriper.h +++ b/src/SimpleRADOSStriper.h @@ -118,7 +118,7 @@ private: std::thread lock_keeper; std::condition_variable lock_keeper_cvar; std::mutex lock_keeper_mutex; - time last_renewal = time::min(); + time last_renewal = clock::zero(); std::chrono::milliseconds lock_keeper_interval{2000}; std::chrono::milliseconds lock_keeper_timeout{30000}; std::atomic blocklisted = false; diff --git a/src/client/Client.cc b/src/client/Client.cc index 66e964658ef01..99b9c06c2086b 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -6963,10 +6963,9 @@ void Client::tick() void Client::start_tick_thread() { upkeeper = std::thread([this]() { - using time = ceph::coarse_mono_time; using sec = std::chrono::seconds; - auto last_tick = time::min(); + auto last_tick = clock::zero(); std::unique_lock cl(client_lock); while (!tick_thread_stopped) { diff --git a/src/common/ceph_time.h b/src/common/ceph_time.h index c7d2bb96c769c..6ada4d8944cd1 100644 --- a/src/common/ceph_time.h +++ b/src/common/ceph_time.h @@ -119,11 +119,11 @@ public: } static bool is_zero(const time_point& t) { - return (t == time_point::min()); + return (t == zero()); } static time_point zero() { - return time_point::min(); + return time_point(); } // Allow conversion to/from any clock with the same interface as @@ -222,11 +222,11 @@ public: } static bool is_zero(const time_point& t) { - return (t == time_point::min()); + return (t == zero()); } static time_point zero() { - return time_point::min(); + return time_point(); } static time_t to_time_t(const time_point& t) noexcept { @@ -283,7 +283,7 @@ public: // High-resolution monotonic clock class mono_clock { public: - typedef timespan duration; + typedef signedspan duration; typedef duration::rep rep; typedef duration::period period; typedef std::chrono::time_point time_point; @@ -297,11 +297,11 @@ public: } static bool is_zero(const time_point& t) { - return (t == time_point::min()); + return (t == zero()); } static time_point zero() { - return time_point::min(); + return time_point(); } }; @@ -309,7 +309,7 @@ public: // monotonic clock class coarse_mono_clock { public: - typedef timespan duration; + typedef signedspan duration; typedef duration::rep rep; typedef duration::period period; typedef std::chrono::time_point time_point; @@ -334,11 +334,11 @@ public: } static bool is_zero(const time_point& t) { - return (t == time_point::min()); + return (t == zero()); } static time_point zero() { - return time_point::min(); + return time_point(); } }; @@ -447,11 +447,11 @@ auto ceil(const std::chrono::time_point& timepoint, ceil(timepoint.time_since_epoch(), precision)); } -inline timespan make_timespan(const double d) { - return std::chrono::duration_cast( +inline signedspan make_timespan(const double d) { + return std::chrono::duration_cast( std::chrono::duration(d)); } -inline std::optional maybe_timespan(const double d) { +inline std::optional maybe_timespan(const double d) { return d ? std::make_optional(make_timespan(d)) : std::nullopt; } diff --git a/src/common/ceph_timer.h b/src/common/ceph_timer.h index 16625208024c8..74fb4add85127 100644 --- a/src/common/ceph_timer.h +++ b/src/common/ceph_timer.h @@ -53,7 +53,7 @@ class timer { using sh = bi::set_member_hook>; struct event { - typename TC::time_point t = typename TC::time_point::min(); + typename TC::time_point t = typename TC::zero(); std::uint64_t id = 0; fu2::unique_function f; diff --git a/src/librbd/cache/ObjectCacherObjectDispatch.cc b/src/librbd/cache/ObjectCacherObjectDispatch.cc index 91d720abe90aa..baa8685513da4 100644 --- a/src/librbd/cache/ObjectCacherObjectDispatch.cc +++ b/src/librbd/cache/ObjectCacherObjectDispatch.cc @@ -333,7 +333,7 @@ bool ObjectCacherObjectDispatch::write( m_image_ctx->image_lock.lock_shared(); ObjectCacher::OSDWrite *wr = m_object_cacher->prepare_write( - snapc, data, ceph::real_time::min(), op_flags, *journal_tid); + snapc, data, ceph::real_clock::zero(), op_flags, *journal_tid); m_image_ctx->image_lock.unlock_shared(); ObjectExtent extent(data_object_name(m_image_ctx, object_no), diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc index d4ba1e53a4741..cd9a7aa4c3d58 100644 --- a/src/mds/MDCache.cc +++ b/src/mds/MDCache.cc @@ -6533,7 +6533,7 @@ void MDCache::_truncate_inode(CInode *in, LogSegment *ls) << header.change_attr << " offset: " << header.file_offset << " blen: " << header.block_size << dendl; filer.write(in->ino(), &layout, *snapc, header.file_offset, header.block_size, - data, ceph::real_time::min(), 0, + data, ceph::real_clock::zero(), 0, new C_OnFinisher(new C_IO_MDC_TruncateWriteFinish(this, in, ls, header.block_size), mds->finisher)); @@ -6554,7 +6554,7 @@ void MDCache::_truncate_inode(CInode *in, LogSegment *ls) dout(10) << "_truncate_inode truncate on inode " << *in << dendl; filer.truncate(in->ino(), &layout, *snapc, pi->truncate_size, length, - pi->truncate_seq, ceph::real_time::min(), 0, + pi->truncate_seq, ceph::real_clock::zero(), 0, new C_OnFinisher(new C_IO_MDC_TruncateFinish(this, in, ls), mds->finisher)); } @@ -6610,7 +6610,7 @@ void MDCache::truncate_inode_write_finish(CInode *in, LogSegment *ls, */ uint64_t length = pi->truncate_from - pi->truncate_size + block_size; filer.truncate(in->ino(), &layout, *snapc, pi->truncate_size, length, - pi->truncate_seq, ceph::real_time::min(), 0, + pi->truncate_seq, ceph::real_clock::zero(), 0, new C_OnFinisher(new C_IO_MDC_TruncateFinish(this, in, ls), mds->finisher)); } diff --git a/src/mds/MDCache.h b/src/mds/MDCache.h index 1280fa1d01e44..22441e7257bb0 100644 --- a/src/mds/MDCache.h +++ b/src/mds/MDCache.h @@ -1386,8 +1386,8 @@ class MDCache { std::thread upkeeper; ceph::mutex upkeep_mutex = ceph::make_mutex("MDCache::upkeep_mutex"); ceph::condition_variable upkeep_cvar; - time upkeep_last_trim = time::min(); - time upkeep_last_release = time::min(); + time upkeep_last_trim = clock::zero(); + time upkeep_last_release = clock::zero(); std::atomic upkeep_trim_shutdown{false}; }; diff --git a/src/mon/MgrMonitor.cc b/src/mon/MgrMonitor.cc index 958bf669140cf..d1022d697ee4f 100644 --- a/src/mon/MgrMonitor.cc +++ b/src/mon/MgrMonitor.cc @@ -779,7 +779,7 @@ void MgrMonitor::tick() const auto mgr_tick_period = g_conf().get_val("mgr_tick_period"); - if (last_tick != ceph::coarse_mono_clock::time_point::min() + if (last_tick != ceph::coarse_mono_clock::zero() && (now - last_tick > (mgr_beacon_grace - mgr_tick_period))) { // This case handles either local slowness (calls being delayed // for whatever reason) or cluster election slowness (a long gap diff --git a/src/test/osdc/object_cacher_stress.cc b/src/test/osdc/object_cacher_stress.cc index 096f9b49ebfe6..0bfdd48eb98ba 100644 --- a/src/test/osdc/object_cacher_stress.cc +++ b/src/test/osdc/object_cacher_stress.cc @@ -115,7 +115,7 @@ int stress_test(uint64_t num_ops, uint64_t num_objs, ceph_assert(r == 0); } else { ObjectCacher::OSDWrite *wr = obc.prepare_write(snapc, bl, - ceph::real_time::min(), 0, + ceph::real_clock::zero(), 0, ++journal_tid); wr->extents.push_back(op->extent); lock.lock(); @@ -203,7 +203,7 @@ int correctness_test(uint64_t delay_ns) std::map create_finishers; for (int i = 0; i < 4; ++i) { ObjectCacher::OSDWrite *wr = obc.prepare_write(snapc, zeroes_bl, - ceph::real_time::min(), 0, + ceph::real_clock::zero(), 0, ++journal_tid); ObjectExtent extent(oid, 0, zeroes_bl.length()*i, zeroes_bl.length(), 0); extent.oloc.pool = 0; @@ -222,7 +222,7 @@ int correctness_test(uint64_t delay_ns) ones_bl.append(ones); for (int i = 1<<18; i < 1<<22; i+=1<<18) { ObjectCacher::OSDWrite *wr = obc.prepare_write(snapc, ones_bl, - ceph::real_time::min(), 0, + ceph::real_clock::zero(), 0, ++journal_tid); ObjectExtent extent(oid, 0, i, ones_bl.length(), 0); extent.oloc.pool = 0; @@ -289,7 +289,7 @@ int correctness_test(uint64_t delay_ns) std::cout << "Data (correctly) not available without fetching" << std::endl; ObjectCacher::OSDWrite *verify_wr = obc.prepare_write(snapc, ones_bl, - ceph::real_time::min(), 0, + ceph::real_clock::zero(), 0, ++journal_tid); ObjectExtent verify_extent(oid, 0, (1<<18)+(1<<16), ones_bl.length(), 0); verify_extent.oloc.pool = 0; -- 2.39.5