[scudo][standalone] Precommit pages

[yaneury]

Summary

On Fuchsia, this CL changes garbage collection
to precommit all pages if the |Buffer| doesn't
fit into the static buffer size.

A test program (scudotest) was used that deliberately
grows a size class high water mark to the point where
the pre-allocated static buffer is no longer used for
garbage collection.

Traces showed that precommiting the Vmar removes ~30 page faults
and ~.22ms of wall time.*

Before: https://ui.perfetto.dev/#!/?s=7da19fc3f59448eef51fd6fd03283bb87b702cf1a565bcbe6c9c28371671
After: https://ui.perfetto.dev/#!/?s=97707cd99b2c9efd1e6569b2deb97e3d16f8be532c59a0cc12463c37fbb1d8

*: Use the added zx_vmar_op_range as a reference point to observe
the differences.

For more context, see https://fxbug.dev/115594.

[cryptoad]

compiler-rt/lib/scudo/standalone/release.h

117

Probably can use a construct like MAP_ALLOWNOMEM | (SCUDO_FUCHSIA ? MAP_PRECOMMIT : 0) (or something in that general realm) to avoid the #ifdef duplication

[Chia-hungDuan]

The reduction of number of page faults is expected. I'm curious about how it impacts the real cases(Especially when we have D140311). Like the scenario mentioned in https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=115594#c3

[yaneury]

Readability improvements

[yaneury]

Readability improvements

[Chia-hungDuan]

compiler-rt/lib/scudo/standalone/release.h

117

Sorry I should leave the comment earlier. Can we do something like

// Comment...
int MmapFlags = ...;
...
... = map (... MmapFlags ...);

So that the comment be attached to the flag closer and we can refine the comment to be more Fuchsia specific.
Just to raise the concern here again (it was in D139897). We may over commit pages before we can allocate page map only for groups to be released (I'm working on this). Now this is known to be benefit to Fuchsia platform so I think it'll be better to make it Fuchsia specific at this moment.

[yaneury]

Readability improvements, use MmapFlags var

[cryptoad]

compiler-rt/lib/scudo/standalone/release.h

114

If you go that route, const this, and also flags is a uptr in the prototype of map, so we should keep that type to avoid compiler complains.

[yaneury]

Set type of MmapFlags to const uptr

[Chia-hungDuan]

compiler-rt/lib/scudo/standalone/fuchsia.cpp

106–109

Just out of curious, can this be done at _zx_vmar_map?

[yaneury]

compiler-rt/lib/scudo/standalone/fuchsia.cpp

106–109

It's not supported AFAIK but would be a useful addition.

[Chia-hungDuan]

compiler-rt/lib/scudo/standalone/fuchsia.cpp

106–109

Is ZX_VM_MAP_RANGE something similar? Sorry, I'm not familiar with Fuchsia, just did a random scan of that page.

[yaneury]

compiler-rt/lib/scudo/standalone/fuchsia.cpp

106–109

Good question. Asked the kernel folks about this and there's a slight difference. ZX_VM_MAP_RANGE will only fill hardware translation for present pages. ZX_VMAR_OP_COMMIT, on the other hand, does this AND commits the absent pages into the backing VMO(s).

[yaneury]

Btw, how can I get this change to land? I don't see a submit button...

[yaneury]

@Chia-hungDuan, would you mind submitting this change?

[Chia-hungDuan]

In D140320#4008151, @yaneury wrote:

@Chia-hungDuan, would you mind submitting this change?

Sure, I'll do it later

compiler-rt/lib/scudo/standalone/fuchsia.cpp

106–109

Thanks for explanation!

compiler-rt/lib/scudo/standalone/common.h

 bool getRandom(void *Buffer, uptr Length, bool Blocking = false);
 
 // Platform memory mapping functions.
 
 #define MAP_ALLOWNOMEM (1U << 0)
 #define MAP_NOACCESS (1U << 1)
 #define MAP_RESIZABLE (1U << 2)
 #define MAP_MEMTAG (1U << 3)
+#define MAP_PRECOMMIT (1U << 4)
 
 // Our platform memory mapping use is restricted to 3 scenarios:
 // - reserve memory at a random address (MAP_NOACCESS);
 // - commit memory in a previously reserved space;
 // - commit memory at a random address.
 // As such, only a subset of parameters combinations is valid, which is checked
 // by the function implementation. The Data parameter allows to pass opaque
 // platform specific data to the function.

compiler-rt/lib/scudo/standalone/fuchsia.cpp

zx_vm_option_t MapFlags =
       ZX_VM_PERM_READ | ZX_VM_PERM_WRITE | ZX_VM_ALLOW_FAULTS;
   if (Addr)
     DCHECK(Data);
   const uint64_t Offset =
       Addr ? reinterpret_cast<uintptr_t>(Addr) - Data->VmarBase : 0;
   if (Offset)
     MapFlags |= ZX_VM_SPECIFIC;
   Status = _zx_vmar_map(Vmar, MapFlags, Offset, Vmo, VmoSize, Size, &P);
+  if (UNLIKELY(Status != ZX_OK)) {
+    if (Status != ZX_ERR_NO_MEMORY || !AllowNoMem)
+      dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY ? Size : 0);
+    return nullptr;
+  }
+
+  if (Flags & MAP_PRECOMMIT) {
+    Status = _zx_vmar_op_range(Vmar, ZX_VMAR_OP_COMMIT, P, Size,
+                               /*buffer=*/nullptr, /*buffer_size=*/0);
+  }
+
   // No need to track the Vmo if we don't intend on resizing it. Close it.
   if (Flags & MAP_RESIZABLE) {
     DCHECK(Data);
     if (Data->Vmo == ZX_HANDLE_INVALID)
       Data->Vmo = Vmo;
     else
       DCHECK_EQ(Data->Vmo, Vmo);
   } else {
     CHECK_EQ(_zx_handle_close(Vmo), ZX_OK);
   }
   if (UNLIKELY(Status != ZX_OK)) {
     if (Status != ZX_ERR_NO_MEMORY || !AllowNoMem)
       dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY ? Size : 0);
     return nullptr;
   }
+
   if (Data)
     Data->VmoSize += Size;
 
   return reinterpret_cast<void *>(P);
 }
 
 void unmap(void *Addr, uptr Size, uptr Flags, MapPlatformData *Data) {
   if (Flags & UNMAP_ALL) {

compiler-rt/lib/scudo/standalone/release.h

SizePerRegion =
         roundUpTo(NumCounters, static_cast<uptr>(1U) << PackingRatioLog) >>
         PackingRatioLog;
     BufferSize = SizePerRegion * sizeof(*Buffer) * Regions;
     if (BufferSize <= (StaticBufferCount * sizeof(Buffer[0])) &&
         Mutex.tryLock()) {
       Buffer = &StaticBuffer[0];
       memset(Buffer, 0, BufferSize);
     } else {
+      // When using a heap-based buffer, precommit the pages backing the
+      // Vmar by passing |MAP_PRECOMMIT| flag. This allows an optimization
+      // where page fault exceptions are skipped as the allocated memory
+      // is accessed.
+      const uptr MmapFlags =
+          MAP_ALLOWNOMEM | (SCUDO_FUCHSIA ? MAP_PRECOMMIT : 0);
       Buffer = reinterpret_cast<uptr *>(
           map(nullptr, roundUpTo(BufferSize, getPageSizeCached()),
-              "scudo:counters", MAP_ALLOWNOMEM, &MapData));
+              "scudo:counters", MmapFlags, &MapData));
     }
   }
 
   bool isAllocated() const { return !!Buffer; }
 
   uptr getCount() const { return NumCounters; }
 
   uptr get(uptr Region, uptr I) const {