Freeing the commands and the command chain created during dispatch

[eddierichter-amd]

No description provided.

[makslevental]

I can't comment since it isn't a touched line but don't you need to free the cmds themselves too?

ROCR-Runtime/runtime/hsa-runtime/core/runtime/amd_aie_aql_queue.cpp

Line 378 in 54b07d9

if (CreateCmd(cmd_size, &cmd_bo_handle, &cmd, fd))

also in this file is the ring_buf_ which is allocated but never freed (like in AqlQueue)

ROCR-Runtime/runtime/hsa-runtime/core/runtime/amd_aie_aql_queue.cpp

Line 104 in 54b07d9

ring_buf_ = agent_.system_allocator()(queue_size_bytes_, 4096,

[eddierichter-amd]

I can't comment since it isn't a touched line but don't you need to free the cmds themselves too?

ROCR-Runtime/runtime/hsa-runtime/core/runtime/amd_aie_aql_queue.cpp

Line 378 in 54b07d9

if (CreateCmd(cmd_size, &cmd_bo_handle, &cmd, fd))

also in this file is the ring_buf_ which is allocated but never freed (like in AqlQueue)

ROCR-Runtime/runtime/hsa-runtime/core/runtime/amd_aie_aql_queue.cpp

Line 104 in 54b07d9

ring_buf_ = agent_.system_allocator()(queue_size_bytes_, 4096,

We go through all of the cmd_handles and free all of them here: https://github.com/nod-ai/ROCR-Runtime/pull/29/files#diff-d794c4f5b69c550f71f08837a96b68bb2dad1cd8750f1c41623a92d4dcef9a7eR444-R447. Are there some commands that we are missing? Good point on the queue will add that to this PR as well.

[makslevental]

sorry I should've been more specific - shouldn't the buffers themselves be munmaped? Like I did here in the other PR https://github.com/nod-ai/ROCR-Runtime/pull/28/files#diff-d794c4f5b69c550f71f08837a96b68bb2dad1cd8750f1c41623a92d4dcef9a7e

[eddierichter-amd]

sorry I should've been more specific - shouldn't the buffers themselves be munmaped? Like I did here in the other PR https://github.com/nod-ai/ROCR-Runtime/pull/28/files#diff-d794c4f5b69c550f71f08837a96b68bb2dad1cd8750f1c41623a92d4dcef9a7e

Wow you are good! Didn't realize you had a PR that had this :). Alright I am both unmapping and closing as that is consistent with how the XRT shim is dealing with cmds (unmapping: https://github.com/amd/xdna-driver/blob/867e2f9f5f21596ec642e9ca5321de7a0863c14d/src/shim/bo.cpp#L59-L62 and closing: https://github.com/amd/xdna-driver/blob/867e2f9f5f21596ec642e9ca5321de7a0863c14d/src/shim/bo.cpp#L23-L27) if we learn that this is redundant happy to remove closing or unmapping. Also added the queue ring buf deallocation via the system deallocator.

[makslevental]

LGTM - in my local testing of basically the same changes this made most (all?) of the kernel leaks go away.