Skip to content

Commit

Permalink
Tidy up mappings when buffer map commands fail to execute (#552)
Browse files Browse the repository at this point in the history
Buffer mappings are created at command creation time. When the command
fails execution, either directly or because its dependencies failed,
the mapping needs to be destroyed.

Caught by the Vulkan validation layers.

Change-Id: I53eef3c4bc4c7f4c5f94ade39b16bde558e9da2c

Signed-off-by: Kévin Petit <[email protected]>
  • Loading branch information
kpet authored Jun 21, 2023
1 parent 0e86eb4 commit 9ff592c
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 1 deletion.
8 changes: 8 additions & 0 deletions src/memory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,14 @@ struct cvk_buffer : public cvk_mem {
return mapping;
}

void cleanup_mapping(cvk_buffer_mapping& mapping) {
std::lock_guard<std::mutex> lock(m_mappings_lock);
if (m_mappings.count(mapping.ptr)) {
m_mappings.erase(mapping.ptr);
}
mapping.buffer->unmap();
}

uint64_t device_address() const {
VkBufferDeviceAddressInfo info{};
info.buffer = vulkan_buffer();
Expand Down
5 changes: 5 additions & 0 deletions src/queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1289,6 +1289,7 @@ cl_int cvk_command_map_buffer::build(void** map_ptr) {
return CL_OUT_OF_RESOURCES;
}

m_mapping_needs_releasing_on_destruction = true;
*map_ptr = m_mapping.buffer->map_ptr(m_offset);

return CL_SUCCESS;
Expand All @@ -1307,6 +1308,10 @@ cl_int cvk_command_map_buffer::do_action() {
success = m_buffer->copy_to(dst, m_offset, m_size);
}

if (success) {
m_mapping_needs_releasing_on_destruction = false;
}

return success ? CL_COMPLETE : CL_OUT_OF_RESOURCES;
}

Expand Down
8 changes: 7 additions & 1 deletion src/queue.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,12 @@ struct cvk_command_map_buffer final : public cvk_command_buffer_base_region {
size_t offset, size_t size, cl_map_flags flags)
: cvk_command_buffer_base_region(queue, CL_COMMAND_MAP_BUFFER, buffer,
offset, size),
m_flags(flags) {}
m_flags(flags), m_mapping_needs_releasing_on_destruction(false) {}
~cvk_command_map_buffer() {
if (m_mapping_needs_releasing_on_destruction) {
m_buffer->cleanup_mapping(m_mapping);
}
}
CHECK_RETURN cl_int build(void** map_ptr);
CHECK_RETURN cl_int do_action() override final;

Expand All @@ -857,6 +862,7 @@ struct cvk_command_map_buffer final : public cvk_command_buffer_base_region {
private:
cl_map_flags m_flags;
cvk_buffer_mapping m_mapping;
bool m_mapping_needs_releasing_on_destruction;
};

struct cvk_command_unmap_buffer final : public cvk_command_buffer_base {
Expand Down

0 comments on commit 9ff592c

Please sign in to comment.