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,51 @@ 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; } + } - const bool RegionIsExhausted = Region->Exhausted; - if (UNLIKELY(RegionIsExhausted || - !populateFreeList(C, ClassId, Region))) { + bool PrintStats = false; + + while (true) { + if (Region->MMLock.tryLock()) { + const bool RegionIsExhausted = Region->Exhausted; + const bool FreeListRefilled = !RegionIsExhausted && populateFreeList(C, ClassId, Region); PrintStats = !RegionIsExhausted && Region->Exhausted; + + Region->MMLock.unlock(); + + if (!FreeListRefilled) + break; } 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 so that any `pushBlocks` call can unblock the waiting here + // earlier. + } + + // Freelist is refilled. Try `popBatchImpl` again. + { + ScopedLock L(Region->FLLock); + TransferBatch *B = popBatchImpl(C, ClassId, Region); + if (LIKELY(B)) { + Region->FreeListInfo.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); @@ -192,26 +213,29 @@ 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 (UNLIKELY(Size == 1U && - !populateFreeList(C, SizeClassMap::BatchClassId, Region))) { - 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(); + if (UNLIKELY(Size == 1U)) { + ScopedLock L(Region->MMLock); + if (UNLIKELY( + !populateFreeList(C, SizeClassMap::BatchClassId, Region))) { + 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(); + } } + ScopedLock L(Region->FLLock); pushBlocksImpl(C, SizeClassMap::BatchClassId, Region, Array, Size); Region->FreeListInfo.PushedBlocks += Size; @@ -236,12 +260,19 @@ 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; + } + + if (ClassId == SizeClassMap::BatchClassId) + return; - Region->FreeListInfo.PushedBlocks += Size; - if (ClassId != SizeClassMap::BatchClassId) + if (Region->MMLock.tryLock()) { releaseToOSMaybe(Region, ClassId); + Region->MMLock.unlock(); + } } void disable() NO_THREAD_SAFETY_ANALYSIS { @@ -249,17 +280,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(); } } @@ -271,7 +306,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; @@ -287,11 +323,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", @@ -300,7 +340,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); } } @@ -323,7 +364,7 @@ if (I == SizeClassMap::BatchClassId) continue; RegionInfo *Region = getRegionInfo(I); - ScopedLock L(Region->Mutex); + ScopedLock L(Region->MMLock); TotalReleasedBytes += releaseToOSMaybe(Region, I, ReleaseType); } return TotalReleasedBytes; @@ -361,7 +402,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 @@ -436,16 +477,20 @@ }; struct UnpaddedRegionInfo { - HybridMutex Mutex; - // This is initialized before thread creation. + // Mutex for operations on freelist + HybridMutex FLLock; + // Mutex for memmap operations + HybridMutex MMLock; + // `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 - @@ -516,7 +561,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) { @@ -691,7 +736,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; @@ -721,7 +766,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); @@ -782,6 +827,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++) { @@ -800,6 +847,8 @@ pushBlocksImpl(C, ClassId, Region, &ShuffleArray[NumberOfBlocks - N], N, /*SameGroup=*/true); } else { + ScopedLock L(Region->FLLock); + pushBlocksImpl(C, ClassId, Region, ShuffleArray, NumberOfBlocks, /*SameGroup=*/true); } @@ -812,7 +861,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 = @@ -833,7 +882,10 @@ NOINLINE uptr releaseToOSMaybe(RegionInfo *Region, uptr ClassId, ReleaseToOS ReleaseType = ReleaseToOS::Normal) - REQUIRES(Region->Mutex) { + REQUIRES(Region->MMLock) { + // TODO(chiahungduan): Release `Region->FLLock` while doing page release. + ScopedLock L(Region->FLLock); + const uptr BlockSize = getSizeByClassId(ClassId); const uptr PageSize = getPageSizeCached();