Request for review 7191817: -XX:+UseSerialGC -XX:+UseLargePages crashes with SIGFPE on Mac OS X

[David Holmes]

On 25/10/2012 10:42 PM, harold seigel wrote:
>
> I used _ALLBSD_SOURCE because that is the conditional that is used to
> disable the usage of Large Pages for MacOS X (in module os_bsd.cpp).

Okay - that agrees with other uses in shared code too (which is more 
relevant).

That said I think you can clean this up a little further by using the 
BSD_ONLY and NOT_BSD macros. That way the actual conditional is not exposed.

Thanks,
David

> Harold
>
> On 10/25/2012 3:14 AM, Staffan Larsen wrote:
>> On 25 okt 2012, at 05:39, David Holmes<David.Holmes at oracle.com> wrote:
>>
>>> Hi Harold,
>>>
>>> My only query is whether _ALLBSD_SOURCE is the right conditional for
>>> this? Exactly which platform(s) don't support this?
>>>
>>> The reason I ask is that even the BSD sources (eg
>>> ./os/bsd/vm/os_bsd.hpp) contain ifdefs for _ALL_BSD_SOURCE which
>>> indicates that some bsd builds do not in fact set _ALLBSD_SOURCE !
>>> Which I find extremely confusing.
>> I actually think this is just leftovers from the port and should be
>> removed. I think when the port to bsd was made, we started with the
>> linux code and then added these ifdefs to show what the "orginal" code
>> looked like while working on the new code. I may be wrong here and
>> someone maybe has a better explanation. The code that is ifdef:ed away
>> looks very similar to the linux code.
>>
>> I have a patch somewhere to remove this code, but I haven't gotten
>> around to finish it up.
>>
>> /Staffan
>>
>>> Thanks,
>>> David
>>>
>>> On 25/10/2012 12:22 AM, harold seigel wrote:
>>>> Please review this updated webrev:
>>>> http://cr.openjdk.java.net/~coleenp/bug_7191817_2/
>>>> <http://cr.openjdk.java.net/%7Ecoleenp/bug_7191817_2/>
>>>>
>>>> It incorporates Dave's suggestion to use the UNSUPPORTED_OPTION macro.
>>>>
>>>> Thanks, Harold
>>>>
>>>> On 10/24/2012 5:46 AM, David Holmes wrote:
>>>>> Hi Harold,
>>>>>
>>>>> We already have a macro for this:
>>>>>
>>>>> // Disable options not supported in this release, with a warning if
>>>>> they
>>>>> // were explicitly requested on the command-line
>>>>> #define UNSUPPORTED_OPTION(opt, description) \
>>>>> do { \
>>>>> if (opt) { \
>>>>> if (FLAG_IS_CMDLINE(opt)) { \
>>>>> warning(description " is disabled in this release."); \
>>>>> } \
>>>>> FLAG_SET_DEFAULT(opt, false); \
>>>>> } \
>>>>> } while(0)
>>>>>
>>>>> We use this on platforms were not all features are supported.
>>>>>
>>>>> David
>>>>>
>>>>> On 24/10/2012 5:04 AM, harold seigel wrote:
>>>>>> Summary: Support for the -XX:+UseLargePages flag causes crashes on
>>>>>> BSD
>>>>>> platforms such as MacOS X. This proposed change adds code to print
>>>>>> out a
>>>>>> message if -XX:+UseLargePages is specified on this platform and to
>>>>>> then
>>>>>> ignore the flag.
>>>>>>
>>>>>> Open webrev at http://cr.openjdk.java.net/~coleenp/bug_7191817
>>>>>> <http://cr.openjdk.java.net/%7Ecoleenp/bug_7191817>
>>>>>>
>>>>>> Bug link at http://bugs.sun.com/view_bug.do?bug_id=7191817
>>>>>>
>>>>>> This change was tested with JPRT and JCK tests. Also, tests were
>>>>>> run by
>>>>>> hand on a MacOS X machine specifying the -XX:+UseLargePages flag
>>>>>> to make
>>>>>> sure that the message was output and no crashes occurred.
>>>>>>
>>>>>> Thanks, Harold