From 81c3047f0f50b544c2a2054ec852c7cffcee70ab Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 10 Sep 2015 13:31:04 +1000 Subject: [PATCH 1/5] Revised the public StatFSOp docs for the BlockSize/IoSize split. --- fuseops/ops.go | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/fuseops/ops.go b/fuseops/ops.go index eee34c5..dfd62c6 100644 --- a/fuseops/ops.go +++ b/fuseops/ops.go @@ -46,13 +46,14 @@ type StatFSOp struct { // with the block counts below, by callers of statfs(2) to infer the file // system's capacity and space availability. // - // On Linux this can be any value, which will be faitfully returned to the - // caller of statfs(2) (see the code walk above). On OS X it appears it must - // be a power of 2 in [2^9, 2^17]. + // On Linux this is surfaced as statfs::f_frsize, matching the posix standard + // (http://goo.gl/LktgrF), which says that f_blocks and friends are in units + // of f_frsize. On OS X this is surfaced as statfs::f_bsize, which plays the + // same roll. // - // On OS X this also affects statfs::f_iosize, which is documented as the - // "optimal transfer block size". It does not appear to cause osxfuse to - // change the size of data in WriteFile ops, though. + // On Linux this can be any value, and will be faithfully returned to the + // caller of statfs(2) (see the code walk above). On OS X it appears it must + // be a power of 2 in the range [2^9, 2^17]. // // This interface does not distinguish between blocks and block fragments. BlockSize uint32 @@ -67,6 +68,17 @@ type StatFSOp struct { BlocksFree uint64 BlocksAvailable uint64 + // The preferred size of writes to and reads from the file system, in bytes. + // This may affect clients that use statfs(2) to size buffers correctly. + // + // On Linux this is surfaced as statfs::f_bsize, and on OS X as + // statfs::f_iosize. Both are documented in `man 2 statfs` as "optimal + // transfer block size". + // + // On Linux this can be any value. On OS X it appears it must be a power of 2 + // in the range [2^9, 2^20]. + IoSize uint32 + // The total number of inodes in the file system, and how many remain free. Inodes uint64 InodesFree uint64 From 7c543380ea0382a260bd00d7dfd92d6d51179589 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 10 Sep 2015 13:41:26 +1000 Subject: [PATCH 2/5] Add tests for the newly-documented behavior on Darwin. --- fuseops/ops.go | 4 +- samples/statfs/statfs_darwin_test.go | 100 +++++++++++++++++++++------ samples/statfs/statfs_test.go | 3 + 3 files changed, 85 insertions(+), 22 deletions(-) diff --git a/fuseops/ops.go b/fuseops/ops.go index dfd62c6..5399769 100644 --- a/fuseops/ops.go +++ b/fuseops/ops.go @@ -69,7 +69,9 @@ type StatFSOp struct { BlocksAvailable uint64 // The preferred size of writes to and reads from the file system, in bytes. - // This may affect clients that use statfs(2) to size buffers correctly. + // This may affect clients that use statfs(2) to size buffers correctly. It + // does not appear to influence the size of writes sent from the kernel to + // the file system daemon. // // On Linux this is surfaced as statfs::f_bsize, and on OS X as // statfs::f_iosize. Both are documented in `man 2 statfs` as "optimal diff --git a/samples/statfs/statfs_darwin_test.go b/samples/statfs/statfs_darwin_test.go index 1ec9c7b..a1ff8ef 100644 --- a/samples/statfs/statfs_darwin_test.go +++ b/samples/statfs/statfs_darwin_test.go @@ -16,6 +16,7 @@ package statfs_test import ( "fmt" + "math" "regexp" "syscall" @@ -81,6 +82,7 @@ func (t *StatFSTest) Syscall_NonZeroValues() { // Set up the canned response. canned := fuseops.StatFSOp{ BlockSize: 1 << 15, + IoSize: 1 << 16, Blocks: 1<<51 + 3, BlocksFree: 1<<43 + 5, @@ -97,7 +99,7 @@ func (t *StatFSTest) Syscall_NonZeroValues() { AssertEq(nil, err) ExpectEq(canned.BlockSize, stat.Bsize) - ExpectEq(canned.BlockSize, stat.Iosize) + ExpectEq(canned.IoSize, stat.Iosize) ExpectEq(canned.Blocks, stat.Blocks) ExpectEq(canned.BlocksFree, stat.Bfree) ExpectEq(canned.BlocksAvailable, stat.Bavail) @@ -108,30 +110,35 @@ func (t *StatFSTest) Syscall_NonZeroValues() { ExpectEq(fsName, convertName(stat.Mntfromname[:])) } -func (t *StatFSTest) UnsupportedBlockSizes() { +func (t *StatFSTest) BlockSizes() { var err error - // Test a bunch of block sizes that the OS doesn't support faithfully, - // checking what it transforms them too. + // Test a bunch of block sizes that the OS does or doesn't support + // faithfully, checking what it transforms them too. testCases := []struct { - fsBlockSize uint32 - expectedBsize uint32 - expectedIosize uint32 + fsBlockSize uint32 + expectedBsize uint32 }{ - 0: {0, 4096, 65536}, - 1: {1, 512, 512}, - 2: {3, 512, 512}, - 3: {511, 512, 512}, - 4: {513, 1024, 1024}, - 5: {1023, 1024, 1024}, - 6: {4095, 4096, 4096}, - 7: {1<<17 - 1, 1 << 17, 131072}, - 8: {1<<17 + 1, 1 << 17, 1 << 18}, - 9: {1<<18 + 1, 1 << 17, 1 << 19}, - 10: {1<<19 + 1, 1 << 17, 1 << 20}, - 11: {1<<20 + 1, 1 << 17, 1 << 20}, - 12: {1 << 21, 1 << 17, 1 << 20}, - 13: {1 << 30, 1 << 17, 1 << 20}, + 0: {0, 4096}, + 1: {1, 512}, + 2: {3, 512}, + 3: {511, 512}, + 4: {512, 512}, + 5: {513, 1024}, + 6: {1023, 1024}, + 7: {1024, 1024}, + 8: {4095, 4096}, + 9: {1 << 16, 1 << 16}, + 10: {1<<17 - 1, 1 << 17}, + 11: {1 << 17, 1 << 17}, + 12: {1<<17 + 1, 1 << 17}, + 13: {1 << 18, 1 << 17}, + 14: {1 << 20, 1 << 17}, + 15: {math.MaxInt32 - 1, 1 << 17}, + 16: {math.MaxInt32, 1 << 17}, + 17: {math.MaxInt32 + 1, 512}, + 18: {math.MaxInt32 + 1<<15, 1 << 15}, + 19: {math.MaxUint32, 1 << 17}, } for i, tc := range testCases { @@ -151,6 +158,57 @@ func (t *StatFSTest) UnsupportedBlockSizes() { AssertEq(nil, err) ExpectEq(tc.expectedBsize, stat.Bsize, "%s", desc) + } +} + +func (t *StatFSTest) IoSizes() { + var err error + + // Test a bunch of io sizes that the OS does or doesn't support faithfully, + // checking what it transforms them too. + testCases := []struct { + fsIoSize uint32 + expectedIosize uint32 + }{ + 0: {0, 4096}, + 1: {1, 512}, + 2: {3, 512}, + 3: {511, 512}, + 4: {512, 512}, + 5: {513, 1024}, + 6: {1023, 1024}, + 7: {1024, 1024}, + 8: {4095, 4096}, + 9: {1 << 16, 1 << 16}, + 10: {1<<17 - 1, 1 << 17}, + 11: {1 << 17, 1 << 17}, + 12: {1<<17 + 1, 1 << 18}, + 13: {1<<20 - 1, 1 << 20}, + 14: {1 << 20, 1 << 20}, + 15: {1<<20 + 1, 1 << 20}, + 16: {math.MaxInt32 - 1, 1 << 20}, + 17: {math.MaxInt32, 1 << 20}, + 18: {math.MaxInt32 + 1, 512}, + 19: {math.MaxInt32 + 1<<15, 1 << 15}, + 20: {math.MaxUint32, 1 << 20}, + } + + for i, tc := range testCases { + desc := fmt.Sprintf("Case %d: IO size %d", i, tc.fsIoSize) + + // Set up. + canned := fuseops.StatFSOp{ + IoSize: tc.fsIoSize, + Blocks: 10, + } + + t.fs.SetStatFSResponse(canned) + + // Check. + var stat syscall.Statfs_t + err = syscall.Statfs(t.Dir, &stat) + AssertEq(nil, err) + ExpectEq(tc.expectedIosize, stat.Iosize, "%s", desc) } } diff --git a/samples/statfs/statfs_test.go b/samples/statfs/statfs_test.go index e37be43..8f7a4cd 100644 --- a/samples/statfs/statfs_test.go +++ b/samples/statfs/statfs_test.go @@ -146,6 +146,8 @@ func (t *StatFSTest) CapacityAndFreeSpace() { Blocks: 1024, BlocksFree: 896, BlocksAvailable: 768, + + IoSize: 1024, // Shouldn't matter. } // Check that df agrees with us about a range of block sizes. @@ -173,6 +175,7 @@ func (t *StatFSTest) WriteSize() { // Set up a smallish block size. canned := fuseops.StatFSOp{ BlockSize: 8192, + IoSize: 16384, Blocks: 1234, BlocksFree: 1234, BlocksAvailable: 1234, From 33a476cafcb41031b037d388973c651650d72cab Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 10 Sep 2015 13:43:12 +1000 Subject: [PATCH 3/5] Attempt to do what the documentation now says. --- conversions.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/conversions.go b/conversions.go index 4ccbbcd..ee5f041 100644 --- a/conversions.go +++ b/conversions.go @@ -565,8 +565,8 @@ func (c *Connection) kernelResponseForOp( out.St.Files = o.Inodes out.St.Ffree = o.InodesFree - // The posix spec for sys/statvfs.h defines the following fields, among - // others: + // The posix spec for sys/statvfs.h (http://goo.gl/LktgrF) defines the + // following fields of statvfs, among others: // // f_bsize File system block size. // f_frsize Fundamental file system block size. @@ -587,11 +587,10 @@ func (c *Connection) kernelResponseForOp( // fragments in UNIX file systems first appeared in the early 1980s // with the 4.2BSD Fast File System.) // - // Confusingly, it appears as though osxfuse surfaces f_bsize as f_iosize - // (of advisory use only), and f_frsize as f_bsize (which affects free - // space display in the Finder). In any case, we don't care to let the user - // distinguish, so set both to the same value. - out.St.Bsize = o.BlockSize + // Confusingly, it appears as though osxfuse surfaces fuse_kstatfs::bsize + // as statfs::f_iosize (of advisory use only), and fuse_kstatfs::frsize as + // statfs::f_bsize (which affects free space display in the Finder). + out.St.Bsize = o.IoSize out.St.Frsize = o.BlockSize case *initOp: From 6565b52bcba71e3a7f76b4b9bb9c4777516793b8 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 10 Sep 2015 13:51:07 +1000 Subject: [PATCH 4/5] Fixed the documentation to match reality on darwin. --- fuseops/ops.go | 16 +++++++++--- samples/statfs/statfs_darwin_test.go | 38 +++++++++++++--------------- 2 files changed, 29 insertions(+), 25 deletions(-) diff --git a/fuseops/ops.go b/fuseops/ops.go index 5399769..040fc87 100644 --- a/fuseops/ops.go +++ b/fuseops/ops.go @@ -51,9 +51,16 @@ type StatFSOp struct { // of f_frsize. On OS X this is surfaced as statfs::f_bsize, which plays the // same roll. // + // It appears as though the original intent of statvfs::f_frsize in the posix + // standard was to support a smaller addressable unit than statvfs::f_bsize + // (cf. The Linux Programming Interface by Michael Kerrisk, + // https://goo.gl/5LZMxQ). Therefore users should probably arrange for this + // to be no larger than IoSize. + // // On Linux this can be any value, and will be faithfully returned to the - // caller of statfs(2) (see the code walk above). On OS X it appears it must - // be a power of 2 in the range [2^9, 2^17]. + // caller of statfs(2) (see the code walk above). On OS X it appears that + // only powers of 2 in the range [2^9, 2^17] are preserved, and a value of + // zero is treated as 4096. // // This interface does not distinguish between blocks and block fragments. BlockSize uint32 @@ -77,8 +84,9 @@ type StatFSOp struct { // statfs::f_iosize. Both are documented in `man 2 statfs` as "optimal // transfer block size". // - // On Linux this can be any value. On OS X it appears it must be a power of 2 - // in the range [2^9, 2^20]. + // On Linux this can be any value. On OS X it appears that only powers of 2 + // in the range [2^12, 2^20] are faithfully preserved, and a value of zero is + // treated as 65536. IoSize uint32 // The total number of inodes in the file system, and how many remain free. diff --git a/samples/statfs/statfs_darwin_test.go b/samples/statfs/statfs_darwin_test.go index a1ff8ef..b19600e 100644 --- a/samples/statfs/statfs_darwin_test.go +++ b/samples/statfs/statfs_darwin_test.go @@ -170,27 +170,23 @@ func (t *StatFSTest) IoSizes() { fsIoSize uint32 expectedIosize uint32 }{ - 0: {0, 4096}, - 1: {1, 512}, - 2: {3, 512}, - 3: {511, 512}, - 4: {512, 512}, - 5: {513, 1024}, - 6: {1023, 1024}, - 7: {1024, 1024}, - 8: {4095, 4096}, - 9: {1 << 16, 1 << 16}, - 10: {1<<17 - 1, 1 << 17}, - 11: {1 << 17, 1 << 17}, - 12: {1<<17 + 1, 1 << 18}, - 13: {1<<20 - 1, 1 << 20}, - 14: {1 << 20, 1 << 20}, - 15: {1<<20 + 1, 1 << 20}, - 16: {math.MaxInt32 - 1, 1 << 20}, - 17: {math.MaxInt32, 1 << 20}, - 18: {math.MaxInt32 + 1, 512}, - 19: {math.MaxInt32 + 1<<15, 1 << 15}, - 20: {math.MaxUint32, 1 << 20}, + 0: {0, 65536}, + 1: {1, 4096}, + 2: {3, 4096}, + 3: {4095, 4096}, + 4: {4096, 4096}, + 5: {4097, 8192}, + 6: {8191, 8192}, + 7: {8192, 8192}, + 8: {8193, 16384}, + 9: {1<<20 - 1, 1 << 20}, + 10: {1 << 20, 1 << 20}, + 11: {1<<20 + 1, 1 << 20}, + 12: {math.MaxInt32 - 1, 1 << 20}, + 13: {math.MaxInt32, 1 << 20}, + 14: {math.MaxInt32 + 1, 4096}, + 15: {math.MaxInt32 + 1<<15, 1 << 15}, + 16: {math.MaxUint32, 1 << 20}, } for i, tc := range testCases { From 246d37a0baea6f8b4ce240f3a45dc280a92a4efa Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 10 Sep 2015 03:54:29 +0000 Subject: [PATCH 5/5] Updated Linux tests for the new behavior. --- samples/statfs/statfs_linux_test.go | 44 +++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/samples/statfs/statfs_linux_test.go b/samples/statfs/statfs_linux_test.go index 40bae68..86532dd 100644 --- a/samples/statfs/statfs_linux_test.go +++ b/samples/statfs/statfs_linux_test.go @@ -61,6 +61,7 @@ func (t *StatFSTest) Syscall_NonZeroValues() { // Set up the canned response. canned := fuseops.StatFSOp{ BlockSize: 1 << 15, + IoSize: 1 << 16, Blocks: 1<<51 + 3, BlocksFree: 1<<43 + 5, @@ -76,8 +77,8 @@ func (t *StatFSTest) Syscall_NonZeroValues() { err = syscall.Statfs(t.Dir, &stat) AssertEq(nil, err) - ExpectEq(canned.BlockSize, stat.Bsize) ExpectEq(canned.BlockSize, stat.Frsize) + ExpectEq(canned.IoSize, stat.Bsize) ExpectEq(canned.Blocks, stat.Blocks) ExpectEq(canned.BlocksFree, stat.Bfree) ExpectEq(canned.BlocksAvailable, stat.Bavail) @@ -85,7 +86,7 @@ func (t *StatFSTest) Syscall_NonZeroValues() { ExpectEq(canned.InodesFree, stat.Ffree) } -func (t *StatFSTest) WackyBlockSizes() { +func (t *StatFSTest) BlockSizes() { var err error // Test a bunch of weird block sizes that OS X would be cranky about. @@ -98,6 +99,7 @@ func (t *StatFSTest) WackyBlockSizes() { 1<<20 + 0, 1<<20 + 1, math.MaxInt32, + math.MaxInt32 + 1, math.MaxUint32, } @@ -117,7 +119,43 @@ func (t *StatFSTest) WackyBlockSizes() { err = syscall.Statfs(t.Dir, &stat) AssertEq(nil, err) - ExpectEq(bs, stat.Bsize, "%s", desc) ExpectEq(bs, stat.Frsize, "%s", desc) } } + +func (t *StatFSTest) IoSizes() { + var err error + + // Test a bunch of weird IO sizes that OS X would be cranky about. + ioSizes := []uint32{ + 0, + 1, + 3, + 17, + 1<<20 - 1, + 1<<20 + 0, + 1<<20 + 1, + math.MaxInt32, + math.MaxInt32 + 1, + math.MaxUint32, + } + + for _, bs := range ioSizes { + desc := fmt.Sprintf("IO size %d", bs) + + // Set up. + canned := fuseops.StatFSOp{ + IoSize: bs, + Blocks: 10, + } + + t.fs.SetStatFSResponse(canned) + + // Check. + var stat syscall.Statfs_t + err = syscall.Statfs(t.Dir, &stat) + AssertEq(nil, err) + + ExpectEq(bs, stat.Bsize, "%s", desc) + } +}