Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

8261401: Add sanity check for UseSHM large pages similar to the one used with hugetlb large pages #2488

Closed
wants to merge 9 commits into from
Closed

8261401: Add sanity check for UseSHM large pages similar to the one used with hugetlb large pages #2488

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c092a28
Only UseSHM if sanity check pass
kstefanj Jan 29, 2021
a1b34aa
8261401-test-fixes
kstefanj Feb 9, 2021
1ae85f9
8261401-self-review
kstefanj Feb 9, 2021
376b223
8261401-check-effective
kstefanj Feb 10, 2021
bf6cbce
Thomas review
kstefanj Feb 10, 2021
6a2606b
Only warn if UseLargePages was explicitly set.
kstefanj Feb 11, 2021
681dc92
Merge branch 'master' into 8261401-shm-sanity
kstefanj Feb 15, 2021
a9d8c0f
Clean up test after merge
kstefanj Feb 15, 2021
8354e74
Thomas review - Updated comments
kstefanj Feb 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 32 additions & 1 deletion src/hotspot/os/linux/os_linux.cpp
View file
Open in desktop
Expand Up @@ -3564,6 +3564,28 @@ bool os::Linux::hugetlbfs_sanity_check(bool warn, size_t page_size) {
return result;
}

bool os::Linux::shm_hugetlbfs_sanity_check(bool warn, size_t page_size) {
// Try to create a large shared memory segment.
int shmid = shmget(IPC_PRIVATE, page_size, SHM_HUGETLB|IPC_CREAT|SHM_R|SHM_W);
if (shmid == -1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the flags should contain the SHM_HUGE_* flags (considering large pages) similar to hugetlbfs for proper checking to avoid the failure like in JDK-8261636 reported just recently for the same reason.

According to the man pages] such flags exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would agree if the later usage of shmget() would make use of those flags but it don't. We might want o add a CR for enabling use of other page sizes than the default large page size for SHM as well. But there is also discussions about trying to remove this SHM support.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am good with creating a CR for that. We can always close it later.

// Possible reasons for shmget failure:
// 1. shmmax is too small for the request.
// > check shmmax value: cat /proc/sys/kernel/shmmax
// > increase shmmax value: echo "0xffffffff" > /proc/sys/kernel/shmmax
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd avoid using a 32 bit constant on potentially 64 bit systems. According to that same man page, the limit on 64 bits is 127 TB which is more than the suggested 4 GB :)

The man page talks about ULONG_MAX - 2^24 as default (for 64 bit systems?).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is a copy of the old one where we actually use shmget(), but I agree that it could use an update. Not even sure we have to suggest a specific value, something like:

Suggested change
// > increase shmmax value: echo "0xffffffff" > /proc/sys/kernel/shmmax
// > increase shmmax value: echo "new value" > /proc/sys/kernel/shmmax

Might be good enough.

I will also update the original comment.

// 2. not enough large page memory.
// > check available large pages: cat /proc/meminfo
// > increase amount of large pages:
// echo new_value > /proc/sys/vm/nr_hugepages
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just my opinion, but I would prefer to refer to some generic documentation about these options (and use the "new" locations in /sys/kernel/mm/hugepages/hugepages-*/ instead of the "legacy" /proc/sys, e.g. https://lwn.net/Articles/376606/ - yes this is not official documentation, but the best I could find on the spot)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, the documentation I usually refer to is:
https://www.kernel.org/doc/Documentation/vm/hugetlbpage.txt

Since our SHM-support currently only handles the default large page size I think we should suggest to use:
sysctl -w vm.nr_hugepages=new_value

Suggested change
// echo new_value > /proc/sys/vm/nr_hugepages
// sysctl -w vm.nr_hugepages=new_value
// > For more information regarding large pages please refer to:
// https://www.kernel.org/doc/Documentation/vm/hugetlbpage.txt

if (warn) {
warning("Large pages using UseSHM are not configured on this system.");
}
return false;
}
// Managed to create a segment, now delete it.
shmctl(shmid, IPC_RMID, NULL);
return true;
}

// From the coredump_filter documentation:
//
// - (bit 0) anonymous private memory
Expand Down Expand Up @@ -3748,7 +3770,16 @@ bool os::Linux::setup_large_page_type(size_t page_size) {
UseHugeTLBFS = false;
}

return UseSHM;
if (UseSHM) {
bool warn_on_failure = !FLAG_IS_DEFAULT(UseSHM);
if (shm_hugetlbfs_sanity_check(warn_on_failure, page_size)) {
return true;
}
UseSHM = false;
}

log_warning(pagesize)("UseLargePages disabled, no large pages configured and available on the system.");
kstefanj marked this conversation as resolved.
Show resolved Hide resolved
return false;
}

void os::large_page_init() {
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/os/linux/os_linux.hpp
View file
Open in desktop
Expand Up @@ -85,6 +85,7 @@ class Linux {
static bool setup_large_page_type(size_t page_size);
static bool transparent_huge_pages_sanity_check(bool warn, size_t pages_size);
static bool hugetlbfs_sanity_check(bool warn, size_t page_size);
static bool shm_hugetlbfs_sanity_check(bool warn, size_t page_size);

static char* reserve_memory_special_shm(size_t bytes, size_t alignment, char* req_addr, bool exec);
static char* reserve_memory_special_huge_tlbfs(size_t bytes, size_t alignment, char* req_addr, bool exec);
Expand Down
17 changes: 14 additions & 3 deletions test/hotspot/jtreg/gc/g1/TestLargePageUseForAuxMemory.java
View file
Open in desktop
Expand Up @@ -64,6 +64,11 @@ static void checkSize(OutputAnalyzer output, long expectedSize, String pattern)
}
}

static boolean checkLargePagesEnabled(OutputAnalyzer output) {
String lp = output.firstMatch("Large Page Support: (\\w*)", 1);
return lp != null && lp.equals("Enabled");
}

static void checkSmallTables(OutputAnalyzer output, long expectedPageSize) throws Exception {
checkSize(output, expectedPageSize, "Block Offset Table: .*page_size=([^ ]+)");
checkSize(output, expectedPageSize, "Card Counts Table: .*page_size=([^ ]+)");
Expand All @@ -82,15 +87,21 @@ static void testVM(String what, long heapsize, boolean cardsShouldUseLargePages,
pb = ProcessTools.createJavaProcessBuilder("-XX:+UseG1GC",
"-XX:G1HeapRegionSize=" + HEAP_REGION_SIZE,
"-Xmx" + heapsize,
"-Xlog:pagesize",
"-Xlog:gc+init,pagesize",
"-XX:+UseLargePages",
"-XX:+IgnoreUnrecognizedVMOptions", // there is no ObjectAlignmentInBytes in 32 bit builds
"-XX:ObjectAlignmentInBytes=8",
"-version");

OutputAnalyzer output = new OutputAnalyzer(pb.start());
checkSmallTables(output, (cardsShouldUseLargePages ? largePageSize : smallPageSize));
checkBitmaps(output, (bitmapShouldUseLargePages ? largePageSize : smallPageSize));
// Make sure large pages are really enabled.
if (checkLargePagesEnabled(output)) {
checkSmallTables(output, (cardsShouldUseLargePages ? largePageSize : smallPageSize));
checkBitmaps(output, (bitmapShouldUseLargePages ? largePageSize : smallPageSize));
} else {
checkSmallTables(output, smallPageSize);
checkBitmaps(output, smallPageSize);
}
output.shouldHaveExitValue(0);

// Test with large page disabled.
Expand Down
9 changes: 7 additions & 2 deletions test/hotspot/jtreg/gc/g1/TestLargePageUseForHeap.java
View file
Open in desktop
Expand Up @@ -61,6 +61,11 @@ static void checkSize(OutputAnalyzer output, long expectedSize, String pattern)
}

static boolean checkLargePageEnabled(OutputAnalyzer output) {
String lp = output.firstMatch("Large Page Support: (\\w*)", 1);
// Make sure large pages really are enabled.
if (lp == null || lp.equals("Disabled")) {
return false;
}
// This message is printed when tried to reserve a memory with large page but it failed.
String errorStr = "Reserve regular memory without large pages";
String heapPattern = ".*Heap: ";
Expand All @@ -84,7 +89,7 @@ static void testVM(long regionSize) throws Exception {
pb = ProcessTools.createJavaProcessBuilder("-XX:+UseG1GC",
"-XX:G1HeapRegionSize=" + regionSize,
"-Xmx128m",
"-Xlog:pagesize,gc+heap+coops=debug",
"-Xlog:gc+init,pagesize,gc+heap+coops=debug",
"-XX:+UseLargePages",
"-version");

Expand All @@ -97,7 +102,7 @@ static void testVM(long regionSize) throws Exception {
pb = ProcessTools.createJavaProcessBuilder("-XX:+UseG1GC",
"-XX:G1HeapRegionSize=" + regionSize,
"-Xmx128m",
"-Xlog:pagesize,gc+heap+coops=debug",
"-Xlog:gc+init,pagesize,gc+heap+coops=debug",
"-XX:-UseLargePages",
"-version");

Expand Down