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); } }