From 0e2f1b7abd219f39d67cc2cecd00c441a13eeb29 Mon Sep 17 00:00:00 2001 From: Adin Scannell Date: Mon, 27 Jan 2020 15:17:58 -0800 Subject: Update package locations. Because the abi will depend on the core types for marshalling (usermem, context, safemem, safecopy), these need to be flattened from the sentry directory. These packages contain no sentry-specific details. PiperOrigin-RevId: 291811289 --- pkg/sentry/mm/lifecycle.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'pkg/sentry/mm/lifecycle.go') diff --git a/pkg/sentry/mm/lifecycle.go b/pkg/sentry/mm/lifecycle.go index 4e9ca1de6..47b8fbf43 100644 --- a/pkg/sentry/mm/lifecycle.go +++ b/pkg/sentry/mm/lifecycle.go @@ -19,13 +19,13 @@ import ( "sync/atomic" "gvisor.dev/gvisor/pkg/atomicbitops" + "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/sentry/arch" - "gvisor.dev/gvisor/pkg/sentry/context" "gvisor.dev/gvisor/pkg/sentry/limits" "gvisor.dev/gvisor/pkg/sentry/memmap" "gvisor.dev/gvisor/pkg/sentry/pgalloc" "gvisor.dev/gvisor/pkg/sentry/platform" - "gvisor.dev/gvisor/pkg/sentry/usermem" + "gvisor.dev/gvisor/pkg/usermem" ) // NewMemoryManager returns a new MemoryManager with no mappings and 1 user. -- cgit v1.2.3 From 906eb6295d54a05663a223f1dc379a16148de2d1 Mon Sep 17 00:00:00 2001 From: gVisor bot Date: Tue, 18 Feb 2020 13:42:31 -0800 Subject: atomicbitops package cleanups - Redocument memory ordering from "no ordering" to "acquire-release". (No functional change: both LOCK WHATEVER on x86, and LDAXR/STLXR loops on ARM64, already have this property.) - Remove IncUnlessZeroInt32 and DecUnlessOneInt32, which were only faster than the equivalent loops using sync/atomic before the Go compiler inlined non-unsafe.Pointer atomics many releases ago. PiperOrigin-RevId: 295811743 --- pkg/atomicbitops/BUILD | 9 +- pkg/atomicbitops/atomic_bitops.go | 60 ------- pkg/atomicbitops/atomic_bitops_amd64.s | 115 -------------- pkg/atomicbitops/atomic_bitops_arm64.s | 139 ---------------- pkg/atomicbitops/atomic_bitops_common.go | 147 ----------------- pkg/atomicbitops/atomic_bitops_test.go | 262 ------------------------------- pkg/atomicbitops/atomicbitops.go | 47 ++++++ pkg/atomicbitops/atomicbitops_amd64.s | 77 +++++++++ pkg/atomicbitops/atomicbitops_arm64.s | 105 +++++++++++++ pkg/atomicbitops/atomicbitops_noasm.go | 105 +++++++++++++ pkg/atomicbitops/atomicbitops_test.go | 198 +++++++++++++++++++++++ pkg/sentry/mm/address_space.go | 23 ++- pkg/sentry/mm/lifecycle.go | 11 +- 13 files changed, 563 insertions(+), 735 deletions(-) delete mode 100644 pkg/atomicbitops/atomic_bitops.go delete mode 100644 pkg/atomicbitops/atomic_bitops_amd64.s delete mode 100644 pkg/atomicbitops/atomic_bitops_arm64.s delete mode 100644 pkg/atomicbitops/atomic_bitops_common.go delete mode 100644 pkg/atomicbitops/atomic_bitops_test.go create mode 100644 pkg/atomicbitops/atomicbitops.go create mode 100644 pkg/atomicbitops/atomicbitops_amd64.s create mode 100644 pkg/atomicbitops/atomicbitops_arm64.s create mode 100644 pkg/atomicbitops/atomicbitops_noasm.go create mode 100644 pkg/atomicbitops/atomicbitops_test.go (limited to 'pkg/sentry/mm/lifecycle.go') diff --git a/pkg/atomicbitops/BUILD b/pkg/atomicbitops/BUILD index 3948074ba..ba8b06071 100644 --- a/pkg/atomicbitops/BUILD +++ b/pkg/atomicbitops/BUILD @@ -5,10 +5,9 @@ package(licenses = ["notice"]) go_library( name = "atomicbitops", srcs = [ - "atomic_bitops.go", - "atomic_bitops_amd64.s", - "atomic_bitops_arm64.s", - "atomic_bitops_common.go", + "atomicbitops.go", + "atomicbitops_amd64.s", + "atomicbitops_noasm.go", ], visibility = ["//:sandbox"], ) @@ -16,7 +15,7 @@ go_library( go_test( name = "atomicbitops_test", size = "small", - srcs = ["atomic_bitops_test.go"], + srcs = ["atomicbitops_test.go"], library = ":atomicbitops", deps = ["//pkg/sync"], ) diff --git a/pkg/atomicbitops/atomic_bitops.go b/pkg/atomicbitops/atomic_bitops.go deleted file mode 100644 index fcc41a9ea..000000000 --- a/pkg/atomicbitops/atomic_bitops.go +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright 2018 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. - -// +build amd64 arm64 - -// Package atomicbitops provides basic bitwise operations in an atomic way. -// The implementation on amd64 leverages the LOCK prefix directly instead of -// relying on the generic cas primitives, and the arm64 leverages the LDAXR -// and STLXR pair primitives. -// -// WARNING: the bitwise ops provided in this package doesn't imply any memory -// ordering. Using them to construct locks must employ proper memory barriers. -package atomicbitops - -// AndUint32 atomically applies bitwise and operation to *addr with val. -func AndUint32(addr *uint32, val uint32) - -// OrUint32 atomically applies bitwise or operation to *addr with val. -func OrUint32(addr *uint32, val uint32) - -// XorUint32 atomically applies bitwise xor operation to *addr with val. -func XorUint32(addr *uint32, val uint32) - -// CompareAndSwapUint32 is like sync/atomic.CompareAndSwapUint32, but returns -// the value previously stored at addr. -func CompareAndSwapUint32(addr *uint32, old, new uint32) uint32 - -// AndUint64 atomically applies bitwise and operation to *addr with val. -func AndUint64(addr *uint64, val uint64) - -// OrUint64 atomically applies bitwise or operation to *addr with val. -func OrUint64(addr *uint64, val uint64) - -// XorUint64 atomically applies bitwise xor operation to *addr with val. -func XorUint64(addr *uint64, val uint64) - -// CompareAndSwapUint64 is like sync/atomic.CompareAndSwapUint64, but returns -// the value previously stored at addr. -func CompareAndSwapUint64(addr *uint64, old, new uint64) uint64 - -// IncUnlessZeroInt32 increments the value stored at the given address and -// returns true; unless the value stored in the pointer is zero, in which case -// it is left unmodified and false is returned. -func IncUnlessZeroInt32(addr *int32) bool - -// DecUnlessOneInt32 decrements the value stored at the given address and -// returns true; unless the value stored in the pointer is 1, in which case it -// is left unmodified and false is returned. -func DecUnlessOneInt32(addr *int32) bool diff --git a/pkg/atomicbitops/atomic_bitops_amd64.s b/pkg/atomicbitops/atomic_bitops_amd64.s deleted file mode 100644 index db0972001..000000000 --- a/pkg/atomicbitops/atomic_bitops_amd64.s +++ /dev/null @@ -1,115 +0,0 @@ -// Copyright 2018 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. - -// +build amd64 - -#include "textflag.h" - -TEXT ·AndUint32(SB),$0-12 - MOVQ addr+0(FP), BP - MOVL val+8(FP), AX - LOCK - ANDL AX, 0(BP) - RET - -TEXT ·OrUint32(SB),$0-12 - MOVQ addr+0(FP), BP - MOVL val+8(FP), AX - LOCK - ORL AX, 0(BP) - RET - -TEXT ·XorUint32(SB),$0-12 - MOVQ addr+0(FP), BP - MOVL val+8(FP), AX - LOCK - XORL AX, 0(BP) - RET - -TEXT ·CompareAndSwapUint32(SB),$0-20 - MOVQ addr+0(FP), DI - MOVL old+8(FP), AX - MOVL new+12(FP), DX - LOCK - CMPXCHGL DX, 0(DI) - MOVL AX, ret+16(FP) - RET - -TEXT ·AndUint64(SB),$0-16 - MOVQ addr+0(FP), BP - MOVQ val+8(FP), AX - LOCK - ANDQ AX, 0(BP) - RET - -TEXT ·OrUint64(SB),$0-16 - MOVQ addr+0(FP), BP - MOVQ val+8(FP), AX - LOCK - ORQ AX, 0(BP) - RET - -TEXT ·XorUint64(SB),$0-16 - MOVQ addr+0(FP), BP - MOVQ val+8(FP), AX - LOCK - XORQ AX, 0(BP) - RET - -TEXT ·CompareAndSwapUint64(SB),$0-32 - MOVQ addr+0(FP), DI - MOVQ old+8(FP), AX - MOVQ new+16(FP), DX - LOCK - CMPXCHGQ DX, 0(DI) - MOVQ AX, ret+24(FP) - RET - -TEXT ·IncUnlessZeroInt32(SB),NOSPLIT,$0-9 - MOVQ addr+0(FP), DI - MOVL 0(DI), AX - -retry: - TESTL AX, AX - JZ fail - LEAL 1(AX), DX - LOCK - CMPXCHGL DX, 0(DI) - JNZ retry - - SETEQ ret+8(FP) - RET - -fail: - MOVB AX, ret+8(FP) - RET - -TEXT ·DecUnlessOneInt32(SB),NOSPLIT,$0-9 - MOVQ addr+0(FP), DI - MOVL 0(DI), AX - -retry: - LEAL -1(AX), DX - TESTL DX, DX - JZ fail - LOCK - CMPXCHGL DX, 0(DI) - JNZ retry - - SETEQ ret+8(FP) - RET - -fail: - MOVB DX, ret+8(FP) - RET diff --git a/pkg/atomicbitops/atomic_bitops_arm64.s b/pkg/atomicbitops/atomic_bitops_arm64.s deleted file mode 100644 index 97f8808c1..000000000 --- a/pkg/atomicbitops/atomic_bitops_arm64.s +++ /dev/null @@ -1,139 +0,0 @@ -// Copyright 2019 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. - -// +build arm64 - -#include "textflag.h" - -TEXT ·AndUint32(SB),$0-12 - MOVD ptr+0(FP), R0 - MOVW val+8(FP), R1 -again: - LDAXRW (R0), R2 - ANDW R1, R2 - STLXRW R2, (R0), R3 - CBNZ R3, again - RET - -TEXT ·OrUint32(SB),$0-12 - MOVD ptr+0(FP), R0 - MOVW val+8(FP), R1 -again: - LDAXRW (R0), R2 - ORRW R1, R2 - STLXRW R2, (R0), R3 - CBNZ R3, again - RET - -TEXT ·XorUint32(SB),$0-12 - MOVD ptr+0(FP), R0 - MOVW val+8(FP), R1 -again: - LDAXRW (R0), R2 - EORW R1, R2 - STLXRW R2, (R0), R3 - CBNZ R3, again - RET - -TEXT ·CompareAndSwapUint32(SB),$0-20 - MOVD addr+0(FP), R0 - MOVW old+8(FP), R1 - MOVW new+12(FP), R2 - -again: - LDAXRW (R0), R3 - CMPW R1, R3 - BNE done - STLXRW R2, (R0), R4 - CBNZ R4, again -done: - MOVW R3, prev+16(FP) - RET - -TEXT ·AndUint64(SB),$0-16 - MOVD ptr+0(FP), R0 - MOVD val+8(FP), R1 -again: - LDAXR (R0), R2 - AND R1, R2 - STLXR R2, (R0), R3 - CBNZ R3, again - RET - -TEXT ·OrUint64(SB),$0-16 - MOVD ptr+0(FP), R0 - MOVD val+8(FP), R1 -again: - LDAXR (R0), R2 - ORR R1, R2 - STLXR R2, (R0), R3 - CBNZ R3, again - RET - -TEXT ·XorUint64(SB),$0-16 - MOVD ptr+0(FP), R0 - MOVD val+8(FP), R1 -again: - LDAXR (R0), R2 - EOR R1, R2 - STLXR R2, (R0), R3 - CBNZ R3, again - RET - -TEXT ·CompareAndSwapUint64(SB),$0-32 - MOVD addr+0(FP), R0 - MOVD old+8(FP), R1 - MOVD new+16(FP), R2 - -again: - LDAXR (R0), R3 - CMP R1, R3 - BNE done - STLXR R2, (R0), R4 - CBNZ R4, again -done: - MOVD R3, prev+24(FP) - RET - -TEXT ·IncUnlessZeroInt32(SB),NOSPLIT,$0-9 - MOVD addr+0(FP), R0 - -again: - LDAXRW (R0), R1 - CBZ R1, fail - ADDW $1, R1 - STLXRW R1, (R0), R2 - CBNZ R2, again - MOVW $1, R2 - MOVB R2, ret+8(FP) - RET -fail: - MOVB ZR, ret+8(FP) - RET - -TEXT ·DecUnlessOneInt32(SB),NOSPLIT,$0-9 - MOVD addr+0(FP), R0 - -again: - LDAXRW (R0), R1 - SUBSW $1, R1, R1 - BEQ fail - STLXRW R1, (R0), R2 - CBNZ R2, again - MOVW $1, R2 - MOVB R2, ret+8(FP) - RET -fail: - MOVB ZR, ret+8(FP) - RET diff --git a/pkg/atomicbitops/atomic_bitops_common.go b/pkg/atomicbitops/atomic_bitops_common.go deleted file mode 100644 index 85163ad62..000000000 --- a/pkg/atomicbitops/atomic_bitops_common.go +++ /dev/null @@ -1,147 +0,0 @@ -// Copyright 2018 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. - -// +build !amd64,!arm64 - -package atomicbitops - -import ( - "sync/atomic" -) - -// AndUint32 atomically applies bitwise and operation to *addr with val. -func AndUint32(addr *uint32, val uint32) { - for { - o := atomic.LoadUint32(addr) - n := o & val - if atomic.CompareAndSwapUint32(addr, o, n) { - break - } - } -} - -// OrUint32 atomically applies bitwise or operation to *addr with val. -func OrUint32(addr *uint32, val uint32) { - for { - o := atomic.LoadUint32(addr) - n := o | val - if atomic.CompareAndSwapUint32(addr, o, n) { - break - } - } -} - -// XorUint32 atomically applies bitwise xor operation to *addr with val. -func XorUint32(addr *uint32, val uint32) { - for { - o := atomic.LoadUint32(addr) - n := o ^ val - if atomic.CompareAndSwapUint32(addr, o, n) { - break - } - } -} - -// CompareAndSwapUint32 is like sync/atomic.CompareAndSwapUint32, but returns -// the value previously stored at addr. -func CompareAndSwapUint32(addr *uint32, old, new uint32) (prev uint32) { - for { - prev = atomic.LoadUint32(addr) - if prev != old { - return - } - if atomic.CompareAndSwapUint32(addr, old, new) { - return - } - } -} - -// AndUint64 atomically applies bitwise and operation to *addr with val. -func AndUint64(addr *uint64, val uint64) { - for { - o := atomic.LoadUint64(addr) - n := o & val - if atomic.CompareAndSwapUint64(addr, o, n) { - break - } - } -} - -// OrUint64 atomically applies bitwise or operation to *addr with val. -func OrUint64(addr *uint64, val uint64) { - for { - o := atomic.LoadUint64(addr) - n := o | val - if atomic.CompareAndSwapUint64(addr, o, n) { - break - } - } -} - -// XorUint64 atomically applies bitwise xor operation to *addr with val. -func XorUint64(addr *uint64, val uint64) { - for { - o := atomic.LoadUint64(addr) - n := o ^ val - if atomic.CompareAndSwapUint64(addr, o, n) { - break - } - } -} - -// CompareAndSwapUint64 is like sync/atomic.CompareAndSwapUint64, but returns -// the value previously stored at addr. -func CompareAndSwapUint64(addr *uint64, old, new uint64) (prev uint64) { - for { - prev = atomic.LoadUint64(addr) - if prev != old { - return - } - if atomic.CompareAndSwapUint64(addr, old, new) { - return - } - } -} - -// IncUnlessZeroInt32 increments the value stored at the given address and -// returns true; unless the value stored in the pointer is zero, in which case -// it is left unmodified and false is returned. -func IncUnlessZeroInt32(addr *int32) bool { - for { - v := atomic.LoadInt32(addr) - if v == 0 { - return false - } - - if atomic.CompareAndSwapInt32(addr, v, v+1) { - return true - } - } -} - -// DecUnlessOneInt32 decrements the value stored at the given address and -// returns true; unless the value stored in the pointer is 1, in which case it -// is left unmodified and false is returned. -func DecUnlessOneInt32(addr *int32) bool { - for { - v := atomic.LoadInt32(addr) - if v == 1 { - return false - } - - if atomic.CompareAndSwapInt32(addr, v, v-1) { - return true - } - } -} diff --git a/pkg/atomicbitops/atomic_bitops_test.go b/pkg/atomicbitops/atomic_bitops_test.go deleted file mode 100644 index 9466d3e23..000000000 --- a/pkg/atomicbitops/atomic_bitops_test.go +++ /dev/null @@ -1,262 +0,0 @@ -// Copyright 2018 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 atomicbitops - -import ( - "runtime" - "testing" - - "gvisor.dev/gvisor/pkg/sync" -) - -const iterations = 100 - -func detectRaces32(val, target uint32, fn func(*uint32, uint32)) bool { - runtime.GOMAXPROCS(100) - for n := 0; n < iterations; n++ { - x := val - var wg sync.WaitGroup - for i := uint32(0); i < 32; i++ { - wg.Add(1) - go func(a *uint32, i uint32) { - defer wg.Done() - fn(a, uint32(1< Date: Tue, 25 Feb 2020 13:42:34 -0800 Subject: Add option to skip stuck tasks waiting for address space PiperOrigin-RevId: 297192390 --- pkg/sentry/kernel/kernel.go | 4 ++++ pkg/sentry/kernel/task_context.go | 2 +- pkg/sentry/kernel/task_exec.go | 2 +- pkg/sentry/kernel/task_usermem.go | 2 +- pkg/sentry/mm/address_space.go | 23 ++++++++++++++--------- pkg/sentry/mm/lifecycle.go | 26 ++++++++++++++------------ pkg/sentry/mm/mm.go | 5 +++++ pkg/sentry/mm/mm_test.go | 2 +- 8 files changed, 41 insertions(+), 25 deletions(-) (limited to 'pkg/sentry/mm/lifecycle.go') diff --git a/pkg/sentry/kernel/kernel.go b/pkg/sentry/kernel/kernel.go index c62fd6eb1..8b76750e9 100644 --- a/pkg/sentry/kernel/kernel.go +++ b/pkg/sentry/kernel/kernel.go @@ -247,6 +247,10 @@ type Kernel struct { // VFS keeps the filesystem state used across the kernel. vfs vfs.VirtualFilesystem + + // If set to true, report address space activation waits as if the task is in + // external wait so that the watchdog doesn't report the task stuck. + SleepForAddressSpaceActivation bool } // InitKernelArgs holds arguments to Init. diff --git a/pkg/sentry/kernel/task_context.go b/pkg/sentry/kernel/task_context.go index 2be982684..0158b1788 100644 --- a/pkg/sentry/kernel/task_context.go +++ b/pkg/sentry/kernel/task_context.go @@ -140,7 +140,7 @@ func (k *Kernel) LoadTaskImage(ctx context.Context, args loader.LoadArgs) (*Task } // Prepare a new user address space to load into. - m := mm.NewMemoryManager(k, k) + m := mm.NewMemoryManager(k, k, k.SleepForAddressSpaceActivation) defer m.DecUsers(ctx) args.MemoryManager = m diff --git a/pkg/sentry/kernel/task_exec.go b/pkg/sentry/kernel/task_exec.go index 8f57a34a6..00c425cca 100644 --- a/pkg/sentry/kernel/task_exec.go +++ b/pkg/sentry/kernel/task_exec.go @@ -220,7 +220,7 @@ func (r *runSyscallAfterExecStop) execute(t *Task) taskRunState { t.mu.Unlock() t.unstopVforkParent() // NOTE(b/30316266): All locks must be dropped prior to calling Activate. - t.MemoryManager().Activate() + t.MemoryManager().Activate(t) t.ptraceExec(oldTID) return (*runSyscallExit)(nil) diff --git a/pkg/sentry/kernel/task_usermem.go b/pkg/sentry/kernel/task_usermem.go index 2bf3ce8a8..b02044ad2 100644 --- a/pkg/sentry/kernel/task_usermem.go +++ b/pkg/sentry/kernel/task_usermem.go @@ -30,7 +30,7 @@ var MAX_RW_COUNT = int(usermem.Addr(math.MaxInt32).RoundDown()) // Activate ensures that the task has an active address space. func (t *Task) Activate() { if mm := t.MemoryManager(); mm != nil { - if err := mm.Activate(); err != nil { + if err := mm.Activate(t); err != nil { panic("unable to activate mm: " + err.Error()) } } diff --git a/pkg/sentry/mm/address_space.go b/pkg/sentry/mm/address_space.go index 94d39af60..0332fc71c 100644 --- a/pkg/sentry/mm/address_space.go +++ b/pkg/sentry/mm/address_space.go @@ -18,6 +18,7 @@ import ( "fmt" "sync/atomic" + "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/sentry/platform" "gvisor.dev/gvisor/pkg/usermem" ) @@ -38,7 +39,7 @@ func (mm *MemoryManager) AddressSpace() platform.AddressSpace { // // When this MemoryManager is no longer needed by a task, it should call // Deactivate to release the reference. -func (mm *MemoryManager) Activate() error { +func (mm *MemoryManager) Activate(ctx context.Context) error { // Fast path: the MemoryManager already has an active // platform.AddressSpace, and we just need to indicate that we need it too. for { @@ -91,16 +92,20 @@ func (mm *MemoryManager) Activate() error { if as == nil { // AddressSpace is unavailable, we must wait. // - // activeMu must not be held while waiting, as the user - // of the address space we are waiting on may attempt - // to take activeMu. - // - // Don't call UninterruptibleSleepStart to register the - // wait to allow the watchdog stuck task to trigger in - // case a process is starved waiting for the address - // space. + // activeMu must not be held while waiting, as the user of the address + // space we are waiting on may attempt to take activeMu. mm.activeMu.Unlock() + + sleep := mm.p.CooperativelySchedulesAddressSpace() && mm.sleepForActivation + if sleep { + // Mark this task sleeping while waiting for the address space to + // prevent the watchdog from reporting it as a stuck task. + ctx.UninterruptibleSleepStart(false) + } <-c + if sleep { + ctx.UninterruptibleSleepFinish(false) + } continue } diff --git a/pkg/sentry/mm/lifecycle.go b/pkg/sentry/mm/lifecycle.go index 3c263ebaa..d8a5b9d29 100644 --- a/pkg/sentry/mm/lifecycle.go +++ b/pkg/sentry/mm/lifecycle.go @@ -28,16 +28,17 @@ import ( ) // NewMemoryManager returns a new MemoryManager with no mappings and 1 user. -func NewMemoryManager(p platform.Platform, mfp pgalloc.MemoryFileProvider) *MemoryManager { +func NewMemoryManager(p platform.Platform, mfp pgalloc.MemoryFileProvider, sleepForActivation bool) *MemoryManager { return &MemoryManager{ - p: p, - mfp: mfp, - haveASIO: p.SupportsAddressSpaceIO(), - privateRefs: &privateRefs{}, - users: 1, - auxv: arch.Auxv{}, - dumpability: UserDumpable, - aioManager: aioManager{contexts: make(map[uint64]*AIOContext)}, + p: p, + mfp: mfp, + haveASIO: p.SupportsAddressSpaceIO(), + privateRefs: &privateRefs{}, + users: 1, + auxv: arch.Auxv{}, + dumpability: UserDumpable, + aioManager: aioManager{contexts: make(map[uint64]*AIOContext)}, + sleepForActivation: sleepForActivation, } } @@ -79,9 +80,10 @@ func (mm *MemoryManager) Fork(ctx context.Context) (*MemoryManager, error) { envv: mm.envv, auxv: append(arch.Auxv(nil), mm.auxv...), // IncRef'd below, once we know that there isn't an error. - executable: mm.executable, - dumpability: mm.dumpability, - aioManager: aioManager{contexts: make(map[uint64]*AIOContext)}, + executable: mm.executable, + dumpability: mm.dumpability, + aioManager: aioManager{contexts: make(map[uint64]*AIOContext)}, + sleepForActivation: mm.sleepForActivation, } // Copy vmas. diff --git a/pkg/sentry/mm/mm.go b/pkg/sentry/mm/mm.go index 637383c7a..c2195ae11 100644 --- a/pkg/sentry/mm/mm.go +++ b/pkg/sentry/mm/mm.go @@ -226,6 +226,11 @@ type MemoryManager struct { // aioManager keeps track of AIOContexts used for async IOs. AIOManager // must be cloned when CLONE_VM is used. aioManager aioManager + + // sleepForActivation indicates whether the task should report to be sleeping + // before trying to activate the address space. When set to true, delays in + // activation are not reported as stuck tasks by the watchdog. + sleepForActivation bool } // vma represents a virtual memory area. diff --git a/pkg/sentry/mm/mm_test.go b/pkg/sentry/mm/mm_test.go index edacca741..fdc308542 100644 --- a/pkg/sentry/mm/mm_test.go +++ b/pkg/sentry/mm/mm_test.go @@ -31,7 +31,7 @@ import ( func testMemoryManager(ctx context.Context) *MemoryManager { p := platform.FromContext(ctx) mfp := pgalloc.MemoryFileProviderFromContext(ctx) - mm := NewMemoryManager(p, mfp) + mm := NewMemoryManager(p, mfp, false) mm.layout = arch.MmapLayout{ MinAddr: p.MinUserAddress(), MaxAddr: p.MaxUserAddress(), -- cgit v1.2.3 From be415eedcb3d5f61ed9f6a90bba8df2ecd54d8c8 Mon Sep 17 00:00:00 2001 From: Bin Lu Date: Wed, 26 Feb 2020 04:17:54 -0500 Subject: add arch-specific feature into mm Signed-off-by: Bin Lu --- pkg/sentry/mm/lifecycle.go | 1 + pkg/sentry/mm/metadata.go | 14 ++++++++++++++ pkg/sentry/mm/mm.go | 3 +++ 3 files changed, 18 insertions(+) (limited to 'pkg/sentry/mm/lifecycle.go') diff --git a/pkg/sentry/mm/lifecycle.go b/pkg/sentry/mm/lifecycle.go index d8a5b9d29..e6269107c 100644 --- a/pkg/sentry/mm/lifecycle.go +++ b/pkg/sentry/mm/lifecycle.go @@ -84,6 +84,7 @@ func (mm *MemoryManager) Fork(ctx context.Context) (*MemoryManager, error) { dumpability: mm.dumpability, aioManager: aioManager{contexts: make(map[uint64]*AIOContext)}, sleepForActivation: mm.sleepForActivation, + vdsoSigReturnAddr : mm.vdsoSigReturnAddr, } // Copy vmas. diff --git a/pkg/sentry/mm/metadata.go b/pkg/sentry/mm/metadata.go index 6a49334f4..28e5057f7 100644 --- a/pkg/sentry/mm/metadata.go +++ b/pkg/sentry/mm/metadata.go @@ -167,3 +167,17 @@ func (mm *MemoryManager) SetExecutable(file fsbridge.File) { orig.DecRef() } } + +// VDSOSigReturn returns the address of vdso_sigreturn. +func (mm *MemoryManager) VDSOSigReturn() uint64 { + mm.metadataMu.Lock() + defer mm.metadataMu.Unlock() + return mm.vdsoSigReturnAddr +} + +// SetVDSOSigReturn sets the address of vdso_sigreturn. +func (mm *MemoryManager) SetVDSOSigReturn(addr uint64) { + mm.metadataMu.Lock() + defer mm.metadataMu.Unlock() + mm.vdsoSigReturnAddr = addr +} diff --git a/pkg/sentry/mm/mm.go b/pkg/sentry/mm/mm.go index c2195ae11..34d3bde7a 100644 --- a/pkg/sentry/mm/mm.go +++ b/pkg/sentry/mm/mm.go @@ -231,6 +231,9 @@ type MemoryManager struct { // before trying to activate the address space. When set to true, delays in // activation are not reported as stuck tasks by the watchdog. sleepForActivation bool + + // vdsoSigReturnAddr is the address of 'vdso_sigreturn'. + vdsoSigReturnAddr uint64 } // vma represents a virtual memory area. -- cgit v1.2.3