RFR: 8305819: LogConfigurationTest intermittently fails on AArch64

[Andrew Haley]

On Fri, 21 Apr 2023 09:42:31 GMT, gaogao-mem <duke at openjdk.org> wrote:

>> Atomic access is unnecessary when reading the list? I'd like to know the reasoning behind that.
>> I guess you could argue that dependency would save you on the read side, but I'd like to see and understand the code. It's very risky to depend on that,
>
> The list is read in src/hotspot/share/logging/logTagSet.cpp by iterator in src/hotspot/share/logging/logOutputList.hpp.
> For example, in void LogTagSet::log(LogLevelType level, const char* msg), the list is read as follows:
>   ```
> src/hotspot/share/logging/logTagSet.cpp
> 
> LogOutputList::Iterator it = _output_list.iterator(level);
>   LogDecorations decorations(level, *this, _decorators);
> 
>   for (; it != _output_list.end(); it++) {
>     (*it)->write(decorations, msg);
>   }
> 
> And I intended to add loadload in Iterator as follows:
>    ```
> src/hotspot/share/logging/logOutputList.hpp
> 
> void operator++(int) {
>           _current = _current->_next;
> +      OrderAccess::loadload();
>   }
>   Iterator iterator(LogLevelType level = LogLevel::Last) {
>         increase_readers();
> -    return Iterator(this, _level_start[level]);
> +    LogOutputNode* start=_level_start[level];
> +    OrderAccess::loadload();
> +    return Iterator(this, start);
>    }

I don't understand why some sort of atomic operation isn't required on the reading side. I'd think something like `Atomic::load_acquire()` would be appropriate here, along with `Atomic::release_store()` on the writing side. I think I need you to explain, with references, why you don't think so.

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

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