Skip to content
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

runtime: merge workarounds to false deadlocks #40518

Open
prattmic opened this issue Jul 31, 2020 · 4 comments
Open

runtime: merge workarounds to false deadlocks #40518

prattmic opened this issue Jul 31, 2020 · 4 comments
Assignees
Labels
Milestone

Comments

@prattmic
Copy link
Member

@prattmic prattmic commented Jul 31, 2020

In #40368, I avoided false deadlocks from startm in sysmon by reserving M ids in advance to make it look like the new M is already running to checkdead.

Later, I discovered that @dvyukov fixed a nearly identical issue in #6070 by temporarily decrementing the number of idle locked M's, to make checkdead see an extra M. Slightly different approach, but same end result.

We should merge these two approaches back to one just to keep things simpler and easier to understand.

cc @aclements @mknyszek @ianlancetaylor

@aclements
Copy link
Member

@aclements aclements commented Jul 31, 2020

With your change to startm to reserve an ID while holding sched.lock, is the logic in sysmon even necessary?

Loading

@prattmic
Copy link
Member Author

@prattmic prattmic commented Jul 31, 2020

I'm almost certain that the incidlelocked logic in sysmon is unnecessary after my change. There is similar logic in retake that I'll need to look at more closely.

Loading

@aclements
Copy link
Member

@aclements aclements commented Dec 8, 2020

Bumping to 1.17, but @prattmic, please bump to Backlog if you don't plan on working on this in 1.17.

Loading

@aclements aclements removed this from the Go1.16 milestone Dec 8, 2020
@aclements aclements added this to the Go1.17 milestone Dec 8, 2020
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 30, 2021

@prattmic Is anything going to happen in 1.17? If not, we should change the milestone to Backlog. Thanks.

Loading

@prattmic prattmic removed this from the Go1.17 milestone May 3, 2021
@prattmic prattmic added this to the Backlog milestone May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants