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

[sangheon]

Hi Kim,

Thanks for the review!

Sangheon


On 03/08/2017 10:12 AM, Kim Barrett wrote:
>> 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.
>