RFR: JDK-8129626: G1: set_in_progress() and clear_started() needs a barrier on non-TSO platforms
Bengt Rutisson
bengt.rutisson at oracle.com
Tue Jun 23 20:25:00 UTC 2015
Hi everyone,
Could I have a couple of reviews for this change?
https://bugs.openjdk.java.net/browse/JDK-8129626
http://cr.openjdk.java.net/~brutisso/8129626/webrev.00/
We need to add a barrier between the calls to set_in_progress() and
clear_started() to make sure that other threads sees the correct value
when they use ConcurrentMarkThread::during_cycle().
Thanks to Per and Bertrand for helping out identifying and sorting this out.
From the bug report:
ConcurrentMarkThread::during_cycle() is implemented as:
bool during_cycle() { return started() || in_progress(); }
So, it checks both ConcurrentMarkThread::_started and
ConcurrentMarkThread::_in_progress and they are meant to overlap. That
is, we should not set _started to false until after we have set
_in_progress to true. This is done in sleepBeforeNextCycle():
void ConcurrentMarkThread::sleepBeforeNextCycle() {
// We join here because we don't want to do the "shouldConcurrentMark()"
// below while the world is otherwise stopped.
assert(!in_progress(), "should have been cleared");
MutexLockerEx x(CGC_lock, Mutex::_no_safepoint_check_flag);
while (!started() && !_should_terminate) {
CGC_lock->wait(Mutex::_no_safepoint_check_flag);
}
if (started()) {
set_in_progress();
clear_started();
}
}
On non-TSO platforms there is a risk that the write to _in_progress
(from set_in_progress()) is seen by other threads after the write to
_started (in clear_started()). In that case there is a window when
during_cycle() may return false even though we are in a concurrent cycle.
Thanks,
Bengt
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150623/cb32e9a9/attachment.htm>
More information about the hotspot-gc-dev
mailing list