Protect changes to memory object mappings with a mutex (#541)

Both command enqueue and execution need to mutate mappings and they can be concurrent. Change-Id: Ic94b2a4ddb6170e1ed1b74399d51e9f39df1d3b7 Signed-off-by: Kévin Petit <[email protected]>
kpet · Jun 2, 2023 · b4f0d81 · b4f0d81
1 parent 12faaa4
commit b4f0d81
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 20 deletions.
diff --git a/src/memory.hpp b/src/memory.hpp
@@ -373,6 +373,7 @@ struct cvk_buffer : public cvk_mem {
     }
 
     bool insert_mapping(const cvk_buffer_mapping& mapping) {
+        std::lock_guard<std::mutex> lock(m_mappings_lock);
         auto num_mappings_with_same_pointer = m_mappings.count(mapping.ptr);
         // TODO support multiple mappings with the same pointer
         if (num_mappings_with_same_pointer != 0) {
@@ -385,6 +386,7 @@ struct cvk_buffer : public cvk_mem {
     }
 
     cvk_buffer_mapping remove_mapping(void* ptr) {
+        std::lock_guard<std::mutex> lock(m_mappings_lock);
         CVK_ASSERT(m_mappings.count(ptr) > 0);
         auto mapping = m_mappings.at(ptr);
         m_mappings.erase(ptr);
@@ -409,6 +411,7 @@ struct cvk_buffer : public cvk_mem {
 
     VkBuffer m_buffer;
     std::unordered_map<void*, cvk_buffer_mapping> m_mappings;
+    std::mutex m_mappings_lock;
 };
 
 using cvk_buffer_holder = refcounted_holder<cvk_buffer>;
@@ -581,6 +584,7 @@ struct cvk_image : public cvk_mem {
                                 std::array<size_t, 3> origin,
                                 std::array<size_t, 3> region,
                                 cl_map_flags flags, bool handle_host_ptr) {
+        std::lock_guard<std::mutex> lock(m_mappings_lock);
         // TODO try to reuse existing mappings
         // TODO add overlap checks
 
@@ -623,12 +627,14 @@ struct cvk_image : public cvk_mem {
             return false;
         }
 
+        // TODO should insertion be deferred, as done for buffers?
         m_mappings[mapping.ptr].push_back(mapping);
 
         return true;
     }
 
     cvk_image_mapping remove_mapping(void* ptr) {
+        std::lock_guard<std::mutex> lock(m_mappings_lock);
         CVK_ASSERT(m_mappings.count(ptr) > 0);
         auto mapping = m_mappings.at(ptr).front();
         m_mappings.at(ptr).pop_front();
@@ -643,6 +649,7 @@ struct cvk_image : public cvk_mem {
     }
 
     cvk_image_mapping mapping_for(void* ptr) {
+        std::lock_guard<std::mutex> lock(m_mappings_lock);
         CVK_ASSERT(m_mappings.count(ptr) > 0);
         auto mapping = m_mappings.at(ptr).front();
         return mapping;
@@ -738,6 +745,7 @@ struct cvk_image : public cvk_mem {
     VkImageView m_sampled_view;
     VkImageView m_storage_view;
     std::unordered_map<void*, std::list<cvk_image_mapping>> m_mappings;
+    std::mutex m_mappings_lock;
     std::unique_ptr<cvk_buffer> m_init_data;
 };
 

diff --git a/tests/conformance/results-amd-7950x.json b/tests/conformance/results-amd-7950x.json
@@ -759,16 +759,28 @@
       "retcode": 1
     },
     "Images (clFillImage max size)": {
-      "duration": "00:00:03.073710",
-      "has_results": false,
-      "results": {},
-      "retcode": -6
+      "duration": "00:00:12.015982",
+      "has_results": true,
+      "results": {
+        "1D": "pass",
+        "1Darray": "pass",
+        "2D": "pass",
+        "2Darray": "pass",
+        "3D": "pass"
+      },
+      "retcode": 0
     },
     "Images (clFillImage pitch)": {
-      "duration": "00:00:05.749350",
-      "has_results": false,
-      "results": {},
-      "retcode": -6
+      "duration": "00:00:11.917343",
+      "has_results": true,
+      "results": {
+        "1D": "pass",
+        "1Darray": "pass",
+        "2D": "pass",
+        "2Darray": "pass",
+        "3D": "pass"
+      },
+      "retcode": 0
     },
     "Images (clFillImage)": {
       "duration": "00:00:07.716860",

diff --git a/tests/conformance/results-intel-a750.json b/tests/conformance/results-intel-a750.json
@@ -759,22 +759,40 @@
       "retcode": 1
     },
     "Images (clFillImage max size)": {
-      "duration": "00:00:11.033466",
-      "has_results": false,
-      "results": {},
-      "retcode": -11
+      "duration": "00:00:34.566830",
+      "has_results": true,
+      "results": {
+        "1D": "pass",
+        "1Darray": "pass",
+        "2D": "pass",
+        "2Darray": "pass",
+        "3D": "pass"
+      },
+      "retcode": 0
     },
     "Images (clFillImage pitch)": {
-      "duration": "00:00:26.927571",
-      "has_results": false,
-      "results": {},
-      "retcode": -11
+      "duration": "00:00:32.019595",
+      "has_results": true,
+      "results": {
+        "1D": "pass",
+        "1Darray": "pass",
+        "2D": "pass",
+        "2Darray": "pass",
+        "3D": "pass"
+      },
+      "retcode": 0
     },
     "Images (clFillImage)": {
-      "duration": "00:00:13.602608",
-      "has_results": false,
-      "results": {},
-      "retcode": -6
+      "duration": "00:00:26.987112",
+      "has_results": true,
+      "results": {
+        "1D": "pass",
+        "1Darray": "pass",
+        "2D": "pass",
+        "2Darray": "pass",
+        "3D": "pass"
+      },
+      "retcode": 0
     },
     "Images (clReadWriteImage max size)": {
       "duration": "00:00:08.666396",