RFR (S): 8182703: Correct G1 barrier queue lock orderings

[Kim Barrett]

> On Jun 26, 2017, at 9:34 AM, Erik Österlund <erik.osterlund at oracle.com> wrote:
> 
> Hi,
> 
> Webrev: http://cr.openjdk.java.net/~eosterlund/8182703/webrev.02/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8182703

------------------------------------------------------------------------------
src/share/vm/gc/g1/ptrQueue.cpp
Removing unlock / relock around
  78   qset()->enqueue_complete_buffer(node);

I would prefer that this part of this changeset not be made at this
time.

This part isn't necessary for the main point of this changeset.  It's
a cleanup that is enabled by the lock rank changes, where the rank
changes are required for other reasons.

It also at least conflicts with, and probably breaks, a pending change
of mine.  (I have a largish stack of patches in this area that didn't
quite make it into JDK 9 before the original FC date, and which I've
been (all too slowly) trying to work my way through and bring into JDK
10.)

The RFR says:

> 2) There is an issue that the shared queue locks have a "special"
> rank, which is below the lock ranks used by the cbl monitor and free
> list monitor. This leads to an issue when these locks have to be taken
> while holding the shared queue locks. The current solution is to drop
> the shared queue locks temporarily, introducing nasty data
> races. These races are guarded, but the whole race seems very
> unnecessary.

This isn't entirely accurate, as the shared queue locks are not
"special" rank; the current lock ranks are described correctly later
in the RFR.

It's true there is an "interesting" bit of code there to temporarily
drop the shared queue lock.  I don't think it's harmful to do so, and
could have some small benefit now.  More importantly, one of the
changes in that afore mentioned stack of patches puts more (possibly
significantly more in some cases) work into that dropped-lock region.
And if that idea ultimately doesn't pan out, simply removing the
unlock/relock pair is not, IMO, the right way to clean things up;
there is some additional refactoring that ought to be done.

------------------------------------------------------------------------------

The lock ranking changes look good.