Skip to content

Release pageheap_lock while HugePageFiller unbacks free pages. #1038

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
copybara-service wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Release pageheap_lock while HugePageFiller unbacks free pages. #1038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion tcmalloc/huge_page_aware_allocator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ class HugePageAwareAllocator final : public PageAllocatorInterface {
void ReleaseHugepage(FillerType::Tracker* pt)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock);
// Returns hugepages that the filler emptied while it did not hold
// pageheap_lock (during TreatHugepageTrackers) to the cache.
// pageheap_lock (during ReleasePages or TreatHugepageTrackers) to the cache.
void DrainFreedTrackers() ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock);
// Return an allocation from a single hugepage.
void DeleteFromHugepage(FillerType::Tracker* pt, Range r, bool might_abandon,
Expand Down Expand Up @@ -1054,6 +1054,7 @@ inline Length HugePageAwareAllocator<Forwarder>::ReleaseAtLeastNPages(
forwarder_.filler_skip_subrelease_long_interval()},
forwarder_.release_partial_alloc_pages(),
/*hit_limit*/ false);
DrainFreedTrackers();
}
}

Expand Down Expand Up @@ -1258,6 +1259,7 @@ HugePageAwareAllocator<Forwarder>::ReleaseAtLeastNPagesBreakingHugepages(
released += filler_.ReleasePages(n - released, SkipSubreleaseIntervals{},
/*release_partial_alloc_pages=*/false,
/*hit_limit=*/true);
DrainFreedTrackers();

info_.RecordRelease(n, released, reason);
return released;
Expand Down
8 changes: 7 additions & 1 deletion tcmalloc/huge_page_aware_allocator_fuzz.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,13 @@ void GatherAndCheckStats::Perform(State& state) const {
}
uint64_t used_bytes =
stats.system_bytes - stats.free_bytes - stats.unmapped_bytes;
TC_CHECK_EQ(used_bytes,
// We only get here with pending_release_ != 0 from a reentrant subprogram.
// HugeCache takes a range out of its free stats while it is being released
// (used == allocated + pending), whereas HugePageFiller accounts pages in
// flight as unmapped (used == allocated), so used_bytes can land anywhere in
// between.
TC_CHECK_GE(used_bytes, state.allocated.in_bytes());
TC_CHECK_LE(used_bytes,
state.allocated.in_bytes() +
state.allocator.forwarder().pending_release_.in_bytes());
}
Expand Down
356 changes: 219 additions & 137 deletions tcmalloc/huge_page_filler.h
View file
Open in desktop

Large diffs are not rendered by default.

203 changes: 103 additions & 100 deletions tcmalloc/huge_page_filler_fuzz.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,25 @@ class MockUnback final : public MemoryModifyFunction {
State& state_;
};

// Mirrors HugePageAwareAllocator::UnbackWithoutLock: drops pageheap_lock
// around the unback, which lets MockUnback's release_callback_ run reentrant
// instructions against the filler.
class MockUnbackWithoutLock final : public MemoryModifyFunction {
public:
explicit MockUnbackWithoutLock(MockUnback& unback) : unback_(unback) {}
[[nodiscard]] MemoryModifyStatus operator()(Range r) override
ABSL_NO_THREAD_SAFETY_ANALYSIS {
pageheap_lock.AssertHeld();
pageheap_lock.unlock();
MemoryModifyStatus ret = unback_(r);
pageheap_lock.lock();
return ret;
}

private:
MockUnback& unback_;
};

class MockSetAnonVmaName final : public MemoryTagFunction {
public:
void operator()(Range r, std::optional<absl::string_view> name) override {}
Expand Down Expand Up @@ -368,18 +387,19 @@ struct State {
size_t num_instructions)
: subrelease_unbacked_mode(subrelease_unbacked_mode),
unback(*this),
unback_without_lock(unback),
collapse(*this),
filler(Clock{.now = mock_clock, .freq = freq}, MemoryTag::kNormal,
unback, unback, collapse, set_anon_vma_name,
unback, unback_without_lock, collapse, set_anon_vma_name,
subrelease_unbacked_mode) {
fake_clock = 0;
output.resize(1 << 20);
// 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.
// TODO(b/73749855): Releasing the pageheap_lock during HandleReleaseFree
// will eliminate the need for this.
released_set.reserve(kPagesPerHugePage.raw_num() * num_instructions);

auto release_callback = [this]() {
Expand Down Expand Up @@ -408,7 +428,9 @@ struct State {
}

~State() {
// Shut down, confirm filler is empty.
// Shut down, confirm filler is empty. Put may drop pageheap_lock, so
// make sure no further instructions run reentrantly while we iterate.
reentrant_stack.clear();
CHECK_EQ(released_set.size(), filler.unmapped_pages().raw_num());
for (auto& [pt, v] : allocs) {
for (size_t i = 0, n = v.size(); i < n; ++i) {
Expand All @@ -428,50 +450,29 @@ struct State {
void RunInstructions(absl::Span<const Instruction> instrs) {
for (const auto& instruction : instrs) {
std::visit([&](const auto& instr) { instr.Perform(*this); }, instruction);
if (depth == 0) {
CheckInvariants();
}
}
}

// Pages held by live allocations on pt.
Length LivePagesOn(PageTracker* pt) const {
Length n;
auto it = allocs.find(pt);
if (it == allocs.end()) return n;
for (const auto& [alloc, alloc_info] : it->second) {
n += alloc.n;
}
return n;
}

void CheckInvariants() {
PageHeapSpinLockHolder l;
TC_CHECK_EQ(filler.size().raw_num(), trackers.size());
TC_CHECK_EQ(filler.unmapped_pages().raw_num(), released_set.size());
// Sparse and dense allocations live on disjoint sets of hugepages, so the
// per-density counters track our live allocations exactly.
for (int d = 0; d < AccessDensityPrediction::kPredictionCounts; ++d) {
TC_CHECK_EQ(
filler.pages_allocated(static_cast<AccessDensityPrediction>(d)),
live_pages[d]);
}
TC_CHECK_LE(filler.used_pages_in_any_subreleased(), filler.used_pages());
TC_CHECK_LE(filler.FreePagesInPartialAllocs(), filler.free_pages());
TC_CHECK_EQ(
filler.used_pages() + filler.free_pages() + filler.unmapped_pages(),
filler.size().in_pages());
}

// ReleasePages may claim credit for pages unmapped earlier and left
// unaccounted, so it reports at least the pages it unmapped just now, and
// nothing is unmapped while unback is failing.
void CheckReleased(Length released, Length unmapped_before) const {
const Length unmapped_after = filler.unmapped_pages();
TC_CHECK_GE(unmapped_after, unmapped_before);
TC_CHECK_GE(released, unmapped_after - unmapped_before);
if (!unback_success) {
TC_CHECK_EQ(unmapped_after, unmapped_before);
// Deletes trackers that were emptied by a reentrant Deallocate while the
// filler had dropped pageheap_lock. Trackers still pinned by an outer
// ReleasePages or TreatHugepageTrackers are left for that caller.
void DrainFreedTrackers() {
while (true) {
PageTracker* pt;
{
PageHeapSpinLockHolder l;
pt = filler.FetchFullyFreedTracker();
}
if (pt == nullptr) {
return;
}
HugePage hp = pt->location();
for (PageId p = hp.first_page(),
end = hp.first_page() + kPagesPerHugePage;
p != end; ++p) {
released_set.erase(p);
}
delete pt;
}
}

Expand All @@ -487,6 +488,7 @@ struct State {
absl::flat_hash_set<PageId> released_set;

MockUnback unback;
MockUnbackWithoutLock unback_without_lock;
MockCollapse collapse;
MockSetAnonVmaName set_anon_vma_name;
HugePageFiller<PageTracker> filler;
Expand All @@ -496,12 +498,10 @@ struct State {
std::vector<std::pair<Range, SpanAllocInfo>>>
allocs;
size_t next_hugepage = 1;
// Pages held by live allocations, by predicted access density.
Length live_pages[AccessDensityPrediction::kPredictionCounts];
std::vector<absl::Span<const Instruction>> reentrant_stack;
int depth = 0;
// Bumped whenever a reentrant subprogram runs, so an operation can tell
// whether other instructions interleaved with it.
// Number of reentrant subprograms run so far. Postconditions that assume
// no concurrent activity are skipped when this changes during an operation.
size_t reentrant_runs = 0;
bool treating_trackers = false;
std::string output;
Expand Down Expand Up @@ -589,31 +589,15 @@ void Allocate::Perform(State& state) const {
state.filler.Contribute(result.pt, donated, alloc_info);
}
state.trackers.push_back(result.pt);
} else {
// The filler only hands out hugepages it still owns.
TC_CHECK(state.allocs.contains(result.pt));
}

// The range lies within the tracker's hugepage and is disjoint from every
// live allocation on it.
const HugePage hp = result.pt->location();
TC_CHECK(HugePageContaining(result.page) == hp);
TC_CHECK(result.page + n <= hp.first_page() + kPagesPerHugePage);
for (const auto& [live, live_info] : state.allocs[result.pt]) {
TC_CHECK(!(result.page < live.p + live.n && live.p < result.page + n));
}

for (PageId p = result.page, end = p + n; p != end; ++p) {
// Only a previously released hugepage can hand out unmapped pages.
TC_CHECK(result.from_released || !state.released_set.contains(p));
state.released_set.erase(p);
}

state.allocs[result.pt].push_back({{result.page, n}, alloc_info});
state.live_pages[alloc_info.density] += n;

if (state.depth == 0) {
TC_CHECK_EQ(result.pt->used_pages(), state.LivePagesOn(result.pt));
TC_CHECK_EQ(state.filler.size().raw_num(), state.trackers.size());
TC_CHECK_EQ(state.filler.unmapped_pages().raw_num(),
state.released_set.size());
Expand All @@ -639,21 +623,15 @@ 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;
ret = state.filler.Put(pt, alloc, alloc_info);
}
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) {
Expand Down Expand Up @@ -685,9 +663,8 @@ void Release::Perform(State& state) const {
Length desired(desired_pages);
size_t to_release_from_partial_allocs;

const Length unmapped_before = state.filler.unmapped_pages();
const size_t runs_before = state.reentrant_runs;
Length released;
const size_t reentrant_runs = state.reentrant_runs;
{
PageHeapSpinLockHolder l;
to_release_from_partial_allocs =
Expand All @@ -696,13 +673,12 @@ void Release::Perform(State& state) const {
released = state.filler.ReleasePages(desired, skip_subrelease_intervals,
release_partial_allocs, hit_limit);
}
if (state.depth == 0 && runs_before == state.reentrant_runs) {
state.CheckReleased(released, unmapped_before);
}
state.DrainFreedTrackers();

if (!release_partial_allocs || hit_limit ||
skip_subrelease_intervals.SkipSubreleaseEnabled() ||
!state.unback_success || state.depth != 0) {
!state.unback_success || state.depth != 0 ||
state.reentrant_runs != reentrant_runs) {
return;
}
TC_CHECK_GE(released.raw_num(), to_release_from_partial_allocs);
Expand Down Expand Up @@ -752,7 +728,6 @@ void ModelTail::Perform(State& state) const {

state.allocs[pt].push_back(
{{start, n}, {1, AccessDensityPrediction::kSparse}});
state.live_pages[AccessDensityPrediction::kSparse] += n;

if (state.depth == 0) {
TC_CHECK_EQ(state.filler.size().raw_num(), state.trackers.size());
Expand All @@ -765,20 +740,17 @@ void MemoryLimitHitRelease::Perform(State& state) const {
Length desired_len(desired);
Length released;
const Length free = state.filler.free_pages();
const Length unmapped_before = state.filler.unmapped_pages();
const size_t runs_before = state.reentrant_runs;
const size_t reentrant_runs = state.reentrant_runs;
{
PageHeapSpinLockHolder l;
released = state.filler.ReleasePages(desired_len, SkipSubreleaseIntervals{},
/*release_partial_alloc_pages=*/false,
/*hit_limit=*/true);
}
if (state.depth != 0) {
state.DrainFreedTrackers();
if (state.depth != 0 || state.reentrant_runs != reentrant_runs) {
return;
}
if (runs_before == state.reentrant_runs) {
state.CheckReleased(released, unmapped_before);
}
const Length expected =
state.unback_success ? std::min(free, desired_len) : Length(0);
TC_CHECK_GE(released.raw_num(), expected.raw_num());
Expand Down Expand Up @@ -810,23 +782,19 @@ void TreatTrackers::Perform(State& state) const {
state.treating_trackers = true;
FakePageFlags pageflags(state);
FakeResidency residency(state);
PageHeapSpinLockHolder l;
state.filler.TreatHugepageTrackers(
enable_collapse ? EnableCollapse::kEnabled : EnableCollapse::kDisabled,
enable_unfiltered_collapse ? EnableUnfilteredCollapse::kEnabled
: EnableUnfilteredCollapse::kDisabled,
enable_release_stale_pages ? ReleaseStalePages::kEnabled
: ReleaseStalePages::kDisabled,
&pageflags, &residency);
state.treating_trackers = false;
while (PageTracker* pt = state.filler.FetchFullyFreedTracker()) {
HugePage hp = pt->location();
for (PageId p = hp.first_page(), end = hp.first_page() + kPagesPerHugePage;
p != end; ++p) {
state.released_set.erase(p);
}
delete pt;
{
PageHeapSpinLockHolder l;
state.filler.TreatHugepageTrackers(
enable_collapse ? EnableCollapse::kEnabled : EnableCollapse::kDisabled,
enable_unfiltered_collapse ? EnableUnfilteredCollapse::kEnabled
: EnableUnfilteredCollapse::kDisabled,
enable_release_stale_pages ? ReleaseStalePages::kEnabled
: ReleaseStalePages::kDisabled,
&pageflags, &residency);
}
state.treating_trackers = false;
state.DrainFreedTrackers();
PageHeapSpinLockHolder l;
for (PageTracker* pt : state.trackers) {
HugePage hp = pt->location();
const PageBitmap& rel = pt->released_by_page();
Expand Down Expand Up @@ -1439,6 +1407,41 @@ TEST(HugePageFillerTest, Regression_b525818096) {
SubreleaseUnbackedMode::kDisabled);
}

// Stats gathered from inside a limit-hit release, and an allocation from
// inside a later one, with SubreleaseUnbackedMode::kEnabled.
TEST(HugePageFillerTest, ReentrantStatsDuringLimitHitRelease) {
FuzzFiller(
{SetCollapseLatency{.latency = absl::Nanoseconds(9223372036854775807)},
ReentrantSubprogram{.subprogram = {}}, GatherStatsPbtxt{},
GatherStatsPbtxt{},
Allocate{
.length = 14317, .num_objects = 3536510400, .density_dense = false},
Release{.hit_limit = true,
.use_peak_interval = false,
.peak_interval = absl::Nanoseconds(9223372036854775807),
.short_interval = absl::Nanoseconds(1),
.long_interval = absl::Nanoseconds(9223372036854775807),
.desired_pages = 32767,
.release_partial_allocs = false},
ReentrantSubprogram{.subprogram = {ReentrantSubprogram{.subprogram = {}},
GatherStatsPbtxt{}}},
GatherStats{},
ReentrantSubprogram{
.subprogram = {Allocate{
.length = 0, .num_objects = 1, .density_dense = true}}}},
SubreleaseUnbackedMode::kEnabled);
}

// A deallocation from inside a memory-limit release with two nearly full
// hugepages.
TEST(HugePageFillerTest, ReentrantDeallocateDuringMemoryLimitRelease) {
FuzzFiller({ModelTail{.length = 511}, ModelTail{.length = 511},
ReentrantSubprogram{.subprogram = {Deallocate{.tracker_index = 0,
.alloc_index = 0}}},
MemoryLimitHitRelease{.desired = 2}},
SubreleaseUnbackedMode::kEnabled);
}

TEST(HugePageFillerTest, b547364068) {
FuzzFiller(
{GatherStats{},
Expand Down
Loading
Loading