[15] RFR(S): 8246676: monitor list lock operations need more fencing and 8247280
Patricio Chilano
patricio.chilano.mateo at oracle.com
Wed Jul 15 20:50:20 UTC 2020
Hi Dan,
Changes look good to me. Thanks for the fixes.
Patricio
On 7/15/20 11:23 AM, Daniel D. Daugherty wrote:
> On 7/15/20 2:46 AM, David Holmes wrote:
>> Hi Dan,
>>
>> On 15/07/2020 12:30 pm, Daniel D. Daugherty wrote:
>>> Greetings,
>>>
>>> These fixes are targeted for JDK15 and I would like to push both of
>>> these
>>> fixes before the RDP2 cutoff on Thursday.
>>>
>>> I have a JDK15 fix ready for a couple of related ObjectMonitor bug
>>> fixes:
>>>
>>> JDK-8246676 monitor list lock operations need more fencing
>>> https://bugs.openjdk.java.net/browse/JDK-8246676
>>>
>>> JDK-8247280 more fencing needed in async deflation for non-TSO
>>> machines
>>> https://bugs.openjdk.java.net/browse/JDK-8247280
>>>
>>> The fix for JDK-8246676 has been through three rounds of preliminary
>>> code review with David H.; Erik O. and Robbin participated in the first
>>> preliminary code review round. Mostly comment changes or backouts of
>>> code changes to return to the baseline code were done in the second and
>>> third preliminary rounds.
>>>
>>> The fix for JDK-8247280 has been through two rounds of preliminary
>>> code review with David H. The bug fix itself was suggested by Erik
>>> O. so
>>> he's likely on-board with my implementation of the fix. :-)
>>>
>>> Many thanks to David H., Erik O. and Robbin for their many emails on
>>> these topics and for reviewing the preliminary webrevs.
>>>
>>> Here are the two webrevs:
>>>
>>> http://cr.openjdk.java.net/~dcubed/8247280-webrev/0-for-jdk15/
>>
>> My only follow up here is the proper fix for using Atomics with
>> enums. The patch is below and I've tested it with tiers 1-3 (link
>> sent separately).
>
> Thanks! I appreciate the help with the metaprogramming stuff...
>
>
>>
>>> http://cr.openjdk.java.net/~dcubed/8246676-webrev/0-for-jdk15/
>>
>> Also looks good.
>
> Thanks!
>
>
>>
>>> The project is currently baselined on jdk-15+30 and has gone through
>>> Mach5 Tier[1-3],4,5,6,7,8 testing with no regressions. I've also run
>>> my inflation stress kit on Linux-X64 and macOSX without any
>>> regressions.
>>>
>>> Thanks, in advance, for any comments, questions or suggestions.
>>>
>>> Dan
>>
>> Thanks for working through all the details on this.
>
> Thank you for the many emails, discussions and reviews. This memory
> consistency stuff is hard to get right.
>
>
>>
>> David
>> -----
>>
>> diff -r 3bef86e53c51 src/hotspot/share/runtime/objectMonitor.hpp
>> --- a/src/hotspot/share/runtime/objectMonitor.hpp
>> +++ b/src/hotspot/share/runtime/objectMonitor.hpp
>> @@ -27,6 +27,7 @@
>>
>> #include "memory/allocation.hpp"
>> #include "memory/padded.hpp"
>> +#include "metaprogramming/isRegisteredEnum.hpp"
>> #include "oops/markWord.hpp"
>> #include "runtime/os.hpp"
>> #include "runtime/park.hpp"
>> @@ -372,4 +373,7 @@
>> void install_displaced_markword_in_object(const oop obj);
>> };
>>
>> +// Register for atomic operations.
>> +template<> struct IsRegisteredEnum<ObjectMonitor::AllocationState> :
>> public TrueType {};
>> +
>> #endif // SHARE_RUNTIME_OBJECTMONITOR_HPP
>> diff -r 3bef86e53c51 src/hotspot/share/runtime/objectMonitor.inline.hpp
>> --- a/src/hotspot/share/runtime/objectMonitor.inline.hpp
>> +++ b/src/hotspot/share/runtime/objectMonitor.inline.hpp
>> @@ -196,7 +196,7 @@
>> }
>>
>> inline void
>> ObjectMonitor::release_set_allocation_state(ObjectMonitor::AllocationState
>> s) {
>> - Atomic::release_store((int*)&_allocation_state, (int)s);
>> + Atomic::release_store(&_allocation_state, s);
>> }
>>
>> inline void
>> ObjectMonitor::set_allocation_state(ObjectMonitor::AllocationState s) {
>> @@ -208,7 +208,7 @@
>> }
>>
>> inline ObjectMonitor::AllocationState
>> ObjectMonitor::allocation_state_acquire() const {
>> - return
>> (AllocationState)Atomic::load_acquire((int*)&_allocation_state);
>> + return Atomic::load_acquire(&_allocation_state);
>> }
>>
>> inline bool ObjectMonitor::is_free() const {
>
> That ended up being much simpler than I imagined late last night... :-)
>
> Thanks for coding this up and for running it through a Mach5 Tier[1-3].
> I very much appreciate it.
>
> Dan
>
More information about the hotspot-runtime-dev
mailing list