RFR: 8166811: Missing memory fences between memory allocation and refinement

[Kim Barrett]

> On Nov 8, 2016, at 7:52 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> On Tue, 2016-10-25 at 19:13 -0400, Kim Barrett wrote:
>> There is still a situation where processing can fail, namely an
>> in-progress humongous allocation that hasn't set the klass yet.  We
>> continue to handle that as before.
> 
> - I am not completely sure about whether this case is handled
> correctly. I am mostly concerned that the information used before the
> fence may not be the correct ones, but the checks expect them to be
> valid.
> 
> Probably I am overlooking something critical somewhere.
> 
> A: allocates humongous object C, sets region type, issues storestore,
> sets top pointers, writes the object, and then sets C.y = x to mark a
> card
> 
> Refinement: gets card (and assuming we have no further synchronization
> around which is not true, e.g. the enqueuing)
> 
>  592   if (!r->is_old_or_humongous()) {
> 
> assume refinement thread has not received the "type" correctly yet, so
> must be Free. So the card will be filtered out incorrectly?
> 
> That is contradictory to what I said in the other email about the
> comment discussion, but I only thoroughly looked at the comment aspect
> there. :)
> 
> I think at this point in general we can't do anything but !is_young(),
> as we can't ignore cards in "Free" regions - they may be for cards for
> humongous ones where the thread did not receive top and/or the type
> yet?
> 
> - assuming this works due to other synchronization,

This is the critical point.  There *is* synchronization there.

In the scenario described, the card that was marked and enqueued after
the object was created will pass through some synchronization barriers
(full locks, perhaps someday lock-free but with appropriate memory
barriers) along the way to refinement.

This is the "easy" case.  If only it were that simple...

The additional checks are to deal with the possibility of stale cards.

> […] I have another
> similar concern with later trimming:
> 
> 653 } else {
> 654   // Non-humongous objects are only allocated in the old-gen during
> 655   // GC, so if region is old then top is stable.  Humongous object
> 656   // allocation sets top last; if top has not yet been set, then
> 657   // we'll end up with an empty intersection.
> 658   scan_limit = r->top();
> 659 }
> 660 if (scan_limit <= start) {
> 661   // If the trimmed region is empty, the card must be stale.
> 662   return false;
> 663 }
> 
> Assume that the current value of top for a humongous object has not
> been seen yet by the thread and we end up with an empty intersection.
> 
> Now, didn't we potentially just drop a card to a humongous object in
> waiting to scan but did not re-enqueue it? (And we did not clear the
> card table value either?)
> 
> We may do it after the fence though I think.
> 
> Maybe I am completely wrong though, what do you think?

If we see the old (zero) value of top in conjunction with a humongous
region type, it is because this is a stale card.  If this were a
non-stale card, the synchronization between enqueuing the card and
reaching refinement would have ensured we see an up-to-date top (as
well as an up-to-date type).  Card table entries for a free region are
cleaned before the region can be allocated (and there are locks in the
allocation path that provide the needed ordering).  Since this is a
stale card and regions are allocated with clean card table entries,
the dirty card table entry check having passed implies there is
another (non-stale and not-yet-processed) card making its way to
refinement through the usual channels, including the needed
synchronization barriers.


> - another stale comment:
> 
>  636   // a card beyond the heap.  This is not safe without a perm
>  637   // gen at the upper end of the heap.
> 
> Could everything after "without" be removed in this sentence? We
> haven't had a "perm gen" for a long time…

Yes.  I’ll make that change.