review request 7140866: assert(covered) failed: Card for end of new region not committed

[Dean Long]

On 2/14/2012 8:55 PM, Bengt Rutisson wrote:
>
> Hi again Dean,
>
> Please disregard my last email. The changed behavior I was thinking 
> about was actually for "-XX:-UseSharedSpaces -XX:-DumpSharedSpaces", 
> but in that case I think we bail out already at line 207 and never get 
> in to the code you changed.
>
>  207   if (spec()->enable_shared_spaces()) {
>
> Sorry for the confusion.
>
> Change looks good! Ship it!
>
Great.  Thanks for the review.

> (Copyright year should be 2012.)
>
Thanks for catching that.  Fixed.

dl

> Bengt
>
> On 2012-02-14 23:24, Bengt Rutisson wrote:
>>
>> Hi Dean,
>>
>> Thanks for fixing this. Overall I think your change look good, but I 
>> have one question:
>>
>> The check on line 282 uses spec()->enable_shared_spaces(), which I 
>> think is basically the same as "UseSharedSpaces || DumpSharedSpaces". 
>> So, this means that if we are running with "-XX:-UseSharedSpaces 
>> -XX:+DumpSharedSpaces" we would before your change do a call to
>>
>> _rs->resize_covered_region(MemRegion(readonly_bottom, readwrite_end))
>>
>> but after your change we won't do that call, right? Is this an 
>> intended change of behavior?
>>
>> I admit that I know too little about this to even know if the above 
>> command line makes sense, but it seems to be allowed.
>>
>> Thanks,
>> Bengt
>>
>> On 2012-02-13 23:09, Dean Long wrote:
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7140866
>>>
>>> http://cr.openjdk.java.net/~dlong/7140866/webrev.00/
>>>
>>> The problem happens when trying to use a shared archive.  If for 
>>> some reason mapping the shared file fails,
>>> we call
>>>
>>>     _rs->resize_covered_region(MemRegion(shared_bottom, 
>>> shared_bottom));
>>>
>>> to undo the earlier call to
>>>
>>>     _rs->resize_covered_region(MemRegion(readonly_bottom, 
>>> readwrite_end));
>>>
>>> Unfortunately the resize code is not prepared to deal with a 
>>> zero-sized region so an assert fails.
>>> Rather than touch the resize code, this fix changes the caller to 
>>> only call resize_covered_region()
>>> once after the shared file is mapped.
>>>
>>> Tested fix on a particular linux arm platform where the mmap fails 
>>> 100% of the time due to a different issue.
>>> Ran -Xshare:dump first then a few GC tests.
>>>
>>> dl
>>
>