From a1e097f8e806c9634d7fd77cc33ad9c2a22db65c Mon Sep 17 00:00:00 2001 From: Chris Kennelly CA Date: Wed, 16 Sep 2026 00:34:19 -0700 Subject: [PATCH] Release pageheap_lock while releasing memory to the OS from HugePageFiller. Drop pageheap_lock around unback syscalls in HugePageFiller::ReleasePages (via UnbackSubreleaseFunction) and HugePageFiller::HandleFullyFreedTracker while preserving tracker invariants against concurrent allocations and deallocations: - Remove the PageTracker from regular_alloc_ / regular_alloc_partial_released_ before invoking ReleaseFree, temporarily pinning the subreleasing range in RangeTracker so concurrent TryGet calls skip it and concurrent Put calls cannot return the tracker as fully freed while unback is in flight. - Re-evaluate tracker state upon reacquiring pageheap_lock after ReleaseFree: if empty, defer whole-hugepage unback via StoreFullyFreedTracker for the caller to drain via HandleFullyFreedTracker; otherwise reinsert into the appropriate tracker list. - Stop multi-range ReleaseFree loops immediately if concurrent Put calls empty the tracker during an unlocked unback. - Update n_was_released_ and RecordLifetime before dropping pageheap_lock in HandleFullyFreedTracker so filler statistics remain consistent during unlocked unback. PiperOrigin-RevId: 982325222 --- tcmalloc/huge_page_aware_allocator.h | 6 + tcmalloc/huge_page_filler.h | 364 ++++++++++++++------------- tcmalloc/huge_page_filler_fuzz.cc | 169 +++++-------- tcmalloc/huge_page_filler_test.cc | 275 +++++++++++++++++++- tcmalloc/huge_page_options.h | 1 + tcmalloc/huge_page_tracker.h | 15 +- tcmalloc/huge_page_treatment.h | 8 +- 7 files changed, 551 insertions(+), 287 deletions(-) diff --git a/tcmalloc/huge_page_aware_allocator.h b/tcmalloc/huge_page_aware_allocator.h index 5c11851c9..31e99db60 100644 --- a/tcmalloc/huge_page_aware_allocator.h +++ b/tcmalloc/huge_page_aware_allocator.h @@ -1054,6 +1054,9 @@ inline Length HugePageAwareAllocator::ReleaseAtLeastNPages( forwarder_.filler_skip_subrelease_long_interval()}, forwarder_.release_partial_alloc_pages(), /*hit_limit*/ false); + while (PageTracker* pt = filler_.FetchFullyFreedTracker()) { + ReleaseHugepage(pt); + } } } @@ -1258,6 +1261,9 @@ HugePageAwareAllocator::ReleaseAtLeastNPagesBreakingHugepages( released += filler_.ReleasePages(n - released, SkipSubreleaseIntervals{}, /*release_partial_alloc_pages=*/false, /*hit_limit=*/true); + while (PageTracker* pt = filler_.FetchFullyFreedTracker()) { + ReleaseHugepage(pt); + } info_.RecordRelease(n, released, reason); return released; diff --git a/tcmalloc/huge_page_filler.h b/tcmalloc/huge_page_filler.h index 7cc73f6b6..afe8299e8 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; @@ -831,9 +835,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 @@ -964,9 +968,9 @@ class HugePageFiller { // Records a list of fully freed trackers. We might end up with trackers that // are fully freed, but not deleted, when: the trackers are being userspace- - // collapsed, and an intermediate Put operation deallocates all the pages - // in the tracker. The list temporarily holds these trackers before they are - // deleted, once the collapse operation completes. + // collapsed or subreleased, and an intermediate Put operation deallocates all + // the pages in the tracker. The list temporarily holds these trackers before + // they are deleted, once the operation completes. TList fully_freed_trackers_; HugePageTreatmentStats treatment_stats_ ABSL_GUARDED_BY(pageheap_lock); @@ -985,43 +989,65 @@ class HugePageFiller { void RemoveFromFillerList(TrackerType* absl_nonnull pt); // Put pt in the appropriate PageTrackerList. void AddToFillerList(TrackerType* absl_nonnull pt); - // Retires a tracker that has become empty. Returns pt if the caller now owns - // it; returns nullptr if pt was parked on fully_freed_trackers_ because a - // 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) - ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); // Like AddToFillerList(), but for use when donating from the tail of a // multi-hugepage allocation. void DonateToFillerList(TrackerType* absl_nonnull pt); + void HandleFullyFreedTracker(TrackerType* absl_nonnull pt) + ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); + + class UnbackSubreleaseFunction : public MemoryModifyFunction { + public: + UnbackSubreleaseFunction(HugePageFiller& filler, + TrackerType* absl_nonnull pt) + : filler_(filler), pt_(pt) {} + + MemoryModifyStatus operator()(Range r) override + ABSL_NO_THREAD_SAFETY_ANALYSIS { + const AccessDensityPrediction type = + pt_->HasDenseSpans() ? AccessDensityPrediction::kDense + : AccessDensityPrediction::kSparse; + filler_.pages_allocated_[type] += r.n; + filler_.AddToFillerList(pt_); + MemoryModifyStatus res = filler_.unback_without_lock_(r); + filler_.RemoveFromFillerList(pt_); + TC_ASSERT_GE(filler_.pages_allocated_[type], r.n); + filler_.pages_allocated_[type] -= r.n; + if (ABSL_PREDICT_TRUE(res.success)) { + filler_.unmapped_ += r.n; + } + return res; + } + + private: + HugePageFiller& filler_; + TrackerType* absl_nonnull pt_; + }; + + Length ReleaseFreeFromTracker(TrackerType* absl_nonnull pt) + ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); + void PrintAllocStatsInPbtxt(absl::string_view field, PbtxtRegion& hpaa, const HugePageFillerStats& stats, AccessDensityPrediction count) const; 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 @@ -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,62 +1331,41 @@ 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]); } } -// Marks r as usable by new allocations into *pt; returns pt if that hugepage is -// now empty (nullptr otherwise.) -// -// REQUIRES: pt is owned by this object (has been Contribute()), and {pt, -// Range(p, n)} was the result of a previous TryGet. 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()) { - TC_ASSERT_GE(pages_allocated_[AccessDensityPrediction::kDense], r.n); - pages_allocated_[AccessDensityPrediction::kDense] -= r.n; - } else { - TC_ASSERT_GE(pages_allocated_[AccessDensityPrediction::kSparse], r.n); - pages_allocated_[AccessDensityPrediction::kSparse] -= r.n; +inline void HugePageFiller::HandleFullyFreedTracker( + TrackerType* pt) { + TC_ASSERT_EQ(pt->nallocs(), 0); + --size_; + 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 (ABSL_PREDICT_FALSE(pt->fully_freed())) { - return HandleFullyFreedTracker(pt, now); - } - AddToFillerList(pt); - UpdateFillerStatsTracker(now); - return nullptr; -} + RecordLifetime(pt); -template -inline TrackerType* absl_nullable -HugePageFiller::HandleFullyFreedTracker(TrackerType* pt, - int64_t now) { - TC_ASSERT_EQ(pt->nallocs(), 0); - --size_; if (pt->released()) { #ifndef TCMALLOC_INTERNAL_LEGACY_LOCKING const Length free_pages = kPagesPerHugePage; @@ -1391,8 +1392,18 @@ HugePageFiller::HandleFullyFreedTracker(TrackerType* pt, } } } +} - if (pt->was_released()) { +template +inline Length HugePageFiller::ReleaseFreeFromTracker( + TrackerType* pt) { + RemoveFromFillerList(pt); + UnbackSubreleaseFunction unback(*this, pt); + Length ret = pt->ReleaseFree(unback); + TC_ASSERT_GE(unmapped_, pt->released_pages()); + if (pt->longest_free_range() == kPagesPerHugePage) { + HandleFullyFreedTracker(pt); + } else if (pt->was_released() && pt->released()) { pt->set_was_released(/*status=*/false); if (pt->HasDenseSpans()) { --n_was_released_[AccessDensityPrediction::kDense]; @@ -1400,21 +1411,42 @@ HugePageFiller::HandleFullyFreedTracker(TrackerType* pt, --n_was_released_[AccessDensityPrediction::kSparse]; } } + AddToFillerList(pt); + return ret; +} - 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); - return nullptr; +// Marks r as usable by new allocations into *pt; returns pt if that hugepage is +// now empty (nullptr otherwise.) +// +// REQUIRES: pt is owned by this object (has been Contribute()), and {pt, +// Range(p, n)} was the result of a previous TryGet. +template +inline TrackerType* HugePageFiller::Put( + TrackerType* pt, Range r, SpanAllocInfo span_alloc_info) { + RemoveFromFillerList(pt); + pt->Put(r, span_alloc_info); + if (pt->HasDenseSpans()) { + TC_ASSERT_GE(pages_allocated_[AccessDensityPrediction::kDense], r.n); + pages_allocated_[AccessDensityPrediction::kDense] -= r.n; + } else { + TC_ASSERT_GE(pages_allocated_[AccessDensityPrediction::kSparse], r.n); + pages_allocated_[AccessDensityPrediction::kSparse] -= r.n; } - 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); + + if (pt->longest_free_range() == kPagesPerHugePage) { + HandleFullyFreedTracker(pt); + if (!pt->DontFreeTracker()) { + 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; + } } - return pt; + AddToFillerList(pt); + UpdateFillerStatsTracker(); + return nullptr; } template @@ -1442,7 +1474,7 @@ inline void HugePageFiller::Contribute( } ++size_; - UpdateFillerStatsTracker(clock_.now()); + UpdateFillerStatsTracker(); } template @@ -1454,10 +1486,11 @@ inline int HugePageFiller::SelectCandidates( TC_ASSERT_GT(pt.free_pages(), Length(0)); TC_ASSERT_GT(pt.free_pages(), pt.released_pages()); - // If the tracker is being collapsed, don't release it. Collapse might race + // If the tracker is being collapsed or already has an active operation + // (such as concurrent subrelease), don't release it. Collapse might race // with the release, and we might collapse the pages that have been recently // released. - if (pt.BeingCollapsed()) return; + if (pt.DontFreeTracker() || pt.BeingCollapsed()) return; // If we have few candidates, we can avoid creating a heap. // @@ -1498,47 +1531,33 @@ inline Length HugePageFiller::ReleaseCandidates( absl::Span candidates, Length target) { absl::c_sort(candidates, CompareForSubrelease); + for (TrackerType* c : candidates) { + c->SetDontFreeTracker(HugePageTreatmentType::kSubrelease); + } + 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]; 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 + if (best->fully_freed() || best->free_pages() <= best->released_pages() || + best->BeingCollapsed()) { + continue; + } if (best->unbroken()) { ++total_broken; } - RemoveFromFillerList(best); - Length ret = best->ReleaseFree(unback_); - unmapped_ += ret; - TC_ASSERT_GE(unmapped_, best->released_pages()); + Length ret = ReleaseFreeFromTracker(best); 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]; - } + } + + for (TrackerType* c : candidates) { + c->ClearDontFreeTracker(HugePageTreatmentType::kSubrelease); + if (!c->DontFreeTracker() && c->fully_freed() && + c->GetTagState().sampled_for_tagging) { + c->SetAnonVmaName(set_anon_vma_name_, /*name=*/std::nullopt); } } @@ -1576,7 +1595,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. @@ -1961,12 +1980,19 @@ inline void HugePageFiller::TreatHugepageTrackers( template inline Length HugePageFiller::HandleReleaseFree( PageTracker* tracker) { - RemoveFromFillerList(tracker); - Length released_length = tracker->ReleaseFree(unback_); + if (tracker->fully_freed() || + tracker->free_pages() <= tracker->released_pages()) { + return Length(0); + } + tracker->SetDontFreeTracker(HugePageTreatmentType::kSubrelease); + Length released_length = ReleaseFreeFromTracker(tracker); subrelease_stats_.total_pages_subreleased += released_length; - unmapped_ += released_length; unmapping_unaccounted_ += released_length; - AddToFillerList(tracker); + tracker->ClearDontFreeTracker(HugePageTreatmentType::kSubrelease); + if (!tracker->DontFreeTracker() && tracker->fully_freed() && + tracker->GetTagState().sampled_for_tagging) { + tracker->SetAnonVmaName(set_anon_vma_name_, /*name=*/std::nullopt); + } return released_length; } @@ -1992,7 +2018,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 +2258,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 +2462,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(); } @@ -2495,6 +2516,9 @@ template inline size_t HugePageFiller::ListFor( const TrackerType& pt) const { if (pt.HasDenseSpans()) { + if (ABSL_PREDICT_FALSE(pt.longest_free_range() == Length(0))) { + return 0; + } return DenseListFor(pt.nallocs()); } return SparseListFor(pt.longest_free_range(), IndexFor(pt)); @@ -2532,13 +2556,13 @@ inline void HugePageFiller::RemoveFromFillerList(TrackerType* pt) { template inline TrackerType* absl_nullable HugePageFiller::FetchFullyFreedTracker() { - if (fully_freed_trackers_.empty()) { - return nullptr; + 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 diff --git a/tcmalloc/huge_page_filler_fuzz.cc b/tcmalloc/huge_page_filler_fuzz.cc index 0d86c7203..9f30a13ae 100644 --- a/tcmalloc/huge_page_filler_fuzz.cc +++ b/tcmalloc/huge_page_filler_fuzz.cc @@ -79,12 +79,14 @@ Bitmap GetBitmap(int value) { class MockUnback final : public MemoryModifyFunction { public: - explicit MockUnback(State& state) : state_(state) {} + MockUnback(State& state, bool lock_held) + : state_(state), lock_held_(lock_held) {} [[nodiscard]] MemoryModifyStatus operator()(Range r) override; std::function release_callback_; private: State& state_; + bool lock_held_; }; class MockSetAnonVmaName final : public MemoryTagFunction { @@ -367,19 +369,17 @@ struct State { explicit State(SubreleaseUnbackedMode subrelease_unbacked_mode, size_t num_instructions) : subrelease_unbacked_mode(subrelease_unbacked_mode), - unback(*this), + unback(*this, /*lock_held=*/false), + unback_without_lock(*this, /*lock_held=*/true), 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); // 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); auto release_callback = [this]() { @@ -397,17 +397,24 @@ struct State { reentrant_stack.pop_back(); depth++; - reentrant_runs++; ScopedAllocationAllow allow; RunInstructions(ops); depth--; }; unback.release_callback_ = release_callback; + unback_without_lock.release_callback_ = release_callback; collapse.release_callback_ = release_callback; } ~State() { + unback.release_callback_ = nullptr; + unback_without_lock.release_callback_ = nullptr; + collapse.release_callback_ = nullptr; + { + PageHeapSpinLockHolder l; + DrainFullyFreedTrackers(); + } // Shut down, confirm filler is empty. CHECK_EQ(released_set.size(), filler.unmapped_pages().raw_num()); for (auto& [pt, v] : allocs) { @@ -425,53 +432,34 @@ struct State { CHECK(filler.size() == NHugePages(0)); } - void RunInstructions(absl::Span instrs) { - for (const auto& instruction : instrs) { - std::visit([&](const auto& instr) { instr.Perform(*this); }, instruction); - if (depth == 0) { - CheckInvariants(); + void DrainFullyFreedTrackers() ABSL_EXCLUSIVE_LOCKS_REQUIRED( + tcmalloc::tcmalloc_internal::pageheap_lock) { + while (PageTracker* pt = filler.FetchFullyFreedTracker()) { + HugePage hp = pt->location(); + for (PageId p = hp.first_page(), + end = hp.first_page() + kPagesPerHugePage; + p != end; ++p) { + released_set.erase(p); } + delete pt; } - } - - // 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; + for (PageTracker* pt : trackers) { + HugePage hp = pt->location(); + const PageBitmap& 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)) { + released_set.insert(p); + } else { + released_set.erase(p); + } + } } - 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); + void RunInstructions(absl::Span instrs) { + for (const auto& instruction : instrs) { + std::visit([&](const auto& instr) { instr.Perform(*this); }, instruction); } } @@ -487,6 +475,7 @@ struct State { absl::flat_hash_set released_set; MockUnback unback; + MockUnback unback_without_lock; MockCollapse collapse; MockSetAnonVmaName set_anon_vma_name; HugePageFiller filler; @@ -496,21 +485,24 @@ 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. - size_t reentrant_runs = 0; bool treating_trackers = false; std::string output; }; -MemoryModifyStatus MockUnback::operator()(Range r) { +MemoryModifyStatus MockUnback::operator()(Range r) + ABSL_NO_THREAD_SAFETY_ANALYSIS { + if (lock_held_) { + tcmalloc::tcmalloc_internal::pageheap_lock.AssertHeld(); + tcmalloc::tcmalloc_internal::pageheap_lock.unlock(); + } if (release_callback_) { release_callback_(); } + if (lock_held_) { + tcmalloc::tcmalloc_internal::pageheap_lock.lock(); + } if (!state_.unback_success) { return {.success = false, .error_number = 0}; } @@ -589,31 +581,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 +615,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 +622,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) { @@ -661,6 +631,10 @@ void Deallocate::Perform(State& state) const { } delete ret; } + { + PageHeapSpinLockHolder l; + state.DrainFullyFreedTrackers(); + } if (state.depth == 0) { TC_CHECK_EQ(state.filler.size().raw_num(), state.trackers.size()); @@ -684,9 +658,8 @@ void Release::Perform(State& state) const { } Length desired(desired_pages); size_t to_release_from_partial_allocs; + const size_t reentrant_before = state.reentrant_stack.size(); - const Length unmapped_before = state.filler.unmapped_pages(); - const size_t runs_before = state.reentrant_runs; Length released; { PageHeapSpinLockHolder l; @@ -695,14 +668,13 @@ void Release::Perform(State& state) const { state.filler.FreePagesInPartialAllocs().raw_num(); 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.DrainFullyFreedTrackers(); } if (!release_partial_allocs || hit_limit || skip_subrelease_intervals.SkipSubreleaseEnabled() || - !state.unback_success || state.depth != 0) { + !state.unback_success || state.depth != 0 || + state.reentrant_stack.size() != reentrant_before) { return; } TC_CHECK_GE(released.raw_num(), to_release_from_partial_allocs); @@ -752,7 +724,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 +736,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_before = state.reentrant_stack.size(); { PageHeapSpinLockHolder l; released = state.filler.ReleasePages(desired_len, SkipSubreleaseIntervals{}, /*release_partial_alloc_pages=*/false, /*hit_limit=*/true); + state.DrainFullyFreedTrackers(); } - if (state.depth != 0) { + if (state.depth != 0 || state.reentrant_stack.size() != reentrant_before) { 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()); @@ -819,26 +787,7 @@ void TreatTrackers::Perform(State& state) const { : 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; - } - for (PageTracker* pt : state.trackers) { - HugePage hp = pt->location(); - const PageBitmap& 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)) { - state.released_set.insert(p); - } else { - state.released_set.erase(p); - } - } - } + state.DrainFullyFreedTrackers(); } void UpdateBitmaps::Perform(State& state) const { diff --git a/tcmalloc/huge_page_filler_test.cc b/tcmalloc/huge_page_filler_test.cc index 876c8b38f..ae38d9a4b 100644 --- a/tcmalloc/huge_page_filler_test.cc +++ b/tcmalloc/huge_page_filler_test.cc @@ -340,9 +340,12 @@ class MockSetAnonVmaName final : public MemoryTagFunction { class BlockingUnback final : public MemoryModifyFunction { public: - constexpr BlockingUnback() = default; + BlockingUnback() = default; [[nodiscard]] MemoryModifyStatus operator()(Range r) override { + if (callback_) { + callback_(r); + } if (!mu_) { return {.success = success_, .error_number = 0}; } @@ -356,6 +359,7 @@ class BlockingUnback final : public MemoryModifyFunction { return {.success = success_, .error_number = 0}; } + std::function callback_; absl::BlockingCounter* counter_ = nullptr; bool success_ = true; @@ -407,9 +411,23 @@ class FillerTest : public testing::Test { // filler inits), so its time series has the same initial state (e.g., first // epoch) ClockResetter clock_resetter_; + struct UnbackWithoutLock : public MemoryModifyFunction { + explicit UnbackWithoutLock(BlockingUnback& unback) : unback_(unback) {} + MemoryModifyStatus operator()(Range r) override + ABSL_NO_THREAD_SAFETY_ANALYSIS { + const bool was_held = pageheap_lock.IsHeld(); + if (was_held) pageheap_lock.unlock(); + MemoryModifyStatus ret = unback_(r); + if (was_held) pageheap_lock.lock(); + return ret; + } + BlockingUnback& unback_; + }; + SubreleaseUnbackedMode mode_ = SubreleaseUnbackedMode::kDisabled; HugePageFiller filler_; BlockingUnback blocking_unback_; + UnbackWithoutLock unback_without_lock_{blocking_unback_}; MockCollapse collapse_; MockSetAnonVmaName set_anon_vma_name_; @@ -417,7 +435,7 @@ 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_, + MemoryTag::kNormal, blocking_unback_, unback_without_lock_, collapse_, set_anon_vma_name_, mode) { // Reset success state blocking_unback_.success_ = true; @@ -6629,6 +6647,259 @@ TEST_F(FillerTest, ConcurrentTreatmentInterferenceStress) { CheckStats(); } +TEST_F(FillerTest, ReleasePagesUnlocksPageHeapLockAndHandlesReentrantPut) { + randomize_density_ = false; + auto alloc1 = Allocate(Length(1)); + auto alloc2 = Allocate(Length(kPagesPerHugePage.raw_num() - 1)); + ASSERT_EQ(alloc1.pt, alloc2.pt); + + // Free alloc2 so the huge page becomes partially released / candidate for + // subrelease, while alloc1 keeps the tracker alive. + Delete(alloc2); + + int callback_count = 0; + bool lock_held_in_unback = true; + PageTracker* put_ret = nullptr; + size_t nallocs_in_unback = 0; + bool fully_freed_in_unback = true; + bool dont_free_in_unback = false; + PageTracker* fetched_in_unback = nullptr; + + blocking_unback_.callback_ = [&](Range r) { + ++callback_count; + // Verify pageheap_lock is NOT held during unback_without_lock. + lock_held_in_unback = pageheap_lock.IsHeld(); + + PageHeapSpinLockHolder l; + // Reentrantly inspect stats while tracker is mid-subrelease. + (void)filler_.stats(); + + if (callback_count == 1) { + // Reentrantly free the last remaining allocation on the candidate + // tracker while it is pinned with kSubrelease and its subreleasing range + // is marked in tracker_. + put_ret = filler_.Put(alloc1.pt, Range(alloc1.p, alloc1.n), + alloc1.span_alloc_info); + total_allocated_ -= alloc1.n; + nallocs_in_unback = alloc1.pt->nallocs(); + fully_freed_in_unback = alloc1.pt->fully_freed(); + dont_free_in_unback = alloc1.pt->DontFreeTracker(); + // FetchFullyFreedTracker must not return the tracker while subrelease is + // still in progress. + fetched_in_unback = filler_.FetchFullyFreedTracker(); + } + }; + + Length released; + bool fully_freed_after = false; + bool dont_free_after = true; + PageTracker* pt = nullptr; + { + PageHeapSpinLockHolder l; + released = + filler_.ReleasePages(kPagesPerHugePage, SkipSubreleaseIntervals{}, + /*release_partial_alloc_pages=*/true, + /*hit_limit=*/true); + fully_freed_after = alloc1.pt->fully_freed(); + dont_free_after = alloc1.pt->DontFreeTracker(); + // Now that subrelease completed, unmarked the range in tracker_, and + // cleared kSubrelease, the fully freed tracker can be fetched. + pt = filler_.FetchFullyFreedTracker(); + } + blocking_unback_.callback_ = nullptr; + + EXPECT_FALSE(lock_held_in_unback); + EXPECT_EQ(put_ret, nullptr); + EXPECT_EQ(nallocs_in_unback, 1); + EXPECT_FALSE(fully_freed_in_unback); + EXPECT_TRUE(dont_free_in_unback); + EXPECT_EQ(fetched_in_unback, nullptr); + EXPECT_GT(released, Length(0)); + EXPECT_GE(callback_count, 1); + EXPECT_TRUE(fully_freed_after); + EXPECT_FALSE(dont_free_after); + ASSERT_EQ(pt, alloc1.pt); + --hp_contained_; + delete pt; + CheckStats(); +} + +TEST_F(FillerTest, ReleasePagesMultiRangeStopsWhenEmptiedByReentrantPut) { + randomize_density_ = false; + auto alloc1 = Allocate(Length(10)); + auto alloc2 = Allocate(Length(1)); + auto alloc3 = Allocate(Length(20)); + auto alloc4 = Allocate(Length(kPagesPerHugePage.raw_num() - 31)); + ASSERT_EQ(alloc1.pt, alloc2.pt); + ASSERT_EQ(alloc1.pt, alloc3.pt); + ASSERT_EQ(alloc1.pt, alloc4.pt); + + // Create two disjoint free ranges [0, 10) and [11, 31). + Delete(alloc1); + Delete(alloc3); + + std::vector unbacked_lengths; + unbacked_lengths.reserve(4); + PageTracker* put2_res = nullptr; + PageTracker* put4_res = nullptr; + PageTracker* fetched_in_unback = nullptr; + Length used_in_unback; + blocking_unback_.callback_ = [&](Range r) { + EXPECT_FALSE(pageheap_lock.IsHeld()); + unbacked_lengths.push_back(r.n); + + PageHeapSpinLockHolder l; + (void)filler_.stats(); + if (unbacked_lengths.size() == 1) { + // Reentrantly free both remaining allocations during the first subrange + // unback so only the pinned subrange [0, 10) remains marked in tracker_ + // until ReleasePages returns and Unmark(0, 10) makes empty() true. + put2_res = filler_.Put(alloc2.pt, Range(alloc2.p, alloc2.n), + alloc2.span_alloc_info); + total_allocated_ -= alloc2.n; + put4_res = filler_.Put(alloc4.pt, Range(alloc4.p, alloc4.n), + alloc4.span_alloc_info); + total_allocated_ -= alloc4.n; + used_in_unback = alloc1.pt->used_pages(); + fetched_in_unback = filler_.FetchFullyFreedTracker(); + } + }; + + Length released; + PageTracker* pt; + { + PageHeapSpinLockHolder l; + released = + filler_.ReleasePages(kPagesPerHugePage, SkipSubreleaseIntervals{}, + /*release_partial_alloc_pages=*/true, + /*hit_limit=*/true); + pt = filler_.FetchFullyFreedTracker(); + } + blocking_unback_.callback_ = nullptr; + + // ReleaseFree must stop immediately after the first subrange (Length(10)) + // once Unmark(0, 10) makes empty() true, and HandleFullyFreedTracker then + // unbacks the full hugepage (kPagesPerHugePage). + EXPECT_EQ(put2_res, nullptr); + EXPECT_EQ(put4_res, nullptr); + EXPECT_EQ(used_in_unback, Length(10)); + EXPECT_EQ(fetched_in_unback, nullptr); + EXPECT_EQ(released, Length(10)); + EXPECT_THAT(unbacked_lengths, + testing::ElementsAre(Length(10), kPagesPerHugePage)); + ASSERT_EQ(pt, alloc1.pt); + EXPECT_TRUE(pt->empty()); + --hp_contained_; + delete pt; + CheckStats(); +} + +TEST_F(FillerTest, ReleasePagesMultiRangeHandlesReentrantTryGet) { + randomize_density_ = false; + auto alloc1 = Allocate(Length(10)); + auto alloc2 = Allocate(Length(1)); + auto alloc3 = Allocate(Length(20)); + auto alloc4 = Allocate(Length(kPagesPerHugePage.raw_num() - 31)); + ASSERT_EQ(alloc1.pt, alloc2.pt); + ASSERT_EQ(alloc1.pt, alloc3.pt); + ASSERT_EQ(alloc1.pt, alloc4.pt); + + // Create two disjoint free ranges [0, 10) and [11, 31). + Delete(alloc1); + Delete(alloc3); + + std::vector unbacked_ranges; + unbacked_ranges.reserve(4); + HugePageFiller::TryGetResult reentrant_get = {nullptr, + PageId{0}}; + blocking_unback_.callback_ = [&](Range r) { + EXPECT_FALSE(pageheap_lock.IsHeld()); + unbacked_ranges.push_back(r); + + PageHeapSpinLockHolder l; + if (unbacked_ranges.size() == 1) { + // While [0, 10) is marked in-use in free_ during unback, a reentrant + // TryGet(5) must skip [0, 10) and allocate [11, 16) from the second + // free range. + reentrant_get = filler_.TryGet(Length(5), alloc2.span_alloc_info); + total_allocated_ += Length(5); + } + }; + + Length released; + { + PageHeapSpinLockHolder l; + released = + filler_.ReleasePages(kPagesPerHugePage, SkipSubreleaseIntervals{}, + /*release_partial_alloc_pages=*/true, + /*hit_limit=*/true); + } + blocking_unback_.callback_ = nullptr; + + ASSERT_EQ(reentrant_get.pt, alloc1.pt); + EXPECT_EQ(reentrant_get.page, alloc3.p); + ASSERT_EQ(unbacked_ranges.size(), 2); + EXPECT_EQ(unbacked_ranges[0].p, alloc1.p); + EXPECT_EQ(unbacked_ranges[0].n, Length(10)); + EXPECT_EQ(unbacked_ranges[1].p, alloc3.p + Length(5)); + EXPECT_EQ(unbacked_ranges[1].n, Length(15)); + EXPECT_EQ(released, Length(25)); + + PageTracker* put_reentrant_res = nullptr; + { + PageHeapSpinLockHolder l; + put_reentrant_res = + filler_.Put(reentrant_get.pt, Range(reentrant_get.page, Length(5)), + alloc2.span_alloc_info); + total_allocated_ -= Length(5); + } + EXPECT_EQ(put_reentrant_res, nullptr); + Delete(alloc2); + Delete(alloc4); + CheckStats(); +} + +TEST_F(FillerTest, ReleasePagesHandlesUnbackFailureWithReentrantPut) { + randomize_density_ = false; + auto alloc1 = Allocate(Length(1)); + auto alloc2 = Allocate(Length(kPagesPerHugePage.raw_num() - 1)); + ASSERT_EQ(alloc1.pt, alloc2.pt); + + Delete(alloc2); + + PageTracker* put1_res = nullptr; + blocking_unback_.success_ = false; + blocking_unback_.callback_ = [&](Range r) { + EXPECT_FALSE(pageheap_lock.IsHeld()); + PageHeapSpinLockHolder l; + put1_res = filler_.Put(alloc1.pt, Range(alloc1.p, alloc1.n), + alloc1.span_alloc_info); + total_allocated_ -= alloc1.n; + }; + + Length released; + PageTracker* pt; + { + PageHeapSpinLockHolder l; + released = + filler_.ReleasePages(kPagesPerHugePage, SkipSubreleaseIntervals{}, + /*release_partial_alloc_pages=*/true, + /*hit_limit=*/true); + pt = filler_.FetchFullyFreedTracker(); + } + blocking_unback_.callback_ = nullptr; + blocking_unback_.success_ = true; + + EXPECT_EQ(put1_res, nullptr); + EXPECT_EQ(released, Length(0)); + ASSERT_EQ(pt, alloc1.pt); + EXPECT_FALSE(pt->released()); + EXPECT_FALSE(pt->was_released()); + --hp_contained_; + delete pt; + CheckStats(); +} + TEST(SkipSubreleaseIntervalsTest, EmptyIsNotEnabled) { // When we have a limit hit, we pass SkipSubreleaseIntervals{} to the // filler. Make sure it doesn't signal that we should skip the limit. 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..cc6f815fa 100644 --- a/tcmalloc/huge_page_tracker.h +++ b/tcmalloc/huge_page_tracker.h @@ -490,11 +490,21 @@ inline Length PageTracker::ReleaseFree(MemoryModifyFunction& unback) { TC_ASSERT_EQ(released_by_page_.CountBits(free_index, length), 0); 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 [free_index, free_index + length) as allocated while unbacking so + // concurrent allocations do not allocate from this range and concurrent + // deallocations do not observe longest_free_range() == kPagesPerHugePage. + tracker_.Mark(free_index, length); + const bool released = ReleasePages(Range(p, Length(length)), unback); + tracker_.Unmark(free_index, length); + if (ABSL_PREDICT_TRUE(released)) { + // Mark pages as released. released_by_page_.SetRange(free_index, length); + released_count_ += length; count += length; } + if (empty()) { + break; + } index = end; } else { @@ -504,7 +514,6 @@ inline Length PageTracker::ReleaseFree(MemoryModifyFunction& unback) { } } - released_count_ += count; if (count > 0) { hugepage_residency_state_.maybe_hugepage_backed = false; } diff --git a/tcmalloc/huge_page_treatment.h b/tcmalloc/huge_page_treatment.h index 1239bfc89..d34c972a7 100644 --- a/tcmalloc/huge_page_treatment.h +++ b/tcmalloc/huge_page_treatment.h @@ -356,6 +356,7 @@ class HugePageUnbackedTrackerTreatment final : public HugePageTreatment { } void SelectEligibleTrackers(PageTracker& pt) override { + if (pt.DontFreeTracker() || pt.BeingCollapsed()) return; auto PushCandidate = [&](PageTracker& pt) GOOGLE_MALLOC_SECTION { if (num_valid_trackers_ < kTotalTrackersToScan) { @@ -488,8 +489,8 @@ class HugePageUnbackedTrackerTreatment final : public HugePageTreatment { for (int i = 0; i < num_valid_trackers_; ++i) { PageTracker* tracker = residency_states_[i].tracker; TC_ASSERT_NE(tracker, nullptr); - tracker->ClearDontFreeTracker(HugePageTreatmentType::kCollapse); if (tracker->fully_freed()) { + tracker->ClearDontFreeTracker(HugePageTreatmentType::kCollapse); continue; } tracker->SetHugePageResidencyState(residency_states_[i].tracker_state); @@ -497,6 +498,7 @@ class HugePageUnbackedTrackerTreatment final : public HugePageTreatment { if (subrelease_unbacked_mode_ == SubreleaseUnbackedMode::kEnabled) { page_filler_.OnCollapseSuccess(tracker); } + tracker->ClearDontFreeTracker(HugePageTreatmentType::kCollapse); continue; } @@ -520,7 +522,8 @@ class HugePageUnbackedTrackerTreatment final : public HugePageTreatment { } } - if (subrelease_unbacked_mode_ == SubreleaseUnbackedMode::kEnabled) { + if (!tracker->fully_freed() && + subrelease_unbacked_mode_ == SubreleaseUnbackedMode::kEnabled) { Length released_length = page_filler_.HandleUnbackedHugePage( tracker, residency_states_[i].tracker_state.unbacked); if (released_length > Length(0)) { @@ -528,6 +531,7 @@ class HugePageUnbackedTrackerTreatment final : public HugePageTreatment { released_length.raw_num(); } } + tracker->ClearDontFreeTracker(HugePageTreatmentType::kCollapse); } }