Commit 37f7753a by FritzFlorian

Fix memory order in single write, multiple read lock.

parent 0548562e
...@@ -3,6 +3,9 @@ ...@@ -3,6 +3,9 @@
#define PLS_SYSTEM_DETAILS_H #define PLS_SYSTEM_DETAILS_H
#include <cstdint> #include <cstdint>
#if (COMPILER == MVCC)
#include <emmintrin.h>
#endif
namespace pls { namespace pls {
namespace internal { namespace internal {
...@@ -36,7 +39,6 @@ constexpr std::uintptr_t CACHE_LINE_SIZE = 64; ...@@ -36,7 +39,6 @@ constexpr std::uintptr_t CACHE_LINE_SIZE = 64;
* Choose the implementation appropriate for your compiler-cpu combination. * Choose the implementation appropriate for your compiler-cpu combination.
*/ */
#if (COMPILER == MVCC) #if (COMPILER == MVCC)
#include <emmintrin.h>
inline void relax_cpu() { inline void relax_cpu() {
_mm_pause(); _mm_pause();
} }
......
...@@ -16,7 +16,7 @@ class abstract_task { ...@@ -16,7 +16,7 @@ class abstract_task {
private: private:
unsigned int depth_; unsigned int depth_;
abstract_task::id unique_id_; abstract_task::id unique_id_;
abstract_task *child_task_; abstract_task *volatile child_task_;
public: public:
abstract_task(const unsigned int depth, const abstract_task::id &unique_id) : abstract_task(const unsigned int depth, const abstract_task::id &unique_id) :
...@@ -26,7 +26,7 @@ class abstract_task { ...@@ -26,7 +26,7 @@ class abstract_task {
virtual void execute() = 0; virtual void execute() = 0;
void set_child(abstract_task *child_task) { child_task_ = child_task; } void set_child(abstract_task *child_task) { child_task_ = child_task; }
abstract_task *child() { return child_task_; } abstract_task *child() const { return child_task_; }
void set_depth(unsigned int depth) { depth_ = depth; } void set_depth(unsigned int depth) { depth_ = depth; }
unsigned int depth() const { return depth_; } unsigned int depth() const { return depth_; }
......
...@@ -11,7 +11,7 @@ bool swmr_spin_lock::reader_try_lock() { ...@@ -11,7 +11,7 @@ bool swmr_spin_lock::reader_try_lock() {
return false; return false;
} }
// We think we can enter the region // We think we can enter the region
readers_++; readers_.fetch_add(1, std::memory_order_acquire);
if (write_request_.load() == 1) { if (write_request_.load() == 1) {
// Whoops, the writer acquires the lock, so we back off again // Whoops, the writer acquires the lock, so we back off again
readers_--; readers_--;
...@@ -23,21 +23,29 @@ bool swmr_spin_lock::reader_try_lock() { ...@@ -23,21 +23,29 @@ bool swmr_spin_lock::reader_try_lock() {
void swmr_spin_lock::reader_unlock() { void swmr_spin_lock::reader_unlock() {
PROFILE_LOCK("Release Read Lock") PROFILE_LOCK("Release Read Lock")
readers_--; readers_.fetch_add(-1, std::memory_order_release);
} }
void swmr_spin_lock::writer_lock() { void swmr_spin_lock::writer_lock() {
PROFILE_LOCK("Acquire Write Lock") PROFILE_LOCK("Acquire Write Lock")
// Tell the readers that we would like to write // Tell the readers that we would like to write
write_request_ = 1; int expected;
while (true) {
expected = 0;
if (write_request_.compare_exchange_weak(expected, 1, std::memory_order_acquire)) {
break;
}
system_details::relax_cpu(); // Spin until WE set the write lock flag
}
// Wait for all of them to exit the critical section // Wait for all of them to exit the critical section
while (readers_.load(std::memory_order_relaxed) > 0) while (readers_.load() > 0)
system_details::relax_cpu(); // Spin, not expensive as relaxed load system_details::relax_cpu(); // Spin, not expensive as relaxed load
} }
void swmr_spin_lock::writer_unlock() { void swmr_spin_lock::writer_unlock() {
PROFILE_LOCK("Release Write Lock") PROFILE_LOCK("Release Write Lock")
write_request_ = 0; write_request_.store(0, std::memory_order_release);
} }
} }
......
...@@ -31,7 +31,7 @@ bool abstract_task::steal_work() { ...@@ -31,7 +31,7 @@ bool abstract_task::steal_work() {
PROFILE_STEALING("Go to our level") PROFILE_STEALING("Go to our level")
abstract_task *current_task = target_state->root_task_; abstract_task *current_task = target_state->root_task_;
while (current_task != nullptr && current_task->depth() < depth()) { while (current_task != nullptr && current_task->depth() < depth()) {
current_task = current_task->child_task_; current_task = current_task->child();
} }
PROFILE_END_BLOCK PROFILE_END_BLOCK
...@@ -48,7 +48,7 @@ bool abstract_task::steal_work() { ...@@ -48,7 +48,7 @@ bool abstract_task::steal_work() {
} }
// No success, we need to steal work from a deeper level using 'top level task stealing' // No success, we need to steal work from a deeper level using 'top level task stealing'
current_task = current_task->child_task_; current_task = current_task->child();
} }
} }
PROFILE_END_BLOCK; PROFILE_END_BLOCK;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment