[go] runtime: eliminate _Psyscall
Michael Knyszek (Gerrit)
Michael Knyszek added 12 comments![]( "Open in Gerrit")
// Double-check that we didn't make a grave mistake. If the G is running and not
// coming out of a syscall, then we cannot safely read its stack.
//Michael KnyszekThis is fine in the context of reading this CL, but I think out of context it could be cryptic. I think this should explicitly mention the window in exitsyscall where we are Grunning without a P.
Done
// Careful: once we enter _Gsyscall, our M's P wiring is no longer ours to modify.Michael KnyszekIsn't this irrelevant? The P is already gone.
Instead this should probably be a warning about the small window of _Grunning without a P.
Also, while I'm not sure about permanent testing, you could do some one-off testing by adding some spinning here to increase the probability of others observing a G in this state.
fixed the comment, leaving open for testing
Michael KnyszekOh, another window, fun!
this one is the more problematic one in general, it seems. luckily there seems to be very little that relies on an invariant like a running G having a P, since readgstatus' result is always slightly in the past. you can't assume it has a P.
(suspendG assumes it has an M, but that's fine. goroutine profiling cares, but only for a runtime assertion. but who knows, there may be more.)
// lost our P or not (determined by exitsyscallfast).Michael Knyszekupdate
Done
// Trace that we're starting again, because there was a tracev2.GoSysBlockMichael KnyszekOops, tracev2.GoSysBlock doesn't even exist.
Done
if sched.stopwait == freezeStopWait || (sched.disable.user && !schedEnabled(gp)) {Michael KnyszekWhy add this? It seems reasonable, I'm wondering why it wasn't here before.
I... don't remember why I added this. I'll move it into a follow-up CL.
incidlelocked(-1)Michael KnyszekThis is no longer necessary because exitsyscall can't reach mput until after we resume, right?
hm, on second thought, this may still be necessary unless we call handoffp before resuming? I'm thinking like:
sysmon: stop T1 in _Gsyscall
sysmon: take T1's P
sysmon: resume T1
T1: exitsyscallNoP -> stopm -> mput (everything is idle)
I added it back in.
if status != _Gsyscall && status != _Gextra {Michael KnyszekWhy does this continue for extra M Gs?
added a comment. it's because extra M Gs might have a P attached to them. it's like an extension of the syscall state.
// It is assigned an M and a P (g.m and g.m.p are valid).Michael KnyszekNote that it might not have a P (when observed externally).
Done
// goroutine. It is not on a run queue. It is assigned an M.Michael KnyszekNote that it might not have a P.
Done
// p is the currently atttached P for executing Go code, nil if not executing user Go code.Michael Knyszekattached
Done
Related details
- Austin Clements
- Cherry Mui
- Ian Lance Taylor
- Michael Pratt
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Knyszek (Gerrit)
Michael Knyszek voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Michael Knyszek (Gerrit)
Michael Knyszek voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Austin Clements
- Cherry Mui
- Ian Lance Taylor
- Michael Pratt
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |