From f5d80cf9abe9908e03a5ece7b639e02767fb17b5 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 29 Jul 2015 09:48:18 +1000 Subject: [PATCH 01/13] Defined the new API for ReadFileOp and ReadDirOp. --- fuseops/ops.go | 49 +++++++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/fuseops/ops.go b/fuseops/ops.go index d73c42c..5a857bf 100644 --- a/fuseops/ops.go +++ b/fuseops/ops.go @@ -384,25 +384,25 @@ type ReadDirOp struct { // offset, and return array offsets into that cached listing. Offset DirOffset - // The maximum number of bytes to return in ReadDirResponse.Data. A smaller - // number is acceptable. - Size int - - // Set by the file system: a buffer consisting of a sequence of FUSE - // directory entries in the format generated by fuse_add_direntry - // (http://goo.gl/qCcHCV), which is consumed by parse_dirfile - // (http://goo.gl/2WUmD2). Use fuseutil.AppendDirent to generate this data. + // The destination buffer, whose length gives the size of the read. // - // The buffer must not exceed the length specified in ReadDirRequest.Size. It - // is okay for the final entry to be truncated; parse_dirfile copes with this - // by ignoring the partial record. + // The output data should consist of a sequence of FUSE directory entries in + // the format generated by fuse_add_direntry (http://goo.gl/qCcHCV), which is + // consumed by parse_dirfile (http://goo.gl/2WUmD2). Use + // fuseutil.AppendDirent to generate this data. + // + // It is okay for the final entry to be truncated if it doesn't fit; + // parse_dirfile copes with this by ignoring the partial record. // // Each entry returned exposes a directory offset to the user that may later // show up in ReadDirRequest.Offset. See notes on that field for more // information. - // - // An empty buffer indicates the end of the directory has been reached. - Data []byte + Dst []byte + + // Set by the file system: the number of bytes read into Dst. It is okay for + // this to be less than len(Dst) if there is less data available. A value of + // zero means that the end of the directory has been reached. + BytesRead int } // Release a previously-minted directory handle. The kernel sends this when @@ -455,20 +455,21 @@ type ReadFileOp struct { Inode InodeID Handle HandleID - // The range of the file to read. + // The offset within the file at which to read. + Offset int64 + + // The destination buffer, whose length gives the size of the read. + Dst []byte + + // Set by the file system: the number of bytes read. // - // The FUSE documentation requires that exactly the number of bytes be - // returned, except in the case of EOF or error (http://goo.gl/ZgfBkF). This - // appears to be because it uses file mmapping machinery + // The FUSE documentation requires that exactly the requested number of bytes + // be returned, except in the case of EOF or error (http://goo.gl/ZgfBkF). + // This appears to be because it uses file mmapping machinery // (http://goo.gl/SGxnaN) to read a page at a time. It appears to understand // where EOF is by checking the inode size (http://goo.gl/0BkqKD), returned // by a previous call to LookUpInode, GetInodeAttributes, etc. - Offset int64 - Size int - - // Set by the file system: the data read. If this is less than the requested - // size, it indicates EOF. An error should not be returned in this case. - Data []byte + BytesRead int } // Write data to a file previously opened with CreateFile or OpenFile. From c08043788acd2ade660fc2e0b862d1f87462c6b2 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 29 Jul 2015 09:49:28 +1000 Subject: [PATCH 02/13] Fixed conversions.go. --- conversions.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/conversions.go b/conversions.go index feedf0d..481fc39 100644 --- a/conversions.go +++ b/conversions.go @@ -252,7 +252,7 @@ func convertInMessage( Inode: fuseops.InodeID(m.Header().Nodeid), Handle: fuseops.HandleID(in.Fh), Offset: int64(in.Offset), - Size: int(in.Size), + Dst: make([]byte, in.Size), } case fusekernel.OpReaddir: @@ -266,7 +266,7 @@ func convertInMessage( Inode: fuseops.InodeID(m.Header().Nodeid), Handle: fuseops.HandleID(in.Fh), Offset: fuseops.DirOffset(in.Offset), - Size: int(in.Size), + Dst: make([]byte, in.Size), } case fusekernel.OpRelease: @@ -486,7 +486,7 @@ func (c *Connection) kernelResponseForOp( case *fuseops.ReadDirOp: m = c.getOutMessage() - m.Append(o.Data) + m.Append(o.Dst[:o.BytesRead]) case *fuseops.ReleaseDirHandleOp: m = c.getOutMessage() @@ -498,7 +498,7 @@ func (c *Connection) kernelResponseForOp( case *fuseops.ReadFileOp: m = c.getOutMessage() - m.Append(o.Data) + m.Append(o.Dst[:o.BytesRead]) case *fuseops.WriteFileOp: m = c.getOutMessage() From 6d01ffa44f803c3cc4c58ad7b8555fa4f1347564 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 29 Jul 2015 10:08:16 +1000 Subject: [PATCH 03/13] Replaced AppendDirent with WriteDirent. --- fuseops/ops.go | 20 ++++++++++++-------- fuseutil/dirent.go | 42 +++++++++++++++++++++++++++--------------- 2 files changed, 39 insertions(+), 23 deletions(-) diff --git a/fuseops/ops.go b/fuseops/ops.go index 5a857bf..6e6b412 100644 --- a/fuseops/ops.go +++ b/fuseops/ops.go @@ -388,20 +388,24 @@ type ReadDirOp struct { // // The output data should consist of a sequence of FUSE directory entries in // the format generated by fuse_add_direntry (http://goo.gl/qCcHCV), which is - // consumed by parse_dirfile (http://goo.gl/2WUmD2). Use - // fuseutil.AppendDirent to generate this data. - // - // It is okay for the final entry to be truncated if it doesn't fit; - // parse_dirfile copes with this by ignoring the partial record. + // consumed by parse_dirfile (http://goo.gl/2WUmD2). Use fuseutil.WriteDirent + // to generate this data. // // Each entry returned exposes a directory offset to the user that may later // show up in ReadDirRequest.Offset. See notes on that field for more // information. Dst []byte - // Set by the file system: the number of bytes read into Dst. It is okay for - // this to be less than len(Dst) if there is less data available. A value of - // zero means that the end of the directory has been reached. + // Set by the file system: the number of bytes read into Dst. + // + // It is okay for this to be less than len(Dst) if there are not enough + // entries available or the final entry would not fit. + // + // Zero means that the end of the directory has been reached. This is + // unambiguous because NAME_MAX (https://goo.gl/ZxzKaE) plus the size of + // fuse_dirent (https://goo.gl/WO8s3F) plus the 8-byte alignment of + // FUSE_DIRENT_ALIGN (http://goo.gl/UziWvH) is less than the read size of + // PAGE_SIZE used by fuse_readdir (cf. https://goo.gl/VajtS2). BytesRead int } diff --git a/fuseutil/dirent.go b/fuseutil/dirent.go index fcd5bdd..28bbc2f 100644 --- a/fuseutil/dirent.go +++ b/fuseutil/dirent.go @@ -35,7 +35,7 @@ const ( ) // A struct representing an entry within a directory file, describing a child. -// See notes on fuseops.ReadDirOp and on AppendDirent for details. +// See notes on fuseops.ReadDirOp and on WriteDirent for details. type Dirent struct { // The (opaque) offset within the directory file of the entry following this // one. See notes on fuseops.ReadDirOp.Offset for details. @@ -50,10 +50,11 @@ type Dirent struct { Type DirentType } -// Append the supplied directory entry to the given buffer in the format -// expected in fuseops.ReadFileOp.Data, returning the resulting buffer. -func AppendDirent(input []byte, d Dirent) (output []byte) { - // We want to append bytes with the layout of fuse_dirent +// Write the supplied directory entry intto the given buffer in the format +// expected in fuseops.ReadFileOp.Data, returning the number of bytes written. +// Return zero if the entry would not fit. +func WriteDirent(buf []byte, d Dirent) (n int) { + // We want to write bytes with the layout of fuse_dirent // (http://goo.gl/BmFxob) in host order. The struct must be aligned according // to FUSE_DIRENT_ALIGN (http://goo.gl/UziWvH), which dictates 8-byte // alignment. @@ -65,10 +66,23 @@ func AppendDirent(input []byte, d Dirent) (output []byte) { name [0]byte } - const alignment = 8 - const nameOffset = 8 + 8 + 4 + 4 + const direntAlignment = 8 + const direntSize = 8 + 8 + 4 + 4 - // Write the header into the buffer. + // Compute the number of bytes of padding we'll need to maintain alignment + // for the next entry. + var padLen int + if len(d.Name)%direntAlignment != 0 { + padLen = direntAlignment - (len(d.Name) % direntAlignment) + } + + // Do we have enough room? + totalLen := direntSize + len(d.Name) + padLen + if totalLen > len(buf) { + return + } + + // Write the header. de := fuse_dirent{ ino: uint64(d.Inode), off: uint64(d.Offset), @@ -76,17 +90,15 @@ func AppendDirent(input []byte, d Dirent) (output []byte) { type_: uint32(d.Type), } - output = append(input, (*[nameOffset]byte)(unsafe.Pointer(&de))[:]...) + n += copy(buf[n:], (*[direntSize]byte)(unsafe.Pointer(&de))[:]) // Write the name afterward. - output = append(output, d.Name...) + n += copy(buf[n:], d.Name) // Add any necessary padding. - if len(d.Name)%alignment != 0 { - padLen := alignment - (len(d.Name) % alignment) - - var padding [alignment]byte - output = append(output, padding[:padLen]...) + if padLen != 0 { + var padding [direntAlignment]byte + n += copy(buf, padding[:padLen]) } return From 339bc03e5eb9233c132e86317b4c5d0206a123b6 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 29 Jul 2015 10:12:11 +1000 Subject: [PATCH 04/13] Attempted to fix memfs. --- samples/memfs/inode.go | 11 +++++------ samples/memfs/memfs.go | 10 +++++----- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/samples/memfs/inode.go b/samples/memfs/inode.go index 78eca0b..2ec2397 100644 --- a/samples/memfs/inode.go +++ b/samples/memfs/inode.go @@ -278,7 +278,7 @@ func (in *inode) RemoveChild(name string) { // Serve a ReadDir request. // // REQUIRES: in.isDir() -func (in *inode) ReadDir(offset int, size int) (data []byte) { +func (in *inode) ReadDir(p []byte, offset int) (n int) { if !in.isDir() { panic("ReadDir called on non-directory.") } @@ -291,13 +291,12 @@ func (in *inode) ReadDir(offset int, size int) (data []byte) { continue } - data = fuseutil.AppendDirent(data, in.entries[i]) - - // Trim and stop early if we've exceeded the requested size. - if len(data) > size { - data = data[:size] + tmp := fuseutil.WriteDirent(p[n:], in.entries[i]) + if n == 0 { break } + + n += tmp } return diff --git a/samples/memfs/memfs.go b/samples/memfs/memfs.go index e1bf030..b2499ae 100644 --- a/samples/memfs/memfs.go +++ b/samples/memfs/memfs.go @@ -428,7 +428,9 @@ func (fs *memFS) Rename( existingID, _, ok := newParent.LookUpChild(op.NewName) if ok { existing := fs.getInodeOrDie(existingID) - if existing.isDir() && len(existing.ReadDir(0, 1024)) > 0 { + + var buf [4096]byte + if existing.isDir() && existing.ReadDir(buf[:], 0) > 0 { err = fuse.ENOTEMPTY return } @@ -538,7 +540,7 @@ func (fs *memFS) ReadDir( inode := fs.getInodeOrDie(op.Inode) // Serve the request. - op.Data = inode.ReadDir(int(op.Offset), op.Size) + op.BytesRead = inode.ReadDir(op.Dst, int(op.Offset)) return } @@ -571,9 +573,7 @@ func (fs *memFS) ReadFile( inode := fs.getInodeOrDie(op.Inode) // Serve the request. - op.Data = make([]byte, op.Size) - n, err := inode.ReadAt(op.Data, op.Offset) - op.Data = op.Data[:n] + op.BytesRead, err = inode.ReadAt(op.Dst, op.Offset) // Don't return EOF errors; we just indicate EOF to fuse using a short read. if err == io.EOF { From 5296835c1fed280804f9c2d7c89c7948552cdd9f Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 29 Jul 2015 10:12:59 +1000 Subject: [PATCH 05/13] Fixed a dumb bug. --- samples/memfs/inode.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/memfs/inode.go b/samples/memfs/inode.go index 2ec2397..ea6c172 100644 --- a/samples/memfs/inode.go +++ b/samples/memfs/inode.go @@ -292,7 +292,7 @@ func (in *inode) ReadDir(p []byte, offset int) (n int) { } tmp := fuseutil.WriteDirent(p[n:], in.entries[i]) - if n == 0 { + if tmp == 0 { break } From b0d206254f464655893e8c1c8547390a8cbb618c Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 29 Jul 2015 10:20:53 +1000 Subject: [PATCH 06/13] Fixed another dumb bug. --- fuseutil/dirent.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fuseutil/dirent.go b/fuseutil/dirent.go index 28bbc2f..dbe7754 100644 --- a/fuseutil/dirent.go +++ b/fuseutil/dirent.go @@ -98,7 +98,7 @@ func WriteDirent(buf []byte, d Dirent) (n int) { // Add any necessary padding. if padLen != 0 { var padding [direntAlignment]byte - n += copy(buf, padding[:padLen]) + n += copy(buf[n:], padding[:padLen]) } return From 85b3f9ed423e210de2e580218d61cfa4249722ee Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 29 Jul 2015 10:22:26 +1000 Subject: [PATCH 07/13] Fixed hellofs. --- samples/hellofs/hello_fs.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/samples/hellofs/hello_fs.go b/samples/hellofs/hello_fs.go index 59d2c1a..0d31559 100644 --- a/samples/hellofs/hello_fs.go +++ b/samples/hellofs/hello_fs.go @@ -228,11 +228,12 @@ func (fs *helloFS) ReadDir( // Resume at the specified offset into the array. for _, e := range entries { - op.Data = fuseutil.AppendDirent(op.Data, e) - if len(op.Data) > op.Size { - op.Data = op.Data[:op.Size] + n := fuseutil.WriteDirent(op.Dst[op.BytesRead:], e) + if n == 0 { break } + + op.BytesRead += n } return @@ -251,9 +252,7 @@ func (fs *helloFS) ReadFile( // Let io.ReaderAt deal with the semantics. reader := strings.NewReader("Hello, world!") - op.Data = make([]byte, op.Size) - n, err := reader.ReadAt(op.Data, op.Offset) - op.Data = op.Data[:n] + op.BytesRead, err = reader.ReadAt(op.Dst, op.Offset) // Special case: FUSE doesn't expect us to return io.EOF. if err == io.EOF { From c12f80b1e27dec0fe7d9ee558dfe06335b8bee1f Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 29 Jul 2015 10:23:51 +1000 Subject: [PATCH 08/13] Fixed flushfs. --- samples/flushfs/flush_fs.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/samples/flushfs/flush_fs.go b/samples/flushfs/flush_fs.go index 5a642de..7dea888 100644 --- a/samples/flushfs/flush_fs.go +++ b/samples/flushfs/flush_fs.go @@ -180,8 +180,7 @@ func (fs *flushFS) ReadFile( } // Read what we can. - op.Data = make([]byte, op.Size) - copy(op.Data, fs.fooContents[op.Offset:]) + op.BytesRead = copy(op.Dst, fs.fooContents[op.Offset:]) return } @@ -298,13 +297,15 @@ func (fs *flushFS) ReadDir( // Fill in the listing. for _, de := range dirents { - op.Data = fuseutil.AppendDirent(op.Data, de) - } + n := fuseutil.WriteDirent(op.Dst[op.BytesRead:], de) - // We don't support doing this in anything more than one shot. - if len(op.Data) > op.Size { - err = fmt.Errorf("Couldn't fit listing in %v bytes", op.Size) - return + // We don't support doing this in anything more than one shot. + if n == 0 { + err = fmt.Errorf("Couldn't fit listing in %v bytes", len(op.Dst)) + return + } + + op.BytesRead += n } return From d903c709d3a473437c850de76c9b31c87441a50e Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 29 Jul 2015 10:39:37 +1000 Subject: [PATCH 09/13] Refactored allocation out OutMessage structs. In preparation for allocating them up-front for ReadFile and ReadDir. --- connection.go | 10 ++++---- conversions.go | 62 ++++++++++++++++++-------------------------------- 2 files changed, 28 insertions(+), 44 deletions(-) diff --git a/connection.go b/connection.go index d35184b..4f9b4d7 100644 --- a/connection.go +++ b/connection.go @@ -450,15 +450,17 @@ func (c *Connection) Reply(ctx context.Context, opErr error) { } // Send the reply to the kernel, if one is required. - outMsg := c.kernelResponse(m.Header().Unique, op, opErr) - if outMsg != nil { - err := c.writeMessage(outMsg.Bytes()) - c.putOutMessage(outMsg) + outMsg := c.getOutMessage() + noResponse := c.kernelResponse(outMsg, m.Header().Unique, op, opErr) + if !noResponse { + err := c.writeMessage(outMsg.Bytes()) if err != nil && c.errorLogger != nil { c.errorLogger.Printf("writeMessage: %v", err) } } + + c.putOutMessage(outMsg) } // Close the connection. Must not be called until operations that were read diff --git a/conversions.go b/conversions.go index 481fc39..558d1d1 100644 --- a/conversions.go +++ b/conversions.go @@ -387,51 +387,45 @@ func convertInMessage( // Outgoing messages //////////////////////////////////////////////////////////////////////// -// Return the response that should be sent to the kernel, or nil if the op -// requires no response. +// Fill in the response that should be sent to the kernel, or set noResponse if +// the op requires no response. func (c *Connection) kernelResponse( + m *buffer.OutMessage, fuseID uint64, op interface{}, - opErr error) (m *buffer.OutMessage) { - // If the user replied with an error, create a response containing just the - // result header with the error filled in. Otherwise create an appropriate - // response. + opErr error) (noResponse bool) { + h := m.OutHeader() + h.Unique = fuseID + + // Did the user return an error? Otherwise, fill in the rest of the response. if opErr != nil { - m = c.getOutMessage() if errno, ok := opErr.(syscall.Errno); ok { m.OutHeader().Error = -int32(errno) } else { m.OutHeader().Error = -int32(syscall.EIO) } } else { - m = c.kernelResponseForOp(op) - } - - // Fill in the rest of the header, if a response is required. - if m != nil { - h := m.OutHeader() - h.Unique = fuseID - h.Len = uint32(m.Len()) + noResponse = c.kernelResponseForOp(m, op) } + h.Len = uint32(m.Len()) return } // Like kernelResponse, but assumes the user replied with a nil error to the -// op. Returns a nil response if no response is required. +// op. func (c *Connection) kernelResponseForOp( - op interface{}) (m *buffer.OutMessage) { + m *buffer.OutMessage, + op interface{}) (noResponse bool) { // Create the appropriate output message switch o := op.(type) { case *fuseops.LookUpInodeOp: size := fusekernel.EntryOutSize(c.protocol) - m = c.getOutMessage() out := (*fusekernel.EntryOut)(m.Grow(size)) convertChildInodeEntry(&o.Entry, out) case *fuseops.GetInodeAttributesOp: size := fusekernel.AttrOutSize(c.protocol) - m = c.getOutMessage() out := (*fusekernel.AttrOut)(m.Grow(size)) out.AttrValid, out.AttrValidNsec = convertExpirationTime( o.AttributesExpiration) @@ -439,24 +433,21 @@ func (c *Connection) kernelResponseForOp( case *fuseops.SetInodeAttributesOp: size := fusekernel.AttrOutSize(c.protocol) - m = c.getOutMessage() out := (*fusekernel.AttrOut)(m.Grow(size)) out.AttrValid, out.AttrValidNsec = convertExpirationTime( o.AttributesExpiration) convertAttributes(o.Inode, &o.Attributes, &out.Attr) case *fuseops.ForgetInodeOp: - // No response. + noResponse = true case *fuseops.MkDirOp: size := fusekernel.EntryOutSize(c.protocol) - m = c.getOutMessage() out := (*fusekernel.EntryOut)(m.Grow(size)) convertChildInodeEntry(&o.Entry, out) case *fuseops.CreateFileOp: eSize := fusekernel.EntryOutSize(c.protocol) - m = c.getOutMessage() e := (*fusekernel.EntryOut)(m.Grow(eSize)) convertChildInodeEntry(&o.Entry, e) @@ -466,67 +457,58 @@ func (c *Connection) kernelResponseForOp( case *fuseops.CreateSymlinkOp: size := fusekernel.EntryOutSize(c.protocol) - m = c.getOutMessage() out := (*fusekernel.EntryOut)(m.Grow(size)) convertChildInodeEntry(&o.Entry, out) case *fuseops.RenameOp: - m = c.getOutMessage() + // Empty response case *fuseops.RmDirOp: - m = c.getOutMessage() + // Empty response case *fuseops.UnlinkOp: - m = c.getOutMessage() + // Empty response case *fuseops.OpenDirOp: - m = c.getOutMessage() out := (*fusekernel.OpenOut)(m.Grow(unsafe.Sizeof(fusekernel.OpenOut{}))) out.Fh = uint64(o.Handle) case *fuseops.ReadDirOp: - m = c.getOutMessage() m.Append(o.Dst[:o.BytesRead]) case *fuseops.ReleaseDirHandleOp: - m = c.getOutMessage() + // Empty response case *fuseops.OpenFileOp: - m = c.getOutMessage() out := (*fusekernel.OpenOut)(m.Grow(unsafe.Sizeof(fusekernel.OpenOut{}))) out.Fh = uint64(o.Handle) case *fuseops.ReadFileOp: - m = c.getOutMessage() m.Append(o.Dst[:o.BytesRead]) case *fuseops.WriteFileOp: - m = c.getOutMessage() out := (*fusekernel.WriteOut)(m.Grow(unsafe.Sizeof(fusekernel.WriteOut{}))) out.Size = uint32(len(o.Data)) case *fuseops.SyncFileOp: - m = c.getOutMessage() + // Empty response case *fuseops.FlushFileOp: - m = c.getOutMessage() + // Empty response case *fuseops.ReleaseFileHandleOp: - m = c.getOutMessage() + // Empty response case *fuseops.ReadSymlinkOp: - m = c.getOutMessage() m.AppendString(o.Target) case *statFSOp: - m = c.getOutMessage() m.Grow(unsafe.Sizeof(fusekernel.StatfsOut{})) case *interruptOp: - // No response. + noResponse = true case *initOp: - m = c.getOutMessage() out := (*fusekernel.InitOut)(m.Grow(unsafe.Sizeof(fusekernel.InitOut{}))) out.Major = o.Library.Major From 2e422a1305e6b2db1510b8d0ce1eb279bca5008d Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 29 Jul 2015 10:51:24 +1000 Subject: [PATCH 10/13] Move where out messages are created. --- connection.go | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/connection.go b/connection.go index 4f9b4d7..d888e68 100644 --- a/connection.go +++ b/connection.go @@ -86,9 +86,10 @@ type Connection struct { // State that is maintained for each in-flight op. This is stuffed into the // context that the user uses to reply to the op. type opState struct { - inMsg *buffer.InMessage - op interface{} - opID uint32 // For logging + inMsg *buffer.InMessage + outMsg *buffer.OutMessage + op interface{} + opID uint32 // For logging } // Create a connection wrapping the supplied file descriptor connected to the @@ -370,14 +371,14 @@ func (c *Connection) ReadOp() (ctx context.Context, op interface{}, err error) { // Keep going until we find a request we know how to convert. for { // Read the next message from the kernel. - var m *buffer.InMessage - m, err = c.readMessage() + var inMsg *buffer.InMessage + inMsg, err = c.readMessage() if err != nil { return } // Convert the message to an op. - op, err = convertInMessage(m, c.protocol) + op, err = convertInMessage(inMsg, c.protocol) if err != nil { err = fmt.Errorf("convertInMessage: %v", err) return @@ -395,9 +396,12 @@ func (c *Connection) ReadOp() (ctx context.Context, op interface{}, err error) { continue } + // Allocate an output message up front, to be used later when replying. + outMsg := c.getOutMessage() + // Set up a context that remembers information about this op. - ctx = c.beginOp(m.Header().Opcode, m.Header().Unique) - ctx = context.WithValue(ctx, contextKey, opState{m, op, opID}) + ctx = c.beginOp(inMsg.Header().Opcode, inMsg.Header().Unique) + ctx = context.WithValue(ctx, contextKey, opState{inMsg, outMsg, op, opID}) // Special case: responding to statfs is required to make mounting work on // OS X. We don't currently expose the capability for the file system to @@ -426,14 +430,16 @@ func (c *Connection) Reply(ctx context.Context, opErr error) { } op := state.op - m := state.inMsg + inMsg := state.inMsg + outMsg := state.outMsg opID := state.opID - // Make sure we destroy the message when we're done. - defer c.putInMessage(m) + // Make sure we destroy the messages when we're done. + defer c.putInMessage(inMsg) + defer c.putOutMessage(outMsg) // Clean up state for this op. - c.finishOp(m.Header().Opcode, m.Header().Unique) + c.finishOp(inMsg.Header().Opcode, inMsg.Header().Unique) // Debug logging if c.debugLogger != nil { @@ -450,8 +456,7 @@ func (c *Connection) Reply(ctx context.Context, opErr error) { } // Send the reply to the kernel, if one is required. - outMsg := c.getOutMessage() - noResponse := c.kernelResponse(outMsg, m.Header().Unique, op, opErr) + noResponse := c.kernelResponse(outMsg, inMsg.Header().Unique, op, opErr) if !noResponse { err := c.writeMessage(outMsg.Bytes()) @@ -459,8 +464,6 @@ func (c *Connection) Reply(ctx context.Context, opErr error) { c.errorLogger.Printf("writeMessage: %v", err) } } - - c.putOutMessage(outMsg) } // Close the connection. Must not be called until operations that were read From bbb262ee48256894f8b3cb496554cda9c725206d Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 29 Jul 2015 11:02:29 +1000 Subject: [PATCH 11/13] Read directly into out messages for files. --- connection.go | 7 +-- conversions.go | 110 +++++++++++++++++++-------------- internal/buffer/out_message.go | 16 ++++- 3 files changed, 81 insertions(+), 52 deletions(-) diff --git a/connection.go b/connection.go index d888e68..9c3ce74 100644 --- a/connection.go +++ b/connection.go @@ -378,8 +378,10 @@ func (c *Connection) ReadOp() (ctx context.Context, op interface{}, err error) { } // Convert the message to an op. - op, err = convertInMessage(inMsg, c.protocol) + outMsg := c.getOutMessage() + op, err = convertInMessage(inMsg, outMsg, c.protocol) if err != nil { + c.putOutMessage(outMsg) err = fmt.Errorf("convertInMessage: %v", err) return } @@ -396,9 +398,6 @@ func (c *Connection) ReadOp() (ctx context.Context, op interface{}, err error) { continue } - // Allocate an output message up front, to be used later when replying. - outMsg := c.getOutMessage() - // Set up a context that remembers information about this op. ctx = c.beginOp(inMsg.Header().Opcode, inMsg.Header().Unique) ctx = context.WithValue(ctx, contextKey, opState{inMsg, outMsg, op, opID}) diff --git a/conversions.go b/conversions.go index 558d1d1..3fd23bb 100644 --- a/conversions.go +++ b/conversions.go @@ -19,6 +19,7 @@ import ( "errors" "fmt" "os" + "reflect" "syscall" "time" "unsafe" @@ -37,11 +38,12 @@ import ( // // The caller is responsible for arranging for the message to be destroyed. func convertInMessage( - m *buffer.InMessage, + inMsg *buffer.InMessage, + outMsg *buffer.OutMessage, protocol fusekernel.Protocol) (o interface{}, err error) { - switch m.Header().Opcode { + switch inMsg.Header().Opcode { case fusekernel.OpLookup: - buf := m.ConsumeBytes(m.Len()) + buf := inMsg.ConsumeBytes(inMsg.Len()) n := len(buf) if n == 0 || buf[n-1] != '\x00' { err = errors.New("Corrupt OpLookup") @@ -49,25 +51,25 @@ func convertInMessage( } o = &fuseops.LookUpInodeOp{ - Parent: fuseops.InodeID(m.Header().Nodeid), + Parent: fuseops.InodeID(inMsg.Header().Nodeid), Name: string(buf[:n-1]), } case fusekernel.OpGetattr: o = &fuseops.GetInodeAttributesOp{ - Inode: fuseops.InodeID(m.Header().Nodeid), + Inode: fuseops.InodeID(inMsg.Header().Nodeid), } case fusekernel.OpSetattr: type input fusekernel.SetattrIn - in := (*input)(m.Consume(unsafe.Sizeof(input{}))) + in := (*input)(inMsg.Consume(unsafe.Sizeof(input{}))) if in == nil { err = errors.New("Corrupt OpSetattr") return } to := &fuseops.SetInodeAttributesOp{ - Inode: fuseops.InodeID(m.Header().Nodeid), + Inode: fuseops.InodeID(inMsg.Header().Nodeid), } o = to @@ -93,25 +95,25 @@ func convertInMessage( case fusekernel.OpForget: type input fusekernel.ForgetIn - in := (*input)(m.Consume(unsafe.Sizeof(input{}))) + in := (*input)(inMsg.Consume(unsafe.Sizeof(input{}))) if in == nil { err = errors.New("Corrupt OpForget") return } o = &fuseops.ForgetInodeOp{ - Inode: fuseops.InodeID(m.Header().Nodeid), + Inode: fuseops.InodeID(inMsg.Header().Nodeid), N: in.Nlookup, } case fusekernel.OpMkdir: - in := (*fusekernel.MkdirIn)(m.Consume(fusekernel.MkdirInSize(protocol))) + in := (*fusekernel.MkdirIn)(inMsg.Consume(fusekernel.MkdirInSize(protocol))) if in == nil { err = errors.New("Corrupt OpMkdir") return } - name := m.ConsumeBytes(m.Len()) + name := inMsg.ConsumeBytes(inMsg.Len()) i := bytes.IndexByte(name, '\x00') if i < 0 { err = errors.New("Corrupt OpMkdir") @@ -120,7 +122,7 @@ func convertInMessage( name = name[:i] o = &fuseops.MkDirOp{ - Parent: fuseops.InodeID(m.Header().Nodeid), + Parent: fuseops.InodeID(inMsg.Header().Nodeid), Name: string(name), // On Linux, vfs_mkdir calls through to the inode with at most @@ -133,13 +135,13 @@ func convertInMessage( } case fusekernel.OpCreate: - in := (*fusekernel.CreateIn)(m.Consume(fusekernel.CreateInSize(protocol))) + in := (*fusekernel.CreateIn)(inMsg.Consume(fusekernel.CreateInSize(protocol))) if in == nil { err = errors.New("Corrupt OpCreate") return } - name := m.ConsumeBytes(m.Len()) + name := inMsg.ConsumeBytes(inMsg.Len()) i := bytes.IndexByte(name, '\x00') if i < 0 { err = errors.New("Corrupt OpCreate") @@ -148,14 +150,14 @@ func convertInMessage( name = name[:i] o = &fuseops.CreateFileOp{ - Parent: fuseops.InodeID(m.Header().Nodeid), + Parent: fuseops.InodeID(inMsg.Header().Nodeid), Name: string(name), Mode: convertFileMode(in.Mode), } case fusekernel.OpSymlink: // The message is "newName\0target\0". - names := m.ConsumeBytes(m.Len()) + names := inMsg.ConsumeBytes(inMsg.Len()) if len(names) == 0 || names[len(names)-1] != 0 { err = errors.New("Corrupt OpSymlink") return @@ -168,20 +170,20 @@ func convertInMessage( newName, target := names[0:i], names[i+1:len(names)-1] o = &fuseops.CreateSymlinkOp{ - Parent: fuseops.InodeID(m.Header().Nodeid), + Parent: fuseops.InodeID(inMsg.Header().Nodeid), Name: string(newName), Target: string(target), } case fusekernel.OpRename: type input fusekernel.RenameIn - in := (*input)(m.Consume(unsafe.Sizeof(input{}))) + in := (*input)(inMsg.Consume(unsafe.Sizeof(input{}))) if in == nil { err = errors.New("Corrupt OpRename") return } - names := m.ConsumeBytes(m.Len()) + names := inMsg.ConsumeBytes(inMsg.Len()) // names should be "old\x00new\x00" if len(names) < 4 { err = errors.New("Corrupt OpRename") @@ -199,14 +201,14 @@ func convertInMessage( oldName, newName := names[:i], names[i+1:len(names)-1] o = &fuseops.RenameOp{ - OldParent: fuseops.InodeID(m.Header().Nodeid), + OldParent: fuseops.InodeID(inMsg.Header().Nodeid), OldName: string(oldName), NewParent: fuseops.InodeID(in.Newdir), NewName: string(newName), } case fusekernel.OpUnlink: - buf := m.ConsumeBytes(m.Len()) + buf := inMsg.ConsumeBytes(inMsg.Len()) n := len(buf) if n == 0 || buf[n-1] != '\x00' { err = errors.New("Corrupt OpUnlink") @@ -214,12 +216,12 @@ func convertInMessage( } o = &fuseops.UnlinkOp{ - Parent: fuseops.InodeID(m.Header().Nodeid), + Parent: fuseops.InodeID(inMsg.Header().Nodeid), Name: string(buf[:n-1]), } case fusekernel.OpRmdir: - buf := m.ConsumeBytes(m.Len()) + buf := inMsg.ConsumeBytes(inMsg.Len()) n := len(buf) if n == 0 || buf[n-1] != '\x00' { err = errors.New("Corrupt OpRmdir") @@ -227,43 +229,56 @@ func convertInMessage( } o = &fuseops.RmDirOp{ - Parent: fuseops.InodeID(m.Header().Nodeid), + Parent: fuseops.InodeID(inMsg.Header().Nodeid), Name: string(buf[:n-1]), } case fusekernel.OpOpen: o = &fuseops.OpenFileOp{ - Inode: fuseops.InodeID(m.Header().Nodeid), + Inode: fuseops.InodeID(inMsg.Header().Nodeid), } case fusekernel.OpOpendir: o = &fuseops.OpenDirOp{ - Inode: fuseops.InodeID(m.Header().Nodeid), + Inode: fuseops.InodeID(inMsg.Header().Nodeid), } case fusekernel.OpRead: - in := (*fusekernel.ReadIn)(m.Consume(fusekernel.ReadInSize(protocol))) + in := (*fusekernel.ReadIn)(inMsg.Consume(fusekernel.ReadInSize(protocol))) if in == nil { err = errors.New("Corrupt OpRead") return } - o = &fuseops.ReadFileOp{ - Inode: fuseops.InodeID(m.Header().Nodeid), + to := &fuseops.ReadFileOp{ + Inode: fuseops.InodeID(inMsg.Header().Nodeid), Handle: fuseops.HandleID(in.Fh), Offset: int64(in.Offset), Dst: make([]byte, in.Size), } + o = to + + readSize := int(in.Size) + p := outMsg.GrowNoZero(uintptr(readSize)) + if p == nil { + err = fmt.Errorf("Can't grow for %d-byte read", readSize) + return + } + + sh := (*reflect.SliceHeader)(unsafe.Pointer(&to.Dst)) + sh.Data = uintptr(p) + sh.Len = readSize + sh.Cap = readSize case fusekernel.OpReaddir: - in := (*fusekernel.ReadIn)(m.Consume(fusekernel.ReadInSize(protocol))) + in := (*fusekernel.ReadIn)(inMsg.Consume(fusekernel.ReadInSize(protocol))) if in == nil { err = errors.New("Corrupt OpReaddir") return } o = &fuseops.ReadDirOp{ - Inode: fuseops.InodeID(m.Header().Nodeid), + Inode: fuseops.InodeID(inMsg.Header().Nodeid), Handle: fuseops.HandleID(in.Fh), Offset: fuseops.DirOffset(in.Offset), Dst: make([]byte, in.Size), @@ -271,7 +286,7 @@ func convertInMessage( case fusekernel.OpRelease: type input fusekernel.ReleaseIn - in := (*input)(m.Consume(unsafe.Sizeof(input{}))) + in := (*input)(inMsg.Consume(unsafe.Sizeof(input{}))) if in == nil { err = errors.New("Corrupt OpRelease") return @@ -283,7 +298,7 @@ func convertInMessage( case fusekernel.OpReleasedir: type input fusekernel.ReleaseIn - in := (*input)(m.Consume(unsafe.Sizeof(input{}))) + in := (*input)(inMsg.Consume(unsafe.Sizeof(input{}))) if in == nil { err = errors.New("Corrupt OpReleasedir") return @@ -294,20 +309,20 @@ func convertInMessage( } case fusekernel.OpWrite: - in := (*fusekernel.WriteIn)(m.Consume(fusekernel.WriteInSize(protocol))) + in := (*fusekernel.WriteIn)(inMsg.Consume(fusekernel.WriteInSize(protocol))) if in == nil { err = errors.New("Corrupt OpWrite") return } - buf := m.ConsumeBytes(m.Len()) + buf := inMsg.ConsumeBytes(inMsg.Len()) if len(buf) < int(in.Size) { err = errors.New("Corrupt OpWrite") return } o = &fuseops.WriteFileOp{ - Inode: fuseops.InodeID(m.Header().Nodeid), + Inode: fuseops.InodeID(inMsg.Header().Nodeid), Handle: fuseops.HandleID(in.Fh), Data: buf, Offset: int64(in.Offset), @@ -315,33 +330,33 @@ func convertInMessage( case fusekernel.OpFsync: type input fusekernel.FsyncIn - in := (*input)(m.Consume(unsafe.Sizeof(input{}))) + in := (*input)(inMsg.Consume(unsafe.Sizeof(input{}))) if in == nil { err = errors.New("Corrupt OpFsync") return } o = &fuseops.SyncFileOp{ - Inode: fuseops.InodeID(m.Header().Nodeid), + Inode: fuseops.InodeID(inMsg.Header().Nodeid), Handle: fuseops.HandleID(in.Fh), } case fusekernel.OpFlush: type input fusekernel.FlushIn - in := (*input)(m.Consume(unsafe.Sizeof(input{}))) + in := (*input)(inMsg.Consume(unsafe.Sizeof(input{}))) if in == nil { err = errors.New("Corrupt OpFlush") return } o = &fuseops.FlushFileOp{ - Inode: fuseops.InodeID(m.Header().Nodeid), + Inode: fuseops.InodeID(inMsg.Header().Nodeid), Handle: fuseops.HandleID(in.Fh), } case fusekernel.OpReadlink: o = &fuseops.ReadSymlinkOp{ - Inode: fuseops.InodeID(m.Header().Nodeid), + Inode: fuseops.InodeID(inMsg.Header().Nodeid), } case fusekernel.OpStatfs: @@ -349,7 +364,7 @@ func convertInMessage( case fusekernel.OpInterrupt: type input fusekernel.InterruptIn - in := (*input)(m.Consume(unsafe.Sizeof(input{}))) + in := (*input)(inMsg.Consume(unsafe.Sizeof(input{}))) if in == nil { err = errors.New("Corrupt OpInterrupt") return @@ -361,7 +376,7 @@ func convertInMessage( case fusekernel.OpInit: type input fusekernel.InitIn - in := (*input)(m.Consume(unsafe.Sizeof(input{}))) + in := (*input)(inMsg.Consume(unsafe.Sizeof(input{}))) if in == nil { err = errors.New("Corrupt OpInit") return @@ -375,8 +390,8 @@ func convertInMessage( default: o = &unknownOp{ - opCode: m.Header().Opcode, - inode: fuseops.InodeID(m.Header().Nodeid), + opCode: inMsg.Header().Opcode, + inode: fuseops.InodeID(inMsg.Header().Nodeid), } } @@ -484,7 +499,10 @@ func (c *Connection) kernelResponseForOp( out.Fh = uint64(o.Handle) case *fuseops.ReadFileOp: - m.Append(o.Dst[:o.BytesRead]) + // convertInMessage already set up the destination buffer to be at the end + // of the out message. We need only shrink to the right size based on how + // much the user read. + m.Shrink(uintptr(m.Len() - (int(buffer.OutMessageInitialSize) + o.BytesRead))) case *fuseops.WriteFileOp: out := (*fusekernel.WriteOut)(m.Grow(unsafe.Sizeof(fusekernel.WriteOut{}))) diff --git a/internal/buffer/out_message.go b/internal/buffer/out_message.go index 59b5d42..aedc1e7 100644 --- a/internal/buffer/out_message.go +++ b/internal/buffer/out_message.go @@ -25,6 +25,9 @@ import ( const outHeaderSize = unsafe.Sizeof(fusekernel.OutHeader{}) +// OutMessage structs begin life with Len() == OutMessageInitialSize. +const OutMessageInitialSize = outHeaderSize + // We size out messages to be large enough to hold a header for the response // plus the largest read that may come in. const outMessageSize = outHeaderSize + MaxReadSize @@ -53,8 +56,8 @@ func init() { // Reset the message so that it is ready to be used again. Afterward, the // contents are solely a zeroed header. func (m *OutMessage) Reset() { - m.offset = outHeaderSize - memclr(unsafe.Pointer(&m.storage), outHeaderSize) + m.offset = OutMessageInitialSize + memclr(unsafe.Pointer(&m.storage), OutMessageInitialSize) } // Return a pointer to the header at the start of the message. @@ -87,6 +90,15 @@ func (b *OutMessage) GrowNoZero(size uintptr) (p unsafe.Pointer) { return } +// Throw away the last n bytes. Panics if n is out of range. +func (b *OutMessage) Shrink(n uintptr) { + if n > b.offset-OutMessageInitialSize { + panic(fmt.Sprintf("Shrink(%d) out of range for offset %d", n, b.offset)) + } + + b.offset -= n +} + // Equivalent to growing by the length of p, then copying p over the new // segment. Panics if there is not enough room available. func (b *OutMessage) Append(src []byte) { From ea97708e20bc1d1f6293cb46e8d6d244ef6dc274 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 29 Jul 2015 11:03:27 +1000 Subject: [PATCH 12/13] Read directly into out messages for directories. --- conversions.go | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/conversions.go b/conversions.go index 3fd23bb..cb67a7a 100644 --- a/conversions.go +++ b/conversions.go @@ -277,12 +277,24 @@ func convertInMessage( return } - o = &fuseops.ReadDirOp{ + to := &fuseops.ReadDirOp{ Inode: fuseops.InodeID(inMsg.Header().Nodeid), Handle: fuseops.HandleID(in.Fh), Offset: fuseops.DirOffset(in.Offset), - Dst: make([]byte, in.Size), } + o = to + + readSize := int(in.Size) + p := outMsg.GrowNoZero(uintptr(readSize)) + if p == nil { + err = fmt.Errorf("Can't grow for %d-byte read", readSize) + return + } + + sh := (*reflect.SliceHeader)(unsafe.Pointer(&to.Dst)) + sh.Data = uintptr(p) + sh.Len = readSize + sh.Cap = readSize case fusekernel.OpRelease: type input fusekernel.ReleaseIn @@ -489,7 +501,10 @@ func (c *Connection) kernelResponseForOp( out.Fh = uint64(o.Handle) case *fuseops.ReadDirOp: - m.Append(o.Dst[:o.BytesRead]) + // convertInMessage already set up the destination buffer to be at the end + // of the out message. We need only shrink to the right size based on how + // much the user read. + m.Shrink(uintptr(m.Len() - (int(buffer.OutMessageInitialSize) + o.BytesRead))) case *fuseops.ReleaseDirHandleOp: // Empty response From ab0580a452e726db7a480cdc7131f5d858774372 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 29 Jul 2015 11:14:17 +1000 Subject: [PATCH 13/13] Fixed a very dumb bug. --- conversions.go | 1 - 1 file changed, 1 deletion(-) diff --git a/conversions.go b/conversions.go index cb67a7a..6ba2f18 100644 --- a/conversions.go +++ b/conversions.go @@ -254,7 +254,6 @@ func convertInMessage( Inode: fuseops.InodeID(inMsg.Header().Nodeid), Handle: fuseops.HandleID(in.Fh), Offset: int64(in.Offset), - Dst: make([]byte, in.Size), } o = to