RFR(M) 8150353: PPC64LE: Support RTM on linux
Thomas Stüfe
thomas.stuefe at gmail.com
Thu Feb 25 10:57:25 UTC 2016
Hi Gustavo,
I can host this for you, if you have no access to cr.openjdk.java.net. Just
send me the patch file.
About your change:
src/cpu/ppc/vm/globalDefinitions_ppc.hpp
small nit: Could be probably be merged with the paragraph above where we do
the same thing for AIX, but I do not have strong emotions.
src/cpu/ppc/vm/vm_version_ppc.cpp
+ // Please, refer to commit 4b4fadba057c1af7689fc8fa182b13baL7
Does this refer to an OpenJDK change? If yes, could you please instead
mention the OpenJDK bug number instead?
src/os/linux/vm/os_linux.cpp
os::Linux::initialize_os_info()
Please make this code more robust:
- Check the return value of sscanf (must be 3, otherwise your assumption
about the version format was wrong)
- Could this happen: "3.2" ? If yes, could you please handle it too?
- Please handle overflow - if any one of minor/fix is > 256, something
sensible should happen (for major, this probably indicates an error).
Possibly cap out at 256?
If version cannot be read successfully, the VM should not abort imho but
behave gracefully. Worst that should happen is that RTM gets deactivated.
Kind Regards, Thomas
On Thu, Feb 25, 2016 at 9:21 AM, David Holmes <david.holmes at oracle.com>
wrote:
> On 25/02/2016 6:11 PM, Gustavo Romero wrote:
>
>> Hi David,
>>
>> OK, I'll fix that.
>>
>> Should I re-send the RFR with the right URL and abandon this one?
>>
>
> I think you can just post the new URL to this one.
>
> In case a contribution is not so small, even so is it fine to include it
>> inline?
>>
>
> Well it's preferable, for larger contributions, to find someone to host
> the webrev for you.
>
> Cheers,
> David
>
>
> Thank you.
>>
>> Regards,
>> Gustavo
>>
>> On 25-02-2016 04:54, David Holmes wrote:
>>
>>> Hi Gustavo,
>>>
>>> Just a point of order. All contributions to the OpenJDK must be made
>>> through OpenJDK infrastructure. So you must either get someone to host your
>>> webrev on cr.openjdk.java.net, or include it inline in
>>> email (attachments tend to get stripped).
>>>
>>> Sorry for the inconvenience.
>>>
>>> David
>>>
>>> On 25/02/2016 5:50 AM, Gustavo Romero wrote:
>>>
>>>> Hi Martin,
>>>>
>>>> Both little and big endian Linux kernel contain the syscall change, so
>>>> I did not include:
>>>>
>>>> #if defined(COMPILER2) && (defined(AIX) || defined(VM_LITTLE_ENDIAN)
>>>>
>>>> in globalDefinitions_ppc.hpp.
>>>>
>>>> Please, could you review the following change?
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8150353
>>>> Webrev (hotspot): http://81.de.7a9f.ip4.static.sl-reverse.com/webrev/
>>>>
>>>> Summary:
>>>>
>>>> * Enable RTM support for Linux on PPC64 (LE and BE).
>>>> * Fix C2 compiler buffer size issue.
>>>>
>>>> Thank you.
>>>>
>>>> Regards,
>>>> Gustavo
>>>>
>>>>
>>>
>>
More information about the hotspot-dev
mailing list