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

[Bengt Rutisson]

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!

(Copyright year should be 2012.)

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
>