From 82fafbab5f8ce1530037fd245d678e917119caa0 Mon Sep 17 00:00:00 2001 From: Arnaud Giersch Date: Fri, 5 Mar 2021 23:44:33 +0100 Subject: [PATCH] Modernize simcall mutex_trylock. --- include/simgrid/simix.h | 2 +- src/kernel/activity/MutexImpl.cpp | 5 ----- src/mc/api.cpp | 15 ++------------- src/mc/checker/SafetyChecker.cpp | 5 ++++- src/mc/checker/SimcallObserver.cpp | 15 +++++++++++++++ src/mc/checker/SimcallObserver.hpp | 10 ++++++++++ src/mc/mc_base.cpp | 3 +-- src/s4u/s4u_Mutex.cpp | 5 ++++- src/simix/libsmx.cpp | 4 ++-- src/simix/popping_accessors.hpp | 26 -------------------------- src/simix/popping_bodies.cpp | 7 ------- src/simix/popping_enum.hpp | 3 +-- src/simix/popping_generated.cpp | 6 ------ src/simix/simcalls.in | 1 - 14 files changed, 40 insertions(+), 67 deletions(-) diff --git a/include/simgrid/simix.h b/include/simgrid/simix.h index 46bc0cce00..f01edbf8f8 100644 --- a/include/simgrid/simix.h +++ b/include/simgrid/simix.h @@ -200,7 +200,7 @@ XBT_ATTRIB_DEPRECATED_v330("Please use an ActivityImpl* for first parameter") in 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_PUBLIC int simcall_mutex_trylock(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); XBT_ATTRIB_DEPRECATED_v330("Please use sg_cond_init()") XBT_PUBLIC smx_cond_t simcall_cond_init(); diff --git a/src/kernel/activity/MutexImpl.cpp b/src/kernel/activity/MutexImpl.cpp index a3578b9fb3..5569f8af10 100644 --- a/src/kernel/activity/MutexImpl.cpp +++ b/src/kernel/activity/MutexImpl.cpp @@ -103,8 +103,3 @@ void simcall_HANDLER_mutex_lock(smx_simcall_t simcall, smx_mutex_t mutex) { mutex->lock(simcall->issuer_); } - -int simcall_HANDLER_mutex_trylock(smx_simcall_t simcall, smx_mutex_t mutex) -{ - return mutex->try_lock(simcall->issuer_); -} diff --git a/src/mc/api.cpp b/src/mc/api.cpp index cab95844dd..a1dd5f69be 100644 --- a/src/mc/api.cpp +++ b/src/mc/api.cpp @@ -822,18 +822,11 @@ std::string Api::request_to_string(smx_simcall_t req, int value, RequestType req } break; - case Simcall::MUTEX_TRYLOCK: case Simcall::MUTEX_LOCK: { - if (req->call_ == Simcall::MUTEX_LOCK) - type = "Mutex LOCK"; - else - type = "Mutex TRYLOCK"; - + type = "Mutex LOCK"; simgrid::mc::Remote mutex; mc_model_checker->get_remote_simulation().read_bytes(mutex.get_buffer(), sizeof(mutex), - remote(req->call_ == Simcall::MUTEX_LOCK - ? simcall_mutex_lock__get__mutex(req) - : simcall_mutex_trylock__get__mutex(req))); + 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() @@ -921,10 +914,6 @@ std::string Api::request_get_dot_output(smx_simcall_t req, int value) const } break; - case Simcall::MUTEX_TRYLOCK: - label = "[" + get_actor_dot_label(issuer) + "] Mutex TRYLOCK"; - break; - case Simcall::MUTEX_LOCK: label = "[" + get_actor_dot_label(issuer) + "] Mutex LOCK"; break; diff --git a/src/mc/checker/SafetyChecker.cpp b/src/mc/checker/SafetyChecker.cpp index 0c109f94cb..c54759cd74 100644 --- a/src/mc/checker/SafetyChecker.cpp +++ b/src/mc/checker/SafetyChecker.cpp @@ -187,7 +187,10 @@ void SafetyChecker::backtrack() stack_.pop_back(); if (reductionMode_ == ReductionMode::dpor) { smx_simcall_t req = &state->internal_req_; - if (req->call_ == simix::Simcall::MUTEX_LOCK || req->call_ == simix::Simcall::MUTEX_TRYLOCK) + // FIXME: need something less ugly than this substring search + if (req->call_ == simix::Simcall::MUTEX_LOCK || + (req->observer_ && + api::get().request_to_string(req, 0, RequestType::internal).find("Mutex") != std::string::npos)) xbt_die("Mutex is currently not supported with DPOR, use --cfg=model-check/reduction:none"); const kernel::actor::ActorImpl* issuer = api::get().simcall_get_issuer(req); diff --git a/src/mc/checker/SimcallObserver.cpp b/src/mc/checker/SimcallObserver.cpp index 78a62d959f..9c601be80c 100644 --- a/src/mc/checker/SimcallObserver.cpp +++ b/src/mc/checker/SimcallObserver.cpp @@ -5,6 +5,7 @@ #include "src/mc/checker/SimcallObserver.hpp" #include "simgrid/s4u/Host.hpp" +#include "src/kernel/activity/MutexImpl.hpp" #include "src/kernel/actor/ActorImpl.hpp" XBT_LOG_NEW_DEFAULT_SUBCATEGORY(mc_observer, mc, "Logging specific to MC simcall observation"); @@ -56,5 +57,19 @@ std::string MutexUnlockSimcall::dot_label() const return SimcallObserver::dot_label() + "Mutex UNLOCK"; } +std::string MutexTrylockSimcall::to_string(int time_considered) const +{ + std::string res = SimcallObserver::to_string(time_considered) + "Mutex TRYLOCK"; + 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 MutexTrylockSimcall::dot_label() const +{ + return SimcallObserver::dot_label() + "Mutex TRYLOCK"; +} + } // namespace mc } // namespace simgrid diff --git a/src/mc/checker/SimcallObserver.hpp b/src/mc/checker/SimcallObserver.hpp index 0c6abc7589..6499887c7a 100644 --- a/src/mc/checker/SimcallObserver.hpp +++ b/src/mc/checker/SimcallObserver.hpp @@ -73,6 +73,16 @@ public: std::string to_string(int times_considered) const override; std::string dot_label() const override; }; + +class MutexTrylockSimcall : public SimcallObserver { + kernel::activity::MutexImpl* mutex_; + +public: + MutexTrylockSimcall(smx_actor_t actor, kernel::activity::MutexImpl* mutex) : SimcallObserver(actor), mutex_(mutex) {} + 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 344752bb67..f6f98ced43 100644 --- a/src/mc/mc_base.cpp +++ b/src/mc/mc_base.cpp @@ -169,8 +169,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::MUTEX_TRYLOCK; + req->call_ == Simcall::COMM_TESTANY || req->call_ == Simcall::MUTEX_LOCK; } } diff --git a/src/s4u/s4u_Mutex.cpp b/src/s4u/s4u_Mutex.cpp index d6a65f6d48..38b57a6e95 100644 --- a/src/s4u/s4u_Mutex.cpp +++ b/src/s4u/s4u_Mutex.cpp @@ -38,7 +38,10 @@ void Mutex::unlock() /** @brief Acquire the mutex if it's free, and return false (without blocking) if not */ bool Mutex::try_lock() { - return simcall_mutex_trylock(pimpl_); + kernel::actor::ActorImpl* issuer = kernel::actor::ActorImpl::self(); + mc::MutexTrylockSimcall observer{issuer, pimpl_}; + return kernel::actor::simcall([&observer] { return observer.get_mutex()->try_lock(observer.get_issuer()); }, + &observer); } /** @brief Create a new mutex diff --git a/src/simix/libsmx.cpp b/src/simix/libsmx.cpp index c061cdd4b3..0187a487fb 100644 --- a/src/simix/libsmx.cpp +++ b/src/simix/libsmx.cpp @@ -264,9 +264,9 @@ void simcall_mutex_lock(smx_mutex_t mutex) * @ingroup simix_synchro_management * */ -int simcall_mutex_trylock(smx_mutex_t mutex) +int simcall_mutex_trylock(smx_mutex_t mutex) // XBT_ATTRIB_DEPRECATD_v331 { - return simcall_BODY_mutex_trylock(mutex); + return mutex->mutex().try_lock(); } /** diff --git a/src/simix/popping_accessors.hpp b/src/simix/popping_accessors.hpp index 2d0d22aad1..edbfb20179 100644 --- a/src/simix/popping_accessors.hpp +++ b/src/simix/popping_accessors.hpp @@ -697,31 +697,6 @@ static inline void simcall_mutex_lock__set__mutex(smx_simcall_t simcall, smx_mut simgrid::simix::marshal(simcall->args_[0], arg); } -static inline smx_mutex_t simcall_mutex_trylock__get__mutex(smx_simcall_t simcall) -{ - return simgrid::simix::unmarshal(simcall->args_[0]); -} -static inline smx_mutex_t simcall_mutex_trylock__getraw__mutex(smx_simcall_t simcall) -{ - return simgrid::simix::unmarshal_raw(simcall->args_[0]); -} -static inline void simcall_mutex_trylock__set__mutex(smx_simcall_t simcall, smx_mutex_t arg) -{ - simgrid::simix::marshal(simcall->args_[0], arg); -} -static inline int simcall_mutex_trylock__get__result(smx_simcall_t simcall) -{ - return simgrid::simix::unmarshal(simcall->result_); -} -static inline int simcall_mutex_trylock__getraw__result(smx_simcall_t simcall) -{ - return simgrid::simix::unmarshal_raw(simcall->result_); -} -static inline void simcall_mutex_trylock__set__result(smx_simcall_t simcall, int result) -{ - simgrid::simix::marshal(simcall->result_, result); -} - static inline smx_cond_t simcall_cond_wait__get__cond(smx_simcall_t simcall) { return simgrid::simix::unmarshal(simcall->args_[0]); @@ -884,7 +859,6 @@ XBT_PRIVATE void simcall_HANDLER_comm_wait(smx_simcall_t simcall, simgrid::kerne 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 int simcall_HANDLER_mutex_trylock(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 cc9f47e870..ecd753707f 100644 --- a/src/simix/popping_bodies.cpp +++ b/src/simix/popping_bodies.cpp @@ -111,13 +111,6 @@ inline static void simcall_BODY_mutex_lock(smx_mutex_t mutex) return simcall(Simcall::MUTEX_LOCK, mutex); } -inline static int simcall_BODY_mutex_trylock(smx_mutex_t mutex) -{ - if (false) /* Go to that function to follow the code flow through the simcall barrier */ - simcall_HANDLER_mutex_trylock(&SIMIX_process_self()->simcall_, mutex); - return simcall(Simcall::MUTEX_TRYLOCK, 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 05b9c67c24..eb82c44447 100644 --- a/src/simix/popping_enum.hpp +++ b/src/simix/popping_enum.hpp @@ -31,7 +31,6 @@ enum class Simcall { COMM_TEST, COMM_TESTANY, MUTEX_LOCK, - MUTEX_TRYLOCK, COND_WAIT, COND_WAIT_TIMEOUT, SEM_ACQUIRE, @@ -40,6 +39,6 @@ enum class Simcall { RUN_BLOCKING, }; -constexpr int NUM_SIMCALLS = 18; +constexpr int NUM_SIMCALLS = 17; } // namespace simix } // namespace simgrid diff --git a/src/simix/popping_generated.cpp b/src/simix/popping_generated.cpp index 550131a2e7..8a2e0c2af6 100644 --- a/src/simix/popping_generated.cpp +++ b/src/simix/popping_generated.cpp @@ -39,7 +39,6 @@ constexpr std::array simcall_names{{ "Simcall::COMM_TEST", "Simcall::COMM_TESTANY", "Simcall::MUTEX_LOCK", - "Simcall::MUTEX_TRYLOCK", "Simcall::COND_WAIT", "Simcall::COND_WAIT_TIMEOUT", "Simcall::SEM_ACQUIRE", @@ -104,11 +103,6 @@ void simgrid::kernel::actor::ActorImpl::simcall_handle(int times_considered_) simcall_HANDLER_mutex_lock(&simcall_, simgrid::simix::unmarshal(simcall_.args_[0])); break; - case Simcall::MUTEX_TRYLOCK: - simgrid::simix::marshal(simcall_.result_, simcall_HANDLER_mutex_trylock(&simcall_, simgrid::simix::unmarshal(simcall_.args_[0]))); - simcall_answer(); - 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 8d296c2495..052868a4ba 100644 --- a/src/simix/simcalls.in +++ b/src/simix/simcalls.in @@ -47,7 +47,6 @@ 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]]; -int mutex_trylock(smx_mutex_t mutex); 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