RFR: Add new pinned/cset region state for evac-failure-path

[Roman Kennke]

Am 08.11.2017 um 13:56 schrieb Aleksey Shipilev:
> On 11/08/2017 01:43 PM, Roman Kennke wrote:
>> We sometimes get an assert failure when running OOM during evac and attempting to pin a region
>> afterwards: it would try to make a cset region pinned. This is normally not allowed, but necessary
>> on the failure path, as a temporary state until mark-compact cleaned it up.
>>
>> I solved it by adding a new region-state pinned-cset: when a region is in cset and is made pinned,
>> it checks that it's on cancelled-gc path and goes to pinned-cset. From there it can only go back to
>> cset (when unpinning the region) or to pinned (when clearing the cset state in mark-compact).
>>
>> http://cr.openjdk.java.net/~rkennke/pinned-cset/webrev.00/
> I have a larger problem with the patch like this. Suppose we are on cancellation path, and we have
> already half-evacuated the cset region. Then pinning comes, but WB is disabled already, and so we
> move the region to pinned_cset state. Now, Full GC code would not move the region data, making sure
> pinned objects stay pinned.
Yes, too bad. But this is already the case: we can get half-evacuated 
regions in mark-compact, and we can get pinned regions in mark-compact 
(disabling compaction for them). None of it is a problem now. This patch 
doesn't really change it.
> Does it update the pointers right, though?
Yes. Every reference to an already-evacuated object will get updated. 
Every reference to a not-evacuated object will remain. Just as it is now.

>   E.g. could it happen that we are left with pointers to
> pinned_cset after Full GC?
Yes, but only to objects that are not evacuated.
>   Does it recycle those cset regions now, thus corrupting the heap?
No.

All that the patch does is introduce legal region state changes for the 
exceptional path: suppose we'd try to do without pinned_cset, what could 
we do when pinning a cset region? We can make it pinned, but lose the 
info that it was cset, and then when unpinning, go back to regular? This 
sounds wrong.


> Nits:
>
>   *) In make_pinned and make_unpinned: move _pinned case towards _pinned_cset:
This is only possible in make_pinned(). In make_unpinned() we need to 
move it to _cset instead of _regular. Also, we don't want to loose the 
guarantee(cancelled_concgc()).

However, I discovered a bug in my impl: I moved from _cset -> _pinned, 
but intended to move _cset -> _pinned_cset.

Differential:
http://cr.openjdk.java.net/~rkennke/pinned-cset/webrev.01.diff 
<http://cr.openjdk.java.net/%7Erkennke/pinned-cset/webrev.01.diff>
Full:
http://cr.openjdk.java.net/~rkennke/pinned-cset/webrev.01 
<http://cr.openjdk.java.net/%7Erkennke/pinned-cset/webrev.01>

Better?

Roman