sync.SeqCount relies on the following memory orderings:
- All stores following BeginWrite() in program order happen after the atomic
read-modify-write (RMW) of SeqCount.epoch. In the Go 1.19 memory model, this
is implied by atomic loads being acquire-seqcst.
- All stores preceding EndWrite() in program order happen before the RMW of
SeqCount.epoch. In the Go 1.19 memory model, this is implied by atomic stores
being release-seqcst.
- All loads following BeginRead() in program order happen after the load of
SeqCount.epoch. In the Go 1.19 memory model, this is implied by atomic loads
being acquire-seqcst.
- All loads preceding ReadOk() in program order happen before the load of
SeqCount.epoch. The Go 1.19 memory model does not imply this property.
The x86 memory model *does* imply this final property, and in practice the
current Go compiler does not reorder memory accesses around the load of
SeqCount.epoch, so sync.SeqCount behaves correctly on x86.
However, on ARM64, the instruction that is actually emitted for the atomic load
from SeqCount.epoch is LDAR:
```
gvisor/pkg/sentry/kernel/kernel.SeqAtomicTryLoadTaskGoroutineSchedInfo():
gvisor/pkg/sentry/kernel/seqatomic_taskgoroutineschedinfo_unsafe.go:34
56371c: f9400025 ldr x5, [x1]
563720: f9400426 ldr x6, [x1, #8]
563724: f9400822 ldr x2, [x1, #16]
563728: f9400c23 ldr x3, [x1, #24]
gvisor/pkg/sentry/kernel/seqatomic_taskgoroutineschedinfo_unsafe.go:36
56372c: d503201f nop
gvisor/pkg/sync/sync.(*SeqCount).ReadOk():
gvisor/pkg/sync/seqcount.go:107
563730: 88dffc07 ldar w7, [x0]
563734: 6b0400ff cmp w7, w4
```
LDAR is explicitly documented as not implying the required memory ordering:
https://developer.arm.com/documentation/den0024/latest/Memory-Ordering/Barriers/One-way-barriers
Consequently, SeqCount.ReadOk() is incorrectly memory-ordered on weakly-ordered
architectures. To fix this, we need to introduce an explicit memory fence.
On ARM64, there is no way to implement the memory fence in question without
resorting to assembly, so the implementation is straightforward. On x86, we
introduce a compiler fence, since future compilers might otherwise reorder
memory accesses to after atomic loads; the only apparent way to do so is also
by using assembly, which unfortunately introduces overhead:
- After the call to sync.MemoryFenceReads(), callers zero XMM15 and reload the
runtime.g pointer from %fs:-8, reflecting the switch from ABI0 to
ABIInternal. This is a relatively small cost.
- Before the call to sync.MemoryFenceReads(), callers spill all registers to
the stack, since ABI0 function calls clobber all registers. The cost of this
depends on the state of the caller before the call, and is not reflected in
BenchmarkSeqCountReadUncontended (which does not read any protected state
between the calls to BeginRead() and ReadOk()).
Both of these problems are caused by Go assembly functions being restricted to
ABI0. Go provides a way to mark assembly functions as using ABIInternal
instead, but restricts its use to functions in package runtime
(https://github.com/golang/go/issues/44065). runtime.publicationBarrier(),
which is semantically "sync.MemoryFenceWrites()", is implemented as a compiler
fence on x86; defining sync.MemoryFenceReads() as an alias for that function
(using go:linkname) would mitigate the former problem, but not the latter.
Thus, for simplicity, we define sync.MemoryFenceReads() in (ABI0) assembly, and
have no choice but to eat the overhead.
("Fence" and "barrier" are often used interchangeably in this context; Linux
uses "barrier" (e.g. `smp_rmb()`), while C++ uses "fence" (e.g.
`std::atomic_memory_fence()`). We choose "fence" to reduce ambiguity with
"write barriers", since Go is a GC'd language.)
PiperOrigin-RevId: 573861378
The last remaining !go1.22 build is protecting the definition of
pkg/sync.maptype, which is a copy of runtime.maptype. We need to ensure these
definitions match so we can safely access the hasher field.
At its core, this CL achieves this check by ensuring that
unsafe.Offsetof(maptype{}.Hasher) matches the offset in the runtime version of
the type.
Several things happen along the way to achieve this:
* As of May 2023, runtime.maptype is actually a type alias for
internal/abi.MapType. checkoffset was failing to record the offsets because it
skipped type aliases for no good reason. Simply removing the type alias check
is sufficient to make type aliases work. (This part of the CL is technically
unnecessary because this CL ultimately references internal/abi.MapType
directly in anticipation of removal of the type alias. But there is no reason
not to allow type aliases).
* The checkconst / checkoffset regexp unintentionally does not allow / in
package paths, even though the rest of the package supports /. Fix this.
* checkconst was comparing the literal AST expression string against the
runtime value (i.e., "unsafe.Offsetof(maptype{}.Hasher)" vs "72", which fails
comparison. Switch to getting the resolved constant value from the type
checker.
* nogo/check.importer only loads package facts on direct import (stored in
importer.cache). If a package is not directly imported ImportPackageFact will
not find the facts. Typically packages need to ensure they directly depend on
packages they want facts from (e.g., pkg/sync has a dummy import of runtime in
runtime.go). This doesn't work for internal/abi because we cannot directly
import an internal package. Work around this as a hack by unconditionally
"importing" internal/abi when analyzing any package.
With regard to the last point, not that the nogo/defs.bzl nogo integration only
provides facts from the direct dependencies and the entire stdlib (since the
stdlib is analyzed as one bundle). So this trick only works for a stdlib
package. A bazel package indirect dependency would be missing facts altogether.
PiperOrigin-RevId: 549999084
This allows fact information to be validated in the underlying source files,
but requires us to explicitly maintain this in appropriate version-tagged, and
architecture-tagged files. This is more explicit and safer.
This mechanism uses a special regular expression for matching a +checkconst
stanza to validate constant values, sizes and offsets. This applies to both Go
source files and assembly files.
PiperOrigin-RevId: 511867507
This removes the need to check the offset every release.
I've also removed the negative build tag from runtime_amd64.go, which less
obviously correct. In theory, we should check that this package's use of
nmspinning is still valid in each release, but there is no way to automate that
and I don't realistically seeing checking happening beyond verifying tests
work. Additionally, the existing use is already suspect. :)
The good news is the misuse is likely to cause scheduling issues, not memory
corruption.
PiperOrigin-RevId: 505114683
Some synchronization patterns require the ability to simultaneously wake and
sleep a goroutine. For the sleep package, this is the case when a waker must be
asserted when a subsequent fetch is imminent.
Currently, this operation results in significant P churn in the runtime, which
ping-pongs execution between multiple system threads and cores and consumes a
significant amount of host CPU (and because of the context switches, this can
be significant worse with mitigations for side channel vulnerabilities).
The solution is to introduce a dedicated mechanism for a synchronous switch
which does not wake another runtime P (see golang/go#32113). This can be used
by the `AssertAndFetch` API in the sleep package.
The benchmark results for this package are very similiar to raw channel
operations for all cases, with the exception of operations that do not wait.
The primary advantage is more precise control over scheduling. This will be
used in a subsequent change.
```
BenchmarkGoAssertNonWaiting
BenchmarkGoAssertNonWaiting-8 261364384 4.976 ns/op
BenchmarkGoSingleSelect
BenchmarkGoSingleSelect-8 20946358 57.77 ns/op
BenchmarkGoMultiSelect
BenchmarkGoMultiSelect-8 6071697 197.0 ns/op
BenchmarkGoWaitOnSingleSelect
BenchmarkGoWaitOnSingleSelect-8 4978051 235.4 ns/op
BenchmarkGoWaitOnMultiSelect
BenchmarkGoWaitOnMultiSelect-8 2309224 520.2 ns/op
BenchmarkSleeperAssertNonWaiting
BenchmarkSleeperAssertNonWaiting-8 447325033 2.657 ns/op
BenchmarkSleeperSingleSelect
BenchmarkSleeperSingleSelect-8 21488844 55.19 ns/op
BenchmarkSleeperMultiSelect
BenchmarkSleeperMultiSelect-8 21851674 54.89 ns/op
BenchmarkSleeperWaitOnSingleSelect
BenchmarkSleeperWaitOnSingleSelect-8 2860327 416.4 ns/op
BenchmarkSleeperWaitOnSingleSelectSync
BenchmarkSleeperWaitOnSingleSelectSync-8 2741733 427.1 ns/op
BenchmarkSleeperWaitOnMultiSelect
BenchmarkSleeperWaitOnMultiSelect-8 2867484 418.1 ns/op
BenchmarkSleeperWaitOnMultiSelectSync
BenchmarkSleeperWaitOnMultiSelectSync-8 2789158 427.9 ns/op
```
PiperOrigin-RevId: 415581417
- Don't attempt to create directory is controller is not
present in the system
- Ensure that all files being written exist in cgroupfs
- Attempt to delete directories during Uninstall even if
other deletions have failed
Fixes#6446
PiperOrigin-RevId: 402614820
The presence of multiple packages in a single directory sometimes
confuses `go mod`, producing output like:
go: downloading gvisor.dev/gvisor v0.0.0-20210601174640-77dc0f5bc94d
$GOMODCACHE/gvisor.dev/gvisor@v0.0.0-20210601174640-77dc0f5bc94d/pkg/linewriter/linewriter.go:21:2: found packages sync (aliases.go) and seqatomic (generic_atomicptr_unsafe.go) in $GOMODCACHE/gvisor.dev/gvisor@v0.0.0-20210601174640-77dc0f5bc94d/pkg/sync
imports.go:67:2: found packages tcp (accept.go) and rcv (rcv_test.go) in $GOMODCACHE/gvisor.dev/gvisor@v0.0.0-20210601174640-77dc0f5bc94d/pkg/tcpip/transport/tcp
PiperOrigin-RevId: 376956213
- Use atomic add rather than CAS in every Gate method, which is slightly
faster in most cases.
- Implement Close wakeup using gopark/goready to avoid channel allocation.
New benchmarks:
name old time/op new time/op delta
GateEnterLeave-12 16.7ns ± 1% 10.3ns ± 1% -38.44% (p=0.000 n=9+8)
GateClose-12 50.2ns ± 8% 42.4ns ± 6% -15.44% (p=0.000 n=10+10)
GateEnterLeaveAsyncClose-12 972ns ± 2% 640ns ± 7% -34.15% (p=0.000 n=9+10)
PiperOrigin-RevId: 359336344
This function does not exist in Go 1.13. We need to add an adaptor
to build against Go 1.13, which is the default Ubuntu version.
PiperOrigin-RevId: 343929132
We would like to track locks ordering to detect ordering violations. Detecting
violations is much simpler if mutexes must be unlocked by the same goroutine
that locked them.
Thus, as a first step to tracking lock ordering, add this lock/unlock
requirement to gVisor's sync.Mutex. This is more strict than the Go standard
library's sync.Mutex, but initial testing indicates only a single lock that is
used across goroutines. The new sync.CrossGoroutineMutex relaxes the
requirement (but will not provide lock order checking).
Due to the additional overhead, enforcement is only enabled with the
"checklocks" build tag. Build with this tag using:
bazel build --define=gotags=checklocks ...
From my spot-checking, this has no changed inlining properties when disabled.
Updates #4804
PiperOrigin-RevId: 343370200
gVisor bot
Jamie LiuandgVisor bot
Michael PrattandgVisor bot
Adin ScannellandgVisor bot
Ian Gudger
Tamir DubersteinandgVisor bot
Fabricio VoznikaandgVisor bot
Zach KoopmansandgVisor bot
