From nobody Tue Dec 07 05:08:22 2021 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id C7E2C18C92F9; Tue, 7 Dec 2021 05:08:23 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4J7SxL6W4zz55j1; Tue, 7 Dec 2021 05:08:22 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id C13C319B27; Tue, 7 Dec 2021 05:08:22 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 1B758MOU095826; Tue, 7 Dec 2021 05:08:22 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1B758MXL095825; Tue, 7 Dec 2021 05:08:22 GMT (envelope-from git) Date: Tue, 7 Dec 2021 05:08:22 GMT Message-Id: <202112070508.1B758MXL095825@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Alan Somers Subject: git: 94b01af1e595 - stable/13 - fusefs: quiet some cache-related warnings List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: asomers X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: 94b01af1e5953131d0ca2875e6a0cc882047540a Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1638853702; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=aAirj4PNSCtf/QxNGH2XtEsgvMApc49bd7xlghYP+Oc=; b=tc3yLRg3QYdCP2P9zo8EUG3One2WZI5+v8L/r3scvd3JvAjMCnVC7m+9mwAr/9Uo2zXIA8 lgsIu3in93C1X77C+HNeHq0dWDT+ONS6K3kEnHBeOz5Vu3t39hKWQAKZ+1GmgsShPmWwfn IQJ4AHCV+743afbgPxoSFE1EZBBHksSPz8FhNA7JumByDAT8FPBMK6TaaLCLauCMhqDj8w vlExKFuxiRPcD5LHac3vrbZLoaahTLq8HTTo3sDZCvwYi5mhESUAlLMua45p5b0AJGoRL8 rqfMPBlp97sGqnHsQZtSIuz+rJuHLnmPc9dlk3vQsJPVylpSxA4KFmJntkEn1w== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1638853702; a=rsa-sha256; cv=none; b=cnFkAekKnXXNzKVedvpphuOOGaTMFgTVy3Zf3QI8yxoNQiKkqvEw76tXtAMV8t6A0KCi4R WZK2E52FEsGbUAufLK9UaBaUBoL835QzGI9d96yJ4KDiAU6u/VumtYclCAL7cqelO/3e7y Nr5jjXXzF+5wbKnhxE3tVJN4fzQn/8ROQtX175qoaNPnH34QVuSLI5RUAAoOqnyewvbjlz eVVsI2/SRLE8Eqec+Exf04xhZCn8cuaSStnU7cudKmChXSQ4Jl55SA+LytmoYUD3H+hdjF D7qghpaMTlwMlYI0ZRhp2glc9XOHbqb+XABoq7xoC8MDXL7PguuKPth3/DuYnw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by asomers: URL: https://cgit.FreeBSD.org/src/commit/?id=94b01af1e5953131d0ca2875e6a0cc882047540a commit 94b01af1e5953131d0ca2875e6a0cc882047540a Author: Alan Somers AuthorDate: 2021-10-03 16:59:04 +0000 Commit: Alan Somers CommitDate: 2021-12-07 05:04:44 +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 Tested by: Agata , 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 c52f12d3a71a..ffa4a40095b2 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; } @@ -806,7 +852,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; } @@ -912,26 +958,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; @@ -1231,7 +1259,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 20a10d7dfda0..e9fa3857227a 100644 --- a/sys/fs/fuse/fuse_internal.h +++ b/sys/fs/fuse/fuse_internal.h @@ -223,7 +223,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 58adfde88871..26f3039f0e14 100644 --- a/sys/fs/fuse/fuse_io.c +++ b/sys/fs/fuse/fuse_io.c @@ -571,7 +571,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; @@ -715,7 +715,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 d19e49358a22..c8982c5a48a1 100644 --- a/sys/fs/fuse/fuse_node.c +++ b/sys/fs/fuse/fuse_node.c @@ -388,11 +388,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; @@ -433,6 +436,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 b188996066d5..3d5e168bcde2 100644 --- a/sys/fs/fuse/fuse_vfsops.c +++ b/sys/fs/fuse/fuse_vfsops.c @@ -539,7 +539,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; @@ -576,47 +575,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 4c8c8c1d4461..dd8ff0fcc45a 100644 --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -895,7 +895,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); @@ -1151,7 +1151,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); @@ -1360,52 +1360,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);