From 32418239aedb3367984845facdd6721c22fff18b Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Fri, 24 Jul 2015 15:26:07 +1000 Subject: [PATCH 1/4] Removed read locks around the device. We require no conrrent calls to ReadOp, and that ServeOps doesn't return until all ops have been responded to, so I believe this should be safe. In particular, fuseshim uses the locks only to exclude reads and writes during closing, not for anything else. --- connection.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/connection.go b/connection.go index 2d5d9ee..776de3e 100644 --- a/connection.go +++ b/connection.go @@ -224,13 +224,8 @@ func (c *Connection) readMessage() (m *buffer.InMessage, err error) { // Loop past transient errors. for { - // Lock and read. - // - // TODO(jacobsa): Ensure that we document concurrency constraints that make - // it safe, then kill the lock here. - c.wrapped.Rio.RLock() + // Attempt a reaed. err = m.Init(c.wrapped.Dev) - c.wrapped.Rio.RUnlock() // Special cases: // From ee2b961839c49f61c0d565653a29c916713b70af Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Fri, 24 Jul 2015 15:31:16 +1000 Subject: [PATCH 2/4] Don't depend on fuseshim.Conn for sending messages. --- connection.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/connection.go b/connection.go index 776de3e..7380d78 100644 --- a/connection.go +++ b/connection.go @@ -255,6 +255,22 @@ func (c *Connection) readMessage() (m *buffer.InMessage, err error) { } } +// Write the supplied message to the kernel. +func (c *Connection) writeMessage(msg []byte) (err error) { + // Avoid the retry loop in os.File.Write. + n, err := syscall.Write(int(c.wrapped.Dev.Fd()), msg) + if err != nil { + return + } + + if n != len(msg) { + err = fmt.Errorf("Wrote %d bytes; expected %d", n, len(msg)) + return + } + + return +} + // Read the next op from the kernel process. Return io.EOF if the kernel has // closed the connection. // @@ -312,9 +328,9 @@ func (c *Connection) ReadOp() (op fuseops.Op, err error) { } // Send the reply to the kernel. - err = c.wrapped.WriteToKernel(replyMsg) + err = c.writeMessage(replyMsg) if err != nil { - err = fmt.Errorf("WriteToKernel: %v", err) + err = fmt.Errorf("writeMessage: %v", err) return } From cd47dbb4b8069385e038805bc187f998d8ba3fd9 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Fri, 24 Jul 2015 15:32:50 +1000 Subject: [PATCH 3/4] Refactored fields a bit. --- connection.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/connection.go b/connection.go index 7380d78..964c48d 100644 --- a/connection.go +++ b/connection.go @@ -38,6 +38,11 @@ type Connection struct { errorLogger *log.Logger wrapped *fuseshim.Conn + // The device through which we're talking to the kernel, and the protocol + // version that we're using to talk to it. + dev *os.File + protocol fusekernel.Protocol + // The context from which all op contexts inherit. parentCtx context.Context @@ -66,6 +71,8 @@ func newConnection( debugLogger: debugLogger, errorLogger: errorLogger, wrapped: wrapped, + dev: wrapped.Dev, + protocol: wrapped.Protocol(), parentCtx: parentCtx, cancelFuncs: make(map[uint64]func()), } @@ -225,7 +232,7 @@ func (c *Connection) readMessage() (m *buffer.InMessage, err error) { // Loop past transient errors. for { // Attempt a reaed. - err = m.Init(c.wrapped.Dev) + err = m.Init(c.dev) // Special cases: // @@ -258,7 +265,7 @@ func (c *Connection) readMessage() (m *buffer.InMessage, err error) { // Write the supplied message to the kernel. func (c *Connection) writeMessage(msg []byte) (err error) { // Avoid the retry loop in os.File.Write. - n, err := syscall.Write(int(c.wrapped.Dev.Fd()), msg) + n, err := syscall.Write(int(c.dev.Fd()), msg) if err != nil { return } @@ -341,7 +348,7 @@ func (c *Connection) ReadOp() (op fuseops.Op, err error) { op, err = fuseops.Convert( opCtx, m, - c.wrapped.Protocol(), + c.protocol, debugLogForOp, c.errorLogger, sendReply) From a721a505bf747b3b6505a5f135c04d39197fba86 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Fri, 24 Jul 2015 15:35:31 +1000 Subject: [PATCH 4/4] Document the reason for a restriction. --- connection.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/connection.go b/connection.go index 964c48d..4e83b8b 100644 --- a/connection.go +++ b/connection.go @@ -388,6 +388,9 @@ func (c *Connection) waitForReady() (err error) { // Close the connection. Must not be called until operations that were read // from the connection have been responded to. func (c *Connection) close() (err error) { - err = c.wrapped.Close() + // Posix doesn't say that close can be called concurrently with read or + // write, but luckily we exclude the possibility of a race by requiring the + // user to respond to all ops first. + err = c.dev.Close() return }