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(