From 606bbb4bbf3acab474e2406b727fa6c2d47ca606 Mon Sep 17 00:00:00 2001 From: Tony Astolfi Date: Mon, 11 May 2026 12:19:19 -0400 Subject: [PATCH 1/4] Fix various compilation errors on gcc 15 and -fsanitize=address; add eviction pause experimental feature (mostly for A/B benchmarking and ablation tests) --- CMakeLists.txt | 9 ++ conan.lock | 2 +- conanfile.py | 18 ++-- cor.yml | 2 +- src/llfs/api_types.hpp | 8 ++ src/llfs/page_allocator.cpp | 2 + src/llfs/page_cache_slot.cpp | 44 +++++++++- src/llfs/page_cache_slot.hpp | 33 ++++++++ src/llfs/page_cache_slot_pool.cpp | 2 +- src/llfs/page_cache_slot_pool.hpp | 2 + src/llfs/traversal_order.hpp | 132 +---------------------------- src/llfs/volume.test.cpp | 1 + src/llfs/worker_task_fuse_impl.hpp | 102 +++++++++++----------- 13 files changed, 166 insertions(+), 191 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d12c84b0..ea80930c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -12,6 +12,15 @@ project(llfs CXX) include(${COR_CMAKE_INCLUDE_DIR}/common.cmake) include(${CMAKE_BINARY_DIR}/conan_find_requirements.cmake) +#==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +# Disable link-time optimization if the compilation flags are not compatible. +# +if ("${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_DEBUG} ${CMAKE_CXX_FLAGS_RELEASE} ${CMAKE_CXX_FLAGS_RELWITHDEBINFO}" + MATCHES "-fsanitize=address|-fno-lto") + message("\nNDTE: Disabling LTO because of compile flags: ${CMAKE_MATCH_0}\n") + set(CMAKE_INTERPROCEDURAL_OPTIMIZATION FALSE) +endif () + #==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - # Do not attempt to set/fix rpath for binaries. # diff --git a/conan.lock b/conan.lock index 40d9c06e..e43a9b6e 100644 --- a/conan.lock +++ b/conan.lock @@ -1,7 +1,7 @@ { "version": "0.5", "requires": [ - "batteries/0.63.0", + "batteries/0.68.0", "boost/1.88.0", "bzip2/1.0.8", "cli11/2.5.0", diff --git a/conanfile.py b/conanfile.py index 0391e99a..a41ef609 100644 --- a/conanfile.py +++ b/conanfile.py @@ -4,12 +4,11 @@ # See https://www.apache.org/licenses/LICENSE-2.0 for license information. # SPDX short identifier: Apache-2.0 # -#+++++++++++-+-+--+----- --- -- - - - - +import io, os, platform, shlex, subprocess, sys from conan import ConanFile from conan.errors import ConanInvalidConfiguration - -import io, os, platform, shlex, subprocess +from conan.tools.scm import Version class LlfsConan(ConanFile): @@ -37,7 +36,7 @@ class LlfsConan(ConanFile): "shared": False, } - python_requires = "cor_recipe_utils/0.18.2" + python_requires = "cor_recipe_utils/0.19.1" python_requires_extend = "cor_recipe_utils.ConanFileBase" tool_requires = [ @@ -119,20 +118,25 @@ def package_info(self): def package_id(self): return self.cor.package_id_lib_default(self) + #+++++++++++-+-+--+----- --- -- - - - - + def validate_build(self): if self.settings.compiler == "gcc": out_capture = io.StringIO() + from_conf = self.conf.get('tools.build:compiler_executables') cc_name = ( self.buildenv.vars(self).get('CC') or self.buildenv.vars(self).get('CXX') or os.getenv('CC') or os.getenv('CXX') or + (from_conf and from_conf.get('cpp')) or 'gcc' ) self.run(shlex.join([cc_name, '-dumpversion']), stdout=out_capture, shell=True) - actual_compiler_version = out_capture.getvalue().strip() - profile_compiler_version = str(self.settings.compiler.version) - if profile_compiler_version != actual_compiler_version: + actual_compiler_version = Version(out_capture.getvalue().strip()) + profile_compiler_version = Version(str(self.settings.compiler.version)) + + if profile_compiler_version.major != actual_compiler_version.major: raise ConanInvalidConfiguration(f"Compiler version mismatch: actual={actual_compiler_version}" f", expected={profile_compiler_version}") diff --git a/cor.yml b/cor.yml index b6e82d21..2a24d1bf 100644 --- a/cor.yml +++ b/cor.yml @@ -1,3 +1,3 @@ cor: cli: - version: 0.18.2 + version: 0.19.1 diff --git a/src/llfs/api_types.hpp b/src/llfs/api_types.hpp index 775c4042..f93ee2be 100644 --- a/src/llfs/api_types.hpp +++ b/src/llfs/api_types.hpp @@ -37,10 +37,18 @@ BATT_STRONG_TYPEDEF(bool, UseParallelCopy); */ BATT_STRONG_TYPEDEF(u64, TrimDelayByteCount); +/** \brief A number of bytes at the scale of file sizes. + */ +BATT_STRONG_TYPEDEF(i64, FileByteCount); + /** \brief Wrapper for off_t used as an offset. */ BATT_STRONG_TYPEDEF(i64, FileOffset); +/** \brief A file size or number of bytes at the scale of file sizes. + */ +BATT_STRONG_TYPEDEF(i64, FileSize); + /** \brief Wrapper for off_t used as a dirent offset. */ BATT_STRONG_TYPEDEF(off_t, DirentOffset); diff --git a/src/llfs/page_allocator.cpp b/src/llfs/page_allocator.cpp index dd510ede..b92d8472 100644 --- a/src/llfs/page_allocator.cpp +++ b/src/llfs/page_allocator.cpp @@ -20,6 +20,8 @@ #include #include +#include + namespace llfs { u64 PageAllocator::calculate_log_size(u64 physical_page_count, u64 max_attachments) diff --git a/src/llfs/page_cache_slot.cpp b/src/llfs/page_cache_slot.cpp index 4a737b4a..4595c369 100644 --- a/src/llfs/page_cache_slot.cpp +++ b/src/llfs/page_cache_slot.cpp @@ -53,16 +53,21 @@ auto PageCacheSlot::fill(PageId key, PageSize page_size, i64 lru_priority, Exter this->page_size_ = page_size; this->update_latest_use(lru_priority); - auto observed_state = this->state_.fetch_add(kPinCountDelta) + kPinCountDelta; - BATT_CHECK_EQ(observed_state & Self::kOverflowMask, 0); - BATT_CHECK(Self::is_pinned(observed_state)); + if (PageCacheSlot::eviction_pause() == false) { + auto observed_state = this->state_.fetch_add(kPinCountDelta) + kPinCountDelta; + BATT_CHECK_EQ(observed_state & Self::kOverflowMask, 0); + BATT_CHECK(Self::is_pinned(observed_state)); + } this->pool_.metrics().admit_count.add(1); this->pool_.metrics().admit_byte_count.add(page_size); claim.absorb(); - this->add_pinned_bytes(page_size); + if (PageCacheSlot::eviction_pause() == false) { + this->add_pinned_bytes(page_size); + PageCacheSlot::add_pin_count(); + } this->add_ref(); this->set_valid(); @@ -91,6 +96,10 @@ void PageCacheSlot::clear() // bool PageCacheSlot::evict() { + if (PageCacheSlot::eviction_pause()) { + return false; + } + // Use a CAS loop here to guarantee an atomic transition from Valid + Filled (unpinned) state to // Invalid. // @@ -116,6 +125,10 @@ bool PageCacheSlot::evict_if_key_equals(PageId key) { static_assert(std::is_same_vstate_)::value_type, u64>); + if (PageCacheSlot::eviction_pause()) { + return false; + } + // The slot must be pinned in order to read the key, so increase the pin count. // const u64 old_state = this->state_.fetch_add(kPinCountDelta, std::memory_order_acquire); @@ -126,6 +139,7 @@ bool PageCacheSlot::evict_if_key_equals(PageId key) this->add_pinned_bytes(this->page_size_); this->add_ref(); } + PageCacheSlot::add_pin_count(); BATT_CHECK_EQ(observed_state & Self::kOverflowMask, 0); @@ -156,6 +170,8 @@ bool PageCacheSlot::evict_if_key_equals(PageId key) this->add_unpinned_bytes(saved_page_size); this->on_evict_success(nullptr); + PageCacheSlot::add_unpin_count(); + // At this point, we always expect to be going from pinned to unpinned. // In order to successfully evict the slot, we must be holding the only pin, // as guarenteed by the first if statement in the for loop. @@ -172,6 +188,10 @@ bool PageCacheSlot::evict_and_release_pin(ExternalAllocation* reclaim) { static_assert(std::is_same_vstate_)::value_type, u64>); + if (PageCacheSlot::eviction_pause()) { + return false; + } + u64 observed_state = this->state_.load(std::memory_order_acquire); BATT_CHECK(Self::is_pinned(observed_state)); @@ -203,6 +223,8 @@ bool PageCacheSlot::evict_and_release_pin(ExternalAllocation* reclaim) this->add_unpinned_bytes(saved_page_size); this->on_evict_success(reclaim); + PageCacheSlot::add_unpin_count(); + // At this point, we always expect to be going from pinned to unpinned. // In order to successfully evict the slot, we must be holding the only pin, // as guarenteed by the first if statement in the for loop. @@ -267,6 +289,20 @@ void PageCacheSlot::notify_last_ref_released() #endif // LLFS_PAGE_CACHE_SLOT_UPDATE_POOL_REF_COUNT +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// +/*static*/ void PageCacheSlot::add_pin_count() +{ + Pool::Metrics::instance().pin_count.add(1); +} + +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// +/*static*/ void PageCacheSlot::add_unpin_count() +{ + Pool::Metrics::instance().unpin_count.add(1); +} + //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - // void PageCacheSlot::ExternalAllocation::release() noexcept diff --git a/src/llfs/page_cache_slot.hpp b/src/llfs/page_cache_slot.hpp index fa8bd6fa..5efbe1c4 100644 --- a/src/llfs/page_cache_slot.hpp +++ b/src/llfs/page_cache_slot.hpp @@ -269,6 +269,17 @@ class PageCacheSlot //+++++++++++-+-+--+----- --- -- - - - - + /** \brief When set to true, no evictions are allowed. Any pins that happen while evictions are + * paused do nothing. + */ + static std::atomic& eviction_pause() noexcept + { + static std::atomic b_{false}; + return b_; + } + + //+++++++++++-+-+--+----- --- -- - - - - + PageCacheSlot(const PageCacheSlot&) = delete; PageCacheSlot& operator=(const PageCacheSlot&) = delete; @@ -426,6 +437,11 @@ class PageCacheSlot //+++++++++++-+-+--+----- --- -- - - - - private: + static void add_pin_count(); + static void add_unpin_count(); + + //+++++++++++-+-+--+----- --- -- - - - - + /** \brief The implementation of acquire_pin; returns true iff successful. */ bool acquire_pin_impl(PageId key); @@ -561,6 +577,10 @@ BATT_ALWAYS_INLINE inline void PageCacheSlot::remove_ref() // BATT_ALWAYS_INLINE inline void PageCacheSlot::extend_pin() { + if (PageCacheSlot::eviction_pause()) { + return; + } + const auto old_state = this->state_.fetch_add(kPinCountDelta, std::memory_order_relaxed); const auto new_state = old_state + kPinCountDelta; @@ -569,6 +589,8 @@ BATT_ALWAYS_INLINE inline void PageCacheSlot::extend_pin() LLFS_PAGE_CACHE_ASSERT(Self::is_pinned(old_state)) << "This method should never be called in cases where the current pin count might be 0; " "use acquire_pin() instead."; + + PageCacheSlot::add_pin_count(); } //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - @@ -608,6 +630,10 @@ BATT_ALWAYS_INLINE inline i64 PageCacheSlot::get_latest_use() const // BATT_ALWAYS_INLINE inline void PageCacheSlot::release_pin() { + if (PageCacheSlot::eviction_pause()) { + return; + } + // Save the page size while the slot is pinned, so we can update metrics below if necessary. // const usize saved_page_size = this->page_size_; @@ -635,6 +661,8 @@ BATT_ALWAYS_INLINE inline void PageCacheSlot::release_pin() this->remove_ref(); } + + PageCacheSlot::add_unpin_count(); } //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - @@ -686,6 +714,10 @@ BATT_ALWAYS_INLINE inline auto PageCacheSlot::acquire_pin(PageId key, IgnoreKey IgnoreGeneration ignore_generation) -> PinnedRef { + if (PageCacheSlot::eviction_pause()) { + return PinnedRef{this, CallerPromisesTheyAcquiredPinCount{}}; + } + // Increment the pin count. // const auto old_state = this->state_.fetch_add(kPinCountDelta, std::memory_order_acquire); @@ -704,6 +736,7 @@ BATT_ALWAYS_INLINE inline auto PageCacheSlot::acquire_pin(PageId key, IgnoreKey this->add_pinned_bytes(this->page_size_); this->add_ref(); } + PageCacheSlot::add_pin_count(); // If the pin_count > 1 (because of the fetch_add above) and the slot is valid, it is safe to read // the key. If the key doesn't match, release the ref and return failure. diff --git a/src/llfs/page_cache_slot_pool.cpp b/src/llfs/page_cache_slot_pool.cpp index 4704dfe1..d9046010 100644 --- a/src/llfs/page_cache_slot_pool.cpp +++ b/src/llfs/page_cache_slot_pool.cpp @@ -61,7 +61,7 @@ namespace llfs { , name_{std::move(name)} , slot_storage_{new SlotStorage[n_slots]} { - LLFS_LOG_INFO_FIRST_N(10) << "PageCacheSlot::Pool created, n_slots=" << this->n_slots_; + LLFS_VLOG(1) << "PageCacheSlot::Pool created, n_slots=" << this->n_slots_; this->metrics_.total_capacity_allocated.add(this->max_byte_size_.load()); } diff --git a/src/llfs/page_cache_slot_pool.hpp b/src/llfs/page_cache_slot_pool.hpp index 1358ce09..577746bd 100644 --- a/src/llfs/page_cache_slot_pool.hpp +++ b/src/llfs/page_cache_slot_pool.hpp @@ -69,10 +69,12 @@ class PageCacheSlot::Pool : public boost::intrusive_ref_counter /** \brief The total size of all pages that have ever been pinned to the cache. */ FastCountMetric pinned_byte_count{0}; + FastCountMetric pin_count{0}; /** \brief The total size of all pages that have ever been unpinned from the cache. */ FastCountMetric unpinned_byte_count{0}; + FastCountMetric unpin_count{0}; CountMetric total_capacity_allocated{0}; CountMetric total_capacity_freed{0}; diff --git a/src/llfs/traversal_order.hpp b/src/llfs/traversal_order.hpp index 7d9617a1..81975564 100644 --- a/src/llfs/traversal_order.hpp +++ b/src/llfs/traversal_order.hpp @@ -13,137 +13,13 @@ #include // -#include - -#include -#include -#include +#include namespace llfs { -template -struct TraversalItem { - T item; - usize insert_order; - i32 insert_depth; -}; - -//=#=#==#==#===============+=+=+=+=++=++++++++++++++-++-+--+-+----+--------------- - -template -class BreadthFirstOrder -{ - public: - using Item = TraversalItem; - - //+++++++++++-+-+--+----- --- -- - - - - - - bool empty() const noexcept - { - return this->queue_.empty(); - } - - template - void push(Args&&... args) - { - this->queue_.emplace_back(Item{ - T{BATT_FORWARD(args)...}, - this->insert_count_, - this->current_depth_ + 1, - }); - this->insert_count_ += 1; - } - - T pop() - { - Item& next = this->queue_.front(); - this->current_depth_ = next.insert_depth; - - T item = std::move(next.item); - - this->queue_.pop_front(); - - return item; - } - - private: - usize insert_count_ = 0; - i32 current_depth_ = -1; - std::deque queue_; -}; - -//=#=#==#==#===============+=+=+=+=++=++++++++++++++-++-+--+-+----+--------------- - -template -class VanEmdeBoasOrder -{ - public: - using Item = TraversalItem; - - static i32 get_priority(i32 depth) - { - if (depth == 0) { - return 64; - } - - // Find the position of the most significant bit that differs between depth and parent_depth - // (depth - 1); the more leading zeroes (i.e., the lower that bit's position), the _higher_ the - // priority in the heap. - // - const i32 parent_depth = depth - 1; - return __builtin_clz(parent_depth ^ depth); - } - - struct CompareFn { - bool operator()(const Item& l, const Item& r) const - { - i32 l_priority = get_priority(l.insert_depth); - i32 r_priority = get_priority(r.insert_depth); - - return l_priority < r_priority || - (l_priority == r_priority && (l.insert_order > r.insert_order)); - } - }; - - //+++++++++++-+-+--+----- --- -- - - - - - - bool empty() const noexcept - { - return this->heap_.empty(); - } - - template - void push(Args&&... args) - { - this->heap_.push_back(Item{ - T{BATT_FORWARD(args)...}, - this->insert_count_, - this->current_depth_ + 1, - }); - this->insert_count_ += 1; - - std::push_heap(this->heap_.begin(), this->heap_.end(), VanEmdeBoasOrder::CompareFn{}); - } - - T pop() - { - std::pop_heap(this->heap_.begin(), this->heap_.end(), VanEmdeBoasOrder::CompareFn{}); - - Item& next = this->heap_.back(); - this->current_depth_ = next.insert_depth; - - T item = std::move(next.item); - - this->heap_.pop_back(); - - return item; - } - - private: - usize insert_count_ = 0; - i32 current_depth_ = -1; - std::vector heap_; -}; +using batt::algo::BreadthFirstOrder; +using batt::algo::TraversalItem; +using batt::algo::VanEmdeBoasOrder; } //namespace llfs diff --git a/src/llfs/volume.test.cpp b/src/llfs/volume.test.cpp index 2db46fa2..b6dfeabe 100644 --- a/src/llfs/volume.test.cpp +++ b/src/llfs/volume.test.cpp @@ -28,6 +28,7 @@ #include #include +#include #include namespace { diff --git a/src/llfs/worker_task_fuse_impl.hpp b/src/llfs/worker_task_fuse_impl.hpp index d6b01d18..bd57b94c 100644 --- a/src/llfs/worker_task_fuse_impl.hpp +++ b/src/llfs/worker_task_fuse_impl.hpp @@ -44,7 +44,7 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(name); batt::Status push_status = this->work_queue_->push_job( - [this, req, parent, name = std::string{name}, handler = BATT_FORWARD(handler)] { + [this, req, parent, name = std::string{name}, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->lookup(req, parent, name)); }); @@ -61,8 +61,8 @@ class WorkerTaskFuseImpl : public FuseImpl LLFS_VLOG(1) << BATT_THIS_FUNCTION << BATT_INSPECT(req) << BATT_INSPECT(ino) << BATT_INSPECT(nlookup); - batt::Status push_status = - this->work_queue_->push_job([this, req, ino, nlookup, handler = BATT_FORWARD(handler)] { + batt::Status push_status = this->work_queue_->push_job( + [this, req, ino, nlookup, handler = BATT_FORWARD(handler)]() mutable { auto on_scope_exit = batt::finally([&] { BATT_FORWARD(handler)(); }); @@ -85,7 +85,7 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(ino); batt::Status push_status = - this->work_queue_->push_job([this, req, ino, handler = BATT_FORWARD(handler)] { + this->work_queue_->push_job([this, req, ino, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->get_attributes(req, ino)); }); @@ -124,7 +124,7 @@ class WorkerTaskFuseImpl : public FuseImpl }(); batt::Status push_status = this->work_queue_->push_job( - [this, req, ino, attr = *attr, to_set, fh, handler = BATT_FORWARD(handler)] { + [this, req, ino, attr = *attr, to_set, fh, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler) (this->derived_this()->set_attributes(req, ino, &attr, to_set, fh)); }); @@ -142,7 +142,7 @@ class WorkerTaskFuseImpl : public FuseImpl LLFS_VLOG(1) << BATT_THIS_FUNCTION << BATT_INSPECT(req) << BATT_INSPECT(ino); batt::Status push_status = - this->work_queue_->push_job([this, req, ino, handler = BATT_FORWARD(handler)] { + this->work_queue_->push_job([this, req, ino, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->readlink(req, ino)); }); @@ -164,8 +164,9 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(DumpFileMode{mode}) // << BATT_INSPECT(rdev); - batt::Status push_status = this->work_queue_->push_job( - [this, req, parent, name = std::string{name}, mode, rdev, handler = BATT_FORWARD(handler)] { + batt::Status push_status = + this->work_queue_->push_job([this, req, parent, name = std::string{name}, mode, rdev, + handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->make_node(req, parent, name, mode, rdev)); }); @@ -183,8 +184,9 @@ class WorkerTaskFuseImpl : public FuseImpl LLFS_VLOG(1) << BATT_THIS_FUNCTION << BATT_INSPECT(req) << BATT_INSPECT(parent) << BATT_INSPECT(name) << BATT_INSPECT(DumpFileMode{mode}); - batt::Status push_status = this->work_queue_->push_job( - [this, req, parent, name = std::string{name}, mode, handler = BATT_FORWARD(handler)] { + batt::Status push_status = + this->work_queue_->push_job([this, req, parent, name = std::string{name}, mode, + handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->make_directory(req, parent, name, mode)); }); @@ -202,7 +204,7 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(name); batt::Status push_status = this->work_queue_->push_job( - [this, req, parent, name = std::string{name}, handler = BATT_FORWARD(handler)] { + [this, req, parent, name = std::string{name}, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->unlink(req, parent, name)); }); @@ -221,7 +223,7 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(name); batt::Status push_status = this->work_queue_->push_job( - [this, req, parent, name = std::string{name}, handler = BATT_FORWARD(handler)] { + [this, req, parent, name = std::string{name}, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->remove_directory(req, parent, name)); }); @@ -239,9 +241,9 @@ class WorkerTaskFuseImpl : public FuseImpl LLFS_VLOG(1) << BATT_THIS_FUNCTION << BATT_INSPECT(req) << BATT_INSPECT(link) << BATT_INSPECT(parent) << BATT_INSPECT(name); - batt::Status push_status = - this->work_queue_->push_job([this, req, link = std::string{link}, parent, - name = std::string{name}, handler = BATT_FORWARD(handler)] { + batt::Status push_status = this->work_queue_->push_job( + [this, req, link = std::string{link}, parent, name = std::string{name}, + handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->symbolic_link(req, link, parent, name)); }); @@ -262,7 +264,7 @@ class WorkerTaskFuseImpl : public FuseImpl batt::Status push_status = this->work_queue_->push_job( [this, req, parent, name = std::string{name}, newparent, newname = std::string{newname}, - flags, handler = BATT_FORWARD(handler)] { + flags, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler) (this->derived_this()->rename(req, parent, name, newparent, newname, flags)); }); @@ -283,7 +285,7 @@ class WorkerTaskFuseImpl : public FuseImpl batt::Status push_status = this->work_queue_->push_job([this, req, ino, newparent, newname = std::string{newname}, - handler = BATT_FORWARD(handler)] { + handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->hard_link(req, ino, newparent, newname)); }); @@ -302,8 +304,8 @@ class WorkerTaskFuseImpl : public FuseImpl BATT_CHECK_NOT_NULLPTR(fi); - batt::Status push_status = - this->work_queue_->push_job([this, req, ino, fi = *fi, handler = BATT_FORWARD(handler)] { + batt::Status push_status = this->work_queue_->push_job( + [this, req, ino, fi = *fi, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->open(req, ino, fi)); }); @@ -322,7 +324,7 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(size) << BATT_INSPECT(offset) << BATT_INSPECT(fh); batt::Status push_status = this->work_queue_->push_job( - [this, req, ino, size, offset, fh, handler = BATT_FORWARD(handler)] { + [this, req, ino, size, offset, fh, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->read(req, ino, size, offset, fh)); }); @@ -342,7 +344,7 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(fh); batt::Status push_status = this->work_queue_->push_job( - [this, req, ino, buffer, offset, fh, handler = BATT_FORWARD(handler)] { + [this, req, ino, buffer, offset, fh, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->write(req, ino, buffer, offset, fh)); }); @@ -359,8 +361,8 @@ class WorkerTaskFuseImpl : public FuseImpl LLFS_VLOG(1) << BATT_THIS_FUNCTION << BATT_INSPECT(req) << BATT_INSPECT(ino) << BATT_INSPECT(fh); - batt::Status push_status = - this->work_queue_->push_job([this, req, ino, fh, handler = BATT_FORWARD(handler)] { + batt::Status push_status = this->work_queue_->push_job( + [this, req, ino, fh, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->flush(req, ino, fh)); }); @@ -378,8 +380,8 @@ class WorkerTaskFuseImpl : public FuseImpl LLFS_VLOG(1) << BATT_THIS_FUNCTION << BATT_INSPECT(req) << BATT_INSPECT(ino) << BATT_INSPECT(fh); - batt::Status push_status = - this->work_queue_->push_job([this, req, ino, fh, flags, handler = BATT_FORWARD(handler)] { + batt::Status push_status = this->work_queue_->push_job( + [this, req, ino, fh, flags, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->release(req, ino, fh, flags)); }); @@ -398,7 +400,7 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(datasync) << BATT_INSPECT(fh); batt::Status push_status = this->work_queue_->push_job( - [this, req, ino, datasync, fh, handler = BATT_FORWARD(handler)] { + [this, req, ino, datasync, fh, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->fsync(req, ino, datasync, fh)); }); @@ -417,8 +419,8 @@ class WorkerTaskFuseImpl : public FuseImpl BATT_CHECK_NOT_NULLPTR(fi); - batt::Status push_status = - this->work_queue_->push_job([this, req, ino, fi = *fi, handler = BATT_FORWARD(handler)] { + batt::Status push_status = this->work_queue_->push_job( + [this, req, ino, fi = *fi, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->opendir(req, ino, fi)); }); @@ -441,7 +443,7 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(fh); batt::Status push_status = this->work_queue_->push_job( - [this, req, ino, size, off, fh, handler = BATT_FORWARD(handler)] { + [this, req, ino, size, off, fh, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler) (this->derived_this()->readdir(req, ino, size, off, fh)); }); @@ -461,8 +463,8 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(ino) // << BATT_INSPECT(fh); - batt::Status push_status = - this->work_queue_->push_job([this, req, ino, fh, handler = BATT_FORWARD(handler)] { + batt::Status push_status = this->work_queue_->push_job( + [this, req, ino, fh, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->releasedir(req, ino, fh)); }); @@ -484,7 +486,7 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(fh); batt::Status push_status = this->work_queue_->push_job( - [this, req, ino, datasync, fh, handler = BATT_FORWARD(handler)] { + [this, req, ino, datasync, fh, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler) (this->derived_this()->fsyncdir(req, ino, datasync, fh)); }); @@ -504,7 +506,7 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(ino); batt::Status push_status = - this->work_queue_->push_job([this, req, ino, handler = BATT_FORWARD(handler)] { + this->work_queue_->push_job([this, req, ino, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->statfs(req, ino)); }); @@ -526,8 +528,8 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(batt::make_printable(attr)) // << BATT_INSPECT(flags); - batt::Status push_status = - this->work_queue_->push_job([this, req, ino, attr, flags, handler = BATT_FORWARD(handler)] { + batt::Status push_status = this->work_queue_->push_job( + [this, req, ino, attr, flags, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler) (this->derived_this()->set_extended_attribute(req, ino, attr, flags)); }); @@ -549,8 +551,9 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(name) // << BATT_INSPECT(size); - batt::Status push_status = this->work_queue_->push_job( - [this, req, ino, name = std::string{name}, size, handler = BATT_FORWARD(handler)] { + batt::Status push_status = + this->work_queue_->push_job([this, req, ino, name = std::string{name}, size, + handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler) (this->derived_this()->get_extended_attribute(req, ino, name, size)); }); @@ -573,7 +576,7 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(name); batt::Status push_status = this->work_queue_->push_job( - [this, req, ino, name = std::string{name}, handler = BATT_FORWARD(handler)] { + [this, req, ino, name = std::string{name}, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->remove_extended_attribute(req, ino, name)); }); @@ -592,8 +595,8 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(ino) // << BATT_INSPECT(mask); - batt::Status push_status = - this->work_queue_->push_job([this, req, ino, mask, handler = BATT_FORWARD(handler)] { + batt::Status push_status = this->work_queue_->push_job( + [this, req, ino, mask, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->check_access(req, ino, mask)); }); @@ -619,7 +622,7 @@ class WorkerTaskFuseImpl : public FuseImpl batt::Status push_status = this->work_queue_->push_job([this, req, parent, name = std::string{name}, mode, fi = *fi, - handler = BATT_FORWARD(handler)] { + handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->create(req, parent, name, mode, fi)); }); @@ -648,8 +651,9 @@ class WorkerTaskFuseImpl : public FuseImpl BATT_CHECK_NOT_NULLPTR(fi); - batt::Status push_status = this->work_queue_->push_job( - [this, req, ino, cmd, arg, fi, flags, in_buf, out_bufsz, handler = BATT_FORWARD(handler)] { + batt::Status push_status = + this->work_queue_->push_job([this, req, ino, cmd, arg, fi, flags, in_buf, out_bufsz, + handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler) (this->derived_this()->ioctl(req, ino, cmd, arg, fi, flags, in_buf, out_bufsz)); }); @@ -676,7 +680,7 @@ class WorkerTaskFuseImpl : public FuseImpl batt::Status push_status = this->work_queue_->push_job([this, req, ino, bufv, offset, fh, storage = std::move(storage), - handler = BATT_FORWARD(handler)] { + handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler)(this->derived_this()->write_buf(req, ino, bufv, offset, fh)); }); @@ -699,7 +703,7 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(bufv); batt::Status push_status = this->work_queue_->push_job( - [this, req, cookie, ino, offset, bufv, handler = BATT_FORWARD(handler)] { + [this, req, cookie, ino, offset, bufv, handler = BATT_FORWARD(handler)]() mutable { auto on_scope_exit = batt::finally([&] { BATT_FORWARD(handler)(); }); @@ -722,8 +726,8 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(req) // /*<< BATT_INSPECT(batt::make_printable(forgets)) TODO [tastolfi 2023-06-30] */; - batt::Status push_status = - this->work_queue_->push_job([this, req, forgets, handler = BATT_FORWARD(handler)] { + batt::Status push_status = this->work_queue_->push_job( + [this, req, forgets, handler = BATT_FORWARD(handler)]() mutable { auto on_scope_exit = batt::finally([&] { BATT_FORWARD(handler)(); }); @@ -751,7 +755,7 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(fi); batt::Status push_status = this->work_queue_->push_job( - [this, req, ino, mode, offset, length, fi, handler = BATT_FORWARD(handler)] { + [this, req, ino, mode, offset, length, fi, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler) (this->derived_this()->file_allocate(req, ino, mode, offset, length, fi)); }); @@ -775,7 +779,7 @@ class WorkerTaskFuseImpl : public FuseImpl << BATT_INSPECT(fh); batt::Status push_status = this->work_queue_->push_job( - [this, req, ino, size, offset, fh, handler = BATT_FORWARD(handler)] { + [this, req, ino, size, offset, fh, handler = BATT_FORWARD(handler)]() mutable { BATT_FORWARD(handler) (this->derived_this()->readdirplus(req, ino, size, offset, fh)); }); From 36d4a8a5ad5f5e9bea603c4b1b08144222305671 Mon Sep 17 00:00:00 2001 From: Tony Astolfi Date: Tue, 12 May 2026 16:39:24 -0400 Subject: [PATCH 2/4] Disable LTO. --- CMakeLists.txt | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ea80930c..8c78a118 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -13,13 +13,9 @@ include(${COR_CMAKE_INCLUDE_DIR}/common.cmake) include(${CMAKE_BINARY_DIR}/conan_find_requirements.cmake) #==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - -# Disable link-time optimization if the compilation flags are not compatible. +# Disable link-time optimization. (Breaks Apple Silicon and Windows builds) # -if ("${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_DEBUG} ${CMAKE_CXX_FLAGS_RELEASE} ${CMAKE_CXX_FLAGS_RELWITHDEBINFO}" - MATCHES "-fsanitize=address|-fno-lto") - message("\nNDTE: Disabling LTO because of compile flags: ${CMAKE_MATCH_0}\n") - set(CMAKE_INTERPROCEDURAL_OPTIMIZATION FALSE) -endif () +set(CMAKE_INTERPROCEDURAL_OPTIMIZATION FALSE) #==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - # Do not attempt to set/fix rpath for binaries. From 11ea8c0f8659549f8936ba16192a5dfaaba27716 Mon Sep 17 00:00:00 2001 From: Tony Astolfi Date: Tue, 12 May 2026 17:06:33 -0400 Subject: [PATCH 3/4] Upgrade batteries. --- conan.lock | 4 ++-- src/llfs/page_recycler.cpp | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/conan.lock b/conan.lock index e43a9b6e..0a69b01a 100644 --- a/conan.lock +++ b/conan.lock @@ -1,7 +1,7 @@ { "version": "0.5", "requires": [ - "batteries/0.68.0", + "batteries/0.70.0", "boost/1.88.0", "bzip2/1.0.8", "cli11/2.5.0", @@ -26,7 +26,7 @@ "ninja/1.13.2" ], "python_requires": [ - "cor_recipe_utils/0.18.2" + "cor_recipe_utils/0.19.1" ], "overrides": { "boost/[>=1.88.0 <2]": [ diff --git a/src/llfs/page_recycler.cpp b/src/llfs/page_recycler.cpp index 9661169b..9c13d08e 100644 --- a/src/llfs/page_recycler.cpp +++ b/src/llfs/page_recycler.cpp @@ -388,7 +388,8 @@ StatusOr PageRecycler::recycle_pages_depth_0( const usize needed_size = options.insert_grant_size(); BATT_DEBUG_INFO("[PageRecycler::recycle_page_depth_0] waiting for log space; " - << BATT_INSPECT(needed_size) << BATT_INSPECT(this->insert_grant_pool_.size()) + << BATT_INSPECT(this->stop_requested_.load()) << BATT_INSPECT(needed_size) + << BATT_INSPECT(this->insert_grant_pool_.size()) << BATT_INSPECT(options.total_grant_size_for_depth(depth)) << BATT_INSPECT(this->slot_writer_.in_use_size()) << BATT_INSPECT(this->slot_writer_.log_capacity()) From cd8cb87120a08a861d0b567810f5d1c89acc3413 Mon Sep 17 00:00:00 2001 From: Tony Astolfi Date: Tue, 12 May 2026 19:31:12 -0400 Subject: [PATCH 4/4] Fix hang in PageRecyclerTest.CrashRecovery --- conan.lock | 2 +- src/llfs/page_recycler.cpp | 7 ++-- src/llfs/page_recycler.test.cpp | 63 +++++++++++++-------------------- 3 files changed, 30 insertions(+), 42 deletions(-) diff --git a/conan.lock b/conan.lock index 0a69b01a..5e0c520f 100644 --- a/conan.lock +++ b/conan.lock @@ -1,7 +1,7 @@ { "version": "0.5", "requires": [ - "batteries/0.70.0", + "batteries/0.70.1", "boost/1.88.0", "bzip2/1.0.8", "cli11/2.5.0", diff --git a/src/llfs/page_recycler.cpp b/src/llfs/page_recycler.cpp index 9c13d08e..956c58e1 100644 --- a/src/llfs/page_recycler.cpp +++ b/src/llfs/page_recycler.cpp @@ -361,12 +361,13 @@ StatusOr PageRecycler::recycle_pages_depth_0( const Slice& page_ids_in, llfs::slot_offset_type volume_trim_slot) noexcept { constexpr i32 depth = 0; + // Check to make sure request was never seen before. // std::vector page_ids_list(page_ids_in.begin(), page_ids_in.end()); { auto locked_state = this->state_.lock(); - if (!is_page_recycling_allowed(page_ids_in, volume_trim_slot)) { + if (!this->is_page_recycling_allowed(page_ids_in, volume_trim_slot)) { return this->wal_device_->slot_range(LogReadMode::kDurable).upper_bound; } } @@ -474,9 +475,9 @@ StatusOr PageRecycler::recycle_pages(const Slice "specify depth == 0 and grant == nullptr; other values are " "for PageRecycler internal use only."; - return recycle_pages_depth_0(page_ids, volume_trim_slot); + return this->recycle_pages_depth_0(page_ids, volume_trim_slot); } else { - return recycle_pages_depth_n(page_ids, *grant, depth); + return this->recycle_pages_depth_n(page_ids, *grant, depth); } } diff --git a/src/llfs/page_recycler.test.cpp b/src/llfs/page_recycler.test.cpp index 8bf9b630..d9d3db7c 100644 --- a/src/llfs/page_recycler.test.cpp +++ b/src/llfs/page_recycler.test.cpp @@ -230,9 +230,9 @@ class PageRecyclerTest : public ::testing::Test // Returns an ever increasing unique_offset value for page recycler calls. // - slot_offset_type get_and_incr_unique_offset() + slot_offset_type get_next_unique_offset() { - return ++this->unique_offset_; + return this->unique_offset_.fetch_add(1) + 1; } //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - @@ -276,11 +276,7 @@ class PageRecyclerTest : public ::testing::Test // This is to track unique_offset for PageRecycler tests. // - slot_offset_type unique_offset_{0}; - - // This mutex is to ensure serialized access to unique_offset counter. - // - std::mutex recycle_pages_mutex_; + std::atomic unique_offset_{0}; }; class FakePageDeleter : public PageDeleter @@ -381,9 +377,8 @@ class FakePageDeleter : public PageDeleter // Recursively recycle any newly dead pages. If we try to recycle the same page multiple // times, that is OK, since PageIds are never reused. // - std::lock_guard lock(this->test_->recycle_pages_mutex_); result = this->test_->recycler_->recycle_pages(as_slice(dead_pages), - this->test_->get_and_incr_unique_offset(), // + this->test_->get_next_unique_offset(), // &recycle_grant, depth + 1); BATT_REQUIRE_OK(result); @@ -521,9 +516,9 @@ void PageRecyclerTest::run_crash_recovery_test() BATT_DEBUG_INFO("Test - recycle_pages"); { - std::lock_guard lock(this->recycle_pages_mutex_); StatusOr recycle_status = - recycler.recycle_pages(to_recycle, this->get_and_incr_unique_offset()); + recycler.recycle_pages(to_recycle, this->get_next_unique_offset()); + if (!recycle_status.ok()) { failed = true; break; @@ -678,24 +673,21 @@ TEST_F(PageRecyclerTest, DuplicatePageDeletion) << BATT_INSPECT(this->p_mem_log_->driver().get_trim_pos()); BATT_CHECK_EQ(recycle_depth + 1, 1); - { - std::lock_guard lock(this->recycle_pages_mutex_); - BATT_CHECK_OK(this->recycler_->recycle_page(this->fake_page_id_.back(), - this->get_and_incr_unique_offset(), - &recycle_grant, recycle_depth + 1)); - } + + BATT_CHECK_OK(this->recycler_->recycle_page(this->fake_page_id_.back(), + this->get_next_unique_offset(), + &recycle_grant, recycle_depth + 1)); + delete_count.fetch_add(1); BATT_CHECK_OK(continue_count.await_equal(1)); return batt::OkStatus(); })); for (usize i = 0; i < this->fake_page_id_.size() - 2; ++i) { - { - std::lock_guard lock(this->recycle_pages_mutex_); - batt::StatusOr result = this->recycler_->recycle_page( - this->fake_page_id_[i], this->get_and_incr_unique_offset()); - ASSERT_TRUE(result.ok()) << BATT_INSPECT(result); - } + batt::StatusOr result = + this->recycler_->recycle_page(this->fake_page_id_[i], this->get_next_unique_offset()); + + ASSERT_TRUE(result.ok()) << BATT_INSPECT(result); if (i == 0) { LLFS_VLOG(1) << "waiting for " << BATT_INSPECT(this->fake_page_id_[0]); @@ -794,10 +786,8 @@ TEST_F(PageRecyclerTest, DuplicatePageDeletion) // everything else the recycler has queued. // { - std::lock_guard lock(this->recycle_pages_mutex_); - batt::StatusOr result = - this->recycler_->recycle_page(this->fake_page_id_[this->fake_page_id_.size() - 2], - this->get_and_incr_unique_offset()); + batt::StatusOr result = this->recycler_->recycle_page( + this->fake_page_id_[this->fake_page_id_.size() - 2], this->get_next_unique_offset()); } // Wait for all pages to be deleted. @@ -859,9 +849,8 @@ TEST_F(PageRecyclerTest, NoRefreshBatchedPage) // Give some PageIds to delete. // { - std::lock_guard lock(this->recycle_pages_mutex_); - batt::StatusOr result = this->recycler_->recycle_page( - this->fake_page_id_[0], this->get_and_incr_unique_offset()); + batt::StatusOr result = + this->recycler_->recycle_page(this->fake_page_id_[0], this->get_next_unique_offset()); ASSERT_TRUE(result.ok()) << BATT_INSPECT(result); } @@ -874,16 +863,14 @@ TEST_F(PageRecyclerTest, NoRefreshBatchedPage) // refresh the page we just recycled. // for (usize i = 1; i < this->fake_page_id_.size(); ++i) { - { - std::lock_guard lock(this->recycle_pages_mutex_); - batt::StatusOr result = this->recycler_->recycle_page( - this->fake_page_id_[i], this->get_and_incr_unique_offset()); + batt::StatusOr result = + this->recycler_->recycle_page(this->fake_page_id_[i], this->get_next_unique_offset()); + + ASSERT_TRUE(result.ok()) << BATT_INSPECT(result); - ASSERT_TRUE(result.ok()) << BATT_INSPECT(result); - batt::Status flush_status = this->recycler_->await_flush(*result); + batt::Status flush_status = this->recycler_->await_flush(*result); - EXPECT_TRUE(flush_status.ok()) << BATT_INSPECT(flush_status); - } + EXPECT_TRUE(flush_status.ok()) << BATT_INSPECT(flush_status); } this->save_log_snapshot();