go
/
gvisor
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Make pipe min/max sizes match linux. 

The default pipe size already matched linux, and is unchanged.

Furthermore `atomicIOBytes` is made a proper constant (as it is in Linux). We
were plumbing usermem.PageSize everywhere, so this is no functional change.

PiperOrigin-RevId: 340497006
This commit is contained in:
Nicolas Lacasse 2020-11-03 12:10:01 -08:00 committed by gVisor bot
parent 861c11bfa7
commit 723464ec55
11 changed files with 37 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
nogo.yaml
View File

@ -284,22 +284,6 @@ global:
- pkg/sentry/kernel/kcov.go:223
- pkg/sentry/kernel/kernel.go:343
- pkg/sentry/kernel/kernel.go:368
- pkg/sentry/kernel/pipe/node_test.go:112
- pkg/sentry/kernel/pipe/node_test.go:119
- pkg/sentry/kernel/pipe/node_test.go:130
- pkg/sentry/kernel/pipe/node_test.go:137
- pkg/sentry/kernel/pipe/node_test.go:149
- pkg/sentry/kernel/pipe/node_test.go:150
- pkg/sentry/kernel/pipe/node_test.go:158
- pkg/sentry/kernel/pipe/node_test.go:174
- pkg/sentry/kernel/pipe/node_test.go:180
- pkg/sentry/kernel/pipe/node_test.go:193
- pkg/sentry/kernel/pipe/node_test.go:202
- pkg/sentry/kernel/pipe/node_test.go:205
- pkg/sentry/kernel/pipe/node_test.go:216
- pkg/sentry/kernel/pipe/node_test.go:219
- pkg/sentry/kernel/pipe/node_test.go:271
- pkg/sentry/kernel/pipe/node_test.go:290
- pkg/sentry/kernel/pipe/pipe_test.go:93
- pkg/sentry/kernel/pipe/reader_writer.go:65
- pkg/sentry/kernel/posixtimer.go:157

3
pkg/sentry/fs/gofer/path.go
View File

@ -25,7 +25,6 @@ import (
"gvisor.dev/gvisor/pkg/sentry/kernel/pipe"
"gvisor.dev/gvisor/pkg/sentry/socket/unix/transport"
"gvisor.dev/gvisor/pkg/syserror"
"gvisor.dev/gvisor/pkg/usermem"
)
// maxFilenameLen is the maximum length of a filename. This is dictated by 9P's
@ -305,7 +304,7 @@ func (i *inodeOperations) createInternalFifo(ctx context.Context, dir *fs.Inode,
}
// First create a pipe.
p := pipe.NewPipe(true /* isNamed */, pipe.DefaultPipeSize, usermem.PageSize)
p := pipe.NewPipe(true /* isNamed */, pipe.DefaultPipeSize)
// Wrap the fileOps with our Fifo.
iops := &fifo{

2
pkg/sentry/fs/tmpfs/tmpfs.go
View File

@ -336,7 +336,7 @@ type Fifo struct {
// NewFifo creates a new named pipe.
func NewFifo(ctx context.Context, owner fs.FileOwner, perms fs.FilePermissions, msrc *fs.MountSource) *fs.Inode {
// First create a pipe.
p := pipe.NewPipe(true /* isNamed */, pipe.DefaultPipeSize, usermem.PageSize)
p := pipe.NewPipe(true /* isNamed */, pipe.DefaultPipeSize)
// Build pipe InodeOperations.
iops := pipe.NewInodeOperations(ctx, perms, p)

3
pkg/sentry/fsimpl/gofer/filesystem.go
View File

@ -30,7 +30,6 @@ import (
"gvisor.dev/gvisor/pkg/sentry/socket/unix/transport"
"gvisor.dev/gvisor/pkg/sentry/vfs"
"gvisor.dev/gvisor/pkg/syserror"
"gvisor.dev/gvisor/pkg/usermem"
)
// Sync implements vfs.FilesystemImpl.Sync.
@ -842,7 +841,7 @@ func (fs *filesystem) MknodAt(ctx context.Context, rp *vfs.ResolvingPath, opts v
mode: opts.Mode,
kuid: creds.EffectiveKUID,
kgid: creds.EffectiveKGID,
pipe: pipe.NewVFSPipe(true /* isNamed */, pipe.DefaultPipeSize, usermem.PageSize),
pipe: pipe.NewVFSPipe(true /* isNamed */, pipe.DefaultPipeSize),
})
return nil
}

2
pkg/sentry/fsimpl/pipefs/pipefs.go
View File

@ -101,7 +101,7 @@ type inode struct {
func newInode(ctx context.Context, fs *filesystem) *inode {
creds := auth.CredentialsFromContext(ctx)
return &inode{
pipe: pipe.NewVFSPipe(false /* isNamed */, pipe.DefaultPipeSize, usermem.PageSize),
pipe: pipe.NewVFSPipe(false /* isNamed */, pipe.DefaultPipeSize),
ino: fs.Filesystem.NextIno(),
uid: creds.EffectiveKUID,
gid: creds.EffectiveKGID,

3
pkg/sentry/fsimpl/tmpfs/named_pipe.go
View File

@ -18,7 +18,6 @@ import (
"gvisor.dev/gvisor/pkg/abi/linux"
"gvisor.dev/gvisor/pkg/sentry/kernel/auth"
"gvisor.dev/gvisor/pkg/sentry/kernel/pipe"
"gvisor.dev/gvisor/pkg/usermem"
)
// +stateify savable
@ -32,7 +31,7 @@ type namedPipe struct {
// * fs.mu must be locked.
// * rp.Mount().CheckBeginWrite() has been called successfully.
func (fs *filesystem) newNamedPipe(kuid auth.KUID, kgid auth.KGID, mode linux.FileMode) *inode {
file := &namedPipe{pipe: pipe.NewVFSPipe(true /* isNamed */, pipe.DefaultPipeSize, usermem.PageSize)}
file := &namedPipe{pipe: pipe.NewVFSPipe(true /* isNamed */, pipe.DefaultPipeSize)}
file.inode.init(file, fs, kuid, kgid, linux.S_IFIFO|mode)
file.inode.nlink = 1 // Only the parent has a link.
return &file.inode

8
pkg/sentry/kernel/pipe/node_test.go
View File

@ -22,7 +22,6 @@ import (
"gvisor.dev/gvisor/pkg/sentry/contexttest"
"gvisor.dev/gvisor/pkg/sentry/fs"
"gvisor.dev/gvisor/pkg/syserror"
"gvisor.dev/gvisor/pkg/usermem"
)
type sleeper struct {
@ -66,7 +65,8 @@ func testOpenOrDie(ctx context.Context, t *testing.T, n fs.InodeOperations, flag
d := fs.NewDirent(ctx, inode, "pipe")
file, err := n.GetFile(ctx, d, flags)
if err != nil {
t.Fatalf("open with flags %+v failed: %v", flags, err)
t.Errorf("open with flags %+v failed: %v", flags, err)
return nil, err
}
if doneChan != nil {
doneChan <- struct{}{}
@ -85,11 +85,11 @@ func testOpen(ctx context.Context, t *testing.T, n fs.InodeOperations, flags fs.
}
func newNamedPipe(t *testing.T) *Pipe {
return NewPipe(true, DefaultPipeSize, usermem.PageSize)
return NewPipe(true, DefaultPipeSize)
}
func newAnonPipe(t *testing.T) *Pipe {
return NewPipe(false, DefaultPipeSize, usermem.PageSize)
return NewPipe(false, DefaultPipeSize)
}
// assertRecvBlocks ensures that a recv attempt on c blocks for at least

52
pkg/sentry/kernel/pipe/pipe.go
View File

@ -26,18 +26,27 @@ import (
"gvisor.dev/gvisor/pkg/sentry/fs"
"gvisor.dev/gvisor/pkg/sync"
"gvisor.dev/gvisor/pkg/syserror"
"gvisor.dev/gvisor/pkg/usermem"
"gvisor.dev/gvisor/pkg/waiter"
)
const (
// MinimumPipeSize is a hard limit of the minimum size of a pipe.
MinimumPipeSize = 64 << 10
// DefaultPipeSize is the system-wide default size of a pipe in bytes.
DefaultPipeSize = MinimumPipeSize
// It corresponds to fs/pipe.c:pipe_min_size.
MinimumPipeSize = usermem.PageSize
// MaximumPipeSize is a hard limit on the maximum size of a pipe.
MaximumPipeSize = 8 << 20
// It corresponds to fs/pipe.c:pipe_max_size.
MaximumPipeSize = 1048576
// DefaultPipeSize is the system-wide default size of a pipe in bytes.
// It corresponds to pipe_fs_i.h:PIPE_DEF_BUFFERS.
DefaultPipeSize = 16 * usermem.PageSize
// atomicIOBytes is the maximum number of bytes that the pipe will
// guarantee atomic reads or writes atomically.
// It corresponds to limits.h:PIPE_BUF.
atomicIOBytes = 4096
)
// Pipe is an encapsulation of a platform-independent pipe.
@ -53,12 +62,6 @@ type Pipe struct {
// This value is immutable.
isNamed bool
// atomicIOBytes is the maximum number of bytes that the pipe will
// guarantee atomic reads or writes atomically.
//
// This value is immutable.
atomicIOBytes int64
// The number of active readers for this pipe.
//
// Access atomically.
@ -94,47 +97,34 @@ type Pipe struct {
// NewPipe initializes and returns a pipe.
//
// N.B. The size and atomicIOBytes will be bounded.
func NewPipe(isNamed bool, sizeBytes, atomicIOBytes int64) *Pipe {
// N.B. The size will be bounded.
func NewPipe(isNamed bool, sizeBytes int64) *Pipe {
if sizeBytes < MinimumPipeSize {
sizeBytes = MinimumPipeSize
}
if sizeBytes > MaximumPipeSize {
sizeBytes = MaximumPipeSize
}
if atomicIOBytes <= 0 {
atomicIOBytes = 1
}
if atomicIOBytes > sizeBytes {
atomicIOBytes = sizeBytes
}
var p Pipe
initPipe(&p, isNamed, sizeBytes, atomicIOBytes)
initPipe(&p, isNamed, sizeBytes)
return &p
}
func initPipe(pipe *Pipe, isNamed bool, sizeBytes, atomicIOBytes int64) {
func initPipe(pipe *Pipe, isNamed bool, sizeBytes int64) {
if sizeBytes < MinimumPipeSize {
sizeBytes = MinimumPipeSize
}
if sizeBytes > MaximumPipeSize {
sizeBytes = MaximumPipeSize
}
if atomicIOBytes <= 0 {
atomicIOBytes = 1
}
if atomicIOBytes > sizeBytes {
atomicIOBytes = sizeBytes
}
pipe.isNamed = isNamed
pipe.max = sizeBytes
pipe.atomicIOBytes = atomicIOBytes
}
// NewConnectedPipe initializes a pipe and returns a pair of objects
// representing the read and write ends of the pipe.
func NewConnectedPipe(ctx context.Context, sizeBytes, atomicIOBytes int64) (*fs.File, *fs.File) {
p := NewPipe(false /* isNamed */, sizeBytes, atomicIOBytes)
func NewConnectedPipe(ctx context.Context, sizeBytes int64) (*fs.File, *fs.File) {
p := NewPipe(false /* isNamed */, sizeBytes)
// Build an fs.Dirent for the pipe which will be shared by both
// returned files.
@ -264,7 +254,7 @@ func (p *Pipe) writeLocked(ctx context.Context, ops writeOps) (int64, error) {
wanted := ops.left()
avail := p.max - p.view.Size()
if wanted > avail {
if wanted <= p.atomicIOBytes {
if wanted <= atomicIOBytes {
return 0, syserror.ErrWouldBlock
}
ops.limit(avail)

8
pkg/sentry/kernel/pipe/pipe_test.go
View File

@ -26,7 +26,7 @@ import (
func TestPipeRW(t *testing.T) {
ctx := contexttest.Context(t)
r, w := NewConnectedPipe(ctx, 65536, 4096)
r, w := NewConnectedPipe(ctx, 65536)
defer r.DecRef(ctx)
defer w.DecRef(ctx)
@ -46,7 +46,7 @@ func TestPipeRW(t *testing.T) {
func TestPipeReadBlock(t *testing.T) {
ctx := contexttest.Context(t)
r, w := NewConnectedPipe(ctx, 65536, 4096)
r, w := NewConnectedPipe(ctx, 65536)
defer r.DecRef(ctx)
defer w.DecRef(ctx)
@ -61,7 +61,7 @@ func TestPipeWriteBlock(t *testing.T) {
const capacity = MinimumPipeSize
ctx := contexttest.Context(t)
r, w := NewConnectedPipe(ctx, capacity, atomicIOBytes)
r, w := NewConnectedPipe(ctx, capacity)
defer r.DecRef(ctx)
defer w.DecRef(ctx)
@ -76,7 +76,7 @@ func TestPipeWriteUntilEnd(t *testing.T) {
const atomicIOBytes = 2
ctx := contexttest.Context(t)
r, w := NewConnectedPipe(ctx, atomicIOBytes, atomicIOBytes)
r, w := NewConnectedPipe(ctx, atomicIOBytes)
defer r.DecRef(ctx)
defer w.DecRef(ctx)

4
pkg/sentry/kernel/pipe/vfs.go
View File

@ -54,9 +54,9 @@ type VFSPipe struct {
}
// NewVFSPipe returns an initialized VFSPipe.
func NewVFSPipe(isNamed bool, sizeBytes, atomicIOBytes int64) *VFSPipe {
func NewVFSPipe(isNamed bool, sizeBytes int64) *VFSPipe {
var vp VFSPipe
initPipe(&vp.pipe, isNamed, sizeBytes, atomicIOBytes)
initPipe(&vp.pipe, isNamed, sizeBytes)
return &vp
}

2
pkg/sentry/syscalls/linux/sys_pipe.go
View File

@ -32,7 +32,7 @@ func pipe2(t *kernel.Task, addr usermem.Addr, flags uint) (uintptr, error) {
if flags&^(linux.O_NONBLOCK|linux.O_CLOEXEC) != 0 {
return 0, syserror.EINVAL
}
r, w := pipe.NewConnectedPipe(t, pipe.DefaultPipeSize, usermem.PageSize)
r, w := pipe.NewConnectedPipe(t, pipe.DefaultPipeSize)
r.SetFlags(linuxToFlags(flags).Settable())
defer r.DecRef(t)