layer: add DeviceNotToTrace guards in GetDeviceQueue and QueueSubmit - #51
Conversation
|
This was just something I noticed :) |
rjodinchr
left a comment
rjodinchr
left a comment
There was a problem hiding this comment.
I'm fine with most of it.
I think that we still want to keep the TRACE_EVENT even when we do not trace the device.
I believe "DeviceNotToTrace" represents a device where we cannot track the submissions but no reason not to have perfetto traces for the call to those functions.
Maybe we would also want to add a field in the trace to mention that the device will not be traced (meaning, we will not follow each submission).
What do you think?
44f88b5 to
68e6b43
Compare
|
I've moved the I'll push a second update to the commit that implements the additional field in the trace. |
68e6b43 to
b2144d4
Compare
Previously, devices with missing dispatch table entries were added to DeviceNotToTrace and vksp_GetDeviceProcAddr would skip returning the layer's intercepted functions for them, meaning no Perfetto traces were emitted at all. Instead, always return the layer's intercepted functions so that Perfetto traces are emitted for all devices. Add a trace event at device creation to indicate when a device will not have its submissions tracked. Guard submission-tracking setup in vksp_GetDeviceQueue and vksp_QueueSubmit so they skip the timeline semaphore thread machinery for DeviceNotToTrace devices, avoiding null function pointer crashes.
b2144d4 to
060337d
Compare
| { | ||
| std::lock_guard<std::mutex> lock(glock); | ||
|
|
||
| if (DeviceNotToTrace.count(device) == 0) { |
There was a problem hiding this comment.
I understand that even if we don't want to follow the submissions, we still want to trace as much as possible.
But what would happen if vksp_GetDeviceProcAddr is looking for something we don't have. It feels like, it will call the vksp_<X> function, which itself will try to call the function from the dispatch table which has not been filled. So a potential segfault could occur.
Am I missing something? Should we protect against that?
There was a problem hiding this comment.
Oh, good catch. Yes. I've added a commit that should protect against that.
Only return the vksp_* wrapper from GetDeviceProcAddr when the corresponding dispatch table entry is non-null. If a function was missing during device creation (causing the device to be added to DeviceNotToTrace), fall through to the real driver's GetDeviceProcAddr instead of returning a wrapper that would segfault.
2 participants
If a device was marked as non-traceable (e.g., due to a missing function), GetDeviceQueue would still allocate tracing infrastructure and QueueSubmit would dereference uninitialized state. Add early-out checks for DeviceNotToTrace in both functions, and move the QueueToDevice assignment before the guard so the queue-to-device mapping is always available for QueueSubmit's passthrough path.