From a8c60905c3beb971d32d750d24409b60c3f882c4 Mon Sep 17 00:00:00 2001
From: Martin Quinson <martin.quinson@ens-rennes.fr>
Date: Wed, 23 Feb 2022 22:29:20 +0100
Subject: [PATCH] Mutex do not need a locked_ field. owner_ != null is enough

---
 ChangeLog                         |  1 +
 src/kernel/activity/MutexImpl.cpp | 14 +++++---------
 src/kernel/activity/MutexImpl.hpp |  2 --
 src/mc/explo/SafetyChecker.cpp    |  5 +++--
 4 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 02714ed607..6cb9aa8d8e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -4,6 +4,7 @@ MC:
  - Rework the internals, for simpler and modern code. This shall unlock many future improvements.
  - You can now define plugins onto SafetyChecker (a simple DFS explorer), using the declared signals.
    See CommunicationDeterminism for an example.
+ - Seems to work on Arm64 architectures too.
 
 SMPI:
  - fix for FG#100 by ensuring small asynchronous messages never overtake larger
diff --git a/src/kernel/activity/MutexImpl.cpp b/src/kernel/activity/MutexImpl.cpp
index 5087f66556..8051a4910e 100644
--- a/src/kernel/activity/MutexImpl.cpp
+++ b/src/kernel/activity/MutexImpl.cpp
@@ -28,7 +28,7 @@ bool MutexAcquisitionImpl::test(actor::ActorImpl*)
 }
 void MutexAcquisitionImpl::wait_for(actor::ActorImpl* issuer, double timeout)
 {
-  xbt_assert(mutex_->locked_); // it was locked either by someone else or by me during the lock_async
+  xbt_assert(mutex_->owner_ != nullptr); // it was locked either by someone else or by me during the lock_async
   xbt_assert(
       issuer == issuer_,
       "Actors can only wait acquisitions that they created themselves while this one was created by actor id %ld.",
@@ -57,12 +57,11 @@ MutexAcquisitionImplPtr MutexImpl::lock_async(actor::ActorImpl* issuer)
 {
   auto res = MutexAcquisitionImplPtr(new kernel::activity::MutexAcquisitionImpl(issuer, this), true);
 
-  if (locked_) {
+  if (owner_ != nullptr) {
     /* FIXME: check if the host is active ? */
     /* Somebody using the mutex, use a synchronization to get host failures */
     sleeping_.push_back(res);
   } else {
-    locked_ = true;
     owner_  = issuer;
   }
   return res;
@@ -77,12 +76,11 @@ bool MutexImpl::try_lock(actor::ActorImpl* issuer)
 {
   XBT_IN("(%p, %p)", this, issuer);
   MC_CHECK_NO_DPOR();
-  if (locked_) {
+  if (owner_ != nullptr) {
     XBT_OUT();
     return false;
   }
 
-  locked_ = true;
   owner_  = issuer;
   XBT_OUT();
   return true;
@@ -97,9 +95,8 @@ bool MutexImpl::try_lock(actor::ActorImpl* issuer)
 void MutexImpl::unlock(actor::ActorImpl* issuer)
 {
   XBT_IN("(%p, %p)", this, issuer);
-  xbt_assert(locked_, "Cannot release that mutex: it was not locked.");
-  xbt_assert(issuer == owner_, "Cannot release that mutex: it was locked by %s (pid:%ld), not by you.",
-             owner_->get_cname(), owner_->get_pid());
+  xbt_assert(issuer == owner_, "Cannot release that mutex: you're not the owner. %s is (pid:%ld).",
+             owner_ != nullptr ? owner_->get_cname() : "(nobody)", owner_ != nullptr ? owner_->get_pid() : -1);
 
   if (not sleeping_.empty()) {
     /* Give the ownership to the first waiting actor */
@@ -112,7 +109,6 @@ void MutexImpl::unlock(actor::ActorImpl* issuer)
     sleeping_.pop_front();
   } else {
     /* nobody to wake up */
-    locked_ = false;
     owner_  = nullptr;
   }
   XBT_OUT();
diff --git a/src/kernel/activity/MutexImpl.hpp b/src/kernel/activity/MutexImpl.hpp
index 5f9e27d5cb..bbdef1cd9f 100644
--- a/src/kernel/activity/MutexImpl.hpp
+++ b/src/kernel/activity/MutexImpl.hpp
@@ -65,7 +65,6 @@ public:
 class XBT_PUBLIC MutexImpl {
   std::atomic_int_fast32_t refcount_{1};
   s4u::Mutex piface_;
-  bool locked_ = false;
   actor::ActorImpl* owner_ = nullptr;
   // List of sleeping actors:
   std::deque<MutexAcquisitionImplPtr> sleeping_;
@@ -80,7 +79,6 @@ public:
   MutexAcquisitionImplPtr lock_async(actor::ActorImpl* issuer);
   bool try_lock(actor::ActorImpl* issuer);
   void unlock(actor::ActorImpl* issuer);
-  bool is_locked() const { return locked_; }
 
   MutexImpl* ref();
   void unref();
diff --git a/src/mc/explo/SafetyChecker.cpp b/src/mc/explo/SafetyChecker.cpp
index 07e176da34..5d173e1aa8 100644
--- a/src/mc/explo/SafetyChecker.cpp
+++ b/src/mc/explo/SafetyChecker.cpp
@@ -233,7 +233,8 @@ void SafetyChecker::backtrack()
     if (state->count_todo() && stack_.size() < (std::size_t)_sg_mc_max_depth) {
       /* We found a back-tracking point, let's loop */
       XBT_DEBUG("Back-tracking to state %ld at depth %zu", state->num_, stack_.size() + 1);
-      stack_.push_back(std::move(state));
+      stack_.push_back(
+          std::move(state)); // Put it back on the stack from which it was removed earlier in this while loop
       this->restore_state();
       XBT_DEBUG("Back-tracking to state %ld at depth %zu done", stack_.back()->num_, stack_.size());
       break;
@@ -259,7 +260,7 @@ void SafetyChecker::restore_state()
 
   /* Traverse the stack from the state at position start and re-execute the transitions */
   for (std::unique_ptr<State> const& state : stack_) {
-    if (state == stack_.back()) // If we are arrived on the target state, don't replay the outgoing transition *.
+    if (state == stack_.back()) /* If we are arrived on the target state, don't replay the outgoing transition */
       break;
     state->get_transition()->replay();
     on_transition_replay_signal(state->get_transition());
-- 
2.20.1