Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 12 Oct 2021 05:13:08 GMT
From:      Alan Somers <asomers@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 076040aacb5d - stable/12 - fusefs: quiet some cache-related warnings
Message-ID:  <202110120513.19C5D8Z9008876@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by asomers:

URL: https://cgit.FreeBSD.org/src/commit/?id=076040aacb5d58dd924853350136f08d7f37b969

commit 076040aacb5d58dd924853350136f08d7f37b969
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2021-10-03 16:59:04 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2021-10-12 00:29:45 +0000

    fusefs: quiet some cache-related warnings
    
    If the FUSE server does something that would make our cache incoherent,
    we should print a warning to the user.  However, we previously warned in
    some situations when we shouldn't, such as if the file's size changed on
    the server _after_ our own attribute cache had expired.  This change
    suppresses the warning in cases like that.  It also moves the warning
    logic to a single place within the code.
    
    PR:             256936
    Reported by:    Agata <chogata@moosefs.pro>
    Tested by:      Agata <chogata@moosefs.pro>, jSML4ThWwBID69YC@protonmail.com
    
    (cherry picked from commit 5d94aaacb5180798b2f698e33937f068386004eb)
---
 sys/fs/fuse/fuse_internal.c | 74 +++++++++++++++++++++++++++++++--------------
 sys/fs/fuse/fuse_internal.h |  3 +-
 sys/fs/fuse/fuse_io.c       |  4 +--
 sys/fs/fuse/fuse_node.c     | 19 ++++++++++--
 sys/fs/fuse/fuse_node.h     |  2 +-
 sys/fs/fuse/fuse_vfsops.c   | 40 +-----------------------
 sys/fs/fuse/fuse_vnops.c    | 41 ++-----------------------
 7 files changed, 76 insertions(+), 107 deletions(-)

diff --git a/sys/fs/fuse/fuse_internal.c b/sys/fs/fuse/fuse_internal.c
index f70533b54028..90383759f1bd 100644
--- a/sys/fs/fuse/fuse_internal.c
+++ b/sys/fs/fuse/fuse_internal.c
@@ -255,7 +255,8 @@ fuse_internal_access(struct vnode *vp,
  */
 void
 fuse_internal_cache_attrs(struct vnode *vp, struct fuse_attr *attr,
-	uint64_t attr_valid, uint32_t attr_valid_nsec, struct vattr *vap)
+	uint64_t attr_valid, uint32_t attr_valid_nsec, struct vattr *vap,
+	bool from_server)
 {
 	struct mount *mp;
 	struct fuse_vnode_data *fvdat;
@@ -271,9 +272,54 @@ fuse_internal_cache_attrs(struct vnode *vp, struct fuse_attr *attr,
 	fuse_validity_2_bintime(attr_valid, attr_valid_nsec,
 		&fvdat->attr_cache_timeout);
 
+	if (vnode_isreg(vp) &&
+	    fvdat->cached_attrs.va_size != VNOVAL &&
+	    attr->size != fvdat->cached_attrs.va_size)
+	{
+		if ( data->cache_mode == FUSE_CACHE_WB &&
+		    fvdat->flag & FN_SIZECHANGE)
+		{
+			const char *msg;
+
+			/*
+			 * The server changed the file's size even though we're
+			 * using writeback cacheing and and we have outstanding
+			 * dirty writes!  That's a server bug.
+			 */
+			if (fuse_libabi_geq(data, 7, 23)) {
+				msg = "writeback cache incoherent!."
+				    "To prevent data corruption, disable "
+				    "the writeback cache according to your "
+				    "FUSE server's documentation.";
+			} else {
+				msg = "writeback cache incoherent!."
+				    "To prevent data corruption, disable "
+				    "the writeback cache by setting "
+				    "vfs.fusefs.data_cache_mode to 0 or 1.";
+			}
+			fuse_warn(data, FSESS_WARN_WB_CACHE_INCOHERENT, msg);
+		}
+		if (fuse_vnode_attr_cache_valid(vp) &&
+		    data->cache_mode != FUSE_CACHE_UC)
+		{
+			/*
+			 * The server changed the file's size even though we
+			 * have it cached and our cache has not yet expired.
+			 * That's a bug.
+			 */
+			fuse_warn(data, FSESS_WARN_CACHE_INCOHERENT,
+			    "cache incoherent!  "
+			    "To prevent "
+			    "data corruption, disable the data cache "
+			    "by mounting with -o direct_io, or as "
+			    "directed otherwise by your FUSE server's "
+			    "documentation.");
+		}
+	}
+
 	/* Fix our buffers if the filesize changed without us knowing */
 	if (vnode_isreg(vp) && attr->size != fvdat->cached_attrs.va_size) {
-		(void)fuse_vnode_setsize(vp, attr->size);
+		(void)fuse_vnode_setsize(vp, attr->size, from_server);
 		fvdat->cached_attrs.va_size = attr->size;
 	}
 
@@ -807,7 +853,7 @@ fuse_internal_newentry_core(struct vnode *dvp,
 	fuse_vnode_clear_attr_cache(dvp);
 
 	fuse_internal_cache_attrs(*vpp, &feo->attr, feo->attr_valid,
-		feo->attr_valid_nsec, NULL);
+		feo->attr_valid_nsec, NULL, true);
 
 	return err;
 }
@@ -913,26 +959,8 @@ fuse_internal_do_getattr(struct vnode *vp, struct vattr *vap,
 		fao->attr.mtime = old_mtime.tv_sec;
 		fao->attr.mtimensec = old_mtime.tv_nsec;
 	}
-	if (vnode_isreg(vp) &&
-	    fvdat->cached_attrs.va_size != VNOVAL &&
-	    fao->attr.size != fvdat->cached_attrs.va_size) {
-		/*
-		 * The server changed the file's size even though we had it
-		 * cached!  That's a server bug.
-		 */
-		struct mount *mp = vnode_mount(vp);
-		struct fuse_data *data = fuse_get_mpdata(mp);
-
-		fuse_warn(data, FSESS_WARN_CACHE_INCOHERENT,
-		    "cache incoherent!  "
-		    "To prevent data corruption, disable the data cache "
-		    "by mounting with -o direct_io, or as directed "
-		    "otherwise by your FUSE server's documentation.");
-		int iosize = fuse_iosize(vp);
-		v_inval_buf_range(vp, 0, INT64_MAX, iosize);
-	}
 	fuse_internal_cache_attrs(vp, &fao->attr, fao->attr_valid,
-		fao->attr_valid_nsec, vap);
+		fao->attr_valid_nsec, vap, true);
 	if (vtyp != vnode_vtype(vp)) {
 		fuse_internal_vnode_disappear(vp);
 		err = ENOENT;
@@ -1216,7 +1244,7 @@ int fuse_internal_setattr(struct vnode *vp, struct vattr *vap,
 		struct fuse_attr_out *fao = (struct fuse_attr_out*)fdi.answ;
 		fuse_vnode_undirty_cached_timestamps(vp);
 		fuse_internal_cache_attrs(vp, &fao->attr, fao->attr_valid,
-			fao->attr_valid_nsec, NULL);
+			fao->attr_valid_nsec, NULL, false);
 	}
 
 out:
diff --git a/sys/fs/fuse/fuse_internal.h b/sys/fs/fuse/fuse_internal.h
index 85581f3c29c5..295d01b49795 100644
--- a/sys/fs/fuse/fuse_internal.h
+++ b/sys/fs/fuse/fuse_internal.h
@@ -224,7 +224,8 @@ int fuse_internal_access(struct vnode *vp, accmode_t mode,
 
 /* attributes */
 void fuse_internal_cache_attrs(struct vnode *vp, struct fuse_attr *attr,
-	uint64_t attr_valid, uint32_t attr_valid_nsec, struct vattr *vap);
+	uint64_t attr_valid, uint32_t attr_valid_nsec, struct vattr *vap,
+	bool from_server);
 
 /* fsync */
 
diff --git a/sys/fs/fuse/fuse_io.c b/sys/fs/fuse/fuse_io.c
index 5769ac8eccd1..96ee2496fd54 100644
--- a/sys/fs/fuse/fuse_io.c
+++ b/sys/fs/fuse/fuse_io.c
@@ -611,7 +611,7 @@ retry:
 		as_written_offset = uio->uio_offset - diff;
 
 		if (as_written_offset - diff > filesize)
-			fuse_vnode_setsize(vp, as_written_offset);
+			fuse_vnode_setsize(vp, as_written_offset, false);
 		if (as_written_offset - diff >= filesize)
 			fvdat->flag &= ~FN_SIZECHANGE;
 
@@ -755,7 +755,7 @@ again:
 			 * Extend file _after_ locking buffer so we won't race
 			 * with other readers
 			 */
-			err = fuse_vnode_setsize(vp, uio->uio_offset + n);
+			err = fuse_vnode_setsize(vp, uio->uio_offset + n, false);
 			filesize = uio->uio_offset + n;
 			fvdat->flag |= FN_SIZECHANGE;
 			if (err) {
diff --git a/sys/fs/fuse/fuse_node.c b/sys/fs/fuse/fuse_node.c
index fe21dc2881b0..d616d834de28 100644
--- a/sys/fs/fuse/fuse_node.c
+++ b/sys/fs/fuse/fuse_node.c
@@ -387,11 +387,14 @@ fuse_vnode_savesize(struct vnode *vp, struct ucred *cred, pid_t pid)
 }
 
 /*
- * Adjust the vnode's size to a new value, such as that provided by
- * FUSE_GETATTR.
+ * Adjust the vnode's size to a new value.
+ *
+ * If the new value came from the server, such as from a FUSE_GETATTR
+ * operation, set `from_server` true.  But if it came from a local operation,
+ * such as write(2) or truncate(2), set `from_server` false.
  */
 int
-fuse_vnode_setsize(struct vnode *vp, off_t newsize)
+fuse_vnode_setsize(struct vnode *vp, off_t newsize, bool from_server)
 {
 	struct fuse_vnode_data *fvdat = VTOFUD(vp);
 	struct vattr *attrs;
@@ -432,6 +435,16 @@ fuse_vnode_setsize(struct vnode *vp, off_t newsize)
 		MPASS(bp->b_flags & B_VMIO);
 		vfs_bio_clrbuf(bp);
 		bp->b_dirtyend = MIN(bp->b_dirtyend, newsize - lbn * iosize);
+	} else if (from_server && newsize > oldsize && oldsize != VNOVAL) {
+		/*
+		 * The FUSE server changed the file size behind our back.  We
+		 * should invalidate the entire cache.
+		 */
+		daddr_t left_lbn, end_lbn;
+
+		left_lbn = oldsize / iosize;
+		end_lbn = howmany(newsize, iosize);
+		v_inval_buf_range(vp, 0, end_lbn, iosize);
 	}
 out:
 	if (bp)
diff --git a/sys/fs/fuse/fuse_node.h b/sys/fs/fuse/fuse_node.h
index c92874334aa1..8f69ef9a08f3 100644
--- a/sys/fs/fuse/fuse_node.h
+++ b/sys/fs/fuse/fuse_node.h
@@ -199,7 +199,7 @@ void fuse_vnode_open(struct vnode *vp, int32_t fuse_open_flags,
 
 int fuse_vnode_savesize(struct vnode *vp, struct ucred *cred, pid_t pid);
 
-int fuse_vnode_setsize(struct vnode *vp, off_t newsize);
+int fuse_vnode_setsize(struct vnode *vp, off_t newsize, bool from_server);
 
 void fuse_vnode_undirty_cached_timestamps(struct vnode *vp);
 
diff --git a/sys/fs/fuse/fuse_vfsops.c b/sys/fs/fuse/fuse_vfsops.c
index a4a0ce3b86d0..a07fa88a7496 100644
--- a/sys/fs/fuse/fuse_vfsops.c
+++ b/sys/fs/fuse/fuse_vfsops.c
@@ -537,7 +537,6 @@ fuse_vfsop_vget(struct mount *mp, ino_t ino, int flags, struct vnode **vpp)
 	struct fuse_entry_out *feo;
 	struct fuse_vnode_data *fvdat;
 	const char dot[] = ".";
-	off_t filesize;
 	enum vtype vtyp;
 	int error;
 
@@ -574,47 +573,10 @@ fuse_vfsop_vget(struct mount *mp, ino_t ino, int flags, struct vnode **vpp)
 	error = fuse_vnode_get(mp, feo, nodeid, NULL, vpp, NULL, vtyp);
 	if (error)
 		goto out;
-	filesize = feo->attr.size;
-
-	/*
-	 * In the case where we are looking up a FUSE node represented by an
-	 * existing cached vnode, and the true size reported by FUSE_LOOKUP
-	 * doesn't match the vnode's cached size, then any cached writes beyond
-	 * the file's current size are lost.
-	 *
-	 * We can get here:
-	 * * following attribute cache expiration, or
-	 * * due a bug in the daemon, or
-	 */
 	fvdat = VTOFUD(*vpp);
-	if (vnode_isreg(*vpp) &&
-	    filesize != fvdat->cached_attrs.va_size &&
-	    fvdat->flag & FN_SIZECHANGE) {
-		if (data->cache_mode == fuse_data_cache_mode) {
-			const char *msg;
-
-			if (fuse_libabi_geq(data, 7, 23)) {
-				msg = "writeback cache incoherent!."
-				    "To prevent data corruption, disable "
-				    "the writeback cache according to your "
-				    "FUSE server's documentation.";
-			} else {
-				msg = "writeback cache incoherent!."
-				    "To prevent data corruption, disable "
-				    "the writeback cache by setting "
-				    "vfs.fusefs.data_cache_mode to 0 or 1.";
-			}
-			fuse_warn(data, FSESS_WARN_WB_CACHE_INCOHERENT, msg);
-		} else {
-			/* If we get here, it's likely a fusefs kernel bug */
-			printf("%s: WB cache incoherent on %s!\n", __func__,
-			    vnode_mount(*vpp)->mnt_stat.f_mntonname);
-		}
-		fvdat->flag &= ~FN_SIZECHANGE;
-	}
 
 	fuse_internal_cache_attrs(*vpp, &feo->attr, feo->attr_valid,
-		feo->attr_valid_nsec, NULL);
+		feo->attr_valid_nsec, NULL, true);
 	fuse_validity_2_bintime(feo->entry_valid, feo->entry_valid_nsec,
 		&fvdat->entry_cache_timeout);
 out:
diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c
index d13ae257e0a8..a441a004dd05 100644
--- a/sys/fs/fuse/fuse_vnops.c
+++ b/sys/fs/fuse/fuse_vnops.c
@@ -734,7 +734,7 @@ fuse_vnop_create(struct vop_create_args *ap)
 	}
 	ASSERT_VOP_ELOCKED(*vpp, "fuse_vnop_create");
 	fuse_internal_cache_attrs(*vpp, &feo->attr, feo->attr_valid,
-		feo->attr_valid_nsec, NULL);
+		feo->attr_valid_nsec, NULL, true);
 
 	fuse_filehandle_init(*vpp, FUFH_RDWR, NULL, td, cred, foo);
 	fuse_vnode_open(*vpp, foo->open_flags, td);
@@ -941,7 +941,7 @@ fuse_vnop_link(struct vop_link_args *ap)
 		 */
 		fuse_vnode_clear_attr_cache(tdvp);
 		fuse_internal_cache_attrs(vp, &feo->attr, feo->attr_valid,
-			feo->attr_valid_nsec, NULL);
+			feo->attr_valid_nsec, NULL, true);
 	}
 out:
 	fdisp_destroy(&fdi);
@@ -1149,52 +1149,17 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
 			*vpp = dvp;
 		} else {
 			struct fuse_vnode_data *fvdat;
-			struct vattr *vap;
 
 			err = fuse_vnode_get(vnode_mount(dvp), feo, nid, dvp,
 			    &vp, cnp, vtyp);
 			if (err)
 				goto out;
 			*vpp = vp;
-
-			/*
-			 * In the case where we are looking up a FUSE node
-			 * represented by an existing cached vnode, and the
-			 * true size reported by FUSE_LOOKUP doesn't match
-			 * the vnode's cached size, then any cached writes
-			 * beyond the file's current size are lost.
-			 *
-			 * We can get here:
-			 * * following attribute cache expiration, or
-			 * * due a bug in the daemon, or
-			 */
 			fvdat = VTOFUD(vp);
-			if (vnode_isreg(vp) &&
-			    ((filesize != fvdat->cached_attrs.va_size &&
-			      fvdat->flag & FN_SIZECHANGE) ||
-			     ((vap = VTOVA(vp)) &&
-			      filesize != vap->va_size)))
-			{
-				fvdat->flag &= ~FN_SIZECHANGE;
-				/*
-				 * The server changed the file's size even
-				 * though we had it cached, or had dirty writes
-				 * in the WB cache!
-				 */
-				fuse_warn(data, FSESS_WARN_CACHE_INCOHERENT,
-				    "cache incoherent!  "
-				    "To prevent "
-				    "data corruption, disable the data cache "
-				    "by mounting with -o direct_io, or as "
-				    "directed otherwise by your FUSE server's "
-				    "documentation.");
-				int iosize = fuse_iosize(vp);
-				v_inval_buf_range(vp, 0, INT64_MAX, iosize);
-			}
 
 			MPASS(feo != NULL);
 			fuse_internal_cache_attrs(*vpp, &feo->attr,
-				feo->attr_valid, feo->attr_valid_nsec, NULL);
+				feo->attr_valid, feo->attr_valid_nsec, NULL, true);
 			fuse_validity_2_bintime(feo->entry_valid,
 				feo->entry_valid_nsec,
 				&fvdat->entry_cache_timeout);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202110120513.19C5D8Z9008876>