From 105f47d2d088949a2eb867803bf69635228fc040 Mon Sep 17 00:00:00 2001 From: Daniel Krebs Date: Tue, 15 May 2018 12:13:51 +0200 Subject: [PATCH] common/memory: add check-callback to getPath() to select desired path This is a workaround until we have a better heuristic (maybe shortest path?) to choose between multiple paths in the graph. Since the (abstract) graph has no idea about memory translations, getPath() may even yield paths that are no valid translation because a pair of inbound/outbound edges must not neccessarily share a common address window, but from the perspective of the abstract graph present a valid path. The callback function is used by the MemoryManager to verify if a path candidate represents a valid translation. --- fpga/include/villas/directed_graph.hpp | 13 +++++++++-- fpga/include/villas/memory_manager.hpp | 14 +++++++++++- fpga/include/villas/utils.hpp | 9 ++++++++ fpga/lib/common/memory_manager.cpp | 30 +++++++++++++++++++++++--- 4 files changed, 60 insertions(+), 6 deletions(-) diff --git a/fpga/include/villas/directed_graph.hpp b/fpga/include/villas/directed_graph.hpp index b75912335..48899a606 100644 --- a/fpga/include/villas/directed_graph.hpp +++ b/fpga/include/villas/directed_graph.hpp @@ -212,9 +212,17 @@ public: vertexGetEdges(VertexIdentifier vertexId) const { return getVertex(vertexId)->edges; } + + using check_path_fn = std::function; + + static bool + checkPath(const Path&) + { return true; } + bool getPath(VertexIdentifier fromVertexId, VertexIdentifier toVertexId, - Path& path) + Path& path, + check_path_fn pathCheckFunc = checkPath) { if(fromVertexId == toVertexId) { // arrived at the destination @@ -244,7 +252,8 @@ public: path.push_back(edgeId); // recursive, depth-first search - if(getPath(edgeOfFromVertex->to, toVertexId, path)) { + if(getPath(edgeOfFromVertex->to, toVertexId, path, pathCheckFunc) and + pathCheckFunc(path)) { // path found, we're done return true; } else { diff --git a/fpga/include/villas/memory_manager.hpp b/fpga/include/villas/memory_manager.hpp index 96d1ccda2..012910509 100644 --- a/fpga/include/villas/memory_manager.hpp +++ b/fpga/include/villas/memory_manager.hpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include "log.hpp" @@ -73,7 +74,12 @@ private: // This is a singleton, so private constructor ... MemoryManager() : memoryGraph("MemoryGraph"), - logger(loggerGetOrCreate("MemoryManager")) {} + logger(loggerGetOrCreate("MemoryManager")) + { + pathCheckFunc = [&](const MemoryGraph::Path& path) { + return this->pathCheck(path); + }; + } // ... and no copying or assigning MemoryManager(const MemoryManager&) = delete; @@ -144,6 +150,8 @@ public: using AddressSpaceId = MemoryGraph::VertexIdentifier; using MappingId = MemoryGraph::EdgeIdentifier; + struct InvalidTranslation : public std::exception {}; + /// Get singleton instance static MemoryManager& get(); @@ -210,6 +218,8 @@ private: getTranslationFromMapping(const Mapping& mapping) { return MemoryTranslation(mapping.src, mapping.dest, mapping.size); } + bool + pathCheck(const MemoryGraph::Path& path); private: /// Directed graph that stores address spaces and memory mappings @@ -221,6 +231,8 @@ private: /// Logger for universal access in this class SpdLogger logger; + MemoryGraph::check_path_fn pathCheckFunc; + /// Static pointer to global instance, because this is a singleton static MemoryManager* instance; }; diff --git a/fpga/include/villas/utils.hpp b/fpga/include/villas/utils.hpp index b2b5b314c..d368de915 100644 --- a/fpga/include/villas/utils.hpp +++ b/fpga/include/villas/utils.hpp @@ -10,6 +10,15 @@ namespace utils { std::vector tokenize(std::string s, std::string delimiter); + +template +void +assertExcept(bool condition, const T& exception) +{ + if(not condition) + throw exception; +} + } // namespace utils } // namespace villas diff --git a/fpga/lib/common/memory_manager.cpp b/fpga/lib/common/memory_manager.cpp index f8e01e9b0..d832896c6 100644 --- a/fpga/lib/common/memory_manager.cpp +++ b/fpga/lib/common/memory_manager.cpp @@ -2,8 +2,11 @@ #include #include +#include #include "memory_manager.hpp" +using namespace villas::utils; + namespace villas { MemoryManager* @@ -76,7 +79,8 @@ MemoryManager::getTranslation(MemoryManager::AddressSpaceId fromAddrSpaceId, { // find a path through the memory graph MemoryGraph::Path path; - if(not memoryGraph.getPath(fromAddrSpaceId, toAddrSpaceId, path)) { + if(not memoryGraph.getPath(fromAddrSpaceId, toAddrSpaceId, path, pathCheckFunc)) { + auto fromAddrSpace = memoryGraph.getVertex(fromAddrSpaceId); auto toAddrSpace = memoryGraph.getVertex(toAddrSpaceId); @@ -98,6 +102,26 @@ MemoryManager::getTranslation(MemoryManager::AddressSpaceId fromAddrSpaceId, return translation; } +bool +MemoryManager::pathCheck(const MemoryGraph::Path& path) +{ + // start with an identity mapping + MemoryTranslation translation(0, 0, SIZE_MAX); + + // Try to add all mappings together to a common translation. If this fails + // there is a non-overlapping window + for(auto& mappingId : path) { + auto mapping = memoryGraph.getEdge(mappingId); + try { + translation += getTranslationFromMapping(*mapping); + } catch(const InvalidTranslation&) { + return false; + } + } + + return true; +} + uintptr_t MemoryTranslation::getLocalAddr(uintptr_t addrInForeignAddrSpace) const { @@ -125,8 +149,8 @@ MemoryTranslation::operator+=(const MemoryTranslation& other) const uintptr_t other_src_high = other.src + other.size; // make sure there is a common memory area - assert(other.src < this_dst_high); - assert(this->dst < other_src_high); + assertExcept(other.src < this_dst_high, MemoryManager::InvalidTranslation()); + assertExcept(this->dst < other_src_high, MemoryManager::InvalidTranslation()); const uintptr_t hi = std::max(this_dst_high, other_src_high); const uintptr_t lo = std::min(this->dst, other.src);