[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: [CAUTION] RE: RFR(xxs): 8241395: Factor out platform independent code for os::xxx_memory_special
From:       "Schmidt, Lutz" <lutz.schmidt () sap ! com>
Date:       2020-03-24 19:52:09
Message-ID: 4F2864B5-80DE-497D-9276-467D46A8ACFA () sap ! com
[Download RAW message or body]

Perfect.
Thank you, Thomas.
Lutz

From: Thomas Stüfe <thomas.stuefe@gmail.com>
Date: Tuesday, 24. March 2020 at 18:33
To: Lutz Schmidt <lutz.schmidt@sap.com>
Cc: "Doerr, Martin (martin.doerr@sap.com)" <martin.doerr@sap.com>, "Baesken, \
Matthias" <matthias.baesken@sap.com>, "hotspot-runtime-dev@openjdk.java.net" \
                <hotspot-runtime-dev@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@sap.com<mailto:lutz.schmidt@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@openjdk.java.net<mailto:hotspot-runtime-dev-bounces@openjdk.java.net> \
on behalf of martin.doerr@sap.com<mailto:martin.doerr@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@openjdk.java.net<mailto:bounces@openjdk.java.net>> On Behalf Of \
Baesken, Matthias  > Sent: Dienstag, 24. März 2020 16:25
    > To: Thomas Stüfe <thomas.stuefe@gmail.com<mailto:thomas.stuefe@gmail.com>>; \
hotspot-runtime-  > dev@openjdk.java.net<mailto:dev@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@gmail.com<mailto:thomas.stuefe@gmail.com><mailto:thomas.stuefe@gmail.com<mailto:thomas.stuefe@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@openjdk.java.net<mailto:dev@openjdk.java.net><mailto:hotspot-runtime-dev@openjdk.java.net<mailto:hotspot-runtime-dev@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


[prev in list] [next in list] [prev in thread] [next in thread]