From d78a7f87e28a15c0f1e71e4510a055554e0e5e9b Mon Sep 17 00:00:00 2001 From: Arnaud Giersch Date: Mon, 8 Mar 2021 14:55:22 +0100 Subject: [PATCH] Modernize simcall mutex_lock. --- include/simgrid/simix.h | 2 +- src/kernel/activity/ConditionVariableImpl.cpp | 2 +- src/kernel/activity/MutexImpl.cpp | 7 ------- src/mc/api.cpp | 20 ------------------- src/mc/checker/SimcallObserver.cpp | 18 +++++++++++++++++ src/mc/checker/SimcallObserver.hpp | 11 ++++++++++ src/mc/mc_base.cpp | 11 +--------- src/s4u/s4u_Mutex.cpp | 4 +++- src/simix/libsmx.cpp | 4 ++-- src/simix/popping_accessors.hpp | 14 ------------- src/simix/popping_bodies.cpp | 7 ------- src/simix/popping_enum.hpp | 3 +-- src/simix/popping_generated.cpp | 5 ----- src/simix/simcalls.in | 2 -- 14 files changed, 38 insertions(+), 72 deletions(-) diff --git a/include/simgrid/simix.h b/include/simgrid/simix.h index f01edbf8f8..b7efc34dfd 100644 --- a/include/simgrid/simix.h +++ b/include/simgrid/simix.h @@ -199,7 +199,7 @@ XBT_ATTRIB_DEPRECATED_v330("Please use an ActivityImpl* for first parameter") in /************************** Synchro simcalls **********************************/ SG_BEGIN_DECL XBT_ATTRIB_DEPRECATED_v330("Please use sg_mutex_init()") XBT_PUBLIC smx_mutex_t simcall_mutex_init(); -XBT_PUBLIC void simcall_mutex_lock(smx_mutex_t mutex); +XBT_ATTRIB_DEPRECATED_v331("Please use sg_mutex_lock()") XBT_PUBLIC void simcall_mutex_lock(smx_mutex_t mutex); XBT_ATTRIB_DEPRECATED_v331("Please use sg_mutex_try_lock()") XBT_PUBLIC int simcall_mutex_trylock(smx_mutex_t mutex); XBT_ATTRIB_DEPRECATED_v331("Please use sg_mutex_unlock()") XBT_PUBLIC void simcall_mutex_unlock(smx_mutex_t mutex); diff --git a/src/kernel/activity/ConditionVariableImpl.cpp b/src/kernel/activity/ConditionVariableImpl.cpp index f8567e9336..12ef6d0029 100644 --- a/src/kernel/activity/ConditionVariableImpl.cpp +++ b/src/kernel/activity/ConditionVariableImpl.cpp @@ -55,7 +55,7 @@ void ConditionVariableImpl::signal() simcall_mutex = simcall_cond_wait__get__mutex(simcall); else simcall_mutex = simcall_cond_wait_timeout__get__mutex(simcall); - simcall->call_ = simix::Simcall::MUTEX_LOCK; + simcall->call_ = simix::Simcall::RUN_BLOCKING; simcall_mutex->lock(simcall->issuer_); } diff --git a/src/kernel/activity/MutexImpl.cpp b/src/kernel/activity/MutexImpl.cpp index 0b4cf38771..2930115ed9 100644 --- a/src/kernel/activity/MutexImpl.cpp +++ b/src/kernel/activity/MutexImpl.cpp @@ -108,10 +108,3 @@ void MutexImpl::unref() } // namespace activity } // namespace kernel } // namespace simgrid - -// Simcall handlers: - -void simcall_HANDLER_mutex_lock(smx_simcall_t simcall, smx_mutex_t mutex) -{ - mutex->lock(simcall->issuer_); -} diff --git a/src/mc/api.cpp b/src/mc/api.cpp index 681ec490a7..4ff1ec04dc 100644 --- a/src/mc/api.cpp +++ b/src/mc/api.cpp @@ -810,22 +810,6 @@ std::string Api::request_to_string(smx_simcall_t req, int value) const } break; - case Simcall::MUTEX_LOCK: { - type = "Mutex LOCK"; - simgrid::mc::Remote mutex; - mc_model_checker->get_remote_simulation().read_bytes(mutex.get_buffer(), sizeof(mutex), - remote(simcall_mutex_lock__get__mutex(req))); - args = "locked = " + std::to_string(mutex.get_buffer()->is_locked()) + ", owner = "; - if (mutex.get_buffer()->get_owner() != nullptr) - args += std::to_string(mc_model_checker->get_remote_simulation() - .resolve_actor(simgrid::mc::remote(mutex.get_buffer()->get_owner())) - ->get_pid()); - else - args += "-1"; - args += ", sleeping = n/a"; - break; - } - default: type = SIMIX_simcall_name(req->call_); args = "??"; @@ -902,10 +886,6 @@ std::string Api::request_get_dot_output(smx_simcall_t req, int value) const } break; - case Simcall::MUTEX_LOCK: - label = "[" + get_actor_dot_label(issuer) + "] Mutex LOCK"; - break; - default: THROW_UNIMPLEMENTED; } diff --git a/src/mc/checker/SimcallObserver.cpp b/src/mc/checker/SimcallObserver.cpp index 9c601be80c..5a384fd56c 100644 --- a/src/mc/checker/SimcallObserver.cpp +++ b/src/mc/checker/SimcallObserver.cpp @@ -71,5 +71,23 @@ std::string MutexTrylockSimcall::dot_label() const return SimcallObserver::dot_label() + "Mutex TRYLOCK"; } +std::string MutexLockSimcall::to_string(int time_considered) const +{ + std::string res = SimcallObserver::to_string(time_considered) + "Mutex LOCK"; + res += "(locked = " + std::to_string(mutex_->is_locked()); + res += ", owner = " + std::to_string(mutex_->get_owner() ? mutex_->get_owner()->get_pid() : -1); + res += ", sleeping = n/a)"; + return res; +} + +std::string MutexLockSimcall::dot_label() const +{ + return SimcallObserver::dot_label() + "Mutex LOCK"; +} + +bool MutexLockSimcall::is_enabled() const +{ + return mutex_->get_owner() == nullptr || mutex_->get_owner() == get_issuer(); +} } // namespace mc } // namespace simgrid diff --git a/src/mc/checker/SimcallObserver.hpp b/src/mc/checker/SimcallObserver.hpp index 6499887c7a..3ae882edb7 100644 --- a/src/mc/checker/SimcallObserver.hpp +++ b/src/mc/checker/SimcallObserver.hpp @@ -83,6 +83,17 @@ public: std::string dot_label() const override; kernel::activity::MutexImpl* get_mutex() const { return mutex_; } }; + +class MutexLockSimcall : public SimcallObserver { + kernel::activity::MutexImpl* mutex_; + +public: + MutexLockSimcall(smx_actor_t actor, kernel::activity::MutexImpl* mutex) : SimcallObserver(actor), mutex_(mutex) {} + bool is_enabled() const override; + std::string to_string(int times_considered) const override; + std::string dot_label() const override; + kernel::activity::MutexImpl* get_mutex() const { return mutex_; } +}; } // namespace mc } // namespace simgrid diff --git a/src/mc/mc_base.cpp b/src/mc/mc_base.cpp index f6f98ced43..d678be44ff 100644 --- a/src/mc/mc_base.cpp +++ b/src/mc/mc_base.cpp @@ -125,14 +125,6 @@ bool actor_is_enabled(smx_actor_t actor) return false; } - case Simcall::MUTEX_LOCK: { - const kernel::activity::MutexImpl* mutex = simcall_mutex_lock__get__mutex(req); - - if (mutex->get_owner() == nullptr) - return true; - return mutex->get_owner()->get_pid() == req->issuer_->get_pid(); - } - case Simcall::SEM_ACQUIRE: { static bool warned = false; if (not warned) @@ -168,8 +160,7 @@ bool request_is_visible(const s_smx_simcall* req) using simix::Simcall; return req->call_ == Simcall::COMM_ISEND || req->call_ == Simcall::COMM_IRECV || req->call_ == Simcall::COMM_WAIT || - req->call_ == Simcall::COMM_WAITANY || req->call_ == Simcall::COMM_TEST || - req->call_ == Simcall::COMM_TESTANY || req->call_ == Simcall::MUTEX_LOCK; + req->call_ == Simcall::COMM_WAITANY || req->call_ == Simcall::COMM_TEST || req->call_ == Simcall::COMM_TESTANY; } } diff --git a/src/s4u/s4u_Mutex.cpp b/src/s4u/s4u_Mutex.cpp index 38b57a6e95..8975cb7bae 100644 --- a/src/s4u/s4u_Mutex.cpp +++ b/src/s4u/s4u_Mutex.cpp @@ -21,7 +21,9 @@ Mutex::~Mutex() /** @brief Blocks the calling actor until the mutex can be obtained */ void Mutex::lock() { - simcall_mutex_lock(pimpl_); + kernel::actor::ActorImpl* issuer = kernel::actor::ActorImpl::self(); + mc::MutexLockSimcall observer{issuer, pimpl_}; + kernel::actor::simcall_blocking([&observer] { observer.get_mutex()->lock(observer.get_issuer()); }, &observer); } /** @brief Release the ownership of the mutex, unleashing a blocked actor (if any) diff --git a/src/simix/libsmx.cpp b/src/simix/libsmx.cpp index 0187a487fb..2e8c87d683 100644 --- a/src/simix/libsmx.cpp +++ b/src/simix/libsmx.cpp @@ -255,9 +255,9 @@ smx_mutex_t simcall_mutex_init() // XBT_ATTRIB_DEPRECATED_v330 * @ingroup simix_synchro_management * */ -void simcall_mutex_lock(smx_mutex_t mutex) +void simcall_mutex_lock(smx_mutex_t mutex) // XBT_ATTRIB_DEPRECATD_v331 { - simcall_BODY_mutex_lock(mutex); + mutex->mutex().lock(); } /** diff --git a/src/simix/popping_accessors.hpp b/src/simix/popping_accessors.hpp index edbfb20179..091a7cca07 100644 --- a/src/simix/popping_accessors.hpp +++ b/src/simix/popping_accessors.hpp @@ -684,19 +684,6 @@ static inline void simcall_comm_testany__set__result(smx_simcall_t simcall, int simgrid::simix::marshal(simcall->result_, result); } -static inline smx_mutex_t simcall_mutex_lock__get__mutex(smx_simcall_t simcall) -{ - return simgrid::simix::unmarshal(simcall->args_[0]); -} -static inline smx_mutex_t simcall_mutex_lock__getraw__mutex(smx_simcall_t simcall) -{ - return simgrid::simix::unmarshal_raw(simcall->args_[0]); -} -static inline void simcall_mutex_lock__set__mutex(smx_simcall_t simcall, smx_mutex_t arg) -{ - simgrid::simix::marshal(simcall->args_[0], arg); -} - static inline smx_cond_t simcall_cond_wait__get__cond(smx_simcall_t simcall) { return simgrid::simix::unmarshal(simcall->args_[0]); @@ -858,7 +845,6 @@ XBT_PRIVATE void simcall_HANDLER_comm_waitany(smx_simcall_t simcall, simgrid::ke XBT_PRIVATE void simcall_HANDLER_comm_wait(smx_simcall_t simcall, simgrid::kernel::activity::CommImpl* comm, double timeout); XBT_PRIVATE void simcall_HANDLER_comm_test(smx_simcall_t simcall, simgrid::kernel::activity::CommImpl* comm); XBT_PRIVATE void simcall_HANDLER_comm_testany(smx_simcall_t simcall, simgrid::kernel::activity::CommImpl** comms, size_t count); -XBT_PRIVATE void simcall_HANDLER_mutex_lock(smx_simcall_t simcall, smx_mutex_t mutex); XBT_PRIVATE void simcall_HANDLER_cond_wait(smx_simcall_t simcall, smx_cond_t cond, smx_mutex_t mutex); XBT_PRIVATE void simcall_HANDLER_cond_wait_timeout(smx_simcall_t simcall, smx_cond_t cond, smx_mutex_t mutex, double timeout); XBT_PRIVATE void simcall_HANDLER_sem_acquire(smx_simcall_t simcall, smx_sem_t sem); diff --git a/src/simix/popping_bodies.cpp b/src/simix/popping_bodies.cpp index ecd753707f..fc04c38c1c 100644 --- a/src/simix/popping_bodies.cpp +++ b/src/simix/popping_bodies.cpp @@ -104,13 +104,6 @@ inline static int simcall_BODY_comm_testany(simgrid::kernel::activity::CommImpl* return simcall(Simcall::COMM_TESTANY, comms, count); } -inline static void simcall_BODY_mutex_lock(smx_mutex_t mutex) -{ - if (false) /* Go to that function to follow the code flow through the simcall barrier */ - simcall_HANDLER_mutex_lock(&SIMIX_process_self()->simcall_, mutex); - return simcall(Simcall::MUTEX_LOCK, mutex); -} - inline static void simcall_BODY_cond_wait(smx_cond_t cond, smx_mutex_t mutex) { if (false) /* Go to that function to follow the code flow through the simcall barrier */ diff --git a/src/simix/popping_enum.hpp b/src/simix/popping_enum.hpp index eb82c44447..5e21f81bfb 100644 --- a/src/simix/popping_enum.hpp +++ b/src/simix/popping_enum.hpp @@ -30,7 +30,6 @@ enum class Simcall { COMM_WAIT, COMM_TEST, COMM_TESTANY, - MUTEX_LOCK, COND_WAIT, COND_WAIT_TIMEOUT, SEM_ACQUIRE, @@ -39,6 +38,6 @@ enum class Simcall { RUN_BLOCKING, }; -constexpr int NUM_SIMCALLS = 17; +constexpr int NUM_SIMCALLS = 16; } // namespace simix } // namespace simgrid diff --git a/src/simix/popping_generated.cpp b/src/simix/popping_generated.cpp index 8a2e0c2af6..8f47b97cd5 100644 --- a/src/simix/popping_generated.cpp +++ b/src/simix/popping_generated.cpp @@ -38,7 +38,6 @@ constexpr std::array simcall_names{{ "Simcall::COMM_WAIT", "Simcall::COMM_TEST", "Simcall::COMM_TESTANY", - "Simcall::MUTEX_LOCK", "Simcall::COND_WAIT", "Simcall::COND_WAIT_TIMEOUT", "Simcall::SEM_ACQUIRE", @@ -99,10 +98,6 @@ void simgrid::kernel::actor::ActorImpl::simcall_handle(int times_considered_) simcall_HANDLER_comm_testany(&simcall_, simgrid::simix::unmarshal(simcall_.args_[0]), simgrid::simix::unmarshal(simcall_.args_[1])); break; - case Simcall::MUTEX_LOCK: - simcall_HANDLER_mutex_lock(&simcall_, simgrid::simix::unmarshal(simcall_.args_[0])); - break; - case Simcall::COND_WAIT: simcall_HANDLER_cond_wait(&simcall_, simgrid::simix::unmarshal(simcall_.args_[0]), simgrid::simix::unmarshal(simcall_.args_[1])); break; diff --git a/src/simix/simcalls.in b/src/simix/simcalls.in index 052868a4ba..d83300b0b8 100644 --- a/src/simix/simcalls.in +++ b/src/simix/simcalls.in @@ -46,8 +46,6 @@ void comm_wait(simgrid::kernel::activity::CommImpl* comm, double timeo bool comm_test(simgrid::kernel::activity::CommImpl* comm) [[block]]; int comm_testany(simgrid::kernel::activity::CommImpl** comms, size_t count) [[block]]; -void mutex_lock(smx_mutex_t mutex) [[block]]; - void cond_wait(smx_cond_t cond, smx_mutex_t mutex) [[block]]; int cond_wait_timeout(smx_cond_t cond, smx_mutex_t mutex, double timeout) [[block]]; -- 2.20.1