Request for review (S): 7097516 G1: assert(0<= from_card && from_card<HeapRegion::CardsPerRegion) failed: Must be in range.
Bengt Rutisson
bengt.rutisson at oracle.com
Wed Oct 19 09:45:53 UTC 2011
Hi again,
Updated webrev based on comments from Tony:
http://cr.openjdk.java.net/~brutisso/7097516/webrev.02/
Bengt
On 2011-10-19 09:14, Bengt Rutisson wrote:
>
> Hi all,
>
> Could I have a couple of reviews of this change?
>
> http://cr.openjdk.java.net/~brutisso/7097516/webrev.01/
>
> Some background:
>
> The humongous start regions in G1 gets their _end marker set to the
> end of the last humongous continues region for the humongous object.
> This means that the start region overlaps all of the continues
> regions. Code that handles regions need to be aware of this special case.
>
> However, PerRegionTable::add_reference_work() had a subtle issue with
> this special case. Here is what happened:
>
> A thread, TA, looks up the PerRegionTable, PRT, for a particular heap
> region. This heap region happens to be a humongous continues region.
> Let's call it HRA.
>
> Another thread, TB, wants to look up the PerRegionTable for the
> humongous start region of that same humongous object. Let's call that
> heap region HRB. If we are unlucky TB is the first thread that wants
> the PerRegionTable for HRB and thus has to allocate it. Now, there is
> a limit to how many PerRegionTables we allow in the system. If this
> limit has been reached TB will find a suitable existing PerRegionTable
> and coarsen that to use a bitmap for the remset instead of the
> PerRegionTable.
>
> This means that TB can actually "steal" the PRT from underneath TA.
>
> TA is aware that this can happen. So, before it tries to do any work
> with the information in the PRT it tries to verify that it hasn't been
> "stolen" (actually coarsened). It does this by storing the heap region
> for the PRT in a local variable (called loc_hr) and then check that
> the reference that it is about to handle still belong to the heap region.
>
> Basically:
>
> HeapRegion* loc_hr = hr();
> if (loc_hr->is_in_reserved(from)) {
> ...
> }
>
> Normally this is safe. If TA gets HRB in loc_hr the test will fail.
> However, in the case where HRB is the humongous start region the test
> will not fail. Instead we will pass the test and execute the code
> inside the if. There we try to calculate a card index based on the
> bottom of the heap region and the from address. Now this card index
> will be larger than the number of cards we have per region, since from
> actually is not in the region.
>
> The fix is to introduce a new method called
> HeapRegion::is_in_reserved_raw() and use this in the test. The _raw
> method will not return false even for humongous start objects in the
> example above.
>
> I picked the name is_in_reserved_raw() to be consistent with the
> heap_region_containing()/heap_region_containing_raw() methods. I am
> not particularly fond of the name, so if there are better suggestions
> I am happy to change it.
>
> Testing
> I was able to reproduce the issue on bur398-216 after 40-50
> iterations. With my fix it has now ran 540 iteration without failing.
>
> Other uses of is_in_reserved()
> Tony and I went through the G1 code and looked for other uses of
> HeapRegion::is_in_reserved(). As far as we can tell the other uses all
> avoid the issue that PerRegionTable::add_reference_work() has. Most of
> them because they are used during evacuation where humongous objects
> are not present at all.
>
> Webrev:
> http://cr.openjdk.java.net/~brutisso/7097516/webrev.01/
>
> CR:
> 7097516 G1: assert(0<= from_card &&
> from_card<HeapRegion::CardsPerRegion) failed: Must be in range.
> http://monaco.us.oracle.com/detail.jsf?cr=7097516
>
> Thanks,
> Bengt
More information about the hotspot-gc-dev
mailing list