[scudo] Call getStats when the region is exhausted
authorChia-hung Duan <chiahungduan@google.com>
Tue, 17 Jan 2023 19:04:52 +0000 (19:04 +0000)
committerChia-hung Duan <chiahungduan@google.com>
Mon, 13 Feb 2023 23:12:15 +0000 (23:12 +0000)
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

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

index 68bf229..cf70d7b 100644 (file)
@@ -100,17 +100,39 @@ public:
   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 @@ public:
 
     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 @@ private:
           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)
index a37faac..16eae8c 100644 (file)
@@ -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);
index d38451d..3a78ab6 100644 (file)
@@ -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);