From 7abd83ca4572cc70fb5cac86c605855115221a59 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Constantin=20F=C3=BCrst?= <c@fuersten.info>
Date: Wed, 7 Feb 2024 17:20:30 +0100
Subject: [PATCH] fix offloading cache bug, introduced by not using maxptr for
 secondary invalid value, more care needed to be taken on when the secondary
 invalid would be set and how, now we set in in cachedata:init for both the
 local task in access and the one in cache state

---
 offloading-cacher/cache.hpp | 25 +++++++++++++++----------
 offloading-cacher/main.cpp  |  4 +++-
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/offloading-cacher/cache.hpp b/offloading-cacher/cache.hpp
index 77c92cc..77ddc8c 100644
--- a/offloading-cacher/cache.hpp
+++ b/offloading-cacher/cache.hpp
@@ -125,11 +125,11 @@ namespace dsacache {
         uint8_t* GetSource() const { return src_; }
         int32_t GetRefCount() const { return active_->load(); }
         void SetCacheToSource() { cache_->store(src_); delete_ = false; }
-        void SetTaskHandlersAndCache(uint8_t* cache, std::vector<dml_handler>* handlers, std::vector<dml_handler>* invalid_handlers);
+        void SetTaskHandlersAndCache(uint8_t* cache, std::vector<dml_handler>* handlers);
 
         // initializes the class after which it is thread safe
         // but may only be destroyed safely after setting handlers
-        void Init();
+        void Init(std::vector<dml_handler>* invalid_handlers);
 
         friend Cache;
 
@@ -420,8 +420,13 @@ inline std::unique_ptr<dsacache::CacheData> dsacache::Cache::Access(uint8_t* dat
         // initialize the task now for thread safety
         // as we are now sure that we will submit work
         // to it and will not delete it beforehand
+        // of the one in cache state - must be
+        // performed for the local and cache-state
+        // instance as Init will modify values that
+        // are not shared but copied on copy-construct
 
-        task->Init();
+        state.first->second.Init(&invalid_handlers_);
+        task->Init(&invalid_handlers_);
     }
 
     SubmitTask(task.get(), dst_node, src_node);
@@ -498,7 +503,7 @@ inline void dsacache::Cache::SubmitTask(CacheData* task, const int dst_node, con
         handlers->emplace_back(ExecuteCopy(local_src, local_dst, local_size, executing_nodes[i]));
     }
 
-    task->SetTaskHandlersAndCache(dst, handlers, &invalid_handlers_);
+    task->SetTaskHandlersAndCache(dst, handlers);
 }
 
 inline dml::handler<dml::mem_copy_operation, std::allocator<uint8_t>> dsacache::Cache::ExecuteCopy(
@@ -508,13 +513,13 @@ inline dml::handler<dml::mem_copy_operation, std::allocator<uint8_t>> dsacache::
     dml::data_view dstv = dml::make_view(dst, size);
 
     if (CheckFlag(flags_, FLAG_HANDLE_PF)) {
-        return dml::submit<dml::hardware>(
+        return dml::submit<dml::software>(
                 dml::mem_copy.block_on_fault(), srcv, dstv,
                 dml::execution_interface<dml::hardware,std::allocator<uint8_t>>(), node
         );
     }
     else {
-        return dml::submit<dml::hardware>(
+        return dml::submit<dml::software>(
                 dml::mem_copy, srcv, dstv,
                 dml::execution_interface<dml::hardware,std::allocator<uint8_t>>(), node
         );
@@ -752,7 +757,7 @@ inline void dsacache::CacheData::WaitOnCompletion() {
     // ensure that no other thread snatched the handlers before us
     // and in case one did, wait again and then return
 
-    if (local_handlers == nullptr || local_handlers == invalid_handlers_) {
+    if (local_handlers == invalid_handlers_) {
         cache_->wait(nullptr);
         return;
     }
@@ -802,14 +807,14 @@ inline void dsacache::CacheData::WaitOnCompletion() {
     handlers_->notify_all();
 }
 
-void dsacache::CacheData::SetTaskHandlersAndCache(uint8_t* cache, std::vector<dml_handler>* handlers, std::vector<dml_handler>* invalid_handlers) {
+void dsacache::CacheData::SetTaskHandlersAndCache(uint8_t* cache, std::vector<dml_handler>* handlers) {
     *incomplete_cache_ = cache;
     handlers_->store(handlers);
     handlers_->notify_one();
-    invalid_handlers_ = invalid_handlers;
 }
 
-void dsacache::CacheData::Init() {
+void dsacache::CacheData::Init(std::vector<dml_handler>* invalid_handlers) {
     cache_->store(nullptr);
     delete_ = true;
+    invalid_handlers_ = invalid_handlers;
 }
diff --git a/offloading-cacher/main.cpp b/offloading-cacher/main.cpp
index df09f5b..92d9a7e 100644
--- a/offloading-cacher/main.cpp
+++ b/offloading-cacher/main.cpp
@@ -100,7 +100,8 @@ std::unique_ptr<dsacache::CacheData> PerformAccessAndTest(uint8_t* src, const si
             size * sizeof(uint8_t)
     );
 
-    data_cache->WaitOnCompletion(dsacache::WAIT_WEAK);
+    data_cache->SetFlags(dsacache::FLAG_WAIT_WEAK);
+    data_cache->WaitOnCompletion();
 
     uint8_t* cached_imm = reinterpret_cast<uint8_t *>(data_cache->GetDataLocation());
 
@@ -118,6 +119,7 @@ std::unique_ptr<dsacache::CacheData> PerformAccessAndTest(uint8_t* src, const si
 
     // waits for the completion of the asynchronous caching operation
 
+    data_cache->SetFlags(dsacache::FLAG_DEFAULT);
     data_cache->WaitOnCompletion();
 
     // gets the cache-data-location from the struct