Skip to content

Commit 4730aea

Browse files
committed
[scudo] Reapply "Update secondary cache time-based release logic"
This reapplies commit e5271fe. In the event that MTE is turned on, in which case released cache entries may be inserted into the cache, all released cache entries are inserted after `LastUnreleasedEntry`. Additionally, when a cache entry is moved from `Quarantine` to `Entries`, its time field will only be updated if the cache entry's memory has not been released.
1 parent 12285cc commit 4730aea

File tree

1 file changed

+59
-33
lines changed

1 file changed

+59
-33
lines changed

compiler-rt/lib/scudo/standalone/secondary.h

Lines changed: 59 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ class MapAllocatorCache {
247247
// The cache is initially empty
248248
LRUHead = CachedBlock::InvalidEntry;
249249
LRUTail = CachedBlock::InvalidEntry;
250+
LastUnreleasedEntry = CachedBlock::InvalidEntry;
250251

251252
// Available entries will be retrieved starting from the beginning of the
252253
// Entries array
@@ -321,9 +322,13 @@ class MapAllocatorCache {
321322
}
322323
CachedBlock PrevEntry = Quarantine[QuarantinePos];
323324
Quarantine[QuarantinePos] = Entry;
324-
if (OldestTime == 0)
325-
OldestTime = Entry.Time;
326325
Entry = PrevEntry;
326+
// Update the entry time to reflect the time that the
327+
// quarantined memory is placed in the Entries array
328+
// If an entry has a time value of 0, it has already
329+
// been released, so its time value must remain 0
330+
if (Entry.Time != 0)
331+
Entry.Time = Time;
327332
}
328333

329334
// All excess entries are evicted from the cache
@@ -334,16 +339,12 @@ class MapAllocatorCache {
334339
}
335340

336341
insert(Entry);
337-
338-
if (OldestTime == 0)
339-
OldestTime = Entry.Time;
340342
} while (0);
341343

342344
for (MemMapT &EvictMemMap : EvictionMemMaps)
343345
unmapCallBack(EvictMemMap);
344346

345347
if (Interval >= 0) {
346-
// TODO: Add ReleaseToOS logic to LRU algorithm
347348
releaseOlderThan(Time - static_cast<u64>(Interval) * 1000000);
348349
}
349350
}
@@ -523,22 +524,37 @@ class MapAllocatorCache {
523524
// Cache should be populated with valid entries when not empty
524525
DCHECK_NE(AvailableHead, CachedBlock::InvalidEntry);
525526

526-
u32 FreeIndex = AvailableHead;
527+
u16 FreeIndex = AvailableHead;
527528
AvailableHead = Entries[AvailableHead].Next;
529+
Entries[FreeIndex] = Entry;
528530

529-
if (EntriesCount == 0) {
530-
LRUTail = static_cast<u16>(FreeIndex);
531+
// Check list order
532+
if (EntriesCount > 1)
533+
DCHECK_GE(Entries[LRUHead].Time, Entries[Entries[LRUHead].Next].Time);
534+
535+
// Released entry goes after LastUnreleasedEntry rather than at LRUHead
536+
if (Entry.Time == 0 && LastUnreleasedEntry != CachedBlock::InvalidEntry) {
537+
Entries[FreeIndex].Next = Entries[LastUnreleasedEntry].Next;
538+
Entries[FreeIndex].Prev = LastUnreleasedEntry;
539+
Entries[LastUnreleasedEntry].Next = FreeIndex;
540+
if (LRUTail == LastUnreleasedEntry) {
541+
LRUTail = FreeIndex;
542+
} else {
543+
Entries[Entries[FreeIndex].Next].Prev = FreeIndex;
544+
}
531545
} else {
532-
// Check list order
533-
if (EntriesCount > 1)
534-
DCHECK_GE(Entries[LRUHead].Time, Entries[Entries[LRUHead].Next].Time);
535-
Entries[LRUHead].Prev = static_cast<u16>(FreeIndex);
546+
Entries[FreeIndex].Next = LRUHead;
547+
Entries[FreeIndex].Prev = CachedBlock::InvalidEntry;
548+
if (EntriesCount == 0) {
549+
LRUTail = FreeIndex;
550+
} else {
551+
Entries[LRUHead].Prev = FreeIndex;
552+
}
553+
LRUHead = FreeIndex;
554+
if (LastUnreleasedEntry == CachedBlock::InvalidEntry)
555+
LastUnreleasedEntry = FreeIndex;
536556
}
537557

538-
Entries[FreeIndex] = Entry;
539-
Entries[FreeIndex].Next = LRUHead;
540-
Entries[FreeIndex].Prev = CachedBlock::InvalidEntry;
541-
LRUHead = static_cast<u16>(FreeIndex);
542558
EntriesCount++;
543559

544560
// Availability stack should not have available entries when all entries
@@ -552,6 +568,9 @@ class MapAllocatorCache {
552568

553569
Entries[I].invalidate();
554570

571+
if (I == LastUnreleasedEntry)
572+
LastUnreleasedEntry = Entries[LastUnreleasedEntry].Prev;
573+
555574
if (I == LRUHead)
556575
LRUHead = Entries[I].Next;
557576
else
@@ -593,35 +612,39 @@ class MapAllocatorCache {
593612
}
594613
}
595614

596-
void releaseIfOlderThan(CachedBlock &Entry, u64 Time) REQUIRES(Mutex) {
597-
if (!Entry.isValid() || !Entry.Time)
598-
return;
599-
if (Entry.Time > Time) {
600-
if (OldestTime == 0 || Entry.Time < OldestTime)
601-
OldestTime = Entry.Time;
602-
return;
603-
}
615+
inline void release(CachedBlock &Entry) {
616+
DCHECK(Entry.Time != 0);
604617
Entry.MemMap.releaseAndZeroPagesToOS(Entry.CommitBase, Entry.CommitSize);
605618
Entry.Time = 0;
606619
}
607620

608621
void releaseOlderThan(u64 Time) EXCLUDES(Mutex) {
609622
ScopedLock L(Mutex);
610-
if (!EntriesCount || OldestTime == 0 || OldestTime > Time)
623+
if (!EntriesCount)
611624
return;
612-
OldestTime = 0;
613-
for (uptr I = 0; I < Config::getQuarantineSize(); I++)
614-
releaseIfOlderThan(Quarantine[I], Time);
615-
for (uptr I = 0; I < Config::getEntriesArraySize(); I++)
616-
releaseIfOlderThan(Entries[I], Time);
617-
}
618625

626+
// TODO: Add conditional to skip iteration over quarantine
627+
// if quarantine is disabled
628+
for (uptr I = 0; I < Config::getQuarantineSize(); I++) {
629+
CachedBlock &ReleaseEntry = Quarantine[I];
630+
if (!ReleaseEntry.isValid() || !ReleaseEntry.Time ||
631+
ReleaseEntry.Time > Time)
632+
continue;
633+
release(ReleaseEntry);
634+
}
635+
636+
// Release oldest entries first by releasing from decommit base
637+
while (LastUnreleasedEntry != CachedBlock::InvalidEntry &&
638+
Entries[LastUnreleasedEntry].Time <= Time) {
639+
release(Entries[LastUnreleasedEntry]);
640+
LastUnreleasedEntry = Entries[LastUnreleasedEntry].Prev;
641+
}
642+
}
619643
HybridMutex Mutex;
620644
u32 EntriesCount GUARDED_BY(Mutex) = 0;
621645
u32 QuarantinePos GUARDED_BY(Mutex) = 0;
622646
atomic_u32 MaxEntriesCount = {};
623647
atomic_uptr MaxEntrySize = {};
624-
u64 OldestTime GUARDED_BY(Mutex) = 0;
625648
atomic_s32 ReleaseToOsIntervalMs = {};
626649
u32 CallsToRetrieve GUARDED_BY(Mutex) = 0;
627650
u32 SuccessfulRetrieves GUARDED_BY(Mutex) = 0;
@@ -636,6 +659,9 @@ class MapAllocatorCache {
636659
u16 LRUTail GUARDED_BY(Mutex) = 0;
637660
// The AvailableHead is the top of the stack of available entries
638661
u16 AvailableHead GUARDED_BY(Mutex) = 0;
662+
// The LastUnreleasedEntry is the least recently used entry that has not
663+
// been released
664+
u16 LastUnreleasedEntry GUARDED_BY(Mutex) = 0;
639665
};
640666

641667
template <typename Config> class MapAllocator {

0 commit comments

Comments
 (0)