<div dir="ltr">On it, thanks!</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jan 25, 2019 at 6:57 AM Yvan Roux <<a href="mailto:yvan.roux@linaro.org">yvan.roux@linaro.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Kostya<br>
<br>
This change broke armv7 bot, logs are available here:<br>
<a href="http://lab.llvm.org:8011/builders/clang-cmake-armv7-full/builds/4600/steps/ninja%20check%201/logs/FAIL%3A%20Scudo-armhf%3A%3A%20rss.c" rel="noreferrer" target="_blank">http://lab.llvm.org:8011/builders/clang-cmake-armv7-full/builds/4600/steps/ninja%20check%201/logs/FAIL%3A%20Scudo-armhf%3A%3A%20rss.c</a><br>
<br>
Cheers,<br>
Yvan<br>
<br>
<br>
On Thu, 24 Jan 2019 at 16:56, Kostya Kortchinsky via llvm-commits<br>
<<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
><br>
> Author: cryptoad<br>
> Date: Thu Jan 24 07:56:54 2019<br>
> New Revision: 352057<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=352057&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=352057&view=rev</a><br>
> Log:<br>
> [scudo] Tuning changes based on feedback from current use<br>
><br>
> Summary:<br>
> This tunes several of the default parameters used within the allocator:<br>
> - disable the deallocation type mismatch on Android by default; this<br>
> was causing too many issues with third party libraries;<br>
> - change the default `SizeClassMap` to `Dense`, it caches less entries<br>
> and is way more memory efficient overall;<br>
> - relax the timing of the RSS checks, 10 times per second was too much,<br>
> lower it to 4 times (every 250ms), and update the test so that it<br>
> passes with the new default.<br>
><br>
> Reviewers: eugenis<br>
><br>
> Reviewed By: eugenis<br>
><br>
> Subscribers: srhines, delcypher, #sanitizers, llvm-commits<br>
><br>
> Differential Revision: <a href="https://reviews.llvm.org/D57116" rel="noreferrer" target="_blank">https://reviews.llvm.org/D57116</a><br>
><br>
> Modified:<br>
> compiler-rt/trunk/lib/scudo/scudo_allocator.cpp<br>
> compiler-rt/trunk/lib/scudo/scudo_flags.inc<br>
> compiler-rt/trunk/lib/scudo/scudo_platform.h<br>
> compiler-rt/trunk/test/scudo/rss.c<br>
><br>
> Modified: compiler-rt/trunk/lib/scudo/scudo_allocator.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/scudo/scudo_allocator.cpp?rev=352057&r1=352056&r2=352057&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/scudo/scudo_allocator.cpp?rev=352057&r1=352056&r2=352057&view=diff</a><br>
> ==============================================================================<br>
> --- compiler-rt/trunk/lib/scudo/scudo_allocator.cpp (original)<br>
> +++ compiler-rt/trunk/lib/scudo/scudo_allocator.cpp Thu Jan 24 07:56:54 2019<br>
> @@ -587,11 +587,11 @@ NOINLINE void Allocator::performSanityCh<br>
> }<br>
><br>
> // Opportunistic RSS limit check. This will update the RSS limit status, if<br>
> -// it can, every 100ms, otherwise it will just return the current one.<br>
> +// it can, every 250ms, otherwise it will just return the current one.<br>
> NOINLINE bool Allocator::isRssLimitExceeded() {<br>
> u64 LastCheck = atomic_load_relaxed(&RssLastCheckedAtNS);<br>
> const u64 CurrentCheck = MonotonicNanoTime();<br>
> - if (LIKELY(CurrentCheck < LastCheck + (100ULL * 1000000ULL)))<br>
> + if (LIKELY(CurrentCheck < LastCheck + (250ULL * 1000000ULL)))<br>
> return atomic_load_relaxed(&RssLimitExceeded);<br>
> if (!atomic_compare_exchange_weak(&RssLastCheckedAtNS, &LastCheck,<br>
> CurrentCheck, memory_order_relaxed))<br>
><br>
> Modified: compiler-rt/trunk/lib/scudo/scudo_flags.inc<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/scudo/scudo_flags.inc?rev=352057&r1=352056&r2=352057&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/scudo/scudo_flags.inc?rev=352057&r1=352056&r2=352057&view=diff</a><br>
> ==============================================================================<br>
> --- compiler-rt/trunk/lib/scudo/scudo_flags.inc (original)<br>
> +++ compiler-rt/trunk/lib/scudo/scudo_flags.inc Thu Jan 24 07:56:54 2019<br>
> @@ -36,7 +36,9 @@ SCUDO_FLAG(int, QuarantineChunksUpToSize<br>
> "Size in bytes up to which chunks will be quarantined (if lower than"<br>
> "or equal to). Defaults to 256 (32-bit) or 2048 (64-bit)")<br>
><br>
> -SCUDO_FLAG(bool, DeallocationTypeMismatch, true,<br>
> +// Disable the deallocation type check by default on Android, it causes too many<br>
> +// issues with third party libraries.<br>
> +SCUDO_FLAG(bool, DeallocationTypeMismatch, !SANITIZER_ANDROID,<br>
> "Report errors on malloc/delete, new/free, new/delete[], etc.")<br>
><br>
> SCUDO_FLAG(bool, DeleteSizeMismatch, true,<br>
><br>
> Modified: compiler-rt/trunk/lib/scudo/scudo_platform.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/scudo/scudo_platform.h?rev=352057&r1=352056&r2=352057&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/scudo/scudo_platform.h?rev=352057&r1=352056&r2=352057&view=diff</a><br>
> ==============================================================================<br>
> --- compiler-rt/trunk/lib/scudo/scudo_platform.h (original)<br>
> +++ compiler-rt/trunk/lib/scudo/scudo_platform.h Thu Jan 24 07:56:54 2019<br>
> @@ -79,7 +79,7 @@ const uptr RegionSizeLog = SANITIZER_AND<br>
> #endif // SANITIZER_CAN_USE_ALLOCATOR64<br>
><br>
> #if !defined(SCUDO_SIZE_CLASS_MAP)<br>
> -# define SCUDO_SIZE_CLASS_MAP Default<br>
> +# define SCUDO_SIZE_CLASS_MAP Dense<br>
> #endif<br>
><br>
> #define SIZE_CLASS_MAP_TYPE SIZE_CLASS_MAP_TYPE_(SCUDO_SIZE_CLASS_MAP)<br>
><br>
> Modified: compiler-rt/trunk/test/scudo/rss.c<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/scudo/rss.c?rev=352057&r1=352056&r2=352057&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/scudo/rss.c?rev=352057&r1=352056&r2=352057&view=diff</a><br>
> ==============================================================================<br>
> --- compiler-rt/trunk/test/scudo/rss.c (original)<br>
> +++ compiler-rt/trunk/test/scudo/rss.c Thu Jan 24 07:56:54 2019<br>
> @@ -1,15 +1,15 @@<br>
> // RUN: %clang_scudo %s -o %t<br>
> // RUN: %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-nolimit<br>
> -// RUN: %env_scudo_opts="soft_rss_limit_mb=256" %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-nolimit<br>
> -// RUN: %env_scudo_opts="hard_rss_limit_mb=256" %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-nolimit<br>
> -// RUN: %env_scudo_opts="soft_rss_limit_mb=64:allocator_may_return_null=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit<br>
> -// RUN: %env_scudo_opts="soft_rss_limit_mb=64:allocator_may_return_null=1" %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit-returnnull<br>
> -// RUN: %env_scudo_opts="soft_rss_limit_mb=64:allocator_may_return_null=0:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit<br>
> -// RUN: %env_scudo_opts="soft_rss_limit_mb=64:allocator_may_return_null=1:can_use_proc_maps_statm=0" %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit-returnnull<br>
> -// RUN: %env_scudo_opts="hard_rss_limit_mb=64:allocator_may_return_null=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit<br>
> -// RUN: %env_scudo_opts="hard_rss_limit_mb=64:allocator_may_return_null=1" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit<br>
> -// RUN: %env_scudo_opts="hard_rss_limit_mb=64:allocator_may_return_null=0:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit<br>
> -// RUN: %env_scudo_opts="hard_rss_limit_mb=64:allocator_may_return_null=1:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit<br>
> +// RUN: %env_scudo_opts="soft_rss_limit_mb=128" %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-nolimit<br>
> +// RUN: %env_scudo_opts="hard_rss_limit_mb=128" %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-nolimit<br>
> +// RUN: %env_scudo_opts="soft_rss_limit_mb=32:allocator_may_return_null=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit<br>
> +// RUN: %env_scudo_opts="soft_rss_limit_mb=32:allocator_may_return_null=1" %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit-returnnull<br>
> +// RUN: %env_scudo_opts="soft_rss_limit_mb=32:allocator_may_return_null=0:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit<br>
> +// RUN: %env_scudo_opts="soft_rss_limit_mb=32:allocator_may_return_null=1:can_use_proc_maps_statm=0" %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit-returnnull<br>
> +// RUN: %env_scudo_opts="hard_rss_limit_mb=32:allocator_may_return_null=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit<br>
> +// RUN: %env_scudo_opts="hard_rss_limit_mb=32:allocator_may_return_null=1" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit<br>
> +// RUN: %env_scudo_opts="hard_rss_limit_mb=32:allocator_may_return_null=0:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit<br>
> +// RUN: %env_scudo_opts="hard_rss_limit_mb=32:allocator_may_return_null=1:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit<br>
><br>
> // Tests that the soft and hard RSS limits work as intended. Without limit or<br>
> // with a high limit, the test should pass without any malloc returning NULL or<br>
> @@ -22,7 +22,7 @@<br>
> #include <string.h><br>
> #include <unistd.h><br>
><br>
> -static const size_t kNumAllocs = 128;<br>
> +static const size_t kNumAllocs = 64;<br>
> static const size_t kAllocSize = 1 << 20; // 1MB.<br>
><br>
> static void *allocs[kNumAllocs];<br>
> @@ -31,7 +31,7 @@ int main(int argc, char *argv[]) {<br>
> int returned_null = 0;<br>
> for (int i = 0; i < kNumAllocs; i++) {<br>
> if ((i & 0xf) == 0)<br>
> - usleep(50000);<br>
> + usleep(100000);<br>
> allocs[i] = malloc(kAllocSize);<br>
> if (allocs[i])<br>
> memset(allocs[i], 0xff, kAllocSize); // Dirty the pages.<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>