From 62c07d4306a42be1ed34494a37603bb6dd27a98b Mon Sep 17 00:00:00 2001 From: Arnaud Giersch Date: Sat, 13 Mar 2021 22:25:04 +0100 Subject: [PATCH] Modernize simcall cond_wait. --- include/simgrid/simix.h | 3 ++- src/kernel/activity/ConditionVariableImpl.cpp | 17 ++++++------ src/mc/checker/SimcallObserver.cpp | 20 ++++++++++++++ src/mc/checker/SimcallObserver.hpp | 18 +++++++++++++ src/mc/mc_base.cpp | 8 ------ src/s4u/s4u_ConditionVariable.cpp | 11 ++++++-- 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 | 5 ---- src/simix/simcalls.in | 1 - 12 files changed, 60 insertions(+), 63 deletions(-) diff --git a/include/simgrid/simix.h b/include/simgrid/simix.h index 87bdc50b20..ab286fd862 100644 --- a/include/simgrid/simix.h +++ b/include/simgrid/simix.h @@ -204,7 +204,8 @@ XBT_ATTRIB_DEPRECATED_v331("Please use sg_mutex_try_lock()") XBT_PUBLIC int simc 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(); -XBT_PUBLIC void simcall_cond_wait(smx_cond_t cond, smx_mutex_t mutex); +XBT_ATTRIB_DEPRECATED_v331("Please use sg_cond_wait()") XBT_PUBLIC + void simcall_cond_wait(smx_cond_t cond, smx_mutex_t mutex); XBT_PUBLIC int simcall_cond_wait_timeout(smx_cond_t cond, smx_mutex_t mutex, double max_duration); XBT_ATTRIB_DEPRECATED_v331("Please use sg_sem_acquire()") XBT_PUBLIC void simcall_sem_acquire(smx_sem_t sem); diff --git a/src/kernel/activity/ConditionVariableImpl.cpp b/src/kernel/activity/ConditionVariableImpl.cpp index 12ef6d0029..bb15f64a3a 100644 --- a/src/kernel/activity/ConditionVariableImpl.cpp +++ b/src/kernel/activity/ConditionVariableImpl.cpp @@ -7,17 +7,12 @@ #include "simgrid/Exception.hpp" #include "src/kernel/activity/MutexImpl.hpp" #include "src/kernel/activity/SynchroRaw.hpp" +#include "src/mc/checker/SimcallObserver.hpp" XBT_LOG_NEW_DEFAULT_SUBCATEGORY(simix_condition, simix_synchro, "Condition variables"); /********************************* Condition **********************************/ -/** @brief Handle a condition waiting simcall without timeouts */ -void simcall_HANDLER_cond_wait(smx_simcall_t simcall, smx_cond_t cond, smx_mutex_t mutex) -{ - cond->wait(mutex, -1, simcall->issuer_); -} - /** @brief Handle a condition waiting simcall with timeouts */ void simcall_HANDLER_cond_wait_timeout(smx_simcall_t simcall, smx_cond_t cond, smx_mutex_t mutex, double timeout) { @@ -51,10 +46,14 @@ void ConditionVariableImpl::signal() /* Now transform the cond wait simcall into a mutex lock one */ smx_simcall_t simcall = &proc.simcall_; MutexImpl* simcall_mutex; - if (simcall->call_ == simix::Simcall::COND_WAIT) - simcall_mutex = simcall_cond_wait__get__mutex(simcall); - else + if (simcall->call_ == simix::Simcall::COND_WAIT_TIMEOUT) simcall_mutex = simcall_cond_wait_timeout__get__mutex(simcall); + else { + // FIXME? using here the MC observer to solve a problem not related to MC + auto* observer = dynamic_cast(simcall->observer_); + xbt_assert(observer != nullptr); + simcall_mutex = observer->get_mutex(); + } simcall->call_ = simix::Simcall::RUN_BLOCKING; simcall_mutex->lock(simcall->issuer_); diff --git a/src/mc/checker/SimcallObserver.cpp b/src/mc/checker/SimcallObserver.cpp index 3dd9d0e747..297e84f5bb 100644 --- a/src/mc/checker/SimcallObserver.cpp +++ b/src/mc/checker/SimcallObserver.cpp @@ -76,6 +76,26 @@ bool MutexLockSimcall::is_enabled() const return not blocking_ || mutex_->get_owner() == nullptr || mutex_->get_owner() == get_issuer(); } +std::string ConditionWaitSimcall::to_string(int time_considered) const +{ + return SimcallObserver::to_string(time_considered) + "Condition WAIT"; +} + +std::string ConditionWaitSimcall::dot_label() const +{ + return SimcallObserver::dot_label() + "Condition WAIT"; +} + +bool ConditionWaitSimcall::is_enabled() const +{ + static bool warned = false; + if (not warned) { + XBT_INFO("Using condition variables in model-checked code is still experimental. Use at your own risk"); + warned = true; + } + return true; +} + std::string SemAcquireSimcall::to_string(int time_considered) const { return SimcallObserver::to_string(time_considered) + "Sem ACQUIRE"; diff --git a/src/mc/checker/SimcallObserver.hpp b/src/mc/checker/SimcallObserver.hpp index 2dec9072cd..f27e55a911 100644 --- a/src/mc/checker/SimcallObserver.hpp +++ b/src/mc/checker/SimcallObserver.hpp @@ -89,6 +89,24 @@ public: kernel::activity::MutexImpl* get_mutex() const { return mutex_; } }; +class ConditionWaitSimcall : public SimcallObserver { + kernel::activity::ConditionVariableImpl* const cond_; + kernel::activity::MutexImpl* const mutex_; + +public: + ConditionWaitSimcall(smx_actor_t actor, kernel::activity::ConditionVariableImpl* cond, + kernel::activity::MutexImpl* mutex) + : SimcallObserver(actor), cond_(cond), mutex_(mutex) + { + } + bool is_enabled() const override; + bool is_visible() const override { return false; } + std::string to_string(int times_considered) const override; + std::string dot_label() const override; + kernel::activity::ConditionVariableImpl* get_cond() const { return cond_; } + kernel::activity::MutexImpl* get_mutex() const { return mutex_; } +}; + class SemAcquireSimcall : public SimcallObserver { kernel::activity::SemaphoreImpl* const sem_; diff --git a/src/mc/mc_base.cpp b/src/mc/mc_base.cpp index 546b67e06b..21c2e8cdaf 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::COND_WAIT: { - static bool warned = false; - if (not warned) - XBT_INFO("Using condition variables in model-checked code is still experimental. Use at your own risk"); - warned = true; - return true; - } - default: /* The rest of the requests are always enabled */ return true; diff --git a/src/s4u/s4u_ConditionVariable.cpp b/src/s4u/s4u_ConditionVariable.cpp index 102d683ffb..e43d0e9ff3 100644 --- a/src/s4u/s4u_ConditionVariable.cpp +++ b/src/s4u/s4u_ConditionVariable.cpp @@ -8,6 +8,7 @@ #include "simgrid/s4u/ConditionVariable.hpp" #include "simgrid/simix.h" #include "src/kernel/activity/ConditionVariableImpl.hpp" +#include "src/mc/checker/SimcallObserver.hpp" #include "xbt/log.hpp" #include @@ -28,12 +29,18 @@ ConditionVariablePtr ConditionVariable::create() */ void ConditionVariable::wait(MutexPtr lock) { - simcall_cond_wait(pimpl_, lock->pimpl_); + kernel::actor::ActorImpl* issuer = kernel::actor::ActorImpl::self(); + mc::ConditionWaitSimcall observer{issuer, pimpl_, lock->pimpl_}; + kernel::actor::simcall_blocking( + [&observer] { observer.get_cond()->wait(observer.get_mutex(), -1.0, observer.get_issuer()); }, &observer); } void ConditionVariable::wait(const std::unique_lock& lock) { - simcall_cond_wait(pimpl_, lock.mutex()->pimpl_); + kernel::actor::ActorImpl* issuer = kernel::actor::ActorImpl::self(); + mc::ConditionWaitSimcall observer{issuer, pimpl_, lock.mutex()->pimpl_}; + kernel::actor::simcall_blocking( + [&observer] { observer.get_cond()->wait(observer.get_mutex(), -1.0, observer.get_issuer()); }, &observer); } std::cv_status s4u::ConditionVariable::wait_for(const std::unique_lock& lock, double timeout) diff --git a/src/simix/libsmx.cpp b/src/simix/libsmx.cpp index cd8b9dfbdd..b203c72e4c 100644 --- a/src/simix/libsmx.cpp +++ b/src/simix/libsmx.cpp @@ -292,9 +292,9 @@ smx_cond_t simcall_cond_init() // XBT_ATTRIB_DEPRECATED_v330 * @ingroup simix_synchro_management * */ -void simcall_cond_wait(smx_cond_t cond, smx_mutex_t mutex) +void simcall_cond_wait(smx_cond_t cond, smx_mutex_t mutex) // XBT_ATTRIB_DEPRECATED_v331 { - simcall_BODY_cond_wait(cond, mutex); + cond->get_iface()->wait(std::unique_lock(mutex->mutex())); } /** diff --git a/src/simix/popping_accessors.hpp b/src/simix/popping_accessors.hpp index 6ee3f889fd..de128f15b1 100644 --- a/src/simix/popping_accessors.hpp +++ b/src/simix/popping_accessors.hpp @@ -684,31 +684,6 @@ static inline void simcall_comm_testany__set__result(smx_simcall_t simcall, int 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]); -} -static inline smx_cond_t simcall_cond_wait__getraw__cond(smx_simcall_t simcall) -{ - return simgrid::simix::unmarshal_raw(simcall->args_[0]); -} -static inline void simcall_cond_wait__set__cond(smx_simcall_t simcall, smx_cond_t arg) -{ - simgrid::simix::marshal(simcall->args_[0], arg); -} -static inline smx_mutex_t simcall_cond_wait__get__mutex(smx_simcall_t simcall) -{ - return simgrid::simix::unmarshal(simcall->args_[1]); -} -static inline smx_mutex_t simcall_cond_wait__getraw__mutex(smx_simcall_t simcall) -{ - return simgrid::simix::unmarshal_raw(simcall->args_[1]); -} -static inline void simcall_cond_wait__set__mutex(smx_simcall_t simcall, smx_mutex_t arg) -{ - simgrid::simix::marshal(simcall->args_[1], arg); -} - static inline smx_cond_t simcall_cond_wait_timeout__get__cond(smx_simcall_t simcall) { return simgrid::simix::unmarshal(simcall->args_[0]); @@ -832,6 +807,5 @@ 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_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_timeout(smx_simcall_t simcall, smx_sem_t sem, double timeout); diff --git a/src/simix/popping_bodies.cpp b/src/simix/popping_bodies.cpp index 9f69d93511..a9f46ba40f 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_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 */ - simcall_HANDLER_cond_wait(&SIMIX_process_self()->simcall_, cond, mutex); - return simcall(Simcall::COND_WAIT, cond, mutex); -} - inline static int simcall_BODY_cond_wait_timeout(smx_cond_t cond, smx_mutex_t mutex, double timeout) { 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 050d04c7e4..f49f4ba409 100644 --- a/src/simix/popping_enum.hpp +++ b/src/simix/popping_enum.hpp @@ -30,13 +30,12 @@ enum class Simcall { COMM_WAIT, COMM_TEST, COMM_TESTANY, - COND_WAIT, COND_WAIT_TIMEOUT, SEM_ACQUIRE_TIMEOUT, RUN_KERNEL, RUN_BLOCKING, }; -constexpr int NUM_SIMCALLS = 15; +constexpr int NUM_SIMCALLS = 14; } // namespace simix } // namespace simgrid diff --git a/src/simix/popping_generated.cpp b/src/simix/popping_generated.cpp index b3f1b8ae50..8d75097c26 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::COND_WAIT", "Simcall::COND_WAIT_TIMEOUT", "Simcall::SEM_ACQUIRE_TIMEOUT", "Simcall::RUN_KERNEL", @@ -97,10 +96,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::COND_WAIT: - simcall_HANDLER_cond_wait(&simcall_, simgrid::simix::unmarshal(simcall_.args_[0]), simgrid::simix::unmarshal(simcall_.args_[1])); - break; - case Simcall::COND_WAIT_TIMEOUT: simcall_HANDLER_cond_wait_timeout(&simcall_, simgrid::simix::unmarshal(simcall_.args_[0]), simgrid::simix::unmarshal(simcall_.args_[1]), simgrid::simix::unmarshal(simcall_.args_[2])); break; diff --git a/src/simix/simcalls.in b/src/simix/simcalls.in index 6ea13d2f23..a206d29d2d 100644 --- a/src/simix/simcalls.in +++ b/src/simix/simcalls.in @@ -46,7 +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 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]]; int sem_acquire_timeout(smx_sem_t sem, double timeout) [[block]]; -- 2.20.1