Make smpi_switch_data_segment check if a switch is needed, and return true when it...

Kill global SMPI_switch_data_segment.

diff --git a/include/smpi/smpi.h b/include/smpi/smpi.h
index f9cba1c..69d1169 100644 (file)
--- a/include/smpi/smpi.h
+++ b/include/smpi/smpi.h
@@ -1141,7 +1141,6 @@ XBT_PUBLIC MPI_Comm smpi_process_comm_self();
 XBT_PUBLIC MPI_Info smpi_process_info_env();
 XBT_PUBLIC void* smpi_process_get_user_data();
 XBT_PUBLIC void smpi_process_set_user_data(void*);
-XBT_PUBLIC void smpi_init_options();
 
 XBT_PUBLIC void smpi_execute_flops(double flops);
 XBT_PUBLIC void smpi_execute_flops_benched(double flops);

diff --git a/src/kernel/actor/ActorImpl.cpp b/src/kernel/actor/ActorImpl.cpp
index 78f5be7..c70843f 100644 (file)
--- a/src/kernel/actor/ActorImpl.cpp
+++ b/src/kernel/actor/ActorImpl.cpp
@@ -3,9 +3,9 @@
 /* This program is free software; you can redistribute it and/or modify it
  * under the terms of the license (GNU LGPL) which comes with this package. */
 
+#include "simgrid/s4u/Actor.hpp"
 #include "mc/mc.h"
 #include "simgrid/Exception.hpp"
-#include "simgrid/s4u/Actor.hpp"
 #include "simgrid/s4u/Exec.hpp"
 #include "src/kernel/EngineImpl.hpp"
 #include "src/kernel/activity/CommImpl.hpp"
@@ -16,6 +16,7 @@
 #include "src/mc/mc_replay.hpp"
 #include "src/mc/remote/AppSide.hpp"
 #include "src/simix/smx_private.hpp"
+#include "src/smpi/include/private.hpp"
 #include "src/surf/HostImpl.hpp"
 #include "src/surf/cpu_interface.hpp"
 
@@ -307,9 +308,8 @@ void ActorImpl::yield()
     }
   }
 
-  if (SMPI_switch_data_segment && not finished_) {
-    SMPI_switch_data_segment(get_iface());
-  }
+  if (not finished_)
+    smpi_switch_data_segment(get_iface());
 }
 
 /** This actor will be terminated automatically when the last non-daemon actor finishes */

diff --git a/src/kernel/actor/ActorImpl.hpp b/src/kernel/actor/ActorImpl.hpp
index 289e3ae..b8fc8ac 100644 (file)
--- a/src/kernel/actor/ActorImpl.hpp
+++ b/src/kernel/actor/ActorImpl.hpp
@@ -205,6 +205,4 @@ XBT_PUBLIC unsigned long* get_maxpid_addr(); // In MC mode, the application send
 } // namespace kernel
 } // namespace simgrid
 
-extern void (*SMPI_switch_data_segment)(simgrid::s4u::ActorPtr actor);
-
 #endif

diff --git a/src/simix/smx_global.cpp b/src/simix/smx_global.cpp
index 4bc14b5..21a2e63 100644 (file)
--- a/src/simix/smx_global.cpp
+++ b/src/simix/smx_global.cpp
@@ -29,8 +29,6 @@ XBT_LOG_NEW_DEFAULT_SUBCATEGORY(simix_kernel, simix, "Logging specific to SIMIX
 
 std::unique_ptr<simgrid::simix::Global> simix_global;
 
-void (*SMPI_switch_data_segment)(simgrid::s4u::ActorPtr) = nullptr;
-
 namespace simgrid {
 namespace simix {
 config::Flag<bool> cfg_verbose_exit{"debug/verbose-exit", "Display the actor status at exit", true};

diff --git a/src/smpi/include/private.hpp b/src/smpi/include/private.hpp
index 4e56553..8c5eb13 100644 (file)
--- a/src/smpi/include/private.hpp
+++ b/src/smpi/include/private.hpp
@@ -115,10 +115,12 @@ XBT_PRIVATE double smpi_cfg_auto_shared_malloc_thresh();
 XBT_PRIVATE bool smpi_cfg_display_alloc();
 
 // utilities
+XBT_PUBLIC void smpi_init_options(bool called_by_smpimain = false);
+
 extern XBT_PRIVATE char* smpi_data_exe_start; // start of the data+bss segment of the executable
 extern XBT_PRIVATE size_t smpi_data_exe_size; // size of the data+bss segment of the executable
 
-XBT_PRIVATE void smpi_switch_data_segment(simgrid::s4u::ActorPtr actor);
+XBT_PRIVATE bool smpi_switch_data_segment(simgrid::s4u::ActorPtr actor, const void* addr = nullptr);
 
 XBT_PRIVATE void smpi_prepare_global_memory_segment();
 XBT_PRIVATE void smpi_backup_global_memory_segment();

diff --git a/src/smpi/internals/smpi_actor.cpp b/src/smpi/internals/smpi_actor.cpp
index bdb72ce..616c8a0 100644 (file)
--- a/src/smpi/internals/smpi_actor.cpp
+++ b/src/smpi/internals/smpi_actor.cpp
@@ -217,7 +217,7 @@ void ActorExt::init()
                                             "Aborting, please check compilation process and use smpirun.");
 
   simgrid::s4u::Actor* self = simgrid::s4u::Actor::self();
-  // cheinrich: I'm not sure what the impact of the SMPI_switch_data_segment on this call is. I moved
+  // cheinrich: I'm not sure what the impact of the smpi_switch_data_segment on this call is. I moved
   // this up here so that I can set the privatized region before the switch.
   ActorExt* ext = smpi_process();
   // if we are in MPI_Init and argc handling has already been done.
@@ -228,7 +228,7 @@ void ActorExt::init()
     /* Now using the segment index of this process  */
     ext->set_privatized_region(smpi_init_global_memory_segment_process());
     /* Done at the process's creation */
-    SMPI_switch_data_segment(self);
+    smpi_switch_data_segment(self);
   }
 
   ext->instance_id_ = self->get_property("instance_id");

diff --git a/src/smpi/internals/smpi_bench.cpp b/src/smpi/internals/smpi_bench.cpp
index a213c7f..86b297f 100644 (file)
--- a/src/smpi/internals/smpi_bench.cpp
+++ b/src/smpi/internals/smpi_bench.cpp
@@ -78,9 +78,7 @@ void smpi_execute_flops_benched(double flops) {
 
 void smpi_bench_begin()
 {
-  if (smpi_cfg_privatization() == SmpiPrivStrategies::MMAP) {
-    smpi_switch_data_segment(simgrid::s4u::Actor::self());
-  }
+  smpi_switch_data_segment(simgrid::s4u::Actor::self());
 
   if (MC_is_active() || MC_record_replay_is_active())
     return;

diff --git a/src/smpi/internals/smpi_config.cpp b/src/smpi/internals/smpi_config.cpp
index 7d640a1..9a4fe2e 100644 (file)
--- a/src/smpi/internals/smpi_config.cpp
+++ b/src/smpi/internals/smpi_config.cpp
@@ -187,7 +187,8 @@ double smpi_cfg_auto_shared_malloc_thresh(){
   return _smpi_cfg_auto_shared_malloc_thresh;
 }
 
-void smpi_init_options(){
+void smpi_init_options(bool called_by_smpimain)
+{
   // return if already called
   if(_smpi_options_initialized)
     return;
@@ -212,30 +213,29 @@ void smpi_init_options(){
   if (default_privatization == nullptr)
     default_privatization = "no";
 
-
-  simgrid::config::declare_flag<std::string>( "smpi/privatization", 
-    "How we should privatize global variable at runtime (no, yes, mmap, dlopen).",
-    default_privatization, [](const std::string& smpi_privatize_option){
-      if (smpi_privatize_option == "no" || smpi_privatize_option == "0")
-        _smpi_cfg_privatization = SmpiPrivStrategies::NONE;
-      else if (smpi_privatize_option == "yes" || smpi_privatize_option == "1")
-        _smpi_cfg_privatization = SmpiPrivStrategies::DEFAULT;
-      else if (smpi_privatize_option == "mmap")
-        _smpi_cfg_privatization = SmpiPrivStrategies::MMAP;
-      else if (smpi_privatize_option == "dlopen")
-        _smpi_cfg_privatization = SmpiPrivStrategies::DLOPEN;
-      else
-        xbt_die("Invalid value for smpi/privatization: '%s'", smpi_privatize_option.c_str());
-        
-      if (not SMPI_switch_data_segment) {
-        XBT_DEBUG("Running without smpi_main(); disable smpi/privatization.");
-        _smpi_cfg_privatization = SmpiPrivStrategies::NONE;
-      }
-      if (not HAVE_WORKING_MMAP && _smpi_cfg_privatization == SmpiPrivStrategies::MMAP) {
-        XBT_INFO("mmap privatization is broken on this platform, switching to dlopen privatization instead.");
-        _smpi_cfg_privatization = SmpiPrivStrategies::DLOPEN;
-      }
-    });
+  simgrid::config::declare_flag<std::string>(
+      "smpi/privatization", "How we should privatize global variable at runtime (no, yes, mmap, dlopen).",
+      default_privatization, [called_by_smpimain](const std::string& smpi_privatize_option) {
+        if (smpi_privatize_option == "no" || smpi_privatize_option == "0")
+          _smpi_cfg_privatization = SmpiPrivStrategies::NONE;
+        else if (smpi_privatize_option == "yes" || smpi_privatize_option == "1")
+          _smpi_cfg_privatization = SmpiPrivStrategies::DEFAULT;
+        else if (smpi_privatize_option == "mmap")
+          _smpi_cfg_privatization = SmpiPrivStrategies::MMAP;
+        else if (smpi_privatize_option == "dlopen")
+          _smpi_cfg_privatization = SmpiPrivStrategies::DLOPEN;
+        else
+          xbt_die("Invalid value for smpi/privatization: '%s'", smpi_privatize_option.c_str());
+
+        if (not called_by_smpimain) {
+          XBT_DEBUG("Running without smpi_main(); disable smpi/privatization.");
+          _smpi_cfg_privatization = SmpiPrivStrategies::NONE;
+        }
+        if (not HAVE_WORKING_MMAP && _smpi_cfg_privatization == SmpiPrivStrategies::MMAP) {
+          XBT_INFO("mmap privatization is broken on this platform, switching to dlopen privatization instead.");
+          _smpi_cfg_privatization = SmpiPrivStrategies::DLOPEN;
+        }
+      });
 
   simgrid::config::declare_flag<std::string>("smpi/privatize-libs", 
                                             "Add libraries (; separated) to privatize (libgfortran for example)."

diff --git a/src/smpi/internals/smpi_global.cpp b/src/smpi/internals/smpi_global.cpp
index ba06a81..7fbb861 100644 (file)
--- a/src/smpi/internals/smpi_global.cpp
+++ b/src/smpi/internals/smpi_global.cpp
@@ -184,21 +184,15 @@ void smpi_comm_copy_buffer_callback(simgrid::kernel::activity::CommImpl* comm, v
   auto private_blocks = merge_private_blocks(src_private_blocks, dst_private_blocks);
   check_blocks(private_blocks, buff_size);
   void* tmpbuff=buff;
-  if ((smpi_cfg_privatization() == SmpiPrivStrategies::MMAP) &&
-      (static_cast<char*>(buff) >= smpi_data_exe_start) &&
-      (static_cast<char*>(buff) < smpi_data_exe_start + smpi_data_exe_size)) {
+  if (smpi_switch_data_segment(comm->src_actor_->get_iface(), buff)) {
     XBT_DEBUG("Privatization : We are copying from a zone inside global memory... Saving data to temp buffer !");
-    smpi_switch_data_segment(comm->src_actor_->get_iface());
     tmpbuff = xbt_malloc(buff_size);
     memcpy_private(tmpbuff, buff, private_blocks);
   }
 
-  if ((smpi_cfg_privatization() == SmpiPrivStrategies::MMAP) &&
-      ((char*)comm->dst_buff_ >= smpi_data_exe_start) &&
-      ((char*)comm->dst_buff_ < smpi_data_exe_start + smpi_data_exe_size)) {
+  if (smpi_switch_data_segment(comm->dst_actor_->get_iface(), comm->dst_buff_))
     XBT_DEBUG("Privatization : We are copying to a zone inside global memory - Switch data segment");
-    smpi_switch_data_segment(comm->dst_actor_->get_iface());
-  }
+
   XBT_DEBUG("Copying %zu bytes from %p to %p", buff_size, tmpbuff, comm->dst_buff_);
   memcpy_private(comm->dst_buff_, tmpbuff, private_blocks);
 
@@ -524,9 +518,8 @@ int smpi_main(const char* executable, int argc, char* argv[])
      * configuration tools */
     return 0;
   }
-  
-  SMPI_switch_data_segment = &smpi_switch_data_segment;
-  smpi_init_options();
+
+  smpi_init_options(true);
   simgrid::instr::init();
   SIMIX_global_init(&argc, argv);
 

diff --git a/src/smpi/internals/smpi_memory.cpp b/src/smpi/internals/smpi_memory.cpp
index 991377a..40bd1e4 100644 (file)
--- a/src/smpi/internals/smpi_memory.cpp
+++ b/src/smpi/internals/smpi_memory.cpp
@@ -175,16 +175,25 @@ void* smpi_temp_shm_mmap(int fd, size_t size)
 /** Map a given SMPI privatization segment (make an SMPI process active)
  *
  *  When doing a state restoration, the state of the restored variables  might not be consistent with the state of the
- *  virtual memory. In this case, we to change the data segment.
+ *  virtual memory. In this case, we have to change the data segment.
+ *
+ *  If 'addr' is not null, only switch if it's an address from the data segment.
+ *
+ *  Returns 'true' if the segment has to be switched (mmap privatization and 'addr' in data segment).
  */
-void smpi_switch_data_segment(simgrid::s4u::ActorPtr actor)
+bool smpi_switch_data_segment(simgrid::s4u::ActorPtr actor, const void* addr)
 {
+  if (smpi_cfg_privatization() != SmpiPrivStrategies::MMAP || smpi_data_exe_size == 0)
+    return false; // no need to switch
+
+  if (addr != nullptr &&
+      not(static_cast<const char*>(addr) >= smpi_data_exe_start &&
+          static_cast<const char*>(addr) < smpi_data_exe_start + smpi_data_exe_size))
+    return false; // no need to switch, addr is not concerned
+
   static aid_t smpi_loaded_page = -1;
   if (smpi_loaded_page == actor->get_pid()) // no need to switch, we've already loaded the one we want
-    return;
-
-  if (smpi_data_exe_size == 0) // no need to switch
-    return;
+    return true;                            // return 'true' anyway
 
 #if HAVE_PRIVATIZATION
   // FIXME, cross-process support (mmap across process when necessary)
@@ -196,6 +205,8 @@ void smpi_switch_data_segment(simgrid::s4u::ActorPtr actor)
              "Couldn't map the new region (errno %d): %s", errno, strerror(errno));
   smpi_loaded_page = actor->get_pid();
 #endif
+
+  return true;
 }
 
 /**

diff --git a/src/smpi/mpi/smpi_comm.cpp b/src/smpi/mpi/smpi_comm.cpp
index 2c32d66..115b798 100644 (file)
--- a/src/smpi/mpi/smpi_comm.cpp
+++ b/src/smpi/mpi/smpi_comm.cpp
@@ -71,10 +71,9 @@ void Comm::destroy(Comm* comm)
 }
 
 int Comm::dup(MPI_Comm* newcomm){
-  if (smpi_cfg_privatization() == SmpiPrivStrategies::MMAP) {
-    // we need to switch as the called function may silently touch global variables
-    smpi_switch_data_segment(s4u::Actor::self());
-  }
+  // we need to switch as the called function may silently touch global variables
+  smpi_switch_data_segment(s4u::Actor::self());
+
   auto* cp     = new Group(this->group());
   (*newcomm)   = new  Comm(cp, this->topo());
 
@@ -411,10 +410,9 @@ void Comm::init_smp(){
     smpi_process()->set_replaying(false);
   }
 
-  if (smpi_cfg_privatization() == SmpiPrivStrategies::MMAP) {
-    // we need to switch as the called function may silently touch global variables
-    smpi_switch_data_segment(s4u::Actor::self());
-  }
+  // we need to switch as the called function may silently touch global variables
+  smpi_switch_data_segment(s4u::Actor::self());
+
   // identify neighbors in comm
   MPI_Comm comm_intra = find_intra_comm(&leader);
 
@@ -425,11 +423,6 @@ void Comm::init_smp(){
 
   allgather__ring(&leader, 1, MPI_INT , leaders_map, 1, MPI_INT, this);
 
-  if (smpi_cfg_privatization() == SmpiPrivStrategies::MMAP) {
-    // we need to switch as the called function may silently touch global variables
-    smpi_switch_data_segment(s4u::Actor::self());
-  }
-
   if(leaders_map_==nullptr){
     leaders_map_= leaders_map;
   }else{
@@ -499,10 +492,9 @@ void Comm::init_smp(){
   }
   bcast__scatter_LR_allgather(&is_uniform_, 1, MPI_INT, 0, comm_intra);
 
-  if (smpi_cfg_privatization() == SmpiPrivStrategies::MMAP) {
-    // we need to switch as the called function may silently touch global variables
-    smpi_switch_data_segment(s4u::Actor::self());
-  }
+  // we need to switch as the called function may silently touch global variables
+  smpi_switch_data_segment(s4u::Actor::self());
+
   // Are the ranks blocked ? = allocated contiguously on the SMP nodes
   int is_blocked=1;
   int prev      = this->group()->rank(comm_intra->group()->actor(0));

diff --git a/src/smpi/mpi/smpi_datatype.cpp b/src/smpi/mpi/smpi_datatype.cpp
index 3f5b211..ecdef35 100644 (file)
--- a/src/smpi/mpi/smpi_datatype.cpp
+++ b/src/smpi/mpi/smpi_datatype.cpp
@@ -341,9 +341,8 @@ int Datatype::copy(const void* sendbuf, int sendcount, MPI_Datatype sendtype, vo
 {
   // FIXME Handle the case of a partial shared malloc.
 
-  if (smpi_cfg_privatization() == SmpiPrivStrategies::MMAP) {
-    smpi_switch_data_segment(simgrid::s4u::Actor::self());
-  }
+  smpi_switch_data_segment(simgrid::s4u::Actor::self());
+
   /* First check if we really have something to do */
   size_t offset = 0;
   std::vector<std::pair<size_t, size_t>> private_blocks;

diff --git a/src/smpi/mpi/smpi_op.cpp b/src/smpi/mpi/smpi_op.cpp
index afc3264..a0501c1 100644 (file)
--- a/src/smpi/mpi/smpi_op.cpp
+++ b/src/smpi/mpi/smpi_op.cpp
@@ -266,11 +266,8 @@ namespace smpi{
 
 void Op::apply(const void* invec, void* inoutvec, const int* len, MPI_Datatype datatype) const
 {
-  if (smpi_cfg_privatization() == SmpiPrivStrategies::MMAP) {
-    // we need to switch as the called function may silently touch global variables
-    XBT_DEBUG("Applying operation, switch to the right data frame ");
-    smpi_switch_data_segment(simgrid::s4u::Actor::self());
-  }
+  // we need to switch as the called function may silently touch global variables
+  smpi_switch_data_segment(simgrid::s4u::Actor::self());
 
   if (not smpi_process()->replaying() && *len > 0) {
     XBT_DEBUG("Applying operation of length %d from %p and from/to %p", *len, invec, inoutvec);

diff --git a/src/smpi/mpi/smpi_request.cpp b/src/smpi/mpi/smpi_request.cpp
index fe8e35b..2526f7c 100644 (file)
--- a/src/smpi/mpi/smpi_request.cpp
+++ b/src/smpi/mpi/smpi_request.cpp
@@ -506,12 +506,9 @@ void Request::start()
       if (not(type_->flags() & DT_FLAG_DERIVED)) {
         oldbuf = buf_;
         if (not process->replaying() && oldbuf != nullptr && size_ != 0) {
-          if ((smpi_cfg_privatization() != SmpiPrivStrategies::NONE) &&
-              (static_cast<char*>(buf_) >= smpi_data_exe_start) &&
-              (static_cast<char*>(buf_) < smpi_data_exe_start + smpi_data_exe_size)) {
+          if (smpi_switch_data_segment(simgrid::s4u::Actor::by_pid(src_), buf_))
             XBT_DEBUG("Privatization : We are sending from a zone inside global memory. Switch data segment ");
-            smpi_switch_data_segment(simgrid::s4u::Actor::by_pid(src_));
-          }
+
           //we need this temporary buffer even for bsend, as it will be released in the copy callback and we don't have a way to differentiate it
           //so actually ... don't use manually attached buffer space.
           buf = xbt_malloc(size_);
@@ -939,12 +936,8 @@ void Request::finish_wait(MPI_Request* request, MPI_Status * status)
       // FIXME Handle the case of a partial shared malloc.
       if (((req->flags_ & MPI_REQ_ACCUMULATE) != 0) ||
           (datatype->flags() & DT_FLAG_DERIVED)) { // && (not smpi_is_shared(req->old_buf_))){
-        if (not smpi_process()->replaying() && smpi_cfg_privatization() != SmpiPrivStrategies::NONE &&
-            static_cast<char*>(req->old_buf_) >= smpi_data_exe_start &&
-            static_cast<char*>(req->old_buf_) < smpi_data_exe_start + smpi_data_exe_size) {
+        if (not smpi_process()->replaying() && smpi_switch_data_segment(simgrid::s4u::Actor::self(), req->old_buf_))
           XBT_VERB("Privatization : We are unserializing to a zone in global memory  Switch data segment ");
-          smpi_switch_data_segment(simgrid::s4u::Actor::self());
-        }
 
         if(datatype->flags() & DT_FLAG_DERIVED){
           // This part handles the problem of non-contiguous memory the unserialization at the reception