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

On 2012-02-13 23:09, Dean Long wrote:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_idq40866

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

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 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.)

Bengt

On Mon, Feb 13, 2012 at 2:09 PM, Dean Long wrote:

http://bugs.sun.com/**bugdatabase/view_bug.do?bug_**idq40866<http://bugs.sun.com/bugdatabase/view_bug.do?bug_idq40866>

http://cr.openjdk.java.net/~**dlong/7140866/webrev.00/<http://cr.openjdk.java.net/%7Edlong/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

On 2/14/2012 4:30 PM, Srinivas Ramakrishna wrote:
Looks good to me.

-- ramki (openjdk: ysr)

On Mon, Feb 13, 2012 at 2:09 PM, Dean Long <dean.long at oracle.com
wrote:

http://bugs.sun.com/bugdatabase/view_bug.do?bug_idq40866

http://cr.openjdk.java.net/~dlong/7140866/webrev.00/
<http://cr.openjdk.java.net/%7Edlong/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