- ppc-aix-port-dev - openjdk.org

RFR(M): 8204524: Unnecessary memory barriers in G1ParScanThreadState::copy_to_survivor_space
by Michihiro Horie 07 Jun '18

07 Jun '18

Dear all, Would you please review the following change? Bug: https://bugs.openjdk.java.net/browse/JDK-8204524 Webrev: http://cr.openjdk.java.net/~mhorie/8204524/webrev.00 G1ParScanThreadState::copy_to_survivor_space tries to move live objects to a different location. It uses a forwarding technique and allows multiple threads to compete for performing the copy step. A copy is performed after a thread succeeds in the CAS. CAS-failed threads are not allowed to dereference the forwardee concurrently. Current code is already written so that CAS-failed threads do not dereference the forwardee. Also, this constraint is documented in a caller function mark_forwarded_object as “the object might be in the process of being copied by another worker so we cannot trust that its to-space image is well-formed”. There is no copy that must finish before the CAS. Threads that failed in the CAS must not dereference the forwardee. Therefore, no fence is necessary before and after the CAS. I measured SPECjbb2015 with this change. As a result, critical-jOPS performance improved by 27% on POWER8. Best regards, -- Michihiro, IBM Research - Tokyo

1 0

RFR(M): 8154736: enhancement of cmpxchg and copy_to_survivor for ppc64
by Michihiro Horie 05 Jun '18

05 Jun '18

Dear all, I would like to ask reviews on 8154736 “enhancement of cmpxchg and copy_to_survivor”. The change adds options to avoid expensive syncs with compare-and-exchange. An experiment by using SPECjbb2015 showed 6% improvement in critical-jOPS. This change focuses on ppc64 but would be potentially beneficial for aarch64. Although discussions stopped at http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2016-October/002718… , I would like to restart the review by taking over Hiroshi's work if the discussion is still open. Bug: https://bugs.openjdk.java.net/browse/JDK-8154736 Webrev: http://cr.openjdk.java.net/~mhorie/8154736/webrev.08/ Previous review had discussions on improper log output ( http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2016-September/0026… ). Logs can be generated properly with this change, but I would like to ask if we should use “if(log) OrderAccess:acquire()” as is in webrev or more general approach with a call to OrderAccess:consume() with empty implementation on all supported platforms. Also, there were discussions on the problem of unawareness of copied obj ( http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2016-October/002696… ). This change adds “release” in cmpxchg_pre_membar. This was discussed in http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2016-October/002698… . I measured SPECjbb2015 with its multi JVMs mode on a POWER8 node (for JDK11 , I modified MANIFEST in specjbb2015.jar to specify locations of JAXB related libraries). As a result, critical-jOPS improved by 6% due to this change. Best regards, -- Michihiro, IBM Research - Tokyo

7 33

Re: RFR 8197387: jcmd started by "root" must be allowed to access all VM processes
by Daniil Titov 31 May '18

31 May '18

Hi Thomas, Thank you for the review. Just in in case I put an updated webrev with suggested changes at http://cr.openjdk.java.net/~dtitov/8197387/webrev.03/ . I am also including ppc-aix-port-dev(a)openjdk.java.net mail list since the changes affect AIX native code. I did these AIX changes myself and I need to get them verified before the push. May I ask someone who has access to an AIX machine try this patch to ensure that AIX build is fine? Webrev: http://cr.openjdk.java.net/~dtitov/8197387/webrev.03/ Issue: https://bugs.openjdk.java.net/browse/JDK-8197387 Thanks a lot! Best regards, Daniil On 5/29/18, 9:54 PM, "Thomas Stüfe" <thomas.stuefe(a)gmail.com> wrote: Hi Daniil, Looks fine. Small nits: - please add short comments to os_posix.hpp on the new prototypes. At least on matches_effective_uid_and_gid_or_root: e.g. // Returns true if either given uid is effective uid and given gid is effective gid, or if given uid is root. - src/hotspot/os/posix/os_posix.cpp: +bool os::Posix::matches_effective_uid_and_gid_or_root(uid_t uid, gid_t gid) { + return is_root(uid) || ( geteuid() == uid && getegid() == gid); +} + please remove extra space before geteuid() If you change above points, I do not need another webrev. Reviewed. -- Btw: I wonder why we find it necessary in the hotspot to check for both uid AND gid to match effective uid and effective gid? Seems strange. But since this logic is not touched by your change, your change is okay. Best Regards, Thomas On Tue, May 29, 2018 at 11:33 PM, Daniil Titov <daniil.x.titov(a)oracle.com> wrote: > Hi Thomas, > > Please review a new version of the fix that includes the changes suggested. > > Webrev: http://cr.openjdk.java.net/~dtitov/8197387/webrev.02/ > Bug: https://bugs.openjdk.java.net/browse/JDK-8197387 > > Thank you, > Daniil > > > On 5/24/18, 10:51 PM, "Thomas Stüfe" <thomas.stuefe(a)gmail.com> wrote: > > Hi Daniil, > > here is my review: > > - Like Roger I would prefer to have the uid checks factored out. At > least for the hotspot coding, I do not know where to put it in jdk > coding. For the hotspot parts, I would add something like: > > os::Posix::is_root(uid_t uid) ; > os::Posix::matches_effective_uid_or_root(uid_t uid) // return > isroot(uid) || uid == geteuid > os::Posix::matches_effective_group_id(gid_t gid) // return gid == getegid > > to os_posix.hpp/os_posix.cpp > > Other than that, the changes make sense. > > Kind Regards, Thomas > > > > > On Thu, May 24, 2018 at 3:11 AM, Daniil Titov <daniil.x.titov(a)oracle.com> wrote: > > Please review the changes that fix JDK-8197387. > > > > There are 2 problems here: > > 1. JVM ignores .attach_pid<pid> file if it is owned by the user different from the one that owns this JVM process > > 2. jcmd checks that .java_pid<pid> socket is owned by the same user that runs jcmd and reports an error otherwise > > > > The fix relaxes these checks to allow jcmd started by "root" (UID = 0) access JVMs started by another users. > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8197387 > > Webrev: http://cr.openjdk.java.net/~dtitov/8197387/webrev.01/ > > > > Best regards, > > Daniil > > > > > > >

3 5

RFR(S): 8203669: PPC64: Fix jtreg RTM tests after "8203305: Improve TM detection for enabling RTM on Linux / POWER9"
by Gustavo Romero 30 May '18

30 May '18

Hi, Could the following small fix be reviewed please? It touches shared code. bug: https://bugs.openjdk.java.net/browse/JDK-8203669 webrev: http://cr.openjdk.java.net/~gromero/POWER9/8203669 Since change "8203305: Improve TM detection for enabling RTM on Linux / POWER9" JVM on PPC64 is not advertising anymore the feature "tcheck" which indicated that RTM was supported by the JVM in case both CPU and OS supported hardware transactional memory. It happens that several tests under /test/hotspot/jtreg/compiler/rtm rely on the JVM to advertise if RTM is supported by the JVM on a given CPU/OS and so they were failing to be skipped (or not) on different PPC64/Linux combinations. That change fixes it by re-introducing a feature to inform that RTM is supported by the JVM when appropriated (given valid OS/CPU support) and now it does it by advertising that feature as on x86_64, reporting the feature as "rtm". It also incorporates a check for when OS does not support RTM but CPU does in test: TestUseRTMLockingOptionOnUnsupportedCPU.java. Just on PPC64 it might be the case that although CPU supports RTM (HTM) instructions the OS version might not support it, like on old kernels that do not abort transactions on syscalls. JVM correctly reports each case so the test was adapted to test them. That change also corrects a couple of nit typos elsewhere not related to "8203305". Thank you. Best regards, Gustavo

3 9

POWER9: Is there a way to improve the random number generation on PPC64?
by Gustavo Romero 29 May '18

29 May '18

Hi, POWER9 processors introduced a new single instruction to generate a random number called 'darn' (Deliver A Random Number) [1, 2]. The random number generator behind this instruction is NIST SP800-90B and SP800-90C compliant and provides a minimum of 0.5 bits of entropy per bit. That instruction is as simple as "darn RT, L", where RT is general 64-bit purpose register and L is a 2-bit operand to select the random number format. One can call 'darn' many times to obtain a new random number each time. Initially I think it can help on the improving (throughput) of SecureRandom.generateSeed() method & friends from JCE (NativePRNG provider). If that holds, so it has to be done both for Interpreter and JIT. Currently generateSeed() from NativePRNG basically reads from /dev/random by default (which blocks from time to time) or /dev/urandom if instructed to do so. Could somebody please help me to figure out the appropriate place to exploit such a P9 instruction for interpreted mode, given that code for generateSeed() is pure Java and behind scenes just opens /dev/random file and reads from it? For instance, is it correct to exploit it on a C/C++ code and attach that by means of a JNI? Finally, for JITed mode, I think that a way to exploit such a feature would be by matching an specific sub-tree in Ideal Graph and from that emit a `darn` instruction, however I could not figure one sound sub-tree with known nodes (AddI, LoadN, Parm, etc) that could be matched for that purpose. How do porters usually proceed in this case? Any comments shedding some light on that is much appreciated. Thanks and best regards, Gustavo [1] https://www.docdroid.net/tWT7hjD/powerisa-v30.pdf, p. 79 [2] https://openpowerfoundation.org/?resource_lib=power-isa-version-3-0

3 21

RFR(XS): 8203305: PPC64: Improve TM detection for enabling RTM on Linux / POWER9
by Gustavo Romero 17 May '18

17 May '18

Hi, Could the following small change be reviewed please? It improves the TM detection for enabling (or not) RTM on Linux. With that change JVM uses TM capability flags advertised by the kernel instead of parsing major/minor kernel version. That way is better to address TM on POWER9 which now supports a new TM mode called "TM with no Suspend Mode" and that is not mature yet (so I'm disabling it for now). The new mode is just available on POWER9 DD2.1 NV (baremetal) and so TM on POWER9 VMs behave as on POWER8 hence RTM is enabled on POWER9 VM only. Nothing changes regarding AIX. bug: https://bugs.openjdk.java.net/browse/JDK-8203305 webrev: http://cr.openjdk.java.net/~gromero/POWER9/8203305/v1 Thank you. Best regards, Gustavo

2 2

RFR(M) 8188165: PPC64: Optimize Unsafe.copyMemory and arraycopy
by Matthew Brandyberry 16 May '18

16 May '18

This is specific to PPC64LE only. The emphasis in the proposed code is on minimizing branches. Thus, this code makes no attempt to avoid misaligned accesses and each block is designed to copy as many elements as possible. As one data point, this yields as much as a 13x improvement in jbyte_disjoint_arraycopy for certain misaligned scenarios. Bug: https://bugs.openjdk.java.net/browse/JDK-8188165 Webrev: http://cr.openjdk.java.net/~mbrandy/8188165/jdk10/v1/ Thanks, -Matt

2 2

RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1
by Baesken, Matthias 16 May '18

16 May '18

Hello , could you please review this small adjustment to the symbol visibility compilation settings on AIX ? Currently we use XLC 12.1 to compile JDK on AIX . However XLC 12.1 does not support the "-qvisibility=hidden" setting currently set on AIX. It was introduced with XLC 13.1 . Christoph found some info about it here : https://www.ibm.com/developerworks/aix/library/au-aix-symbol-visibility-par… Setting it only generates hundreds of warnings in the build log , warnings look like this : XlC12.1 bash-4.4$ xlC -qversion IBM XL C/C++ for AIX, V12.1 (5765-J02, 5725-C72) Version: 12.01.0000.0019 bash-4.4$ xlC -qvisibility=hidden sizeof.c -o sizeof_aixxlc 1506-173 (W) Option visibility=hidden is not valid. Enter xlC for list of valid options. Compare to XLC13.1 bash-3.00$ xlC -qversion IBM XL C/C++ for AIX, V13.1 (5725-C72, 5765-J07) Version: 13.01.0000.0008 bash-3.00$ xlC -qvisibility=default sizeof.c -o sizeof_aixxlc bash-3.00$ xlC -qvisibility=hidden sizeof.c -o sizeof_aixxlc So it is better to avoid setting these flags when using xlc12.1 . Please review : Bug : https://bugs.openjdk.java.net/browse/JDK-8202322 Change : http://cr.openjdk.java.net/~mbaesken/webrevs/8202322/ Best regards, Matthias

6 17

RFR (S): 8202650: Enforce group for attach listener file
by Langer, Christoph 09 May '18

09 May '18

Hi, please review a change for correctly setting the group for the attach listener file: Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8202650.0/ Bug: https://bugs.openjdk.java.net/browse/JDK-8202650 The attach listener file, usually /tmp/.java_pid<pid> is created from the hotspot JVM process. Usually it will belong to the process user and group. However, when the directory where it is created has set the s-bit for groups, the group of the directory is taken. This will cause errors when the attach client tries to connect and it is checked whether the group of the attach file matches the client processes' group. In my webrev I only implemented the change for AIX because we have run into an issue on that platform. But I can see this code already in place for attachListener_bsd.cpp. And I'm wondering if this should also be added to attachListener_linux.cpp because the sticky-bit could be set with the same effects on Linux, too. Any opinions about that? Thanks and best regards Christoph

5 11

RFR: 8202435: [aix] print program break as part of memory info into hs-err file
by Thomas Stüfe 04 May '18

04 May '18

Hi all, may I please have reviews for this tiny, AIX-specific enhancement. bug: https://bugs.openjdk.java.net/browse/JDK-8202435 webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8202435-aix-print-program-break/… It prints out program break information as part of os::print_memory_info().

3 4