[scudo] Improve the message of region exhaustion#68444
Merged
ChiaHungDuan merged 1 commit intollvm:mainfrom Oct 6, 2023
Merged
[scudo] Improve the message of region exhaustion#68444ChiaHungDuan merged 1 commit intollvm:mainfrom
ChiaHungDuan merged 1 commit intollvm:mainfrom
Conversation
Member
|
@llvm/pr-subscribers-compiler-rt-sanitizer ChangesIn this CL, we move the printing of allocator stats from primary.h to combined.h. This will also dump the secondary stats and reduce the log spam when an OOM happens Also change the symbol Full diff: https://github.com/llvm/llvm-project/pull/68444.diff 2 Files Affected:
diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index b013f03a73ae40d..b1700e5ecef7f5b 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -367,10 +367,9 @@ class Allocator {
auto *TSD = TSDRegistry.getTSDAndLock(&UnlockRequired);
TSD->assertLocked(/*BypassCheck=*/!UnlockRequired);
Block = TSD->getCache().allocate(ClassId);
- // If the allocation failed, the most likely reason with a 32-bit primary
- // is the region being full. In that event, retry in each successively
- // larger class until it fits. If it fails to fit in the largest class,
- // fallback to the Secondary.
+ // If the allocation failed, retry in each successively larger class until
+ // it fits. If it fails to fit in the largest class, fallback to the
+ // Secondary.
if (UNLIKELY(!Block)) {
while (ClassId < SizeClassMap::LargestClassId && !Block)
Block = TSD->getCache().allocate(++ClassId);
@@ -388,6 +387,7 @@ class Allocator {
if (UNLIKELY(!Block)) {
if (Options.get(OptionBit::MayReturnNull))
return nullptr;
+ printStats();
reportOutOfMemory(NeededSize);
}
diff --git a/compiler-rt/lib/scudo/standalone/primary64.h b/compiler-rt/lib/scudo/standalone/primary64.h
index ed0c4deaac2c8bf..6d160b4c64d75fc 100644
--- a/compiler-rt/lib/scudo/standalone/primary64.h
+++ b/compiler-rt/lib/scudo/standalone/primary64.h
@@ -214,7 +214,7 @@ template <typename Config> class SizeClassAllocator64 {
return B;
}
- bool PrintStats = false;
+ bool ReportRegionExhausted = false;
TransferBatch *B = nullptr;
while (true) {
@@ -235,19 +235,13 @@ template <typename Config> class SizeClassAllocator64 {
const bool RegionIsExhausted = Region->Exhausted;
if (!RegionIsExhausted)
B = populateFreeListAndPopBatch(C, ClassId, Region);
- PrintStats = !RegionIsExhausted && Region->Exhausted;
+ ReportRegionExhausted = !RegionIsExhausted && Region->Exhausted;
break;
}
- // Note that `getStats()` requires locking each region so we can't call it
- // while locking the Region->Mutex in the above.
- if (UNLIKELY(PrintStats)) {
- ScopedString Str;
- getStats(&Str);
- Str.append(
- "Scudo OOM: The process has exhausted %zuM for size class %zu.\n",
- RegionSize >> 20, getSizeByClassId(ClassId));
- Str.output();
+ if (UNLIKELY(ReportRegionExhausted)) {
+ Printf("Can't populate more pages for size class %zu.\n",
+ getSizeByClassId(ClassId));
// Theoretically, BatchClass shouldn't be used up. Abort immediately when
// it happens.
@@ -978,7 +972,7 @@ template <typename Config> class SizeClassAllocator64 {
"%s %02zu (%6zu): mapped: %6zuK popped: %7zu pushed: %7zu "
"inuse: %6zu total: %6zu releases: %6zu last "
"released: %6zuK latest pushed bytes: %6zuK region: 0x%zx (0x%zx)\n",
- Region->Exhausted ? "F" : " ", ClassId, getSizeByClassId(ClassId),
+ Region->Exhausted ? "E" : " ", ClassId, getSizeByClassId(ClassId),
Region->MemMapInfo.MappedUser >> 10, Region->FreeListInfo.PoppedBlocks,
Region->FreeListInfo.PushedBlocks, InUseBlocks, TotalChunks,
Region->ReleaseInfo.RangesReleased,
|
In this CL, we move the printing of allocator stats from primary.h to combined.h. This will also dump the secondary stats and reduce the log spam when an OOM happens Also change the symbol `F` to `E` to indicate region pages exhausted. It means the region can't map more pages for blocks but it may still have free blocks to allocate. `F` may hint the failure of fatel error in the region. Also update the related comments.
727599a to
2420ba9
Compare
cferris1000
approved these changes
Oct 6, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In this CL, we move the printing of allocator stats from primary.h to combined.h. This will also dump the secondary stats and reduce the log spam when an OOM happens
Also change the symbol
FtoEto indicate region pages exhausted. It means the region can't map more pages for blocks but it may still have free blocks to allocate.Fmay hint the failure of fatel error in the region. Also update the related comments.