From owner-svn-src-projects@freebsd.org Mon Jun 17 23:34:13 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 6D66D15C7478 for ; Mon, 17 Jun 2019 23:34:13 +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 0B8D78744F; Mon, 17 Jun 2019 23:34:13 +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 EEF5E4EC1; Mon, 17 Jun 2019 23:34:12 +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 x5HNYC5s011508; Mon, 17 Jun 2019 23:34:12 GMT (envelope-from asomers@FreeBSD.org) Received: (from asomers@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x5HNYCZC011505; Mon, 17 Jun 2019 23:34:12 GMT (envelope-from asomers@FreeBSD.org) Message-Id: <201906172334.x5HNYCZC011505@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: asomers set sender to asomers@FreeBSD.org using -f From: Alan Somers Date: Mon, 17 Jun 2019 23:34:12 +0000 (UTC) To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r349162 - 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: 349162 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 0B8D78744F X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.99 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-1.00)[-0.999,0]; NEURAL_HAM_SHORT(-0.99)[-0.988,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US]; 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: Mon, 17 Jun 2019 23:34:13 -0000 Author: asomers Date: Mon Jun 17 23:34:11 2019 New Revision: 349162 URL: https://svnweb.freebsd.org/changeset/base/349162 Log: fusefs: multiple fixes related to the write cache * Don't always write the last page synchronously. That's not actually required. It was probably just masking another bug that I fixed later, possibly in r349021. * Enable the NotifyWriteback tests now that Writeback cache is working. * Add a test to ensure that the write cache isn't flushed synchronously when in writeback mode. Sponsored by: The FreeBSD Foundation Modified: projects/fuse2/sys/fs/fuse/fuse_io.c projects/fuse2/tests/sys/fs/fusefs/notify.cc projects/fuse2/tests/sys/fs/fusefs/write.cc Modified: projects/fuse2/sys/fs/fuse/fuse_io.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_io.c Mon Jun 17 23:03:30 2019 (r349161) +++ projects/fuse2/sys/fs/fuse/fuse_io.c Mon Jun 17 23:34:11 2019 (r349162) @@ -789,23 +789,7 @@ again: vfs_bio_set_flags(bp, ioflag); - if (last_page) { - /* - * When writing the last page of a file we must write - * synchronously. If we didn't, then a subsequent - * operation could extend the file, making the last - * page of this buffer invalid because it would only be - * partially cached. - * - * As an optimization, it would be allowable to only - * write the last page synchronously. Or, it should be - * possible to synchronously flush the last - * 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) { + if (ioflag & IO_SYNC) { SDT_PROBE2(fusefs, , io, write_biobackend_issue, 2, bp); err = bwrite(bp); } else if (vm_page_count_severe() || Modified: projects/fuse2/tests/sys/fs/fusefs/notify.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/notify.cc Mon Jun 17 23:03:30 2019 (r349161) +++ projects/fuse2/tests/sys/fs/fusefs/notify.cc Mon Jun 17 23:34:11 2019 (r349162) @@ -50,6 +50,18 @@ using namespace testing; class Notify: public FuseTest { public: +/* Ignore an optional FUSE_FSYNC */ +void maybe_expect_fsync(uint64_t ino) +{ + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in.header.opcode == FUSE_FSYNC && + in.header.nodeid == ino); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnErrno(0))); +} + void expect_lookup(uint64_t parent, const char *relpath, uint64_t ino, off_t size, Sequence &seq) { @@ -76,6 +88,7 @@ virtual void SetUp() { int val = 0; size_t size = sizeof(val); + m_async = true; Notify::SetUp(); if (IsSkipped()) return; @@ -363,8 +376,7 @@ TEST_F(Notify, inval_inode_with_clean_cache) /* Deliberately leak fd. close(2) will be tested in release.cc */ } -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=238312 */ -TEST_F(NotifyWriteback, DISABLED_inval_inode_with_dirty_cache) +TEST_F(NotifyWriteback, inval_inode_with_dirty_cache) { const static char FULLPATH[] = "mountpoint/foo"; const static char RELPATH[] = "foo"; @@ -384,8 +396,14 @@ TEST_F(NotifyWriteback, DISABLED_inval_inode_with_dirt fd = open(FULLPATH, O_RDWR); ASSERT_EQ(bufsize, write(fd, CONTENTS, bufsize)) << strerror(errno); - /* Evict the data cache */ expect_write(ino, 0, bufsize, CONTENTS); + /* + * The FUSE protocol does not require an fsync here, but FreeBSD's + * bufobj_invalbuf sends it anyway + */ + maybe_expect_fsync(ino); + + /* Evict the data cache */ iia.mock = m_mock; iia.ino = ino; iia.off = 0; @@ -398,8 +416,7 @@ TEST_F(NotifyWriteback, DISABLED_inval_inode_with_dirt /* Deliberately leak fd. close(2) will be tested in release.cc */ } -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=238312 */ -TEST_F(NotifyWriteback, DISABLED_inval_inode_attrs_only) +TEST_F(NotifyWriteback, inval_inode_attrs_only) { const static char FULLPATH[] = "mountpoint/foo"; const static char RELPATH[] = "foo"; @@ -425,7 +442,7 @@ TEST_F(NotifyWriteback, DISABLED_inval_inode_attrs_onl EXPECT_CALL(*m_mock, process( ResultOf([=](auto in) { return (in.header.opcode == FUSE_GETATTR && - in.header.nodeid == FUSE_ROOT_ID); + in.header.nodeid == ino); }, Eq(true)), _) ).WillOnce(Invoke(ReturnImmediate([=](auto i __unused, auto& out) { Modified: projects/fuse2/tests/sys/fs/fusefs/write.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/write.cc Mon Jun 17 23:03:30 2019 (r349161) +++ projects/fuse2/tests/sys/fs/fusefs/write.cc Mon Jun 17 23:34:11 2019 (r349162) @@ -90,6 +90,31 @@ void expect_write(uint64_t ino, uint64_t offset, uint6 FuseTest::expect_write(ino, offset, isize, osize, 0, 0, contents); } +/* Expect a write that may or may not come, depending on the cache mode */ +void maybe_expect_write(uint64_t ino, uint64_t offset, uint64_t size, + const void *contents) +{ + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + const char *buf = (const char*)in.body.bytes + + sizeof(struct fuse_write_in); + + return (in.header.opcode == FUSE_WRITE && + in.header.nodeid == ino && + in.body.write.offset == offset && + in.body.write.size == size && + 0 == bcmp(buf, contents, size)); + }, Eq(true)), + _) + ).Times(AtMost(1)) + .WillRepeatedly(Invoke( + ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, write); + out.body.write.size = size; + }) + )); +} + }; class WriteCacheable: public Write { @@ -196,6 +221,14 @@ void expect_write(uint64_t ino, uint64_t offset, uint6 } }; +class WriteBackAsync: public WriteBack { +public: +virtual void SetUp() { + m_async = true; + WriteBack::SetUp(); +} +}; + /* Tests for clustered writes with WriteBack cacheing */ class WriteCluster: public WriteBack { public: @@ -302,7 +335,7 @@ TEST_F(Write, append_to_cached) expect_lookup(RELPATH, ino, oldsize); expect_open(ino, 0, 1); expect_read(ino, 0, oldsize, oldsize, oldcontents); - expect_write(ino, oldsize, BUFSIZE, BUFSIZE, CONTENTS); + maybe_expect_write(ino, oldsize, BUFSIZE, CONTENTS); /* Must open O_RDWR or fuse(4) implicitly sets direct_io */ fd = open(FULLPATH, O_RDWR | O_APPEND); @@ -610,7 +643,7 @@ TEST_F(Write, write_large) expect_lookup(RELPATH, ino, 0); expect_open(ino, 0, 1); expect_write(ino, 0, halfbufsize, halfbufsize, contents); - expect_write(ino, halfbufsize, halfbufsize, halfbufsize, + maybe_expect_write(ino, halfbufsize, halfbufsize, &contents[halfbufsize / sizeof(int)]); fd = open(FULLPATH, O_WRONLY); @@ -662,7 +695,7 @@ TEST_F(Write_7_8, write) } /* In writeback mode, dirty data should be written on close */ -TEST_F(WriteBack, close) +TEST_F(WriteBackAsync, close) { const char FULLPATH[] = "mountpoint/some_file.txt"; const char RELPATH[] = "some_file.txt"; @@ -795,7 +828,7 @@ TEST_F(WriteBack, rmw) FuseTest::expect_lookup(RELPATH, ino, S_IFREG | 0644, fsize, 1); expect_open(ino, 0, 1); expect_read(ino, 0, fsize, fsize, INITIAL, O_WRONLY); - expect_write(ino, offset, bufsize, bufsize, CONTENTS); + maybe_expect_write(ino, offset, bufsize, CONTENTS); fd = open(FULLPATH, O_WRONLY); EXPECT_LE(0, fd) << strerror(errno); @@ -808,7 +841,7 @@ TEST_F(WriteBack, rmw) /* * Without direct_io, writes should be committed to cache */ -TEST_F(WriteBack, writeback) +TEST_F(WriteBack, cache) { const char FULLPATH[] = "mountpoint/some_file.txt"; const char RELPATH[] = "some_file.txt"; @@ -865,6 +898,36 @@ TEST_F(WriteBack, o_direct) ASSERT_EQ(0, fcntl(fd, F_SETFL, 0)) << strerror(errno); ASSERT_EQ(bufsize, read(fd, readbuf, bufsize)) << strerror(errno); /* Deliberately leak fd. close(2) will be tested in release.cc */ +} + +/* + * When mounted with -o async, the writeback cache mode should delay writes + */ +TEST_F(WriteBackAsync, delay) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + const char *CONTENTS = "abcdefgh"; + uint64_t ino = 42; + int fd; + ssize_t bufsize = strlen(CONTENTS); + + expect_lookup(RELPATH, ino, 0); + expect_open(ino, 0, 1); + /* Write should be cached, but FUSE_WRITE shouldn't be sent */ + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in.header.opcode == FUSE_WRITE); + }, Eq(true)), + _) + ).Times(0); + + fd = open(FULLPATH, O_RDWR); + EXPECT_LE(0, fd) << strerror(errno); + + ASSERT_EQ(bufsize, write(fd, CONTENTS, bufsize)) << strerror(errno); + + /* Don't close the file because that would flush the cache */ } /*