From 3f2c630c74d2cc9f78b5c92ae48fb3646f12fe96 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Tue, 9 Apr 2019 00:47:38 +0000 Subject: [PATCH] fusefs: implement attribute cache timeouts The FUSE protocol allows the server to specify the timeout period for the client's attribute and entry caches. This commit implements the timeout period for the attribute cache. The entry cache's timeout period is currently disabled because it panics, and is guarded by the vfs.fusefs.lookup_cache_expire sysctl. PR: 235773 Reported by: cem Sponsored by: The FreeBSD Foundation --- sys/fs/fuse/fuse_internal.c | 19 +++++++++----- sys/fs/fuse/fuse_internal.h | 2 ++ sys/fs/fuse/fuse_node.c | 17 ++++++++++++- sys/fs/fuse/fuse_node.h | 18 +++++++++++--- sys/fs/fuse/fuse_vnops.c | 45 +++++++++++++++++++++++++++++++--- tests/sys/fs/fusefs/getattr.cc | 10 ++++---- tests/sys/fs/fusefs/lookup.cc | 17 ++++++++----- 7 files changed, 102 insertions(+), 26 deletions(-) diff --git a/sys/fs/fuse/fuse_internal.c b/sys/fs/fuse/fuse_internal.c index bffe98b6097f..34595a329151 100644 --- a/sys/fs/fuse/fuse_internal.c +++ b/sys/fs/fuse/fuse_internal.c @@ -189,15 +189,22 @@ fuse_internal_cache_attrs(struct vnode *vp, struct fuse_attr *attr, struct mount *mp; struct fuse_vnode_data *fvdat; struct vattr *vp_cache_at; + struct timespec now, duration, timeout; mp = vnode_mount(vp); fvdat = VTOFUD(vp); - /* Honor explicit do-not-cache requests from user filesystems. */ - if (attr_valid == 0 && attr_valid_nsec == 0) - fvdat->valid_attr_cache = false; - else - fvdat->valid_attr_cache = true; + getnanouptime(&now); + /* "+ 2" is the bound of attr_valid_nsec + now.tv_nsec */ + /* Why oh why isn't there a TIME_MAX defined? */ + if (attr_valid >= INT_MAX || attr_valid + now.tv_sec + 2 >= INT_MAX) { + fvdat->attr_cache_timeout.sec = INT_MAX; + } else { + duration.tv_sec = attr_valid; + duration.tv_nsec = attr_valid_nsec; + timespecadd(&duration, &now, &timeout); + timespec2bintime(&timeout, &fvdat->attr_cache_timeout); + } vp_cache_at = VTOVA(vp); @@ -646,7 +653,7 @@ fuse_internal_vnode_disappear(struct vnode *vp) ASSERT_VOP_ELOCKED(vp, "fuse_internal_vnode_disappear"); fvdat->flag |= FN_REVOKED; - fvdat->valid_attr_cache = false; + bintime_clear(&fvdat->attr_cache_timeout); cache_purge(vp); } diff --git a/sys/fs/fuse/fuse_internal.h b/sys/fs/fuse/fuse_internal.h index 49064994831b..8baf666a403d 100644 --- a/sys/fs/fuse/fuse_internal.h +++ b/sys/fs/fuse/fuse_internal.h @@ -68,6 +68,8 @@ #include "fuse_ipc.h" #include "fuse_node.h" +extern int fuse_lookup_cache_expire; + static inline bool vfs_isrdonly(struct mount *mp) { diff --git a/sys/fs/fuse/fuse_node.c b/sys/fs/fuse/fuse_node.c index 66b6b1959b66..20338684bd5a 100644 --- a/sys/fs/fuse/fuse_node.c +++ b/sys/fs/fuse/fuse_node.c @@ -293,7 +293,22 @@ fuse_vnode_get(struct mount *mp, (feo->entry_valid != 0 || feo->entry_valid_nsec != 0)) { ASSERT_VOP_LOCKED(*vpp, "fuse_vnode_get"); ASSERT_VOP_LOCKED(dvp, "fuse_vnode_get"); - cache_enter(dvp, *vpp, cnp); + if (fuse_lookup_cache_expire) { + struct timespec duration, now, timeout; + + getnanouptime(&now); + if (feo->entry_valid >= INT_MAX || + feo->entry_valid + now.tv_sec + 2 >= INT_MAX) { + timeout.tv_sec = INT_MAX; + } else { + duration.tv_sec = feo->entry_valid; + duration.tv_nsec = feo->entry_valid_nsec; + timespecadd(&duration, &now, &timeout); + } + cache_enter_time(dvp, *vpp, cnp, &timeout, NULL); + } else { + cache_enter(dvp, *vpp, cnp); + } } /* diff --git a/sys/fs/fuse/fuse_node.h b/sys/fs/fuse/fuse_node.h index fea87fcc9a4e..855b81937500 100644 --- a/sys/fs/fuse/fuse_node.h +++ b/sys/fs/fuse/fuse_node.h @@ -87,7 +87,8 @@ struct fuse_vnode_data { uint32_t flag; /** meta **/ - bool valid_attr_cache; + /* The monotonic time after which the attr cache is invalid */ + struct bintime attr_cache_timeout; struct vattr cached_attrs; off_t filesize; uint64_t nlookup; @@ -97,9 +98,18 @@ struct fuse_vnode_data { #define VTOFUD(vp) \ ((struct fuse_vnode_data *)((vp)->v_data)) #define VTOI(vp) (VTOFUD(vp)->nid) -#define VTOVA(vp) \ - (VTOFUD(vp)->valid_attr_cache ? \ - &(VTOFUD(vp)->cached_attrs) : NULL) +static inline struct vattr* +VTOVA(struct vnode *vp) +{ + struct bintime now; + + getbinuptime(&now); + if (bintime_cmp(&(VTOFUD(vp)->attr_cache_timeout), &now, >)) + return &(VTOFUD(vp)->cached_attrs); + else + return NULL; +} + #define VTOILLU(vp) ((uint64_t)(VTOFUD(vp) ? VTOI(vp) : 0)) #define FUSE_NULL_ID 0 diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c index 2729956fda93..90bcbeedb71b 100644 --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -195,10 +195,11 @@ static u_long fuse_lookup_cache_misses = 0; SYSCTL_ULONG(_vfs_fusefs, OID_AUTO, lookup_cache_misses, CTLFLAG_RD, &fuse_lookup_cache_misses, 0, "number of cache misses in lookup"); -int fuse_lookup_cache_enable = 1; +int fuse_lookup_cache_expire = 0; -SYSCTL_INT(_vfs_fusefs, OID_AUTO, lookup_cache_enable, CTLFLAG_RW, - &fuse_lookup_cache_enable, 0, "if non-zero, enable lookup cache"); +SYSCTL_INT(_vfs_fusefs, OID_AUTO, lookup_cache_expire, CTLFLAG_RW, + &fuse_lookup_cache_expire, 0, + "if non-zero, expire fuse lookup cache entries at the proper time"); /* * XXX: This feature is highly experimental and can bring to instabilities, @@ -749,7 +750,43 @@ fuse_vnop_lookup(struct vop_lookup_args *ap) fdisp_init(&fdi, 0); op = FUSE_GETATTR; goto calldaemon; - } else if (fuse_lookup_cache_enable) { + } else if (fuse_lookup_cache_expire) { + struct timespec now, timeout; + + err = cache_lookup(dvp, vpp, cnp, &timeout, NULL); + switch (err) { + + case -1: /* positive match */ + getnanouptime(&now); + if (timespeccmp(&timeout, &now, <=)) { + atomic_add_acq_long(&fuse_lookup_cache_hits, 1); + } else { + /* Cache timeout */ + atomic_add_acq_long(&fuse_lookup_cache_misses, + 1); + cache_purge(*vpp); + break; + } + return 0; + + case 0: /* no match in cache */ + atomic_add_acq_long(&fuse_lookup_cache_misses, 1); + break; + + case ENOENT: /* negative match */ + getnanouptime(&now); + if (timespeccmp(&timeout, &now, >)) { + /* Cache timeout */ + printf("Purging vnode %p name=%s\n", *vpp, + cnp->cn_nameptr); + fuse_internal_vnode_disappear(*vpp); + break; + } + /* fall through */ + default: + return err; + } + } else { err = cache_lookup(dvp, vpp, cnp, NULL, NULL); switch (err) { diff --git a/tests/sys/fs/fusefs/getattr.cc b/tests/sys/fs/fusefs/getattr.cc index d4ba90907573..72ed86fd58e6 100644 --- a/tests/sys/fs/fusefs/getattr.cc +++ b/tests/sys/fs/fusefs/getattr.cc @@ -94,8 +94,7 @@ TEST_F(Getattr, attr_cache) * discard the cached attributes and requery the daemon after the timeout * period passes. */ -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=235773 */ -TEST_F(Getattr, DISABLED_attr_cache_timeout) +TEST_F(Getattr, attr_cache_timeout) { const char FULLPATH[] = "mountpoint/some_file.txt"; const char RELPATH[] = "some_file.txt"; @@ -107,7 +106,7 @@ TEST_F(Getattr, DISABLED_attr_cache_timeout) */ long timeout_ns = 250'000'000; - expect_lookup(RELPATH, ino, S_IFREG | 0644, 0, 2, 0, 0); + expect_lookup(RELPATH, ino, S_IFREG | 0644, 0, 1, 0, 0); EXPECT_CALL(*m_mock, process( ResultOf([](auto in) { return (in->header.opcode == FUSE_GETATTR && @@ -118,13 +117,14 @@ TEST_F(Getattr, DISABLED_attr_cache_timeout) .WillRepeatedly(Invoke(ReturnImmediate([=](auto i __unused, auto out) { SET_OUT_HEADER_LEN(out, attr); out->body.attr.attr_valid_nsec = timeout_ns; - out->body.attr.attr_valid = UINT64_MAX; + out->body.attr.attr_valid = 0; out->body.attr.attr.ino = ino; // Must match nodeid out->body.attr.attr.mode = S_IFREG | 0644; }))); + EXPECT_EQ(0, stat(FULLPATH, &sb)); usleep(2 * timeout_ns / 1000); - /* Timeout has expire. stat(2) should requery the daemon */ + /* Timeout has expired. stat(2) should requery the daemon */ EXPECT_EQ(0, stat(FULLPATH, &sb)); } diff --git a/tests/sys/fs/fusefs/lookup.cc b/tests/sys/fs/fusefs/lookup.cc index 8b47286bb4cf..9b1d966df6f2 100644 --- a/tests/sys/fs/fusefs/lookup.cc +++ b/tests/sys/fs/fusefs/lookup.cc @@ -104,8 +104,7 @@ TEST_F(Lookup, attr_cache) * If lookup returns a finite but non-zero cache timeout, then we should discard * the cached attributes and requery the daemon. */ -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=235773 */ -TEST_F(Lookup, DISABLED_attr_cache_timeout) +TEST_F(Lookup, attr_cache_timeout) { const char FULLPATH[] = "mountpoint/some_file.txt"; const char RELPATH[] = "some_file.txt"; @@ -118,6 +117,7 @@ TEST_F(Lookup, DISABLED_attr_cache_timeout) long timeout_ns = 250'000'000; EXPECT_LOOKUP(1, RELPATH) + .Times(2) .WillRepeatedly(Invoke(ReturnImmediate([=](auto in __unused, auto out) { SET_OUT_HEADER_LEN(out, entry); out->body.entry.nodeid = ino; @@ -125,10 +125,10 @@ TEST_F(Lookup, DISABLED_attr_cache_timeout) out->body.entry.attr.ino = ino; // Must match nodeid out->body.entry.attr.mode = S_IFREG | 0644; }))); - expect_getattr(ino, 0); - /* access(2) will issue a VOP_LOOKUP but not a VOP_GETATTR */ + /* access(2) will issue a VOP_LOOKUP and fill the attr cache */ ASSERT_EQ(0, access(FULLPATH, F_OK)) << strerror(errno); + /* Next access(2) will use the cached attributes */ usleep(2 * timeout_ns / 1000); /* The cache has timed out; VOP_GETATTR should query the daemon*/ ASSERT_EQ(0, stat(FULLPATH, &sb)) << strerror(errno); @@ -222,16 +222,21 @@ TEST_F(Lookup, DISABLED_entry_cache_timeout) */ long timeout_ns = 250'000'000; - EXPECT_LOOKUP(1, RELPATH).Times(2) + EXPECT_LOOKUP(1, RELPATH) + .Times(2) .WillRepeatedly(Invoke(ReturnImmediate([=](auto in __unused, auto out) { SET_OUT_HEADER_LEN(out, entry); out->body.entry.entry_valid_nsec = timeout_ns; out->body.entry.attr.mode = S_IFREG | 0644; out->body.entry.nodeid = 14; }))); + + /* access(2) will issue a VOP_LOOKUP and fill the entry cache */ + ASSERT_EQ(0, access(FULLPATH, F_OK)) << strerror(errno); + /* Next access(2) will use the cached entry */ ASSERT_EQ(0, access(FULLPATH, F_OK)) << strerror(errno); usleep(2 * timeout_ns / 1000); - /* The cache has timed out; VOP_LOOKUP should query the daemon*/ + /* The cache has timed out; VOP_LOOKUP should requery the daemon*/ ASSERT_EQ(0, access(FULLPATH, F_OK)) << strerror(errno); }