Request for review (XS): 8006431: os::Bsd::initialize_system_info() sets _physical_memory too large

[Staffan Larsen]

Still good!

/Staffan

On 18 jan 2013, at 13:47, Bengt Rutisson <bengt.rutisson at oracle.com> wrote:

> 
> 
> Hi Mikael,
> 
> Thanks for the review!
> 
> On 1/18/13 8:09 AM, Mikael Vidstedt wrote:
>> 
>> Looks good, assuming u_long is a 64-bit type.
> 
> Good point. It seems like u_long is a 64 bit value on OSX, but that's not guaranteed since it is just "unsigned long". I changed mem_val to be julong, which should always be 64 bit. (The instance variable _physical_memory is also a julong.)
> 
> So, now it is a two-word review request  :)
> 
> diff --git a/src/os/bsd/vm/os_bsd.cpp b/src/os/bsd/vm/os_bsd.cpp
> --- a/src/os/bsd/vm/os_bsd.cpp
> +++ b/src/os/bsd/vm/os_bsd.cpp
> @@ -243,7 +243,7 @@
>    int mib[2];
>    size_t len;
>    int cpu_val;
> -  u_long mem_val;
> +  julong mem_val;
>  
>    /* get processors count via hw.ncpus sysctl */
>    mib[0] = CTL_HW;
> @@ -260,7 +260,7 @@
>     * instead of hw.physmem because we need size of allocatable memory
>     */
>    mib[0] = CTL_HW;
> -  mib[1] = HW_USERMEM;
> +  mib[1] = HW_MEMSIZE;
>    len = sizeof(mem_val);
>    if (sysctl(mib, 2, &mem_val, &len, NULL, 0) != -1)
>         _physical_memory = mem_val;
> 
> 
> Thanks,
> Bengt
>  
> 
>> 
>> /Mikael
>> 
>> On 17 jan 2013, at 22:36, Bengt Rutisson <bengt.rutisson at oracle.com> wrote:
>> 
>>> 
>>> Hi all,
>>> 
>>> Could I have a couple of reviews for this small fix?
>>> 
>>> http://cr.openjdk.java.net/~brutisso/8006431/webrev.00/
>>> 
>>> On OSX we used HW_USERMEM value from sysctl() to get the available physical memory on the machine. This is a 32 bit value but we store it in a 64 bit variable. This means that we get kind of random and normally very large values for what we think the physical memory is.
>>> 
>>> We actually don't want a 32 bit value since we want to handle machines with more than 2 or 4 GB of memory. Instead of HW_USERMEM we should be using HW_MEMSIZE which will give us a 64 bit value.
>>> 
>>> See:
>>> https://developer.apple.com/library/mac/#documentation/Darwin/Reference/ManPages/man3/sysctl.3.html
>>> 
>>> Thanks Staffan Larsen for both detecting the problem and providing a solution.
>>> 
>>> This is a one-word-change. So, to save you a mouse click on the webrev link above, here is the diff:
>>> 
>>> --- a/src/os/bsd/vm/os_bsd.cpp
>>> +++ b/src/os/bsd/vm/os_bsd.cpp
>>> @@ -260,7 +260,7 @@
>>>     * instead of hw.physmem because we need size of allocatable memory
>>>     */
>>>    mib[0] = CTL_HW;
>>> -  mib[1] = HW_USERMEM;
>>> +  mib[1] = HW_MEMSIZE;
>>>    len = sizeof(mem_val);
>>>    if (sysctl(mib, 2, &mem_val, &len, NULL, 0) != -1)
>>>         _physical_memory = mem_val;
>>> 
>>> Thanks,
>>> Bengt
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130118/cb372d55/attachment.html