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

Jump to bottom

runtime: add Gscan to the lock ranking order #38922

Closed
danscales opened this issue May 7, 2020 · 4 comments
Closed

runtime: add Gscan to the lock ranking order #38922

danscales opened this issue May 7, 2020 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.15

Comments

@danscales
Copy link
Contributor

I'd like to check in this lock ranking update, https://go-review.googlesource.com/c/go/+/228417, that inserts the Gscan bit into the locking order. This has been in process for a while, but only just finished up now. This change is useful for documentation and checking of the ordering between locks and the Gscan bit. Just in working on this change, I've added more documentation on the almost-cycle between the Gscan bit and the hchan locks (not actually possible because the relevant G is suspended in the case of the unusual Gscan -> hchan ordering), and also ran into a possible deadlock (very unlikely) involving the paniclk and allg locks.

Since this code is only activated when GOEXPERIMENT=staticlockranking, this should be quite safe. We have a builder now that automatically runs staticlockranking for linux-amd64 on each check in.

/cc @andybons @rsc
@danscales danscales added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label May 7, 2020
@FiloSottile
Copy link
Contributor

This was mailed before the freeze, so IIUC it's fine to merge until EOD today (one week after the freeze).

@gopherbot
Copy link

Change https://golang.org/cl/228417 mentions this issue: runtime: incorporate Gscan acquire/release into lock ranking order

@danscales
Copy link
Contributor Author

This was mailed before the freeze, so IIUC it's fine to merge until EOD today (one week after the freeze).

OK, I see, but I'll let @andybons confirm.

@andybons
Copy link
Member

andybons commented May 7, 2020

yep you’re good.

@andybons andybons changed the title freeze exception: runtime: add Gscan to the lock ranking order runtime: add Gscan to the lock ranking order May 7, 2020
@andybons andybons added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels May 7, 2020
@andybons andybons added this to the Go1.15 milestone May 7, 2020
@chainhelen chainhelen mentioned this issue Aug 17, 2020
Closed
@golang golang locked and limited conversation to collaborators May 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.15
Development

No branches or pull requests
4 participants
@andybons @FiloSottile @danscales @gopherbot