Page MenuHomePhabricator

[tsan] Respect no_huge_pages_for_shadow.
ClosedPublic

Authored by ckennelly on Aug 12 2020, 9:17 AM.

Details

Summary

Disable huge pages in the TSan shadow regions when no_huge_pages_for_shadow == true (default).

Diff Detail

Event Timeline

ckennelly created this revision.Aug 12 2020, 9:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 12 2020, 9:17 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript
ckennelly requested review of this revision.Aug 12 2020, 9:17 AM

I wonder if it would pay to selectively apply this for the parts of the shadow that correspond to shared library mappings.
Otherwise we are losing the benefits of huge pages on the heap, which should have a pretty dense shadow.
@dvyukov

compiler-rt/lib/tsan/rtl/tsan_platform_posix.cpp
93–94

This call can be removed now.

106–107

same

I wonder if it would pay to selectively apply this for the parts of the shadow that correspond to shared library mappings.
Otherwise we are losing the benefits of huge pages on the heap, which should have a pretty dense shadow.
@dvyukov

Hard to say without any data.
I know we are close to RAM limits in lots of our use cases, so if we trade a bit memory for performance, we sure will get lots of new failures without any simple ways to resolve them.
Tsan may flush shadow pretty aggressively. Consider: heap may be "dense", but now if we have just 1K region that is not used between 2 flushes, we won't page shadow for it in. With huge pages we may page in way more.

ckennelly updated this revision to Diff 285404.Aug 13 2020, 9:27 AM
ckennelly marked 2 inline comments as done.

Applied edits, and lint errors

eugenis accepted this revision.Aug 13 2020, 12:22 PM

OK, this seems to be the safe choice then. LGTM.

This revision is now accepted and ready to land.Aug 13 2020, 12:22 PM
This revision was automatically updated to reflect the committed changes.
hubert.reinterpretcast added inline comments.
compiler-rt/lib/tsan/rtl/tsan_platform_posix.cpp
93

@ckennelly:
Removing these calls are causing build failures because the internal-linkage function is no longer referenced anywhere: http://lab.llvm.org:8011/builders/clang-ppc64le-rhel/builds/6480/steps/build%20stage%201/logs/stdio