<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    Generally when you have a storestore on the write side you need a
    loadload on the read side to prevent the second read from floating
    above the first one. The reading thread could read in_progress as 0
    before it reads starting. Meanwhile the write thread writes 1 to
    in_progress, issues storestore, clears starting. Reading thread then
    reads starting as 0. I don't know if the CGC mutex somehow
    eliminates this issue as I'm not familiar with the code in detail.<br>
    <br>
    bill<br>
    <br>
    <div class="moz-cite-prefix">On 6/23/2015 4:25 PM, Bengt Rutisson
      wrote:<br>
    </div>
    <blockquote cite="mid:5589C09C.6000305@oracle.com" type="cite">
      <meta http-equiv="content-type" content="text/html; charset=utf-8">
      <br>
      Hi everyone,<br>
      <br>
      Could I have a couple of reviews for this change?<br>
      <br>
      <a moz-do-not-send="true" class="moz-txt-link-freetext"
        href="https://bugs.openjdk.java.net/browse/JDK-8129626">https://bugs.openjdk.java.net/browse/JDK-8129626</a><br>
      <a moz-do-not-send="true" class="moz-txt-link-freetext"
        href="http://cr.openjdk.java.net/%7Ebrutisso/8129626/webrev.00/">http://cr.openjdk.java.net/~brutisso/8129626/webrev.00/</a><br>
      <br>
      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().<br>
      <br>
      Thanks to Per and Bertrand for helping out identifying and sorting
      this out.<br>
      <br>
      From the bug report:<br>
      <br>
      <meta http-equiv="content-type" content="text/html; charset=utf-8">
      ConcurrentMarkThread::during_cycle() is implemented as: <br>
      <br>
      bool during_cycle() { return started() || in_progress(); } <br>
      <br>
      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():
      <br>
      <br>
      void ConcurrentMarkThread::sleepBeforeNextCycle() { <br>
        // We join here because we don't want to do the
      "shouldConcurrentMark()" <br>
        // below while the world is otherwise stopped. <br>
        assert(!in_progress(), "should have been cleared"); <br>
      <br>
        MutexLockerEx x(CGC_lock, Mutex::_no_safepoint_check_flag); <br>
        while (!started() && !_should_terminate) { <br>
          CGC_lock->wait(Mutex::_no_safepoint_check_flag); <br>
        } <br>
      <br>
        if (started()) { <br>
          set_in_progress(); <br>
          clear_started(); <br>
        } <br>
      } <br>
      <br>
      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. <br>
      <br>
      <br>
      Thanks,<br>
      Bengt<br>
    </blockquote>
    <br>
  </body>
</html>