venus: guest vram race conditions between shmem resource import and acquire
Problem
During dEQP-VK tests execution, it was found that on the host side virglrendrerer occasionally reports about invalid resource IDs and fails corresponding tests. The same problem can be reproduced by running vkjson in the loop >100 times in loop:
while
do
echo "Run vkjson $i"
let i++
adb shell "cmd gpu vkjson > /dev/null"
echo done
sleep 1
done
On the host side, the following errors appearing:
virgl_render_server[1007]: vkr: failed to set reply stream: invalid res_id 516
virgl_render_server[1007]: vkr: vkSetReplyCommandStreamMESA resulted in CS error
virgl_render_server[1007]: vkr: ring_submit_cmd: vn_dispatch_command failed
virgl_render_server[1186]: vkr: failed to set reply stream: invalid res_id 606
virgl_render_server[1186]: vkr: vkSetReplyCommandStreamMESA resulted in CS error
virgl_render_server[1186]: vkr: ring_submit_cmd: vn_dispatch_command failed
virgl_render_server[1269]: vkr: failed to set reply stream: invalid res_id 648
virgl_render_server[1269]: vkr: vkSetReplyCommandStreamMESA resulted in CS error
virgl_render_server[1269]: vkr: ring_submit_cmd: vn_dispatch_command failed
Analysis
After adding more detailed logs and some verbosity, it's visible, that missing resource #470, that used by vkSetReplyCommandStreamMESA command, was imported to vkr_renderer after the exact command execution:
52663 virgl_render_server[903]: vkr: vkr_context_import_resource_internal res_id 469 [tid 903]
52664 virgl_render_server[903]: vkr: vkr_context_submit_cmd [tid 903]
52665 virgl_render_server[903]: vkr: vkCreateRingMESA get command [tid 903]
52666 virgl_render_server[903]: vkr: vkCreateRingMESA done command [tid 903]
52667 virgl_render_server[903]: vkr: vkr_ring_submit_cmd [tid 905]
52668 virgl_render_server[903]: vkr: vkSetReplyCommandStreamMESA get command [tid 905]
52669 virgl_render_server[903]: vkr: failed to set reply stream: invalid res_id 470
52670 virgl_render_server[903]: vkr: vkSetReplyCommandStreamMESA done command [tid 905]
52671 virgl_render_server[903]: vkr: vkSetReplyCommandStreamMESA resulted in CS error
52672 virgl_render_server[903]: vkr: ring_submit_cmd: vn_dispatch_command failed
52673 virgl_render_server[903]: vkr: vkr_context_import_resource_internal res_id 470 [tid 903]
52674 virgl_render_server[906]: vkr: vkr_context_import_resource_internal res_id 471 [tid 906]
From virglrenderer side:
From the code of virglrenderer and logs above. it's visible, that we have two threads in work:
Thread_1 [TID 903]: This thread polls socket IPC, gets IMPORT_RESOURCE/CREATE_RESOURCE/SUBMIT_CMD commands and particularly invocates vkr_context_import_resource_internal().
Thread_2 [TID 905]: This thread is created by the handler of vkCreateRingMESA and listens vkr-ring and executes vkr commands. Particularly from this thread happening invocation execution of vkSetReplyCommandStreamMESA command.
Execution of vkCreateRingMESA command triggered by SUBMIT_CMD command in Thread_1 after IMPORT_RESOURCE processing, and will always succeed. vkCreateRingMESA command handler starts vkr-ring's Thread_2 and reading of command vkSetReplyCommandStreamMESA already happening in new Thread_2 without any sync to Thread_1.
Therefore, for commands that are already placed into vkr-ring, possible scenario when resources are still not imported or not created.
From Mesa side:
Mesa handles instance creation in vn_CreateInstance and after vkCreateRingMESA invocates vn_call_vkEnumerateInstanceVersion() command. This command contains a long reply and vn_ring_submit_command() triggers vkr shmem expansion.
The exact problem is that we don't have any sync between vn_instance_reply_shmem_alloc() and vn_ring_set_reply_shmem_locked() in code of Mesa:
void
vn_ring_submit_command(struct vn_ring *ring,
struct vn_ring_submit_command *submit)
{
assert(!vn_cs_encoder_is_empty(&submit->command));
vn_cs_encoder_commit(&submit->command);
size_t reply_offset = 0;
if (submit->reply_size) {
submit->reply_shmem = vn_instance_reply_shmem_alloc(
ring->instance, submit->reply_size, &reply_offset);
if (!submit->reply_shmem)
return;
}
mtx_lock(&ring->mutex);
if (submit->reply_size) {
vn_ring_set_reply_shmem_locked(ring, submit->reply_shmem, reply_offset,
submit->reply_size);
}
At the same time vn_ring_set_reply_shmem_locked() has a dependency on previously created shmem resource:
static inline void
vn_ring_set_reply_shmem_locked(struct vn_ring *ring,
....
const struct VkCommandStreamDescriptionMESA stream = {
.resourceId = shmem->res_id,
.offset = offset,
.size = size,
};
vn_encode_vkSetReplyCommandStreamMESA(&local_enc, 0, &stream);
....
And when vn_ring_set_reply_shmem_locked() puts the vkSetReplyCommandStreamMESA command into the ring and sets the resource ID of allocated shmem, we have no guarantee that the resource has already been created and imported on the host side.
Possible solution
Looking into the vn_ring_submit_command() visible that in general such race conditions are possible not only with vn_call_vkEnumerateInstanceVersion command but with any command that contains dependency on previously created resource.
The possible solution to fix this problem is to create a waiting list of resources. When _vkr_context_get_re_source() is called from vkr-ring thread and fails to find a resource, it puts the resource into the waiting list and blocks for a reasonable time, until the resource is imported by vkr_context_import_resource_internal() in another thread. Then it signals cond var and unblocks vkr-ring thread back.
Draft implementation of this proposal I'll share in the branch: https://gitlab.freedesktop.org/andrewgazizov/virglrenderer/-/commits/resource_races
Any comments on this problem and proposal are welcomed. Thanks in advance!
/cc @zzyiwei