Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions conversions.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,11 +394,8 @@ func convertInMessage(
Uid: inMsg.Header().Uid,
},
}
if !config.UseVectoredRead {
// Use part of the incoming message storage as the read buffer
// For vectored zero-copy reads, don't allocate any buffers
to.Dst = inMsg.GetFree(int(in.Size))
}
// Use part of the incoming message storage as the read buffer.
to.Dst = inMsg.GetFree(int(in.Size))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this part of your change performance-neutral? Going by the comment, we’re now “allocating” (reserving?) more buffers.

cc @vitalif who contributed vectored read support back in 2022

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be performance-neutral because inMsg.GetFree() doesn't actually allocate. It merely makes the free space that was already allocated in the incoming message available for use by the output message.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. So let me confirm the facts to make sure we’re on the same page:

  1. Each buffer (InMessage) is MaxWriteSize (1 MB) + one page (4096 bytes) in size.
  2. When vectored read support is enabled, we currently stop using the 1 MB buffer.
  3. This means we’re “wasting” 1 MB of RAM, but only for read requests — write requests will fill that 1 MB.

Is that accurate?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is correct.

o = to

case fusekernel.OpReaddir:
Expand Down Expand Up @@ -933,10 +930,10 @@ func (c *Connection) kernelResponseForOp(
}

case *fuseops.ReadFileOp:
if o.Dst != nil {
m.Append(o.Dst)
} else {
if o.Data != nil {
m.Append(o.Data...)
} else {
m.Append(o.Dst)
}
m.ShrinkTo(buffer.OutMessageHeaderSize + o.BytesRead)

Expand Down
5 changes: 3 additions & 2 deletions fuseops/ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -704,11 +704,12 @@ type ReadFileOp struct {
Size int64

// The destination buffer, whose length gives the size of the read.
// For vectored reads, this field is always nil as the buffer is not provided.
// The file system can write to this buffer for non-vectored reads.
Dst []byte

// Set by the file system:
// A list of slices of data to send back to the client for vectored reads.
// A list of slices of data to send back to the client.
// If this field is populated, the contents of `Dst` will be ignored.
Data [][]byte

// Set by the file system: the number of bytes read.
Expand Down
28 changes: 21 additions & 7 deletions mount_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,13 +157,6 @@ type MountConfig struct {
// actually utilise any form of qualifiable UNIX permissions.
DisableDefaultPermissions bool

// Use vectored reads.
// Vectored read allows file systems to avoid memory copying overhead if
// the data is already in memory when they return it to FUSE.
// When turned on, ReadFileOp.Dst is always nil and the FS must return data
// being read from the file as a list of slices in ReadFileOp.Data.
UseVectoredRead bool

// OS X only.
//
// The name of the mounted volume, as displayed in the Finder. If empty, a
Expand Down Expand Up @@ -222,6 +215,27 @@ type MountConfig struct {
// If EnableReaddirplus is true and this flag is false, the kernel will always
// use ReaddirPlus for directory listing.
EnableAutoReaddirplus bool

// UseVectoredRead is a legacy flag kept for backward compatibility. It is now a no-op.
//
// The term vectored read was a misnomer for this flag. Its actual meaning was that
// the file system would allocate its own buffers for read operations.
//
// When this flag was disabled, the FUSE library provided a buffer
// in ReadFileOp.Dst. This buffer utilized unused space within the
// incoming kernel request message's buffer (InMessage.storage), which is
// sized to hold a page plus the maximum write size. Since read requests are
// small, there is ample room for read responses.
//
// Conversely, when this flag was enabled, ReadFileOp.Dst was always nil.
// This allowed file systems to avoid memory copying overhead if the data was
// already in memory, by requiring them to return the data as a list of slices
// in ReadFileOp.Data.
//
// Currently, both the read mechanisms can coexist. The library's behavior is
// to always provide ReadFileOp.Dst. If the file system populates ReadFileOp.Data,
// that data will be used for a vectored read, irrespective of this flag's value.
UseVectoredRead bool
}

type FUSEImpl uint8
Expand Down
23 changes: 23 additions & 0 deletions samples/memfs/inode.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,29 @@ func (in *inode) ReadAt(p []byte, off int64) (int, error) {
return n, nil
}

// Perform a vectored read from the file's contents.
//
// REQUIRES: in.isFile()
func (in *inode) VectoredReadAt(op *fuseops.ReadFileOp) (bytesRead int, err error) {
if !in.isFile() {
panic("VectoredReadAt called on non-file.")
}

// For testing, split the file into 1-byte chunks for the vectored read.
op.Data = make([][]byte, 0, len(in.contents))
for i := int64(0); i < int64(len(in.contents)); i++ {
if i >= op.Offset && op.BytesRead < int(op.Size) {
op.Data = append(op.Data, in.contents[i:i+1])
bytesRead++
}
}

if bytesRead < int(op.Size) {
err = io.EOF
}
return bytesRead, err
}

// Write to the file's contents. See documentation for ioutil.WriterAt.
//
// REQUIRES: in.isFile()
Expand Down
16 changes: 12 additions & 4 deletions samples/memfs/memfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ import (
)

const (
FileOpenFlagsXattrName = "fileOpenFlagsXattr"
CheckFileOpenFlagsFileName = "checkFileOpenFlags"
FileOpenFlagsXattrName = "fileOpenFlagsXattr"
CheckFileOpenFlagsFileName = "checkFileOpenFlags"
EnableVectoredReadXattrName = "enableVectoredRead"
)

type memFS struct {
Expand Down Expand Up @@ -663,8 +664,15 @@ func (fs *memFS) ReadFile(
inode := fs.getInodeOrDie(op.Inode)

// Serve the request.
var err error
op.BytesRead, err = inode.ReadAt(op.Dst, op.Offset)
var err error // Check if we should perform a vectored read.
if _, ok := inode.xattrs[EnableVectoredReadXattrName]; ok {
// For testing purpose only.
// Set attribute (name=EnableVectoredReadXattrName, value=true) to test
// whether vectored read is working correctly.
op.BytesRead, err = inode.VectoredReadAt(op)
} else {
op.BytesRead, err = inode.ReadAt(op.Dst, op.Offset)
}

op.Callback = fs.readFileCallback

Expand Down
38 changes: 38 additions & 0 deletions samples/memfs/memfs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1919,6 +1919,44 @@ func (t *MemFSTest) RemoveXAttr() {
AssertEq(fuse.ENOATTR, err)
}

func (t *MemFSTest) NonVectoredRead() {
var err error
const contents = "taco"
const fileName = "foo"
filePath := path.Join(t.Dir, fileName)

// Create a file.
err = ioutil.WriteFile(filePath, []byte(contents), 0600)
AssertEq(nil, err)

// Read the file. This will be a non-vectored read by default.
readContents, err := ioutil.ReadFile(filePath)
AssertEq(nil, err)

ExpectEq(contents, string(readContents))
}

func (t *MemFSTest) VectoredRead() {
var err error
const contents = "taco"
const fileName = "foo"
filePath := path.Join(t.Dir, fileName)

// Create a file.
err = ioutil.WriteFile(filePath, []byte(contents), 0600)
AssertEq(nil, err)

// Enable vectored reads for memfs for this file.
err = unix.Setxattr(filePath, memfs.EnableVectoredReadXattrName, []byte("true"), 0)
AssertEq(nil, err)

// Read the file.
readContents, err := ioutil.ReadFile(filePath)
AssertEq(nil, err)

ExpectEq(contents, string(readContents))
}

////////////////////////////////////////////////////////////////////////
// Mknod
////////////////////////////////////////////////////////////////////////
Expand Down
8 changes: 4 additions & 4 deletions samples/mount_readbenchfs/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ import (
"context"
"flag"
"fmt"
"github.com/jacobsa/fuse"
"github.com/jacobsa/fuse/samples/readbenchfs"
"log"
"net/http"
_ "net/http/pprof"
"os"

"github.com/jacobsa/fuse"
"github.com/jacobsa/fuse/samples/readbenchfs"
)

var fMountPoint = flag.String("mount_point", "", "Path to mount point.")
Expand Down Expand Up @@ -38,8 +39,7 @@ func main() {
}

cfg := &fuse.MountConfig{
ReadOnly: *fReadOnly,
UseVectoredRead: *fVectored,
ReadOnly: *fReadOnly,
}

if *fDebug {
Expand Down