From e615adddac29582120ed77f7c51a0d27f4ca52f2 Mon Sep 17 00:00:00 2001 From: Chris Kennelly CA Date: Wed, 16 Sep 2026 13:58:42 -0700 Subject: [PATCH] Release pageheap_lock while HugePageFiller unbacks free pages. ReleaseCandidates held pageheap_lock across every madvise issued for a candidate hugepage, stalling all page-level allocation and deallocation for the duration of the syscall. Unback with unback_without_lock_ instead, as the treatment and HugeCache release paths already do. While a tracker's free pages are being unbacked it is taken off the filler lists (being_released_, counted in n_in_flight_release_), so no other thread can allocate from it, collapse it, or move it between lists; it is accounted as a partially released hugepage for the duration. The pages about to be unbacked are added to unmapped_ before the lock is dropped, as HugeCache does, so readers such as PageAllocator::ShrinkToUsageLimitSlow never see reclaimed memory as backed. Candidates are pinned with HugePageTreatmentType::kRelease so a concurrent Put that empties one cannot free it while ReleaseCandidates still holds a pointer; such trackers are parked on fully_freed_trackers_ and drained by the caller (HugePageAwareAllocator::DrainFreedTrackers). Candidates are re-validated after each lock drop, since an earlier candidate's unback may have changed their state; the debug-only sort-order assertion no longer holds for the same reason. Pages stay free, not allocated, while in flight, so pages_allocated_ and free_pages() remain truthful. PageTracker::ReleaseFree updates released_count_ per run so it matches released_by_page_ whenever the lock is held. HandleFullyFreedTracker now records the lifetime sample and resets the anon VMA name for parked trackers too. PiperOrigin-RevId: 982709352 --- tcmalloc/huge_page_aware_allocator.h | 4 +- tcmalloc/huge_page_aware_allocator_fuzz.cc | 8 +- tcmalloc/huge_page_filler.h | 356 +++++++----- tcmalloc/huge_page_filler_fuzz.cc | 203 +++---- tcmalloc/huge_page_filler_test.cc | 624 ++++++++++++++++++++- tcmalloc/huge_page_options.h | 3 + tcmalloc/huge_page_tracker.h | 24 +- tcmalloc/huge_page_treatment.h | 5 +- 8 files changed, 957 insertions(+), 270 deletions(-) diff --git a/tcmalloc/huge_page_aware_allocator.h b/tcmalloc/huge_page_aware_allocator.h index 5c11851c9..dbee101fc 100644 --- a/tcmalloc/huge_page_aware_allocator.h +++ b/tcmalloc/huge_page_aware_allocator.h @@ -465,7 +465,7 @@ 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. + // pageheap_lock (during ReleasePages or TreatHugepageTrackers) to the cache. 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, @@ -1054,6 +1054,7 @@ inline Length HugePageAwareAllocator::ReleaseAtLeastNPages( forwarder_.filler_skip_subrelease_long_interval()}, forwarder_.release_partial_alloc_pages(), /*hit_limit*/ false); + DrainFreedTrackers(); } } @@ -1258,6 +1259,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 9d7dacfc3..10016b2db 100644 --- a/tcmalloc/huge_page_aware_allocator_fuzz.cc +++ b/tcmalloc/huge_page_aware_allocator_fuzz.cc @@ -735,7 +735,13 @@ void GatherAndCheckStats::Perform(State& state) const { } uint64_t used_bytes = stats.system_bytes - stats.free_bytes - stats.unmapped_bytes; - TC_CHECK_EQ(used_bytes, + // We only get here with pending_release_ != 0 from a reentrant subprogram. + // HugeCache takes a range out of its free stats while it is being released + // (used == allocated + pending), whereas HugePageFiller accounts pages in + // flight as unmapped (used == allocated), so used_bytes can land anywhere in + // between. + TC_CHECK_GE(used_bytes, state.allocated.in_bytes()); + TC_CHECK_LE(used_bytes, state.allocated.in_bytes() + state.allocator.forwarder().pending_release_.in_bytes()); } diff --git a/tcmalloc/huge_page_filler.h b/tcmalloc/huge_page_filler.h index 7cc73f6b6..5f7dbffaa 100644 --- a/tcmalloc/huge_page_filler.h +++ b/tcmalloc/huge_page_filler.h @@ -176,6 +176,11 @@ class UsageInfo { ++native_page_buckets_size_; } + lifetime_bucket_bounds_[0] = 0; + lifetime_bucket_bounds_[1] = 1; + for (int i = 2; i <= kLifetimeBuckets; ++i) { + lifetime_bucket_bounds_[i] = lifetime_bucket_bounds_[i - 1] * 10; + } TC_CHECK_LE(buckets_size_, kBucketCapacity); } @@ -202,8 +207,6 @@ class UsageInfo { kBucketsAtBounds + kBucketsInBetween + kBucketsAtBounds; static constexpr size_t kLifetimeBuckets = 8; - static constexpr size_t kLifetimeBucketBounds[kLifetimeBuckets + 1] = { - 0, 1, 10, 100, 1000, 10000, 100000, 1000000, 10000000}; using LifetimeHisto = uint32_t[kLifetimeBuckets]; using Histo = uint32_t[kBucketCapacity]; @@ -484,11 +487,11 @@ class UsageInfo { int LifetimeBucketNum(absl::Duration duration) { int64_t duration_ms = absl::ToInt64Milliseconds(duration); - auto it = std::upper_bound( - kLifetimeBucketBounds, kLifetimeBucketBounds + kLifetimeBuckets, - static_cast(std::max(0, duration_ms))); - TC_CHECK_NE(it, kLifetimeBucketBounds); - return it - kLifetimeBucketBounds - 1; + auto it = std::upper_bound(lifetime_bucket_bounds_, + lifetime_bucket_bounds_ + kLifetimeBuckets, + duration_ms); + TC_CHECK_NE(it, lifetime_bucket_bounds_); + return it - lifetime_bucket_bounds_ - 1; } int HardwarePageBucketNum(size_t page) { @@ -545,7 +548,7 @@ class UsageInfo { if (i % 6 == 0) { out.printf("\nHugePageFiller:"); } - out.printf(" < %3zu ms <= %6zu", kLifetimeBucketBounds[i], h[i]); + out.printf(" < %3zu ms <= %6zu", lifetime_bucket_bounds_[i], h[i]); } out.printf("\n"); } @@ -591,10 +594,10 @@ class UsageInfo { for (size_t i = 0; i < kLifetimeBuckets; ++i) { if (h[i] == 0) continue; auto hist = hpaa.CreateSubRegion(key); - hist.PrintI64("lower_bound", kLifetimeBucketBounds[i]); - hist.PrintI64("upper_bound", - (i == kLifetimeBuckets - 1 ? kLifetimeBucketBounds[i] - : kLifetimeBucketBounds[i + 1])); + hist.PrintI64("lower_bound", lifetime_bucket_bounds_[i]); + hist.PrintI64("upper_bound", (i == kLifetimeBuckets - 1 + ? lifetime_bucket_bounds_[i] + : lifetime_bucket_bounds_[i + 1])); hist.PrintI64("value", h[i]); } } @@ -685,6 +688,7 @@ class UsageInfo { // Arrays, because they are split per alloc type. size_t bucket_bounds_[kBucketCapacity]; size_t native_page_bucket_bounds_[kBucketCapacity]; + size_t lifetime_bucket_bounds_[kLifetimeBuckets + 1]; size_t hugepage_backed_previously_released_ = 0; int buckets_size_ = 0; int native_page_buckets_size_ = 0; @@ -733,7 +737,9 @@ class HugePageFiller { ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); // Marks r as usable by new allocations into *pt; returns pt if that hugepage - // is now empty (nullptr otherwise.) + // is now empty and the caller now owns it (nullptr otherwise). An empty + // hugepage that a concurrent ReleasePages or TreatHugepageTrackers still + // refers to is retained and surfaced later by FetchFullyFreedTracker. // // REQUIRES: pt is owned by this object (has been Contribute()), and // {pt, Range{p, n}} was the result of a previous TryGet. @@ -772,11 +778,21 @@ class HugePageFiller { return n_used_released_[AccessDensityPrediction::kDense] + n_used_released_[AccessDensityPrediction::kSparse]; } + // Hugepages accounted as partially released: those on + // regular_alloc_partial_released_ plus those whose release is in flight. + HugeLength PartialReleasedHugePages(AccessDensityPrediction type) const { + return (type == AccessDensityPrediction::kSparse + ? regular_alloc_partial_released_.sparse.size() + : regular_alloc_partial_released_.dense.size()) + + n_in_flight_release_[type]; + } Length used_pages_in_partial_released() const { - TC_ASSERT_LE(n_used_partial_released_[AccessDensityPrediction::kSparse], - regular_alloc_partial_released_.sparse.size().in_pages()); - TC_ASSERT_LE(n_used_partial_released_[AccessDensityPrediction::kDense], - regular_alloc_partial_released_.dense.size().in_pages()); + TC_ASSERT_LE( + n_used_partial_released_[AccessDensityPrediction::kSparse], + PartialReleasedHugePages(AccessDensityPrediction::kSparse).in_pages()); + TC_ASSERT_LE( + n_used_partial_released_[AccessDensityPrediction::kDense], + PartialReleasedHugePages(AccessDensityPrediction::kDense).in_pages()); return n_used_partial_released_[AccessDensityPrediction::kDense] + n_used_partial_released_[AccessDensityPrediction::kSparse]; } @@ -812,6 +828,9 @@ class HugePageFiller { // be greater than the desired number of pages. // Returns the number of pages actually released. The releasing target can be // reduced by skip subrelease which is disabled if all intervals are zero. + // + // Drops and reacquires pageheap_lock while unbacking memory. Callers must + // drain FetchFullyFreedTracker afterwards. static constexpr double kPartialAllocPagesRelease = 0.1; Length ReleasePages(Length desired, SkipSubreleaseIntervals intervals, bool release_partial_alloc_pages, bool hit_limit) @@ -831,9 +850,9 @@ class HugePageFiller { }; HugePageFillerStats GetStats() const; - void Print(Printer& out, bool everything, PageFlagsBase& pageflags) const + void Print(Printer& out, bool everything, PageFlagsBase& pageflags) ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); - void PrintInPbtxt(PbtxtRegion& hpaa, PageFlagsBase& pageflags) const + void PrintInPbtxt(PbtxtRegion& hpaa, PageFlagsBase& pageflags) ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); template @@ -969,6 +988,15 @@ class HugePageFiller { // deleted, once the collapse operation completes. TList fully_freed_trackers_; + // Number of trackers whose free pages ReleasePages is releasing to the OS + // with pageheap_lock dropped (see ReleaseCandidates). Such trackers are off + // every list above so that nothing else can allocate from or otherwise + // touch them, and are invisible to per-list stats (like + // fully_freed_trackers_) until the release completes. They are accounted as + // partially released hugepages (n_partial_released and + // n_used_partial_released_). + HugeLength n_in_flight_release_[AccessDensityPrediction::kPredictionCounts]; + HugePageTreatmentStats treatment_stats_ ABSL_GUARDED_BY(pageheap_lock); // n_used_released_ contains the number of pages in huge pages that are not @@ -978,7 +1006,8 @@ class HugePageFiller { HugeLength n_was_released_[AccessDensityPrediction::kPredictionCounts]; // n_used_partial_released_ is the number of pages which have been allocated - // from the hugepages in the set regular_alloc_partial_released. + // from the hugepages in the set regular_alloc_partial_released and from the + // hugepages whose release is in flight. Length n_used_partial_released_[AccessDensityPrediction::kPredictionCounts]; // RemoveFromFillerList pt from the appropriate PageTrackerList. @@ -990,7 +1019,7 @@ class HugePageFiller { // concurrent operation still holds a pointer to it. May drop and reacquire // pageheap_lock. [[nodiscard]] TrackerType* absl_nullable HandleFullyFreedTracker( - TrackerType* absl_nonnull pt, int64_t now) + TrackerType* absl_nonnull pt) ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); // Like AddToFillerList(), but for use when donating from the tail of a // multi-hugepage allocation. @@ -1002,26 +1031,21 @@ class HugePageFiller { static constexpr size_t kLifetimeBuckets = huge_page_filler_internal::UsageInfo::kLifetimeBuckets; - static constexpr auto& kLifetimeBucketBounds = - huge_page_filler_internal::UsageInfo::kLifetimeBucketBounds; using LifetimeHisto = huge_page_filler_internal::UsageInfo::LifetimeHisto; - void RecordLifetime(const TrackerType* pt, int64_t now); - void PrintLifetimeHisto(Printer& out, const LifetimeHisto& h, + void RecordLifetime(const TrackerType* pt); + void PrintLifetimeHisto(Printer& out, LifetimeHisto h, AccessDensityPrediction type, absl::string_view blurb) const; - void PrintLifetimeHistoInPbtxt(PbtxtRegion& hpaa, const LifetimeHisto& h, - absl::string_view key) const; - - [[nodiscard]] int LifetimeBucketNum(absl::Duration duration) const { - return LifetimeBucketNum(absl::ToInt64Milliseconds(duration)); - } + void PrintLifetimeHistoInPbtxt(PbtxtRegion& hpaa, LifetimeHisto h, + absl::string_view key); - [[nodiscard]] int LifetimeBucketNum(int64_t duration_ms) const { - auto it = std::upper_bound( - kLifetimeBucketBounds, kLifetimeBucketBounds + kLifetimeBuckets, - static_cast(std::max(0, duration_ms))); - TC_CHECK_NE(it, kLifetimeBucketBounds); - return it - kLifetimeBucketBounds - 1; + int LifetimeBucketNum(absl::Duration duration) { + int64_t duration_ms = absl::ToInt64Milliseconds(duration); + auto it = std::upper_bound(lifetime_bucket_bounds_, + lifetime_bucket_bounds_ + kLifetimeBuckets, + duration_ms); + TC_CHECK_NE(it, lifetime_bucket_bounds_); + return it - lifetime_bucket_bounds_ - 1; } // CompareForSubrelease identifies the worse candidate for subrelease, between @@ -1053,7 +1077,9 @@ 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. Drops and reacquires pageheap_lock while + // unbacking; every candidate must be pinned with + // HugePageTreatmentType::kRelease and is unpinned on return. Length ReleaseCandidates(absl::Span candidates, Length target) ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); @@ -1068,17 +1094,15 @@ class HugePageFiller { Length unmapping_unaccounted_; // Functionality related to time series tracking. - void UpdateFillerStatsTracker(int64_t now); + void UpdateFillerStatsTracker(); using StatsTrackerType = SubreleaseStatsTracker<600>; StatsTrackerType fillerstats_tracker_; // Lifetime tracking for completely-freed hugepages LifetimeHisto lifetime_histo_[AccessDensityPrediction::kPredictionCounts]{}; + size_t lifetime_bucket_bounds_[kLifetimeBuckets + 1]; Clock clock_; -#ifndef TCMALLOC_INTERNAL_LEGACY_LOCKING - const double ms_per_cycle_; -#endif const MemoryTag tag_; // TODO(b/73749855): Remove remaining uses of unback_. MemoryModifyFunction& unback_; @@ -1112,15 +1136,17 @@ inline HugePageFiller::HugePageFiller( : size_(NHugePages(0)), fillerstats_tracker_(clock, absl::Minutes(10), absl::Minutes(5)), clock_(clock), -#ifndef TCMALLOC_INTERNAL_LEGACY_LOCKING - ms_per_cycle_(1000.0 / clock.freq()), -#endif tag_(tag), unback_(unback), unback_without_lock_(unback_without_lock), collapse_(collapse), set_anon_vma_name_(set_anon_vma_name), subrelease_unbacked_mode_(subrelease_unbacked_mode) { + lifetime_bucket_bounds_[0] = 0; + lifetime_bucket_bounds_[1] = 1; + for (int i = 2; i <= kLifetimeBuckets; ++i) { + lifetime_bucket_bounds_[i] = lifetime_bucket_bounds_[i - 1] * 10; + } } template @@ -1241,8 +1267,13 @@ HugePageFiller::TryGet(Length n, SpanAllocInfo span_alloc_info) { TC_ASSERT(type == AccessDensityPrediction::kSparse || pt->HasDenseSpans()); // Log previous features before modifying the page tracker. - const int64_t now = clock_.now(); +#ifdef TCMALLOC_INTERNAL_LEGACY_LOCKING + const auto now = clock_.now(); +#endif if (ABSL_PREDICT_FALSE(pt->GetTagState().sampled_for_tagging)) { +#ifndef TCMALLOC_INTERNAL_LEGACY_LOCKING + const auto now = clock_.now(); +#endif pt->RecordFeatures(); #ifndef TCMALLOC_INTERNAL_LEGACY_LOCKING pt->SetLastAllocationTime(now); @@ -1268,38 +1299,29 @@ HugePageFiller::TryGet(Length n, SpanAllocInfo span_alloc_info) { // We're being used for an allocation, so we are no longer considered // donated by this point. TC_ASSERT(!pt->donated()); - UpdateFillerStatsTracker(now); + UpdateFillerStatsTracker(); return {pt, page_allocation.page, was_released}; } template -void HugePageFiller::RecordLifetime(const TrackerType* pt, - int64_t now) { -#ifdef TCMALLOC_INTERNAL_LEGACY_LOCKING - now = clock_.now(); -#endif - const double elapsed = std::max(0.0, now - pt->alloctime()); -#ifndef TCMALLOC_INTERNAL_LEGACY_LOCKING - const int64_t elapsed_ms = static_cast(std::min( - static_cast(kLifetimeBucketBounds[kLifetimeBuckets - 1]), - elapsed * ms_per_cycle_)); - const int bucket = LifetimeBucketNum(elapsed_ms); -#else +void HugePageFiller::RecordLifetime(const TrackerType* pt) { + const double now = clock_.now(); const double frequency = clock_.freq(); + const double elapsed = std::max(now - pt->alloctime(), 0); const absl::Duration lifetime = absl::Milliseconds(elapsed * 1000 / frequency); - const int bucket = LifetimeBucketNum(lifetime); -#endif if (pt->HasDenseSpans()) { - ++lifetime_histo_[AccessDensityPrediction::kDense][bucket]; + ++lifetime_histo_[AccessDensityPrediction::kDense] + [LifetimeBucketNum(lifetime)]; } else { - ++lifetime_histo_[AccessDensityPrediction::kSparse][bucket]; + ++lifetime_histo_[AccessDensityPrediction::kSparse] + [LifetimeBucketNum(lifetime)]; } } template void HugePageFiller::PrintLifetimeHisto( - Printer& out, const LifetimeHisto& h, AccessDensityPrediction type, + Printer& out, LifetimeHisto h, AccessDensityPrediction type, absl::string_view blurb) const { absl::string_view typestring = type == AccessDensityPrediction::kDense ? "densely-accessed" @@ -1309,21 +1331,21 @@ void HugePageFiller::PrintLifetimeHisto( if (i % 6 == 0) { out.printf("\nHugePageFiller:"); } - out.printf(" < %3zu ms <= %6zu", kLifetimeBucketBounds[i], h[i]); + out.printf(" < %3zu ms <= %6zu", lifetime_bucket_bounds_[i], h[i]); } out.printf("\n"); } template void HugePageFiller::PrintLifetimeHistoInPbtxt( - PbtxtRegion& hpaa, const LifetimeHisto& h, absl::string_view key) const { + PbtxtRegion& hpaa, LifetimeHisto h, absl::string_view key) { for (size_t i = 0; i < kLifetimeBuckets; ++i) { if (h[i] == 0) continue; auto hist = hpaa.CreateSubRegion(key); - hist.PrintI64("lower_bound", kLifetimeBucketBounds[i]); + hist.PrintI64("lower_bound", lifetime_bucket_bounds_[i]); hist.PrintI64("upper_bound", - (i == kLifetimeBuckets - 1 ? kLifetimeBucketBounds[i] - : kLifetimeBucketBounds[i + 1])); + (i == kLifetimeBuckets - 1 ? lifetime_bucket_bounds_[i] + : lifetime_bucket_bounds_[i + 1])); hist.PrintI64("value", h[i]); } } @@ -1336,11 +1358,6 @@ void HugePageFiller::PrintLifetimeHistoInPbtxt( template inline TrackerType* HugePageFiller::Put( TrackerType* pt, Range r, SpanAllocInfo span_alloc_info) { -#ifndef TCMALLOC_INTERNAL_LEGACY_LOCKING - const int64_t now = clock_.now(); -#else - const int64_t now = 0; -#endif RemoveFromFillerList(pt); pt->Put(r, span_alloc_info); if (pt->HasDenseSpans()) { @@ -1351,19 +1368,22 @@ inline TrackerType* HugePageFiller::Put( pages_allocated_[AccessDensityPrediction::kSparse] -= r.n; } - if (ABSL_PREDICT_FALSE(pt->fully_freed())) { - return HandleFullyFreedTracker(pt, now); + // If ReleasePages is unbacking pt's free pages with pageheap_lock dropped, + // pt stays in flight even if it is now empty. ReleaseCandidates retires it + // once the unback completes. + if (ABSL_PREDICT_FALSE(pt->fully_freed() && !pt->BeingReleased())) { + return HandleFullyFreedTracker(pt); } AddToFillerList(pt); - UpdateFillerStatsTracker(now); + UpdateFillerStatsTracker(); return nullptr; } template inline TrackerType* absl_nullable -HugePageFiller::HandleFullyFreedTracker(TrackerType* pt, - int64_t now) { +HugePageFiller::HandleFullyFreedTracker(TrackerType* pt) { TC_ASSERT_EQ(pt->nallocs(), 0); + TC_ASSERT(!pt->BeingReleased()); --size_; if (pt->released()) { #ifndef TCMALLOC_INTERNAL_LEGACY_LOCKING @@ -1401,19 +1421,19 @@ HugePageFiller::HandleFullyFreedTracker(TrackerType* pt, } } + RecordLifetime(pt); + 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); + } if (ABSL_PREDICT_FALSE(pt->DontFreeTracker())) { // A concurrent operation that dropped pageheap_lock still holds a pointer // to pt. Park it until the last pin is cleared (FetchFullyFreedTracker). AddToFillerList(pt); - UpdateFillerStatsTracker(now); + UpdateFillerStatsTracker(); return nullptr; } - RecordLifetime(pt, now); - UpdateFillerStatsTracker(now); - 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); - } + UpdateFillerStatsTracker(); return pt; } @@ -1442,7 +1462,7 @@ inline void HugePageFiller::Contribute( } ++size_; - UpdateFillerStatsTracker(clock_.now()); + UpdateFillerStatsTracker(); } template @@ -1458,13 +1478,21 @@ inline int HugePageFiller::SelectCandidates( // with the release, and we might collapse the pages that have been recently // released. if (pt.BeingCollapsed()) return; + // If the tracker is already a candidate of another ReleasePages call (which + // dropped pageheap_lock), leave it to that call. + if (pt.DontFreeTracker(HugePageTreatmentType::kRelease)) return; + // Candidates are pinned so that a concurrent Put that empties them cannot + // free them while ReleaseCandidates has pageheap_lock dropped. + // ReleaseCandidates unpins them. + // // 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. if (current_candidates < candidates.size()) { + pt.SetDontFreeTracker(HugePageTreatmentType::kRelease); candidates[current_candidates] = &pt; current_candidates++; @@ -1483,6 +1511,9 @@ inline int HugePageFiller::SelectCandidates( std::pop_heap(candidates.begin(), candidates.begin() + current_candidates, CompareForSubrelease); + candidates[current_candidates - 1]->ClearDontFreeTracker( + HugePageTreatmentType::kRelease); + pt.SetDontFreeTracker(HugePageTreatmentType::kRelease); candidates[current_candidates - 1] = &pt; std::push_heap(candidates.begin(), candidates.begin() + current_candidates, CompareForSubrelease); @@ -1498,48 +1529,73 @@ inline Length HugePageFiller::ReleaseCandidates( absl::Span candidates, Length target) { absl::c_sort(candidates, CompareForSubrelease); + // Snapshot before dropping pageheap_lock; a concurrent ReleasePages may + // change it. + const bool limit_hit = subrelease_stats_.limit_hit(); 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); - - // 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 + TC_ASSERT(best->DontFreeTracker(HugePageTreatmentType::kRelease)); + + // We drop pageheap_lock while unbacking, so a candidate may have changed + // state (or been emptied and parked on fully_freed_trackers_) while an + // earlier candidate was processed. Re-validate it; candidates we do not + // need are just unpinned. + if (total_released >= target || best->fully_freed() || + best->BeingCollapsed() || + best->free_pages() <= best->released_pages()) { + best->ClearDontFreeTracker(HugePageTreatmentType::kRelease); + continue; + } if (best->unbroken()) { ++total_broken; } + // Take best off the filler lists so that no other thread can allocate from + // it (or collapse it, or move it between lists) while we unback its free + // pages with pageheap_lock dropped. Account for the pages we are about to + // unback up front, as HugeCache does, so that free_pages() does not report + // memory the kernel has already reclaimed; correct for runs that failed to + // unback (or pages freed and picked up meanwhile) afterwards. RemoveFromFillerList(best); - Length ret = best->ReleaseFree(unback_); + best->SetBeingReleased(true); + AddToFillerList(best); + const Length to_release = best->free_pages() - best->released_pages(); + unmapped_ += to_release; + + Length ret = best->ReleaseFree(unback_without_lock_); + + RemoveFromFillerList(best); + best->SetBeingReleased(false); unmapped_ += ret; + unmapped_ -= to_release; 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]; + + if (best->fully_freed()) { + // A concurrent Put returned the last allocation while we were unbacking + // and deferred retiring best to us. This may drop pageheap_lock again + // to unback the remainder; best is off every list, so nothing else can + // reach it. best is still pinned, so it is parked on + // fully_freed_trackers_ for the caller to drain. + [[maybe_unused]] TrackerType* freed = HandleFullyFreedTracker(best); + TC_ASSERT_EQ(freed, nullptr); + } else { + 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]; + } } } + best->ClearDontFreeTracker(HugePageTreatmentType::kRelease); } subrelease_stats_.num_pages_subreleased += total_released; @@ -1547,7 +1603,7 @@ inline Length HugePageFiller::ReleaseCandidates( // Keep separate stats if the on going release is triggered by reaching // tcmalloc limit - if (subrelease_stats_.limit_hit()) { + if (limit_hit) { subrelease_stats_.total_pages_subreleased_due_to_limit += total_released; subrelease_stats_.total_hugepages_broken_due_to_limit += total_broken; } @@ -1556,8 +1612,8 @@ inline Length HugePageFiller::ReleaseCandidates( template inline Length HugePageFiller::FreePagesInPartialAllocs() const { - return regular_alloc_partial_released_.sparse.size().in_pages() + - regular_alloc_partial_released_.dense.size().in_pages() + + return PartialReleasedHugePages(AccessDensityPrediction::kSparse).in_pages() + + PartialReleasedHugePages(AccessDensityPrediction::kDense).in_pages() + regular_alloc_released_.sparse.size().in_pages() + regular_alloc_released_.dense.size().in_pages() - used_pages_in_any_subreleased() - unmapped_pages(); @@ -1576,7 +1632,7 @@ inline Length HugePageFiller::GetDesiredSubreleasePages( if (!intervals.SkipSubreleaseEnabled()) { return desired; } - UpdateFillerStatsTracker(clock_.now()); + UpdateFillerStatsTracker(); Length required_pages; // As mentioned above, there are two ways to calculate the demand // requirement. We give priority to using the peak if peak_interval is set. @@ -1788,9 +1844,9 @@ inline HugePageFillerStats HugePageFiller::GetStats() const { regular_alloc_released_.dense.size(); stats.n_partial_released[AccessDensityPrediction::kSparse] = - regular_alloc_partial_released_.sparse.size(); + PartialReleasedHugePages(AccessDensityPrediction::kSparse); stats.n_partial_released[AccessDensityPrediction::kDense] = - regular_alloc_partial_released_.dense.size(); + PartialReleasedHugePages(AccessDensityPrediction::kDense); stats.n_released[AccessDensityPrediction::kSparse] = stats.n_fully_released[AccessDensityPrediction::kSparse] + @@ -1961,6 +2017,10 @@ inline void HugePageFiller::TreatHugepageTrackers( template inline Length HugePageFiller::HandleReleaseFree( PageTracker* tracker) { + // Trackers that are empty or claimed by an in-flight ReleasePages are off + // the filler lists; the caller must skip them. + TC_ASSERT(!tracker->fully_freed()); + TC_ASSERT(!tracker->BeingReleased()); RemoveFromFillerList(tracker); Length released_length = tracker->ReleaseFree(unback_); subrelease_stats_.total_pages_subreleased += released_length; @@ -1972,6 +2032,8 @@ inline Length HugePageFiller::HandleReleaseFree( template inline void HugePageFiller::OnCollapseSuccess(TrackerType* pt) { + TC_ASSERT(!pt->fully_freed()); + TC_ASSERT(!pt->BeingReleased()); if (pt->unbroken()) return; RemoveFromFillerList(pt); pt->set_unbroken(/*status=*/true); @@ -1981,6 +2043,8 @@ inline void HugePageFiller::OnCollapseSuccess(TrackerType* pt) { template inline Length HugePageFiller::HandleUnbackedHugePage( PageTracker* tracker, const PageBitmap& unbacked) { + TC_ASSERT(!tracker->fully_freed()); + TC_ASSERT(!tracker->BeingReleased()); RemoveFromFillerList(tracker); Length unmapped_length = tracker->MarkSubreleased(unbacked); subrelease_stats_.total_pages_subreleased += unmapped_length; @@ -1992,7 +2056,7 @@ inline Length HugePageFiller::HandleUnbackedHugePage( template inline void HugePageFiller::Print(Printer& out, bool everything, - PageFlagsBase& pageflags) const { + PageFlagsBase& pageflags) { out.printf("HugePageFiller: densely pack small requests into hugepages\n"); const HugePageFillerStats stats = GetStats(); @@ -2232,7 +2296,7 @@ inline void HugePageFiller::PrintAllocStatsInPbtxt( template inline void HugePageFiller::PrintInPbtxt( - PbtxtRegion& hpaa, PageFlagsBase& pageflags) const { + PbtxtRegion& hpaa, PageFlagsBase& pageflags) { const HugePageFillerStats stats = GetStats(); // A donated alloc full list is impossible because it would have never been @@ -2436,18 +2500,13 @@ inline void HugePageFiller::PrintInPbtxt( } template -inline void HugePageFiller::UpdateFillerStatsTracker( - [[maybe_unused]] int64_t now) { +inline void HugePageFiller::UpdateFillerStatsTracker() { StatsTrackerType::SubreleaseStats stats; stats.num_pages = pages_allocated(); stats.free_pages = free_pages(); stats.unmapped_pages = unmapped_pages(); stats.num_pages_subreleased = subrelease_stats_.num_pages_subreleased; -#ifdef TCMALLOC_INTERNAL_LEGACY_LOCKING - fillerstats_tracker_.Report(stats, clock_.now()); -#else - fillerstats_tracker_.Report(stats, now); -#endif + fillerstats_tracker_.Report(stats); subrelease_stats_.reset(); } @@ -2502,6 +2561,17 @@ inline size_t HugePageFiller::ListFor( template inline void HugePageFiller::RemoveFromFillerList(TrackerType* pt) { + if (pt->BeingReleased()) { + const AccessDensityPrediction type = pt->HasDenseSpans() + ? AccessDensityPrediction::kDense + : AccessDensityPrediction::kSparse; + TC_ASSERT_GT(n_in_flight_release_[type], NHugePages(0)); + --n_in_flight_release_[type]; + TC_ASSERT_GE(n_used_partial_released_[type], pt->used_pages()); + n_used_partial_released_[type] -= pt->used_pages(); + return; + } + if (pt->donated()) { Length longest = pt->longest_free_range(); TC_ASSERT_LT(longest, kPagesPerHugePage); @@ -2532,13 +2602,16 @@ inline void HugePageFiller::RemoveFromFillerList(TrackerType* pt) { template inline TrackerType* absl_nullable HugePageFiller::FetchFullyFreedTracker() { - if (fully_freed_trackers_.empty()) { - return nullptr; + // A tracker stays pinned while a treatment or release that dropped + // pageheap_lock still holds a pointer to it; whoever clears the last pin + // will drain it. + for (TrackerType* pt : fully_freed_trackers_) { + if (!pt->DontFreeTracker()) { + fully_freed_trackers_.remove(pt); + return pt; + } } - - TrackerType* pt = fully_freed_trackers_.first(); - fully_freed_trackers_.remove(pt); - return pt; + return nullptr; } template @@ -2546,6 +2619,15 @@ inline void HugePageFiller::AddToFillerList(TrackerType* pt) { Length longest = pt->longest_free_range(); TC_ASSERT_LE(longest, kPagesPerHugePage); + if (pt->BeingReleased()) { + const AccessDensityPrediction type = pt->HasDenseSpans() + ? AccessDensityPrediction::kDense + : AccessDensityPrediction::kSparse; + ++n_in_flight_release_[type]; + n_used_partial_released_[type] += pt->used_pages(); + return; + } + if (longest == kPagesPerHugePage) { TC_ASSERT(pt->empty()); TC_ASSERT(pt->DontFreeTracker()); diff --git a/tcmalloc/huge_page_filler_fuzz.cc b/tcmalloc/huge_page_filler_fuzz.cc index 0d86c7203..53e557210 100644 --- a/tcmalloc/huge_page_filler_fuzz.cc +++ b/tcmalloc/huge_page_filler_fuzz.cc @@ -87,6 +87,25 @@ class MockUnback final : public MemoryModifyFunction { State& state_; }; +// Mirrors HugePageAwareAllocator::UnbackWithoutLock: drops pageheap_lock +// around the unback, which lets MockUnback's release_callback_ run reentrant +// instructions against the filler. +class MockUnbackWithoutLock final : public MemoryModifyFunction { + public: + explicit MockUnbackWithoutLock(MockUnback& unback) : unback_(unback) {} + [[nodiscard]] MemoryModifyStatus operator()(Range r) override + ABSL_NO_THREAD_SAFETY_ANALYSIS { + pageheap_lock.AssertHeld(); + pageheap_lock.unlock(); + MemoryModifyStatus ret = unback_(r); + pageheap_lock.lock(); + return ret; + } + + private: + MockUnback& unback_; +}; + class MockSetAnonVmaName final : public MemoryTagFunction { public: void operator()(Range r, std::optional name) override {} @@ -368,9 +387,10 @@ struct State { size_t num_instructions) : subrelease_unbacked_mode(subrelease_unbacked_mode), unback(*this), + unback_without_lock(unback), collapse(*this), filler(Clock{.now = mock_clock, .freq = freq}, MemoryTag::kNormal, - unback, unback, collapse, set_anon_vma_name, + unback, unback_without_lock, collapse, set_anon_vma_name, subrelease_unbacked_mode) { fake_clock = 0; output.resize(1 << 20); @@ -378,8 +398,8 @@ struct State { // 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. + // TODO(b/73749855): Releasing the pageheap_lock during HandleReleaseFree + // will eliminate the need for this. released_set.reserve(kPagesPerHugePage.raw_num() * num_instructions); auto release_callback = [this]() { @@ -408,7 +428,9 @@ struct State { } ~State() { - // Shut down, confirm filler is empty. + // Shut down, confirm filler is empty. Put may drop pageheap_lock, so + // make sure no further instructions run reentrantly while we iterate. + 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) { @@ -428,50 +450,29 @@ struct State { void RunInstructions(absl::Span instrs) { for (const auto& instruction : instrs) { std::visit([&](const auto& instr) { instr.Perform(*this); }, instruction); - if (depth == 0) { - CheckInvariants(); - } - } - } - - // Pages held by live allocations on pt. - Length LivePagesOn(PageTracker* pt) const { - Length n; - auto it = allocs.find(pt); - if (it == allocs.end()) return n; - for (const auto& [alloc, alloc_info] : it->second) { - n += alloc.n; } - return n; } - void CheckInvariants() { - PageHeapSpinLockHolder l; - TC_CHECK_EQ(filler.size().raw_num(), trackers.size()); - TC_CHECK_EQ(filler.unmapped_pages().raw_num(), released_set.size()); - // Sparse and dense allocations live on disjoint sets of hugepages, so the - // per-density counters track our live allocations exactly. - for (int d = 0; d < AccessDensityPrediction::kPredictionCounts; ++d) { - TC_CHECK_EQ( - filler.pages_allocated(static_cast(d)), - live_pages[d]); - } - TC_CHECK_LE(filler.used_pages_in_any_subreleased(), filler.used_pages()); - TC_CHECK_LE(filler.FreePagesInPartialAllocs(), filler.free_pages()); - TC_CHECK_EQ( - filler.used_pages() + filler.free_pages() + filler.unmapped_pages(), - filler.size().in_pages()); - } - - // ReleasePages may claim credit for pages unmapped earlier and left - // unaccounted, so it reports at least the pages it unmapped just now, and - // nothing is unmapped while unback is failing. - void CheckReleased(Length released, Length unmapped_before) const { - const Length unmapped_after = filler.unmapped_pages(); - TC_CHECK_GE(unmapped_after, unmapped_before); - TC_CHECK_GE(released, unmapped_after - unmapped_before); - if (!unback_success) { - TC_CHECK_EQ(unmapped_after, unmapped_before); + // Deletes trackers that were emptied by a reentrant Deallocate while the + // filler had dropped pageheap_lock. Trackers still pinned by an outer + // ReleasePages or TreatHugepageTrackers are left for that caller. + void DrainFreedTrackers() { + 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; } } @@ -487,6 +488,7 @@ struct State { absl::flat_hash_set released_set; MockUnback unback; + MockUnbackWithoutLock unback_without_lock; MockCollapse collapse; MockSetAnonVmaName set_anon_vma_name; HugePageFiller filler; @@ -496,12 +498,10 @@ struct State { std::vector>> allocs; size_t next_hugepage = 1; - // Pages held by live allocations, by predicted access density. - Length live_pages[AccessDensityPrediction::kPredictionCounts]; std::vector> reentrant_stack; int depth = 0; - // Bumped whenever a reentrant subprogram runs, so an operation can tell - // whether other instructions interleaved with it. + // Number of reentrant subprograms run so far. Postconditions that assume + // no concurrent activity are skipped when this changes during an operation. size_t reentrant_runs = 0; bool treating_trackers = false; std::string output; @@ -589,31 +589,15 @@ void Allocate::Perform(State& state) const { state.filler.Contribute(result.pt, donated, alloc_info); } state.trackers.push_back(result.pt); - } else { - // The filler only hands out hugepages it still owns. - TC_CHECK(state.allocs.contains(result.pt)); - } - - // The range lies within the tracker's hugepage and is disjoint from every - // live allocation on it. - const HugePage hp = result.pt->location(); - TC_CHECK(HugePageContaining(result.page) == hp); - TC_CHECK(result.page + n <= hp.first_page() + kPagesPerHugePage); - for (const auto& [live, live_info] : state.allocs[result.pt]) { - TC_CHECK(!(result.page < live.p + live.n && live.p < result.page + n)); } for (PageId p = result.page, end = p + n; p != end; ++p) { - // Only a previously released hugepage can hand out unmapped pages. - TC_CHECK(result.from_released || !state.released_set.contains(p)); state.released_set.erase(p); } state.allocs[result.pt].push_back({{result.page, n}, alloc_info}); - state.live_pages[alloc_info.density] += n; if (state.depth == 0) { - TC_CHECK_EQ(result.pt->used_pages(), state.LivePagesOn(result.pt)); TC_CHECK_EQ(state.filler.size().raw_num(), state.trackers.size()); TC_CHECK_EQ(state.filler.unmapped_pages().raw_num(), state.released_set.size()); @@ -639,7 +623,6 @@ void Deallocate::Perform(State& state) const { state.trackers.resize(state.trackers.size() - 1); } - state.live_pages[alloc_info.density] -= alloc.n; PageTracker* ret; { PageHeapSpinLockHolder l; @@ -647,13 +630,8 @@ void Deallocate::Perform(State& state) const { } if (state.depth == 0) { TC_CHECK_EQ(ret != nullptr, last_alloc); - if (ret == nullptr) { - TC_CHECK_EQ(pt->used_pages(), state.LivePagesOn(pt)); - } } if (ret) { - // Only the hugepage we emptied is handed back. - TC_CHECK_EQ(ret, pt); HugePage hp = ret->location(); for (PageId p = hp.first_page(), end = hp.first_page() + kPagesPerHugePage; p != end; ++p) { @@ -685,9 +663,8 @@ void Release::Perform(State& state) const { Length desired(desired_pages); size_t to_release_from_partial_allocs; - const Length unmapped_before = state.filler.unmapped_pages(); - const size_t runs_before = state.reentrant_runs; Length released; + const size_t reentrant_runs = state.reentrant_runs; { PageHeapSpinLockHolder l; to_release_from_partial_allocs = @@ -696,13 +673,12 @@ void Release::Perform(State& state) const { released = state.filler.ReleasePages(desired, skip_subrelease_intervals, release_partial_allocs, hit_limit); } - if (state.depth == 0 && runs_before == state.reentrant_runs) { - state.CheckReleased(released, unmapped_before); - } + state.DrainFreedTrackers(); if (!release_partial_allocs || hit_limit || skip_subrelease_intervals.SkipSubreleaseEnabled() || - !state.unback_success || state.depth != 0) { + !state.unback_success || state.depth != 0 || + state.reentrant_runs != reentrant_runs) { return; } TC_CHECK_GE(released.raw_num(), to_release_from_partial_allocs); @@ -752,7 +728,6 @@ void ModelTail::Perform(State& state) const { state.allocs[pt].push_back( {{start, n}, {1, AccessDensityPrediction::kSparse}}); - state.live_pages[AccessDensityPrediction::kSparse] += n; if (state.depth == 0) { TC_CHECK_EQ(state.filler.size().raw_num(), state.trackers.size()); @@ -765,20 +740,17 @@ void MemoryLimitHitRelease::Perform(State& state) const { Length desired_len(desired); Length released; const Length free = state.filler.free_pages(); - const Length unmapped_before = state.filler.unmapped_pages(); - const size_t runs_before = state.reentrant_runs; + const size_t reentrant_runs = state.reentrant_runs; { PageHeapSpinLockHolder l; released = state.filler.ReleasePages(desired_len, SkipSubreleaseIntervals{}, /*release_partial_alloc_pages=*/false, /*hit_limit=*/true); } - if (state.depth != 0) { + state.DrainFreedTrackers(); + if (state.depth != 0 || state.reentrant_runs != reentrant_runs) { return; } - if (runs_before == state.reentrant_runs) { - state.CheckReleased(released, unmapped_before); - } const Length expected = state.unback_success ? std::min(free, desired_len) : Length(0); TC_CHECK_GE(released.raw_num(), expected.raw_num()); @@ -810,23 +782,19 @@ 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.DrainFreedTrackers(); + PageHeapSpinLockHolder l; for (PageTracker* pt : state.trackers) { HugePage hp = pt->location(); const PageBitmap& rel = pt->released_by_page(); @@ -1439,6 +1407,41 @@ TEST(HugePageFillerTest, Regression_b525818096) { SubreleaseUnbackedMode::kDisabled); } +// Stats gathered from inside a limit-hit release, and an allocation from +// inside a later one, with SubreleaseUnbackedMode::kEnabled. +TEST(HugePageFillerTest, ReentrantStatsDuringLimitHitRelease) { + FuzzFiller( + {SetCollapseLatency{.latency = absl::Nanoseconds(9223372036854775807)}, + ReentrantSubprogram{.subprogram = {}}, GatherStatsPbtxt{}, + GatherStatsPbtxt{}, + Allocate{ + .length = 14317, .num_objects = 3536510400, .density_dense = false}, + Release{.hit_limit = true, + .use_peak_interval = false, + .peak_interval = absl::Nanoseconds(9223372036854775807), + .short_interval = absl::Nanoseconds(1), + .long_interval = absl::Nanoseconds(9223372036854775807), + .desired_pages = 32767, + .release_partial_allocs = false}, + ReentrantSubprogram{.subprogram = {ReentrantSubprogram{.subprogram = {}}, + GatherStatsPbtxt{}}}, + GatherStats{}, + ReentrantSubprogram{ + .subprogram = {Allocate{ + .length = 0, .num_objects = 1, .density_dense = true}}}}, + SubreleaseUnbackedMode::kEnabled); +} + +// A deallocation from inside a memory-limit release with two nearly full +// hugepages. +TEST(HugePageFillerTest, ReentrantDeallocateDuringMemoryLimitRelease) { + FuzzFiller({ModelTail{.length = 511}, ModelTail{.length = 511}, + ReentrantSubprogram{.subprogram = {Deallocate{.tracker_index = 0, + .alloc_index = 0}}}, + MemoryLimitHitRelease{.desired = 2}}, + SubreleaseUnbackedMode::kEnabled); +} + TEST(HugePageFillerTest, b547364068) { FuzzFiller( {GatherStats{}, diff --git a/tcmalloc/huge_page_filler_test.cc b/tcmalloc/huge_page_filler_test.cc index 876c8b38f..e433c0303 100644 --- a/tcmalloc/huge_page_filler_test.cc +++ b/tcmalloc/huge_page_filler_test.cc @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -287,9 +288,15 @@ class MockCollapse final : public MemoryModifyFunction { EXPECT_EQ(r.n, kPagesPerHugePage); ++collapsed_[r.start_addr()]; FakeClock::Advance(latency_); + if (unlocked_hook_ != nullptr) { + unlocked_hook_(r); + } return {.success = success_, .error_number = error_number_}; } + // Runs during the collapse, i.e. while pageheap_lock is dropped. + std::function unlocked_hook_; + bool TriedCollapse(void* addr) const { PageId p = PageIdContaining(addr); HugePage hp = HugePageContaining(p); @@ -365,6 +372,35 @@ class BlockingUnback final : public MemoryModifyFunction { thread_local absl::Mutex* BlockingUnback::mu_ = nullptr; +// Mirrors HugePageAwareAllocator::UnbackWithoutLock: drops pageheap_lock +// around the unback. Tests may install a hook that runs while the lock is +// dropped to interleave other filler operations with the release in progress. +class BlockingUnbackWithoutLock final : public MemoryModifyFunction { + public: + explicit BlockingUnbackWithoutLock( + BlockingUnback& unback ABSL_ATTRIBUTE_LIFETIME_BOUND) + : unback_(unback) {} + + [[nodiscard]] MemoryModifyStatus operator()(Range r) override + ABSL_NO_THREAD_SAFETY_ANALYSIS { + pageheap_lock.AssertHeld(); + pageheap_lock.unlock(); + if (unlocked_hook_ != nullptr) { + unlocked_hook_(r); + } + MemoryModifyStatus ret = unback_(r); + pageheap_lock.lock(); + return ret; + } + + // Runs with pageheap_lock dropped. The caller may still hold an + // AllocationGuard, so the hook must not allocate. + std::function unlocked_hook_; + + private: + BlockingUnback& unback_; +}; + class FillerTest : public testing::Test { protected: // We have backing of one word per (normal-sized) page for our "hugepages". @@ -410,6 +446,7 @@ class FillerTest : public testing::Test { SubreleaseUnbackedMode mode_ = SubreleaseUnbackedMode::kDisabled; HugePageFiller filler_; BlockingUnback blocking_unback_; + BlockingUnbackWithoutLock blocking_unback_without_lock_{blocking_unback_}; MockCollapse collapse_; MockSetAnonVmaName set_anon_vma_name_; @@ -417,14 +454,36 @@ 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) { + MemoryTag::kNormal, blocking_unback_, + blocking_unback_without_lock_, collapse_, set_anon_vma_name_, + mode) { // Reset success state blocking_unback_.success_ = true; } ~FillerTest() override { EXPECT_EQ(filler_.size(), NHugePages(0)); } + // Deletes every tracker the filler emptied while it did not hold + // pageheap_lock. Returns the number of trackers deleted. + int DrainFreedTrackers() { + 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; + } + } + struct PAlloc { PageTracker* pt; PageId p; @@ -1369,20 +1428,546 @@ TEST_F(FillerTest, ParallelCollapseRelease) { done = true; collapse_thread.join(); - while (true) { - PageTracker* pt; - { + DrainFreedTrackers(); + CheckStats(); +} + +// ReleasePages drops pageheap_lock while unbacking. The tests below use +// BlockingUnbackWithoutLock::unlocked_hook_ to run other filler operations at +// exactly that point. + +TEST_F(FillerTest, ReleaseDropsLockDuringUnback) { + randomize_density_ = false; + PAlloc a = Allocate(Length(1)); + + int calls = 0; + blocking_unback_without_lock_.unlocked_hook_ = [&](Range r) { + ++calls; + EXPECT_FALSE(pageheap_lock.IsHeld()); + EXPECT_EQ(r.n, kPagesPerHugePage - Length(1)); + + PageHeapSpinLockHolder l; + EXPECT_TRUE(a.pt->BeingReleased()); + // The tracker being released is not eligible for allocation. + EXPECT_EQ(filler_.TryGet(Length(1), a.span_alloc_info).pt, nullptr); + // The pages being unbacked are already accounted as unmapped. + EXPECT_EQ(filler_.unmapped_pages(), kPagesPerHugePage - Length(1)); + EXPECT_EQ(filler_.free_pages(), Length(0)); + EXPECT_EQ(filler_.used_pages(), Length(1)); + // The in-flight tracker still counts as a partially released hugepage, + // with none of its free pages available for a partial release. + EXPECT_EQ(filler_.FreePagesInPartialAllocs(), Length(0)); + const HugePageFillerStats stats = filler_.GetStats(); + EXPECT_EQ(stats.n_partial_released[AccessDensityPrediction::kSparse], + NHugePages(1)); + EXPECT_EQ(stats.n_released[AccessDensityPrediction::kSparse], + NHugePages(1)); + EXPECT_EQ(stats.n_total[AccessDensityPrediction::kSparse], NHugePages(1)); + EXPECT_EQ(stats.n_full[AccessDensityPrediction::kSparse], NHugePages(0)); + }; + EXPECT_EQ(ReleasePages(kPagesPerHugePage), kPagesPerHugePage - Length(1)); + blocking_unback_without_lock_.unlocked_hook_ = nullptr; + EXPECT_EQ(calls, 1); + + EXPECT_FALSE(a.pt->BeingReleased()); + EXPECT_TRUE(a.pt->released()); + EXPECT_EQ(filler_.unmapped_pages(), kPagesPerHugePage - Length(1)); + CheckStats(); + Delete(a); +} + +TEST_F(FillerTest, PutDuringReleaseDefersFreeingTracker) { + randomize_density_ = false; + PAlloc a = Allocate(Length(1)); + + int calls = 0; + blocking_unback_without_lock_.unlocked_hook_ = [&](Range r) { + if (calls++ > 0) { + // The rest of the hugepage is unbacked once the tracker is retired. By + // then the tracker has left the filler's accounting entirely. + EXPECT_EQ(r.n, kPagesPerHugePage); PageHeapSpinLockHolder l; - pt = filler_.FetchFullyFreedTracker(); + EXPECT_EQ(filler_.size(), NHugePages(0)); + EXPECT_EQ(filler_.unmapped_pages(), Length(0)); + EXPECT_EQ(filler_.free_pages(), Length(0)); + return; } - if (pt == nullptr) { - break; + PageHeapSpinLockHolder l; + // The tracker is being released, so Put must not hand it back to us even + // though it is now empty. + EXPECT_EQ(filler_.Put(a.pt, Range(a.p, a.n), a.span_alloc_info), nullptr); + EXPECT_TRUE(a.pt->empty()); + EXPECT_TRUE(a.pt->BeingReleased()); + }; + EXPECT_EQ(ReleasePages(kPagesPerHugePage), kPagesPerHugePage - Length(1)); + blocking_unback_without_lock_.unlocked_hook_ = nullptr; + EXPECT_EQ(calls, 2); + // a was returned with Put directly rather than Delete. + total_allocated_ -= a.n; + + // The release retired the tracker and parked it for us. + EXPECT_EQ(filler_.size(), NHugePages(0)); + EXPECT_EQ(filler_.unmapped_pages(), Length(0)); + EXPECT_FALSE(a.pt->BeingReleased()); + EXPECT_EQ(DrainFreedTrackers(), 1); + CheckStats(); +} + +TEST_F(FillerTest, CandidateFreedDuringRelease) { + randomize_density_ = false; + PAlloc a = Allocate(Length(1)); + PAlloc b = Allocate(Length(1), /*donated=*/true); + ASSERT_NE(a.pt, b.pt); + + // Both trackers are candidates. While the first is being released, return + // the allocation on the second. It is pinned as a candidate, so it must be + // parked rather than handed back, and skipped when its turn comes. + PageTracker* parked = nullptr; + int calls = 0; + blocking_unback_without_lock_.unlocked_hook_ = [&](Range r) { + if (calls++ > 0) { + return; + } + PageHeapSpinLockHolder l; + const PAlloc& other = a.pt->BeingReleased() ? b : a; + EXPECT_FALSE(other.pt->BeingReleased()); + parked = other.pt; + EXPECT_EQ( + filler_.Put(other.pt, Range(other.p, other.n), other.span_alloc_info), + nullptr); + EXPECT_TRUE(other.pt->empty()); + }; + EXPECT_EQ(ReleasePages(kPagesPerHugePage), kPagesPerHugePage - Length(1)); + blocking_unback_without_lock_.unlocked_hook_ = nullptr; + // The parked tracker had nothing released, so it was not unbacked. + EXPECT_EQ(calls, 1); + ASSERT_NE(parked, nullptr); + // The parked allocation was returned with Put directly rather than Delete. + total_allocated_ -= Length(1); + + EXPECT_EQ(DrainFreedTrackers(), 1); + const PAlloc& released = (parked == a.pt) ? b : a; + EXPECT_TRUE(released.pt->released()); + EXPECT_FALSE(released.pt->BeingReleased()); + EXPECT_EQ(filler_.unmapped_pages(), kPagesPerHugePage - Length(1)); + CheckStats(); + Delete(released); +} + +TEST_F(FillerTest, NestedReleaseDuringRelease) { + randomize_density_ = false; + PAlloc a = Allocate(Length(1)); + PAlloc b = Allocate(Length(1), /*donated=*/true); + ASSERT_NE(a.pt, b.pt); + + // Both trackers are candidates of the outer release, so a nested release + // must leave them alone. + int calls = 0; + blocking_unback_without_lock_.unlocked_hook_ = [&](Range r) { + if (calls++ > 0) { + return; + } + PageHeapSpinLockHolder l; + EXPECT_EQ(filler_.ReleasePages(kPagesPerHugePage, SkipSubreleaseIntervals{}, + /*release_partial_alloc_pages=*/false, + /*hit_limit=*/true), + Length(0)); + }; + EXPECT_EQ(ReleasePages(2 * kPagesPerHugePage), + 2 * (kPagesPerHugePage - Length(1))); + blocking_unback_without_lock_.unlocked_hook_ = nullptr; + EXPECT_EQ(calls, 2); + + EXPECT_TRUE(a.pt->released()); + EXPECT_TRUE(b.pt->released()); + EXPECT_EQ(filler_.unmapped_pages(), 2 * (kPagesPerHugePage - Length(1))); + // The nested release hit the limit, but released nothing; the outer release + // must not be attributed to it. + EXPECT_EQ(filler_.subrelease_stats().total_pages_subreleased_due_to_limit, + Length(0)); + CheckStats(); + Delete(a); + Delete(b); +} + +TEST_F(FillerTest, PutDuringReleaseKeepsAccounting) { + randomize_density_ = false; + PAlloc a = Allocate(Length(1)); + PAlloc b = Allocate(Length(1)); + ASSERT_EQ(a.pt, b.pt); + + // Return b while a's hugepage is being released. b's page was allocated + // when the release scanned the hugepage, so it is not part of the in-flight + // run: it stays backed and free. + int calls = 0; + blocking_unback_without_lock_.unlocked_hook_ = [&](Range r) { + if (calls++ > 0) { + return; + } + EXPECT_EQ(r.n, kPagesPerHugePage - Length(2)); + PageHeapSpinLockHolder l; + EXPECT_TRUE(b.pt->BeingReleased()); + EXPECT_EQ(filler_.Put(b.pt, Range(b.p, b.n), b.span_alloc_info), nullptr); + EXPECT_FALSE(b.pt->fully_freed()); + EXPECT_TRUE(b.pt->BeingReleased()); + EXPECT_EQ(filler_.used_pages(), Length(1)); + EXPECT_EQ(filler_.free_pages(), Length(1)); + EXPECT_EQ(filler_.unmapped_pages(), kPagesPerHugePage - Length(2)); + }; + EXPECT_EQ(ReleasePages(kPagesPerHugePage), kPagesPerHugePage - Length(2)); + blocking_unback_without_lock_.unlocked_hook_ = nullptr; + EXPECT_EQ(calls, 1); + // b was returned with Put directly rather than Delete. + total_allocated_ -= b.n; + + EXPECT_FALSE(a.pt->BeingReleased()); + EXPECT_EQ(a.pt->released_pages(), kPagesPerHugePage - Length(2)); + EXPECT_EQ(filler_.unmapped_pages(), kPagesPerHugePage - Length(2)); + EXPECT_EQ(filler_.free_pages(), Length(1)); + CheckStats(); + + // The page freed mid-release is picked up by the next release. + 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)); +} + +TEST_F(FillerTest, AllCandidatesFreedDuringRelease) { + randomize_density_ = false; + PAlloc a = Allocate(Length(1)); + PAlloc b = Allocate(Length(1), /*donated=*/true); + ASSERT_NE(a.pt, b.pt); + + // Return both allocations while the first candidate is being released: the + // in-flight tracker is retired by the release once its unback completes, the + // pending one is parked immediately and skipped when its turn comes. Both + // are handed to us afterwards. + int calls = 0; + blocking_unback_without_lock_.unlocked_hook_ = [&](Range r) { + if (calls++ > 0) { + // The remainder of the in-flight hugepage. + EXPECT_EQ(r.n, kPagesPerHugePage); + return; + } + EXPECT_EQ(r.n, kPagesPerHugePage - Length(1)); + PageHeapSpinLockHolder l; + EXPECT_EQ(filler_.Put(a.pt, Range(a.p, a.n), a.span_alloc_info), nullptr); + EXPECT_EQ(filler_.Put(b.pt, Range(b.p, b.n), b.span_alloc_info), nullptr); + EXPECT_TRUE(a.pt->fully_freed()); + EXPECT_TRUE(b.pt->fully_freed()); + EXPECT_EQ(filler_.FetchFullyFreedTracker(), nullptr); + }; + EXPECT_EQ(ReleasePages(2 * kPagesPerHugePage), kPagesPerHugePage - Length(1)); + blocking_unback_without_lock_.unlocked_hook_ = nullptr; + EXPECT_EQ(calls, 2); + total_allocated_ -= a.n + b.n; + + EXPECT_FALSE(a.pt->BeingReleased()); + EXPECT_FALSE(b.pt->BeingReleased()); + EXPECT_FALSE(a.pt->DontFreeTracker()); + EXPECT_FALSE(b.pt->DontFreeTracker()); + EXPECT_EQ(filler_.size(), NHugePages(0)); + EXPECT_EQ(filler_.unmapped_pages(), Length(0)); + EXPECT_EQ(DrainFreedTrackers(), 2); + CheckStats(); +} + +TEST_F(FillerTest, ReleaseTargetMetLeavesCandidatesInPlace) { + randomize_density_ = false; + PAlloc a = Allocate(Length(2)); + PAlloc b = Allocate(Length(1), /*donated=*/true); + ASSERT_NE(a.pt, b.pt); + + // Both hugepages are candidates; b's (less used) is released first and + // satisfies the target by itself. While it is in flight, a's hugepage is + // pinned as a pending candidate but must remain available to allocations. + int calls = 0; + HugePageFiller::TryGetResult got; + blocking_unback_without_lock_.unlocked_hook_ = [&](Range r) { + if (calls++ > 0) { + return; + } + PageHeapSpinLockHolder l; + EXPECT_TRUE(b.pt->BeingReleased()); + EXPECT_FALSE(a.pt->BeingReleased()); + EXPECT_TRUE(a.pt->DontFreeTracker(HugePageTreatmentType::kRelease)); + got = filler_.TryGet(Length(1), a.span_alloc_info); + EXPECT_EQ(got.pt, a.pt); + }; + EXPECT_EQ(ReleasePages(kPagesPerHugePage - Length(1)), + kPagesPerHugePage - Length(1)); + blocking_unback_without_lock_.unlocked_hook_ = nullptr; + EXPECT_EQ(calls, 1); + ASSERT_EQ(got.pt, a.pt); + const PAlloc c = {.pt = got.pt, + .p = got.page, + .n = Length(1), + .mark = 0, + .span_alloc_info = a.span_alloc_info, + .from_released = got.from_released}; + total_allocated_ += c.n; + + // The pending candidate was unpinned without being released and is still + // where allocations find it first. + EXPECT_FALSE(a.pt->DontFreeTracker()); + EXPECT_FALSE(a.pt->released()); + EXPECT_TRUE(b.pt->released()); + EXPECT_EQ(filler_.unmapped_pages(), kPagesPerHugePage - Length(1)); + CheckStats(); + PAlloc d = Allocate(Length(1)); + EXPECT_EQ(d.pt, a.pt); + + EXPECT_FALSE(Delete(d)); + EXPECT_FALSE(DeleteRaw(c)); + EXPECT_TRUE(Delete(a)); + EXPECT_TRUE(Delete(b)); +} + +TEST_F(FillerTest, TreatmentReleasesPendingCandidateDuringRelease) { + randomize_density_ = false; + PAlloc a = Allocate(Length(2)); + PAlloc b = Allocate(Length(1), /*donated=*/true); + ASSERT_NE(a.pt, b.pt); + + // A treatment pass runs while b's hugepage is in flight and a's is a pending + // candidate. a's hugepage has a swapped page, so the pass releases its free + // pages (with pageheap_lock held). The release must then find nothing left + // to do on a's hugepage. + FakePageFlags pageflags; + FakeResidency residency; + for (const PAlloc& p : {a, b}) { + pageflags.MarkHugePageBacked(p.p.start_addr(), + /*is_hugepage_backed=*/false); + pageflags.SetStaleBitmap(p.p.start_addr(), {}); + } + Bitmap unbacked, swapped; + swapped.SetRange(/*index=*/1, /*n=*/1); + residency.SetUnbackedAndSwappedBitmaps(a.p.start_addr(), unbacked, swapped); + residency.SetUnbackedAndSwappedBitmaps(b.p.start_addr(), {}, {}); + + int calls = 0; + blocking_unback_without_lock_.unlocked_hook_ = [&](Range r) { + if (calls++ > 0) { + return; } - EXPECT_EQ(pt->longest_free_range(), kPagesPerHugePage); - EXPECT_TRUE(pt->empty()); - --hp_contained_; - delete pt; + EXPECT_EQ(HugePageContaining(r.p), b.pt->location()); + EXPECT_TRUE(b.pt->BeingReleased()); + EXPECT_TRUE(a.pt->DontFreeTracker(HugePageTreatmentType::kRelease)); + TreatHugepageTrackers(EnableCollapse::kDisabled, + EnableUnfilteredCollapse::kDisabled, + ReleaseStalePages::kDisabled, &pageflags, &residency); + EXPECT_TRUE(a.pt->released()); + EXPECT_EQ(a.pt->released_pages(), kPagesPerHugePage - Length(2)); + // Still pinned by the release in progress. + EXPECT_TRUE(a.pt->DontFreeTracker(HugePageTreatmentType::kRelease)); + }; + EXPECT_EQ(ReleasePages(2 * kPagesPerHugePage), kPagesPerHugePage - Length(1)); + blocking_unback_without_lock_.unlocked_hook_ = nullptr; + EXPECT_EQ(calls, 1); + + EXPECT_EQ(GetHugePageTreatmentStats().treated_pages_subreleased, + (kPagesPerHugePage - Length(2)).raw_num()); + EXPECT_FALSE(a.pt->DontFreeTracker()); + EXPECT_FALSE(b.pt->DontFreeTracker()); + EXPECT_EQ(filler_.unmapped_pages(), 2 * kPagesPerHugePage - Length(3)); + CheckStats(); + + EXPECT_TRUE(Delete(a)); + EXPECT_TRUE(Delete(b)); +} + +TEST_F(FillerTest, CollapseDuringRelease) { + randomize_density_ = false; + PAlloc a = Allocate(Length(2)); + PAlloc b = Allocate(Length(1), /*donated=*/true); + ASSERT_NE(a.pt, b.pt); + + // A treatment pass with collapse enabled runs while b's hugepage is in + // flight and a's is a pending candidate. The in-flight hugepage is off the + // filler lists and cannot be selected; the pending one may be collapsed, and + // is then released like any other candidate once the collapse has finished. + FakePageFlags pageflags; + FakeResidency residency; + for (const PAlloc& p : {a, b}) { + pageflags.MarkHugePageBacked(p.p.start_addr(), + /*is_hugepage_backed=*/false); + pageflags.SetStaleBitmap(p.p.start_addr(), {}); + residency.SetUnbackedAndSwappedBitmaps(p.p.start_addr(), {}, {}); } + + int calls = 0; + blocking_unback_without_lock_.unlocked_hook_ = [&](Range r) { + if (calls++ > 0) { + return; + } + EXPECT_EQ(HugePageContaining(r.p), b.pt->location()); + EXPECT_TRUE(b.pt->BeingReleased()); + TreatHugepageTrackers(EnableCollapse::kEnabled, + EnableUnfilteredCollapse::kDisabled, + ReleaseStalePages::kDisabled, &pageflags, &residency); + EXPECT_FALSE(collapse_.TriedCollapse(b.p.start_addr())); + EXPECT_TRUE(collapse_.TriedCollapse(a.p.start_addr())); + EXPECT_FALSE(a.pt->BeingCollapsed()); + }; + // MockCollapse allocates, so pageheap_lock is taken manually rather than + // through PageHeapSpinLockHolder here. + pageheap_lock.lock(); + const Length released = filler_.ReleasePages( + 2 * kPagesPerHugePage, SkipSubreleaseIntervals{}, + /*release_partial_alloc_pages=*/false, /*hit_limit=*/false); + pageheap_lock.unlock(); + EXPECT_EQ(released, 2 * kPagesPerHugePage - Length(3)); + blocking_unback_without_lock_.unlocked_hook_ = nullptr; + EXPECT_EQ(calls, 2); + + EXPECT_EQ(GetHugePageTreatmentStats().collapse_eligible, 1); + EXPECT_TRUE(a.pt->released()); + EXPECT_TRUE(b.pt->released()); + CheckStats(); + + EXPECT_TRUE(Delete(a)); + EXPECT_TRUE(Delete(b)); +} + +TEST_F(FillerTest, ReleaseDuringCollapse) { + randomize_density_ = false; + PAlloc a = Allocate(Length(1)); + + // Conversely, a release that runs while a's hugepage is being collapsed must + // leave it alone: collapsing requires that none of its pages are released. + FakePageFlags pageflags; + FakeResidency residency; + pageflags.MarkHugePageBacked(a.p.start_addr(), /*is_hugepage_backed=*/false); + pageflags.SetStaleBitmap(a.p.start_addr(), {}); + residency.SetUnbackedAndSwappedBitmaps(a.p.start_addr(), {}, {}); + + int calls = 0; + collapse_.unlocked_hook_ = [&](Range r) { + ++calls; + EXPECT_EQ(HugePageContaining(r.p), a.pt->location()); + EXPECT_TRUE(a.pt->BeingCollapsed()); + EXPECT_EQ(ReleasePages(kPagesPerHugePage), Length(0)); + EXPECT_EQ(HardReleasePages(kPagesPerHugePage), Length(0)); + EXPECT_FALSE(a.pt->released()); + }; + TreatHugepageTrackers(EnableCollapse::kEnabled, + EnableUnfilteredCollapse::kDisabled, + ReleaseStalePages::kDisabled, &pageflags, &residency); + collapse_.unlocked_hook_ = nullptr; + EXPECT_EQ(calls, 1); + EXPECT_FALSE(a.pt->BeingCollapsed()); + EXPECT_FALSE(a.pt->released()); + EXPECT_EQ(filler_.unmapped_pages(), Length(0)); + CheckStats(); + + EXPECT_EQ(ReleasePages(kPagesPerHugePage), kPagesPerHugePage - Length(1)); + EXPECT_TRUE(Delete(a)); +} + +// Releases and collapses from the background while allocations are returned +// and made concurrently. Every operation that drops pageheap_lock must cope +// with the trackers changing underneath it. +TEST_F(FillerTest, ParallelReleaseCollapseAndFree) { + std::atomic done(false); + + FakePageFlags pageflags; + FakeResidency residency; + SpanAllocInfo info; + info.objects_per_span = 1; + info.density = AccessDensityPrediction::kSparse; + std::vector allocated; + const Length kAlloc = kPagesPerHugePage / 2 + Length(1); + // Some trackers are sampled so that retiring them names their VMA; some + // have swapped pages and others stale pages so that treatment passes release + // from them through both paths. + set_anon_vma_name_.SetIgnoreName(true); + for (int i = 0; i < 1000; ++i) { + PAlloc p1 = AllocateWithSpanAllocInfo(kAlloc, info); + allocated.push_back(p1); + p1.pt->SetTagState({.sampled_for_tagging = (i % 8 == 0)}); + pageflags.MarkHugePageBacked(p1.p.start_addr(), + /*is_hugepage_backed=*/false); + Bitmap unbacked, swapped, stale; + unbacked.SetRange(/*index=*/0, /*n=*/128); + if (i % 2 == 0) { + swapped.SetRange(/*index=*/128, /*n=*/128); + } else { + stale.SetRange(/*index=*/128, /*n=*/128); + } + residency.SetUnbackedAndSwappedBitmaps(p1.p.start_addr(), unbacked, + swapped); + pageflags.SetStaleBitmap(p1.p.start_addr(), stale); + } + + // Widen the window in which the lock is dropped. + std::atomic drops(0); + blocking_unback_without_lock_.unlocked_hook_ = [&](Range) { + drops.fetch_add(1, std::memory_order_relaxed); + std::this_thread::yield(); + }; + std::thread release_thread([&]() { + while (!done.load(std::memory_order_acquire)) { + HardReleasePages(kPagesPerHugePage); + ReleasePartialPages(kPagesPerHugePage); + } + }); + std::thread collapse_thread([&]() { + while (!done.load(std::memory_order_acquire)) { + TreatHugepageTrackers( + EnableCollapse::kEnabled, EnableUnfilteredCollapse::kDisabled, + ReleaseStalePages::kEnabled, &pageflags, &residency); + } + }); + + // Wait for the first release to drop the lock so that the loop below + // overlaps with releases rather than running before the thread starts. + while (drops.load(std::memory_order_relaxed) == 0) { + std::this_thread::yield(); + } + + // See ParallelCollapseRelease for why DeleteRaw is used here. Deallocations + // are interleaved with small allocations from existing hugepages so that + // trackers move between lists while releases are in flight. + for (int i = 0; i < 20000; ++i) { + if (absl::Bernoulli(gen_, 0.4)) { + PageTracker* pt; + PageId page; + { + PageHeapSpinLockHolder l; + auto result = filler_.TryGet(Length(1), info); + pt = result.pt; + page = result.page; + } + if (pt != nullptr) { + total_allocated_ += Length(1); + allocated.push_back(PAlloc{.pt = pt, + .p = page, + .n = Length(1), + .mark = 0, + .span_alloc_info = info, + .from_released = false}); + } + } else if (!allocated.empty()) { + const size_t idx = absl::Uniform(gen_, 0, allocated.size()); + std::swap(allocated[idx], allocated.back()); + DeleteRaw(allocated.back()); + allocated.pop_back(); + } + } + for (const PAlloc& p : allocated) { + DeleteRaw(p); + } + + done = true; + release_thread.join(); + collapse_thread.join(); + blocking_unback_without_lock_.unlocked_hook_ = nullptr; + + DrainFreedTrackers(); CheckStats(); } @@ -6612,20 +7197,7 @@ TEST_F(FillerTest, ConcurrentTreatmentInterferenceStress) { 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; - } + DrainFreedTrackers(); CheckStats(); } diff --git a/tcmalloc/huge_page_options.h b/tcmalloc/huge_page_options.h index c9eb0e998..af9ef4393 100644 --- a/tcmalloc/huge_page_options.h +++ b/tcmalloc/huge_page_options.h @@ -25,6 +25,9 @@ namespace tcmalloc::tcmalloc_internal { enum class HugePageTreatmentType : uint8_t { kSampled = 1 << 0, kCollapse = 1 << 1, + // Selected as a candidate by HugePageFiller::ReleasePages, which drops + // pageheap_lock while unbacking. + kRelease = 1 << 2, }; enum class EnableCollapse : uint8_t { diff --git a/tcmalloc/huge_page_tracker.h b/tcmalloc/huge_page_tracker.h index 0e5f15d21..403511d1d 100644 --- a/tcmalloc/huge_page_tracker.h +++ b/tcmalloc/huge_page_tracker.h @@ -302,6 +302,9 @@ class PageTracker : public TList::Elem { return hugepage_residency_state_.being_collapsed; } + void SetBeingReleased(bool value) { being_released_ = value; } + bool BeingReleased() const { return being_released_; } + void SetDontFreeTracker(HugePageTreatmentType type) { dont_free_tracker_mask_ |= static_cast(type); } @@ -309,6 +312,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; @@ -352,6 +358,10 @@ class PageTracker : public TList::Elem { bool abandoned_; bool unbroken_; bool has_dense_spans_ = false; + // Set while HugePageFiller::ReleasePages unbacks free pages of this tracker + // with pageheap_lock dropped. The filler keeps such trackers off its lists + // (see HugePageFiller::AddToFillerList) and Collapse() leaves them alone. + bool being_released_ = false; // This field is used to avoid freeing this tracker prematurely. When this // is set, any maintenance operation (e.g. collapse) that drops // pageheap_lock might manipulate the tracker state without holding the @@ -491,8 +501,12 @@ inline Length PageTracker::ReleaseFree(MemoryModifyFunction& unback) { PageId p = location_.first_page() + Length(free_index); if (ABSL_PREDICT_TRUE(ReleasePages(Range(p, Length(length)), unback))) { - // Mark pages as released. Amortize the update to release_count_. + // Mark pages as released. unback may have dropped pageheap_lock, so + // keep released_count_ in sync with released_by_page_ whenever the + // lock is held: released() and released_pages() are read under the + // lock by other threads (e.g. Collapse(), stats). released_by_page_.SetRange(free_index, length); + released_count_ += length; count += length; } @@ -504,7 +518,6 @@ inline Length PageTracker::ReleaseFree(MemoryModifyFunction& unback) { } } - released_count_ += count; if (count > 0) { hugepage_residency_state_.maybe_hugepage_backed = false; } @@ -542,8 +555,11 @@ inline MemoryModifyStatus PageTracker::Collapse( // store the being_collapsed state. { PageHeapSpinLockHolder l; - // If the tracker is in the released state, we do no want to collapse it. - if (released()) return {.success = false, .error_number = 0}; + // If the tracker is in the released state, or is about to be, we do not + // want to collapse it. + if (released() || BeingReleased()) { + return {.success = false, .error_number = 0}; + } TC_ASSERT(!BeingCollapsed()); SetBeingCollapsed(/*value=*/true); } diff --git a/tcmalloc/huge_page_treatment.h b/tcmalloc/huge_page_treatment.h index 1239bfc89..f12937924 100644 --- a/tcmalloc/huge_page_treatment.h +++ b/tcmalloc/huge_page_treatment.h @@ -489,7 +489,10 @@ class HugePageUnbackedTrackerTreatment final : public HugePageTreatment { PageTracker* tracker = residency_states_[i].tracker; TC_ASSERT_NE(tracker, nullptr); tracker->ClearDontFreeTracker(HugePageTreatmentType::kCollapse); - if (tracker->fully_freed()) { + // The tracker may have been emptied, or claimed by a concurrent + // ReleasePages that dropped pageheap_lock, while we did not hold the + // lock. Either way it is off the filler lists; leave it alone. + if (tracker->fully_freed() || tracker->BeingReleased()) { continue; } tracker->SetHugePageResidencyState(residency_states_[i].tracker_state);