From 19f64478f11d2cde3ba4f0e41aecfa4635ab0e26 Mon Sep 17 00:00:00 2001 From: Laurent Morichetti Date: Sun, 23 Aug 2020 19:52:50 -0700 Subject: [PATCH] Add missing storeload memory fences There is no synchronize with relationship between the monitor micro- lock and the onDeck microlock, so it is possible for an onDeck.load to move above a contendersList.store, or a contendersList.load to move above an ondeck.store. To fix this issue a full memory fence (mm_mfence on x86) is needed after the last store in the contendersList and onDeck critical regions. Change-Id: I5beb7dfe0d21010c5bf00cd65d59b9c7af58e919 [ROCm/clr commit: f10435a1ef71294dfa174100158c9db1ed0872ed] --- projects/clr/rocclr/thread/monitor.cpp | 4 ++++ projects/clr/rocclr/thread/monitor.hpp | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/projects/clr/rocclr/thread/monitor.cpp b/projects/clr/rocclr/thread/monitor.cpp index dbb7949755..9c77f961dc 100644 --- a/projects/clr/rocclr/thread/monitor.cpp +++ b/projects/clr/rocclr/thread/monitor.cpp @@ -205,6 +205,10 @@ void Monitor::finishUnlock() { return; } + // A StoreLoad barrier is required to make sure the onDeck_ store is published before + // the contendersList_ micro-lock check. + std::atomic_thread_fence(std::memory_order_seq_cst); + // We do not have an on-deck thread (semaphore == NULL). Return if // the contention list is empty or if the lock got acquired again. head = contendersList_; diff --git a/projects/clr/rocclr/thread/monitor.hpp b/projects/clr/rocclr/thread/monitor.hpp index 7581d13778..c89ec287aa 100644 --- a/projects/clr/rocclr/thread/monitor.hpp +++ b/projects/clr/rocclr/thread/monitor.hpp @@ -228,6 +228,11 @@ inline void Monitor::unlock() { while (!contendersList_.compare_exchange_weak(ptr, ptr & ~kLockBit, std::memory_order_acq_rel, std::memory_order_acquire)) ; + + // A StoreLoad barrier is required to make sure future loads do not happen before the + // contendersList_ store is published. + std::atomic_thread_fence(std::memory_order_seq_cst); + // // We succeeded the CAS from locked to unlocked. // This is the end of the critical region.