netbird/gvisor
0
0
Fork 0
mirror of https://github.com/netbirdio/gvisor.git synced 2026-05-22 17:12:49 -07:00
Code Issues Packages Projects Releases Wiki Activity

Get rid of FSync RPC batching in gofer client with lisafs.

Browse Source 
This optimization was not used correctly, as described in the change. We could
use it correctly to optimize sync(2) and syncfs(2). But that substantially
increases code complexity.

PiperOrigin-RevId: 433080982
This commit is contained in:
Ayush Ranjan
2022-03-07 17:27:46 -08:00
committed by gVisor bot
parent 261f9c7e6c
commit 3d52015286
4 changed files with 17 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/sentry/fsimpl/gofer/filesystem.go
+10 -16
View File
@@ -56,15 +56,16 @@ func (fs *filesystem) Sync(ctx context.Context) error {
var retErr error
if fs.opts.lisaEnabled {
// Try accumulating all FDIDs to fsync and fsync then via one RPC as
// opposed to making an RPC per FDID. Passing a non-nil accFsyncFDIDs to
// dentry.syncCachedFile() and specialFileFD.sync() will cause them to not
// make an RPC, instead accumulate syncable FDIDs in the passed slice.
accFsyncFDIDs := make([]lisafs.FDID, 0, len(ds)+len(sffds))
// Note that lisafs is capable of batching FSync RPCs. However, we can not
// batch all the FDIDs to be synced from ds and sffds. Because the error
// handling varies based on file type. FSync errors are only considered for
// regular file FDIDs that were opened for writing. We could do individual
// RPCs for such FDIDs and batch the rest, but it increases code complexity
// substantially. We could implement it in the future if need be.
// Sync syncable dentries.
for _, d := range ds {
if err := d.syncCachedFile(ctx, true /* forFilesystemSync */, &accFsyncFDIDs); err != nil {
if err := d.syncCachedFile(ctx, true /* forFilesystemSync */); err != nil {
ctx.Infof("gofer.filesystem.Sync: dentry.syncCachedFile failed: %v", err)
if retErr == nil {
retErr = err
@@ -75,7 +76,7 @@ func (fs *filesystem) Sync(ctx context.Context) error {
// Sync special files, which may be writable but do not use dentry shared
// handles (so they won't be synced by the above).
for _, sffd := range sffds {
if err := sffd.sync(ctx, true /* forFilesystemSync */, &accFsyncFDIDs); err != nil {
if err := sffd.sync(ctx, true /* forFilesystemSync */); err != nil {
ctx.Infof("gofer.filesystem.Sync: specialFileFD.sync failed: %v", err)
if retErr == nil {
retErr = err
@@ -83,19 +84,12 @@ func (fs *filesystem) Sync(ctx context.Context) error {
}
}
if err := fs.clientLisa.SyncFDs(ctx, accFsyncFDIDs); err != nil {
ctx.Infof("gofer.filesystem.Sync: fs.fsyncMultipleFDLisa failed: %v", err)
if retErr == nil {
retErr = err
}
}
return retErr
}
// Sync syncable dentries.
for _, d := range ds {
if err := d.syncCachedFile(ctx, true /* forFilesystemSync */, nil /* accFsyncFDIDsLisa */); err != nil {
if err := d.syncCachedFile(ctx, true /* forFilesystemSync */); err != nil {
ctx.Infof("gofer.filesystem.Sync: dentry.syncCachedFile failed: %v", err)
if retErr == nil {
retErr = err
@@ -106,7 +100,7 @@ func (fs *filesystem) Sync(ctx context.Context) error {
// Sync special files, which may be writable but do not use dentry shared
// handles (so they won't be synced by the above).
for _, sffd := range sffds {
if err := sffd.sync(ctx, true /* forFilesystemSync */, nil /* accFsyncFDIDsLisa */); err != nil {
if err := sffd.sync(ctx, true /* forFilesystemSync */); err != nil {
ctx.Infof("gofer.filesystem.Sync: specialFileFD.sync failed: %v", err)
if retErr == nil {
retErr = err
pkg/sentry/fsimpl/gofer/gofer.go
+4 -12
View File
@@ -2384,11 +2384,11 @@ func (d *dentry) writeHandleLocked() handle {
func (d *dentry) syncRemoteFile(ctx context.Context) error {
d.handleMu.RLock()
defer d.handleMu.RUnlock()
return d.syncRemoteFileLocked(ctx, nil /* accFsyncFDIDsLisa */)
return d.syncRemoteFileLocked(ctx)
}
// Preconditions: d.handleMu must be locked.
func (d *dentry) syncRemoteFileLocked(ctx context.Context, accFsyncFDIDsLisa *[]lisafs.FDID) error {
func (d *dentry) syncRemoteFileLocked(ctx context.Context) error {
// If we have a host FD, fsyncing it is likely to be faster than an fsync
// RPC. Prefer syncing write handles over read handles, since some remote
// filesystem implementations may not sync changes made through write
@@ -2400,10 +2400,6 @@ func (d *dentry) syncRemoteFileLocked(ctx context.Context, accFsyncFDIDsLisa *[]
return err
}
if d.fs.opts.lisaEnabled && d.writeFDLisa.Ok() {
if accFsyncFDIDsLisa != nil {
*accFsyncFDIDsLisa = append(*accFsyncFDIDsLisa, d.writeFDLisa.ID())
return nil
}
return d.writeFDLisa.Sync(ctx)
} else if !d.fs.opts.lisaEnabled && !d.writeFile.isNil() {
return d.writeFile.fsync(ctx)
@@ -2415,10 +2411,6 @@ func (d *dentry) syncRemoteFileLocked(ctx context.Context, accFsyncFDIDsLisa *[]
return err
}
if d.fs.opts.lisaEnabled && d.readFDLisa.Ok() {
if accFsyncFDIDsLisa != nil {
*accFsyncFDIDsLisa = append(*accFsyncFDIDsLisa, d.readFDLisa.ID())
return nil
}
return d.readFDLisa.Sync(ctx)
} else if !d.fs.opts.lisaEnabled && !d.readFile.isNil() {
return d.readFile.fsync(ctx)
@@ -2426,7 +2418,7 @@ func (d *dentry) syncRemoteFileLocked(ctx context.Context, accFsyncFDIDsLisa *[]
return nil
}
func (d *dentry) syncCachedFile(ctx context.Context, forFilesystemSync bool, accFsyncFDIDsLisa *[]lisafs.FDID) error {
func (d *dentry) syncCachedFile(ctx context.Context, forFilesystemSync bool) error {
d.handleMu.RLock()
defer d.handleMu.RUnlock()
h := d.writeHandleLocked()
@@ -2439,7 +2431,7 @@ func (d *dentry) syncCachedFile(ctx context.Context, forFilesystemSync bool, acc
return err
}
}
if err := d.syncRemoteFileLocked(ctx, accFsyncFDIDsLisa); err != nil {
if err := d.syncRemoteFileLocked(ctx); err != nil {
if !forFilesystemSync {
return err
}
pkg/sentry/fsimpl/gofer/regular_file.go
+1 -1
View File
@@ -697,7 +697,7 @@ func regularFileSeekLocked(ctx context.Context, d *dentry, fdOffset, offset int6
// Sync implements vfs.FileDescriptionImpl.Sync.
func (fd *regularFileFD) Sync(ctx context.Context) error {
return fd.dentry().syncCachedFile(ctx, false /* forFilesystemSync */, nil /* accFsyncFDIDsLisa */)
return fd.dentry().syncCachedFile(ctx, false /* forFilesystemSync */)
}
// ConfigureMMap implements vfs.FileDescriptionImpl.ConfigureMMap.
pkg/sentry/fsimpl/gofer/special_file.go
+2 -7
View File
@@ -24,7 +24,6 @@ import (
"gvisor.dev/gvisor/pkg/errors/linuxerr"
"gvisor.dev/gvisor/pkg/fdnotifier"
"gvisor.dev/gvisor/pkg/hostarch"
"gvisor.dev/gvisor/pkg/lisafs"
"gvisor.dev/gvisor/pkg/metric"
"gvisor.dev/gvisor/pkg/p9"
"gvisor.dev/gvisor/pkg/safemem"
@@ -392,10 +391,10 @@ func (fd *specialFileFD) Seek(ctx context.Context, offset int64, whence int32) (
// Sync implements vfs.FileDescriptionImpl.Sync.
func (fd *specialFileFD) Sync(ctx context.Context) error {
return fd.sync(ctx, false /* forFilesystemSync */, nil /* accFsyncFDIDsLisa */)
return fd.sync(ctx, false /* forFilesystemSync */)
}
func (fd *specialFileFD) sync(ctx context.Context, forFilesystemSync bool, accFsyncFDIDsLisa *[]lisafs.FDID) error {
func (fd *specialFileFD) sync(ctx context.Context, forFilesystemSync bool) error {
// Locks to ensure it didn't race with fd.Release().
fd.releaseMu.RLock()
defer fd.releaseMu.RUnlock()
@@ -413,10 +412,6 @@ func (fd *specialFileFD) sync(ctx context.Context, forFilesystemSync bool, accFs
return err
}
if fs := fd.filesystem(); fs.opts.lisaEnabled {
if accFsyncFDIDsLisa != nil {
*accFsyncFDIDsLisa = append(*accFsyncFDIDsLisa, fd.handle.fdLisa.ID())
return nil
}
return fd.handle.fdLisa.Sync(ctx)
}
return fd.handle.file.fsync(ctx)