Skip to content

Commit

Permalink
[scudo] Fix the logic of MaxAllowedFragmentedPages (#107927)
Browse files Browse the repository at this point in the history
MTE doesn't support MaxReleasedCachePages which may break the assumption
that only the first 4 pages will have memory tagged.

GitOrigin-RevId: 6e854a6a01d310689a8b5d50126decd46b3880ea
Change-Id: Ib0aaacd0eb66d155fdd075c03bc1a9e4642e16f7
  • Loading branch information
ChiaHungDuan authored and copybara-github committed Sep 11, 2024
1 parent 7befc97 commit 4632745
Showing 1 changed file with 17 additions and 8 deletions.
25 changes: 17 additions & 8 deletions standalone/secondary.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,16 @@ namespace {
struct CachedBlock {
static constexpr u16 CacheIndexMax = UINT16_MAX;
static constexpr u16 InvalidEntry = CacheIndexMax;
// * 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.
// 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.
static constexpr uptr MaxReleasedCachePages = 4U;

uptr CommitBase = 0;
Expand Down Expand Up @@ -725,8 +728,14 @@ MapAllocator<Config>::tryAllocateFromCache(const Options &Options, uptr Size,
uptr EntryHeaderPos;
uptr MaxAllowedFragmentedPages = MaxUnreleasedCachePages;

if (UNLIKELY(useMemoryTagging<Config>(Options)))
if (LIKELY(!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);
Expand Down

0 comments on commit 4632745

Please sign in to comment.