From owner-svn-src-projects@freebsd.org Thu Jun 13 19:07:05 2019 Return-Path: Delivered-To: svn-src-projects@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id E49FE15BA3CB for ; Thu, 13 Jun 2019 19:07:04 +0000 (UTC) (envelope-from asomers@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) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 5CAF56AD73; Thu, 13 Jun 2019 19:07:04 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 371DE4B14; Thu, 13 Jun 2019 19:07:04 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x5DJ738i081773; Thu, 13 Jun 2019 19:07:03 GMT (envelope-from asomers@FreeBSD.org) Received: (from asomers@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x5DJ73WV081771; Thu, 13 Jun 2019 19:07:03 GMT (envelope-from asomers@FreeBSD.org) Message-Id: <201906131907.x5DJ73WV081771@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: asomers set sender to asomers@FreeBSD.org using -f From: Alan Somers Date: Thu, 13 Jun 2019 19:07:03 +0000 (UTC) To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r349021 - in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs X-SVN-Group: projects X-SVN-Commit-Author: asomers X-SVN-Commit-Paths: in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs X-SVN-Commit-Revision: 349021 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 5CAF56AD73 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.97 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_SHORT(-0.98)[-0.976,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US]; NEURAL_HAM_MEDIUM(-1.00)[-0.998,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0] X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Jun 2019 19:07:05 -0000 Author: asomers Date: Thu Jun 13 19:07:03 2019 New Revision: 349021 URL: https://svnweb.freebsd.org/changeset/base/349021 Log: fusefs: fix a bug with WriteBack cacheing An errant vfs_bio_clrbuf snuck in in r348931. Surprisingly, it doesn't have any effect most of the time. But under some circumstances it cause the buffer to behave in a write-only fashion. Sponsored by: The FreeBSD Foundation Modified: projects/fuse2/sys/fs/fuse/fuse_io.c projects/fuse2/tests/sys/fs/fusefs/io.cc Modified: projects/fuse2/sys/fs/fuse/fuse_io.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_io.c Thu Jun 13 19:04:49 2019 (r349020) +++ projects/fuse2/sys/fs/fuse/fuse_io.c Thu Jun 13 19:07:03 2019 (r349021) @@ -267,7 +267,7 @@ out: } SDT_PROBE_DEFINE4(fusefs, , io, read_bio_backend_start, "int", "int", "int", "int"); -SDT_PROBE_DEFINE2(fusefs, , io, read_bio_backend_feed, "int", "int"); +SDT_PROBE_DEFINE2(fusefs, , io, read_bio_backend_feed, "int", "struct buf*"); SDT_PROBE_DEFINE4(fusefs, , io, read_bio_backend_end, "int", "ssize_t", "int", "struct buf*"); static int @@ -330,8 +330,7 @@ fuse_read_biobackend(struct vnode *vp, struct uio *uio if (on < bcount) n = MIN((unsigned)(bcount - on), uio->uio_resid); if (n > 0) { - SDT_PROBE2(fusefs, , io, read_bio_backend_feed, - n, n + (int)bp->b_resid); + SDT_PROBE2(fusefs, , io, read_bio_backend_feed, n, bp); err = uiomove(bp->b_data + on, n, uio); } vfs_bio_brelse(bp, ioflag); @@ -344,8 +343,8 @@ fuse_read_biobackend(struct vnode *vp, struct uio *uio SDT_PROBE_DEFINE1(fusefs, , io, read_directbackend_start, "struct fuse_read_in*"); -SDT_PROBE_DEFINE2(fusefs, , io, read_directbackend_complete, - "struct fuse_dispatcher*", "struct uio*"); +SDT_PROBE_DEFINE3(fusefs, , io, read_directbackend_complete, + "struct fuse_dispatcher*", "struct fuse_read_in*", "struct uio*"); static int fuse_read_directbackend(struct vnode *vp, struct uio *uio, @@ -390,8 +389,8 @@ fuse_read_directbackend(struct vnode *vp, struct uio * if ((err = fdisp_wait_answ(&fdi))) goto out; - SDT_PROBE2(fusefs, , io, read_directbackend_complete, - fdi.iosize, uio); + SDT_PROBE3(fusefs, , io, read_directbackend_complete, + &fdi, fri, uio); if ((err = uiomove(fdi.answ, MIN(fri->size, fdi.iosize), uio))) break; @@ -555,6 +554,7 @@ retry: SDT_PROBE_DEFINE6(fusefs, , io, write_biobackend_start, "int64_t", "int", "int", "struct uio*", "int", "bool"); SDT_PROBE_DEFINE2(fusefs, , io, write_biobackend_append_race, "long", "int"); +SDT_PROBE_DEFINE2(fusefs, , io, write_biobackend_issue, "int", "struct buf*"); static int fuse_write_biobackend(struct vnode *vp, struct uio *uio, @@ -602,14 +602,14 @@ fuse_write_biobackend(struct vnode *vp, struct uio *ui again: /* Get or create a buffer for the write */ direct_append = uio->uio_offset == filesize && n; - if ((off_t)lbn * biosize + on + n < filesize) { + if (uio->uio_offset + n < filesize) { extending = false; if ((off_t)(lbn + 1) * biosize < filesize) { /* Not the file's last block */ bcount = biosize; } else { /* The file's last block */ - bcount = filesize - (off_t)lbn *biosize; + bcount = filesize - (off_t)lbn * biosize; } } else { extending = true; @@ -650,8 +650,6 @@ again: break; } } - if (biosize > bcount) - vfs_bio_clrbuf(bp); SDT_PROBE6(fusefs, , io, write_biobackend_start, lbn, on, n, uio, bcount, direct_append); @@ -733,6 +731,7 @@ again: * reasons: the only way to know if a write is valid * if its actually written out.) */ + SDT_PROBE2(fusefs, , io, write_biobackend_issue, 0, bp); bwrite(bp); if (bp->b_error == EINTR) { err = EINTR; @@ -780,23 +779,33 @@ again: * already-written page whenever extending a file with * ftruncate or another write. */ + SDT_PROBE2(fusefs, , io, write_biobackend_issue, 1, bp); err = bwrite(bp); } else if (ioflag & IO_SYNC) { + SDT_PROBE2(fusefs, , io, write_biobackend_issue, 2, bp); err = bwrite(bp); } else if (vm_page_count_severe() || buf_dirty_count_severe() || (ioflag & IO_ASYNC)) { /* TODO: enable write clustering later */ + SDT_PROBE2(fusefs, , io, write_biobackend_issue, 3, bp); bawrite(bp); } else if (on == 0 && n == bcount) { - if ((vp->v_mount->mnt_flag & MNT_NOCLUSTERW) == 0) + if ((vp->v_mount->mnt_flag & MNT_NOCLUSTERW) == 0) { + SDT_PROBE2(fusefs, , io, write_biobackend_issue, + 4, bp); bdwrite(bp); - else + } else { + SDT_PROBE2(fusefs, , io, write_biobackend_issue, + 5, bp); bawrite(bp); + } } else if (ioflag & IO_DIRECT) { + SDT_PROBE2(fusefs, , io, write_biobackend_issue, 6, bp); bawrite(bp); } else { bp->b_flags &= ~B_CLUSTEROK; + SDT_PROBE2(fusefs, , io, write_biobackend_issue, 7, bp); bdwrite(bp); } if (err) Modified: projects/fuse2/tests/sys/fs/fusefs/io.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/io.cc Thu Jun 13 19:04:49 2019 (r349020) +++ projects/fuse2/tests/sys/fs/fusefs/io.cc Thu Jun 13 19:07:03 2019 (r349021) @@ -51,6 +51,27 @@ const char FULLPATH[] = "mountpoint/some_file.txt"; const char RELPATH[] = "some_file.txt"; const uint64_t ino = 42; +static void compare(const void *tbuf, const void *controlbuf, off_t baseofs, + ssize_t size) +{ + int i; + + for (i = 0; i < size; i++) { + if (((const char*)tbuf)[i] != ((const char*)controlbuf)[i]) { + off_t ofs = baseofs + i; + FAIL() << "miscompare at offset " + << std::hex + << std::showbase + << ofs + << ". expected = " + << std::setw(2) + << (unsigned)((const uint8_t*)controlbuf)[i] + << " got = " + << (unsigned)((const uint8_t*)tbuf)[i]; + } + } +} + class Io: public FuseTest { public: int m_backing_fd, m_control_fd, m_test_fd; @@ -171,7 +192,7 @@ void do_read(ssize_t size, off_t offs) ASSERT_EQ(size, pread(m_control_fd, control_buf, size, offs)) << strerror(errno); - ASSERT_EQ(0, memcmp(test_buf, control_buf, size)); + compare(test_buf, control_buf, offs, size); free(control_buf); free(test_buf); @@ -306,5 +327,37 @@ TEST_F(Io, truncate_into_dirty_buffer2) do_read(rsize2, rofs2); /* Truncates the dirty buffer */ do_ftruncate(truncsize2); + close(m_test_fd); +} + +/* + * Regression test for a bug introduced in r348931 + * + * Sequence of operations: + * 1) The first write reads lbn so it can modify it + * 2) The first write flushes lbn 3 immediately because it's the end of file + * 3) The first write then flushes lbn 4 because it's the end of the file + * 4) The second write modifies the cached versions of lbn 3 and 4 + * 5) The third write's getblkx invalidates lbn 4's B_CACHE because it's + * extending the buffer. Then it flushes lbn 4 because B_DELWRI was set but + * B_CACHE was clear. + * 6) fuse_write_biobackend erroneously called vfs_bio_clrbuf, putting the + * buffer into a weird write-only state. All read operations would return + * 0. Writes were apparently still processed, because the buffer's contents + * were correct when examined in a core dump. + * 7) The third write reads lbn 4 because cache is clear + * 9) uiomove dutifully copies new data into the buffer + * 10) The buffer's dirty is flushed to lbn 4 + * 11) The read returns all zeros because of step 6. + * + * Based on: + * fsx -WR -l 524388 -o 131072 -P /tmp -S6456 -q fsx.bin + */ +TEST_F(Io, resize_a_valid_buffer_while_extending) +{ + do_write(0x14530, 0x36ee6); /* [0x36ee6, 0x4b415] */ + do_write(0x1507c, 0x33256); /* [0x33256, 0x482d1] */ + do_write(0x175c, 0x4c03d); /* [0x4c03d, 0x4d798] */ + do_read(0xe277, 0x3599c); /* [0x3599c, 0x43c12] */ close(m_test_fd); }