netbird/gvisor
0
0
Fork 0
mirror of https://github.com/netbirdio/gvisor.git synced 2026-05-22 17:12:49 -07:00
Code Issues Packages Projects Releases Wiki Activity

Don't copy structs with sync.Mutex during initialization

Browse Source 
During inititalization inode struct was copied around, but
it isn't great pratice to copy it around since it contains
ref count and sync.Mutex.

Updates #1480

PiperOrigin-RevId: 315983788
This commit is contained in:
Fabricio Voznika
2020-06-11 14:56:19 -07:00
committed by gVisor bot
parent 11dc95e6c5
commit d58d57606a
8 changed files with 65 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/sentry/fsimpl/ext/block_map_file.go
+5 -4
View File
@@ -58,15 +58,16 @@ var _ io.ReaderAt = (*blockMapFile)(nil)
// newBlockMapFile is the blockMapFile constructor. It initializes the file to
// physical blocks map with (at most) the first 12 (direct) blocks.
func newBlockMapFile(regFile regularFile) (*blockMapFile, error) {
file := &blockMapFile{regFile: regFile}
func newBlockMapFile(args inodeArgs) (*blockMapFile, error) {
file := &blockMapFile{}
file.regFile.impl = file
file.regFile.inode.init(args, &file.regFile)
for i := uint(0); i < 4; i++ {
file.coverage[i] = getCoverage(regFile.inode.blkSize, i)
file.coverage[i] = getCoverage(file.regFile.inode.blkSize, i)
}
blkMap := regFile.inode.diskInode.Data()
blkMap := file.regFile.inode.diskInode.Data()
binary.Unmarshal(blkMap[:numDirectBlks*4], binary.LittleEndian, &file.directBlks)
binary.Unmarshal(blkMap[numDirectBlks*4:(numDirectBlks+1)*4], binary.LittleEndian, &file.indirectBlk)
binary.Unmarshal(blkMap[(numDirectBlks+1)*4:(numDirectBlks+2)*4], binary.LittleEndian, &file.doubleIndirectBlk)
pkg/sentry/fsimpl/ext/block_map_test.go
+13 -16
View File
@@ -85,20 +85,6 @@ func (n *blkNumGen) next() uint32 {
// the inode covers and that is written to disk.
func blockMapSetUp(t *testing.T) (*blockMapFile, []byte) {
mockDisk := make([]byte, mockBMDiskSize)
regFile := regularFile{
inode: inode{
fs: &filesystem{
dev: bytes.NewReader(mockDisk),
},
diskInode: &disklayout.InodeNew{
InodeOld: disklayout.InodeOld{
SizeLo: getMockBMFileFize(),
},
},
blkSize: uint64(mockBMBlkSize),
},
}
var fileData []byte
blkNums := newBlkNumGen()
var data []byte
@@ -125,9 +111,20 @@ func blockMapSetUp(t *testing.T) (*blockMapFile, []byte) {
data = binary.Marshal(data, binary.LittleEndian, triplyIndirectBlk)
fileData = append(fileData, writeFileDataToBlock(mockDisk, triplyIndirectBlk, 3, blkNums)...)
copy(regFile.inode.diskInode.Data(), data)
args := inodeArgs{
fs: &filesystem{
dev: bytes.NewReader(mockDisk),
},
diskInode: &disklayout.InodeNew{
InodeOld: disklayout.InodeOld{
SizeLo: getMockBMFileFize(),
},
},
blkSize: uint64(mockBMBlkSize),
}
copy(args.diskInode.Data(), data)
mockFile, err := newBlockMapFile(regFile)
mockFile, err := newBlockMapFile(args)
if err != nil {
t.Fatalf("newBlockMapFile failed: %v", err)
}
pkg/sentry/fsimpl/ext/directory.go
+5 -6
View File
@@ -54,16 +54,15 @@ type directory struct {
}
// newDirectory is the directory constructor.
func newDirectory(inode inode, newDirent bool) (*directory, error) {
func newDirectory(args inodeArgs, newDirent bool) (*directory, error) {
file := &directory{
inode: inode,
childCache: make(map[string]*dentry),
childMap: make(map[string]*dirent),
}
file.inode.impl = file
file.inode.init(args, file)
// Initialize childList by reading dirents from the underlying file.
if inode.diskInode.Flags().Index {
if args.diskInode.Flags().Index {
// TODO(b/134676337): Support hash tree directories. Currently only the '.'
// and '..' entries are read in.
@@ -74,7 +73,7 @@ func newDirectory(inode inode, newDirent bool) (*directory, error) {
// The dirents are organized in a linear array in the file data.
// Extract the file data and decode the dirents.
regFile, err := newRegularFile(inode)
regFile, err := newRegularFile(args)
if err != nil {
return nil, err
}
@@ -82,7 +81,7 @@ func newDirectory(inode inode, newDirent bool) (*directory, error) {
// buf is used as scratch space for reading in dirents from disk and
// unmarshalling them into dirent structs.
buf := make([]byte, disklayout.DirentSize)
size := inode.diskInode.Size()
size := args.diskInode.Size()
for off, inc := uint64(0), uint64(0); off < size; off += inc {
toRead := size - off
if toRead > disklayout.DirentSize {
pkg/sentry/fsimpl/ext/extent_file.go
+3 -2
View File
@@ -38,9 +38,10 @@ var _ io.ReaderAt = (*extentFile)(nil)
// newExtentFile is the extent file constructor. It reads the entire extent
// tree into memory.
// TODO(b/134676337): Build extent tree on demand to reduce memory usage.
func newExtentFile(regFile regularFile) (*extentFile, error) {
file := &extentFile{regFile: regFile}
func newExtentFile(args inodeArgs) (*extentFile, error) {
file := &extentFile{}
file.regFile.impl = file
file.regFile.inode.init(args, &file.regFile)
err := file.buildExtTree()
if err != nil {
return nil, err
pkg/sentry/fsimpl/ext/extent_test.go
+10 -12
View File
@@ -177,21 +177,19 @@ func extentTreeSetUp(t *testing.T, root *disklayout.ExtentNode) (*extentFile, []
t.Helper()
mockDisk := make([]byte, mockExtentBlkSize*10)
mockExtentFile := &extentFile{
regFile: regularFile{
inode: inode{
fs: &filesystem{
dev: bytes.NewReader(mockDisk),
},
diskInode: &disklayout.InodeNew{
InodeOld: disklayout.InodeOld{
SizeLo: uint32(mockExtentBlkSize) * getNumPhyBlks(root),
},
},
blkSize: mockExtentBlkSize,
mockExtentFile := &extentFile{}
args := inodeArgs{
fs: &filesystem{
dev: bytes.NewReader(mockDisk),
},
diskInode: &disklayout.InodeNew{
InodeOld: disklayout.InodeOld{
SizeLo: uint32(mockExtentBlkSize) * getNumPhyBlks(root),
},
},
blkSize: mockExtentBlkSize,
}
mockExtentFile.regFile.inode.init(args, &mockExtentFile.regFile)
fileData := writeTree(&mockExtentFile.regFile.inode, mockDisk, node0, mockExtentBlkSize)
pkg/sentry/fsimpl/ext/inode.go
+19 -4
View File
@@ -118,7 +118,7 @@ func newInode(fs *filesystem, inodeNum uint32) (*inode, error) {
}
// Build the inode based on its type.
inode := inode{
args := inodeArgs{
fs: fs,
inodeNum: inodeNum,
blkSize: blkSize,
@@ -127,19 +127,19 @@ func newInode(fs *filesystem, inodeNum uint32) (*inode, error) {
switch diskInode.Mode().FileType() {
case linux.ModeSymlink:
f, err := newSymlink(inode)
f, err := newSymlink(args)
if err != nil {
return nil, err
}
return &f.inode, nil
case linux.ModeRegular:
f, err := newRegularFile(inode)
f, err := newRegularFile(args)
if err != nil {
return nil, err
}
return &f.inode, nil
case linux.ModeDirectory:
f, err := newDirectory(inode, fs.sb.IncompatibleFeatures().DirentFileType)
f, err := newDirectory(args, fs.sb.IncompatibleFeatures().DirentFileType)
if err != nil {
return nil, err
}
@@ -150,6 +150,21 @@ func newInode(fs *filesystem, inodeNum uint32) (*inode, error) {
}
}
type inodeArgs struct {
fs *filesystem
inodeNum uint32
blkSize uint64
diskInode disklayout.Inode
}
func (in *inode) init(args inodeArgs, impl interface{}) {
in.fs = args.fs
in.inodeNum = args.inodeNum
in.blkSize = args.blkSize
in.diskInode = args.diskInode
in.impl = impl
}
// open creates and returns a file description for the dentry passed in.
func (in *inode) open(rp *vfs.ResolvingPath, vfsd *vfs.Dentry, opts *vfs.OpenOptions) (*vfs.FileDescription, error) {
ats := vfs.AccessTypesForOpenFlags(opts)
pkg/sentry/fsimpl/ext/regular_file.go
+4 -13
View File
@@ -43,28 +43,19 @@ type regularFile struct {
// newRegularFile is the regularFile constructor. It figures out what kind of
// file this is and initializes the fileReader.
func newRegularFile(inode inode) (*regularFile, error) {
regFile := regularFile{
inode: inode,
}
inodeFlags := inode.diskInode.Flags()
if inodeFlags.Extents {
file, err := newExtentFile(regFile)
func newRegularFile(args inodeArgs) (*regularFile, error) {
if args.diskInode.Flags().Extents {
file, err := newExtentFile(args)
if err != nil {
return nil, err
}
file.regFile.inode.impl = &file.regFile
return &file.regFile, nil
}
file, err := newBlockMapFile(regFile)
file, err := newBlockMapFile(args)
if err != nil {
return nil, err
}
file.regFile.inode.impl = &file.regFile
return &file.regFile, nil
}
pkg/sentry/fsimpl/ext/symlink.go
+6 -7
View File
@@ -30,18 +30,17 @@ type symlink struct {
// newSymlink is the symlink constructor. It reads out the symlink target from
// the inode (however it might have been stored).
func newSymlink(inode inode) (*symlink, error) {
var file *symlink
func newSymlink(args inodeArgs) (*symlink, error) {
var link []byte
// If the symlink target is lesser than 60 bytes, its stores in inode.Data().
// Otherwise either extents or block maps will be used to store the link.
size := inode.diskInode.Size()
size := args.diskInode.Size()
if size < 60 {
link = inode.diskInode.Data()[:size]
link = args.diskInode.Data()[:size]
} else {
// Create a regular file out of this inode and read out the target.
regFile, err := newRegularFile(inode)
regFile, err := newRegularFile(args)
if err != nil {
return nil, err
}
@@ -52,8 +51,8 @@ func newSymlink(inode inode) (*symlink, error) {
}
}
file = &symlink{inode: inode, target: string(link)}
file.inode.impl = file
file := &symlink{target: string(link)}
file.inode.init(args, file)
return file, nil
}