src: simplify large pages mapping code

[gabrielschulhof]

• Introduce OnScopeLeave handler for cleaning up mmap()ed range(s).
• Factor out failure scenario at the bottom of the function with
  fail label for use with goto.
• Do not allocate temporary range (nmem) on FreeBSD, because it is
  not used.

The intention is that the steps involved in re-mapping to large pages
become more clearly visible.

Signed-off-by: Gabriel Schulhof gabriel.schulhof@intel.com

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

[gabrielschulhof]

I've been trying to verify the OSX port as well as the FreeBSD port, but the FreeBSD port fails to map to large pages in order to avoid a segfault, and I have not been able to build on our OSX test machines:

test-macstadium-macos10.10-x64-1: #32467
test-macstadium-macos10.12-x64-1: #32447

Things work well on Linux though, so I removed the WIP.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/30032/

[gabrielschulhof]

@devnexen is there a combination involving FreeBSD under which the mapping actually works?

[gabrielschulhof]

@devnexen I tried building on test-digitalocean-freebsd11-x64-2 and on FreeBSD 12.1 from https://www.osboxes.org/freebsd/ and they both refuse to re-map because of the requirement that the region to map not precede the mapping function:

node/src/large_pages/node_large_page.cc

Lines 454 to 457 in 066bdec

	#if defined(__FreeBSD__)
	if (r.from < reinterpret_cast<void*>(&MoveTextRegionToLargePages))
	return -1;
	#endif

[codecov-io]

Codecov Report

Merging #32396 into master will increase coverage by 0.30%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #32396      +/-   ##
==========================================
+ Coverage   97.16%   97.46%   +0.30%     
==========================================
  Files         197      195       -2     
  Lines       65781    65701      -80     
==========================================
+ Hits        63914    64037     +123     
+ Misses       1867     1664     -203     

Impacted Files	Coverage Δ
lib/events.js	95.38% <0.00%> (-4.62%)	⬇️
lib/net.js	98.18% <0.00%> (-0.29%)	⬇️
lib/internal/modules/esm/loader.js	88.57% <0.00%> (-0.18%)	⬇️
lib/_http_server.js	98.27% <0.00%> (-0.12%)	⬇️
lib/_tls_wrap.js	95.45% <0.00%> (-0.02%)	⬇️
lib/zlib.js	98.36% <0.00%> (-0.01%)	⬇️
...l/bootstrap/switches/does_not_own_process_state.js
lib/internal/constants.js
lib/internal/console/global.js
lib/internal/bootstrap/environment.js	100.00% <0.00%> (ø)
... and 15 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c160073...470d2d2. Read the comment docs.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/30041/

[gabrielschulhof]

@devnexen NM - I got it to work.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/30099/

[jasnell]

Suggested change

	if (nmem != nullptr && nmem != MAP_FAILED && munmap(nmem, size) == -1)
	PrintSystemError(errno);
	if (tmem != nullptr && tmem != MAP_FAILED && munmap(tmem, size) == -1)
	PrintSystemError(errno);
	if ((nmem != nullptr && nmem != MAP_FAILED && munmap(nmem, size) == -1) ||
	(tmem != nullptr && tmem != MAP_FAILED && munmap(tmem, size) == -1)) {
	PrintSystemError(errno);
	}

Alternatively, it may be a bit cleaner to use a custom smart pointer with a deleter for these...

e.g. bit more verbose...

  struct Mmap {
    void* mem = nullptr;
    size_t size = 0;
    Mmap(void* start, size_t size, int prot, int flags) {
      mem = mmap(start, size, prot, flags, -1, 0);
    }
  };
  struct MmapDeleter {
    void operator()(Mmap* ptr) const noexcept {
      if (ptr.mem != nullptr &&
          ptr.mem != MAP_FAILED &&
          munmap(ptr.mem, ptr.size) == -1)
        PrintSystemError(errno); 
    }
  };
  using MmapPointer = std::unique_ptr<Mmap, MmapDeleter>;

then...

  MmapPointer nmem;
  MmapPointer tmem;

  // ...
  size_t size = r.to - r.from; 

  // ...
  nmem = std::make_unique(nullptr, size,
                          PROT_READ | PROT_WRITE,
                          MAP_PRIVATE | MAP_ANONYMOUS);
  if (nmem.mem == MAP_FAILED)
    goto fail;

  // ...
  tmem = std::make_unique(start, size,
                          PROT_READ | PROT_WRITE,
                          MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED);
  if (tmem.mem == MAP_FAILED)
    goto fail;

[gabrielschulhof]

@jasnell I've left #32532 as a todo for myself.

[gabrielschulhof]

Landed in fa3fd78.

[targos]

Depends on the large pages change to land on v12.x