From 0d2bf489968deeb94dbd81de24ddf7e946271c9c Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Fri, 31 May 2019 21:22:58 +0000 Subject: [PATCH] fusefs: check the vnode cache when looking up files for the NFS server FUSE allows entries to be cached for a limited amount of time. fusefs's vnop_lookup method already implements that using the timeout functionality of cache_lookup/cache_enter_time. However, lookups for the NFS server go through a separate path: vfs_vget. That path can't use the same timeout functionality because cache_lookup/cache_enter_time only work on pathnames, whereas vfs_vget works by inode number. This commit adds entry timeout information to the fuse vnode structure, and checks it during vfs_vget. This allows the NFS server to take advantage of cached entries. It's also the same path that FUSE's asynchronous cache invalidation operations will use. Sponsored by: The FreeBSD Foundation --- sys/fs/fuse/fuse_internal.c | 1 + sys/fs/fuse/fuse_internal.h | 3 ++ sys/fs/fuse/fuse_node.h | 5 ++++ sys/fs/fuse/fuse_vfsops.c | 29 +++++++++++++++---- sys/fs/fuse/fuse_vnops.c | 11 ++++++-- tests/sys/fs/fusefs/nfs.cc | 55 +++++++++++++++++++++++++++++++------ 6 files changed, 86 insertions(+), 18 deletions(-) diff --git a/sys/fs/fuse/fuse_internal.c b/sys/fs/fuse/fuse_internal.c index ff001029c9b4..1c00a8164d5a 100644 --- a/sys/fs/fuse/fuse_internal.c +++ b/sys/fs/fuse/fuse_internal.c @@ -750,6 +750,7 @@ fuse_internal_vnode_disappear(struct vnode *vp) ASSERT_VOP_ELOCKED(vp, "fuse_internal_vnode_disappear"); fvdat->flag |= FN_REVOKED; bintime_clear(&fvdat->attr_cache_timeout); + bintime_clear(&fvdat->entry_cache_timeout); cache_purge(vp); } diff --git a/sys/fs/fuse/fuse_internal.h b/sys/fs/fuse/fuse_internal.h index 3b0f674dd959..a963c79cc1d8 100644 --- a/sys/fs/fuse/fuse_internal.h +++ b/sys/fs/fuse/fuse_internal.h @@ -68,6 +68,9 @@ #include "fuse_ipc.h" #include "fuse_node.h" +extern u_long fuse_lookup_cache_hits; +extern u_long fuse_lookup_cache_misses; + static inline bool vfs_isrdonly(struct mount *mp) { diff --git a/sys/fs/fuse/fuse_node.h b/sys/fs/fuse/fuse_node.h index 25551c520ab6..81bf6e3b70d4 100644 --- a/sys/fs/fuse/fuse_node.h +++ b/sys/fs/fuse/fuse_node.h @@ -97,6 +97,11 @@ struct fuse_vnode_data { /** meta **/ /* The monotonic time after which the attr cache is invalid */ struct bintime attr_cache_timeout; + /* + * Monotonic time after which the entry is invalid. Used for lookups + * by nodeid instead of pathname. + */ + struct bintime entry_cache_timeout; struct vattr cached_attrs; uint64_t nlookup; enum vtype vtype; diff --git a/sys/fs/fuse/fuse_vfsops.c b/sys/fs/fuse/fuse_vfsops.c index 4bd736c1fca6..07026e1aaf35 100644 --- a/sys/fs/fuse/fuse_vfsops.c +++ b/sys/fs/fuse/fuse_vfsops.c @@ -527,19 +527,34 @@ fuse_vfsop_vget(struct mount *mp, ino_t ino, int flags, struct vnode **vpp) struct fuse_dispatcher fdi; struct fuse_entry_out *feo; struct fuse_vnode_data *fvdat; + struct bintime now; const char dot[] = "."; off_t filesize; enum vtype vtyp; int error; + error = vfs_hash_get(mp, fuse_vnode_hash(nodeid), flags, td, vpp, + fuse_vnode_cmp, &nodeid); + if (error) + return error; /* - * TODO Check the vnode cache, verifying entry cache timeout. Normally - * done during VOP_LOOKUP + * Check the entry cache timeout. We have to do this within fusefs + * instead of by using cache_enter_time/cache_lookup because those + * routines are only intended to work with pathnames, not inodes */ - /*error = vfs_hash_get(mp, fuse_vnode_hash(nodeid), LK_EXCLUSIVE, td, vpp,*/ - /*fuse_vnode_cmp, &nodeid);*/ - /*if (error || *vpp != NULL)*/ - /*return error;*/ + if (*vpp != NULL) { + getbinuptime(&now); + if (bintime_cmp(&(VTOFUD(*vpp)->entry_cache_timeout), &now, >)){ + atomic_add_acq_long(&fuse_lookup_cache_hits, 1); + return 0; + } else { + /* Entry cache timeout */ + atomic_add_acq_long(&fuse_lookup_cache_misses, 1); + cache_purge(*vpp); + vput(*vpp); + *vpp = NULL; + } + } /* Do a LOOKUP, using nodeid as the parent and "." as filename */ fdisp_init(&fdi, sizeof(dot)); @@ -585,6 +600,8 @@ fuse_vfsop_vget(struct mount *mp, ino_t ino, int flags, struct vnode **vpp) fuse_internal_cache_attrs(*vpp, td->td_ucred, &feo->attr, feo->attr_valid, feo->attr_valid_nsec, NULL); + fuse_validity_2_bintime(feo->entry_valid, feo->entry_valid_nsec, + &fvdat->entry_cache_timeout); out: fdisp_destroy(&fdi); return error; diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c index ecfc02422964..3caccdf54ad8 100644 --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -218,12 +218,12 @@ struct vop_vector fuse_vnops = { .vop_vptofh = fuse_vnop_vptofh, }; -static u_long fuse_lookup_cache_hits = 0; +u_long fuse_lookup_cache_hits = 0; SYSCTL_ULONG(_vfs_fusefs, OID_AUTO, lookup_cache_hits, CTLFLAG_RD, &fuse_lookup_cache_hits, 0, "number of positive cache hits in lookup"); -static u_long fuse_lookup_cache_misses = 0; +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"); @@ -965,6 +965,8 @@ fuse_vnop_lookup(struct vop_lookup_args *ap) /* Cache timeout */ atomic_add_acq_long(&fuse_lookup_cache_misses, 1); + bintime_clear( + &VTOFUD(*vpp)->entry_cache_timeout); cache_purge(*vpp); if (dvp != *vpp) vput(*vpp); @@ -1103,6 +1105,9 @@ fuse_vnop_lookup(struct vop_lookup_args *ap) MPASS(feo != NULL); fuse_internal_cache_attrs(*vpp, cred, &feo->attr, feo->attr_valid, feo->attr_valid_nsec, NULL); + fuse_validity_2_bintime(feo->entry_valid, + feo->entry_valid_nsec, + &fvdat->entry_cache_timeout); if ((nameiop == DELETE || nameiop == RENAME) && islastcn) @@ -2536,7 +2541,7 @@ fuse_vnop_print(struct vop_print_args *ap) * Get an NFS filehandle for a FUSE file. * * This will only work for FUSE file systems that guarantee the uniqueness of - * nodeid:generation, which most don't + * nodeid:generation, which most don't. */ /* vop_vptofh { diff --git a/tests/sys/fs/fusefs/nfs.cc b/tests/sys/fs/fusefs/nfs.cc index 6505b3c5ff58..ef2d0310106b 100644 --- a/tests/sys/fs/fusefs/nfs.cc +++ b/tests/sys/fs/fusefs/nfs.cc @@ -145,11 +145,7 @@ TEST_F(Fhstat, lookup_dot) } /* Use a file handle whose entry is still cached */ -/* - * Disabled because fuse_vfsop_vget doesn't yet check the entry cache. No PR - * because that's a feature request, not a bug - */ -TEST_F(Fhstat, DISABLED_cached) +TEST_F(Fhstat, cached) { const char FULLPATH[] = "mountpoint/some_dir/."; const char RELDIRPATH[] = "some_dir"; @@ -157,7 +153,6 @@ TEST_F(Fhstat, DISABLED_cached) struct stat sb; const uint64_t ino = 42; const mode_t mode = S_IFDIR | 0755; - const uid_t uid = 12345; EXPECT_LOOKUP(FUSE_ROOT_ID, RELDIRPATH) .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { @@ -165,15 +160,57 @@ TEST_F(Fhstat, DISABLED_cached) out.body.entry.attr.mode = mode; out.body.entry.nodeid = ino; out.body.entry.generation = 1; - out.body.entry.attr.uid = uid; + out.body.entry.attr.ino = ino; out.body.entry.attr_valid = UINT64_MAX; out.body.entry.entry_valid = UINT64_MAX; }))); ASSERT_EQ(0, getfh(FULLPATH, &fhp)) << strerror(errno); ASSERT_EQ(0, fhstat(&fhp, &sb)) << strerror(errno); - EXPECT_EQ(uid, sb.st_uid); - EXPECT_EQ(mode, sb.st_mode); + EXPECT_EQ(ino, sb.st_ino); +} + +/* File handle entries should expire from the cache, too */ +TEST_F(Fhstat, cache_expired) +{ + const char FULLPATH[] = "mountpoint/some_dir/."; + const char RELDIRPATH[] = "some_dir"; + fhandle_t fhp; + struct stat sb; + const uint64_t ino = 42; + const mode_t mode = S_IFDIR | 0755; + + EXPECT_LOOKUP(FUSE_ROOT_ID, RELDIRPATH) + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, entry); + out.body.entry.attr.mode = mode; + out.body.entry.nodeid = ino; + out.body.entry.generation = 1; + out.body.entry.attr.ino = ino; + out.body.entry.attr_valid = UINT64_MAX; + out.body.entry.entry_valid_nsec = NAP_NS / 2; + }))); + + EXPECT_LOOKUP(ino, ".") + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, entry); + out.body.entry.attr.mode = mode; + out.body.entry.nodeid = ino; + out.body.entry.generation = 1; + out.body.entry.attr.ino = ino; + out.body.entry.attr_valid = UINT64_MAX; + out.body.entry.entry_valid = 0; + }))); + + ASSERT_EQ(0, getfh(FULLPATH, &fhp)) << strerror(errno); + ASSERT_EQ(0, fhstat(&fhp, &sb)) << strerror(errno); + EXPECT_EQ(ino, sb.st_ino); + + nap(); + + /* Cache should be expired; fuse should issue a FUSE_LOOKUP */ + ASSERT_EQ(0, fhstat(&fhp, &sb)) << strerror(errno); + EXPECT_EQ(ino, sb.st_ino); } /*