Skip to content

Release GIL in blocking AIO bindings - #8196

Merged
tohtana merged 4 commits into
deepspeedai:masterfrom
tohtana:tohtana/fix-aio-blocking-gil-deadlock
Jul 31, 2026
Merged

tohtana merged 4 commits into
deepspeedai:masterfrom
tohtana:tohtana/fix-aio-blocking-gil-deadlock

Conversation

@tohtana

@tohtana tohtana commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

The blocking parallel AIO bindings can call the handle's internal wait() while retaining the Python GIL. An AIO worker may need that GIL while completing PyTorch tensor cleanup. This causes a deadlock: the Python caller waits for the worker, while the worker waits for the GIL held by the caller.

This PR releases the GIL at the pread, pwrite, sync_pread, and sync_pwrite pybind entrypoints to avoid the deadlock.

Signed-off-by: Masahiro Tanaka <mtanaka@anyscale.com>
@tohtana
tohtana requested review from loadams and tjruwase as code owners July 30, 2026 17:55

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bab900afc3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread csrc/aio/py_lib/py_ds_aio.cpp
Comment thread csrc/aio/py_lib/py_ds_aio.cpp
Comment thread csrc/aio/py_lib/py_ds_aio.cpp Outdated
Comment thread tests/unit/v1/nvme/test_aio_gil.py
Masahiro Tanaka added 3 commits July 30, 2026 16:16
Signed-off-by: Masahiro Tanaka <mtanaka@anyscale.com>
Signed-off-by: Masahiro Tanaka <mtanaka@anyscale.com>
Signed-off-by: Masahiro Tanaka <mtanaka@anyscale.com>
@sfc-gh-truwase

Copy link
Copy Markdown
Collaborator

@tohtana on further thoughts, I think my perf concerns are invalid because the changes swap GIL for per-handle mutex, and so it should actually improve perf. Do you agree?

@sfc-gh-truwase

Copy link
Copy Markdown
Collaborator

@tohtana do you think similar GIL concerns also apply to the GDS variant:
https://github.com/tohtana/DeepSpeed/blob/master/csrc/gds/py_lib/py_ds_gds.cpp

@tohtana

tohtana commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

Hi @sfc-gh-truwase,

the changes swap GIL for per-handle mutex, and so it should actually improve perf. Do you agree?

Yes, basically it won't hurt performance. We have much smaller critical sections now. When I briefly ran the benchmark, read showed a stable performance (10.066 → 10.029 GB/s). But the write showed a lot of fluctuation in my env and I currently don't have reliable numbers.

@tohtana do you think similar GIL concerns also apply to the GDS variant:
https://github.com/tohtana/DeepSpeed/blob/master/csrc/gds/py_lib/py_ds_gds.cpp

I think the concern is valid, and the exact same approach as this PR is not sufficient. We would need additional work to protect resources specific to GDS. It should be a follow-up PR.

@tohtana
tohtana added this pull request to the merge queue Jul 31, 2026
Merged via the queue into deepspeedai:master with commit e2aae1b Jul 31, 2026
13 checks passed
@tohtana
tohtana deleted the tohtana/fix-aio-blocking-gil-deadlock branch July 31, 2026 17:49
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