From 0da79ed4bf4aa746ab3036505c8aa9c9f3c2bd14 Mon Sep 17 00:00:00 2001 From: Tiwei Bie Date: Sat, 11 Nov 2023 09:28:19 +0800 Subject: [PATCH] erofs: support block based dirent lookup This patch adds the block based dirent lookup support to pkg/erofs, which will do the dirent lookup by doing binary search on disk data directly. This is helpful for searching files in large directories and also reduces the memory overhead. Signed-off-by: Tiwei Bie --- pkg/erofs/BUILD | 1 + pkg/erofs/erofs.go | 271 ++++++++++++++++++--------- pkg/erofs/erofs_unsafe.go | 9 + pkg/sentry/fsimpl/erofs/directory.go | 18 +- runsc/container/container_test.go | 118 ++++++++++++ 5 files changed, 326 insertions(+), 91 deletions(-) diff --git a/pkg/erofs/BUILD b/pkg/erofs/BUILD index b517ee3ed..ce8779eb6 100644 --- a/pkg/erofs/BUILD +++ b/pkg/erofs/BUILD @@ -17,6 +17,7 @@ go_library( "//pkg/abi/linux", "//pkg/cleanup", "//pkg/errors/linuxerr", + "//pkg/gohacks", "//pkg/hostarch", "//pkg/log", "//pkg/marshal", diff --git a/pkg/erofs/erofs.go b/pkg/erofs/erofs.go index 47cdb7b36..08e9d6d75 100644 --- a/pkg/erofs/erofs.go +++ b/pkg/erofs/erofs.go @@ -32,6 +32,7 @@ import ( "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/cleanup" "gvisor.dev/gvisor/pkg/errors/linuxerr" + "gvisor.dev/gvisor/pkg/gohacks" "gvisor.dev/gvisor/pkg/hostarch" "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/marshal" @@ -467,13 +468,15 @@ func (i *Image) Inode(nid uint64) (Inode, error) { return Inode{}, linuxerr.ENOTSUP } + blockSize := uint64(i.BlockSize()) + inode.blocks = (inode.size + (blockSize - 1)) / blockSize + switch dataLayout := inode.DataLayout(); dataLayout { case InodeDataLayoutFlatInline: // Check that whether the file data in the last block fits into // the remaining room of the metadata block. - blockSize := i.BlockSize() - tailSize := uint32(inode.size) & (blockSize - 1) - if tailSize == 0 || tailSize > blockSize-uint32(inodeSize) { + tailSize := inode.size & (blockSize - 1) + if tailSize == 0 || tailSize > blockSize-uint64(inodeSize) { log.Warningf("Inline data not found or cross block boundary at inode (nid=%v)", nid) return Inode{}, linuxerr.EUCLEAN } @@ -506,6 +509,11 @@ type Inode struct { // if it's not zero in the metadata block. idataOff uint64 + // blocks indicates the count of blocks that store the data associated + // with this inode. It will count in the metadata block that includes + // the inline data as well. + blocks uint64 + // format is the format of this inode. format uint16 @@ -656,17 +664,41 @@ func (i *Inode) Data() (safemem.BlockSeq, error) { } } -// blocks returns the number of blocks that contain data. It will count in the -// metadata block which contains the inline data. -func (i *Inode) blocks() uint64 { - blockSize := uint64(i.image.BlockSize()) - return (i.size + (blockSize - 1)) / blockSize +// blockData represents the information of the data in a block. +type blockData struct { + // base indicates the data offset within the image. + base uint64 + // size indicates the data size. + size uint32 } -// IterDirents invokes cb on each entry in the directory represented by this inode. -// The directory entries will be iterated in alphabetical order. +// valid indicates whether this is valid information about the data in a block. +func (b *blockData) valid() bool { + // The data offset within the image will never be zero. + return b.base > 0 +} + +// getBlockDataInfo returns the information of the data in the block identified by +// blockIdx of this inode. // -// https://docs.kernel.org/filesystems/erofs.html#directories +// Precondition: blockIdx < i.blocks. +func (i *Inode) getBlockDataInfo(blockIdx uint64) blockData { + blockSize := i.image.BlockSize() + lastBlock := blockIdx == i.blocks-1 + base := i.idataOff + if !lastBlock || base == 0 { + base = i.dataOff + blockIdx*uint64(blockSize) + } + size := blockSize + if lastBlock { + if tailSize := uint32(i.size) & (blockSize - 1); tailSize != 0 { + size = tailSize + } + } + return blockData{base, size} +} + +// getDirentName returns the name of dirent d in the given block of this inode. // // The on-disk format of one block looks like this: // @@ -692,94 +724,163 @@ func (i *Inode) blocks() uint64 { // // [ (metadata block) inode | optional fields | dirent M+2 | dirent M+3 | name M+2 | name M+3 | optional padding ] // -// All directory entries are _strictly_ recorded in alphabetical order. +// Refer: https://docs.kernel.org/filesystems/erofs.html#directories +func (i *Inode) getDirentName(d *Dirent, block blockData, lastDirent bool) ([]byte, error) { + var nameLen uint32 + if lastDirent { + nameLen = block.size - uint32(d.NameOff) + } else { + nameLen = uint32(direntAfter(d).NameOff - d.NameOff) + } + if uint32(d.NameOff)+nameLen > block.size || nameLen > MaxNameLen || nameLen == 0 { + log.Warningf("Corrupted dirent at inode (nid=%v)", i.Nid()) + return nil, linuxerr.EUCLEAN + } + name, err := i.image.BytesAt(block.base+uint64(d.NameOff), uint64(nameLen)) + if err != nil { + return nil, err + } + if lastDirent { + // Optional padding may exist at the end of a block. + n := bytes.IndexByte(name, 0) + if n == 0 { + log.Warningf("Corrupted dirent at inode (nid=%v)", i.Nid()) + return nil, linuxerr.EUCLEAN + } + if n != -1 { + name = name[:n] + } + } + return name, nil +} + +// getDirent0 returns a pointer to the first dirent in the given block of this inode. +func (i *Inode) getDirent0(block blockData) (*Dirent, error) { + d0, err := i.image.direntAt(block.base) + if err != nil { + return nil, err + } + if d0.NameOff < DirentSize || uint32(d0.NameOff) >= block.size { + log.Warningf("Invalid nameOff0 %v at inode (nid=%v)", d0.NameOff, i.Nid()) + return nil, linuxerr.EUCLEAN + } + return d0, nil +} + +// Lookup looks up a child by the name. The child inode number will be returned on success. +func (i *Inode) Lookup(name string) (uint64, error) { + if !i.IsDir() { + return 0, linuxerr.ENOTDIR + } + + // Currently (Go 1.21), there is no safe and efficient way to do three-way + // string comparisons, so let's convert the string to a byte slice first. + nameBytes := gohacks.ImmutableBytesFromString(name) + + // In EROFS, all directory entries are _strictly_ recorded in alphabetical + // order. The lookup is done by directly performing binary search on the + // disk data similar to what Linux does in fs/erofs/namei.c:erofs_namei(). + var ( + targetBlock blockData + targetNumDirents uint16 + ) + + // Find the block that may contain the target dirent first. + bLeft, bRight := int64(0), int64(i.blocks)-1 + for bLeft <= bRight { + // Cast to uint64 to avoid overflow. + mid := uint64(bLeft+bRight) >> 1 + block := i.getBlockDataInfo(mid) + d0, err := i.getDirent0(block) + if err != nil { + return 0, err + } + numDirents := d0.NameOff / DirentSize + d0Name, err := i.getDirentName(d0, block, numDirents == 1) + if err != nil { + return 0, err + } + switch bytes.Compare(nameBytes, d0Name) { + case 0: + // Found the target dirent. + return d0.Nid(), nil + case 1: + // name > d0Name, this block may contain the target dirent. + targetBlock = block + targetNumDirents = numDirents + bLeft = int64(mid) + 1 + case -1: + // name < d0Name, this is not the block we're looking for. + bRight = int64(mid) - 1 + } + } + + if !targetBlock.valid() { + // The target block was not found. + return 0, linuxerr.ENOENT + } + + // Find the target dirent in the target block. Note that, as the 0th dirent + // has already been checked during the block binary search, we don't need to + // check it again and can define dLeft/dRight as unsigned types. + dLeft, dRight := uint16(1), targetNumDirents-1 + for dLeft <= dRight { + // The sum will never lead to a uint16 overflow, as the maximum value of + // the operands is MaxUint16/DirentSize. + mid := (dLeft + dRight) >> 1 + direntOff := targetBlock.base + uint64(mid)*DirentSize + d, err := i.image.direntAt(direntOff) + if err != nil { + return 0, err + } + dName, err := i.getDirentName(d, targetBlock, mid == targetNumDirents-1) + if err != nil { + return 0, err + } + switch bytes.Compare(nameBytes, dName) { + case 0: + // Found the target dirent. + return d.Nid(), nil + case 1: + // name > dName. + dLeft = mid + 1 + case -1: + // name < dName. + dRight = mid - 1 + } + } + + return 0, linuxerr.ENOENT +} + +// IterDirents invokes cb on each entry in the directory represented by this inode. +// The directory entries will be iterated in alphabetical order. func (i *Inode) IterDirents(cb func(name string, typ uint8, nid uint64) error) error { if !i.IsDir() { return linuxerr.ENOTDIR } - blocks := i.blocks() - blockSize := i.image.BlockSize() - - start := i.dataOff - if blocks == 1 && i.idataOff != 0 { - start = i.idataOff - } - // Iterate all the blocks which contain dirents. - for blocks > 0 { - // Get the max data size of this block. - maxSize := blockSize - if blocks == 1 { - if tailSize := uint32(i.size) & (blockSize - 1); tailSize != 0 { - maxSize = tailSize - } - } - // Get the first dirent in the current block. - direntOff := start - d, err := i.image.direntAt(direntOff) + for blockIdx := uint64(0); blockIdx < i.blocks; blockIdx++ { + block := i.getBlockDataInfo(blockIdx) + d, err := i.getDirent0(block) if err != nil { return err } - // Apart from the offset of the first filename, nameOff0 also indicates - // the total number of dirents in this block. - if d.NameOff < DirentSize || uint32(d.NameOff) >= maxSize { - log.Warningf("Invalid nameOff0 %v at inode (nid=%v)", d.NameOff, i.Nid()) - return linuxerr.EUCLEAN - } - nameOff0 := start + uint64(d.NameOff) // Iterate all the dirents in this block. - for d != nil { - var ( - next *Dirent - nameLen uint32 - ) - direntOff += uint64(d.SizeBytes()) - lastDirent := direntOff >= nameOff0 - if lastDirent { - // There is no more dirent in this block, d is the last one. - next = nil - nameLen = maxSize - uint32(d.NameOff) - } else { - // Get the next adjacent dirent. - if next, err = i.image.direntAt(direntOff); err != nil { - return err - } - nameLen = uint32(next.NameOff - d.NameOff) - } - if uint32(d.NameOff)+nameLen > maxSize || nameLen > MaxNameLen || nameLen == 0 { - log.Warningf("Corrupted dirent at inode (nid=%v)", i.Nid()) - return linuxerr.EUCLEAN - } - buf, err := i.image.BytesAt(start+uint64(d.NameOff), uint64(nameLen)) + numDirents := d.NameOff / DirentSize + for { + name, err := i.getDirentName(d, block, numDirents == 1) if err != nil { return err } - if lastDirent { - if n := bytes.IndexByte(buf, 0); n == 0 { - log.Warningf("Corrupted dirent at inode (nid=%v)", i.Nid()) - return linuxerr.EUCLEAN - } else if n != -1 { - nameLen = uint32(n) - } - } - name := string(buf[:nameLen]) - if err := cb(name, d.FileType, d.Nid()); err != nil { + if err := cb(string(name), d.FileType, d.Nid()); err != nil { return err } - - // Go on to process the next adjacent dirent. - d = next - } - - blocks-- - - if blocks == 1 && i.idataOff != 0 { - // If we have any inline data and this is the last block, we need to process it now. - start = i.idataOff - } else { - // Just go on to process the next adjacent block. - start += uint64(blockSize) + if numDirents--; numDirents == 0 { + break + } + d = direntAfter(d) } } return nil @@ -794,7 +895,7 @@ func (i *Inode) Readlink() (string, error) { size := i.size if i.idataOff != 0 { // Inline symlink data shouldn't cross block boundary. - if i.blocks() > 1 { + if i.blocks > 1 { log.Warningf("Inline data cross block boundary at inode (nid=%v)", i.Nid()) return "", linuxerr.EUCLEAN } diff --git a/pkg/erofs/erofs_unsafe.go b/pkg/erofs/erofs_unsafe.go index 4dcf912ca..fffc44424 100644 --- a/pkg/erofs/erofs_unsafe.go +++ b/pkg/erofs/erofs_unsafe.go @@ -26,3 +26,12 @@ func (i *Image) pointerAt(off uint64) unsafe.Pointer { // unnecessary out-of-range panic calls in the code generated by the compiler. return unsafe.Pointer(&i.bytes[off]) } + +// direntAfter returns a pointer to the next adjacent dirent after dirent d. +// +// Preconditions: +// - d is a pointer to the memory backed by image. +// - d is not the last dirent in its block. +func direntAfter(d *Dirent) *Dirent { + return (*Dirent)(unsafe.Pointer(uintptr(unsafe.Pointer(d)) + DirentSize)) +} diff --git a/pkg/sentry/fsimpl/erofs/directory.go b/pkg/sentry/fsimpl/erofs/directory.go index fadc584f3..13203375a 100644 --- a/pkg/sentry/fsimpl/erofs/directory.go +++ b/pkg/sentry/fsimpl/erofs/directory.go @@ -61,12 +61,18 @@ func (i *inode) getDirents() ([]vfs.Dirent, error) { } func (i *inode) lookup(name string) (uint64, error) { - // TODO: For simplicity, currently a lookup will cause all dirents to be - // read and cached. But it hurts the performance of large directories. - // We should do binary search on disk data directly (like Linux does). - dirents, err := i.getDirents() - if err != nil { - return 0, err + var dirents []vfs.Dirent + + // Lazily fetch dirents. + if i.dirMu.TryRLock() { + dirents = i.dirents // +checklocksforce: TryRLock. + i.dirMu.RUnlock() // +checklocksforce: TryRLock. + } + + if dirents == nil { + // The dirents cache is not available immediately, let's do + // binary search on disk data directly. + return i.Lookup(name) } // The dirents are sorted in alphabetical order. We do binary search diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go index a63180065..5b08a132f 100644 --- a/runsc/container/container_test.go +++ b/runsc/container/container_test.go @@ -3353,3 +3353,121 @@ func TestCheckpointRestoreEROFS(t *testing.T) { }) } } + +// TestLookupEROFS reads the files in EROFS images, which contain some random files, +// and checks if the data is as expected. +func TestLookupEROFS(t *testing.T) { + // Skip this test if mkfs.erofs is not available. + skipIfNotAvailable(t, "mkfs.erofs") + + // Create a temporary directory to save the test files. + testDir, err := ioutil.TempDir(testutil.TmpDir(), "erofs_lookup_test_") + if err != nil { + t.Fatalf("ioutil.TempDir() failed: %v", err) + } + defer os.RemoveAll(testDir) + + spec, _ := sleepSpecConf(t) + conf := testutil.TestConfig(t) + _, bundleDir, cleanup, err := testutil.SetupContainer(spec, conf) + if err != nil { + t.Fatalf("error setting up container: %v", err) + } + defer cleanup() + + // Create and start the container. + args := Args{ + ID: testutil.RandomContainerID(), + Spec: spec, + BundleDir: bundleDir, + } + c, err := New(conf, args) + if err != nil { + t.Fatalf("error creating container: %v", err) + } + defer c.Destroy() + if err := c.Start(conf); err != nil { + t.Fatalf("error starting container: %v", err) + } + + tcs := []struct { + name string + size int + }{ + { + name: "tiny", + size: 1, + }, + { + name: "small", + size: 10, + }, + { + name: "medium", + size: 100, + }, + { + name: "large", + size: 1000, + }, + } + + targetDir := "/mnt" + for _, tc := range tcs { + // Add some randomness to the number of files. + size := tc.size + rand.Intn(tc.size) + + // Create a temporary directory with some random files in it, which will + // be used as the source directory to create the EROFS image. + sourceDir := filepath.Join(testDir, tc.name) + if err := os.Mkdir(sourceDir, 0755); err != nil { + t.Fatalf("os.Mkdir() failed: %v", err) + } + randomFiles := make([]string, 0, size) + for i := 0; i < size; i++ { + file, err := ioutil.TempFile(sourceDir, "") + if err != nil { + t.Fatalf("ioutil.TempFile() failed: %v", err) + } + name := filepath.Base(file.Name()) + if _, err := file.Write([]byte(name)); err != nil { + t.Fatalf("file.Write() failed: %v", err) + } + file.Close() + randomFiles = append(randomFiles, name) + } + + // Create the EROFS image. + imageFile := filepath.Join(testDir, fmt.Sprintf("%s.img", tc.name)) + if err := createImageEROFS(imageFile, sourceDir); err != nil { + t.Fatalf("error creating EROFS image: %v", err) + } + + // Mount the EROFS image in the container. + if err := c.Sandbox.Mount(c.ID, erofs.Name, imageFile, targetDir); err != nil { + t.Fatalf("error mounting EROFS image %q at %q, err: %v", imageFile, targetDir, err) + } + + // Read the files in the EROFS image and check if the data is as expected. + for i, inc := 0, max(size/100, 1); i < size; i += inc { + targetFile := randomFiles[i] + cmd := fmt.Sprintf("cat %s", filepath.Join(targetDir, targetFile)) + if out, err := executeCombinedOutput(conf, c, nil, "/bin/sh", "-c", cmd); err != nil { + t.Fatalf("exec: sh -c %q, err: %v, out: %s", cmd, err, out) + } else if targetFile != string(out) { + t.Errorf("file does not match, got: %s, expected: %s", out, targetFile) + } + } + + // Test for the read failure with a non-existent file. + cmd := fmt.Sprintf("cat %s", filepath.Join(targetDir, "nonexist")) + if out, err := executeCombinedOutput(conf, c, nil, "/bin/sh", "-c", cmd); err == nil { + t.Errorf("exec: sh -c %q, succeeded to read the non-existent file: %s", cmd, out) + } + + // Unmount the EROFS image in the container. + if out, err := executeCombinedOutput(conf, c, nil, "/bin/umount", targetDir); err != nil { + t.Fatalf("exec: umount %q, err: %v, out: %s", targetDir, err, out) + } + } +}