From nobody Tue Dec 07 05:12:43 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 9686D18CA785; Tue, 7 Dec 2021 05:12:44 +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 4J7T2M6zYSz56gB; Tue, 7 Dec 2021 05:12:43 +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 D05C619EA3; Tue, 7 Dec 2021 05:12:43 +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 1B75ChKH009186; Tue, 7 Dec 2021 05:12:43 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1B75ChSQ009185; Tue, 7 Dec 2021 05:12:43 GMT (envelope-from git) Date: Tue, 7 Dec 2021 05:12:43 GMT Message-Id: <202112070512.1B75ChSQ009185@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: bdd7a3f2a17c - stable/13 - fusefs: fix a recurse-on-non-recursive lockmgr panic 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: bdd7a3f2a17c4ed6361f1789a72fb07403e42172 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1638853964; 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=kubFSvSqKMDNGashsG1riy/XfZZLLJxEkKtbmvDrND0=; b=tVG5IH7eTCLYgqemucvTtVVjbImkAtHwslVGRZRF6hAR5HpBQYZSOQxTmwKFawhyXLr1BX 6xGoqjkRlirhWDZ3cDo70xXpN4/fOiMbH8o/YnKsjilDQ+culIbAiUGNHjlKJZC5XwCg7L dUQa+sksIiWeo7upv1UlD/Qim4oXHDy4amC06Hw8AsvSSN4ptRvh/msoWpOIJYr5EIYep5 qPEl/yN0sPNPrH3ECCTZTFmpScMkKYWYa69Kpk/teD1peCI180SxS5yk8PV+dU4UupCMes Xgn055RKbWBd9kMhr15ICStpFlltV728azvERZXUluZHbZnFOyeEhB7Fz7bbiQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1638853964; a=rsa-sha256; cv=none; b=ippCrQHwvFvede4D12iVnK9RyZgq4/qcVO5D/89fMi/c3RNRzsIo4ien72d6T60LCQGRMf mPsValKrUGRzdjvu0EjuVdKCFY1FD2GwNdSkPhU+ujrqhKxQhplIG6KKlj+l91KNMfpO6c 0G+gI/TBGt/VuYd4uu1Eyk7A5yULn9xSdv4/cBxutO44ONeZX4keNm+pUZhaRBlQ73TKq0 SoAz1564fjmh9QJDrvuxg1P1JgMDqXrxYrvWIVcave94T/Qz2sYtPHgL1iiovF9wkM2UcN FuM+62VybCs2uduYm+XWWOpZigol/eSAYoKrasvI47yY5YS3Oow9yCghJk04TQ== 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=bdd7a3f2a17c4ed6361f1789a72fb07403e42172 commit bdd7a3f2a17c4ed6361f1789a72fb07403e42172 Author: Alan Somers AuthorDate: 2021-10-02 18:17:36 +0000 Commit: Alan Somers CommitDate: 2021-12-07 05:08:32 +0000 fusefs: fix a recurse-on-non-recursive lockmgr panic fuse_vnop_bmap needs to know the file's size in order to calculate the optimum amount of readahead. If the file's size is unknown, it must ask the FUSE server. But if the file's data was previously cached and the server reports that its size has shrunk, fusefs must invalidate the cached data. That's not possible during VOP_BMAP because the buffer object is already locked. Fix the panic by not querying the FUSE server for the file's size during VOP_BMAP if we don't need it. That's also a a slight performance optimization. PR: 256937 Reported by: Agata Tested by: Agata (cherry picked from commit 7430017b9978cae054ed99e5160f739e5ca021d5) --- sys/fs/fuse/fuse_vnops.c | 20 ++++++++--- tests/sys/fs/fusefs/bmap.cc | 86 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 4 deletions(-) diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c index dd8ff0fcc45a..9aafbad990c5 100644 --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -516,8 +516,9 @@ fuse_vnop_bmap(struct vop_bmap_args *ap) struct fuse_bmap_in *fbi; struct fuse_bmap_out *fbo; struct fuse_data *data; + struct fuse_vnode_data *fvdat = VTOFUD(vp); uint64_t biosize; - off_t filesize; + off_t fsize; daddr_t lbn = ap->a_bn; daddr_t *pbn = ap->a_bnp; int *runp = ap->a_runp; @@ -550,10 +551,21 @@ fuse_vnop_bmap(struct vop_bmap_args *ap) */ if (runb != NULL) *runb = MIN(lbn, maxrun); - if (runp != NULL) { - error = fuse_vnode_size(vp, &filesize, td->td_ucred, td); + if (runp != NULL && maxrun == 0) + *runp = 0; + else if (runp != NULL) { + /* + * If the file's size is cached, use that value to calculate + * runp, even if the cache is expired. runp is only advisory, + * and the risk of getting it wrong is not worth the cost of + * another upcall. + */ + if (fvdat->cached_attrs.va_size != VNOVAL) + fsize = fvdat->cached_attrs.va_size; + else + error = fuse_vnode_size(vp, &fsize, td->td_ucred, td); if (error == 0) - *runp = MIN(MAX(0, filesize / (off_t)biosize - lbn - 1), + *runp = MIN(MAX(0, fsize / (off_t)biosize - lbn - 1), maxrun); else *runp = 0; diff --git a/tests/sys/fs/fusefs/bmap.cc b/tests/sys/fs/fusefs/bmap.cc index 3f392a4447f2..c635f4d7e46f 100644 --- a/tests/sys/fs/fusefs/bmap.cc +++ b/tests/sys/fs/fusefs/bmap.cc @@ -75,6 +75,8 @@ void expect_lookup(const char *relpath, uint64_t ino, off_t size) } }; +class BmapEof: public Bmap, public WithParamInterface {}; + /* * Test FUSE_BMAP * XXX The FUSE protocol does not include the runp and runb variables, so those @@ -165,3 +167,87 @@ TEST_F(Bmap, default_) leak(fd); } + +/* + * VOP_BMAP should not query the server for the file's size, even if its cached + * attributes have expired. + * Regression test for https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256937 + */ +TEST_P(BmapEof, eof) +{ + /* + * Outline: + * 1) lookup the file, setting attr_valid=0 + * 2) Read more than one block, causing the kernel to issue VOP_BMAP to + * plan readahead. + * 3) Nothing should panic + * 4) Repeat the tests, truncating the file after different numbers of + * GETATTR operations. + */ + Sequence seq; + const off_t filesize = 2 * m_maxbcachebuf; + const ino_t ino = 42; + mode_t mode = S_IFREG | 0644; + void *contents, *buf; + int fd; + int ngetattrs; + + ngetattrs = GetParam(); + contents = calloc(1, filesize); + FuseTest::expect_lookup(RELPATH, ino, mode, filesize, 1, 0); + expect_open(ino, 0, 1); + // Depending on ngetattrs, FUSE_READ could be called with either + // filesize or filesize / 2 . + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in.header.opcode == FUSE_READ && + in.header.nodeid == ino && + in.body.read.offset == 0 && + ( in.body.read.size == filesize || + in.body.read.size == filesize / 2)); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnImmediate([=](auto in, auto& out) { + size_t osize = in.body.read.size; + out.header.len = sizeof(struct fuse_out_header) + osize; + bzero(out.body.bytes, osize); + }))); + EXPECT_CALL(*m_mock, process( + ResultOf([](auto in) { + return (in.header.opcode == FUSE_GETATTR && + in.header.nodeid == ino); + }, Eq(true)), + _) + ).Times(Between(ngetattrs - 1, ngetattrs)) + .InSequence(seq) + .WillRepeatedly(Invoke(ReturnImmediate([=](auto i __unused, auto& out) { + SET_OUT_HEADER_LEN(out, attr); + out.body.attr.attr_valid = 0; + out.body.attr.attr.ino = ino; + out.body.attr.attr.mode = S_IFREG | 0644; + out.body.attr.attr.size = filesize; + }))); + EXPECT_CALL(*m_mock, process( + ResultOf([](auto in) { + return (in.header.opcode == FUSE_GETATTR && + in.header.nodeid == ino); + }, Eq(true)), + _) + ).InSequence(seq) + .WillRepeatedly(Invoke(ReturnImmediate([=](auto i __unused, auto& out) { + SET_OUT_HEADER_LEN(out, attr); + out.body.attr.attr_valid = 0; + out.body.attr.attr.ino = ino; + out.body.attr.attr.mode = S_IFREG | 0644; + out.body.attr.attr.size = filesize / 2; + }))); + + buf = calloc(1, filesize); + fd = open(FULLPATH, O_RDWR); + ASSERT_LE(0, fd) << strerror(errno); + read(fd, buf, filesize); +} + +INSTANTIATE_TEST_CASE_P(BE, BmapEof, + Values(1, 2, 3) +);