Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[compiler-rt][sanitizer_common] Increase min user-map/freearray round… #73600

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

PiJoules
Copy link
Contributor

…ing size

About 90% of the time when running scudo tests on fuchsia+riscv with asan instrumentation, we will be unable to map the arena for scudo's primary allocator. We have enough space, but much of the existing allocations are fragmented enough such that we can't allocate that large enough contiguous space. This reduces fragmentation more by increase min allocation rounding for user-map and freearray in the primary allocator.

Note that changing these values doesn't have a functional effect on the actual size of the user-map or freearray portions of a region. Those are toggled by different constants. This will reduce the number of times mmap is called and reduce the number of fragments.

Locally, I've been able to run the scudo tests under asan-instrumentation on riscv 39-bit vma and haven't run into the SCUDO ERROR: zx_vmar_map error.

…ing size

About 90% of the time when running scudo tests on fuchsia+riscv with
asan instrumentation, we will be unable to map the arena for scudo's
primary allocator. We have enough space, but much of the existing
allocations are fragmented enough such that we can't allocate that large
enough contiguous space. This reduces fragmentation more by increase min
allocation rounding for user-map and freearray in the primary allocator.

Note that changing these values doesn't have a functional effect on the
actual size of the user-map or freearray portions of a region. Those are
toggled by different constants. This will reduce the number
of times mmap is called and reduce the number of fragments.

Locally, I've been able to run the scudo tests under
asan-instrumentation on riscv 39-bit vma and haven't run into the `SCUDO
ERROR: zx_vmar_map` error.
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 28, 2023

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: None (PiJoules)

Changes

…ing size

About 90% of the time when running scudo tests on fuchsia+riscv with asan instrumentation, we will be unable to map the arena for scudo's primary allocator. We have enough space, but much of the existing allocations are fragmented enough such that we can't allocate that large enough contiguous space. This reduces fragmentation more by increase min allocation rounding for user-map and freearray in the primary allocator.

Note that changing these values doesn't have a functional effect on the actual size of the user-map or freearray portions of a region. Those are toggled by different constants. This will reduce the number of times mmap is called and reduce the number of fragments.

Locally, I've been able to run the scudo tests under asan-instrumentation on riscv 39-bit vma and haven't run into the SCUDO ERROR: zx_vmar_map error.


Full diff: https://github.com/llvm/llvm-project/pull/73600.diff

1 Files Affected:

  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h (+2-2)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h
index d77bc05b780203b..34a64f26478fd5e 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h
@@ -641,11 +641,11 @@ class SizeClassAllocator64 {
   // kRegionSize must be <= 2^36, see CompactPtrT.
   COMPILER_CHECK((kRegionSize) <= (1ULL << (SANITIZER_WORDSIZE / 2 + 4)));
   // Call mmap for user memory with at least this size.
-  static const uptr kUserMapSize = 1 << 16;
+  static const uptr kUserMapSize = 1 << 18;
   // Call mmap for metadata memory with at least this size.
   static const uptr kMetaMapSize = 1 << 16;
   // Call mmap for free array memory with at least this size.
-  static const uptr kFreeArrayMapSize = 1 << 16;
+  static const uptr kFreeArrayMapSize = 1 << 18;
 
   atomic_sint32_t release_to_os_interval_ms_;
 

@PiJoules
Copy link
Contributor Author

I also haven't seen any significant time differences when running tests.

Copy link
Collaborator

@vitalybuka vitalybuka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has nothing to do with SCUDO

@vitalybuka
Copy link
Collaborator

I am confused with description, asan does not use SCUDO allocator.

Copy link
Collaborator

@vitalybuka vitalybuka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you just run some subset of scudo tests with asan.

This change looks safe for the rest

@PiJoules
Copy link
Contributor Author

PiJoules commented Nov 28, 2023

I guess you just run some subset of scudo tests with asan.

This change looks safe for the rest

Yeah, sorry I could've described this better. This doesn't have anything to do with scudo. This just helps run asan-instrumented scudo tests on 39-bit vma riscv. The order of operations is:

  1. We load a single binary that has all the tests in compiler-rt/lib/scudo/standalone/tests/. This binary is asan-instrumented.
  2. Before entering main, we setup asan and its allocator.
  3. We enter main and run some scudo tests.
  4. We reach SCUDO_TYPED_TEST(ScudoCombinedTest, IsOwned) which creates the scudo allocator for different target configs.
  5. Scudo's primary allocator attempts to mmap an arena, but fails with SCUDO ERROR: zx_vmar_allocate failed with size 11796480KB (ZX_ERR_NO_RESOURCES). This happens because much of the 39-bit vma is already occupied, mainly by the asan-shadow, asan's primary allocator, and individual mmaps done by asan. There's definitely enough space for scudo to do it's allocation, but there's just not enough contiguous space and reducing the number of freearray and user-map fragments helps.

Normally, we wouldn't have any programs that run both scudo and asan as the allocators. It just so happens that this particular one tests the allocator while the "default" one is asan's in a small vma.

@PiJoules PiJoules merged commit 1a041a3 into llvm:main Nov 28, 2023
5 checks passed
@PiJoules PiJoules deleted the increase-min-user-map-freearray-size branch November 28, 2023 22:23
Guzhu-AMD pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Nov 30, 2023
Local branch amd-gfx 5e0dd61 Merged main:38e435895779 into amd-gfx:14c4d990c781
Remote branch main 1a041a3 [compiler-rt][sanitizer_common] Increase min user-map/freearray round… (llvm#73600)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants