Commit d2138350 authored by Kirill Smelkov's avatar Kirill Smelkov

fuse/test: Fix TestFopenKeepCache, take 2

In 2019 TestOpenKeepCache was first disabled in ce2558b4 (fuse/test:
disable TestFopenKeepCache) and then reenabled in 904ef0cc (fuse/test:
Fix TestFopenKeepCache). The latter patch hoped to fix this test

    - by disabling CAP_AUTO_INVAL_DATA via ExplicitDataCacheControl. This
      should stop kernel from dropping data cache on mtime change;
    - by using the same size for before and after states. This avoid hitting
      cache being dropped when kernel sees file size being changed.

In other words it took actions so that the data cache is not
unexpectedly dropped by the kernel and the test does not fail with
seeing different content after file reopen.

However the test also wants to verify that updated data can be seen
after cache invalidation and here it was not exactly correct: the test
requested full control over the data cache via ExplicitDataCacheControl
mount option, but was explicitly invalidating only file attributes
via pathfs.EntryNotify _without_ invalidating the data cache.

This way the test was actually working before Linux 5.2, because those
older kernels never negotiate CAP_EXPLICIT_INVAL_DATA(*), but starting
from Linux 5.2 CAP_EXPLICIT_INVAL_DATA started to be negotiated, and so
the kernel was dropping only metadata without invalidating data cache on
EntryNotify. This started to lead to the following test failures:

    --- FAIL: TestFopenKeepCache (0.13s)
        cache_test.go:153: ReadFile: got "before" after notify, want "afterX"

@hanwen explains in https://github.com/hanwen/go-fuse/issues/473#issuecomment-1585617107

    It tries to invalidate the file content by issuing an ENTRY_NOTIFY. This
    leads to different mtime returned, but in the init phase, we don´t
    return AUTO_INVAL_DATA, because setupCacheTest explicitly disables it.

    === RUN   TestFopenKeepCache
    12:47:49.958227 callFusermount: executing ["/usr/bin/fusermount3" "/tmp/TestFopenKeepCache1094589261/001/mnt" "-o" "subtype=pathfs.pathInode,max_read=131072"]
    12:47:49.967664 rx 2: INIT n0 {7.38 Ra 131072 FLOCK_LOCKS,IOCTL_DIR,READDIRPLUS,READDIRPLUS_AUTO,PARALLEL_DIROPS,HANDLE_KILLPRIV,ASYNC_READ,BIG_WRITES,SPLICE_WRITE,SPLICE_MOVE,WRITEBACK_CACHE,NO_OPEN_SUPPORT,POSIX_ACL,POSIX_LOCKS,EXPORT_SUPPORT,AUTO_INVAL_DATA,ASYNC_DIO,MAX_PAGES,CACHE_SYMLINKS,NO_OPENDIR_SUPPORT,DONT_MASK,SPLICE_READ,ABORT_ERROR,EXPLICIT_INVAL_DATA,ATOMIC_O_TRUNC,0x70000000} "\a\x00\x00\x00\x00\x00\x00\x00"... 48b
    12:47:49.967708 tx 2:     OK, {7.28 Ra 131072 BIG_WRITES,NO_OPEN_SUPPORT,MAX_PAGES,EXPLICIT_INVAL_DATA,ASYNC_READ,READDIRPLUS,PARALLEL_DIROPS 0/0 Wr 131072 Tg 0 MaxPages 32}

-> Let's fix this.

As explained in 904ef0cc we do not want to reenable AUTO_INVAL_DATA
because then the kernel will start dropping data cache on mtime change
and the test will start to fail in its first part - where we want to
assert that the data stays the same until data cache is invalidated.

So let's change EntryNotify to FileNotify to explicitly invalidate data
cache when we want the file to read with new data.

Hopefully fixes: https://github.com/hanwen/go-fuse/issues/473

(*) see git.kernel.org/linus/ad2ba64dd489 and github.com/hanwen/go-fuse/pull/273

Change-Id: I4ff0f2bef61f0217601eee6d272f7e5563b272d2
parent ce3f09e5
...@@ -99,11 +99,15 @@ func TestFopenKeepCache(t *testing.T) { ...@@ -99,11 +99,15 @@ func TestFopenKeepCache(t *testing.T) {
return st return st
} }
// XXX Linux FUSE client automatically invalidates cache of a file if it sees size change. // Without CAP_EXPLICIT_INVAL_DATA Linux FUSE client automatically
// As workaround we keep len(before) == len(after) to avoid that codepath. // invalidates cache of a file if it sees size change. As workaround we
// See https://github.com/hanwen/go-fuse/pull/273 for details. // keep len(before) == len(after) to avoid that codepath.
// //
// TODO use len(before) != len(after) if kernel supports precise control over data cache. // With CAP_EXPLICIT_INVAL_DATA - even when data cache stays the same,
// but st_size changes, Linux reads file content as dcache[:st_size],
// i.e. either truncated (st_size↓) or extended (st_size↑). Here
// len(before) == len(after) also helps to avoid dealing with those
// peculiarities.
before := "before" before := "before"
after := "afterX" after := "afterX"
if len(before) != len(after) { if len(before) != len(after) {
...@@ -130,7 +134,8 @@ func TestFopenKeepCache(t *testing.T) { ...@@ -130,7 +134,8 @@ func TestFopenKeepCache(t *testing.T) {
// this forces kernel client to relookup/regetattr the file and reread the attributes. // this forces kernel client to relookup/regetattr the file and reread the attributes.
// //
// this way we make sure the kernel knows updated size/mtime before we // this way we make sure the kernel knows updated size/mtime before we
// try to read the file next time. // try to read the file next time, which should invalidate data cache
// if CAP_EXPLICIT_INVAL_DATA was not negotiated.
time.Sleep(100 * time.Millisecond) time.Sleep(100 * time.Millisecond)
_ = xstat(wd + "/mnt/file.txt") _ = xstat(wd + "/mnt/file.txt")
...@@ -143,9 +148,9 @@ func TestFopenKeepCache(t *testing.T) { ...@@ -143,9 +148,9 @@ func TestFopenKeepCache(t *testing.T) {
t.Skipf("protocol v%d has no notify support.", minor) t.Skipf("protocol v%d has no notify support.", minor)
} }
code := pathfs.EntryNotify("", "file.txt") code := pathfs.FileNotify("file.txt", 0, 0) // invalidate direntry and whole data cache
if !code.Ok() { if !code.Ok() {
t.Errorf("EntryNotify: %v", code) t.Errorf("FileNotify: %v", code)
} }
c = xreadFile(wd + "/mnt/file.txt") c = xreadFile(wd + "/mnt/file.txt")
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment