From 50f3b30990b5d361bfb72f50ea69c2e426868bdd Mon Sep 17 00:00:00 2001 From: Chris Kennelly CA Date: Wed, 16 Sep 2026 14:04:33 -0700 Subject: [PATCH] Release pageheap_lock while HugePageFiller unbacks memory. HugePageFiller now releases free pages through unback_without_lock_, as HugeCache already does, so allocations and deallocations proceed while the OS is unbacking a range. Since pageheap_lock is dropped for each range, concurrent operations may observe the filler mid-release: * The tracker being released is removed from the filler lists and marked BeingSubreleased(). Put() on it defers list maintenance (and the transition to empty) to the releasing thread, which handles both after ReleaseFree() returns. TryGet() cannot be served from it. * Pending release candidates are marked HugePageTreatmentType::kSubrelease, which excludes them from another release, from treatment selection, and from being freed back to the page heap should a Put() empty them. A tracker emptied while marked is parked on fully_freed_trackers_ instead; HugePageAwareAllocator drains parked trackers after each release and treatment pass, and FetchFullyFreedTracker() skips trackers that are still referenced. * HugePageUnbackedTrackerTreatment::Restore() re-checks fully_freed() after HandleReleaseFree(), which now drops the lock, and its selection skips trackers held by an in-flight release. FreePagesInPartialAllocs() is now a maintained counter rather than a derivation from list sizes, which would be transiently wrong while a tracker is off the lists. Add unit tests and fuzzer support for reentrant Put(), TryGet(), ReleasePages() and TreatHugepageTrackers() during a release, and a multi-threaded release stress test. PiperOrigin-RevId: 982712772 --- tcmalloc/BUILD | 3 +- tcmalloc/CMakeLists.txt | 3 +- tcmalloc/huge_page_aware_allocator.h | 10 +- tcmalloc/huge_page_aware_allocator_fuzz.cc | 16 +- tcmalloc/huge_page_filler.h | 287 +++++++----- tcmalloc/huge_page_filler_fuzz.cc | 183 ++++++-- tcmalloc/huge_page_filler_test.cc | 500 ++++++++++++++++++--- tcmalloc/huge_page_options.h | 1 + tcmalloc/huge_page_tracker.h | 12 + tcmalloc/huge_page_treatment.h | 9 + 10 files changed, 808 insertions(+), 216 deletions(-) diff --git a/tcmalloc/BUILD b/tcmalloc/BUILD index 227e05643..14903a65a 100644 --- a/tcmalloc/BUILD +++ b/tcmalloc/BUILD @@ -1057,6 +1057,7 @@ cc_test( "//tcmalloc/internal:pageflags", "//tcmalloc/internal:range_tracker", "//tcmalloc/internal:residency", + "//tcmalloc/internal:scoped_allow_allocation", "//tcmalloc/internal:system_allocator", "//tcmalloc/testing:testutil", "@com_github_google_benchmark//:benchmark", @@ -1066,11 +1067,11 @@ cc_test( "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/container:flat_hash_set", "@com_google_absl//absl/flags:flag", + "@com_google_absl//absl/functional:any_invocable", "@com_google_absl//absl/random", "@com_google_absl//absl/status", "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:str_format", - "@com_google_absl//absl/synchronization", "@com_google_absl//absl/time", "@com_google_googletest//:gtest_main", ], diff --git a/tcmalloc/CMakeLists.txt b/tcmalloc/CMakeLists.txt index bcb3361e8..e54fc4eda 100644 --- a/tcmalloc/CMakeLists.txt +++ b/tcmalloc/CMakeLists.txt @@ -1229,6 +1229,7 @@ tcmalloc_cc_test( "GTest::gtest_main" "GTest::gmock_main" "GTest::gmock" + "absl::any_invocable" "absl::base" "absl::core_headers" "absl::flags" @@ -1241,7 +1242,6 @@ tcmalloc_cc_test( "absl::status" "absl::str_format" "absl::strings" - "absl::synchronization" "absl::time" "benchmark::benchmark" "tcmalloc::common_8k_pages" @@ -1252,6 +1252,7 @@ tcmalloc_cc_test( "tcmalloc::internal_pageflags" "tcmalloc::internal_range_tracker" "tcmalloc::internal_residency" + "tcmalloc::internal_scoped_allow_allocation" "tcmalloc::internal_system_allocator" "tcmalloc::tcmalloc" "tcmalloc::testing_testutil" diff --git a/tcmalloc/huge_page_aware_allocator.h b/tcmalloc/huge_page_aware_allocator.h index 4b33b02e6..62a9be023 100644 --- a/tcmalloc/huge_page_aware_allocator.h +++ b/tcmalloc/huge_page_aware_allocator.h @@ -461,8 +461,8 @@ class HugePageAwareAllocator final : public PageAllocatorInterface { void ReleaseHugepage(FillerType::Tracker* pt) ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); - // Returns hugepages that the filler emptied while it did not hold - // pageheap_lock (during TreatHugepageTrackers) to the cache. + // Returns hugepages that the filler emptied while a release or treatment + // had pageheap_lock dropped (see HugePageFiller::FetchFullyFreedTracker). void DrainFreedTrackers() ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); // Return an allocation from a single hugepage. void DeleteFromHugepage(FillerType::Tracker* pt, Range r, bool might_abandon, @@ -490,8 +490,8 @@ inline HugePageAwareAllocator::HugePageAwareAllocator( unback_without_lock_(*this), collapse_(*this), set_anon_vma_name_(*this), - filler_(tag_, unback_, unback_without_lock_, collapse_, - set_anon_vma_name_, forwarder_.subrelease_unbacked_hugepages()), + filler_(tag_, unback_without_lock_, collapse_, set_anon_vma_name_, + forwarder_.subrelease_unbacked_hugepages()), regions_(options.use_huge_region_more_often), tracker_allocator_(forwarder_.arena()), region_allocator_(forwarder_.arena()), @@ -1050,6 +1050,7 @@ inline Length HugePageAwareAllocator::ReleaseAtLeastNPages( forwarder_.filler_skip_subrelease_long_interval()}, forwarder_.release_partial_alloc_pages(), /*hit_limit*/ false); + DrainFreedTrackers(); } } @@ -1254,6 +1255,7 @@ HugePageAwareAllocator::ReleaseAtLeastNPagesBreakingHugepages( released += filler_.ReleasePages(n - released, SkipSubreleaseIntervals{}, /*release_partial_alloc_pages=*/false, /*hit_limit=*/true); + DrainFreedTrackers(); info_.RecordRelease(n, released, reason); return released; diff --git a/tcmalloc/huge_page_aware_allocator_fuzz.cc b/tcmalloc/huge_page_aware_allocator_fuzz.cc index 9b8076231..f13db457c 100644 --- a/tcmalloc/huge_page_aware_allocator_fuzz.cc +++ b/tcmalloc/huge_page_aware_allocator_fuzz.cc @@ -485,8 +485,8 @@ struct State { if (tcmalloc::tcmalloc_internal::pageheap_lock.IsHeld()) { // This permits a slight degree of nondeterminism when linked against // TCMalloc for the real memory allocator, as a background thread could - // also be holding the lock. Nevertheless, HPAA doesn't make it clear - // when we are releasing with/without the pageheap_lock. + // also be holding the lock. HugeCache and HugePageFiller release with + // the lock dropped, HugeRegion does not. // // TODO(b/73749855): When all release paths unconditionally release the // lock, remove this check and take the lock for an instant to ensure it @@ -713,11 +713,15 @@ void GatherAndCheckStats::Perform(State& state) const { PageHeapSpinLockHolder l; stats = state.allocator.stats(); } - uint64_t used_bytes = + const uint64_t used_bytes = stats.system_bytes - stats.free_bytes - stats.unmapped_bytes; - TC_CHECK_EQ(used_bytes, - state.allocated.in_bytes() + - state.allocator.forwarder().pending_release_.in_bytes()); + // While a release has pageheap_lock dropped, HugeCache has already removed + // the range from its size (so it appears used) whereas HugePageFiller keeps + // the pages free until unback succeeds. Outside of a release, the two agree. + const uint64_t pending_bytes = + state.allocator.forwarder().pending_release_.in_bytes(); + TC_CHECK_GE(used_bytes, state.allocated.in_bytes()); + TC_CHECK_LE(used_bytes, state.allocated.in_bytes() + pending_bytes); } void GatherSpanStats::Perform(State& state) const { diff --git a/tcmalloc/huge_page_filler.h b/tcmalloc/huge_page_filler.h index 4aaf65967..c9cdae7aa 100644 --- a/tcmalloc/huge_page_filler.h +++ b/tcmalloc/huge_page_filler.h @@ -702,7 +702,7 @@ template class HugePageFiller { public: explicit HugePageFiller( - MemoryTag tag, MemoryModifyFunction& unback ABSL_ATTRIBUTE_LIFETIME_BOUND, + MemoryTag tag, MemoryModifyFunction& unback_without_lock ABSL_ATTRIBUTE_LIFETIME_BOUND, MemoryModifyFunction& collapse ABSL_ATTRIBUTE_LIFETIME_BOUND, MemoryTagFunction& set_anon_vma_name ABSL_ATTRIBUTE_LIFETIME_BOUND, @@ -710,7 +710,6 @@ class HugePageFiller { HugePageFiller( Clock clock, MemoryTag tag, - MemoryModifyFunction& unback ABSL_ATTRIBUTE_LIFETIME_BOUND, MemoryModifyFunction& unback_without_lock ABSL_ATTRIBUTE_LIFETIME_BOUND, MemoryModifyFunction& collapse ABSL_ATTRIBUTE_LIFETIME_BOUND, MemoryTagFunction& set_anon_vma_name ABSL_ATTRIBUTE_LIFETIME_BOUND, @@ -739,6 +738,10 @@ class HugePageFiller { // Marks r as usable by new allocations into *pt; returns pt if that hugepage // is now empty (nullptr otherwise.) // + // If pt became empty while another thread had dropped pageheap_lock to + // release its free pages (see ReleaseFreeFromTracker()) or to treat it, pt + // is retained and made available via FetchFullyFreedTracker() instead. + // // REQUIRES: pt is owned by this object (has been Contribute()), and // {pt, Range{p, n}} was the result of a previous TryGet. TrackerType* absl_nullable Put(TrackerType* absl_nonnull pt, Range r, @@ -751,6 +754,9 @@ class HugePageFiller { void Contribute(TrackerType* absl_nonnull pt TCMALLOC_CAPTURED_BY_THIS, bool donated, SpanAllocInfo span_alloc_info); + // Returns an empty tracker that Put() could not return to its caller (see + // above), or nullptr if there is none. Trackers still referenced by an + // in-flight release or treatment on another thread are not returned. TrackerType* absl_nullable FetchFullyFreedTracker() ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); @@ -878,7 +884,8 @@ class HugePageFiller { ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); // Utility function to release free pages from a given `page_tracker` - // and handle accounting. + // and handle accounting. Temporarily drops pageheap_lock; on return, + // page_tracker may be empty (and parked for FetchFullyFreedTracker()). Length HandleReleaseFree(PageTracker* page_tracker) ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); @@ -984,6 +991,9 @@ class HugePageFiller { // n_used_partial_released_ is the number of pages which have been allocated // from the hugepages in the set regular_alloc_partial_released. Length n_used_partial_released_[AccessDensityPrediction::kPredictionCounts]; + // n_free_partial_released_ is the number of free pages that have not been + // released on the hugepages in the set regular_alloc_partial_released. + Length n_free_partial_released_; // RemoveFromFillerList pt from the appropriate PageTrackerList. void RemoveFromFillerList(TrackerType* absl_nonnull pt); @@ -1045,11 +1055,28 @@ class HugePageFiller { size_t tracker_start); // Release desired pages from the page trackers in candidates. Returns the - // number of pages released. + // number of pages released. Temporarily drops pageheap_lock. Length ReleaseCandidates(absl::Span candidates, Length target) ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); + // Releases the free pages of pt (which must be on a filler list) to the OS + // and returns the number of pages released. + // + // pageheap_lock is dropped for each call into unback_without_lock_. While + // it is dropped, pt is kept off the filler lists, so TryGet() cannot hand + // out pages whose release is in flight, and pt->BeingSubreleased() is set, + // so concurrent Put()s on pt leave list maintenance to us. If pt became + // empty in the meantime, it is parked for FetchFullyFreedTracker(). + Length ReleaseFreeFromTracker(TrackerType* absl_nonnull pt) + ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); + + // Handles pt (off the filler lists) having become empty. Returns pt if it + // can be returned to the caller, or nullptr if pt->DontFreeTracker() and pt + // was parked on fully_freed_trackers_ instead. + TrackerType* absl_nullable HandleEmptyTracker(TrackerType* absl_nonnull pt) + ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); + HugeLength size_; Length pages_allocated_[AccessDensityPrediction::kPredictionCounts]; @@ -1070,8 +1097,6 @@ class HugePageFiller { Clock clock_; const MemoryTag tag_; - // TODO(b/73749855): Remove remaining uses of unback_. - MemoryModifyFunction& unback_; MemoryModifyFunction& unback_without_lock_; MemoryModifyFunction& collapse_; MemoryTagFunction& set_anon_vma_name_; @@ -1083,27 +1108,24 @@ class HugePageFiller { template inline HugePageFiller::HugePageFiller( - MemoryTag tag, MemoryModifyFunction& unback, - MemoryModifyFunction& unback_without_lock, MemoryModifyFunction& collapse, - MemoryTagFunction& set_anon_vma_name, + MemoryTag tag, MemoryModifyFunction& unback_without_lock, + MemoryModifyFunction& collapse, MemoryTagFunction& set_anon_vma_name, SubreleaseUnbackedMode subrelease_unbacked_mode) : HugePageFiller(Clock{.now = absl::base_internal::CycleClock::Now, .freq = absl::base_internal::CycleClock::Frequency}, - tag, unback, unback_without_lock, collapse, - set_anon_vma_name, subrelease_unbacked_mode) {} + tag, unback_without_lock, collapse, set_anon_vma_name, + subrelease_unbacked_mode) {} // For testing with mock clock template inline HugePageFiller::HugePageFiller( - Clock clock, MemoryTag tag, MemoryModifyFunction& unback, - MemoryModifyFunction& unback_without_lock, MemoryModifyFunction& collapse, - MemoryTagFunction& set_anon_vma_name, + Clock clock, MemoryTag tag, MemoryModifyFunction& unback_without_lock, + MemoryModifyFunction& collapse, MemoryTagFunction& set_anon_vma_name, SubreleaseUnbackedMode subrelease_unbacked_mode) : size_(NHugePages(0)), fillerstats_tracker_(clock, absl::Minutes(10), absl::Minutes(5)), clock_(clock), tag_(tag), - unback_(unback), unback_without_lock_(unback_without_lock), collapse_(collapse), set_anon_vma_name_(set_anon_vma_name), @@ -1324,7 +1346,13 @@ void HugePageFiller::PrintLifetimeHistoInPbtxt( template inline TrackerType* HugePageFiller::Put( TrackerType* pt, Range r, SpanAllocInfo span_alloc_info) { - RemoveFromFillerList(pt); + // If another thread is releasing pt's free pages with pageheap_lock dropped + // (ReleaseFreeFromTracker()), pt is not on any list and that thread owns + // pt's list membership and its transition to empty. + const bool being_subreleased = pt->BeingSubreleased(); + if (!being_subreleased) { + RemoveFromFillerList(pt); + } pt->Put(r, span_alloc_info); if (pt->HasDenseSpans()) { TC_ASSERT_GE(pages_allocated_[AccessDensityPrediction::kDense], r.n); @@ -1334,55 +1362,73 @@ inline TrackerType* HugePageFiller::Put( pages_allocated_[AccessDensityPrediction::kSparse] -= r.n; } + if (being_subreleased) { + UpdateFillerStatsTracker(); + return nullptr; + } + if (pt->longest_free_range() == kPagesPerHugePage) { - TC_ASSERT_EQ(pt->nallocs(), 0); - --size_; - if (pt->released()) { + return HandleEmptyTracker(pt); + } + AddToFillerList(pt); + UpdateFillerStatsTracker(); + return nullptr; +} + +template +inline TrackerType* HugePageFiller::HandleEmptyTracker( + TrackerType* pt) { + TC_ASSERT(pt->empty()); + TC_ASSERT_EQ(pt->nallocs(), 0); + TC_ASSERT(!pt->BeingSubreleased()); + --size_; + if (pt->released()) { #ifndef TCMALLOC_INTERNAL_LEGACY_LOCKING - const Length free_pages = kPagesPerHugePage; + const Length free_pages = kPagesPerHugePage; #else - const Length free_pages = pt->free_pages(); + const Length free_pages = pt->free_pages(); #endif - const Length released_pages = pt->released_pages(); - TC_ASSERT_GE(free_pages, released_pages); - TC_ASSERT_GE(unmapped_, released_pages); - unmapped_ -= released_pages; - - if (free_pages > released_pages) { - // pt is partially released. As the rest of the hugepage-aware - // allocator works in terms of whole hugepages, we need to release the - // rest of the hugepage. This simplifies subsequent accounting by - // allowing us to work with hugepage-granularity, rather than needing to - // retain pt's state indefinitely. - bool success = - unback_without_lock_(HugeRange(pt->location(), NHugePages(1))) - .success; - - if (ABSL_PREDICT_TRUE(success)) { - unmapping_unaccounted_ += free_pages - released_pages; - } + const Length released_pages = pt->released_pages(); + TC_ASSERT_GE(free_pages, released_pages); + TC_ASSERT_GE(unmapped_, released_pages); + unmapped_ -= released_pages; + + if (free_pages > released_pages) { + // pt is partially released. As the rest of the hugepage-aware + // allocator works in terms of whole hugepages, we need to release the + // rest of the hugepage. This simplifies subsequent accounting by + // allowing us to work with hugepage-granularity, rather than needing to + // retain pt's state indefinitely. + bool success = + unback_without_lock_(HugeRange(pt->location(), NHugePages(1))) + .success; + + if (ABSL_PREDICT_TRUE(success)) { + unmapping_unaccounted_ += free_pages - released_pages; } } + } - if (pt->was_released()) { - pt->set_was_released(/*status=*/false); - if (pt->HasDenseSpans()) { - --n_was_released_[AccessDensityPrediction::kDense]; - } else { - --n_was_released_[AccessDensityPrediction::kSparse]; - } + if (pt->was_released()) { + pt->set_was_released(/*status=*/false); + if (pt->HasDenseSpans()) { + --n_was_released_[AccessDensityPrediction::kDense]; + } else { + --n_was_released_[AccessDensityPrediction::kSparse]; } + } - if (!pt->DontFreeTracker()) { - RecordLifetime(pt); - UpdateFillerStatsTracker(); - if (pt->GetTagState().sampled_for_tagging) { - // Set the default region name if the tracked was sampled. - pt->SetAnonVmaName(set_anon_vma_name_, /*name=*/std::nullopt); - } - return pt; + if (!pt->DontFreeTracker()) { + RecordLifetime(pt); + UpdateFillerStatsTracker(); + if (pt->GetTagState().sampled_for_tagging) { + // Set the default region name if the tracked was sampled. + pt->SetAnonVmaName(set_anon_vma_name_, /*name=*/std::nullopt); } + return pt; } + // Park pt on fully_freed_trackers_ until the operation that set + // DontFreeTracker() has completed. AddToFillerList(pt); UpdateFillerStatsTracker(); return nullptr; @@ -1429,14 +1475,26 @@ inline int HugePageFiller::SelectCandidates( // with the release, and we might collapse the pages that have been recently // released. if (pt.BeingCollapsed()) return; + // Likewise if a treatment pass has selected the tracker and may still be + // operating on it with pageheap_lock dropped, or if another in-flight + // ReleaseCandidates() already holds it as a candidate. + if (pt.DontFreeTracker(HugePageTreatmentType::kCollapse) || + pt.DontFreeTracker(HugePageTreatmentType::kSubrelease)) { + return; + } // If we have few candidates, we can avoid creating a heap. // // In ReleaseCandidates(), we unconditionally sort the list and linearly // iterate through it--rather than pop_heap repeatedly--so we only need the // heap for creating a bounded-size priority queue. + // + // Selected candidates are marked kSubrelease: ReleaseCandidates() drops + // pageheap_lock while releasing each of them, so the others must not be + // freed (or selected by another release or treatment) in the meantime. if (current_candidates < candidates.size()) { candidates[current_candidates] = &pt; + pt.SetDontFreeTracker(HugePageTreatmentType::kSubrelease); current_candidates++; if (current_candidates == candidates.size()) { @@ -1454,7 +1512,10 @@ inline int HugePageFiller::SelectCandidates( std::pop_heap(candidates.begin(), candidates.begin() + current_candidates, CompareForSubrelease); + candidates[current_candidates - 1]->ClearDontFreeTracker( + HugePageTreatmentType::kSubrelease); candidates[current_candidates - 1] = &pt; + pt.SetDontFreeTracker(HugePageTreatmentType::kSubrelease); std::push_heap(candidates.begin(), candidates.begin() + current_candidates, CompareForSubrelease); }; @@ -1471,46 +1532,27 @@ inline Length HugePageFiller::ReleaseCandidates( Length total_released; HugeLength total_broken = NHugePages(0); -#ifndef NDEBUG - Length last; -#endif - for (int i = 0; i < candidates.size() && total_released < target; i++) { - TrackerType* best = candidates[i]; + for (TrackerType* best : candidates) { TC_ASSERT_NE(best, nullptr); + TC_ASSERT(best->DontFreeTracker(HugePageTreatmentType::kSubrelease)); + if (total_released >= target) { + best->ClearDontFreeTracker(HugePageTreatmentType::kSubrelease); + continue; + } - // Verify that we have pages that we can release. - TC_ASSERT_NE(best->free_pages(), Length(0)); - // TODO(b/73749855): This assertion may need to be relaxed if we release - // the pageheap_lock here. A candidate could change state with another - // thread while we have the lock released for another candidate. - TC_ASSERT_GT(best->free_pages(), best->released_pages()); - -#ifndef NDEBUG - // Double check that our sorting criteria were applied correctly. - TC_ASSERT_LE(last, best->used_pages()); - last = best->used_pages(); -#endif + // ReleaseFreeFromTracker() drops pageheap_lock, so best may have changed + // since it was selected: concurrent Put()s may have emptied it (it is + // then parked on fully_freed_trackers_, kept alive by kSubrelease) and + // concurrent TryGet()s may have consumed its free pages. + if (best->fully_freed() || best->free_pages() <= best->released_pages()) { + best->ClearDontFreeTracker(HugePageTreatmentType::kSubrelease); + continue; + } if (best->unbroken()) { ++total_broken; } - RemoveFromFillerList(best); - Length ret = best->ReleaseFree(unback_); - unmapped_ += ret; - TC_ASSERT_GE(unmapped_, best->released_pages()); - total_released += ret; - AddToFillerList(best); - // If the candidate we just released from previously had was_released set, - // clear it. was_released is tracked only for pages that aren't in - // released state. - if (best->was_released() && best->released()) { - best->set_was_released(/*status=*/false); - if (best->HasDenseSpans()) { - --n_was_released_[AccessDensityPrediction::kDense]; - } else { - --n_was_released_[AccessDensityPrediction::kSparse]; - } - } + total_released += ReleaseFreeFromTracker(best); } subrelease_stats_.num_pages_subreleased += total_released; @@ -1525,13 +1567,48 @@ inline Length HugePageFiller::ReleaseCandidates( return total_released; } +template +inline Length HugePageFiller::ReleaseFreeFromTracker( + TrackerType* pt) { + TC_ASSERT(!pt->fully_freed()); + TC_ASSERT(!pt->BeingSubreleased()); + RemoveFromFillerList(pt); + pt->SetDontFreeTracker(HugePageTreatmentType::kSubrelease); + pt->SetBeingSubreleased(true); + const Length released = pt->ReleaseFree(unback_without_lock_); + pt->SetBeingSubreleased(false); + unmapped_ += released; + TC_ASSERT_GE(unmapped_, pt->released_pages()); + + if (pt->empty()) { + // Concurrent Put()s emptied pt while pageheap_lock was dropped. Since we + // still hold kSubrelease, pt is parked rather than returned. + TrackerType* ret = HandleEmptyTracker(pt); + TC_ASSERT_EQ(ret, nullptr); + (void)ret; + } else { + AddToFillerList(pt); + // If the tracker we just released from previously had was_released set, + // clear it. was_released is tracked only for pages that aren't in + // released state. + if (pt->was_released() && pt->released()) { + pt->set_was_released(/*status=*/false); + if (pt->HasDenseSpans()) { + --n_was_released_[AccessDensityPrediction::kDense]; + } else { + --n_was_released_[AccessDensityPrediction::kSparse]; + } + } + } + pt->ClearDontFreeTracker(HugePageTreatmentType::kSubrelease); + return released; +} + template inline Length HugePageFiller::FreePagesInPartialAllocs() const { - return regular_alloc_partial_released_.sparse.size().in_pages() + - regular_alloc_partial_released_.dense.size().in_pages() + - regular_alloc_released_.sparse.size().in_pages() + - regular_alloc_released_.dense.size().in_pages() - - used_pages_in_any_subreleased() - unmapped_pages(); + // Hugepages in regular_alloc_released_ have no free pages that are not + // already released, so only regular_alloc_partial_released_ contributes. + return n_free_partial_released_; } template @@ -1932,12 +2009,9 @@ inline void HugePageFiller::TreatHugepageTrackers( template inline Length HugePageFiller::HandleReleaseFree( PageTracker* tracker) { - RemoveFromFillerList(tracker); - Length released_length = tracker->ReleaseFree(unback_); + Length released_length = ReleaseFreeFromTracker(tracker); subrelease_stats_.total_pages_subreleased += released_length; - unmapped_ += released_length; unmapping_unaccounted_ += released_length; - AddToFillerList(tracker); return released_length; } @@ -2468,6 +2542,7 @@ inline size_t HugePageFiller::ListFor( template inline void HugePageFiller::RemoveFromFillerList(TrackerType* pt) { + TC_ASSERT(!pt->BeingSubreleased()); if (pt->donated()) { Length longest = pt->longest_free_range(); TC_ASSERT_LT(longest, kPagesPerHugePage); @@ -2492,23 +2567,28 @@ inline void HugePageFiller::RemoveFromFillerList(TrackerType* pt) { regular_alloc_partial_released_.Remove(pt, i, type); TC_ASSERT_GE(n_used_partial_released_[type], pt->used_pages()); n_used_partial_released_[type] -= pt->used_pages(); + const Length free_unreleased = pt->free_pages() - pt->released_pages(); + TC_ASSERT_GE(n_free_partial_released_, free_unreleased); + n_free_partial_released_ -= free_unreleased; } } template inline TrackerType* absl_nullable HugePageFiller::FetchFullyFreedTracker() { - if (fully_freed_trackers_.empty()) { - return nullptr; + for (TrackerType* pt : fully_freed_trackers_) { + // Skip trackers that a release or treatment pass, which has dropped + // pageheap_lock, still references. + if (pt->DontFreeTracker()) continue; + fully_freed_trackers_.remove(pt); + return pt; } - - TrackerType* pt = fully_freed_trackers_.first(); - fully_freed_trackers_.remove(pt); - return pt; + return nullptr; } template inline void HugePageFiller::AddToFillerList(TrackerType* pt) { + TC_ASSERT(!pt->BeingSubreleased()); Length longest = pt->longest_free_range(); TC_ASSERT_LE(longest, kPagesPerHugePage); @@ -2540,6 +2620,7 @@ inline void HugePageFiller::AddToFillerList(TrackerType* pt) { } else { regular_alloc_partial_released_.Add(pt, i, type); n_used_partial_released_[type] += pt->used_pages(); + n_free_partial_released_ += pt->free_pages() - pt->released_pages(); } } diff --git a/tcmalloc/huge_page_filler_fuzz.cc b/tcmalloc/huge_page_filler_fuzz.cc index e6f46102b..9ed921438 100644 --- a/tcmalloc/huge_page_filler_fuzz.cc +++ b/tcmalloc/huge_page_filler_fuzz.cc @@ -370,22 +370,15 @@ struct State { unback(*this), collapse(*this), filler(Clock{.now = mock_clock, .freq = freq}, MemoryTag::kNormal, - unback, unback, collapse, set_anon_vma_name, - subrelease_unbacked_mode) { + unback, collapse, set_anon_vma_name, subrelease_unbacked_mode) { fake_clock = 0; output.resize(1 << 20); - // To avoid reentrancy during unback, reserve space in released_set. We - // have at most num_instructions allocations, for at most kPagesPerHugePage - // pages each, that we can track the released status of. - // - // TODO(b/73749855): Releasing the pageheap_lock during ReleaseFree will - // eliminate the need for this. - released_set.reserve(kPagesPerHugePage.raw_num() * num_instructions); + // Both unback and collapse are invoked with pageheap_lock dropped. Run + // pending reentrant instructions there to interleave other operations with + // the in-flight release or treatment. auto release_callback = [this]() { - if (tcmalloc::tcmalloc_internal::pageheap_lock.IsHeld()) { - return; - } + TC_CHECK(!tcmalloc::tcmalloc_internal::pageheap_lock.IsHeld()); if (reentrant_stack.empty()) { return; } @@ -407,7 +400,12 @@ struct State { } ~State() { - // Shut down, confirm filler is empty. + // Shut down, confirm filler is empty. Every release and treatment drains + // the trackers it emptied, so none should be parked here. Unbacking the + // remainder of partially released hugepages below drops pageheap_lock, so + // discard any remaining reentrant instructions first: they would mutate + // allocs while it is being iterated. + reentrant_stack.clear(); CHECK_EQ(released_set.size(), filler.unmapped_pages().raw_num()); for (auto& [pt, v] : allocs) { for (size_t i = 0, n = v.size(); i < n; ++i) { @@ -430,6 +428,29 @@ struct State { } } + // Fetches and deletes the trackers that were emptied while a release or + // treatment had pageheap_lock dropped. Their released pages were already + // discounted from the filler's accounting when they were emptied. + void DrainFullyFreedTrackers() { + while (true) { + PageTracker* pt; + { + PageHeapSpinLockHolder l; + pt = filler.FetchFullyFreedTracker(); + } + if (pt == nullptr) { + return; + } + HugePage hp = pt->location(); + for (PageId p = hp.first_page(), + end = hp.first_page() + kPagesPerHugePage; + p != end; ++p) { + released_set.erase(p); + } + delete pt; + } + } + SubreleaseUnbackedMode subrelease_unbacked_mode; bool unback_success = true; bool collapse_success = true; @@ -457,20 +478,30 @@ struct State { std::string output; }; -MemoryModifyStatus MockUnback::operator()(Range r) { - if (release_callback_) { - release_callback_(); - } - if (!state_.unback_success) { - return {.success = false, .error_number = 0}; - } - - PageId end = r.p + r.n; - for (; r.p != end; ++r.p) { - state_.released_set.insert(r.p); +MemoryModifyStatus MockUnback::operator()(Range r) + ABSL_NO_THREAD_SAFETY_ANALYSIS { + // Models HugePageAwareAllocator::UnbackWithoutLock. + TC_CHECK(pageheap_lock.IsHeld()); + pageheap_lock.unlock(); + MemoryModifyStatus ret = {.success = true, + .error_number = state_.error_number}; + { + // The caller's PageHeapSpinLockHolder still forbids allocation. + ScopedAllocationAllow allow; + if (release_callback_) { + release_callback_(); + } + if (!state_.unback_success) { + ret = {.success = false, .error_number = 0}; + } else { + PageId end = r.p + r.n; + for (; r.p != end; ++r.p) { + state_.released_set.insert(r.p); + } + } } - - return {.success = true, .error_number = state_.error_number}; + pageheap_lock.lock(); + return ret; } PageFlagsBase::PageFlagsBitmaps FakePageFlags::GetSinglePageBitmaps( @@ -612,6 +643,9 @@ void Release::Perform(State& state) const { } Length desired(desired_pages); size_t to_release_from_partial_allocs; + // Reentrant instructions run while the lock is dropped may allocate from or + // empty the release candidates, so the amount released is then unpredictable. + const bool reentrant_was_pending = !state.reentrant_stack.empty(); Length released; { @@ -622,10 +656,11 @@ void Release::Perform(State& state) const { released = state.filler.ReleasePages(desired, skip_subrelease_intervals, release_partial_allocs, hit_limit); } + state.DrainFullyFreedTrackers(); if (!release_partial_allocs || hit_limit || skip_subrelease_intervals.SkipSubreleaseEnabled() || - !state.unback_success || state.depth != 0) { + !state.unback_success || state.depth != 0 || reentrant_was_pending) { return; } TC_CHECK_GE(released.raw_num(), to_release_from_partial_allocs); @@ -687,13 +722,15 @@ void MemoryLimitHitRelease::Perform(State& state) const { Length desired_len(desired); Length released; const Length free = state.filler.free_pages(); + const bool reentrant_was_pending = !state.reentrant_stack.empty(); { PageHeapSpinLockHolder l; released = state.filler.ReleasePages(desired_len, SkipSubreleaseIntervals{}, /*release_partial_alloc_pages=*/false, /*hit_limit=*/true); } - if (state.depth != 0) { + state.DrainFullyFreedTrackers(); + if (state.depth != 0 || reentrant_was_pending) { return; } const Length expected = @@ -727,26 +764,25 @@ void TreatTrackers::Perform(State& state) const { state.treating_trackers = true; FakePageFlags pageflags(state); FakeResidency residency(state); - PageHeapSpinLockHolder l; - state.filler.TreatHugepageTrackers( - enable_collapse ? EnableCollapse::kEnabled : EnableCollapse::kDisabled, - enable_unfiltered_collapse ? EnableUnfilteredCollapse::kEnabled - : EnableUnfilteredCollapse::kDisabled, - enable_release_stale_pages ? ReleaseStalePages::kEnabled - : ReleaseStalePages::kDisabled, - &pageflags, &residency); - state.treating_trackers = false; - while (PageTracker* pt = state.filler.FetchFullyFreedTracker()) { - HugePage hp = pt->location(); - for (PageId p = hp.first_page(), end = hp.first_page() + kPagesPerHugePage; - p != end; ++p) { - state.released_set.erase(p); - } - delete pt; + { + PageHeapSpinLockHolder l; + state.filler.TreatHugepageTrackers( + enable_collapse ? EnableCollapse::kEnabled : EnableCollapse::kDisabled, + enable_unfiltered_collapse ? EnableUnfilteredCollapse::kEnabled + : EnableUnfilteredCollapse::kDisabled, + enable_release_stale_pages ? ReleaseStalePages::kEnabled + : ReleaseStalePages::kDisabled, + &pageflags, &residency); } + state.treating_trackers = false; + state.DrainFullyFreedTrackers(); for (PageTracker* pt : state.trackers) { HugePage hp = pt->location(); - const PageBitmap& rel = pt->released_by_page(); + PageBitmap rel; + { + PageHeapSpinLockHolder l; + rel = pt->released_by_page(); + } for (size_t i = 0; i < kPagesPerHugePage.raw_num(); ++i) { PageId p = hp.first_page() + Length(i); if (rel.GetBit(i)) { @@ -1210,6 +1246,63 @@ TEST(HugePageFillerTest, ConcurrentTreatmentInterferenceStress) { FuzzFiller(instructions, SubreleaseUnbackedMode::kDisabled); } +// Interleavings with an in-flight release, which drops pageheap_lock for each +// range unbacked. +TEST(HugePageFillerTest, ReentrantDeallocateEmptiesTrackerDuringRelease) { + // The reentrant Deallocate frees the only allocation of the hugepage being + // released, so the tracker is parked and drained after ReleasePages. + FuzzFiller({Allocate{.length = 1, .num_objects = 1}, + ReentrantSubprogram{.subprogram = {Deallocate{.tracker_index = 0, + .alloc_index = 0}, + GatherStats{}}}, + Release{.desired_pages = 65535, .release_partial_allocs = true}, + GatherSpanStats{}}, + SubreleaseUnbackedMode::kDisabled); +} + +TEST(HugePageFillerTest, ReentrantReleaseAndAllocateDuringRelease) { + // Two hugepages are candidates; the reentrant program allocates, releases + // and deallocates while the first is being released. + FuzzFiller( + {Allocate{.length = 1, .num_objects = 1}, ModelTail{.length = 1}, + ReentrantSubprogram{ + .subprogram = {Allocate{.length = 8, .num_objects = 1}, + MemoryLimitHitRelease{.desired = 65535}, + Deallocate{.tracker_index = 1, .alloc_index = 0}, + Deallocate{.tracker_index = 0, .alloc_index = 0}, + Release{.desired_pages = 65535, + .release_partial_allocs = true}}}, + Release{.desired_pages = 65535, .release_partial_allocs = true}, + GatherStatsPbtxt{}}, + SubreleaseUnbackedMode::kDisabled); +} + +TEST(HugePageFillerTest, ReentrantTreatmentDuringRelease) { + FuzzFiller( + {UpdateBitmaps{.hugepage_backed_set = true, + .hugepage_backed_val = false, + .unbacked_bitmap_val = 0xff, + .swapped_bitmap_val = 0xff}, + Allocate{.length = 1, .num_objects = 1}, ModelTail{.length = 1}, + ReentrantSubprogram{ + .subprogram = {TreatTrackers{.enable_collapse = true, + .enable_unfiltered_collapse = true}, + Deallocate{.tracker_index = 0, .alloc_index = 0}}}, + Release{.desired_pages = 65535, .release_partial_allocs = true}, + TreatTrackers{.enable_collapse = true, + .enable_unfiltered_collapse = true}}, + SubreleaseUnbackedMode::kEnabled); +} + +TEST(HugePageFillerTest, ReentrantModelTailDuringTeardown) { + // The remaining reentrant program would otherwise run while ~State unbacks + // the remainder of the partially released hugepage. + FuzzFiller({ModelTail{.length = 0}, MemoryLimitHitRelease{.desired = 22219}, + ReentrantSubprogram{.subprogram = {ModelTail{.length = 0}}}, + ToggleUnback{}}, + SubreleaseUnbackedMode::kDisabled); +} + TEST(HugePageFillerTest, SubreleaseUnbackedRegression) { FuzzFiller( {ModelTail{.length = 0}, GatherStatsPbtxt{}, diff --git a/tcmalloc/huge_page_filler_test.cc b/tcmalloc/huge_page_filler_test.cc index 986c5e9f4..b3c0b7a4c 100644 --- a/tcmalloc/huge_page_filler_test.cc +++ b/tcmalloc/huge_page_filler_test.cc @@ -40,14 +40,13 @@ #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" #include "absl/flags/flag.h" +#include "absl/functional/any_invocable.h" #include "absl/random/random.h" #include "absl/status/status.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "absl/strings/str_join.h" #include "absl/strings/string_view.h" -#include "absl/synchronization/blocking_counter.h" -#include "absl/synchronization/mutex.h" #include "absl/time/time.h" #include "tcmalloc/common.h" #include "tcmalloc/huge_cache.h" @@ -60,6 +59,7 @@ #include "tcmalloc/internal/pageflags.h" #include "tcmalloc/internal/range_tracker.h" #include "tcmalloc/internal/residency.h" +#include "tcmalloc/internal/scoped_allow_allocation.h" #include "tcmalloc/internal/system_allocator.h" #include "tcmalloc/pages.h" #include "tcmalloc/span.h" @@ -287,6 +287,9 @@ class MockCollapse final : public MemoryModifyFunction { EXPECT_EQ(r.n, kPagesPerHugePage); ++collapsed_[r.start_addr()]; FakeClock::Advance(latency_); + if (callback_) { + callback_(r); + } return {.success = success_, .error_number = error_number_}; } @@ -306,9 +309,14 @@ class MockCollapse final : public MemoryModifyFunction { void SetSuccess(bool success) { success_ = success; } void SetErrorNumber(int error_number) { error_number_ = error_number; } void SetLatency(absl::Duration latency) { latency_ = latency; } + // Runs during collapse, i.e. while pageheap_lock is dropped. + void SetCallback(absl::AnyInvocable callback) { + callback_ = std::move(callback); + } private: absl::flat_hash_map collapsed_; + absl::AnyInvocable callback_; bool success_ = true; int error_number_ = 0; absl::Duration latency_; @@ -338,33 +346,36 @@ class MockSetAnonVmaName final : public MemoryTagFunction { bool ignore_name_ = false; }; -class BlockingUnback final : public MemoryModifyFunction { +// Models HugePageAwareAllocator::UnbackWithoutLock: invoked with +// pageheap_lock held, drops it while the range is "released", and reacquires +// it before returning. Tests may install a callback that runs while the lock +// is dropped to interleave other filler operations with an in-flight release. +class MockUnback final : public MemoryModifyFunction { public: - constexpr BlockingUnback() = default; - - [[nodiscard]] MemoryModifyStatus operator()(Range r) override { - if (!mu_) { - return {.success = success_, .error_number = 0}; - } - - if (counter_) { - counter_->DecrementCount(); + MockUnback() = default; + + [[nodiscard]] MemoryModifyStatus operator()(Range r) override + ABSL_NO_THREAD_SAFETY_ANALYSIS { + TC_CHECK(pageheap_lock.IsHeld()); + // calls_ and success_ are only accessed under pageheap_lock. + ++calls_; + const bool success = success_; + pageheap_lock.unlock(); + if (callback_) { + // The callback may allocate (e.g. to record test failures), which the + // enclosing PageHeapSpinLockHolder would otherwise flag. + ScopedAllocationAllow allow; + callback_(r); } - - mu_->lock(); - mu_->unlock(); - return {.success = success_, .error_number = 0}; + pageheap_lock.lock(); + return {.success = success, .error_number = 0}; } - absl::BlockingCounter* counter_ = nullptr; bool success_ = true; - - private: - static thread_local absl::Mutex* mu_; + int calls_ = 0; + absl::AnyInvocable callback_; }; -thread_local absl::Mutex* BlockingUnback::mu_ = nullptr; - class FillerTest : public testing::Test { protected: // We have backing of one word per (normal-sized) page for our "hugepages". @@ -409,7 +420,7 @@ class FillerTest : public testing::Test { ClockResetter clock_resetter_; SubreleaseUnbackedMode mode_ = SubreleaseUnbackedMode::kDisabled; HugePageFiller filler_; - BlockingUnback blocking_unback_; + MockUnback unback_; MockCollapse collapse_; MockSetAnonVmaName set_anon_vma_name_; @@ -417,11 +428,8 @@ class FillerTest : public testing::Test { SubreleaseUnbackedMode mode = SubreleaseUnbackedMode::kDisabled) : mode_(mode), filler_(Clock{.now = FakeClock::now, .freq = FakeClock::freq}, - MemoryTag::kNormal, blocking_unback_, blocking_unback_, - collapse_, set_anon_vma_name_, mode) { - // Reset success state - blocking_unback_.success_ = true; - } + MemoryTag::kNormal, unback_, collapse_, set_anon_vma_name_, + mode) {} ~FillerTest() override { EXPECT_EQ(filler_.size(), NHugePages(0)); } @@ -635,7 +643,27 @@ class FillerTest : public testing::Test { return false; } - private: + // Fetches and deletes every tracker parked on the filler's fully freed list. + // Returns the number of trackers fetched. + int DrainFullyFreedTrackers() { + int n = 0; + while (true) { + PageTracker* pt; + { + PageHeapSpinLockHolder l; + pt = filler_.FetchFullyFreedTracker(); + } + if (pt == nullptr) { + return n; + } + EXPECT_EQ(pt->longest_free_range(), kPagesPerHugePage); + EXPECT_TRUE(pt->empty()); + --hp_contained_; + delete pt; + ++n; + } + } + PAlloc AllocateRaw(Length n, SpanAllocInfo span_alloc_info, bool donated) { EXPECT_LT(n, kPagesPerHugePage); // Densely-accessed spans are not allocated from donated hugepages. So @@ -668,6 +696,7 @@ class FillerTest : public testing::Test { return ret; } + private: static int64_t clock_; }; @@ -1280,20 +1309,7 @@ TEST_F(FillerTest, ParallelCollapseRelease) { done = true; collapse_thread.join(); - while (true) { - PageTracker* pt; - { - PageHeapSpinLockHolder l; - pt = filler_.FetchFullyFreedTracker(); - } - if (pt == nullptr) { - break; - } - EXPECT_EQ(pt->longest_free_range(), kPagesPerHugePage); - EXPECT_TRUE(pt->empty()); - --hp_contained_; - delete pt; - } + DrainFullyFreedTrackers(); CheckStats(); } @@ -4733,11 +4749,11 @@ TEST_F(FillerTest, b258965495) { ASSERT_TRUE(!a1.empty()); EXPECT_EQ(filler_.size(), NHugePages(1)); - ASSERT_TRUE(blocking_unback_.success_); + ASSERT_TRUE(unback_.success_); // 1 huge page: 2 pages allocated, 0 free, kPagesPerHugePage-2 released EXPECT_EQ(HardReleasePages(kPagesPerHugePage), kPagesPerHugePage - Length(2)); - blocking_unback_.success_ = false; + unback_.success_ = false; // 1 huge page: 3 pages allocated, 0 free, kPagesPerHugePage-3 released auto a2 = AllocateWithSpanAllocInfo(Length(1), a1.front().span_alloc_info); EXPECT_EQ(filler_.size(), NHugePages(1)); @@ -4747,7 +4763,7 @@ TEST_F(FillerTest, b258965495) { // 1 huge page: 2 pages allocated, 1 free, kPagesPerHugePage-3 released Delete(a2); - blocking_unback_.success_ = true; + unback_.success_ = true; // During the deallocation of a1 under PartialRerelease::Return, but before // we mark the pages as free (PageTracker::MaybeRelease), we have: // @@ -6523,20 +6539,392 @@ TEST_F(FillerTest, ConcurrentTreatmentInterferenceStress) { done = true; collapse_thread.join(); - while (true) { - PageTracker* pt; + DrainFullyFreedTrackers(); + CheckStats(); +} + +// The tests below exercise operations that interleave with an in-flight +// release, which drops pageheap_lock while the OS unbacks each range. + +// A concurrent Put() on the hugepage being released must not touch the filler +// lists (the releasing thread owns them) and must leave the accounting exact. +TEST_F(FillerTest, ReentrantPutDuringRelease) { + randomize_density_ = false; + const SpanAllocInfo info = {.objects_per_span = 1, + .density = AccessDensityPrediction::kSparse}; + PAlloc a = AllocateWithSpanAllocInfo(Length(1), info); + PAlloc b = AllocateWithSpanAllocInfo(Length(1), info); + ASSERT_EQ(a.pt, b.pt); + + bool fired = false; + unback_.callback_ = [&](Range r) { + if (fired) return; + fired = true; + EXPECT_EQ(r.n, kPagesPerHugePage - Length(2)); + EXPECT_TRUE(b.pt->BeingSubreleased()); + EXPECT_FALSE(DeleteRaw(b)); + EXPECT_FALSE(b.pt->fully_freed()); + }; + // The 254 free pages are released while b is freed. b's page stays + // backed: it was freed after the hugepage was scanned, and it is picked up + // by the next ReleasePages(). + EXPECT_EQ(ReleasePages(kPagesPerHugePage), kPagesPerHugePage - Length(2)); + EXPECT_TRUE(fired); + EXPECT_FALSE(a.pt->BeingSubreleased()); + EXPECT_EQ(a.pt->released_pages(), kPagesPerHugePage - Length(2)); + EXPECT_EQ(filler_.unmapped_pages(), kPagesPerHugePage - Length(2)); + CheckStats(); + + EXPECT_EQ(ReleasePages(kPagesPerHugePage), Length(1)); + EXPECT_EQ(a.pt->released_pages(), kPagesPerHugePage - Length(1)); + EXPECT_EQ(filler_.unmapped_pages(), kPagesPerHugePage - Length(1)); + CheckStats(); + + EXPECT_TRUE(Delete(a)); + EXPECT_EQ(filler_.unmapped_pages(), Length(0)); +} + +// A concurrent Put() that empties the hugepage being released cannot return +// the tracker to its caller: the releasing thread still references it. It is +// parked for FetchFullyFreedTracker() instead. +TEST_F(FillerTest, ReentrantPutEmptiesTrackerDuringRelease) { + randomize_density_ = false; + const SpanAllocInfo info = {.objects_per_span = 1, + .density = AccessDensityPrediction::kSparse}; + PAlloc a = AllocateWithSpanAllocInfo(Length(1), info); + + bool fired = false; + unback_.callback_ = [&](Range) { + if (fired) return; + fired = true; + EXPECT_FALSE(DeleteRaw(a)); + EXPECT_TRUE(a.pt->fully_freed()); + EXPECT_TRUE(a.pt->BeingSubreleased()); + // Not yet available: the release is still in flight. + PageHeapSpinLockHolder l; + EXPECT_EQ(filler_.FetchFullyFreedTracker(), nullptr); + }; + EXPECT_EQ(ReleasePages(kPagesPerHugePage), kPagesPerHugePage - Length(1)); + EXPECT_TRUE(fired); + // The remainder of the now-empty hugepage was unbacked eagerly. + EXPECT_EQ(unback_.calls_, 2); + EXPECT_EQ(filler_.size(), NHugePages(0)); + EXPECT_EQ(filler_.unmapped_pages(), Length(0)); + EXPECT_EQ(DrainFullyFreedTrackers(), 1); + CheckStats(); +} + +// A concurrent TryGet() must not be served from the hugepage being released. +TEST_F(FillerTest, ReentrantAllocateDuringRelease) { + randomize_density_ = false; + const SpanAllocInfo info = {.objects_per_span = 1, + .density = AccessDensityPrediction::kSparse}; + PAlloc a = AllocateWithSpanAllocInfo(Length(1), info); + + bool fired = false; + PAlloc c; + unback_.callback_ = [&](Range) { + if (fired) return; + fired = true; + c = AllocateWithSpanAllocInfo(Length(1), info); + EXPECT_NE(c.pt, a.pt); + }; + // c's new hugepage is released too, by the next pass of the same call. + EXPECT_EQ(ReleasePages(kPagesPerHugePage), 2 * kPagesPerHugePage - Length(2)); + EXPECT_TRUE(fired); + EXPECT_EQ(filler_.size(), NHugePages(2)); + EXPECT_EQ(a.pt->released_pages(), kPagesPerHugePage - Length(1)); + EXPECT_EQ(c.pt->released_pages(), kPagesPerHugePage - Length(1)); + CheckStats(); + + EXPECT_TRUE(Delete(c)); + EXPECT_TRUE(Delete(a)); +} + +// A concurrent ReleasePages() skips both the hugepage being released and the +// remaining candidates of the in-flight release, which then releases them +// itself. +TEST_F(FillerTest, ReentrantReleaseDuringRelease) { + randomize_density_ = false; + const SpanAllocInfo info = {.objects_per_span = 1, + .density = AccessDensityPrediction::kSparse}; + PAlloc a1 = AllocateWithSpanAllocInfo(Length(1), info); + PAlloc a2 = AllocateWithSpanAllocInfo(Length(1), info); + PAlloc b = AllocateWithSpanAllocInfo(Length(1), info, /*donated=*/true); + ASSERT_EQ(a1.pt, a2.pt); + ASSERT_NE(a1.pt, b.pt); + + bool fired = false; + unback_.callback_ = [&](Range r) { + if (fired) return; + fired = true; + // Candidates are released in order of increasing usage: b's hugepage + // first, with a's hugepage pending. + EXPECT_EQ(HugePageContaining(r.p), b.pt->location()); + EXPECT_TRUE(b.pt->BeingSubreleased()); + EXPECT_TRUE(a1.pt->DontFreeTracker(HugePageTreatmentType::kSubrelease)); + EXPECT_EQ(ReleasePages(kPagesPerHugePage), Length(0)); + EXPECT_EQ(HardReleasePages(kPagesPerHugePage), Length(0)); + }; + EXPECT_EQ(ReleasePages(2 * kPagesPerHugePage), + 2 * kPagesPerHugePage - Length(3)); + EXPECT_TRUE(fired); + EXPECT_EQ(unback_.calls_, 2); + EXPECT_FALSE(a1.pt->DontFreeTracker()); + EXPECT_FALSE(b.pt->DontFreeTracker()); + CheckStats(); + + EXPECT_TRUE(Delete(b)); + EXPECT_FALSE(Delete(a1)); + EXPECT_TRUE(Delete(a2)); +} + +// A pending release candidate that is emptied while the lock is dropped is +// parked (it may not be freed under the in-flight release) and skipped. +TEST_F(FillerTest, PendingCandidateEmptiedDuringRelease) { + randomize_density_ = false; + const SpanAllocInfo info = {.objects_per_span = 1, + .density = AccessDensityPrediction::kSparse}; + PAlloc a1 = AllocateWithSpanAllocInfo(Length(1), info); + PAlloc a2 = AllocateWithSpanAllocInfo(Length(1), info); + PAlloc b = AllocateWithSpanAllocInfo(Length(1), info, /*donated=*/true); + ASSERT_EQ(a1.pt, a2.pt); + ASSERT_NE(a1.pt, b.pt); + + bool fired = false; + unback_.callback_ = [&](Range r) { + if (fired) return; + fired = true; + EXPECT_EQ(HugePageContaining(r.p), b.pt->location()); + EXPECT_FALSE(DeleteRaw(a1)); + EXPECT_FALSE(DeleteRaw(a2)); + EXPECT_TRUE(a1.pt->fully_freed()); + }; + EXPECT_EQ(ReleasePages(2 * kPagesPerHugePage), kPagesPerHugePage - Length(1)); + EXPECT_TRUE(fired); + EXPECT_EQ(unback_.calls_, 1); + EXPECT_EQ(filler_.size(), NHugePages(1)); + EXPECT_EQ(DrainFullyFreedTrackers(), 1); + CheckStats(); + + EXPECT_TRUE(Delete(b)); +} + +// A concurrent treatment pass must not select the hugepage being released nor +// the pending candidates of the in-flight release. +TEST_F(FillerTestWithSubreleaseUnbacked, ReentrantTreatmentDuringRelease) { + randomize_density_ = false; + const SpanAllocInfo info = {.objects_per_span = 1, + .density = AccessDensityPrediction::kSparse}; + FakePageFlags pageflags; + FakeResidency residency; + PAlloc a1 = AllocateWithSpanAllocInfo(Length(1), info); + PAlloc a2 = AllocateWithSpanAllocInfo(Length(1), info); + PAlloc b = AllocateWithSpanAllocInfo(Length(1), info, /*donated=*/true); + ASSERT_EQ(a1.pt, a2.pt); + ASSERT_NE(a1.pt, b.pt); + for (const PAlloc& p : {a1, b}) { + pageflags.MarkHugePageBacked(p.p.start_addr(), false); + pageflags.SetStaleBitmap(p.p.start_addr(), {}); + residency.SetUnbackedAndSwappedBitmaps(p.p.start_addr(), {}, {}); + } + + bool fired = false; + unback_.callback_ = [&](Range r) { + if (fired) return; + fired = true; + EXPECT_EQ(HugePageContaining(r.p), b.pt->location()); + EXPECT_TRUE(a1.pt->DontFreeTracker(HugePageTreatmentType::kSubrelease)); + EXPECT_TRUE(b.pt->DontFreeTracker(HugePageTreatmentType::kSubrelease)); + TreatHugepageTrackers(EnableCollapse::kEnabled, + EnableUnfilteredCollapse::kDisabled, + ReleaseStalePages::kDisabled, &pageflags, &residency); + EXPECT_EQ(GetHugePageTreatmentStats().collapse_eligible, 0); + EXPECT_FALSE(collapse_.TriedCollapse(a1.p.start_addr())); + EXPECT_FALSE(collapse_.TriedCollapse(b.p.start_addr())); + }; + EXPECT_EQ(ReleasePages(2 * kPagesPerHugePage), + 2 * kPagesPerHugePage - Length(3)); + EXPECT_TRUE(fired); + CheckStats(); + + // Once the release has completed, both hugepages are selected again. + EXPECT_FALSE(a1.pt->DontFreeTracker(HugePageTreatmentType::kSubrelease)); + EXPECT_FALSE(b.pt->DontFreeTracker(HugePageTreatmentType::kSubrelease)); + TreatHugepageTrackers(EnableCollapse::kEnabled, + EnableUnfilteredCollapse::kDisabled, + ReleaseStalePages::kDisabled, &pageflags, &residency); + EXPECT_EQ(GetHugePageTreatmentStats().collapse_eligible, 2); + CheckStats(); + + EXPECT_TRUE(Delete(b)); + EXPECT_FALSE(Delete(a1)); + EXPECT_TRUE(Delete(a2)); +} + +// Conversely, a concurrent ReleasePages() must not release from a hugepage +// that a treatment pass has selected, as collapsing requires that none of its +// pages are released. +TEST_F(FillerTest, ReentrantReleaseDuringCollapse) { + randomize_density_ = false; + const SpanAllocInfo info = {.objects_per_span = 1, + .density = AccessDensityPrediction::kSparse}; + FakePageFlags pageflags; + FakeResidency residency; + PAlloc a = AllocateWithSpanAllocInfo(Length(1), info); + pageflags.MarkHugePageBacked(a.p.start_addr(), false); + pageflags.SetStaleBitmap(a.p.start_addr(), {}); + residency.SetUnbackedAndSwappedBitmaps(a.p.start_addr(), {}, {}); + + bool fired = false; + collapse_.SetCallback([&](Range r) { + if (fired) return; + fired = true; + EXPECT_EQ(HugePageContaining(r.p), a.pt->location()); + EXPECT_TRUE(a.pt->DontFreeTracker(HugePageTreatmentType::kCollapse)); + EXPECT_EQ(ReleasePages(kPagesPerHugePage), Length(0)); + EXPECT_EQ(HardReleasePages(kPagesPerHugePage), Length(0)); + EXPECT_EQ(unback_.calls_, 0); + }); + TreatHugepageTrackers(EnableCollapse::kEnabled, + EnableUnfilteredCollapse::kDisabled, + ReleaseStalePages::kDisabled, &pageflags, &residency); + EXPECT_TRUE(fired); + EXPECT_FALSE(a.pt->released()); + CheckStats(); + + EXPECT_EQ(ReleasePages(kPagesPerHugePage), kPagesPerHugePage - Length(1)); + EXPECT_TRUE(Delete(a)); +} + +// Releases from a treatment pass (which also drop pageheap_lock) tolerate the +// tracker being emptied in the meantime. +TEST_F(FillerTest, ReentrantPutDuringTreatmentRelease) { + randomize_density_ = false; + const SpanAllocInfo info = {.objects_per_span = 1, + .density = AccessDensityPrediction::kSparse}; + FakePageFlags pageflags; + FakeResidency residency; + PAlloc a = AllocateWithSpanAllocInfo(Length(1), info); + pageflags.MarkHugePageBacked(a.p.start_addr(), false); + pageflags.SetStaleBitmap(a.p.start_addr(), {}); + Bitmap unbacked, swapped; + swapped.SetRange(0, kMaxResidencyBits); + residency.SetUnbackedAndSwappedBitmaps(a.p.start_addr(), unbacked, swapped); + + bool fired = false; + unback_.callback_ = [&](Range r) { + if (fired) return; + fired = true; + EXPECT_EQ(r.n, kPagesPerHugePage - Length(1)); + EXPECT_FALSE(DeleteRaw(a)); + EXPECT_TRUE(a.pt->fully_freed()); + }; + TreatHugepageTrackers(EnableCollapse::kEnabled, + EnableUnfilteredCollapse::kDisabled, + ReleaseStalePages::kDisabled, &pageflags, &residency); + EXPECT_TRUE(fired); + EXPECT_EQ(GetHugePageTreatmentStats().treated_pages_subreleased, + (kPagesPerHugePage - Length(1)).raw_num()); + EXPECT_EQ(filler_.size(), NHugePages(0)); + EXPECT_EQ(filler_.unmapped_pages(), Length(0)); + EXPECT_EQ(DrainFullyFreedTrackers(), 1); + CheckStats(); +} + +// Runs allocations and deallocations against concurrent release and treatment +// passes, all of which drop pageheap_lock mid-operation, and verifies that the +// filler's accounting is exact afterwards. +TEST_F(FillerTest, ConcurrentReleaseStress) { + randomize_density_ = false; + const SpanAllocInfo info = {.objects_per_span = 1, + .density = AccessDensityPrediction::kSparse}; + FakePageFlags pageflags; + FakeResidency residency; + set_anon_vma_name_.SetIgnoreName(true); + std::mt19937_64 rng(42); + + // Populate a few hundred hugepages. Every hugepage the test touches is + // created here so that the fakes are not mutated concurrently. + std::vector live; + for (int i = 0; i < 2000; ++i) { + const Length n(1 + rng() % 32); + PAlloc p = AllocateWithSpanAllocInfo(n, info); + p.pt->SetTagState({.sampled_for_tagging = (i % 8 == 0), .record_time = 0}); + void* addr = p.pt->location().start_addr(); + pageflags.MarkHugePageBacked(addr, false); + pageflags.SetStaleBitmap(addr, {}); + Bitmap unbacked, swapped; + unbacked.SetRange(0, kMaxResidencyBits / 2); + swapped.SetRange(kMaxResidencyBits / 2, kMaxResidencyBits / 2); + residency.SetUnbackedAndSwappedBitmaps(addr, unbacked, swapped); + live.push_back(p); + } + + std::atomic done(false); + std::thread release_thread([&] { + std::mt19937_64 rng(1); + while (!done.load(std::memory_order_acquire)) { + const Length desired(1 + rng() % (4 * kPagesPerHugePage.raw_num())); + switch (rng() % 3) { + case 0: + ReleasePages(desired); + break; + case 1: + ReleasePartialPages(desired); + break; + default: + HardReleasePages(desired); + break; + } + } + }); + std::thread treatment_thread([&] { + while (!done.load(std::memory_order_acquire)) { + FakeClock::Advance(absl::Minutes(10)); + TreatHugepageTrackers( + EnableCollapse::kEnabled, EnableUnfilteredCollapse::kDisabled, + ReleaseStalePages::kEnabled, &pageflags, &residency); + } + }); + + for (int i = 0; i < 20000; ++i) { + if (rng() % 2 == 0 && !live.empty()) { + const size_t index = rng() % live.size(); + std::swap(live[index], live.back()); + DeleteRaw(live.back()); + live.pop_back(); + continue; + } + // Allocate from an existing hugepage only, if one has room. + const Length n(1 + rng() % 32); + HugePageFiller::TryGetResult result; { PageHeapSpinLockHolder l; - pt = filler_.FetchFullyFreedTracker(); - } - if (pt == nullptr) { - break; + result = filler_.TryGet(n, info); } - EXPECT_EQ(pt->longest_free_range(), kPagesPerHugePage); - EXPECT_TRUE(pt->empty()); - --hp_contained_; - delete pt; + if (result.pt == nullptr) continue; + total_allocated_ += n; + live.push_back(PAlloc{.pt = result.pt, + .p = result.page, + .n = n, + .mark = 0, + .span_alloc_info = info, + .from_released = result.from_released}); } + + done = true; + release_thread.join(); + treatment_thread.join(); + + DrainFullyFreedTrackers(); + CheckStats(); + for (const PAlloc& p : live) { + DeleteRaw(p); + } + EXPECT_EQ(DrainFullyFreedTrackers(), 0); + EXPECT_EQ(filler_.size(), NHugePages(0)); + EXPECT_EQ(filler_.unmapped_pages(), Length(0)); CheckStats(); } diff --git a/tcmalloc/huge_page_options.h b/tcmalloc/huge_page_options.h index c9eb0e998..757a5bcb5 100644 --- a/tcmalloc/huge_page_options.h +++ b/tcmalloc/huge_page_options.h @@ -25,6 +25,7 @@ namespace tcmalloc::tcmalloc_internal { enum class HugePageTreatmentType : uint8_t { kSampled = 1 << 0, kCollapse = 1 << 1, + kSubrelease = 1 << 2, }; enum class EnableCollapse : uint8_t { diff --git a/tcmalloc/huge_page_tracker.h b/tcmalloc/huge_page_tracker.h index 0e5f15d21..868b1c635 100644 --- a/tcmalloc/huge_page_tracker.h +++ b/tcmalloc/huge_page_tracker.h @@ -302,6 +302,14 @@ class PageTracker : public TList::Elem { return hugepage_residency_state_.being_collapsed; } + // Whether HugePageFiller::ReleaseFreeFromTracker() is releasing this + // tracker's free pages with pageheap_lock temporarily dropped. While set, + // the tracker is not on any filler list and HugePageFiller::Put() defers + // list maintenance (and handling of the tracker becoming empty) to the + // releasing thread. + void SetBeingSubreleased(bool value) { being_subreleased_ = value; } + bool BeingSubreleased() const { return being_subreleased_; } + void SetDontFreeTracker(HugePageTreatmentType type) { dont_free_tracker_mask_ |= static_cast(type); } @@ -309,6 +317,9 @@ class PageTracker : public TList::Elem { dont_free_tracker_mask_ &= ~static_cast(type); } bool DontFreeTracker() const { return dont_free_tracker_mask_ != 0; } + bool DontFreeTracker(HugePageTreatmentType type) const { + return (dont_free_tracker_mask_ & static_cast(type)) != 0; + } struct TagState { bool sampled_for_tagging = false; @@ -358,6 +369,7 @@ class PageTracker : public TList::Elem { // lock. When all the pages on the tracked hugepage are freed, this field // is checked to ensure that the tracker is not freed right away. uint8_t dont_free_tracker_mask_ = 0; + bool being_subreleased_ = false; double alloctime_; double last_page_allocation_time_ = 0; diff --git a/tcmalloc/huge_page_treatment.h b/tcmalloc/huge_page_treatment.h index ef8b21a8c..16625609b 100644 --- a/tcmalloc/huge_page_treatment.h +++ b/tcmalloc/huge_page_treatment.h @@ -381,6 +381,10 @@ class HugePageUnbackedTrackerTreatment final : public HugePageTreatment { PageTracker::HugePageResidencyState state = pt.GetHugePageResidencyState(); if (state.maybe_hugepage_backed) return; + // Skip trackers that an in-flight HugePageFiller::ReleasePages() holds as + // a candidate: it may drop pageheap_lock and release from pt after we + // have selected it. + if (pt.DontFreeTracker(HugePageTreatmentType::kSubrelease)) return; if (!state.entry_valid) { PushCandidate(pt); @@ -509,6 +513,11 @@ class HugePageUnbackedTrackerTreatment final : public HugePageTreatment { released_length.raw_num(); } } + // HandleReleaseFree() drops pageheap_lock; the tracker may have been + // emptied (and parked) in the meantime. + if (tracker->fully_freed()) { + continue; + } if (subrelease_unbacked_mode_ == SubreleaseUnbackedMode::kEnabled) { Length released_length = page_filler_.HandleUnbackedHugePage(