Speed up O_APPEND with remote revalidating

Remote revalidating requires to update file size on every write
on a file opened with O_APPEND. If host FD exists, it can be
used to update the size and skip round trip to the gofer. With
this change, O_APPEND writes with remote revalidating is almost
as fast as exclusive mode:

              BM_Append
VFS1           60.7us
VFS2           56.8us
VFS2 exclusive 14.2us
This change    15.8us

Updates #1792

PiperOrigin-RevId: 369486801
This commit is contained in:
Fabricio Voznika
2021-04-20 11:48:48 -07:00
committed by gVisor bot
parent 3fff4c4a0f
commit bf1e14cf8a
3 changed files with 59 additions and 19 deletions
+42 -15
View File
@@ -986,37 +986,64 @@ func (d *dentry) updateFromP9AttrsLocked(mask p9.AttrMask, attr *p9.Attr) {
}
}
// Preconditions: !d.isSynthetic().
// Preconditions: d.metadataMu is locked.
func (d *dentry) refreshSizeLocked(ctx context.Context) error {
d.handleMu.RLock()
if d.writeFD < 0 {
d.handleMu.RUnlock()
// Ask the gofer if we don't have a host FD.
return d.updateFromGetattrLocked(ctx)
}
var stat unix.Statx_t
err := unix.Statx(int(d.writeFD), "", unix.AT_EMPTY_PATH, unix.STATX_SIZE, &stat)
d.handleMu.RUnlock() // must be released before updateSizeLocked()
if err != nil {
return err
}
d.updateSizeLocked(stat.Size)
return nil
}
// Preconditions: !d.isSynthetic().
func (d *dentry) updateFromGetattr(ctx context.Context) error {
// Use d.readFile or d.writeFile, which represent 9P fids that have been
// d.metadataMu must be locked *before* we getAttr so that we do not end up
// updating stale attributes in d.updateFromP9AttrsLocked().
d.metadataMu.Lock()
defer d.metadataMu.Unlock()
return d.updateFromGetattrLocked(ctx)
}
// Preconditions:
// * !d.isSynthetic().
// * d.metadataMu is locked.
func (d *dentry) updateFromGetattrLocked(ctx context.Context) error {
// Use d.readFile or d.writeFile, which represent 9P FIDs that have been
// opened, in preference to d.file, which represents a 9P fid that has not.
// This may be significantly more efficient in some implementations. Prefer
// d.writeFile over d.readFile since some filesystem implementations may
// update a writable handle's metadata after writes to that handle, without
// making metadata updates immediately visible to read-only handles
// representing the same file.
var (
file p9file
handleMuRLocked bool
)
// d.metadataMu must be locked *before* we getAttr so that we do not end up
// updating stale attributes in d.updateFromP9AttrsLocked().
d.metadataMu.Lock()
defer d.metadataMu.Unlock()
d.handleMu.RLock()
if !d.writeFile.isNil() {
handleMuRLocked := true
var file p9file
switch {
case !d.writeFile.isNil():
file = d.writeFile
handleMuRLocked = true
} else if !d.readFile.isNil() {
case !d.readFile.isNil():
file = d.readFile
handleMuRLocked = true
} else {
default:
file = d.file
d.handleMu.RUnlock()
handleMuRLocked = false
}
_, attrMask, attr, err := file.getAttr(ctx, dentryAttrMask())
if handleMuRLocked {
d.handleMu.RUnlock()
d.handleMu.RUnlock() // must be released before updateFromP9AttrsLocked()
}
if err != nil {
return err
+5 -4
View File
@@ -204,18 +204,19 @@ func (fd *regularFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off
}
d := fd.dentry()
d.metadataMu.Lock()
defer d.metadataMu.Unlock()
// If the fd was opened with O_APPEND, make sure the file size is updated.
// There is a possible race here if size is modified externally after
// metadata cache is updated.
if fd.vfsfd.StatusFlags()&linux.O_APPEND != 0 && !d.cachedMetadataAuthoritative() {
if err := d.updateFromGetattr(ctx); err != nil {
if err := d.refreshSizeLocked(ctx); err != nil {
return 0, offset, err
}
}
d.metadataMu.Lock()
defer d.metadataMu.Unlock()
// Set offset to file size if the fd was opened with O_APPEND.
if fd.vfsfd.StatusFlags()&linux.O_APPEND != 0 {
// Holding d.metadataMu is sufficient for reading d.size.
+12
View File
@@ -46,6 +46,18 @@ void BM_Write(benchmark::State& state) {
BENCHMARK(BM_Write)->Range(1, 1 << 26)->UseRealTime();
void BM_Append(benchmark::State& state) {
auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
auto fd = ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_WRONLY | O_APPEND));
const char data = 'a';
for (auto _ : state) {
TEST_CHECK(WriteFd(fd.get(), &data, 1) == 1);
}
}
BENCHMARK(BM_Append);
} // namespace
} // namespace testing