[UR][L0] Don't batch external semaphore operations - #22811
Conversation
External semaphore operations must not be deferred in a batch. With regular (non-immediate) command lists, batching leaves the append sitting in the queue's open command list until some unrelated event flushes it. For a signal that means the external consumer may never observe it; for a wait it means the external producer's signal cannot release a command list that was never submitted, which can deadlock.
There was a problem hiding this comment.
Pull request overview
This PR addresses a correctness issue in the Level Zero (L0) adapter where external semaphore wait/signal operations must be submitted promptly rather than being deferred via command-list batching, which can otherwise lead to unobservable signals or deadlocks.
Changes:
- Disable command batching for
urBindlessImagesWaitExternalSemaphoreExp. - Disable command batching for
urBindlessImagesSignalExternalSemaphoreExp. - Add in-code rationale documenting why batching external semaphore operations is unsafe on non-immediate command lists.
Comments suppressed due to low confidence (1)
unified-runtime/source/adapters/level_zero/image.cpp:188
- These functions enqueue work onto the queue (getAvailableCommandList / executeCommandList) but do not take hQueue->Mutex, unlike other enqueue paths in this adapter (e.g. urBindlessImagesImageCopyExp). That can cause data races on queue state such as the open command list / LastCommandEvent when the same queue is used concurrently.
// External semaphore operations must not be deferred in a batch. With
// regular (non-immediate) command lists, batching leaves the append sitting
// in the queue's open command list until some unrelated event flushes it.
// For a signal that means the external consumer may never observe it; for a
// wait it means the external producer's signal cannot release a command
| // External semaphore operations must not be deferred in a batch. With | ||
| // regular (non-immediate) command lists, batching leaves the append sitting | ||
| // in the queue's open command list until some unrelated event flushes it. | ||
| // For a signal that means the external consumer may never observe it; for a | ||
| // wait it means the external producer's signal cannot release a command |
| // External semaphore operations must not be deferred in a batch. With | ||
| // regular (non-immediate) command lists, batching leaves the append sitting | ||
| // in the queue's open command list until some unrelated event flushes it. | ||
| // For a signal that means the external consumer may never observe it; for a | ||
| // wait it means the external producer's signal cannot release a command |
|
I’m curious because the spec says external semaphore operations with Does this PR effectively relax that and allow the operations on regular (non-immediate) command-list queues? If yes, the bindless-images spec needs an update too. |
|
I think this comment is valid. @intel/unified-runtime-reviewers-level-zero Can you help with review? |
Correct. Patches to L0 runtime are pending release, but we need the UR fix asap. Documentation and E2E tests will follow at a later time. |
Thanks for clarification. I am okay with this approach. |
External semaphore operations must not be deferred in a batch. With regular (non-immediate) command lists, batching leaves the append sitting in the queue's open command list until some unrelated event flushes it. For a signal that means the external consumer may never observe it; for a wait it means the external producer's signal cannot release a command list that was never submitted, which can deadlock.