RFR: JDK-8157141 & JDK-8166454: Solaris getisax(2) and meminfo(2) cleanup

[David Holmes]

Sorry I retract my suggestion - getisax can indeed return 0.

David

On 29/09/2016 2:43 PM, David Holmes wrote:
> On 29/09/2016 9:15 AM, Kim Barrett wrote:
>>> On Sep 27, 2016, at 9:47 AM, Alan Burlison <Alan.Burlison at oracle.com>
>>> wrote:
>>>
>>> On 27/09/2016 00:54, David Holmes wrote:
>>>
>>>> Thanks Alan. It is the creation of the intermediate log-stream that can
>>>> not be accomplished at this stage of VM initialization so we will need
>>>> to stick with what you had. Thanks for trying.
>>>
>>> Done. I've tweaked the output slightly, it is now:
>>>
>>> torside-kz6$ ./java -Xlog:os+cpu -version
>>> [0.177s][info][os,cpu] getisax(2) returned 1 words:
>>> [0.177s][info][os,cpu]     word 1: 0x3ffe8df0
>>>
>>> That's on a M5, on a T7 and similar it will be 2 words. I've respun
>>> the webrev, for reference here are the review links again:
>>>
>>> getisainfo() manpage:
>>> http://docs.oracle.com/cd/E53394_01/html/E54765/getisax-2.html
>>>
>>> meminfo() manpage:
>>> https://docs.oracle.com/cd/E53394_01/html/E54765/meminfo-2.html
>>>
>>> Bugs:   https://bugs.openjdk.java.net/browse/JDK-8157141
>>>        https://bugs.openjdk.java.net/browse/JDK-8166454
>>> Webrev: http://cr.openjdk.java.net/~alanbur/JDK-8157141%2bJDK-8166454/
>>
>> ------------------------------------------------------------------------------
>>
>> src/os_cpu/solaris_sparc/vm/vm_version_solaris_sparc.cpp
>>  377   uint_t av1 = avs[AV_HW1_IDX];
>>  378   if (av1 & AV_SPARC_MUL32)        features |= hardware_mul32_m;
>> ...
>>
>> Good to use the symbolic index AV_HW1_IDX. Bad form to assume we have
>> that many entries.  That should be
>>
>>   if (avn > AV_HW1_IDX) {
>>     uint_t av1 = avs[AV_HW1_IDX];
>>     if (av1 & AV_SPARC_MUL32)        features |= hardware_mul32_m;
>>     ...
>>   }
>
> Surely we can just assert avn > 0 after:
>
>  uint_t avn = getisax(NULL, 0);
>
> It should never be able to return anything < 1 regardless.
>
> David
> -----
>
>> ------------------------------------------------------------------------------
>>
>> src/os_cpu/solaris_sparc/vm/vm_version_solaris_sparc.cpp
>>  368   uint_t avn = getisax(NULL, 0);
>>  369   uint_t *avs = (uint_t*) alloca(avn * sizeof(uint_t));
>>  370   (void) getisax(avs, avn);
>>
>> I still think the use of alloca here is overkill, along with depending
>> on (currently) undocumented behavior of getisax, but this is called
>> once initialization code and you've filed the doc bug, so okay.
>>
>> ------------------------------------------------------------------------------
>>
>>
>> I don't need a new webrev for the insertion of the avn check.
>>
>>> A JPRT hotspot testset run is clean.
>>
>> Have you used logging to hand-verify the expected features are being set?
>>