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

Implement setattr+clunk in 9P

Browse Source 
This is to cover the common pattern: open->read/write->close,
where SetAttr needs to be called to update atime/mtime before
the file is closed.

Benchmark results:

BM_OpenReadClose/10240 CPU
setattr+clunk: 63783 ns
VFS2:          68109 ns
VFS1:          72507 ns

Updates #1198

PiperOrigin-RevId: 329628461
This commit is contained in:
Fabricio Voznika
2020-09-01 19:22:12 -07:00
committed by gVisor bot
parent 40faeaa180
commit 37a217aca4
13 changed files with 389 additions and 107 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/p9/client_file.go
+35 -3
View File
@@ -54,6 +54,8 @@ func (c *Client) newFile(fid FID) *clientFile {
//
// This proxies all of the interfaces found in file.go.
type clientFile struct {
DisallowServerCalls
// client is the originating client.
client *Client
@@ -283,6 +285,39 @@ func (c *clientFile) Close() error {
return nil
}
// SetAttrClose implements File.SetAttrClose.
func (c *clientFile) SetAttrClose(valid SetAttrMask, attr SetAttr) error {
if !versionSupportsTsetattrclunk(c.client.version) {
setAttrErr := c.SetAttr(valid, attr)
// Try to close file even in case of failure above. Since the state of the
// file is unknown to the caller, it will not attempt to close the file
// again.
if err := c.Close(); err != nil {
return err
}
return setAttrErr
}
// Avoid double close.
if !atomic.CompareAndSwapUint32(&c.closed, 0, 1) {
return syscall.EBADF
}
// Send the message.
if err := c.client.sendRecv(&Tsetattrclunk{FID: c.fid, Valid: valid, SetAttr: attr}, &Rsetattrclunk{}); err != nil {
// If an error occurred, we toss away the FID. This isn't ideal,
// but I'm not sure what else makes sense in this context.
log.Warningf("Tsetattrclunk failed, losing FID %v: %v", c.fid, err)
return err
}
// Return the FID to the pool.
c.client.fidPool.Put(uint64(c.fid))
return nil
}
// Open implements File.Open.
func (c *clientFile) Open(flags OpenFlags) (*fd.FD, QID, uint32, error) {
if atomic.LoadUint32(&c.closed) != 0 {
@@ -681,6 +716,3 @@ func (c *clientFile) Flush() error {
return c.client.sendRecv(&Tflushf{FID: c.fid}, &Rflushf{})
}
// Renamed implements File.Renamed.
func (c *clientFile) Renamed(newDir File, newName string) {}
pkg/p9/file.go
+24
View File
@@ -135,6 +135,14 @@ type File interface {
// On the server, Close has no concurrency guarantee.
Close() error
// SetAttrClose is the equivalent of calling SetAttr() followed by Close().
// This can be used to set file times before closing the file in a single
// operation.
//
// On the server, SetAttr has a write concurrency guarantee.
// On the server, Close has no concurrency guarantee.
SetAttrClose(valid SetAttrMask, attr SetAttr) error
// Open must be called prior to using Read, Write or Readdir. Once Open
// is called, some operations, such as Walk, will no longer work.
//
@@ -286,3 +294,19 @@ type DefaultWalkGetAttr struct{}
func (DefaultWalkGetAttr) WalkGetAttr([]string) ([]QID, File, AttrMask, Attr, error) {
return nil, nil, AttrMask{}, Attr{}, syscall.ENOSYS
}
// DisallowClientCalls panics if a client-only function is called.
type DisallowClientCalls struct{}
// SetAttrClose implements File.SetAttrClose.
func (DisallowClientCalls) SetAttrClose(SetAttrMask, SetAttr) error {
panic("SetAttrClose should not be called on the server")
}
// DisallowServerCalls panics if a server-only function is called.
type DisallowServerCalls struct{}
// Renamed implements File.Renamed.
func (*clientFile) Renamed(File, string) {
panic("Renamed should not be called on the client")
}
pkg/p9/handlers.go
+31
View File
@@ -123,6 +123,37 @@ func (t *Tclunk) handle(cs *connState) message {
return &Rclunk{}
}
func (t *Tsetattrclunk) handle(cs *connState) message {
ref, ok := cs.LookupFID(t.FID)
if !ok {
return newErr(syscall.EBADF)
}
defer ref.DecRef()
setAttrErr := ref.safelyWrite(func() error {
// We don't allow setattr on files that have been deleted.
// This might be technically incorrect, as it's possible that
// there were multiple links and you can still change the
// corresponding inode information.
if ref.isDeleted() {
return syscall.EINVAL
}
// Set the attributes.
return ref.file.SetAttr(t.Valid, t.SetAttr)
})
// Try to delete FID even in case of failure above. Since the state of the
// file is unknown to the caller, it will not attempt to close the file again.
if !cs.DeleteFID(t.FID) {
return newErr(syscall.EBADF)
}
if setAttrErr != nil {
return newErr(setAttrErr)
}
return &Rsetattrclunk{}
}
// handle implements handler.handle.
func (t *Tremove) handle(cs *connState) message {
ref, ok := cs.LookupFID(t.FID)
pkg/p9/messages.go
+60
View File
@@ -317,6 +317,64 @@ func (r *Rclunk) String() string {
return "Rclunk{}"
}
// Tsetattrclunk is a setattr+close request.
type Tsetattrclunk struct {
// FID is the FID to change.
FID FID
// Valid is the set of bits which will be used.
Valid SetAttrMask
// SetAttr is the set request.
SetAttr SetAttr
}
// decode implements encoder.decode.
func (t *Tsetattrclunk) decode(b *buffer) {
t.FID = b.ReadFID()
t.Valid.decode(b)
t.SetAttr.decode(b)
}
// encode implements encoder.encode.
func (t *Tsetattrclunk) encode(b *buffer) {
b.WriteFID(t.FID)
t.Valid.encode(b)
t.SetAttr.encode(b)
}
// Type implements message.Type.
func (*Tsetattrclunk) Type() MsgType {
return MsgTsetattrclunk
}
// String implements fmt.Stringer.
func (t *Tsetattrclunk) String() string {
return fmt.Sprintf("Tsetattrclunk{FID: %d, Valid: %v, SetAttr: %s}", t.FID, t.Valid, t.SetAttr)
}
// Rsetattrclunk is a setattr+close response.
type Rsetattrclunk struct {
}
// decode implements encoder.decode.
func (*Rsetattrclunk) decode(*buffer) {
}
// encode implements encoder.encode.
func (*Rsetattrclunk) encode(*buffer) {
}
// Type implements message.Type.
func (*Rsetattrclunk) Type() MsgType {
return MsgRsetattrclunk
}
// String implements fmt.Stringer.
func (r *Rsetattrclunk) String() string {
return "Rsetattrclunk{}"
}
// Tremove is a remove request.
//
// This will eventually be replaced by Tunlinkat.
@@ -2657,6 +2715,8 @@ func init() {
msgRegistry.register(MsgRlconnect, func() message { return &Rlconnect{} })
msgRegistry.register(MsgTallocate, func() message { return &Tallocate{} })
msgRegistry.register(MsgRallocate, func() message { return &Rallocate{} })
msgRegistry.register(MsgTsetattrclunk, func() message { return &Tsetattrclunk{} })
msgRegistry.register(MsgRsetattrclunk, func() message { return &Rsetattrclunk{} })
msgRegistry.register(MsgTchannel, func() message { return &Tchannel{} })
msgRegistry.register(MsgRchannel, func() message { return &Rchannel{} })
}
pkg/p9/messages_test.go
+24
View File
@@ -376,6 +376,30 @@ func TestEncodeDecode(t *testing.T) {
&Rumknod{
Rmknod{QID: QID{Type: 1}},
},
&Tsetattrclunk{
FID: 1,
Valid: SetAttrMask{
Permissions: true,
UID: true,
GID: true,
Size: true,
ATime: true,
MTime: true,
CTime: true,
ATimeNotSystemTime: true,
MTimeNotSystemTime: true,
},
SetAttr: SetAttr{
Permissions: 1,
UID: 2,
GID: 3,
Size: 4,
ATimeSeconds: 5,
ATimeNanoSeconds: 6,
MTimeSeconds: 7,
MTimeNanoSeconds: 8,
},
},
}
for _, enc := range objs {
pkg/p9/p9.go
+82 -80
View File
@@ -315,86 +315,88 @@ type MsgType uint8
// MsgType declarations.
const (
MsgTlerror MsgType = 6
MsgRlerror = 7
MsgTstatfs = 8
MsgRstatfs = 9
MsgTlopen = 12
MsgRlopen = 13
MsgTlcreate = 14
MsgRlcreate = 15
MsgTsymlink = 16
MsgRsymlink = 17
MsgTmknod = 18
MsgRmknod = 19
MsgTrename = 20
MsgRrename = 21
MsgTreadlink = 22
MsgRreadlink = 23
MsgTgetattr = 24
MsgRgetattr = 25
MsgTsetattr = 26
MsgRsetattr = 27
MsgTlistxattr = 28
MsgRlistxattr = 29
MsgTxattrwalk = 30
MsgRxattrwalk = 31
MsgTxattrcreate = 32
MsgRxattrcreate = 33
MsgTgetxattr = 34
MsgRgetxattr = 35
MsgTsetxattr = 36
MsgRsetxattr = 37
MsgTremovexattr = 38
MsgRremovexattr = 39
MsgTreaddir = 40
MsgRreaddir = 41
MsgTfsync = 50
MsgRfsync = 51
MsgTlink = 70
MsgRlink = 71
MsgTmkdir = 72
MsgRmkdir = 73
MsgTrenameat = 74
MsgRrenameat = 75
MsgTunlinkat = 76
MsgRunlinkat = 77
MsgTversion = 100
MsgRversion = 101
MsgTauth = 102
MsgRauth = 103
MsgTattach = 104
MsgRattach = 105
MsgTflush = 108
MsgRflush = 109
MsgTwalk = 110
MsgRwalk = 111
MsgTread = 116
MsgRread = 117
MsgTwrite = 118
MsgRwrite = 119
MsgTclunk = 120
MsgRclunk = 121
MsgTremove = 122
MsgRremove = 123
MsgTflushf = 124
MsgRflushf = 125
MsgTwalkgetattr = 126
MsgRwalkgetattr = 127
MsgTucreate = 128
MsgRucreate = 129
MsgTumkdir = 130
MsgRumkdir = 131
MsgTumknod = 132
MsgRumknod = 133
MsgTusymlink = 134
MsgRusymlink = 135
MsgTlconnect = 136
MsgRlconnect = 137
MsgTallocate = 138
MsgRallocate = 139
MsgTchannel = 250
MsgRchannel = 251
MsgTlerror MsgType = 6
MsgRlerror MsgType = 7
MsgTstatfs MsgType = 8
MsgRstatfs MsgType = 9
MsgTlopen MsgType = 12
MsgRlopen MsgType = 13
MsgTlcreate MsgType = 14
MsgRlcreate MsgType = 15
MsgTsymlink MsgType = 16
MsgRsymlink MsgType = 17
MsgTmknod MsgType = 18
MsgRmknod MsgType = 19
MsgTrename MsgType = 20
MsgRrename MsgType = 21
MsgTreadlink MsgType = 22
MsgRreadlink MsgType = 23
MsgTgetattr MsgType = 24
MsgRgetattr MsgType = 25
MsgTsetattr MsgType = 26
MsgRsetattr MsgType = 27
MsgTlistxattr MsgType = 28
MsgRlistxattr MsgType = 29
MsgTxattrwalk MsgType = 30
MsgRxattrwalk MsgType = 31
MsgTxattrcreate MsgType = 32
MsgRxattrcreate MsgType = 33
MsgTgetxattr MsgType = 34
MsgRgetxattr MsgType = 35
MsgTsetxattr MsgType = 36
MsgRsetxattr MsgType = 37
MsgTremovexattr MsgType = 38
MsgRremovexattr MsgType = 39
MsgTreaddir MsgType = 40
MsgRreaddir MsgType = 41
MsgTfsync MsgType = 50
MsgRfsync MsgType = 51
MsgTlink MsgType = 70
MsgRlink MsgType = 71
MsgTmkdir MsgType = 72
MsgRmkdir MsgType = 73
MsgTrenameat MsgType = 74
MsgRrenameat MsgType = 75
MsgTunlinkat MsgType = 76
MsgRunlinkat MsgType = 77
MsgTversion MsgType = 100
MsgRversion MsgType = 101
MsgTauth MsgType = 102
MsgRauth MsgType = 103
MsgTattach MsgType = 104
MsgRattach MsgType = 105
MsgTflush MsgType = 108
MsgRflush MsgType = 109
MsgTwalk MsgType = 110
MsgRwalk MsgType = 111
MsgTread MsgType = 116
MsgRread MsgType = 117
MsgTwrite MsgType = 118
MsgRwrite MsgType = 119
MsgTclunk MsgType = 120
MsgRclunk MsgType = 121
MsgTremove MsgType = 122
MsgRremove MsgType = 123
MsgTflushf MsgType = 124
MsgRflushf MsgType = 125
MsgTwalkgetattr MsgType = 126
MsgRwalkgetattr MsgType = 127
MsgTucreate MsgType = 128
MsgRucreate MsgType = 129
MsgTumkdir MsgType = 130
MsgRumkdir MsgType = 131
MsgTumknod MsgType = 132
MsgRumknod MsgType = 133
MsgTusymlink MsgType = 134
MsgRusymlink MsgType = 135
MsgTlconnect MsgType = 136
MsgRlconnect MsgType = 137
MsgTallocate MsgType = 138
MsgRallocate MsgType = 139
MsgTsetattrclunk MsgType = 140
MsgRsetattrclunk MsgType = 141
MsgTchannel MsgType = 250
MsgRchannel MsgType = 251
)
// QIDType represents the file type for QIDs.
pkg/p9/p9test/client_test.go
+17 -6
View File
@@ -1225,22 +1225,31 @@ func TestOpen(t *testing.T) {
func TestClose(t *testing.T) {
type closeTest struct {
name string
closeFn func(backend *Mock, f p9.File)
closeFn func(backend *Mock, f p9.File) error
}
cases := []closeTest{
{
name: "close",
closeFn: func(_ *Mock, f p9.File) {
f.Close()
closeFn: func(_ *Mock, f p9.File) error {
return f.Close()
},
},
{
name: "remove",
closeFn: func(backend *Mock, f p9.File) {
closeFn: func(backend *Mock, f p9.File) error {
// Allow the rename call in the parent, automatically translated.
backend.parent.EXPECT().UnlinkAt(gomock.Any(), gomock.Any()).Times(1)
f.(deprecatedRemover).Remove()
return f.(deprecatedRemover).Remove()
},
},
{
name: "setAttrClose",
closeFn: func(backend *Mock, f p9.File) error {
valid := p9.SetAttrMask{ATime: true}
attr := p9.SetAttr{ATimeSeconds: 1, ATimeNanoSeconds: 2}
backend.EXPECT().SetAttr(valid, attr).Times(1)
return f.SetAttrClose(valid, attr)
},
},
}
@@ -1258,7 +1267,9 @@ func TestClose(t *testing.T) {
_, backend, f := walkHelper(h, name, root)
// Close via the prescribed method.
tc.closeFn(backend, f)
if err := tc.closeFn(backend, f); err != nil {
t.Fatalf("closeFn failed: %v", err)
}
// Everything should fail with EBADF.
if _, _, err := f.Walk(nil); err != syscall.EBADF {
pkg/p9/version.go
+7 -1
View File
@@ -26,7 +26,7 @@ const (
//
// Clients are expected to start requesting this version number and
// to continuously decrement it until a Tversion request succeeds.
highestSupportedVersion uint32 = 11
highestSupportedVersion uint32 = 12
// lowestSupportedVersion is the lowest supported version X in a
// version string of the format 9P2000.L.Google.X.
@@ -173,3 +173,9 @@ func versionSupportsGetSetXattr(v uint32) bool {
func versionSupportsListRemoveXattr(v uint32) bool {
return v >= 11
}
// versionSupportsTsetattrclunk returns true if version v supports
// the Tsetattrclunk message.
func versionSupportsTsetattrclunk(v uint32) bool {
return v >= 12
}
pkg/sentry/fsimpl/gofer/gofer.go
+23 -17
View File
@@ -1300,30 +1300,36 @@ func (d *dentry) destroyLocked(ctx context.Context) {
d.handleMu.Unlock()
if !d.file.isNil() {
valid := p9.SetAttrMask{}
attr := p9.SetAttr{}
if !d.isDeleted() {
// Write dirty timestamps back to the remote filesystem.
atimeDirty := atomic.LoadUint32(&d.atimeDirty) != 0
mtimeDirty := atomic.LoadUint32(&d.mtimeDirty) != 0
if atimeDirty || mtimeDirty {
if atomic.LoadUint32(&d.atimeDirty) != 0 {
valid.ATime = true
valid.ATimeNotSystemTime = true
atime := atomic.LoadInt64(&d.atime)
attr.ATimeSeconds = uint64(atime / 1e9)
attr.ATimeNanoSeconds = uint64(atime % 1e9)
}
if atomic.LoadUint32(&d.mtimeDirty) != 0 {
valid.MTime = true
valid.MTimeNotSystemTime = true
mtime := atomic.LoadInt64(&d.mtime)
if err := d.file.setAttr(ctx, p9.SetAttrMask{
ATime: atimeDirty,
ATimeNotSystemTime: atimeDirty,
MTime: mtimeDirty,
MTimeNotSystemTime: mtimeDirty,
}, p9.SetAttr{
ATimeSeconds: uint64(atime / 1e9),
ATimeNanoSeconds: uint64(atime % 1e9),
MTimeSeconds: uint64(mtime / 1e9),
MTimeNanoSeconds: uint64(mtime % 1e9),
}); err != nil {
log.Warningf("gofer.dentry.destroyLocked: failed to write dirty timestamps back: %v", err)
}
attr.MTimeSeconds = uint64(mtime / 1e9)
attr.MTimeNanoSeconds = uint64(mtime % 1e9)
}
}
d.file.close(ctx)
// Check if attributes need to be changed before closing the file.
if valid.ATime || valid.MTime {
if err := d.file.setAttrClose(ctx, valid, attr); err != nil {
log.Warningf("gofer.dentry.destroyLocked: failed to close file with write dirty timestamps: %v", err)
}
} else if err := d.file.close(ctx); err != nil {
log.Warningf("gofer.dentry.destroyLocked: failed to close file: %v", err)
}
d.file = p9file{}
// Remove d from the set of syncable dentries.
d.fs.syncMu.Lock()
delete(d.fs.syncableDentries, d)
pkg/sentry/fsimpl/gofer/p9file.go
+7
View File
@@ -127,6 +127,13 @@ func (f p9file) close(ctx context.Context) error {
return err
}
func (f p9file) setAttrClose(ctx context.Context, valid p9.SetAttrMask, attr p9.SetAttr) error {
ctx.UninterruptibleSleepStart(false)
err := f.file.SetAttrClose(valid, attr)
ctx.UninterruptibleSleepFinish(false)
return err
}
func (f p9file) open(ctx context.Context, flags p9.OpenFlags) (*fd.FD, p9.QID, uint32, error) {
ctx.UninterruptibleSleepStart(false)
fdobj, qid, iounit, err := f.file.Open(flags)
runsc/fsgofer/fsgofer.go
+2
View File
@@ -181,6 +181,8 @@ func (a *attachPoint) makeQID(stat unix.Stat_t) p9.QID {
// The few exceptions where it cannot be done are: utimensat on symlinks, and
// Connect() for the socket address.
type localFile struct {
p9.DisallowClientCalls
// attachPoint is the attachPoint that serves this localFile.
attachPoint *attachPoint
test/perf/linux/BUILD
+16
View File
@@ -354,3 +354,19 @@ cc_binary(
"//test/util:test_util",
],
)
cc_binary(
name = "open_read_close_benchmark",
testonly = 1,
srcs = [
"open_read_close_benchmark.cc",
],
deps = [
gbenchmark,
gtest,
"//test/util:fs_util",
"//test/util:logging",
"//test/util:temp_path",
"//test/util:test_main",
],
)
test/perf/linux/open_read_close_benchmark.cc
+61
View File
@@ -0,0 +1,61 @@
// Copyright 2020 The gVisor Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#include <fcntl.h>
#include <stdlib.h>
#include <unistd.h>
#include <memory>
#include <string>
#include <vector>
#include "gtest/gtest.h"
#include "benchmark/benchmark.h"
#include "test/util/fs_util.h"
#include "test/util/logging.h"
#include "test/util/temp_path.h"
namespace gvisor {
namespace testing {
namespace {
void BM_OpenReadClose(benchmark::State& state) {
const int size = state.range(0);
std::vector<TempPath> cache;
for (int i = 0; i < size; i++) {
auto path = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith(
GetAbsoluteTestTmpdir(), "some content", 0644));
cache.emplace_back(std::move(path));
}
char buf[1];
unsigned int seed = 1;
for (auto _ : state) {
const int chosen = rand_r(&seed) % size;
int fd = open(cache[chosen].path().c_str(), O_RDONLY);
TEST_CHECK(fd != -1);
TEST_CHECK(read(fd, buf, 1) == 1);
close(fd);
}
}
// Gofer dentry cache is 1000 by default. Go over it to force files to be closed
// for real.
BENCHMARK(BM_OpenReadClose)->Range(1000, 16384)->UseRealTime();
} // namespace
} // namespace testing
} // namespace gvisor