RFR: 8244551: Shenandoah: Fix racy update of update_watermark
Roman Kennke
rkennke at redhat.com
Thu May 7 11:37:14 UTC 2020
>>> *) What's the point of this cast?
>>>
>>> 133 *const_cast<HeapWord**>(&_update_watermark) = w;
>>
>> Make access non-volatile. You think it's too much?
>
> I don't think that is relevant. As long as it is not the atomic store (with fences), we are fine.
Ok. I will push the fix with this additional change then:
diff -r 7910475d5001
src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.inline.hpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.inline.hpp
Thu May 07 07:03:25 2020 -0400
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.inline.hpp
Thu May 07 07:36:26 2020 -0400
@@ -128,9 +128,8 @@
// Fast version that avoids synchronization, only to be used at safepoints.
inline void
ShenandoahHeapRegion::set_update_watermark_at_safepoint(HeapWord* w) {
assert(bottom() <= w && w <= top(), "within bounds");
- assert(SafepointSynchronize::is_at_safepoint(),
- "only call set_update_watermark_at_safepoint at safepoint");
- *const_cast<HeapWord**>(&_update_watermark) = w;
+ assert(SafepointSynchronize::is_at_safepoint(), "Should be at
Shenandoah safepoint");
+ _update_watermark = w;
}
#endif // SHARE_GC_SHENANDOAH_SHENANDOAHHEAPREGION_INLINE_HPP
Thanks for reviewing!
Roman
More information about the shenandoah-dev
mailing list