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

[Thomas Schatzl]

Hi Kim,

On Mon, 2016-11-07 at 14:07 -0500, Kim Barrett wrote:
> > 
> > On Nov 7, 2016, at 1:36 PM, Kim Barrett <kim.barrett at oracle.com>
> > wrote:
> > > 
> > > > 
> > > > 
[...]
> > > One could replace "dirtied" by something less specific here to
> > > make it
> > > right again.
> > Good idea.  How about this rewording (using “set to a value”)
> > 
> >  // The region could be young.  Cards for young regions are set to
> > a
> >  // value that allows the post-barrier to filter them
> > out.  However,
> >  // that card setting is performed concurrently.  A write to a
> > young
> >  // object could occur before the card has been set, slipping past
> >  // the filter.
> Oops, no, that isn't right.  (It's been a couple of weeks since I
> looked at this, and forgot part of the problem.)
> 
> Part of what's wrong with the comment is that we can no longer get to
> that point with a young region.  A young region's cards will be ither
> g1_young_gen or clean, never dirty.  Hence the filtering out of non-

Why? I think the reason for this comment has been that the following
could happen:

A: allocate new young region X, allocate object, storestore, stops at
the beginning of the dirty_young_block() method

B: allocate new object B in X, set B.y = something-outside, making the
card "Dirty" since thread A did not actually start doing
dirty_young_block() yet.

Refinement: scans the card; since R does not seem to synchronize with A
either, you may get a "dirty" card in a young (or free, depending on
whether the setting of the region flag in X has already been observed -
but it must be either one) region here in this case?

A: does the work in dirty_young_block()

(The previous is_young() check has indeed been wrong, and
is_old_or_humongous() is better)

> dirty cards a few lines before this comment will have already
> discarded a young card before we reach the test this comment is
> discussing.  So the whole premise of the comment in question, that
> the region could be young, is false.

I think the comment is good after all. I would even emphasize the act
of setting it to "g1_young_gen" by writing something like:

// The region could be young.  Cards for young regions are set to 
// "g1_young_gen" so the post-barrier will filter them out.  However, 
// that dirtying is performed concurrently.  A write to a young object 
// could occur in the same region before the cards have been set to 
// that value, slipping past the filter.

Because then, if somebody removes g1_young_gen, he will hopefully find
this place again by searching for "g1_young_gen" and think about this
situation again. (in theory :))

Thanks,
  Thomas