Skip to content

[L0] Enable Disabling of Queue lock during L0 Sync calls #1492

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 15, 2024
Merged

[L0] Enable Disabling of Queue lock during L0 Sync calls #1492

merged 1 commit into from
Apr 15, 2024

Conversation

nrspruit
Copy link
Contributor

@nrspruit nrspruit commented Apr 4, 2024 •
edited
Loading

  • Prevent Queue Lock deadlock while L0 queue sync or host sync is in progress.

@nrspruit nrspruit force-pushed the l0_queue_sync_unblocking branch from ed1e677 to b28525c Compare April 4, 2024 17:34
@nrspruit nrspruit requested a review from pbalcer April 4, 2024 17:37
@nrspruit nrspruit force-pushed the l0_queue_sync_unblocking branch from edd5ae9 to 8e028ee Compare April 4, 2024 23:20
@nrspruit nrspruit mentioned this pull request Apr 4, 2024
Closed
pbalcer
pbalcer reviewed Apr 5, 2024
View reviewed changes
@nrspruit nrspruit force-pushed the l0_queue_sync_unblocking branch from 8e028ee to a0b84e0 Compare April 5, 2024 21:07
@nrspruit nrspruit marked this pull request as ready for review April 5, 2024 21:08
@nrspruit nrspruit requested a review from a team as a code owner April 5, 2024 21:08
@nrspruit nrspruit changed the title L0 queue sync unblocking [L0] Enable Disabling of Queue lock during L0 Sync calls Apr 5, 2024
nrspruit added a commit to nrspruit/llvm that referenced this pull request Apr 5, 2024
@nrspruit
[UR][L0] Enable Disabling of Queue lock during L0 Sync calls
c165181 
- pre-commit PR for oneapi-src/unified-runtime#1492

Signed-off-by: Neil R. Spruit <[email protected]>
pbalcer
pbalcer approved these changes Apr 8, 2024
View reviewed changes
Copy link
Contributor

@pbalcer pbalcer left a comment

Choose a reason for hiding this comment

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

If we merge this, we should also be able to revert #1371 (but I'm not sure whether we should given the opt-out UrL0QueueSyncNonBlocking flag)

@nrspruit
Copy link
Contributor Author

nrspruit commented Apr 8, 2024 •
edited
Loading

If we merge this, we should also be able to revert #1371 (but I'm not sure whether we should given the opt-out UrL0QueueSyncNonBlocking flag)

So, until we feel "safe" to remove the opt-out flag, then I would probably leave that change in just in case. The opt-out flag is to catch any possible corner cases in the first release, then it may be deprecated.

@nrspruit nrspruit added level-zero L0 adapter specific issues ready to merge Added to PR's which are ready to merge v0.9.x Include in the v0.9.x release labels Apr 8, 2024
@kbenzie
Copy link
Contributor

kbenzie commented Apr 12, 2024

@pbalcer has approved this but this is still pending review from the @oneapi-src/unified-runtime-level-zero-write team. Should @pbalcer (and others?) be in that review team?

@nrspruit
Copy link
Contributor Author

@pbalcer has approved this but this is still pending review from the @oneapi-src/unified-runtime-level-zero-write team. Should @pbalcer (and others?) be in that review team?

@raiyanla is also part of that team so it should have caught his approval, does he need to reapprove?

@kbenzie
Copy link
Contributor

kbenzie commented Apr 12, 2024

@raiyanla is also part of that team so it should have caught his approval, does he need to reapprove?

Weird, perhaps this was a draft PR when that approval happened? Or GitHub being weird. Either way I think its fine, was just a thought about process moving forwards.

@aarongreig aarongreig merged commit 4177e93 into oneapi-src:main Apr 15, 2024
@aarongreig aarongreig mentioned this pull request Apr 15, 2024
Merged
martygrant pushed a commit to intel/llvm that referenced this pull request Apr 16, 2024
@aarongreig
[UR][L0] Pull in a batch of L0 fixes (#13400)
684cd90 
Pulls in fixes
oneapi-src/unified-runtime#1492
oneapi-src/unified-runtime#1494
oneapi-src/unified-runtime#1507
kbenzie pushed a commit to kbenzie/unified-runtime that referenced this pull request Apr 16, 2024
@aarongreig @kbenzie
Merge pull request oneapi-src#1492 from nrspruit/l0_queue_sync_unbloc…
7c3419c 
…king

[L0] Enable Disabling of Queue lock during L0 Sync calls
@kbenzie kbenzie mentioned this pull request Apr 16, 2024
Merged
19 tasks
kbenzie pushed a commit that referenced this pull request Apr 17, 2024
@aarongreig @kbenzie
Merge pull request #1492 from nrspruit/l0_queue_sync_unblocking
4f6997e 
[L0] Enable Disabling of Queue lock during L0 Sync calls
kbenzie pushed a commit to kbenzie/intel-llvm that referenced this pull request Apr 18, 2024
@aarongreig @kbenzie
[UR][L0] Pull in a batch of L0 fixes (intel#13400)
de9de8f 
Pulls in fixes
oneapi-src/unified-runtime#1492
oneapi-src/unified-runtime#1494
oneapi-src/unified-runtime#1507
@kbenzie kbenzie mentioned this pull request Apr 18, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raiyanla raiyanla raiyanla approved these changes

@pbalcer pbalcer pbalcer approved these changes

@winstonzhang-intel winstonzhang-intel Awaiting requested review from winstonzhang-intel

Assignees
No one assigned
Labels
level-zero L0 adapter specific issues ready to merge Added to PR's which are ready to merge v0.9.x Include in the v0.9.x release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nrspruit @kbenzie @raiyanla @pbalcer @aarongreig