Round 2: RFR: 8013651 NMT: reserve/release sequence id's in incorrect order due to race

[Coleen Phillimore]

On 5/22/2013 8:09 PM, Zhengyu Gu wrote:
> Hi Harold,
>
> Thanks for the review.
>
> It can be done either way, since these operations does not require 
> pre-reserveration of sequence number, the sequence number is acquired 
> in NMTTracker::record(...) call.

It seems they should all be before the memory operation to be consistent 
and not require the caller to know which operations need to start the 
memory tracker to pre-reserve the sequence number. It's not really 100% 
consistent but it's good to have the trk declaration (which would be 
nicer as track) first.

I've sent some coding suggestion and requests for comments at the end to 
Zhengyu for the big case statement at the end of memTracker.cpp.   
Otherwise, this looks fine.

Coleen

>
> -Zhengyu
>
> On 5/22/2013 4:57 PM, harold seigel wrote:
>> Hi Zhengyu,
>>
>> In module os.cpp, in functions reserve_memory(), 
>> attempt_reserve_memory(), commit_memory(), and map_memory(), should 
>> the call to the MNTTracker constructor be done after the memory has 
>> been allocated?
>>
>> Thanks, Harold
>>
>>
>> On 5/22/2013 10:28 AM, Zhengyu Gu wrote:
>>> Based on the discussion with Karen, Coleen and Harold, following 
>>> changes are made:
>>>
>>> 1) Renamed NMTTrackOp to NMTTracker, avoid the confusion with VM 
>>> operations.
>>> 2) Used NMTTracker's dtor to discard the tracking operation if no 
>>> recording is done.
>>>
>>> Tests:
>>>  - JPRT
>>>  - vm.quick.testlist on Linux 32, Solaris sparcv9 and Windows 32.
>>>
>>> Thanks,
>>>
>>> -Zhengyu
>>>
>>>
>>> On 5/14/2013 10:01 AM, Zhengyu Gu wrote:
>>>> There can be race conditions between the memory operations and the 
>>>> book keeping records are written. For example, thread 1 releases a 
>>>> virtual memory block, before it can write the release record, 
>>>> thread 2 reserves the same virtual memory block and writes 
>>>> reservation first, as result, NMT indicates the block is "released".
>>>>
>>>> The solution is that, for those operations that can cause the race 
>>>> conditions, NMT should pre-reserve sequence number for it, if the 
>>>> operation succeeds, NMT uses pre-reserved sequence number to write 
>>>> the record.
>>>>
>>>> The tricky part is that, a sequence number is only good for the 
>>>> generation it is acquired, when there are reserved sequence number, 
>>>> NMT has to prevent itself from entering so called "sync-point" 
>>>> where the generation can be advanced.
>>>>
>>>>
>>>> Bug: http://bugs.sun.com/view_bug.do?bug_id=8013651
>>>> Webrev: http://cr.openjdk.java.net/~zgu/8013651/webrev/ 
>>>> <http://cr.openjdk.java.net/%7Ezgu/8013651/webrev/>
>>>>
>>>>
>>>> Tests:
>>>>    1) JPRT
>>>>    2) vm.quick.testlist on Linux 32, Linux x64 and Solaris Sparcv9
>>>>    3) Kitchensink on Linux 32, Linux x64, Solaris Sparcv9 and 
>>>> Windows x64
>>>>    4) NMT jtreg tests on Linux 32, Linux x64 and Solaris Sparcv9
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> -Zhengyu
>>>
>>
>