Make P9 faster for large directories.

As of right now, runsc's fsgofer Readdir implementation is
awfully slow for large directories that have >2000 entries. So
slow that is deserves a description.

This happens because fsgofer confuses the unit of `Count` as
"number of dirents" rather than "number of bytes". lisafs does
not suffer from this.

Lets say there is a  large directory with 100,000 files. When the
application does `ls`, the gofer reads all 100,000 entries from
the host and populates a huge slice with all these dirents.

p9/messages.go:Rreaddir.encode() silently discards 98,000 of
those dirents because only ~2,000 of them fit in `Count` bytes.
Then the gofer client again makes a Readdir RPC with offset
2,000. The gofer reads all 100,000 files, skips first 2,000,
returns next 2,000 and discards 96,000. This repeats until
all files are returned.

Updated fsgofer to realize `Count` as number of bytes to read.
fsgofer only reads upto 80% of the count limit from the host
to take into account the fact that p9.Dirent takes more bytes
to be encoded than unix.Dirent. Added warning logging in
encode() when it is discarding dirents.

Before:
```
$ docker run --runtime=runsc --rm -v /host/test:/test ubuntu bash -c 'time ls test > /dev/null'

real	0m7.826s
user	0m0.120s
sys	0m0.030s
```

After:
```
$ docker run --runtime=runsc --rm -v /host/test:/test ubuntu bash -c 'time ls test > /dev/null'

real	0m0.635s
user	0m0.130s
sys	0m0.040s
```

Updates #6665

PiperOrigin-RevId: 469546979

pkg/p9/client_file.go

@@ -763,13 +763,13 @@ func (c *clientFile) UnlinkAt(name string, flags uint32) error {
 }
 
 // Readdir implements File.Readdir.
-func (c *clientFile) Readdir(offset uint64, count uint32) ([]Dirent, error) {
+func (c *clientFile) Readdir(direntOffset uint64, count uint32) ([]Dirent, error) {
 	if c.closed.Load() != 0 {
 		return nil, unix.EBADF
 	}
 
 	rreaddir := Rreaddir{}
-	if err := c.client.sendRecv(&Treaddir{Directory: c.fid, Offset: offset, Count: count}, &rreaddir); err != nil {
+	if err := c.client.sendRecv(&Treaddir{Directory: c.fid, DirentOffset: direntOffset, Count: count}, &rreaddir); err != nil {
 		return nil, err
 	}
 

pkg/p9/file.go

@@ -275,10 +275,19 @@ type File interface {
 
 	// Readdir reads directory entries.
 	//
-	// This may return io.EOF in addition to unix.Errno values.
+	// This may return io.EOF in addition to unix.Errno values. count is the
+	// number of bytes to read.
+	//
+	// direntOffset is the directory offset at which the read should happen.
+	// direntOffset can be set to 0 to start reading the directory from start.
+	// direntOffset is used more like a cookie. The unit of direntOffset is
+	// unspecified. Gofers can choose their own unit. The client must set it
+	// to one of the values returned in Dirent.Offset, preferably the last offset
+	// returned, which should cause the readdir to continue from where it was
+	// left off.
 	//
 	// On the server, Readdir has a read concurrency guarantee.
-	Readdir(offset uint64, count uint32) ([]Dirent, error)
+	Readdir(direntOffset uint64, count uint32) ([]Dirent, error)
 
 	// Readlink reads the link target.
 	//

pkg/p9/handlers.go

@@ -1093,7 +1093,7 @@ func (t *Treaddir) handle(cs *connState) message {
 		}
 
 		// Read the entries.
-		entries, err = ref.file.Readdir(t.Offset, t.Count)
+		entries, err = ref.file.Readdir(t.DirentOffset, t.Count)
 		if err != nil && err != io.EOF {
 			return err
 		}

pkg/p9/messages.go

@@ -19,6 +19,7 @@ import (
 	"math"
 
 	"gvisor.dev/gvisor/pkg/fd"
+	"gvisor.dev/gvisor/pkg/log"
 )
 
 // ErrInvalidMsgType is returned when an unsupported message type is found.
@@ -1916,8 +1917,8 @@ type Treaddir struct {
 	// Directory is the directory FID to read.
 	Directory FID
 
-	// Offset is the offset to read at.
-	Offset uint64
+	// DirentOffset is the dirent offset to read at.
+	DirentOffset uint64
 
 	// Count is the number of bytes to read.
 	Count uint32
@@ -1926,14 +1927,14 @@ type Treaddir struct {
 // decode implements encoder.decode.
 func (t *Treaddir) decode(b *buffer) {
 	t.Directory = b.ReadFID()
-	t.Offset = b.Read64()
+	t.DirentOffset = b.Read64()
 	t.Count = b.Read32()
 }
 
 // encode implements encoder.encode.
 func (t *Treaddir) encode(b *buffer) {
 	b.WriteFID(t.Directory)
-	b.Write64(t.Offset)
+	b.Write64(t.DirentOffset)
 	b.Write32(t.Count)
 }
 
@@ -1944,7 +1945,7 @@ func (*Treaddir) Type() MsgType {
 
 // String implements fmt.Stringer.
 func (t *Treaddir) String() string {
-	return fmt.Sprintf("Treaddir{DirectoryFID: %d, Offset: %d, Count: %d}", t.Directory, t.Offset, t.Count)
+	return fmt.Sprintf("Treaddir{DirectoryFID: %d, DirentOffset: %d, Count: %d}", t.Directory, t.DirentOffset, t.Count)
 }
 
 // Rreaddir is a readdir response.
@@ -1985,9 +1986,10 @@ func (r *Rreaddir) decode(b *buffer) {
 func (r *Rreaddir) encode(b *buffer) {
 	entriesBuf := buffer{}
 	payloadSize := 0
-	for _, d := range r.Entries {
+	for i, d := range r.Entries {
 		d.encode(&entriesBuf)
 		if len(entriesBuf.data) > int(r.Count) {
+			log.Warningf("hit Rreaddir.Count limit while encoding dirents, discarding %d dirents", len(r.Entries)-i)
 			break
 		}
 		payloadSize = len(entriesBuf.data)

pkg/p9/messages_test.go

@@ -221,9 +221,9 @@ func TestEncodeDecode(t *testing.T) {
 		},
 		&Rsetxattr{},
 		&Treaddir{
-			Directory: 1,
-			Offset:    2,
-			Count:     3,
+			Directory:    1,
+			DirentOffset: 2,
+			Count:        3,
 		},
 		&Rreaddir{
 			// Count must be sufficient to encode a dirent.

pkg/p9/p9.go

@@ -1084,6 +1084,10 @@ func (a *Attr) Apply(mask SetAttrMask, attr SetAttr) {
 	}
 }
 
+// DirentSizeStatic is the number of bytes required to encode a p9.Dirent
+// with an empty name. In other words, it is the static part of its size.
+const DirentSizeStatic = 24
+
 // Dirent is used for readdir.
 type Dirent struct {
 	// QID is the entry QID.

runsc/fsgofer/fsgofer.go

@@ -989,7 +989,7 @@ func (l *localFile) UnlinkAt(name string, flags uint32) error {
 }
 
 // Readdir implements p9.File.
-func (l *localFile) Readdir(offset uint64, count uint32) ([]p9.Dirent, error) {
+func (l *localFile) Readdir(direntOffset uint64, count uint32) ([]p9.Dirent, error) {
 	if l.mode != p9.ReadOnly && l.mode != p9.ReadWrite {
 		return nil, unix.EBADF
 	}
@@ -1010,18 +1010,18 @@ func (l *localFile) Readdir(offset uint64, count uint32) ([]p9.Dirent, error) {
 	// offset is 0, since this is side-effectual (equivalent to rewinddir(3),
 	// which causes the directory stream to resynchronize with the directory's
 	// current contents).
-	if l.lastDirentOffset != offset || offset == 0 {
+	if l.lastDirentOffset != direntOffset || direntOffset == 0 {
 		if _, err := unix.Seek(l.file.FD(), 0, 0); err != nil {
 			return nil, extractErrno(err)
 		}
-		skip = offset
+		skip = direntOffset
 	}
 
-	dirents, err := l.readDirent(l.file.FD(), offset, count, skip)
+	dirents, err := l.readDirent(l.file.FD(), direntOffset, count, skip)
 	if err == nil {
 		// On success, remember the offset that was returned at the current
 		// position.
-		l.lastDirentOffset = offset + uint64(len(dirents))
+		l.lastDirentOffset = direntOffset + uint64(len(dirents))
 	} else {
 		// On failure, the state is unknown, force call to seek() next time.
 		l.lastDirentOffset = math.MaxUint64
@@ -1029,57 +1029,62 @@ func (l *localFile) Readdir(offset uint64, count uint32) ([]p9.Dirent, error) {
 	return dirents, err
 }
 
-func (l *localFile) readDirent(f int, offset uint64, count uint32, skip uint64) ([]p9.Dirent, error) {
+func (l *localFile) readDirent(f int, direntOffset uint64, count uint32, skip uint64) ([]p9.Dirent, error) {
 	var dirents []p9.Dirent
 
-	// Limit 'count' to cap the slice size that is returned.
-	const maxCount = 100000
+	// p9.Dirent takes 5 extra bytes to be encoded than a unix.Dirent.
+	// count will be used to count against the number of bytes read from the
+	// host. So scale it down, so that when encoding the same dirents in p9, we
+	// don't hit count limit. Scale down to 80%.
+	count = (count * 8) / 10
+	// Limit 'count' to cap the amount of data that is returned.
+	const maxCount = 102_400
 	if count > maxCount {
 		count = maxCount
 	}
 
 	// Pre-allocate buffers that will be reused to get partial results.
 	direntsBuf := make([]byte, 8192)
-	names := make([]string, 0, 100)
-
-	end := offset + uint64(count)
-	for offset < end {
-		dirSize, err := unix.ReadDirent(f, direntsBuf)
+	for bytesRead := 0; bytesRead < int(count); {
+		bufEnd := len(direntsBuf)
+		if remaining := int(count) - bytesRead; remaining < bufEnd {
+			bufEnd = remaining
+		}
+		n, err := unix.Getdents(f, direntsBuf[:bufEnd])
 		if err != nil {
+			if err == unix.EINVAL && bufEnd < unixDirentMaxSize {
+				// getdents64(2) returns EINVAL when the result buffer is too small. If
+				// bufEnd is smaller than the max size of unix.Dirent, then just break
+				// here to return all dirents collected till now.
+				return dirents, nil
+			}
 			return dirents, err
 		}
-		if dirSize <= 0 {
+		if n <= 0 {
 			return dirents, nil
 		}
 
-		names := names[:0]
-		_, _, names = unix.ParseDirent(direntsBuf[:dirSize], -1, names)
-
-		// Skip over entries that the caller is not interested in.
-		if skip > 0 {
-			if skip > uint64(len(names)) {
-				skip -= uint64(len(names))
-				names = names[:0]
-			} else {
-				names = names[skip:]
-				skip = 0
-			}
-		}
-		for _, name := range names {
+		parseDirents(direntsBuf[:n], func(ino uint64, off int64, ftype uint8, name string, reclen uint16) bool {
 			stat, err := statAt(l.file.FD(), name)
 			if err != nil {
 				log.Warningf("Readdir is skipping file %q with failed stat, err: %v", path.Join(l.hostPath, name), err)
-				continue
+				return true
+			}
+			if skip > 0 {
+				skip--
+				return true
 			}
 			qid := l.attachPoint.makeQID(&stat)
-			offset++
+			direntOffset++
 			dirents = append(dirents, p9.Dirent{
 				QID:    qid,
 				Type:   qid.Type,
 				Name:   name,
-				Offset: offset,
+				Offset: direntOffset,
 			})
-		}
+			bytesRead += int(reclen)
+			return true
+		})
 	}
 	return dirents, nil
 }

runsc/fsgofer/fsgofer_test.go

@@ -738,7 +738,7 @@ func TestReaddir(t *testing.T) {
 			t.Fatalf("%v: Open(ReadOnly) failed, err: %v", s, err)
 		}
 
-		dirents, err := s.file.Readdir(0, 10)
+		dirents, err := s.file.Readdir(0, 4096)
 		if err != nil {
 			t.Fatalf("%v: Readdir(0, 10) failed, err: %v", s, err)
 		}

runsc/fsgofer/fsgofer_unsafe.go

@@ -21,7 +21,7 @@ import (
 	"gvisor.dev/gvisor/pkg/syserr"
 )
 
-var unixDirentMaxSize uint32 = uint32(unsafe.Sizeof(unix.Dirent{}))
+var unixDirentMaxSize = int(unsafe.Sizeof(unix.Dirent{}))
 
 func utimensat(dirFd int, name string, times [2]unix.Timespec, flags int) error {
 	// utimensat(2) doesn't accept empty name, instead name must be nil to make it
@@ -83,11 +83,14 @@ func renameat(oldDirFD int, oldName string, newDirFD int, newName string) error
 	return nil
 }
 
-func parseDirents(buf []byte, handleDirent func(ino uint64, off int64, ftype uint8, name string) bool) {
+func parseDirents(buf []byte, handleDirent func(ino uint64, off int64, ftype uint8, name string, reclen uint16) bool) {
 	for len(buf) > 0 {
 		// Interpret the buf populated by unix.Getdents as unix.Dirent.
 		dirent := *(*unix.Dirent)(unsafe.Pointer(&buf[0]))
 
+		// Advance buf for the next dirent.
+		buf = buf[dirent.Reclen:]
+
 		// Extracting the name is pretty tedious...
 		var nameBuf [unix.NAME_MAX]byte
 		var nameLen int
@@ -101,12 +104,15 @@ func parseDirents(buf []byte, handleDirent func(ino uint64, off int64, ftype uin
 		}
 		name := string(nameBuf[:nameLen])
 
-		// Deliver results to caller.
-		if !handleDirent(dirent.Ino, dirent.Off, dirent.Type, name) {
-			return
+		// Skip `.` and `..` entries. It is anyways ignored by the client. We also
+		// don't want to leak information about `..`.
+		if name == "." || name == ".." {
+			continue
 		}
 
-		// Advance buf for the next dirent.
-		buf = buf[dirent.Reclen:]
+		// Deliver results to caller.
+		if !handleDirent(dirent.Ino, dirent.Off, dirent.Type, name, dirent.Reclen) {
+			return
+		}
 	}
 }

runsc/fsgofer/lisafs.go

@@ -842,7 +842,7 @@ func (fd *openFDLisa) Getdent64(count uint32, seek0 bool, recordDirent func(lisa
 		}
 		n, err := unix.Getdents(fd.hostFD, direntsBuf[:bufEnd])
 		if err != nil {
-			if err == unix.EINVAL && bufEnd < 268 {
+			if err == unix.EINVAL && bufEnd < unixDirentMaxSize {
 				// getdents64(2) returns EINVAL is returned when the result
 				// buffer is too small. If bufEnd is smaller than the max
 				// size of unix.Dirent, then just break here to return all
@@ -854,9 +854,8 @@ func (fd *openFDLisa) Getdent64(count uint32, seek0 bool, recordDirent func(lisa
 		if n <= 0 {
 			break
 		}
-		bytesRead += n
 
-		parseDirents(direntsBuf[:n], func(ino uint64, off int64, ftype uint8, name string) bool {
+		parseDirents(direntsBuf[:n], func(ino uint64, off int64, ftype uint8, name string, reclen uint16) bool {
 			dirent := lisafs.Dirent64{
 				Ino:  primitive.Uint64(ino),
 				Off:  primitive.Uint64(off),
@@ -874,6 +873,7 @@ func (fd *openFDLisa) Getdent64(count uint32, seek0 bool, recordDirent func(lisa
 			dirent.DevMinor = primitive.Uint32(unix.Minor(stat.Dev))
 			dirent.DevMajor = primitive.Uint32(unix.Major(stat.Dev))
 			recordDirent(dirent)
+			bytesRead += int(reclen)
 			return true
 		})
 	}