RFR: 8305819: LogConfigurationTest intermittently fails on AArch64 [v2]

[Andrew Haley]

On Sat, 6 May 2023 03:04:10 GMT, Dan Gao <duke at openjdk.org> wrote:

>> src/hotspot/share/logging/logOutputList.hpp line 52:
>> 
>>> 50:   struct LogOutputNode : public CHeapObj<mtLogging> {
>>> 51:     LogOutput*      _value;
>>> 52:     LogOutputNode* volatile _next;
>> 
>> Atomic::load_acquire will cast its argument to const volatile T* no matter what.
>>  
>> another reason is that there's no other load of _next field. I don't see any chance compiler elides load instruction for it. 
>> so I don't think we need 'volatile' qualifier for it. same reasons for  _level_start[LogLevel::Count]
>
> I have referred to the usage of Atomic::load_acquire in hotspot, and found that most of them use 'volatile' qualifier even if there is no multiple load of the field in some situation. Furthermore, I think adding volatile will not cause performance or correctness issue (harmless), thus I tend to add it.

Using a volatile qualifier for atomically-accessed fields is standard practice in HotSpot. Apart from anything else, it's good for readability.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13421#discussion_r1186668118