Date: Mon, 28 Dec 2020 18:57:18 GMT From: Alan Somers <asomers@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: f928dbcb167c - main - fusefs: fix the tests for a wider range of maxphys Message-ID: <202012281857.0BSIvI0w008482@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by asomers: URL: https://cgit.FreeBSD.org/src/commit/?id=f928dbcb167c7440212e420687de813fc92c06a4 commit f928dbcb167c7440212e420687de813fc92c06a4 Author: Alan Somers <asomers@FreeBSD.org> AuthorDate: 2020-12-24 06:03:06 +0000 Commit: Alan Somers <asomers@FreeBSD.org> CommitDate: 2020-12-28 18:56:17 +0000 fusefs: fix the tests for a wider range of maxphys maxphys is now a tunable, ever since r368124. The default value is also larger than it used to be. That broke several fusefs tests that made assumptions about maxphys. * WriteCluster.clustering used the MAXPHYS compile-time constant. * WriteBackAsync.direct_io_partially_overlaps_cached_block implicitly depended on the default value of maxphys. Fix it by making the dependency explicit. * Write.write_large implicitly assumed that maxphys would be no more than twice maxbcachebuf. Fix it by explicitly setting m_max_write. * WriteCluster.clustering and several others failed because the MockFS module did not work for max_write > 128KB (which most tests would set when maxphys > 256KB). Limit max_write accordingly. This is the same as fusefs-libs's behavior. * Bmap's tests were originally written for MAXPHYS=128KB. With larger values, the simulated file size was too small. PR: 252096 Reviewed by: emaste Differential Revision: https://reviews.freebsd.org/D27769 --- tests/sys/fs/fusefs/bmap.cc | 12 ++++++++---- tests/sys/fs/fusefs/mockfs.cc | 2 +- tests/sys/fs/fusefs/mockfs.hh | 21 +++++++++++++++++++-- tests/sys/fs/fusefs/utils.cc | 13 ++++++------- tests/sys/fs/fusefs/utils.hh | 3 +-- tests/sys/fs/fusefs/write.cc | 18 +++++++++++++++++- 6 files changed, 52 insertions(+), 17 deletions(-) diff --git a/tests/sys/fs/fusefs/bmap.cc b/tests/sys/fs/fusefs/bmap.cc index b41aeb4f08dd..3f392a4447f2 100644 --- a/tests/sys/fs/fusefs/bmap.cc +++ b/tests/sys/fs/fusefs/bmap.cc @@ -83,10 +83,14 @@ void expect_lookup(const char *relpath, uint64_t ino, off_t size) TEST_F(Bmap, bmap) { struct fiobmap2_arg arg; - const off_t filesize = 1 << 20; - const ino_t ino = 42; - int64_t lbn = 10; + /* + * Pick fsize and lbn large enough that max length runs won't reach + * either beginning or end of file + */ + const off_t filesize = 1 << 30; + int64_t lbn = 100; int64_t pbn = 12345; + const ino_t ino = 42; int fd; expect_lookup(RELPATH, 42, filesize); @@ -112,7 +116,7 @@ TEST_F(Bmap, bmap) TEST_F(Bmap, default_) { struct fiobmap2_arg arg; - const off_t filesize = 1 << 20; + const off_t filesize = 1 << 30; const ino_t ino = 42; int64_t lbn; int fd; diff --git a/tests/sys/fs/fusefs/mockfs.cc b/tests/sys/fs/fusefs/mockfs.cc index f43afd247502..f977c705331f 100644 --- a/tests/sys/fs/fusefs/mockfs.cc +++ b/tests/sys/fs/fusefs/mockfs.cc @@ -365,7 +365,7 @@ MockFS::MockFS(int max_readahead, bool allow_other, bool default_permissions, m_daemon_id = NULL; m_kernel_minor_version = kernel_minor_version; m_maxreadahead = max_readahead; - m_maxwrite = max_write; + m_maxwrite = MIN(max_write, max_max_write); m_nready = -1; m_pm = pm; m_time_gran = time_gran; diff --git a/tests/sys/fs/fusefs/mockfs.hh b/tests/sys/fs/fusefs/mockfs.hh index 4cf6daea7e76..138c125649fd 100644 --- a/tests/sys/fs/fusefs/mockfs.hh +++ b/tests/sys/fs/fusefs/mockfs.hh @@ -72,6 +72,14 @@ extern "C" { extern int verbosity; +/* + * The maximum that a test case can set max_write, limited by the buffer + * supplied when reading from /dev/fuse. This limitation is imposed by + * fusefs-libs, but not by the FUSE protocol. + */ +const uint32_t max_max_write = 0x20000; + + /* This struct isn't defined by fuse_kernel.h or libfuse, but it should be */ struct fuse_create_out { struct fuse_entry_out entry; @@ -138,8 +146,17 @@ struct fuse_init_out_7_22 { union fuse_payloads_in { fuse_access_in access; fuse_bmap_in bmap; - /* value is from fuse_kern_chan.c in fusefs-libs */ - uint8_t bytes[0x21000 - sizeof(struct fuse_in_header)]; + /* + * In fusefs-libs 3.4.2 and below the buffer size is fixed at 0x21000 + * minus the header sizes. fusefs-libs 3.4.3 (and FUSE Protocol 7.29) + * add a FUSE_MAX_PAGES option that allows it to be greater. + * + * See fuse_kern_chan.c in fusefs-libs 2.9.9 and below, or + * FUSE_DEFAULT_MAX_PAGES_PER_REQ in fusefs-libs 3.4.3 and above. + */ + uint8_t bytes[ + max_max_write + 0x1000 - sizeof(struct fuse_in_header) + ]; fuse_create_in create; fuse_flush_in flush; fuse_fsync_in fsync; diff --git a/tests/sys/fs/fusefs/utils.cc b/tests/sys/fs/fusefs/utils.cc index 6e24d9c7b62d..7e9b97c2f228 100644 --- a/tests/sys/fs/fusefs/utils.cc +++ b/tests/sys/fs/fusefs/utils.cc @@ -59,13 +59,6 @@ using namespace testing; */ const uint32_t libfuse_max_write = 32 * getpagesize() + 0x1000 - 4096; -/* - * Set the default max_write to a distinct value from MAXPHYS to catch bugs - * that confuse the two. - */ -const uint32_t default_max_write = MIN(libfuse_max_write, MAXPHYS / 2); - - /* Check that fusefs(4) is accessible and the current user can mount(2) */ void check_environment() { @@ -156,6 +149,12 @@ void FuseTest::SetUp() { ASSERT_EQ(0, sysctlbyname(maxphys_node, &val, &size, NULL, 0)) << strerror(errno); m_maxphys = val; + /* + * Set the default max_write to a distinct value from MAXPHYS to catch + * bugs that confuse the two. + */ + if (m_maxwrite == 0) + m_maxwrite = MIN(libfuse_max_write, (uint32_t)m_maxphys / 2); try { m_mock = new MockFS(m_maxreadahead, m_allow_other, diff --git a/tests/sys/fs/fusefs/utils.hh b/tests/sys/fs/fusefs/utils.hh index 7ec80258ee4e..5d5c5290a60d 100644 --- a/tests/sys/fs/fusefs/utils.hh +++ b/tests/sys/fs/fusefs/utils.hh @@ -55,7 +55,6 @@ const char *cache_mode_to_s(enum cache_mode cm); bool is_unsafe_aio_enabled(void); extern const uint32_t libfuse_max_write; -extern const uint32_t default_max_write; class FuseTest : public ::testing::Test { protected: uint32_t m_maxreadahead; @@ -80,7 +79,7 @@ class FuseTest : public ::testing::Test { FuseTest(): m_maxreadahead(0), - m_maxwrite(default_max_write), + m_maxwrite(0), m_init_flags(0), m_allow_other(false), m_default_permissions(false), diff --git a/tests/sys/fs/fusefs/write.cc b/tests/sys/fs/fusefs/write.cc index e9bf17c5c4d6..78d82805470f 100644 --- a/tests/sys/fs/fusefs/write.cc +++ b/tests/sys/fs/fusefs/write.cc @@ -166,6 +166,7 @@ class WriteBackAsync: public WriteBack { public: virtual void SetUp() { m_async = true; + m_maxwrite = 65536; WriteBack::SetUp(); } }; @@ -194,6 +195,19 @@ virtual void SetUp() { } }; +/* Tests relating to the server's max_write property */ +class WriteMaxWrite: public Write { +public: +virtual void SetUp() { + /* + * For this test, m_maxwrite must be less than either m_maxbcachebuf or + * maxphys. + */ + m_maxwrite = 32768; + Write::SetUp(); +} +}; + void sigxfsz_handler(int __unused sig) { Write::s_sigxfsz = 1; } @@ -643,7 +657,7 @@ TEST_F(Write, write) } /* fuse(4) should not issue writes of greater size than the daemon requests */ -TEST_F(Write, write_large) +TEST_F(WriteMaxWrite, write) { const char FULLPATH[] = "mountpoint/some_file.txt"; const char RELPATH[] = "some_file.txt"; @@ -653,6 +667,8 @@ TEST_F(Write, write_large) ssize_t halfbufsize, bufsize; halfbufsize = m_mock->m_maxwrite; + if (halfbufsize >= m_maxbcachebuf || halfbufsize >= m_maxphys) + GTEST_SKIP() << "Must lower m_maxwrite for this test"; bufsize = halfbufsize * 2; contents = (int*)malloc(bufsize); ASSERT_NE(nullptr, contents);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202012281857.0BSIvI0w008482>