Skip to content

[SYCL] Fix deadlock in Scheduler on Windows #1703

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.

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

Merged
merged 5 commits into from
May 20, 2020
Merged

[SYCL] Fix deadlock in Scheduler on Windows #1703

merged 5 commits into from
May 20, 2020

Conversation

dm-vodopyanov
Copy link
Contributor

@dm-vodopyanov dm-vodopyanov commented May 18, 2020

There was a deadlock in Scheduler on Windows only. This deadlock happens in implementation of std::shared_timed_mutex of MSVC's C++ std library.
The deadlock on Windows happens because lock ordering is not specified in C++ spec when we do lock and lock_shared simultaneously, and lock and lock_shared both wait.
It can be fixed in 2 ways:

  1. upgrade DPC++ runtime to C++17 and change std::shared_timed_mutex to std::shared_mutex, there will be no deadlock on Windows out of the box. No regression on Linux.
  2. implement spinlock

We can't upgrade to C++17 yet, so spinlock was implemented. It fixed the deadlock on Windows. But it creates another deadlock, this time on Linux only in implementation of lock and shared_lock in pthread. To fix this, added #ifdef _WIN32 to separate Windows and Linux parts of code.

@dm-vodopyanov dm-vodopyanov requested a review from a team as a code owner May 18, 2020 09:06
@dm-vodopyanov dm-vodopyanov requested a review from s-kanaev May 18, 2020 09:06
s-kanaev
s-kanaev previously approved these changes May 18, 2020
Copy link
Contributor

@s-kanaev s-kanaev left a comment

Choose a reason for hiding this comment

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

LGTM

@bader
Copy link
Contributor

bader commented May 20, 2020

@dm-vodopyanov, could you resolve conflicts, please?

@dm-vodopyanov
Copy link
Contributor Author

@bader conflicts resolved.

@bader bader merged commit ebace77 into intel:sycl May 20, 2020
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.

3 participants