diff --git a/compiler-rt/lib/scudo/standalone/primary64.h b/compiler-rt/lib/scudo/standalone/primary64.h --- a/compiler-rt/lib/scudo/standalone/primary64.h +++ b/compiler-rt/lib/scudo/standalone/primary64.h @@ -150,30 +150,54 @@ TransferBatch *popBatch(CacheT *C, uptr ClassId) { DCHECK_LT(ClassId, NumClasses); RegionInfo *Region = getRegionInfo(ClassId); - bool PrintStats = false; + { - ScopedLock L(Region->Mutex); + ScopedLock L(Region->FLLock); TransferBatch *B = popBatchImpl(C, ClassId, Region); if (LIKELY(B)) { Region->FreeListInfo.PoppedBlocks += B->getCount(); return B; } + } + + bool PrintStats = false; - const bool RegionIsExhausted = Region->Exhausted; - if (UNLIKELY(RegionIsExhausted || - !populateFreeList(C, ClassId, Region))) { + while (true) { + bool RegionIsExhausted = false; + + if (Region->MMLock.tryLock()) { + RegionIsExhausted = Region->Exhausted; + if (!RegionIsExhausted) + populateFreeList(C, ClassId, Region); PrintStats = !RegionIsExhausted && Region->Exhausted; + + Region->MMLock.unlock(); } else { - B = popBatchImpl(C, ClassId, Region); - // if `populateFreeList` succeeded, we are supposed to get free blocks. - DCHECK_NE(B, nullptr); - Region->FreeListInfo.PoppedBlocks += B->getCount(); - return B; + ScopedLock L(Region->MMLock); + // If we fail to acquire the `MMLock`, that means someone may have + // acquired the lock and we only need to wait its completion. Note that + // the `tryLock` may fail spuriously and we simply do another round of + // `tryLock` if it happens. + // + // TODO(chiahungduan): We should wait on the condition variable of + // freelist because it can be refilled by pushBlocks as well. } + + // FreeList may have been refilled by populateFreeList() or pushBlocks(), + // try block allocation again. + { + ScopedLock L(Region->FLLock); + TransferBatch *B = popBatchImpl(C, ClassId, Region); + if (LIKELY(B)) { + Region->FreeListInfo.PoppedBlocks += B->getCount(); + return B; + } + } + + if (RegionIsExhausted) + 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); @@ -192,45 +216,44 @@ RegionInfo *Region = getRegionInfo(ClassId); if (ClassId == SizeClassMap::BatchClassId) { - 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. - const bool NeedToRefill = - Size == 1U && Region->FreeListInfo.BlockList.empty(); - // If BatchClass has been exhausted, the program should have been - // aborted. - DCHECK(!Region->Exhausted); - - if (UNLIKELY( - NeedToRefill && - !populateFreeList(C, SizeClassMap::BatchClassId, Region))) { - PrintStats = true; - } else { - pushBlocksImpl(C, SizeClassMap::BatchClassId, Region, Array, Size); - Region->FreeListInfo.PushedBlocks += Size; + // 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. + while (true) { + { + ScopedLock L(Region->FLLock); + const bool NeedToRefill = + Size == 1U && Region->FreeListInfo.BlockList.empty(); + if (!NeedToRefill) { + pushBlocksImpl(C, SizeClassMap::BatchClassId, Region, Array, Size); + Region->FreeListInfo.PushedBlocks += Size; + return; + } } - } - // 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(); + bool Refilled; + ScopedLock L(Region->MMLock); + Refilled = populateFreeList(C, SizeClassMap::BatchClassId, Region); + + if (UNLIKELY(!Refilled)) + break; } + // At here, it hints that the BatchClass region has been used up. Dump the + // region stats before the program shutdown. + 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; } @@ -252,12 +275,20 @@ Array[J] = Cur; } - ScopedLock L(Region->Mutex); - pushBlocksImpl(C, ClassId, Region, Array, Size, SameGroup); + { + ScopedLock L(Region->FLLock); + pushBlocksImpl(C, ClassId, Region, Array, Size, SameGroup); + Region->FreeListInfo.PushedBlocks += Size; + } - Region->FreeListInfo.PushedBlocks += Size; - if (ClassId != SizeClassMap::BatchClassId) + if (ClassId == SizeClassMap::BatchClassId) + return; + + if (Region->MMLock.tryLock()) { + ScopedLock L(Region->FLLock); releaseToOSMaybe(Region, ClassId); + Region->MMLock.unlock(); + } } void disable() NO_THREAD_SAFETY_ANALYSIS { @@ -265,17 +296,21 @@ for (sptr I = static_cast(NumClasses) - 1; I >= 0; I--) { if (static_cast(I) == SizeClassMap::BatchClassId) continue; - getRegionInfo(static_cast(I))->Mutex.lock(); + getRegionInfo(static_cast(I))->MMLock.lock(); + getRegionInfo(static_cast(I))->FLLock.lock(); } - getRegionInfo(SizeClassMap::BatchClassId)->Mutex.lock(); + getRegionInfo(SizeClassMap::BatchClassId)->MMLock.lock(); + getRegionInfo(SizeClassMap::BatchClassId)->FLLock.lock(); } void enable() NO_THREAD_SAFETY_ANALYSIS { - getRegionInfo(SizeClassMap::BatchClassId)->Mutex.unlock(); + getRegionInfo(SizeClassMap::BatchClassId)->FLLock.unlock(); + getRegionInfo(SizeClassMap::BatchClassId)->MMLock.unlock(); for (uptr I = 0; I < NumClasses; I++) { if (I == SizeClassMap::BatchClassId) continue; - getRegionInfo(I)->Mutex.unlock(); + getRegionInfo(I)->FLLock.unlock(); + getRegionInfo(I)->MMLock.unlock(); } } @@ -287,7 +322,8 @@ // TODO: The call of `iterateOverBlocks` requires disabling // SizeClassAllocator64. We may consider locking each region on demand // only. - Region->Mutex.assertHeld(); + Region->FLLock.assertHeld(); + Region->MMLock.assertHeld(); const uptr BlockSize = getSizeByClassId(I); const uptr From = Region->RegionBeg; const uptr To = From + Region->MemMapInfo.AllocatedUser; @@ -303,11 +339,15 @@ uptr PushedBlocks = 0; for (uptr I = 0; I < NumClasses; I++) { RegionInfo *Region = getRegionInfo(I); - ScopedLock L(Region->Mutex); - if (Region->MemMapInfo.MappedUser) + { + ScopedLock L(Region->MMLock); TotalMapped += Region->MemMapInfo.MappedUser; - PoppedBlocks += Region->FreeListInfo.PoppedBlocks; - PushedBlocks += Region->FreeListInfo.PushedBlocks; + } + { + ScopedLock L(Region->FLLock); + PoppedBlocks += Region->FreeListInfo.PoppedBlocks; + PushedBlocks += Region->FreeListInfo.PushedBlocks; + } } Str->append("Stats: SizeClassAllocator64: %zuM mapped (%uM rss) in %zu " "allocations; remains %zu\n", @@ -316,7 +356,8 @@ for (uptr I = 0; I < NumClasses; I++) { RegionInfo *Region = getRegionInfo(I); - ScopedLock L(Region->Mutex); + ScopedLock L1(Region->MMLock); + ScopedLock L2(Region->FLLock); getStats(Str, I, Region, 0); } } @@ -339,7 +380,8 @@ if (I == SizeClassMap::BatchClassId) continue; RegionInfo *Region = getRegionInfo(I); - ScopedLock L(Region->Mutex); + ScopedLock L1(Region->MMLock); + ScopedLock L2(Region->FLLock); TotalReleasedBytes += releaseToOSMaybe(Region, I, ReleaseType); } return TotalReleasedBytes; @@ -377,7 +419,7 @@ if (I == SizeClassMap::BatchClassId) continue; uptr Begin = RegionInfoArray[I].RegionBeg; - // TODO(chiahungduan): In fact, We need to lock the RegionInfo::Mutex. + // TODO(chiahungduan): In fact, We need to lock the RegionInfo::MMLock. // However, the RegionInfoData is passed with const qualifier and lock the // mutex requires modifying RegionInfoData, which means we need to remove // the const qualifier. This may lead to another undefined behavior (The @@ -452,16 +494,20 @@ }; struct UnpaddedRegionInfo { - HybridMutex Mutex; - // This is initialized before thread creation. + // Mutex for operations on freelist + HybridMutex FLLock; + // Mutex for memmap operations + HybridMutex MMLock ACQUIRED_BEFORE(FLLock); + // `RegionBeg` is initialized before thread creation and won't be changed. uptr RegionBeg = 0; - u32 RandState GUARDED_BY(Mutex) = 0; - BlocksInfo FreeListInfo GUARDED_BY(Mutex); - PagesInfo MemMapInfo GUARDED_BY(Mutex); + // `RandState` is initialized before thread creation and won't be changed. + u32 RandState = 0; + BlocksInfo FreeListInfo GUARDED_BY(FLLock); + PagesInfo MemMapInfo GUARDED_BY(MMLock); // The minimum size of pushed blocks to trigger page release. - uptr TryReleaseThreshold GUARDED_BY(Mutex) = 0; - ReleaseToOsInfo ReleaseInfo GUARDED_BY(Mutex) = {}; - bool Exhausted GUARDED_BY(Mutex) = false; + uptr TryReleaseThreshold GUARDED_BY(MMLock) = 0; + ReleaseToOsInfo ReleaseInfo GUARDED_BY(MMLock) = {}; + bool Exhausted GUARDED_BY(MMLock) = false; }; struct RegionInfo : UnpaddedRegionInfo { char Padding[SCUDO_CACHE_LINE_SIZE - @@ -532,7 +578,7 @@ // The region mutex needs to be held while calling this method. void pushBlocksImpl(CacheT *C, uptr ClassId, RegionInfo *Region, CompactPtrT *Array, u32 Size, bool SameGroup = false) - REQUIRES(Region->Mutex) { + REQUIRES(Region->FLLock) { DCHECK_GT(Size, 0U); auto CreateGroup = [&](uptr CompactPtrGroupBase) { @@ -707,7 +753,7 @@ // // The region mutex needs to be held while calling this method. TransferBatch *popBatchImpl(CacheT *C, uptr ClassId, RegionInfo *Region) - REQUIRES(Region->Mutex) { + REQUIRES(Region->FLLock) { if (Region->FreeListInfo.BlockList.empty()) return nullptr; @@ -737,7 +783,7 @@ } NOINLINE bool populateFreeList(CacheT *C, uptr ClassId, RegionInfo *Region) - REQUIRES(Region->Mutex) { + REQUIRES(Region->MMLock) EXCLUDES(Region->FLLock) { const uptr Size = getSizeByClassId(ClassId); const u16 MaxCount = TransferBatch::getMaxCached(Size); @@ -798,6 +844,8 @@ ShuffleArray[I] = compactPtrInternal(CompactPtrBase, P); if (ClassId != SizeClassMap::BatchClassId) { + ScopedLock L(Region->FLLock); + u32 N = 1; uptr CurGroup = compactPtrGroup(ShuffleArray[0]); for (u32 I = 1; I < NumberOfBlocks; I++) { @@ -816,6 +864,8 @@ pushBlocksImpl(C, ClassId, Region, &ShuffleArray[NumberOfBlocks - N], N, /*SameGroup=*/true); } else { + ScopedLock L(Region->FLLock); + pushBlocksImpl(C, ClassId, Region, ShuffleArray, NumberOfBlocks, /*SameGroup=*/true); } @@ -828,7 +878,7 @@ } void getStats(ScopedString *Str, uptr ClassId, RegionInfo *Region, uptr Rss) - REQUIRES(Region->Mutex) { + REQUIRES(Region->MMLock, Region->FLLock) { if (Region->MemMapInfo.MappedUser == 0) return; const uptr InUse = @@ -849,7 +899,7 @@ NOINLINE uptr releaseToOSMaybe(RegionInfo *Region, uptr ClassId, ReleaseToOS ReleaseType = ReleaseToOS::Normal) - REQUIRES(Region->Mutex) { + REQUIRES(Region->MMLock, Region->FLLock) { const uptr BlockSize = getSizeByClassId(ClassId); const uptr PageSize = getPageSizeCached();