From 2b99a23b58af6f52e1828cefeb8c6a3610b85279 Mon Sep 17 00:00:00 2001 From: Gabriel Corona Date: Mon, 29 Feb 2016 12:34:22 +0100 Subject: [PATCH] [mc] Cleanup RegionSnapshot buffer code --- src/mc/RegionSnapshot.cpp | 61 +++++++++++++++++++++++++---------- src/mc/RegionSnapshot.hpp | 68 ++++++++++++++++++++++++++++----------- src/mc/mc_checkpoint.cpp | 2 +- src/mc/mc_snapshot.h | 4 +-- 4 files changed, 96 insertions(+), 39 deletions(-) diff --git a/src/mc/RegionSnapshot.cpp b/src/mc/RegionSnapshot.cpp index 7e96951979..a045b2f8ec 100644 --- a/src/mc/RegionSnapshot.cpp +++ b/src/mc/RegionSnapshot.cpp @@ -39,36 +39,63 @@ const char* to_cstr(RegionType region) } } -void data_deleter::operator()(void* p) const +buffer::buffer(std::size_t size, Type type) : size_(size), type_(type) { switch(type_) { - case Free: - std::free(p); + case Type::Malloc: + data_ = malloc(size_); break; - case Munmap: - munmap(p, size_); + case Type::Mmap: + data_ = mmap(nullptr, size, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_POPULATE, -1, 0); + if (data_ == MAP_FAILED) { + data_ = nullptr; + size_ = 0; + type_ = Type::Malloc; + throw std::bad_alloc(); + } + break; + default: + abort(); + } +} + +void buffer::clear() noexcept +{ + switch(type_) { + case Type::Malloc: + std::free(data_); break; + case Type::Mmap: + if (munmap(data_, size_) != 0) + abort(); + break; + default: + abort(); } + data_ = nullptr; + size_ = 0; + type_ = Type::Malloc; } RegionSnapshot dense_region( RegionType region_type, void *start_addr, void* permanent_addr, size_t size) { - simgrid::mc::RegionSnapshot::flat_data_ptr data; - if (!_sg_mc_ksm) - data = simgrid::mc::RegionSnapshot::flat_data_ptr((char*) std::malloc(size)); - else { - char* ptr = (char*) mmap(nullptr, size, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_POPULATE, -1, 0); - if (ptr == MAP_FAILED) - throw std::bad_alloc(); - simgrid::mc::data_deleter deleter( - simgrid::mc::data_deleter::Munmap, size); - data = simgrid::mc::RegionSnapshot::flat_data_ptr(ptr, deleter); - } + simgrid::mc::buffer::Type buffer_type; + if (_sg_mc_ksm) + // We use mmap to allocate the memory in order to madvise it. + // We don't want to madvise the main heap. + // Moreover we get aligned pgaes which is merge-friendly. + buffer_type = simgrid::mc::buffer::Type::Mmap; + else + buffer_type = simgrid::mc::buffer::Type::Malloc; + + simgrid::mc::buffer data(size, buffer_type); + mc_model_checker->process().read_bytes(data.get(), size, remote(permanent_addr), simgrid::mc::ProcessIndexDisabled); + if (_sg_mc_ksm) // Mark the region as mergeable *after* we have written into it. // There no point to let KSM do the hard work before that. @@ -79,7 +106,7 @@ RegionSnapshot dense_region( region.flat_data(std::move(data)); XBT_DEBUG("New region : type : %s, data : %p (real addr %p), size : %zu", - to_cstr(region_type), region.flat_data(), permanent_addr, size); + to_cstr(region_type), region.flat_data().get(), permanent_addr, size); return std::move(region); } diff --git a/src/mc/RegionSnapshot.hpp b/src/mc/RegionSnapshot.hpp index a0031910ec..f1007acc9e 100644 --- a/src/mc/RegionSnapshot.hpp +++ b/src/mc/RegionSnapshot.hpp @@ -37,22 +37,53 @@ enum class StorageType { Privatized = 3 }; -class data_deleter { +class buffer { public: - enum Type { - Free, - Munmap + enum class Type { + Malloc, + Mmap }; private: - Type type_; + void* data_ = nullptr; std::size_t size_; + Type type_ = Type::Malloc; public: - data_deleter() : type_(Free) {} - data_deleter(Type type, std::size_t size) : type_(type), size_(size) {} - void operator()(void* p) const; -}; + buffer() {} + buffer(std::size_t size, Type type = Type::Malloc); + buffer(void* data, std::size_t size, Type type = Type::Malloc) : + data_(data), size_(size), type_(type) {} + void clear() noexcept; + ~buffer() noexcept { clear(); } + + // No copy + buffer(buffer const& buffer) = delete; + buffer& operator=(buffer const& buffer) = delete; + + // Move + buffer(buffer&& that) noexcept + : data_(that.data_), size_(that.size_), type_(that.type_) + { + that.data_ = nullptr; + that.size_ = 0; + that.type_ = Type::Malloc; + } + buffer& operator=(buffer&& that) noexcept + { + clear(); + data_ = that.data_; + size_ = that.size_; + type_ = that.type_; + that.data_ = nullptr; + that.size_ = 0; + that.type_ = Type::Malloc; + return *this; + } -typedef std::unique_ptr unique_data_ptr; + void* get() { return data_; } + const void* get() const { return data_; } + std::size_t size() const { return size_; } + Type type() const { return type_; } +}; /** A copy/snapshot of a given memory region * @@ -77,8 +108,6 @@ public: static const RegionType UnknownRegion = RegionType::Unknown; static const RegionType HeapRegion = RegionType::Heap; static const RegionType DataRegion = RegionType::Data; -public: - typedef unique_data_ptr flat_data_ptr; private: RegionType region_type_; StorageType storage_type_; @@ -101,7 +130,7 @@ private: * */ void *permanent_addr_; - flat_data_ptr flat_data_; + buffer flat_data_; ChunkedData page_numbers_; std::vector privatized_regions_; public: @@ -160,7 +189,7 @@ public: storage_type_ = StorageType::NoData; privatized_regions_.clear(); page_numbers_.clear(); - flat_data_.reset(); + flat_data_.clear(); object_info_ = nullptr; start_addr_ = nullptr; size_ = 0; @@ -170,24 +199,25 @@ public: void clear_data() { storage_type_ = StorageType::NoData; - flat_data_.reset(); + flat_data_.clear(); page_numbers_.clear(); privatized_regions_.clear(); } - void flat_data(flat_data_ptr data) + void flat_data(buffer data) { storage_type_ = StorageType::Flat; flat_data_ = std::move(data); page_numbers_.clear(); privatized_regions_.clear(); } - const char* flat_data() const { return flat_data_.get(); } + const buffer& flat_data() const { return flat_data_; } + buffer& flat_data() { return flat_data_; } void page_data(ChunkedData page_data) { storage_type_ = StorageType::Chunked; - flat_data_.reset(); + flat_data_.clear(); page_numbers_ = std::move(page_data); privatized_regions_.clear(); } @@ -196,7 +226,7 @@ public: void privatized_data(std::vector data) { storage_type_ = StorageType::Privatized; - flat_data_.reset(); + flat_data_.clear(); page_numbers_.clear(); privatized_regions_ = std::move(data); } diff --git a/src/mc/mc_checkpoint.cpp b/src/mc/mc_checkpoint.cpp index baf90a2321..07fa867831 100644 --- a/src/mc/mc_checkpoint.cpp +++ b/src/mc/mc_checkpoint.cpp @@ -66,7 +66,7 @@ static void restore(mc_mem_region_t region) break; case simgrid::mc::StorageType::Flat: - mc_model_checker->process().write_bytes(region->flat_data(), + mc_model_checker->process().write_bytes(region->flat_data().get(), region->size(), region->permanent_address()); break; diff --git a/src/mc/mc_snapshot.h b/src/mc/mc_snapshot.h index 45db54c6c1..a1e7399c2b 100644 --- a/src/mc/mc_snapshot.h +++ b/src/mc/mc_snapshot.h @@ -55,7 +55,7 @@ void* mc_translate_address_region(uintptr_t addr, mc_mem_region_t region, int pr case simgrid::mc::StorageType::Flat: { uintptr_t offset = (uintptr_t) addr - (uintptr_t) region->start().address(); - return (void *) ((uintptr_t) region->flat_data() + offset); + return (void *) ((uintptr_t) region->flat_data().get() + offset); } case simgrid::mc::StorageType::Chunked: @@ -243,7 +243,7 @@ const void* MC_region_read( xbt_die("Storage type not supported"); case simgrid::mc::StorageType::Flat: - return (char*) region->flat_data() + offset; + return (char*) region->flat_data().get() + offset; case simgrid::mc::StorageType::Chunked: { -- 2.20.1