From 2d72f17036910144fcd1a6027fb1a6842e0c5859 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Fri, 21 Feb 2020 08:26:29 -0800 Subject: [PATCH 1/3] src: include large pages source unconditionally Restrict the usage of the C preprocessor directive enabling large pages support to the large pages implementation. This cleans up the code in src/node.cc. --- node.gyp | 6 +-- src/large_pages/node_large_page.cc | 64 +++++++++++++++++++++++------- src/large_pages/node_large_page.h | 3 +- src/node.cc | 20 ++-------- 4 files changed, 55 insertions(+), 38 deletions(-) diff --git a/node.gyp b/node.gyp index ab3953b9bdfc80..1e928262ac8b74 100644 --- a/node.gyp +++ b/node.gyp @@ -620,6 +620,8 @@ 'src/histogram.h', 'src/histogram-inl.h', 'src/js_stream.h', + 'src/large_pages/node_large_page.cc', + 'src/large_pages/node_large_page.h' 'src/memory_tracker.h', 'src/memory_tracker-inl.h', 'src/module_wrap.h', @@ -851,10 +853,6 @@ 'target_arch=="x64" and ' 'node_target_type=="executable"', { 'defines': [ 'NODE_ENABLE_LARGE_CODE_PAGES=1' ], - 'sources': [ - 'src/large_pages/node_large_page.cc', - 'src/large_pages/node_large_page.h' - ], }], [ 'use_openssl_def==1', { # TODO(bnoordhuis) Make all platforms export the same list of symbols. diff --git a/src/large_pages/node_large_page.cc b/src/large_pages/node_large_page.cc index ce58e32e719bc8..c581ad4dc5f39a 100644 --- a/src/large_pages/node_large_page.cc +++ b/src/large_pages/node_large_page.cc @@ -21,6 +21,9 @@ // SPDX-License-Identifier: MIT #include "node_large_page.h" + +// Besides returning ENOTSUP at runtime we do nothing if this define is missing. +#if defined(NODE_ENABLE_LARGE_CODE_PAGES) && NODE_ENABLE_LARGE_CODE_PAGES #include "util.h" #include "uv.h" @@ -73,6 +76,8 @@ extern char __executable_start; namespace node { +namespace { + struct text_region { char* from; char* to; @@ -103,7 +108,7 @@ inline uintptr_t hugepage_align_down(uintptr_t addr) { // 00400000-00452000 r-xp 00000000 08:02 173521 /usr/bin/dbus-daemon // This is also handling the case where the first line is not the binary. -static struct text_region FindNodeTextRegion() { +struct text_region FindNodeTextRegion() { struct text_region nregion; nregion.found_text_region = false; #if defined(__linux__) @@ -263,7 +268,7 @@ static struct text_region FindNodeTextRegion() { } #if defined(__linux__) -static bool IsTransparentHugePagesEnabled() { +bool IsTransparentHugePagesEnabled() { std::ifstream ifs; ifs.open("/sys/kernel/mm/transparent_hugepage/enabled"); @@ -294,6 +299,8 @@ static bool IsSuperPagesEnabled() { } #endif +} // End of anonymous namespace + // Moving the text region to large pages. We need to be very careful. // 1: This function itself should not be moved. // We use a gcc attributes @@ -408,14 +415,26 @@ MoveTextRegionToLargePages(const text_region& r) { if (-1 == munmap(nmem, size)) PrintSystemError(errno); return ret; } +#endif // defined(NODE_ENABLE_LARGE_CODE_PAGES) && NODE_ENABLE_LARGE_CODE_PAGES // This is the primary API called from main. int MapStaticCodeToLargePages() { +#if defined(NODE_ENABLE_LARGE_CODE_PAGES) && NODE_ENABLE_LARGE_CODE_PAGES + bool have_thp = false; +#if defined(__linux__) + have_thp = IsTransparentHugePagesEnabled(); +#elif defined(__FreeBSD__) + have_thp = IsSuperPagesEnabled(); +#elif defined(__APPLE__) + // pse-36 flag is present in recent mac x64 products. + have_thp = true; +#endif + if (!have_thp) + return EACCES; + struct text_region r = FindNodeTextRegion(); - if (r.found_text_region == false) { - PrintWarning("failed to find text region"); - return -1; - } + if (r.found_text_region == false) + return ENOENT; #if defined(__FreeBSD__) if (r.from < reinterpret_cast(&MoveTextRegionToLargePages)) @@ -423,17 +442,32 @@ int MapStaticCodeToLargePages() { #endif return MoveTextRegionToLargePages(r); +#else + return ENOTSUP; +#endif } -bool IsLargePagesEnabled() { -#if defined(__linux__) - return IsTransparentHugePagesEnabled(); -#elif defined(__FreeBSD__) - return IsSuperPagesEnabled(); -#elif defined(__APPLE__) - // pse-36 flag is present in recent mac x64 products. - return true; -#endif +const char* LargePagesError(int status) { + switch (status) { + case ENOTSUP: + return "Mapping to large pages is not supported."; + + case EACCES: + return "Large pages are not enabled."; + + case ENOENT: + return "failed to find text region"; + + case -1: + return "Mapping code to large pages failed. Reverting to default page " + "size."; + + case 0: + return "OK"; + + default: + return "Unknown error"; + } } } // namespace node diff --git a/src/large_pages/node_large_page.h b/src/large_pages/node_large_page.h index bce505585cf0d0..622cf09ede4e1c 100644 --- a/src/large_pages/node_large_page.h +++ b/src/large_pages/node_large_page.h @@ -25,10 +25,9 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS - namespace node { -bool IsLargePagesEnabled(); int MapStaticCodeToLargePages(); +const char* LargePagesError(int status); } // namespace node #endif // NODE_WANT_INTERNALS diff --git a/src/node.cc b/src/node.cc index a0398b1a4f8d2c..aec70381c58e67 100644 --- a/src/node.cc +++ b/src/node.cc @@ -65,9 +65,7 @@ #include "inspector/worker_inspector.h" // ParentInspectorHandle #endif -#ifdef NODE_ENABLE_LARGE_CODE_PAGES #include "large_pages/node_large_page.h" -#endif #ifdef NODE_REPORT #include "node_report.h" @@ -936,25 +934,13 @@ InitializationResult InitializeOncePerProcess(int argc, char** argv) { } } -#if defined(NODE_ENABLE_LARGE_CODE_PAGES) && NODE_ENABLE_LARGE_CODE_PAGES if (per_process::cli_options->use_largepages == "on" || per_process::cli_options->use_largepages == "silent") { - if (node::IsLargePagesEnabled()) { - if (node::MapStaticCodeToLargePages() != 0 && - per_process::cli_options->use_largepages != "silent") { - fprintf(stderr, - "Mapping code to large pages failed. Reverting to default page " - "size.\n"); - } - } else if (per_process::cli_options->use_largepages != "silent") { - fprintf(stderr, "Large pages are not enabled.\n"); + int result = node::MapStaticCodeToLargePages(); + if (per_process::cli_options->use_largepages == "on" && result != 0) { + fprintf(stderr, "%s\n", node::LargePagesError(result)); } } -#else - if (per_process::cli_options->use_largepages == "on") { - fprintf(stderr, "Mapping to large pages is not supported.\n"); - } -#endif // NODE_ENABLE_LARGE_CODE_PAGES if (per_process::cli_options->print_version) { printf("%s\n", NODE_VERSION); From cdb7f73e2519b4de627d9ebd88d8b9cd7a76a113 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Fri, 21 Feb 2020 10:03:09 -0800 Subject: [PATCH 2/3] cerrno must be included unconditionally --- src/large_pages/node_large_page.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/large_pages/node_large_page.cc b/src/large_pages/node_large_page.cc index c581ad4dc5f39a..69572635651e2c 100644 --- a/src/large_pages/node_large_page.cc +++ b/src/large_pages/node_large_page.cc @@ -22,6 +22,8 @@ #include "node_large_page.h" +#include // NOLINT(build/include) + // Besides returning ENOTSUP at runtime we do nothing if this define is missing. #if defined(NODE_ENABLE_LARGE_CODE_PAGES) && NODE_ENABLE_LARGE_CODE_PAGES #include "util.h" @@ -38,7 +40,6 @@ #endif #include // readlink -#include // NOLINT(build/include) #include // PATH_MAX #include #include From 4327245d97013971223e9c66399c87ce680ebc71 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Fri, 21 Feb 2020 10:17:43 -0800 Subject: [PATCH 3/3] make the opening of the node namespace unconditional --- src/large_pages/node_large_page.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/large_pages/node_large_page.cc b/src/large_pages/node_large_page.cc index 69572635651e2c..a72cb65bb65674 100644 --- a/src/large_pages/node_large_page.cc +++ b/src/large_pages/node_large_page.cc @@ -75,7 +75,9 @@ extern char __executable_start; } // extern "C" #endif // defined(__linux__) +#endif // defined(NODE_ENABLE_LARGE_CODE_PAGES) && NODE_ENABLE_LARGE_CODE_PAGES namespace node { +#if defined(NODE_ENABLE_LARGE_CODE_PAGES) && NODE_ENABLE_LARGE_CODE_PAGES namespace {