diff --git a/pkg/sync/BUILD b/pkg/sync/BUILD index ec6e4c56b..e38df4cd1 100644 --- a/pkg/sync/BUILD +++ b/pkg/sync/BUILD @@ -12,6 +12,9 @@ go_library( "aliases.go", "checklocks_off_unsafe.go", "checklocks_on_unsafe.go", + "fence.go", + "fence_amd64.s", + "fence_arm64.s", "gate_unsafe.go", "goyield_go113_unsafe.go", "goyield_unsafe.go", diff --git a/pkg/sync/fence.go b/pkg/sync/fence.go new file mode 100644 index 000000000..6706676a5 --- /dev/null +++ b/pkg/sync/fence.go @@ -0,0 +1,19 @@ +// Copyright 2023 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package sync + +// MemoryFenceReads ensures that all preceding memory loads happen before +// following memory loads. +func MemoryFenceReads() diff --git a/pkg/sync/fence_amd64.s b/pkg/sync/fence_amd64.s new file mode 100644 index 000000000..87766f1d3 --- /dev/null +++ b/pkg/sync/fence_amd64.s @@ -0,0 +1,26 @@ +// Copyright 2023 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build amd64 +// +build amd64 + +#include "textflag.h" + +// func MemoryFenceReads() +TEXT ·MemoryFenceReads(SB),NOSPLIT|NOFRAME,$0-0 + // No memory fence is required on x86. However, a compiler fence is + // required to prevent the compiler from reordering memory accesses. The Go + // compiler will not reorder memory accesses around a call to an assembly + // function; compare runtime.publicationBarrier. + RET diff --git a/pkg/sync/fence_arm64.s b/pkg/sync/fence_arm64.s new file mode 100644 index 000000000..f4f9ce9de --- /dev/null +++ b/pkg/sync/fence_arm64.s @@ -0,0 +1,23 @@ +// Copyright 2023 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build arm64 +// +build arm64 + +#include "textflag.h" + +// func MemoryFenceReads() +TEXT ·MemoryFenceReads(SB),NOSPLIT|NOFRAME,$0-0 + DMB $0x9 // ISHLD + RET diff --git a/pkg/sync/seqcount.go b/pkg/sync/seqcount.go index 9adc95322..c90d2d9fa 100644 --- a/pkg/sync/seqcount.go +++ b/pkg/sync/seqcount.go @@ -39,23 +39,6 @@ type SeqCount struct { // SeqCountEpoch tracks writer critical sections in a SeqCount. type SeqCountEpoch uint32 -// We assume that: -// -// - All functions in sync/atomic that perform a memory read are at least a -// read fence: memory reads before calls to such functions cannot be reordered -// after the call, and memory reads after calls to such functions cannot be -// reordered before the call, even if those reads do not use sync/atomic. -// -// - All functions in sync/atomic that perform a memory write are at least a -// write fence: memory writes before calls to such functions cannot be -// reordered after the call, and memory writes after calls to such functions -// cannot be reordered before the call, even if those writes do not use -// sync/atomic. -// -// As of this writing, the Go memory model completely fails to describe -// sync/atomic, but these properties are implied by -// https://groups.google.com/forum/#!topic/golang-nuts/7EnEhM3U7B8. - // BeginRead indicates the beginning of a reader critical section. Reader // critical sections DO NOT BLOCK writer critical sections, so operations in a // reader critical section MAY RACE with writer critical sections. Races are @@ -104,6 +87,7 @@ func (s *SeqCount) beginReadSlow() SeqCountEpoch { // Reader critical sections do not need to be explicitly terminated; the last // call to ReadOk is implicitly the end of the reader critical section. func (s *SeqCount) ReadOk(epoch SeqCountEpoch) bool { + MemoryFenceReads() return atomic.LoadUint32(&s.epoch) == uint32(epoch) }