Skip to content
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.

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

[scudo] Improve the message of region exhaustion #68444

Merged
merged 1 commit into from
Oct 6, 2023

Conversation

ChiaHungDuan
Copy link
Contributor

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.

@llvmbot
Copy link
Member

llvmbot commented Oct 6, 2023

@llvm/pr-subscribers-compiler-rt-sanitizer

Changes

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.


Full diff: https://github.com/llvm/llvm-project/pull/68444.diff

2 Files Affected:

  • (modified) compiler-rt/lib/scudo/standalone/combined.h (+4-4)
  • (modified) compiler-rt/lib/scudo/standalone/primary64.h (+6-12)
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.
@ChiaHungDuan ChiaHungDuan changed the title [scudo] Improve the message of region exhausted [scudo] Improve the message of region exhaustion Oct 6, 2023
@ChiaHungDuan ChiaHungDuan merged commit b53ff43 into llvm:main Oct 6, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants