From 5242b87e4c28699dd025f54d581cff3c55cd5be7 Mon Sep 17 00:00:00 2001 From: Daniel Krebs Date: Fri, 13 Apr 2018 15:31:18 +0200 Subject: [PATCH] lib/memory: rework allocators to make them extensible and more abstract This is change renders memory allocators only dependend on an address space id that they are managing, allowing easy implementation of other algorithms and instantiation in memory IP blocks. --- fpga/include/villas/memory.hpp | 278 +++++++++++++++++-------- fpga/include/villas/memory_manager.hpp | 8 +- fpga/lib/memory.cpp | 131 +++++++++++- fpga/tests/dma.cpp | 7 +- 4 files changed, 325 insertions(+), 99 deletions(-) diff --git a/fpga/include/villas/memory.hpp b/fpga/include/villas/memory.hpp index 85dae02ea..8a1895b23 100644 --- a/fpga/include/villas/memory.hpp +++ b/fpga/include/villas/memory.hpp @@ -8,115 +8,221 @@ namespace villas { +/** + * @brief Basic memory block backed by an address space in the memory graph + * + * This is a generic representation of a chunk of memory in the system. It can + * reside anywhere and represent different types of memory. + */ class MemoryBlock { -protected: - MemoryBlock(MemoryManager::AddressSpaceId addrSpaceId, size_t size) : - addrSpaceId(addrSpaceId), size(size) {} - public: + using deallocator_fn = std::function; + + MemoryBlock(size_t offset, size_t size, MemoryManager::AddressSpaceId addrSpaceId) : + offset(offset), size(size), addrSpaceId(addrSpaceId) {} + MemoryManager::AddressSpaceId getAddrSpaceId() const { return addrSpaceId; } size_t getSize() const { return size; } -private: - MemoryManager::AddressSpaceId addrSpaceId; - size_t size; + size_t getOffset() const + { return offset; } + +protected: + size_t offset; ///< Offset (or address) inside address space + size_t size; ///< Size in bytes of this block + MemoryManager::AddressSpaceId addrSpaceId; ///< Identifier in memory graph }; -class MemoryAllocator { -}; - - -class HostRam : public MemoryAllocator { +/** + * @brief Wrapper for a MemoryBlock to access the underlying memory directly + * + * The underlying memory block has to be accessible for the current process, + * that means it has to be mapped accordingly and registered to the global + * memory graph. + * Furthermore, this wrapper can be owning the memory block when initialized + * with a moved unique pointer. Otherwise, it just stores a reference to the + * memory block and it's the users responsibility to take care that the memory + * block is valid. + */ +template +class MemoryAccessor { public: + using Type = T; + + // take ownership of the MemoryBlock + MemoryAccessor(std::unique_ptr mem) : + translation(MemoryManager::get().getTranslationFromProcess(mem->getAddrSpaceId())), + memoryBlock(std::move(mem)) {} + + // just act as an accessor, do not take ownership of MemoryBlock + MemoryAccessor(const MemoryBlock& mem) : + translation(MemoryManager::get().getTranslationFromProcess(mem.getAddrSpaceId())) {} + + + T& operator*() const { + return *reinterpret_cast(translation.getLocalAddr(0)); + } + + T& operator[](size_t idx) const { + const size_t offset = sizeof(T) * idx; + return *reinterpret_cast(translation.getLocalAddr(offset)); + } + + T* operator&() const { + return reinterpret_cast(translation.getLocalAddr(0)); + } + + T* operator->() const { + return reinterpret_cast(translation.getLocalAddr(0)); + } + + const MemoryBlock& + getMemoryBlock() const + { if(not memoryBlock) throw std::bad_alloc(); else return *memoryBlock; } + +private: + /// cached memory translation for fast access + MemoryTranslation translation; + + /// take the unique pointer in case user wants this class to have ownership + std::unique_ptr memoryBlock; +}; + + +/** + * @brief Base memory allocator + * + * Note the usage of CRTP idiom here to access methods of derived allocators. + * The concept is explained here at [1]. + * + * [1] https://en.wikipedia.org/wiki/Curiously_recurring_template_pattern + */ +template +class BaseAllocator { +public: + /// memoryAddrSpaceId: memory that is managed by this allocator + BaseAllocator(MemoryManager::AddressSpaceId memoryAddrSpaceId) : + memoryAddrSpaceId(memoryAddrSpaceId) + { + // CRTP + derivedAlloc = static_cast(this); + logger = loggerGetOrCreate(derivedAlloc->getName()); + + // default deallocation callback + free = [&](MemoryBlock* mem) { + logger->warn("no free callback defined for addr space {}, not freeing", + mem->getAddrSpaceId()); + }; + } + + virtual std::unique_ptr + allocateBlock(size_t size) = 0; template - class MemoryBlockHostRam : public MemoryBlock { - friend class HostRam; - private: - MemoryBlockHostRam(void* addr, size_t size, MemoryManager::AddressSpaceId foreignAddrSpaceId) : - MemoryBlock(foreignAddrSpaceId, size), - addr(addr), - translation(MemoryManager::get().getTranslationFromProcess(foreignAddrSpaceId)) - {} - public: - using Type = T; - - MemoryBlockHostRam() = delete; - - T& operator*() { - return *reinterpret_cast(translation.getLocalAddr(0)); - } - - T& operator [](int idx) { - const size_t offset = sizeof(T) * idx; - return *reinterpret_cast(translation.getLocalAddr(offset)); - } - - T* operator &() const { - return reinterpret_cast(translation.getLocalAddr(0)); - } - - private: - // addr needed for freeing later - void* addr; - - // cached memory translation for fast access - MemoryTranslation translation; - }; - - template - static MemoryBlockHostRam + MemoryAccessor allocate(size_t num) { - /* Align to next bigger page size chunk */ - size_t length = num * sizeof(T); - if (length & size_t(0xFFF)) { - length += size_t(0x1000); - length &= size_t(~0xFFF); - } - - void* const addr = HostRam::allocate(length); - if(addr == nullptr) { - throw std::bad_alloc(); - } - - auto& mm = MemoryManager::get(); - - // assemble name for this block - std::stringstream name; - name << std::showbase << std::hex << reinterpret_cast(addr); - - auto blockAddrSpaceId = mm.getProcessAddressSpaceMemoryBlock(name.str()); - - // create mapping from VA space of process to this new block - mm.createMapping(reinterpret_cast(addr), 0, length, - "VA", - mm.getProcessAddressSpace(), - blockAddrSpaceId); - - // create object and corresponding address space in memory manager - return MemoryBlockHostRam(addr, length, blockAddrSpaceId); + const size_t size = num * sizeof(T); + auto mem = allocateBlock(size); + return MemoryAccessor(std::move(mem)); } - template - static inline bool - free(const MemoryBlockHostRam& block) +protected: + void insertMemoryBlock(const MemoryBlock& mem) { - // TODO: remove address space from memory manager - // TODO: how to prevent use after free? - return HostRam::free(block.addr, block.size); + auto& mm = MemoryManager::get(); + mm.createMapping(mem.getOffset(), 0, mem.getSize(), + derivedAlloc->getName(), + memoryAddrSpaceId, + mem.getAddrSpaceId()); } + void removeMemoryBlock(const MemoryBlock& mem) + { + // this will also remove any mapping to and from the memory block + auto& mm = MemoryManager::get(); + mm.removeAddressSpace(mem.getAddrSpaceId()); + } + + MemoryManager::AddressSpaceId getAddrSpaceId() const + { return memoryAddrSpaceId; } + +protected: + MemoryBlock::deallocator_fn free; + SpdLogger logger; + private: - static void* - allocate(size_t length, int flags = 0); + MemoryManager::AddressSpaceId memoryAddrSpaceId; + DerivedAllocator* derivedAlloc; +}; - static bool - free(void*, size_t length); + +/** + * @brief Linear memory allocator + * + * This is the simplest kind of allocator. The idea is to keep a pointer at the + * first memory address of your memory chunk and move it every time an + * allocation is done. Due to its simplicity, this allocator doesn't allow + * specific positions of memory to be freed. Usually, all memory is freed + * together. + */ +class LinearAllocator : public BaseAllocator { +public: + LinearAllocator(MemoryManager::AddressSpaceId memoryAddrSpaceId, + size_t memorySize, + size_t internalOffset = 0); + + size_t getAvailableMemory() const + { return memorySize - nextFreeAddress; } + + std::string getName() const; + + std::unique_ptr + allocateBlock(size_t size); + +private: + static constexpr size_t alignBytes = sizeof(uintptr_t); + static constexpr size_t alignMask = alignBytes - 1; + + size_t getAlignmentPadding(uintptr_t addr) const + { return (alignBytes - (addr & alignMask)) & alignMask; } + +private: + size_t nextFreeAddress; ///< next chunk will be allocated here + size_t memorySize; ///< total size of managed memory + size_t internalOffset; ///< offset in address space (usually 0) +}; + + +/** + * @brief Wrapper around mmap() to create villas memory blocks + * + * This class simply wraps around mmap() and munmap() to allocate memory in the + * host memory via the OS. + */ +class HostRam { +public: + class HostRamAllocator : public BaseAllocator { + public: + HostRamAllocator(); + + std::string getName() const + { return "HostRamAlloc"; } + + std::unique_ptr + allocateBlock(size_t size); + }; + + static HostRamAllocator& + getAllocator() + { return allocator; } + +private: + static HostRamAllocator allocator; }; } // namespace villas diff --git a/fpga/include/villas/memory_manager.hpp b/fpga/include/villas/memory_manager.hpp index fb3db33c2..96d1ccda2 100644 --- a/fpga/include/villas/memory_manager.hpp +++ b/fpga/include/villas/memory_manager.hpp @@ -77,7 +77,7 @@ private: // ... and no copying or assigning MemoryManager(const MemoryManager&) = delete; - MemoryManager& operator=(const MemoryManager&) = delete ; + MemoryManager& operator=(const MemoryManager&) = delete; /** * @brief Custom edge in memory graph representing a memory mapping @@ -153,13 +153,17 @@ public: { return getOrCreateAddressSpace("villas-fpga"); } AddressSpaceId - getProcessAddressSpaceMemoryBlock(const std::string& memoryBlock) + getProcessAddressSpaceMemoryBlock(const std::string& memoryBlock) { return getOrCreateAddressSpace(getSlaveAddrSpaceName("villas-fpga", memoryBlock)); } AddressSpaceId getOrCreateAddressSpace(std::string name); + void + removeAddressSpace(AddressSpaceId addrSpaceId) + { memoryGraph.removeVertex(addrSpaceId); } + /// Create a default mapping MappingId createMapping(uintptr_t src, uintptr_t dest, size_t size, diff --git a/fpga/lib/memory.cpp b/fpga/lib/memory.cpp index f3d2802b4..f2def1432 100644 --- a/fpga/lib/memory.cpp +++ b/fpga/lib/memory.cpp @@ -5,20 +5,135 @@ namespace villas { -bool -HostRam::free(void* addr, size_t length) +std::unique_ptr +HostRam::HostRamAllocator::allocateBlock(size_t size) { - return munmap(addr, length) == 0; + /* Align to next bigger page size chunk */ + if (size & size_t(0xFFF)) { + size += size_t(0x1000); + size &= size_t(~0xFFF); + } + + const int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_32BIT; + const int mmap_protection = PROT_READ | PROT_WRITE; + + const void* addr = mmap(nullptr, size, mmap_protection, mmap_flags, 0, 0); + if(addr == nullptr) { + throw std::bad_alloc(); + } + + auto& mm = MemoryManager::get(); + + // assemble name for this block + std::stringstream name; + name << std::showbase << std::hex << reinterpret_cast(addr); + + auto blockAddrSpaceId = mm.getProcessAddressSpaceMemoryBlock(name.str()); + + const auto localAddr = reinterpret_cast(addr); + std::unique_ptr + mem(new MemoryBlock(localAddr, size, blockAddrSpaceId), this->free); + + insertMemoryBlock(*mem); + + return mem; } -void* -HostRam::allocate(size_t length, int flags) +LinearAllocator::LinearAllocator(MemoryManager::AddressSpaceId memoryAddrSpaceId, + size_t memorySize, + size_t internalOffset) : + BaseAllocator(memoryAddrSpaceId), + nextFreeAddress(0), + memorySize(memorySize), + internalOffset(internalOffset) { - const int mmap_flags = flags | MAP_PRIVATE | MAP_ANONYMOUS | MAP_32BIT; - const int mmap_protection = PROT_READ | PROT_WRITE; + // make sure to start at aligned offset, reduce size in case we need padding + if(const size_t paddingBytes = getAlignmentPadding(internalOffset)) { + assert(paddingBytes < memorySize); - return mmap(nullptr, length, mmap_protection, mmap_flags, 0, 0); + internalOffset += paddingBytes; + memorySize -= paddingBytes; + } + + // deallocation callback + free = [&](MemoryBlock* mem) { + logger->debug("freeing {:#x} bytes at local addr {:#x} (addr space {})", + mem->getSize(), mem->getOffset(), mem->getAddrSpaceId()); + logger->warn("free() not implemented"); + logger->debug("available memory: {:#x} bytes", getAvailableMemory()); + }; +} + + +std::string +LinearAllocator::getName() const +{ + std::stringstream name; + name << "LinearAlloc" << getAddrSpaceId() + << "@0x" << std::hex << internalOffset; + return name.str(); +} + + +std::unique_ptr +LinearAllocator::allocateBlock(size_t size) +{ + if(size > getAvailableMemory()) { + throw std::bad_alloc(); + } + + // assign address + const uintptr_t localAddr = nextFreeAddress + internalOffset; + + // reserve memory + nextFreeAddress += size; + + // make sure it is aligned + if(const size_t paddingBytes = getAlignmentPadding(nextFreeAddress)) { + nextFreeAddress += paddingBytes; + + // if next free address is outside this block due to padding, cap it + nextFreeAddress = std::min(nextFreeAddress, memorySize); + } + + + auto& mm = MemoryManager::get(); + + // assemble name for this block + std::stringstream blockName; + blockName << std::showbase << std::hex << localAddr; + + // create address space + auto addrSpaceName = mm.getSlaveAddrSpaceName(getName(), blockName.str()); + auto addrSpaceId = mm.getOrCreateAddressSpace(addrSpaceName); + + logger->debug("Allocated {:#x} bytes for {}, {:#x} bytes remaining", + size, addrSpaceId, getAvailableMemory()); + + std::unique_ptr + mem(new MemoryBlock(localAddr, size, addrSpaceId), this->free); + + // mount block into the memory graph + insertMemoryBlock(*mem); + + + return mem; +} + + +HostRam::HostRamAllocator +HostRam::allocator; + +HostRam::HostRamAllocator::HostRamAllocator() : + BaseAllocator(MemoryManager::get().getProcessAddressSpace()) +{ + free = [&](MemoryBlock* mem) { + if(::munmap(reinterpret_cast(mem->getOffset()), mem->getSize()) != 0) { + logger->warn("munmap() failed for {:#x} of size {:#x}", + mem->getOffset(), mem->getSize()); + } + }; } } // namespace villas diff --git a/fpga/tests/dma.cpp b/fpga/tests/dma.cpp index fd69cfc94..e8bb4c08d 100644 --- a/fpga/tests/dma.cpp +++ b/fpga/tests/dma.cpp @@ -41,15 +41,16 @@ Test(fpga, dma, .description = "DMA") size_t len = 4 * (1 << 10); /* Allocate memory to use with DMA */ - auto src = villas::HostRam::allocate(len); - auto dst = villas::HostRam::allocate(len); + auto src = villas::HostRam::getAllocator().allocate(len); + auto dst = villas::HostRam::getAllocator().allocate(len); /* Get new random data */ const size_t lenRandom = read_random(&src, len); cr_assert(len == lenRandom, "Failed to get random data"); /* Start transfer */ - cr_assert(dma.pingPong(src, dst, len), "DMA ping pong failed"); + cr_assert(dma.pingPong(src.getMemoryBlock(), dst.getMemoryBlock(), len), + "DMA ping pong failed"); /* Compare data */ cr_assert(memcmp(&src, &dst, len) == 0, "Data not equal");