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,52 @@ 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) { + 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,30 +214,38 @@ 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. - const bool NeedToRefill = - Size == 1U && Region->FreeListInfo.BlockList.empty(); - if (UNLIKELY(NeedToRefill && - !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(); - } + 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; + } + } - pushBlocksImpl(C, SizeClassMap::BatchClassId, Region, Array, Size); - Region->FreeListInfo.PushedBlocks += Size; + 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(); + } + } return; } @@ -238,12 +268,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 { @@ -251,17 +288,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(); } } @@ -273,7 +314,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; @@ -289,11 +331,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", @@ -302,7 +348,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); } } @@ -325,7 +372,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; @@ -363,7 +410,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 @@ -438,16 +485,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 - @@ -518,7 +569,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) { @@ -693,7 +744,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; @@ -723,7 +774,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); @@ -784,6 +835,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++) { @@ -802,6 +855,8 @@ pushBlocksImpl(C, ClassId, Region, &ShuffleArray[NumberOfBlocks - N], N, /*SameGroup=*/true); } else { + ScopedLock L(Region->FLLock); + pushBlocksImpl(C, ClassId, Region, ShuffleArray, NumberOfBlocks, /*SameGroup=*/true); } @@ -814,7 +869,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 = @@ -835,7 +890,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();