[compiler-rt] f7016f8 - [scudo] Call getStats when the region is exhausted

Chia-hung Duan via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 13 15:12:50 PST 2023


Author: Chia-hung Duan
Date: 2023-02-13T23:12:15Z
New Revision: f7016f8a8ea296f980dd38e0bcb4201353ec5430

URL: https://github.com/llvm/llvm-project/commit/f7016f8a8ea296f980dd38e0bcb4201353ec5430
DIFF: https://github.com/llvm/llvm-project/commit/f7016f8a8ea296f980dd38e0bcb4201353ec5430.diff

LOG: [scudo] Call getStats when the region is exhausted

Because of lock contention, we temporarily disabled the printing of
regions' status when it's exhausted. Given that it's useful when the
Region OOM happens, this CL brings it back without lock contention.

Differential Revision: https://reviews.llvm.org/D141955

Added: 
    

Modified: 
    compiler-rt/lib/scudo/standalone/primary64.h
    compiler-rt/lib/scudo/standalone/report.cpp
    compiler-rt/lib/scudo/standalone/report.h

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/scudo/standalone/primary64.h b/compiler-rt/lib/scudo/standalone/primary64.h
index 68bf229364438..cf70d7b9fbd9e 100644
--- a/compiler-rt/lib/scudo/standalone/primary64.h
+++ b/compiler-rt/lib/scudo/standalone/primary64.h
@@ -100,17 +100,39 @@ template <typename Config> class SizeClassAllocator64 {
   TransferBatch *popBatch(CacheT *C, uptr ClassId) {
     DCHECK_LT(ClassId, NumClasses);
     RegionInfo *Region = getRegionInfo(ClassId);
-    ScopedLock L(Region->Mutex);
-    TransferBatch *B = popBatchImpl(C, ClassId);
-    if (UNLIKELY(!B)) {
-      if (UNLIKELY(!populateFreeList(C, ClassId, Region)))
-        return nullptr;
-      B = popBatchImpl(C, ClassId);
-      // if `populateFreeList` succeeded, we are supposed to get free blocks.
-      DCHECK_NE(B, nullptr);
+    bool PrintStats = false;
+    {
+      ScopedLock L(Region->Mutex);
+      TransferBatch *B = popBatchImpl(C, ClassId);
+      if (LIKELY(B)) {
+        Region->Stats.PoppedBlocks += B->getCount();
+        return B;
+      }
+
+      const bool RegionIsExhausted = Region->Exhausted;
+      if (UNLIKELY(RegionIsExhausted ||
+                   !populateFreeList(C, ClassId, Region))) {
+        PrintStats = !RegionIsExhausted && Region->Exhausted;
+      } else {
+        B = popBatchImpl(C, ClassId);
+        // if `populateFreeList` succeeded, we are supposed to get free blocks.
+        DCHECK_NE(B, nullptr);
+        Region->Stats.PoppedBlocks += B->getCount();
+        return B;
+      }
     }
-    Region->Stats.PoppedBlocks += B->getCount();
-    return B;
+
+    // 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();
+    }
+    return nullptr;
   }
 
   // Push the array of free blocks to the designated batch group.
@@ -120,17 +142,41 @@ template <typename Config> class SizeClassAllocator64 {
 
     RegionInfo *Region = getRegionInfo(ClassId);
     if (ClassId == SizeClassMap::BatchClassId) {
-      ScopedLock L(Region->Mutex);
-      // Constructing a batch group in the free list will use two blocks in
-      // BatchClassId. If we are pushing BatchClassId blocks, we will use the
-      // blocks in the array directly (can't delegate local cache which will
-      // cause a recursive allocation). However, The number of free blocks may
-      // be less than two. Therefore, populate the free list before inserting
-      // the blocks.
-      if (Size == 1 && UNLIKELY(!populateFreeList(C, ClassId, Region)))
-        return;
-      pushBlocksImpl(C, ClassId, Array, Size);
-      Region->Stats.PushedBlocks += Size;
+      bool PrintStats = false;
+      {
+        ScopedLock L(Region->Mutex);
+        // Constructing a batch group in the free list will use two blocks in
+        // BatchClassId. If we are pushing BatchClassId blocks, we will use the
+        // blocks in the array directly (can't delegate local cache which will
+        // cause a recursive allocation). However, The number of free blocks may
+        // be less than two. Therefore, populate the free list before inserting
+        // the blocks.
+        if (Size >= 2U) {
+          pushBlocksImpl(C, SizeClassMap::BatchClassId, Array, Size);
+          Region->Stats.PushedBlocks += Size;
+        } else {
+          const bool RegionIsExhausted = Region->Exhausted;
+          if (UNLIKELY(
+                  RegionIsExhausted ||
+                  !populateFreeList(C, SizeClassMap::BatchClassId, Region))) {
+            PrintStats = !RegionIsExhausted && Region->Exhausted;
+          }
+        }
+      }
+
+      // Note that `getStats()` requires the lock of 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();
+        // Theoretically, BatchClass shouldn't be used up. Abort immediately
+        // when it happens.
+        reportOutOfBatchClass();
+      }
       return;
     }
 
@@ -578,19 +624,7 @@ template <typename Config> class SizeClassAllocator64 {
           roundUpTo(TotalUserBytes - MappedUser, MapSizeIncrement);
       const uptr RegionBase = RegionBeg - getRegionBaseByClassId(ClassId);
       if (UNLIKELY(RegionBase + MappedUser + MapSize > RegionSize)) {
-        if (!Region->Exhausted) {
-          Region->Exhausted = true;
-          ScopedString Str;
-          // FIXME: getStats() needs to go over all the regions and
-          // will take the locks of them. Which means we will try to recursively
-          // acquire the `Region->Mutex` which is not supported. It will be
-          // better to log this somewhere else.
-          // getStats(&Str);
-          Str.append(
-              "Scudo OOM: The process has exhausted %zuM for size class %zu.\n",
-              RegionSize >> 20, Size);
-          Str.output();
-        }
+        Region->Exhausted = true;
         return false;
       }
       if (MappedUser == 0)

diff  --git a/compiler-rt/lib/scudo/standalone/report.cpp b/compiler-rt/lib/scudo/standalone/report.cpp
index a37faacbb932a..16eae8c3136d4 100644
--- a/compiler-rt/lib/scudo/standalone/report.cpp
+++ b/compiler-rt/lib/scudo/standalone/report.cpp
@@ -112,6 +112,11 @@ void NORETURN reportAllocationSizeTooBig(uptr UserSize, uptr TotalSize,
                 UserSize, TotalSize, MaxSize);
 }
 
+void NORETURN reportOutOfBatchClass() {
+  ScopedErrorReport Report;
+  Report.append("BatchClass region is used up, can't hold any free block\n");
+}
+
 void NORETURN reportOutOfMemory(uptr RequestedSize) {
   ScopedErrorReport Report;
   Report.append("out of memory trying to allocate %zu bytes\n", RequestedSize);

diff  --git a/compiler-rt/lib/scudo/standalone/report.h b/compiler-rt/lib/scudo/standalone/report.h
index d38451da09881..3a78ab64b13f0 100644
--- a/compiler-rt/lib/scudo/standalone/report.h
+++ b/compiler-rt/lib/scudo/standalone/report.h
@@ -32,6 +32,7 @@ void NORETURN reportSanityCheckError(const char *Field);
 void NORETURN reportAlignmentTooBig(uptr Alignment, uptr MaxAlignment);
 void NORETURN reportAllocationSizeTooBig(uptr UserSize, uptr TotalSize,
                                          uptr MaxSize);
+void NORETURN reportOutOfBatchClass();
 void NORETURN reportOutOfMemory(uptr RequestedSize);
 void NORETURN reportSoftRSSLimit(uptr RssLimitMb);
 void NORETURN reportHardRSSLimit(uptr RssLimitMb);


        


More information about the llvm-commits mailing list