RFR (S): 8147087: Race when reusing PerRegionTable bitmaps may result in dropped remembered set entries

[Thomas Schatzl]

Hi Mikael,

On Tue, 2016-01-19 at 17:47 +0100, Mikael Gerdin wrote:
> Hi Thomas,
> 
> On 2016-01-19 17:09, Thomas Schatzl wrote:
> > Hi all,
> > 
[...]
> > The solution is to delay publishing the value of PRT::_hr after the
> > bitmap clearing, i.e.
> > 
> > void PerRegionTable::init(HeapRegion* hr, bool
> > clear_links_to_all_list)
> >     {
> >       if (clear_links_to_all_list) {
> >         set_next(NULL);
> >         set_prev(NULL);
> >       }
> > -    _hr = hr;
> >       _collision_list_next =
> > NULL;
> >       _occupied = 0;
> >       _bm.clear();
> > +    // Make sure that the
> > bitmap clearing above has been finished before publishing
> > +    // this
> > PRT to concurrent threads.
> > +    OrderAccess::release_store_ptr(&_hr,
> > hr);
> >     }
> > 
> > Which is the whole relevant change. :)
> 
> Shouldn't the corresponding load of _hr in the ::hr() accessor be a 
> load_acquire to mirror the release semantics on the store?
> Otherwise, even if the stores are performed in the correct order a 
> load-reordering machine could see an inconsistent state and 
> experience issues with missing remembered set entries.

All code following the use of hr() seems load-dependent (dereferencing
_hr), so ARM* should be okay.

You are right though, and I might have missed something (also something
about what this load-dependency means), I added this in 

http://cr.openjdk.java.net/~tschatzl/8147087/webrev.1_to_2/
http://cr.openjdk.java.net/~tschatzl/8147087/webrev.2/

Thanks for the review,
  Thomas