svn commit: r349468 - in projects/fuse2: . share/man/man5 sys/fs/fuse tests/sys/fs/fusefs
Alan Somers
asomers at FreeBSD.org
Thu Jun 27 20:18:15 UTC 2019
Author: asomers
Date: Thu Jun 27 20:18:12 2019
New Revision: 349468
URL: https://svnweb.freebsd.org/changeset/base/349468
Log:
fusefs: recycle vnodes after their last unlink
Previously fusefs would never recycle vnodes. After VOP_INACTIVE, they'd
linger around until unmount or the vnlru reclaimed them. This commit
essentially actives and inlines the old reclaim_revoked sysctl, and fixes
some issues dealing with the attribute cache and multiply linked files.
Sponsored by: The FreeBSD Foundation
Modified:
projects/fuse2/UPDATING
projects/fuse2/share/man/man5/fusefs.5
projects/fuse2/sys/fs/fuse/fuse_internal.c
projects/fuse2/sys/fs/fuse/fuse_vnops.c
projects/fuse2/tests/sys/fs/fusefs/rmdir.cc
projects/fuse2/tests/sys/fs/fusefs/unlink.cc
Modified: projects/fuse2/UPDATING
==============================================================================
--- projects/fuse2/UPDATING Thu Jun 27 19:36:30 2019 (r349467)
+++ projects/fuse2/UPDATING Thu Jun 27 20:18:12 2019 (r349468)
@@ -31,17 +31,17 @@ NOTE TO PEOPLE WHO THINK THAT FreeBSD 13.x IS SLOW:
disable the most expensive debugging functionality run
"ln -s 'abort:false,junk:false' /etc/malloc.conf".)
-20190620:
+20190627:
The vfs.fusefs.sync_unmount and vfs.fusefs.init_backgrounded sysctls
and the "-o sync_unmount" and "-o init_backgrounded" mount options have
been removed from mount_fusefs(8). You can safely remove them from
your scripts, because they had no effect.
The vfs.fusefs.fix_broken_io, vfs.fusefs.sync_resize,
- vfs.fusefs.refresh_size, vfs.fusefs.mmap_enable, and
- vfs.fusefs.data_cache_invalidate sysctls have been removed. If you
- felt the need to set any of them to a non-default value, please tell
- asomers at FreeBSD.org why.
+ vfs.fusefs.refresh_size, vfs.fusefs.mmap_enable,
+ vfs.fusefs.reclaim_revoked, and vfs.fusefs.data_cache_invalidate
+ sysctls have been removed. If you felt the need to set any of them to
+ a non-default value, please tell asomers at FreeBSD.org why.
20190612:
Clang, llvm, lld, lldb, compiler-rt, libc++, libunwind and openmp have
Modified: projects/fuse2/share/man/man5/fusefs.5
==============================================================================
--- projects/fuse2/share/man/man5/fusefs.5 Thu Jun 27 19:36:30 2019 (r349467)
+++ projects/fuse2/share/man/man5/fusefs.5 Thu Jun 27 20:18:12 2019 (r349468)
@@ -98,7 +98,6 @@ Current number of allocated FUSE tickets, which is rou
number of FUSE operations currently being processed by daemons.
.\" Undocumented sysctls
.\" ====================
-.\" vfs.fusefs.reclaim_revoked: I don't understand it well-enough
.\" vfs.fusefs.enforce_dev_perms: I don't understand it well enough.
.\" vfs.fusefs.iov_credit: I don't understand it well enough
.\" vfs.fusefs.iov_permanent_bufsize: I don't understand it well enough
Modified: projects/fuse2/sys/fs/fuse/fuse_internal.c
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_internal.c Thu Jun 27 19:36:30 2019 (r349467)
+++ projects/fuse2/sys/fs/fuse/fuse_internal.c Thu Jun 27 20:18:12 2019 (r349468)
@@ -657,6 +657,7 @@ fuse_internal_remove(struct vnode *dvp,
enum fuse_opcode op)
{
struct fuse_dispatcher fdi;
+ nlink_t nlink;
int err = 0;
fdisp_init(&fdi, cnp->cn_namelen + 1);
@@ -667,6 +668,35 @@ fuse_internal_remove(struct vnode *dvp,
err = fdisp_wait_answ(&fdi);
fdisp_destroy(&fdi);
+
+ if (err)
+ return (err);
+
+ /*
+ * Access the cached nlink even if the attr cached has expired. If
+ * it's inaccurate, the worst that will happen is:
+ * 1) We'll recycle the vnode even though the file has another link we
+ * don't know about, costing a bit of cpu time, or
+ * 2) We won't recycle the vnode even though all of its links are gone.
+ * It will linger around until vnlru reclaims it, costing a bit of
+ * temporary memory.
+ */
+ nlink = VTOFUD(vp)->cached_attrs.va_nlink--;
+
+ /*
+ * Purge the parent's attribute cache because the daemon
+ * should've updated its mtime and ctime.
+ */
+ fuse_vnode_clear_attr_cache(dvp);
+
+ /* NB: nlink could be zero if it was never cached */
+ if (nlink <= 1 || vnode_vtype(vp) == VDIR) {
+ fuse_internal_vnode_disappear(vp);
+ } else {
+ cache_purge(vp);
+ fuse_vnode_update(vp, FN_CTIMECHANGE);
+ }
+
return err;
}
@@ -894,8 +924,6 @@ 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);
}
Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_vnops.c Thu Jun 27 19:36:30 2019 (r349467)
+++ projects/fuse2/sys/fs/fuse/fuse_vnops.c Thu Jun 27 20:18:12 2019 (r349468)
@@ -218,15 +218,6 @@ struct vop_vector fuse_vnops = {
.vop_vptofh = fuse_vnop_vptofh,
};
-/*
- * XXX: This feature is highly experimental and can bring to instabilities,
- * needs revisiting before to be enabled by default.
- */
-static int fuse_reclaim_revoked = 0;
-
-SYSCTL_INT(_vfs_fusefs, OID_AUTO, reclaim_revoked, CTLFLAG_RW,
- &fuse_reclaim_revoked, 0, "");
-
uma_zone_t fuse_pbuf_zone;
#define fuse_vm_page_lock(m) vm_page_lock((m));
@@ -880,9 +871,9 @@ fuse_vnop_inactive(struct vop_inactive_args *ap)
fuse_filehandle_close(vp, fufh, td, NULL);
}
- if ((fvdat->flag & FN_REVOKED) != 0 && fuse_reclaim_revoked) {
+ if ((fvdat->flag & FN_REVOKED) != 0)
vrecycle(vp);
- }
+
return 0;
}
@@ -1568,18 +1559,9 @@ fuse_vnop_remove(struct vop_remove_args *ap)
if (vnode_isdir(vp)) {
return EPERM;
}
- cache_purge(vp);
err = fuse_internal_remove(dvp, vp, cnp, FUSE_UNLINK);
- if (err == 0) {
- fuse_internal_vnode_disappear(vp);
- /*
- * Purge the parent's attribute cache because the daemon
- * should've updated its mtime and ctime
- */
- fuse_vnode_clear_attr_cache(dvp);
- }
return err;
}
@@ -1691,14 +1673,6 @@ fuse_vnop_rmdir(struct vop_rmdir_args *ap)
}
err = fuse_internal_remove(dvp, vp, ap->a_cnp, FUSE_RMDIR);
- if (err == 0) {
- fuse_internal_vnode_disappear(vp);
- /*
- * Purge the parent's attribute cache because the daemon
- * should've updated its mtime and ctime
- */
- fuse_vnode_clear_attr_cache(dvp);
- }
return err;
}
Modified: projects/fuse2/tests/sys/fs/fusefs/rmdir.cc
==============================================================================
--- projects/fuse2/tests/sys/fs/fusefs/rmdir.cc Thu Jun 27 19:36:30 2019 (r349467)
+++ projects/fuse2/tests/sys/fs/fusefs/rmdir.cc Thu Jun 27 20:18:12 2019 (r349468)
@@ -30,6 +30,7 @@
extern "C" {
#include <fcntl.h>
+#include <semaphore.h>
}
#include "mockfs.hh"
@@ -55,11 +56,13 @@ void expect_getattr(uint64_t ino, mode_t mode)
})));
}
-void expect_lookup(const char *relpath, uint64_t ino)
+void expect_lookup(const char *relpath, uint64_t ino, int times=1)
{
EXPECT_LOOKUP(FUSE_ROOT_ID, relpath)
- .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
+ .Times(times)
+ .WillRepeatedly(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
SET_OUT_HEADER_LEN(out, entry);
+ out.body.entry.attr_valid = UINT64_MAX;
out.body.entry.attr.mode = S_IFDIR | 0755;
out.body.entry.nodeid = ino;
out.body.entry.attr.nlink = 2;
@@ -83,13 +86,16 @@ void expect_rmdir(uint64_t parent, const char *relpath
* A successful rmdir should clear the parent directory's attribute cache,
* because the fuse daemon should update its mtime and ctime
*/
-TEST_F(Rmdir, clear_attr_cache)
+TEST_F(Rmdir, parent_attr_cache)
{
const char FULLPATH[] = "mountpoint/some_dir";
const char RELPATH[] = "some_dir";
struct stat sb;
+ sem_t sem;
uint64_t ino = 42;
+ ASSERT_EQ(0, sem_init(&sem, 0, 0)) << strerror(errno);
+
EXPECT_CALL(*m_mock, process(
ResultOf([=](auto in) {
return (in.header.opcode == FUSE_GETATTR &&
@@ -105,9 +111,12 @@ TEST_F(Rmdir, clear_attr_cache)
})));
expect_lookup(RELPATH, ino);
expect_rmdir(FUSE_ROOT_ID, RELPATH, 0);
+ expect_forget(ino, 1, &sem);
ASSERT_EQ(0, rmdir(FULLPATH)) << strerror(errno);
EXPECT_EQ(0, stat("mountpoint", &sb)) << strerror(errno);
+ sem_wait(&sem);
+ sem_destroy(&sem);
}
TEST_F(Rmdir, enotempty)
@@ -124,15 +133,40 @@ TEST_F(Rmdir, enotempty)
ASSERT_EQ(ENOTEMPTY, errno);
}
+/* Removing a directory should expire its entry cache */
+TEST_F(Rmdir, entry_cache)
+{
+ const char FULLPATH[] = "mountpoint/some_dir";
+ const char RELPATH[] = "some_dir";
+ sem_t sem;
+ uint64_t ino = 42;
+
+ expect_getattr(1, S_IFDIR | 0755);
+ expect_lookup(RELPATH, ino, 2);
+ expect_rmdir(FUSE_ROOT_ID, RELPATH, 0);
+ expect_forget(ino, 1, &sem);
+
+ ASSERT_EQ(0, rmdir(FULLPATH)) << strerror(errno);
+ ASSERT_EQ(0, access(FULLPATH, F_OK)) << strerror(errno);
+ sem_wait(&sem);
+ sem_destroy(&sem);
+}
+
TEST_F(Rmdir, ok)
{
const char FULLPATH[] = "mountpoint/some_dir";
const char RELPATH[] = "some_dir";
+ sem_t sem;
uint64_t ino = 42;
+ ASSERT_EQ(0, sem_init(&sem, 0, 0)) << strerror(errno);
+
expect_getattr(FUSE_ROOT_ID, S_IFDIR | 0755);
expect_lookup(RELPATH, ino);
expect_rmdir(FUSE_ROOT_ID, RELPATH, 0);
+ expect_forget(ino, 1, &sem);
ASSERT_EQ(0, rmdir(FULLPATH)) << strerror(errno);
+ sem_wait(&sem);
+ sem_destroy(&sem);
}
Modified: projects/fuse2/tests/sys/fs/fusefs/unlink.cc
==============================================================================
--- projects/fuse2/tests/sys/fs/fusefs/unlink.cc Thu Jun 27 19:36:30 2019 (r349467)
+++ projects/fuse2/tests/sys/fs/fusefs/unlink.cc Thu Jun 27 20:18:12 2019 (r349468)
@@ -29,6 +29,7 @@
extern "C" {
#include <fcntl.h>
+#include <semaphore.h>
}
#include "mockfs.hh"
@@ -54,18 +55,61 @@ void expect_getattr(uint64_t ino, mode_t mode)
})));
}
-void expect_lookup(const char *relpath, uint64_t ino, int times)
+void expect_lookup(const char *relpath, uint64_t ino, int times, int nlink=1)
{
- FuseTest::expect_lookup(relpath, ino, S_IFREG | 0644, 0, times);
+ EXPECT_LOOKUP(FUSE_ROOT_ID, relpath)
+ .Times(times)
+ .WillRepeatedly(Invoke(
+ ReturnImmediate([=](auto in __unused, auto& out) {
+ SET_OUT_HEADER_LEN(out, entry);
+ out.body.entry.attr.mode = S_IFREG | 0644;
+ out.body.entry.nodeid = ino;
+ out.body.entry.attr.nlink = nlink;
+ out.body.entry.attr_valid = UINT64_MAX;
+ out.body.entry.attr.size = 0;
+ })));
}
};
/*
+ * Unlinking a multiply linked file should update its ctime and nlink. This
+ * could be handled simply by invalidating the attributes, necessitating a new
+ * GETATTR, but we implement it in-kernel for efficiency's sake.
+ */
+TEST_F(Unlink, attr_cache)
+{
+ const char FULLPATH0[] = "mountpoint/some_file.txt";
+ const char RELPATH0[] = "some_file.txt";
+ const char FULLPATH1[] = "mountpoint/other_file.txt";
+ const char RELPATH1[] = "other_file.txt";
+ uint64_t ino = 42;
+ struct stat sb_old, sb_new;
+ int fd1;
+
+ expect_getattr(1, S_IFDIR | 0755);
+ expect_lookup(RELPATH0, ino, 1, 2);
+ expect_lookup(RELPATH1, ino, 1, 2);
+ expect_open(ino, 0, 1);
+ expect_unlink(1, RELPATH0, 0);
+
+ fd1 = open(FULLPATH1, O_RDONLY);
+ ASSERT_LE(0, fd1) << strerror(errno);
+
+ ASSERT_EQ(0, fstat(fd1, &sb_old)) << strerror(errno);
+ ASSERT_EQ(0, unlink(FULLPATH0)) << strerror(errno);
+ ASSERT_EQ(0, fstat(fd1, &sb_new)) << strerror(errno);
+ EXPECT_NE(sb_old.st_ctime, sb_new.st_ctime);
+ EXPECT_EQ(1u, sb_new.st_nlink);
+
+ leak(fd1);
+}
+
+/*
* A successful unlink should clear the parent directory's attribute cache,
* because the fuse daemon should update its mtime and ctime
*/
-TEST_F(Unlink, clear_attr_cache)
+TEST_F(Unlink, parent_attr_cache)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
const char RELPATH[] = "some_file.txt";
@@ -85,7 +129,8 @@ TEST_F(Unlink, clear_attr_cache)
out.body.attr.attr.mode = S_IFDIR | 0755;
out.body.attr.attr_valid = UINT64_MAX;
})));
- expect_lookup(RELPATH, ino, 1);
+ /* Use nlink=2 so we don't get a FUSE_FORGET */
+ expect_lookup(RELPATH, ino, 1, 2);
expect_unlink(1, RELPATH, 0);
ASSERT_EQ(0, unlink(FULLPATH)) << strerror(errno);
@@ -106,34 +151,101 @@ TEST_F(Unlink, eperm)
ASSERT_EQ(EPERM, errno);
}
+/*
+ * Unlinking a file should expire its entry cache, even if it's multiply linked
+ */
+TEST_F(Unlink, entry_cache)
+{
+ const char FULLPATH[] = "mountpoint/some_file.txt";
+ const char RELPATH[] = "some_file.txt";
+ uint64_t ino = 42;
+
+ expect_getattr(1, S_IFDIR | 0755);
+ expect_lookup(RELPATH, ino, 2, 2);
+ expect_unlink(1, RELPATH, 0);
+
+ ASSERT_EQ(0, unlink(FULLPATH)) << strerror(errno);
+ ASSERT_EQ(0, access(FULLPATH, F_OK)) << strerror(errno);
+}
+
+/*
+ * Unlink a multiply-linked file. There should be no FUSE_FORGET because the
+ * file is still linked.
+ */
+TEST_F(Unlink, multiply_linked)
+{
+ const char FULLPATH0[] = "mountpoint/some_file.txt";
+ const char RELPATH0[] = "some_file.txt";
+ const char FULLPATH1[] = "mountpoint/other_file.txt";
+ const char RELPATH1[] = "other_file.txt";
+ uint64_t ino = 42;
+
+ expect_getattr(1, S_IFDIR | 0755);
+ expect_lookup(RELPATH0, ino, 1, 2);
+ expect_unlink(1, RELPATH0, 0);
+ EXPECT_CALL(*m_mock, process(
+ ResultOf([=](auto in) {
+ return (in.header.opcode == FUSE_FORGET &&
+ in.header.nodeid == ino);
+ }, Eq(true)),
+ _)
+ ).Times(0);
+ expect_lookup(RELPATH1, ino, 1, 1);
+
+ ASSERT_EQ(0, unlink(FULLPATH0)) << strerror(errno);
+
+ /*
+ * The final syscall simply ensures that no FUSE_FORGET was ever sent,
+ * by scheduling an arbitrary different operation after a FUSE_FORGET
+ * would've been sent.
+ */
+ ASSERT_EQ(0, access(FULLPATH1, F_OK)) << strerror(errno);
+}
+
TEST_F(Unlink, ok)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
const char RELPATH[] = "some_file.txt";
uint64_t ino = 42;
+ sem_t sem;
+ ASSERT_EQ(0, sem_init(&sem, 0, 0)) << strerror(errno);
+
expect_getattr(1, S_IFDIR | 0755);
expect_lookup(RELPATH, ino, 1);
expect_unlink(1, RELPATH, 0);
+ expect_forget(ino, 1, &sem);
ASSERT_EQ(0, unlink(FULLPATH)) << strerror(errno);
+ sem_wait(&sem);
+ sem_destroy(&sem);
}
/* Unlink an open file */
TEST_F(Unlink, open_but_deleted)
{
- const char FULLPATH[] = "mountpoint/some_file.txt";
- const char RELPATH[] = "some_file.txt";
+ const char FULLPATH0[] = "mountpoint/some_file.txt";
+ const char RELPATH0[] = "some_file.txt";
+ const char FULLPATH1[] = "mountpoint/other_file.txt";
+ const char RELPATH1[] = "other_file.txt";
uint64_t ino = 42;
int fd;
expect_getattr(1, S_IFDIR | 0755);
- expect_lookup(RELPATH, ino, 2);
+ expect_lookup(RELPATH0, ino, 2);
expect_open(ino, 0, 1);
- expect_unlink(1, RELPATH, 0);
+ expect_unlink(1, RELPATH0, 0);
+ expect_lookup(RELPATH1, ino, 1, 1);
- fd = open(FULLPATH, O_RDWR);
+ fd = open(FULLPATH0, O_RDWR);
ASSERT_LE(0, fd) << strerror(errno);
- ASSERT_EQ(0, unlink(FULLPATH)) << strerror(errno);
+ ASSERT_EQ(0, unlink(FULLPATH0)) << strerror(errno);
+
+ /*
+ * The final syscall simply ensures that no FUSE_FORGET was ever sent,
+ * by scheduling an arbitrary different operation after a FUSE_FORGET
+ * would've been sent.
+ */
+ ASSERT_EQ(0, access(FULLPATH1, F_OK)) << strerror(errno);
leak(fd);
}
More information about the svn-src-projects
mailing list