[compiler-rt] Revert "[scudo] Fix the logic of MaxAllowedFragmentedPages" (PR #108130)

via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 10 18:56:09 PDT 2024


https://github.com/ChiaHungDuan created https://github.com/llvm/llvm-project/pull/108130

Reverts llvm/llvm-project#107927

>From 00b2d78142495605c0c01f9228620a96b5b949d3 Mon Sep 17 00:00:00 2001
From: ChiaHungDuan <f103119 at gmail.com>
Date: Tue, 10 Sep 2024 18:55:12 -0700
Subject: [PATCH] Revert "[scudo] Fix the logic of MaxAllowedFragmentedPages
 (#107927)"

This reverts commit 6e854a6a01d310689a8b5d50126decd46b3880ea.
---
 compiler-rt/lib/scudo/standalone/secondary.h | 25 +++++++-------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/compiler-rt/lib/scudo/standalone/secondary.h b/compiler-rt/lib/scudo/standalone/secondary.h
index c79ec1360b00a7..1a232b9b9fb2d7 100644
--- a/compiler-rt/lib/scudo/standalone/secondary.h
+++ b/compiler-rt/lib/scudo/standalone/secondary.h
@@ -72,16 +72,13 @@ namespace {
 struct CachedBlock {
   static constexpr u16 CacheIndexMax = UINT16_MAX;
   static constexpr u16 InvalidEntry = CacheIndexMax;
-  // We allow a certain amount of fragmentation and part of the fragmented bytes
-  // will be released by `releaseAndZeroPagesToOS()`. This increases the chance
-  // of cache hit rate and reduces the overhead to the RSS at the same time. See
-  // more details in the `MapAllocatorCache::retrieve()` section.
-  //
-  // We arrived at this default value after noticing that mapping in larger
-  // memory regions performs better than releasing memory and forcing a cache
-  // hit. According to the data, it suggests that beyond 4 pages, the release
-  // execution time is longer than the map execution time. In this way,
-  // the default is dependent on the platform.
+  //   * MaxReleasedCachePages default is currently 4
+  //        - We arrived at this value after noticing that mapping
+  //        in larger memory regions performs better than releasing
+  //        memory and forcing a cache hit. According to the data,
+  //        it suggests that beyond 4 pages, the release execution time is
+  //        longer than the map execution time. In this way, the default
+  //        is dependent on the platform.
   static constexpr uptr MaxReleasedCachePages = 4U;
 
   uptr CommitBase = 0;
@@ -728,14 +725,8 @@ MapAllocator<Config>::tryAllocateFromCache(const Options &Options, uptr Size,
   uptr EntryHeaderPos;
   uptr MaxAllowedFragmentedPages = MaxUnreleasedCachePages;
 
-  if (LIKELY(!useMemoryTagging<Config>(Options))) {
+  if (UNLIKELY(useMemoryTagging<Config>(Options)))
     MaxAllowedFragmentedPages += CachedBlock::MaxReleasedCachePages;
-  } else {
-    // TODO: Enable MaxReleasedCachePages may result in pages for an entry being
-    // partially released and it erases the tag of those pages as well. To
-    // support this feature for MTE, we need to tag those pages again.
-    DCHECK_EQ(CachedBlock::MaxReleasedCachePages, 0U);
-  }
 
   Entry = Cache.retrieve(MaxAllowedFragmentedPages, Size, Alignment,
                          getHeadersSize(), EntryHeaderPos);



More information about the llvm-commits mailing list