From b35d70d6818026204833410d2e3917252fa35127 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Mon, 4 Jan 2016 11:31:42 +0100 Subject: [PATCH] Simplify the code taking Cpu::m_speedPeak changes into account The DVFS change set failed to do the right thing and instead added the current speed as a bound to any execution action (in addition, this code was in simix while this performance modeling clearly belongs to surf). I guess that this was not playing well with availability changes due to a trace, since that extra bound was not recomputed correctly. Instead, factorize the code of trace availability events with the m_speedPeak changes created by DVFS. This creates a new Cpu event called onSpeedChange, which is still to be exported properly. For now, only the tracing code listen to that event (it's hardcoded). --- src/simix/smx_host.cpp | 11 +---------- src/surf/cpu_cas01.cpp | 33 ++++++++++++++++++++------------- src/surf/cpu_cas01.hpp | 6 +++++- src/surf/cpu_interface.cpp | 8 ++++++++ src/surf/cpu_interface.hpp | 5 +++++ 5 files changed, 39 insertions(+), 24 deletions(-) diff --git a/src/simix/smx_host.cpp b/src/simix/smx_host.cpp index 92b2b5b17d..5139c62b61 100644 --- a/src/simix/smx_host.cpp +++ b/src/simix/smx_host.cpp @@ -322,16 +322,7 @@ smx_synchro_t SIMIX_process_execute(smx_process_t issuer, const char *name, surf_action_set_data(synchro->execution.surf_exec, synchro); surf_action_set_priority(synchro->execution.surf_exec, priority); - /* Note (hypervisor): for multicore, the bound value being passed to the - * surf layer should not be zero (i.e., unlimited). It should be the - * capacity of a CPU core. - * - * FIXME: this should probably not be part of Simix but of Surf directly. - * That bound is part of the performance model, not of the synchronization - */ - if (bound == 0) - surf_cpu_action_set_bound(synchro->execution.surf_exec, sg_host_get_speed(issuer->host)); - else + if (bound != 0) surf_cpu_action_set_bound(synchro->execution.surf_exec, bound); if (affinity_mask != 0) { diff --git a/src/surf/cpu_cas01.cpp b/src/surf/cpu_cas01.cpp index d38b452506..d2fb281be5 100644 --- a/src/surf/cpu_cas01.cpp +++ b/src/surf/cpu_cas01.cpp @@ -182,6 +182,25 @@ bool CpuCas01::isUsed() return lmm_constraint_used(getModel()->getMaxminSystem(), getConstraint()); } +/** @brief take into account changes of speed (either load or max) */ +void CpuCas01::onSpeedChange() { + lmm_variable_t var = NULL; + lmm_element_t elem = NULL; + + lmm_update_constraint_bound(getModel()->getMaxminSystem(), getConstraint(), + m_core * m_speedScale * m_speedPeak); + while ((var = lmm_get_var_from_cnst + (getModel()->getMaxminSystem(), getConstraint(), &elem))) { + CpuCas01Action *action = static_cast(lmm_variable_id(var)); + + lmm_update_variable_bound(getModel()->getMaxminSystem(), + action->getVariable(), + m_speedScale * m_speedPeak); + } + + Cpu::onSpeedChange(); +} + void CpuCas01::updateState(tmgr_trace_event_t event_type, double value, double date) { lmm_variable_t var = NULL; @@ -192,20 +211,8 @@ void CpuCas01::updateState(tmgr_trace_event_t event_type, double value, double d xbt_assert(m_core == 1, "FIXME: add speed scaling code also for constraint_core[i]"); m_speedScale = value; - lmm_update_constraint_bound(getModel()->getMaxminSystem(), getConstraint(), - m_core * m_speedScale * - m_speedPeak); - TRACE_surf_host_set_speed(date, getName(), - m_core * m_speedScale * - m_speedPeak); - while ((var = lmm_get_var_from_cnst - (getModel()->getMaxminSystem(), getConstraint(), &elem))) { - CpuCas01Action *action = static_cast(lmm_variable_id(var)); + onSpeedChange(); - lmm_update_variable_bound(getModel()->getMaxminSystem(), - action->getVariable(), - m_speedScale * m_speedPeak); - } if (tmgr_trace_event_free(event_type)) p_speedEvent = NULL; } else if (event_type == p_stateEvent) { diff --git a/src/surf/cpu_cas01.hpp b/src/surf/cpu_cas01.hpp index d3e1275d2a..c825a70015 100644 --- a/src/surf/cpu_cas01.hpp +++ b/src/surf/cpu_cas01.hpp @@ -55,7 +55,11 @@ public: void setStateEvent(tmgr_trace_event_t stateEvent); void setPowerEvent(tmgr_trace_event_t stateEvent); - xbt_dynar_t getSpeedPeakList(); + xbt_dynar_t getSpeedPeakList(); // FIXME: killme to hide our internals + +protected: + void onSpeedChange() override; + private: tmgr_trace_event_t p_stateEvent; diff --git a/src/surf/cpu_interface.cpp b/src/surf/cpu_interface.cpp index 232e3811c3..374db5c3f7 100644 --- a/src/surf/cpu_interface.cpp +++ b/src/surf/cpu_interface.cpp @@ -210,6 +210,8 @@ void Cpu::setPState(int pstate_index) double new_peak_speed = xbt_dynar_get_as(plist, pstate_index, double); m_pstate = pstate_index; m_speedPeak = new_peak_speed; + + onSpeedChange(); } int Cpu::getPState() @@ -236,6 +238,12 @@ double Cpu::getAvailableSpeed() return m_speedScale; } +void Cpu::onSpeedChange() { + TRACE_surf_host_set_speed(surf_get_clock(), getName(), + m_core * m_speedScale * m_speedPeak); +} + + int Cpu::getCore() { return m_core; diff --git a/src/surf/cpu_interface.hpp b/src/surf/cpu_interface.hpp index 147d3bd6bb..0ab1e484c8 100644 --- a/src/surf/cpu_interface.hpp +++ b/src/surf/cpu_interface.hpp @@ -129,6 +129,11 @@ public: /** @brief Get the speed, accounting for the trace load and provided process load instead of the real current one */ virtual double getSpeed(double load); +protected: + /** @brief Take speed changes (either load or max) into account */ + virtual void onSpeedChange(); + +public: /** @brief Get the available speed of the current Cpu */ virtual double getAvailableSpeed(); -- 2.20.1