runtime: drop nosplit from primary lockrank functions
acquireLockRank and releaseLockRank are called from nosplit context, and
thus must be nosplit.
lockWithRank, unlockWithRank, and lockWithRankMayAcquire are called from
spittable context, and thus don't strictly need to be nosplit.
The stated reasoning for making these functions nosplit is to avoid
re-entrant calls due to a stack split on function entry taking a lock.
There are two potential issues at play here:
1. A stack split on function entry adds a new lock ordering edge before
we (a) take lock l, or (b) release lock l.
2. A stack split in a child call (such as to lock2) introduces a new
lock ordering edge _in the wrong order_ because e.g., in the case of
lockWithRank, we've noted that l is taken, but the stack split in
lock2 actually takes stack split locks _before_ l is actually locked.
(1) is indeed avoided by marking these functions nosplit, but this is
really just a bit of duct tape that generally has no effect overall. Any
earlier call can have a stack split and introduce the same new edge.
This includes lock/unlock which are not nosplit!
I began this CL as a change to extend nosplit to lock and unlock to try
to make this mitigation more effective, but I've realized that as long
as there is a _single_ nosplit call between a lock and unlock, we can
end up with the edge. There seems to be few enough cases without any
calls that is does not seem worth the extra cognitive load to extend
nosplit throughout all of the locking functions.
(2) is a real issue which would cause incorrect ordering, but it is
already handled by switching to the system stack before recording the
lock ordering. Adding / removing nosplit has no effect on this issue.
Change-Id: I94fbd21b2bf928dbf1bf71aabb6788fc0a012829
Reviewed-on: https://go-review.googlesource.com/c/go/+/254367
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Dan Scales <danscales@google.com>
Trust: Michael Pratt <mpratt@google.com>