RFR(XS) 8191369: NMT: Enhance thread stack tracking

Zhengyu Gu zgu at redhat.com
Mon Nov 20 18:46:32 UTC 2017


Hi Andrew,

Thanks for the review.

>> I'm not sure about the logic of the test when you hit the the
>> ENOMEM/uncommited case.
>>
>> +    mincore_return_value = mincore(test_addr, page_sz, vec);
>> +
>> +    if (mincore_return_value == -1 || (committed_only && (vec[0] &
>> 0x01) == 0)) {
>> +      // Page is not mapped/committed go up
>> +      // to find first mapped/committed page
>> +      if (errno != EAGAIN || (committed_only && (vec[0] & 0x01) == 0)) {
>> +        assert(mincore_return_value != -1 || errno == ENOMEM,
>> "Unexpected mincore errno");
>>
>> Should that not be
>>
>>    +      if ((mincore_return_value != -1 && errno != EAGAIN) ||
>> (committed_only && (vec[0] & 0x01) == 0)) {
> 
> Sorry, that correction should have been:
> 
>    +      if ((mincore_return_value == -1 && errno != EAGAIN) ||
>    (committed_only && (vec[0] & 0x01) == 0)) {

Thanks for pointing out. Updated accordingly:

Webrev: http://cr.openjdk.java.net/~zgu/8191369/webrev.02/

-Zhengyu

> 
>> A successful call to mincore is not guaranteed to reset errno and it
>> might by chance already have value EAGAIN before the call to mincore. If
>> we happened to hit a mapped, committed page first time then the vec bit
>> will be 1 and the code will loop and retest the same address. Unlikely
>> but possible?
>>
>>
>> The adjustment to the edge case logic now looks correct.
>>
>>> The test case for verifying fixes for above two issues can be found
>>> here: http://cr.openjdk.java.net/~zgu/8191369/test_mmap.c
>>
>> Yes, that verifies the edge case handling e.g. I ran it with 256 pages
>> and with 127/8/9 mapped and it gave the right results.
>>
>>> As mentioned in early email, new patch also contains implementation for
>>> Windows.
>>>
>>> Updated Webrev: http://cr.openjdk.java.net/~zgu/8191369/webrev.01/
>>>
>>> Test:
>>>
>>>    Reran hotspot_tier1 tests on Windows x64 and Linux x64 (fastdebug and
>>> release)
>> I have no idea about the validity of the Windows fix but the Linux one
>> looks good modulo that if condition.
>>
>> regards,
>>
>>
>> Andrew Dinn
>> -----------
>> Senior Principal Software Engineer
>> Red Hat UK Ltd
>> Registered in England and Wales under Company Registration No. 03798903
>> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
>>
> 


More information about the hotspot-runtime-dev mailing list