# HG changeset patch
# User rkennke
# Date 1584442165 -3600
#      Tue Mar 17 11:49:25 2020 +0100
# Node ID aac8eb10f78a0a80b6a2dcf8ecee581cc1f3263d
# Parent  daed0d4ec02dfead2296cdbc728c48db77a5ef48
8241081: Shenandoah: Don't concurrently modify update-watermark

diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
@@ -2407,7 +2407,6 @@
       if (r->is_active() && !r->is_cset()) {
         _heap->marked_object_oop_iterate(r, &cl, update_watermark);
       }
-      r->set_update_watermark(r->bottom());
       if (ShenandoahPacing) {
         _heap->pacer()->report_updaterefs(pointer_delta(update_watermark, r->bottom()));
       }
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp b/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp
@@ -261,7 +261,7 @@
   volatile size_t _live_data;
   volatile size_t _critical_pins;
 
-  HeapWord* volatile _update_watermark;
+  HeapWord* _update_watermark;
 
   // Claim some space at the end to protect next region
   DEFINE_PAD_MINUS_SIZE(0, DEFAULT_CACHE_LINE_SIZE, 0);
@@ -431,13 +431,20 @@
   }
 
   HeapWord* get_update_watermark() const {
-    assert(bottom() <= _update_watermark && _update_watermark <= top(), "within bounds");
-    return Atomic::load_acquire(&_update_watermark);
+    // Updates to the update-watermark only happen at safepoints or, when pushing
+    // back the watermark for evacuation regions, under the Shenandoah heap-lock.
+    // Consequently, we should access the field under the same lock. However, since
+    // those updates are only monotonically increasing, possibly reading a stale value
+    // is only conservative - we would not miss to update any fields.
+    HeapWord* watermark = _update_watermark;
+    assert(bottom() <= watermark && watermark <= top(), "within bounds");
+    return watermark;
   }
 
   void set_update_watermark(HeapWord* w) {
+    _heap->assert_heaplock_or_safepoint();
     assert(bottom() <= w && w <= top(), "within bounds");
-    Atomic::release_store(&_update_watermark, w);
+    _update_watermark = w;
   }
 
 private: