[PATCH] D40038: [scudo] Soft and hard RSS limit checks

[Aleksey Shlyapnikov via Phabricator via llvm-commits]

alekseyshl added inline comments.


================
Comment at: lib/scudo/scudo_allocator.cpp:313
+    const u64 CurrentCheck = NanoTime();
+    if (CurrentCheck < LastCheck + (100ULL * 1000000ULL))
+      return atomic_load_relaxed(&RssLimitExceeded);
----------------
How about

if (LIKELY(CurrentCheck < LastCheck + (100ULL * 1000000ULL)))


================
Comment at: lib/scudo/scudo_allocator.cpp:318
+      return atomic_load_relaxed(&RssLimitExceeded);
+    const uptr CurrentRssMb = GetRSS() >> 20;
+    if (HardRssLimitMb && HardRssLimitMb < CurrentRssMb) {
----------------
How fast is it? It might read a file, right?


================
Comment at: lib/scudo/scudo_allocator.cpp:333
+        atomic_store_relaxed(&RssLimitExceeded, false);
+      }
+    }
----------------
I find this structure to be easier to grasp, but it's up to you, just a suggestion (and maybe yours yield a better code):

  if (atomic_load_relaxed(&RssLimitExceeded)) {
    if (CurrentRssMb <= SoftRssLimitMb)
      atomic_store_relaxed(&RssLimitExceeded, false);
  } else {
    if (CurrentRssMb > SoftRssLimitMb) {
      atomic_store_relaxed(&RssLimitExceeded, true);
      Report("%s: soft RSS limit exhausted (%zdMb vs %zdMb)\n",
             SanitizerToolName, SoftRssLimitMb, CurrentRssMb);
    }
  }



================
Comment at: lib/scudo/scudo_allocator.cpp:357
 
+    if (CheckRssLimit && isRssLimitExceeded())
+      return FailureHandler::OnOOM();
----------------
It needs UNLIKELY


================
Comment at: test/scudo/rss.c:45
+    printf("Some of the malloc calls returned NULL (%d)\n", returned_null);
+  printf("Done\n");
+  return 0;
----------------
What's this for?


https://reviews.llvm.org/D40038