-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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.
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
JDK-8322943: runtime/CompressedOops/CompressedClassPointers.java fails on AIX #17708
Conversation
👋 Welcome back jkern! A progress list of the required criteria for merging this PR into |
Webrevs
|
At a first glance, this seems like a really big hammer for a really small and exotic nail. Can you explain what the new value "shm_allocation_granularity" is supposed to describe? And how it differs from vm_allocation_granularity? |
Hi, Thanks for fixing the randomization issue. |
I think there is confusion on several issues. Lets wait for Joachims explanation. |
While almost all platforms allow to mount shared memory (via |
Okay, thank you for confirming your intent. This is as I suspected from your initial change. First off, please rename the issue to make it obvious that this affects not just AIX. I suggest something like "Introduce os::vm_shm_allocation_granularity" or somesuch. But what you attempt to do already exists: this is exactly what And AIX (in shmget mode) is not the only platform with this restriction. We have that on Windows too, where we only can attach at multiples of what MS calls "Allocation Granularity" - I think 64KB. Note that the name is a misnomer, should more precisely be called "Virtual Memory Adress Attach Granuarity" since it does not affect the size of the allocation, only the attach point alignment. The problem in hotspot is that allocation granularity is often misunderstood to be a granularity that affects the reservation size as well. So people use it (even I, accidentally) to align up allocation sizes when it really only affects allocation address alignments. E.g. you can happily allocate a SystemV shm segment of one page (4K), but you will only be able to attach it to SHMLBA aligned addresses. Same on Windows. For a much more detailed explanation, please see https://bugs.openjdk.org/browse/JDK-8253683 - this mis-use of allocation_granularity is a long-standing bug in hotspot. Bottomline, I am against introducing yet another system value just because the one that is supposed to do that job is misused. I much rather see the misuse of allocation granularity cleaned up. |
BTW, just a zoomed-back reminder: all of this complexity is only because we don't use mmap on AIX for os::reserve_memory. We don't use it, because mmap cannot be used with 64K pages. Or can it? We wrote the code originally 20 (?) years ago when the only way to get 64K pages was via shmget. Maybe that changed. It would be good if someone could check if this is still the case. Because using shmget instead of mmap causes a long tail of follow-up problems. mmap is so much easier. Just a thought. Maybe we could throw away all the shmget handling nowadays, who knows. Cheers, Thomas |
Hi Thomas,
Well, that's not really true. As I understand, on AIX, there are both alignments (4k, 256M) at the same time, depending on whether shmat or mmap is used. And both are used. jdk/src/hotspot/os/aix/os_aix.cpp Line 2169 in 9a1b843
|
Yes, I know. Please read https://bugs.openjdk.org/browse/JDK-8253683 for details. I described the issue and the confusion surrounding allocation granularity. I also touched on using different granularities for different memory regions. Please also read the comment from today. We have two ways to deal with this: We can either implement a function that returns granularity as a function of memory region. That would work with mixed alignment requirements. However, that is really costly, invasive, complex, and really not needed because:
Therefore, I am for a simpler solution, which is a single value that is platform-dependent and never changes. And exactly that we have already: |
I wrote a little test programm
and ran this little program with envvar Is this sufficient to prove that mmap is still only using 4K? |
No clue. Maybe there is a different way. Some fancy madvise option? Ask IBM? Astonishing that they have not yet provided large page support for mmap after 20 years. |
I asked IBM. 64K Pages for mmap are available with AIX 7.3. I proved this. Unfortunately our current minimum release is AIX 7.2.5.7 |
Does that current minimum release need 64K pages? How many customers are that? Remember, you still can use the JVM with 4K pages (-XX:-Use64KPages). Its not that the VM is unusable then. |
Currently we have zero customers using SapMachine21, but a new product which customers are forced to install on all their APP-Servers will come soon. And this new product is based on SapMachine21. And it is already announced to SAP customers using AIX APP-servers that the minimum OS release for this product will be AIX 7.2.5 So, I would like to go the following way as sketched above: Going through the code and checking the right usage of os::vm_allocation_granularity() and replacing it by os::vm_page_size() if the usage is not the granularity of the allocation address. @tstuefe : Should I close this PR and open a new one for the new approach, or is it better to revert my previous changes and start again keeping this PR? |
@JoKern65 I would look for a simpler solution as a start. Fixing up usages of os::vm_allocation_granularity() will be a slog (Kudos to you though for being willing to take it on). Can we just fix the test in places for AIX? Note that I have never been a fan of this particular jtreg test anyway. It conflates two different things:
(A) is very dependent on the Operating System and somewhat random (ASLR). We also have now CompressedCPUSpecificClassSpaceReservation (which tests A) and CompressedClassPointersEncodingScheme (which tests B, but is not fleshed out yet). |
I don't think a local test fix makes sense. After all it is a real issue that os::attempt_reserve_memory_between() is using 4K alignment when we try to allocate 256M shmat memory. |
That is a good point. And a good compromise. @JoKern65 can you try this:
|
Yes, I will try that, but one question. Is the following code snipit an example for the incorrect use of vm_allocation_granularity or did I understand something wrong?
|
I think so, yes. |
Because this proposal of introducing a new method os::vm_shm_allocation_granularity() in the shared hotspot code was rejected and the alternative solution works by encapsulating the difference in ifdef AIX brackets instead, I close this PR and have opened a successor using the ifdef AIX bracket (PR 18105) |
Even after recent fixes like
https://bugs.openjdk.org/browse/JDK-8305765
the test runtime/CompressedOops/CompressedClassPointers.java fails on AIX.
This error results from the fact, that on AIX the shmat() allocation granularity is 256MB instead of the standard Pagesize (4KB or 64KB).
As a solution we introduce a new method
os::vm_shm_allocation_granularity()
, which on all platforms except AIX returns the same value asos::vm_allocation_granularity()
, but on AIX it returns (in the apropriate cases) 256MB.This new getter is used at all needed places instead of
os::vm_allocation_granularity()
.Progress
Issue
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/17708/head:pull/17708
$ git checkout pull/17708
Update a local copy of the PR:
$ git checkout pull/17708
$ git pull https://git.openjdk.org/jdk.git pull/17708/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 17708
View PR using the GUI difftool:
$ git pr show -t 17708
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/17708.diff
Webrev
Link to Webrev Comment