RFR(XS): 8164038: Missing volatile keyword at CardTableRS::write_ref_field_gc_par()

Kim Barrett kim.barrett at oracle.com
Wed Mar 8 18:12:15 UTC 2017


> On Mar 7, 2017, at 8:14 PM, sangheon <sangheon.kim at oracle.com> wrote:
> 
> Hi all,
> 
> Could I have some reviews for this tiny change that adds volatile qualifier?
> 
> Currently without 'volatile' we don't have any problem but I would like to add it for the code readability.
> 
> --------
> * Background
> Initially this problem occurred between b127 and b134 (<= b126 or >= b135 are okay). 
> b127 includes "JDK-8155949, Support relaxed semantics in cmpxchg" and b135 includes "JDK-8157904: Atomic::cmpxchg for jbyte is missing a fence on initial failure". 
> The patch for JDK-8155949 changed 'jbyte Atomic::cmpxchg()' which eventually affected not to reload the variable 'entry' at CardTableRS::write_ref_field_gc_par(), so gc/cms/TestBubbleUpRef.java failed on ARM machine. And the patch for JDK-8157904 fixed it.
> But without the patch for JDK-8157904, just adding 'volatile' also fixes this problem.
> 
> After founding the root cause, I saw another CR, "JDK-8033552: Fix missing missing volatile specifiers in CAS operations in GC code" also includes adding 'volatile' from its initial patch so I closed this CR but the final patch didn't include this part intentionally. After discussing with the author of JDK-8033552, Erik Österlund, we decided to fix under this CR.
> 
> CR: https://bugs.openjdk.java.net/browse/JDK-8164038
> Webrev: http://cr.openjdk.java.net/~sangheki/8164038/webrev.0
> Testing: JPRT
> 
> Thanks,
> Sangheon

looks good.



More information about the hotspot-gc-dev mailing list