From a7e81cb3db7cf1f928030d627a5a6d42ffaa6279 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Thu, 4 Apr 2019 20:30:14 +0000 Subject: [PATCH] fusefs: properly handle FOPEN_KEEP_CACHE If a fuse file system returne FOPEN_KEEP_CACHE in the open or create response, then the client is supposed to _not_ clear its caches for that file. I don't know why clearing the caches would be the default given that there's a separate flag to bypass the cache altogether, but that's the way it is. fusefs(5) will now honor this flag. Our behavior is slightly different than Linux's because we reuse file handles. That means that open(2) wont't clear the cache if there's a reusable file handle, even if the file server wouldn't have sent FOPEN_KEEP_CACHE had we opened a new file handle like Linux does. PR: 236560 Sponsored by: The FreeBSD Foundation --- sys/fs/fuse/fuse_file.c | 20 +++++++++++++++----- sys/fs/fuse/fuse_file.h | 2 +- sys/fs/fuse/fuse_node.c | 10 ---------- sys/fs/fuse/fuse_vnops.c | 3 +-- tests/sys/fs/fusefs/read.cc | 11 +++++------ 5 files changed, 22 insertions(+), 24 deletions(-) diff --git a/sys/fs/fuse/fuse_file.c b/sys/fs/fuse/fuse_file.c index 75050f7e985..119f7e911a7 100644 --- a/sys/fs/fuse/fuse_file.c +++ b/sys/fs/fuse/fuse_file.c @@ -79,6 +79,7 @@ __FBSDID("$FreeBSD$"); #include "fuse.h" #include "fuse_file.h" #include "fuse_internal.h" +#include "fuse_io.h" #include "fuse_ipc.h" #include "fuse_node.h" @@ -188,9 +189,7 @@ fuse_filehandle_open(struct vnode *vp, int a_mode, } foo = fdi.answ; - fuse_filehandle_init(vp, fufh_type, fufhp, td->td_proc->p_pid, cred, - foo); - + fuse_filehandle_init(vp, fufh_type, fufhp, td, cred, foo); fuse_vnode_open(vp, foo->open_flags, td); out: @@ -322,7 +321,7 @@ fuse_filehandle_getrw(struct vnode *vp, int fflag, void fuse_filehandle_init(struct vnode *vp, fufh_type_t fufh_type, - struct fuse_filehandle **fufhp, pid_t pid, struct ucred *cred, + struct fuse_filehandle **fufhp, struct thread *td, struct ucred *cred, struct fuse_open_out *foo) { struct fuse_vnode_data *fvdat = VTOFUD(vp); @@ -335,7 +334,7 @@ fuse_filehandle_init(struct vnode *vp, fufh_type_t fufh_type, fufh->fufh_type = fufh_type; fufh->gid = cred->cr_rgid; fufh->uid = cred->cr_uid; - fufh->pid = pid; + fufh->pid = td->td_proc->p_pid; fufh->fuse_open_flags = foo->open_flags; if (!FUFH_IS_VALID(fufh)) { panic("FUSE: init: invalid filehandle id (type=%d)", fufh_type); @@ -345,4 +344,15 @@ fuse_filehandle_init(struct vnode *vp, fufh_type_t fufh_type, *fufhp = fufh; atomic_add_acq_int(&fuse_fh_count, 1); + + if (foo->open_flags & FOPEN_DIRECT_IO) { + ASSERT_VOP_ELOCKED(vp, __func__); + VTOFUD(vp)->flag |= FN_DIRECTIO; + fuse_io_invalbuf(vp, td); + } else { + if ((foo->open_flags & FOPEN_KEEP_CACHE) == 0) + fuse_io_invalbuf(vp, td); + VTOFUD(vp)->flag &= ~FN_DIRECTIO; + } + } diff --git a/sys/fs/fuse/fuse_file.h b/sys/fs/fuse/fuse_file.h index 937909ea828..f34fbf89c18 100644 --- a/sys/fs/fuse/fuse_file.h +++ b/sys/fs/fuse/fuse_file.h @@ -158,7 +158,7 @@ int fuse_filehandle_getrw(struct vnode *vp, int fflag, pid_t pid); void fuse_filehandle_init(struct vnode *vp, fufh_type_t fufh_type, - struct fuse_filehandle **fufhp, pid_t pid, + struct fuse_filehandle **fufhp, struct thread *td, struct ucred *cred, struct fuse_open_out *foo); int fuse_filehandle_open(struct vnode *vp, int mode, struct fuse_filehandle **fufhp, struct thread *td, diff --git a/sys/fs/fuse/fuse_node.c b/sys/fs/fuse/fuse_node.c index 8af26f00426..453645bc0e5 100644 --- a/sys/fs/fuse/fuse_node.c +++ b/sys/fs/fuse/fuse_node.c @@ -329,16 +329,6 @@ fuse_vnode_open(struct vnode *vp, int32_t fuse_open_flags, struct thread *td) * * XXXIP: Handle fd based DIRECT_IO */ - if (fuse_open_flags & FOPEN_DIRECT_IO) { - ASSERT_VOP_ELOCKED(vp, __func__); - VTOFUD(vp)->flag |= FN_DIRECTIO; - fuse_io_invalbuf(vp, td); - } else { - if ((fuse_open_flags & FOPEN_KEEP_CACHE) == 0) - fuse_io_invalbuf(vp, td); - VTOFUD(vp)->flag &= ~FN_DIRECTIO; - } - if (vnode_vtype(vp) == VREG) { /* XXXIP prevent getattr, by using cached node size */ vnode_create_vobject(vp, 0, td); diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c index b20ee17775c..61e11725db5 100644 --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -479,8 +479,7 @@ fuse_vnop_create(struct vop_create_args *ap) } ASSERT_VOP_ELOCKED(*vpp, "fuse_vnop_create"); - fuse_filehandle_init(*vpp, FUFH_RDWR, NULL, td->td_proc->p_pid, cred, - foo); + fuse_filehandle_init(*vpp, FUFH_RDWR, NULL, td, cred, foo); fuse_vnode_open(*vpp, foo->open_flags, td); cache_purge_negative(dvp); diff --git a/tests/sys/fs/fusefs/read.cc b/tests/sys/fs/fusefs/read.cc index dd56202519b..70d43b233f0 100644 --- a/tests/sys/fs/fusefs/read.cc +++ b/tests/sys/fs/fusefs/read.cc @@ -398,8 +398,7 @@ TEST_F(Read, eio) * With the keep_cache option, the kernel may keep its read cache across * multiple open(2)s. */ -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236560 */ -TEST_F(Read, DISABLED_keep_cache) +TEST_F(Read, keep_cache) { const char FULLPATH[] = "mountpoint/some_file.txt"; const char RELPATH[] = "some_file.txt"; @@ -410,7 +409,7 @@ TEST_F(Read, DISABLED_keep_cache) char buf[bufsize]; FuseTest::expect_lookup(RELPATH, ino, S_IFREG | 0644, bufsize, 2); - expect_open(ino, FOPEN_KEEP_CACHE, 1); + expect_open(ino, FOPEN_KEEP_CACHE, 2); expect_getattr(ino, bufsize); expect_read(ino, 0, bufsize, bufsize, CONTENTS); @@ -418,7 +417,7 @@ TEST_F(Read, DISABLED_keep_cache) ASSERT_LE(0, fd0) << strerror(errno); ASSERT_EQ(bufsize, read(fd0, buf, bufsize)) << strerror(errno); - fd1 = open(FULLPATH, O_RDONLY); + fd1 = open(FULLPATH, O_RDWR); ASSERT_LE(0, fd1) << strerror(errno); /* @@ -445,7 +444,7 @@ TEST_F(Read, keep_cache_disabled) char buf[bufsize]; FuseTest::expect_lookup(RELPATH, ino, S_IFREG | 0644, bufsize, 2); - expect_open(ino, FOPEN_KEEP_CACHE, 1); + expect_open(ino, 0, 2); expect_getattr(ino, bufsize); expect_read(ino, 0, bufsize, bufsize, CONTENTS); @@ -453,7 +452,7 @@ TEST_F(Read, keep_cache_disabled) ASSERT_LE(0, fd0) << strerror(errno); ASSERT_EQ(bufsize, read(fd0, buf, bufsize)) << strerror(errno); - fd1 = open(FULLPATH, O_RDONLY); + fd1 = open(FULLPATH, O_RDWR); ASSERT_LE(0, fd1) << strerror(errno); /*