[CAUTION] RE: RFR(xxs): 8241395: Factor out platform independent code for os::xxx_memory_special()

[Schmidt, Lutz]

Perfect.
Thank you, Thomas.
Lutz

From: Thomas Stüfe <thomas.stuefe at gmail.com>
Date: Tuesday, 24. March 2020 at 18:33
To: Lutz Schmidt <lutz.schmidt at sap.com>
Cc: "Doerr, Martin (martin.doerr at sap.com)" <martin.doerr at sap.com>, "Baesken, Matthias" <matthias.baesken at sap.com>, "hotspot-runtime-dev at openjdk.java.net" <hotspot-runtime-dev at openjdk.java.net>
Subject: Re: [CAUTION] RE: RFR(xxs): 8241395: Factor out platform independent code for os::xxx_memory_special()

Hi Lutz,

no special reason for the AIX case, I think our code there predated the other versions. I'll unify the messages as you suggested.

Thanks for the review.

..Thomas

On Tue, Mar 24, 2020 at 5:03 PM Schmidt, Lutz <lutz.schmidt at sap.com<mailto:lutz.schmidt at sap.com>> wrote:
Hi Thomas,

looks good to me in general.

Is there a reason why in os_aix.cpp there is an assert(false) in pd_reserve* and an unimplemented() in pd_release*? For the other OSes, there is a fatal(), if applicable.

In addition, I'd like to see the same failure text in either case.

No new webrev required.

Thanks,
Lutz

On 24.03.20, 16:31, "hotspot-runtime-dev on behalf of Doerr, Martin" <hotspot-runtime-dev-bounces at openjdk.java.net<mailto:hotspot-runtime-dev-bounces at openjdk.java.net> on behalf of martin.doerr at sap.com<mailto:martin.doerr at sap.com>> wrote:

    Hi Thomas,

    looks good to me, too.

    In addition to the copyright, please improve indentation for the 2nd argument line in os_linux.cpp and os_windows.cpp:
    +char* os::pd_reserve_memory_special(size_t bytes, size_t alignment,
                                      char* req_addr, bool exec) {

    and remove extra newline in os.cpp:
    +   return result;
    +
    + }

    No need for a new webrev.

    Best regards,
    Martin


    > -----Original Message-----
    > From: hotspot-runtime-dev <hotspot-runtime-dev-
    > bounces at openjdk.java.net<mailto:bounces at openjdk.java.net>> On Behalf Of Baesken, Matthias
    > Sent: Dienstag, 24. März 2020 16:25
    > To: Thomas Stüfe <thomas.stuefe at gmail.com<mailto:thomas.stuefe at gmail.com>>; hotspot-runtime-
    > dev at openjdk.java.net<mailto:dev at openjdk.java.net>
    > Subject: RE: RFR(xxs): 8241395: Factor out platform independent
    > code for os::xxx_memory_special()
    >
    > Hi Thomas, looks good to me.
    > But you might want to change the copyright year in
    >
    > http://cr.openjdk.java.net/~stuefe/webrevs/8241395-refactor-reserve-
    > memory-
    > special/webrev.00/webrev/src/hotspot/share/runtime/os.hpp.frames.html
    >
    > Thanks, Matthias
    >
    > ---------- Forwarded message ---------
    > From: Thomas Stüfe
    > <thomas.stuefe at gmail.com<mailto:thomas.stuefe at gmail.com><mailto:thomas.stuefe at gmail.com<mailto:thomas.stuefe at gmail.com>>>
    > Date: Sat, Mar 21, 2020 at 1:40 PM
    > Subject: RFR(xxs): 8241395: Factor out platform independent code for
    > os::xxx_memory_special()
    > To: Hotspot dev runtime <hotspot-runtime-
    > dev at openjdk.java.net<mailto:dev at openjdk.java.net><mailto:hotspot-runtime-dev at openjdk.java.net<mailto:hotspot-runtime-dev at openjdk.java.net>>>
    >
    > Hi,
    >
    > may I please have reviews for this little cleanup.
    >
    > A while ago we factored out platform independent behavior for
    > os::reserve_memory() and friends to os.cpp and relegated platform
    > dependent code to os::pd_reserve_memory() etc. This change does the
    > same for os::reserve_memory_special() and os::release_memory_special().
    >
    > Only Linux and Windows are affected since only they have meaningful
    > implementations for UseLargePages.
    >
    > It also adds a small comment about the NMT Tracker object.
    >
    > In addition it asserts - now for all platforms, it used to do this only on Linux -
    > that if both requested address and alignment are given to
    > os::reserve_memory_special() the requested address should at least be
    > aligned to the given alignment (the arguments should be mutually exclusive
    > and I plan to clean this up further).
    >
    > Issue: https://bugs.openjdk.java.net/browse/JDK-8241395
    > webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8241395-refactor-
    > reserve-memory-special/webrev.00/webrev/
    >
    > Thanks, Thomas