Skip to content

[CUDA] Make CudaEvent work with multi-device#2614

Merged
zcbenz merged 3 commits into
ml-explore:mainfrom
zcbenz:cuda-event-device
Sep 27, 2025
Merged

[CUDA] Make CudaEvent work with multi-device#2614
zcbenz merged 3 commits into
ml-explore:mainfrom
zcbenz:cuda-event-device

Conversation

@zcbenz

@zcbenz zcbenz commented Sep 23, 2025

Copy link
Copy Markdown
Collaborator

Most APIs of CUDA event have requirements on being used on the same device with the device that the event was created on, this PR makes sure the CudaEvent class set current device before invoking the APIs.

There is also an issue that the CudaEvent could be destroyed in worker thread when used for waiting on a GPU work in CPU stream, which would cause race conditions when accessing event cache, this PR uses a simple strategy by skipping the cache when event is created or destroyed on worker threads, so we don't have to add thread-safety overhead for those rare cases.

Note that this PR does not guarantee multi-thread or multi-device safety, this is mostly a defensive change.

@angeloskath angeloskath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great to me!

I am missing something here for sure, when will events be created and destroyed in a worker thread 🤔, I thought they would be created and destroyed in the eval loop. I guess that this is precisely your point, they will only be created and destroyed in the eval loop but in case they are created and destroyed somewhere else then skip the cache ...

@zcbenz

zcbenz commented Sep 26, 2025

Copy link
Copy Markdown
Collaborator Author

when will events be created and destroyed in a worker thread

Waiting a CPU stream is implemented by copying the event to the CPU thread, so the event could be destroyed there:

void wait(Stream s) {
if (s.device == mlx::core::Device::cpu) {
scheduler::enqueue(s, [*this]() mutable {
check_recorded();
event_->wait();
});
} else {

Another case is user may run eval() in workers, I did that to implement await eval() in node-mlx, and I also saw some users doing it: #2067

@zcbenz
zcbenz merged commit b466dea into ml-explore:main Sep 27, 2025
7 checks passed
@zcbenz
zcbenz deleted the cuda-event-device branch September 27, 2025 02:27
faisalmemon pushed a commit to faisalmemon/mlx that referenced this pull request Oct 30, 2025
* Set current device when creating cuda event

* Separate cuda events by device

* Avoid race condition in pool
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants