From nobody Tue Dec 14 22:24:29 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 490DE18F4E0B; Tue, 14 Dec 2021 22:24:30 +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 4JDCbd5szCz4b71; Tue, 14 Dec 2021 22:24:29 +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 8F6B210CCC; Tue, 14 Dec 2021 22:24:29 +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 1BEMOT1A050781; Tue, 14 Dec 2021 22:24:29 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1BEMOT4s050780; Tue, 14 Dec 2021 22:24:29 GMT (envelope-from git) Date: Tue, 14 Dec 2021 22:24:29 GMT Message-Id: <202112142224.1BEMOT4s050780@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: 0bade34633f9 - stable/13 - fusefs: update atime on reads when using cached attributes 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: 0bade34633f997c22f5e4e0931df0d534f560a38 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1639520670; 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=vawN64+uZAGUBx2hHiIjZrVMUSZh6zMcOUHrfekx5iE=; b=FFYxmP20Dq8EUIdfjMC861llEOSByfBNjWYy43Dtb5pDCG2ZWXo9RFyNC/4/u6f01A3HRA DXZ++4gGc1L6qX3IMd0erGNHOo2VH+1VbRUJrHHsubcrqB82ZaL9xqQqUIcQWuTqZL2MtP QBOQ76vQFFu3ufe9TP+dkU+ItZxNGDOMhJmW07ixjfVqsmWa+Z0AwmgvKAFxoxOTsiBuWa +L1s8Qpt1DrU+G5n0+vZLQV6hQFHR20dzKFolLR8QmXpqvDhTHC3eDDtrEgGoYrIFtN0sH EvR2LR/EuJYKTDZV8dmopshr13Dht8Yi8zMYxXKMX+Ne6Aes2/AgsTPS6o3tLA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1639520670; a=rsa-sha256; cv=none; b=XjuuiHxJZloVOE0GqySkSkk/Xi76/v6wlB67eAggae6R6lLesX2rBJqKdyjzoTN6j0Efgi FcG66YCJWGaQ7o07aT3JlxtwsRsqm4KxjfcnVO3Q3JYrfbUSir7C6Q6fYseWujbZtA0mI+ zv9dfw+UBpg6mWNaupaI3H1RoIfitjukOdU49/ZYiO5K5D3cDOoNQmNyJ3+lYq5G0typCJ pIZH2PfqSbJDjOipuSqZW6YU/Vj2ErxQJYY53+1zKr6ZBBCD0zmTMp53BNA+4sBkr6Vy/H RNPU1vkODSeRxkhlLymkJwJhImZsi69Sl6p0zgMOU0vswYtvz56RstXjMSEChQ== 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=0bade34633f997c22f5e4e0931df0d534f560a38 commit 0bade34633f997c22f5e4e0931df0d534f560a38 Author: Alan Somers AuthorDate: 2021-11-29 01:53:31 +0000 Commit: Alan Somers CommitDate: 2021-12-14 22:15:53 +0000 fusefs: update atime on reads when using cached attributes When using cached attributes, whether or not the data cache is enabled, fusefs must update a file's atime whenever it reads from it, so long as it wasn't mounted with -o noatime. Update it in-kernel, and flush it to the server on close or during the next setattr operation. The downside is that close() will now frequently trigger a FUSE_SETATTR upcall. But if you care about performance, you should be using -o noatime anyway. Reviewed by: pfg Differential Revision: https://reviews.freebsd.org/D33145 (cherry picked from commit 91972cfcddf950d7a9c33df5a9171ada1805a144) fusefs: fix 32-bit build of the tests after 91972cfcddf (cherry picked from commit d109559ddbf7afe311c1f1795ece137071406db8) --- sys/fs/fuse/fuse_internal.c | 11 +- sys/fs/fuse/fuse_io.c | 3 +- sys/fs/fuse/fuse_node.c | 12 +- sys/fs/fuse/fuse_node.h | 3 +- sys/fs/fuse/fuse_vnops.c | 10 +- tests/sys/fs/fusefs/cache.cc | 1 + tests/sys/fs/fusefs/io.cc | 1 + tests/sys/fs/fusefs/mockfs.cc | 6 +- tests/sys/fs/fusefs/mockfs.hh | 3 +- tests/sys/fs/fusefs/read.cc | 296 ++++++++++++++++++++++++++++++++++++++++++ tests/sys/fs/fusefs/utils.cc | 2 +- tests/sys/fs/fusefs/utils.hh | 2 + 12 files changed, 341 insertions(+), 9 deletions(-) diff --git a/sys/fs/fuse/fuse_internal.c b/sys/fs/fuse/fuse_internal.c index ffa4a40095b2..403116593bd9 100644 --- a/sys/fs/fuse/fuse_internal.c +++ b/sys/fs/fuse/fuse_internal.c @@ -926,6 +926,7 @@ fuse_internal_do_getattr(struct vnode *vp, struct vattr *vap, struct fuse_getattr_in *fgai; struct fuse_attr_out *fao; off_t old_filesize = fvdat->cached_attrs.va_size; + struct timespec old_atime = fvdat->cached_attrs.va_atime; struct timespec old_ctime = fvdat->cached_attrs.va_ctime; struct timespec old_mtime = fvdat->cached_attrs.va_mtime; enum vtype vtyp; @@ -950,6 +951,10 @@ fuse_internal_do_getattr(struct vnode *vp, struct vattr *vap, vtyp = IFTOVT(fao->attr.mode); if (fvdat->flag & FN_SIZECHANGE) fao->attr.size = old_filesize; + if (fvdat->flag & FN_ATIMECHANGE) { + fao->attr.atime = old_atime.tv_sec; + fao->attr.atimensec = old_atime.tv_nsec; + } if (fvdat->flag & FN_CTIMECHANGE) { fao->attr.ctime = old_ctime.tv_sec; fao->attr.ctimensec = old_ctime.tv_nsec; @@ -1209,6 +1214,10 @@ int fuse_internal_setattr(struct vnode *vp, struct vattr *vap, fsai->valid |= FATTR_ATIME; if (vap->va_vaflags & VA_UTIMES_NULL) fsai->valid |= FATTR_ATIME_NOW; + } else if (fvdat->flag & FN_ATIMECHANGE) { + fsai->atime = fvdat->cached_attrs.va_atime.tv_sec; + fsai->atimensec = fvdat->cached_attrs.va_atime.tv_nsec; + fsai->valid |= FATTR_ATIME; } if (vap->va_mtime.tv_sec != VNOVAL) { fsai->mtime = vap->va_mtime.tv_sec; @@ -1257,7 +1266,7 @@ int fuse_internal_setattr(struct vnode *vp, struct vattr *vap, } if (err == 0) { struct fuse_attr_out *fao = (struct fuse_attr_out*)fdi.answ; - fuse_vnode_undirty_cached_timestamps(vp); + fuse_vnode_undirty_cached_timestamps(vp, true); fuse_internal_cache_attrs(vp, &fao->attr, fao->attr_valid, fao->attr_valid_nsec, NULL, false); } diff --git a/sys/fs/fuse/fuse_io.c b/sys/fs/fuse/fuse_io.c index a240217fcc08..78398a990d7d 100644 --- a/sys/fs/fuse/fuse_io.c +++ b/sys/fs/fuse/fuse_io.c @@ -236,6 +236,7 @@ fuse_io_dispatch(struct vnode *vp, struct uio *uio, int ioflag, switch (uio->uio_rw) { case UIO_READ: + fuse_vnode_update(vp, FN_ATIMECHANGE); if (directio) { SDT_PROBE2(fusefs, , io, trace, 1, "direct read of vnode"); @@ -616,7 +617,7 @@ retry: fdisp_destroy(&fdi); if (wrote_anything) - fuse_vnode_undirty_cached_timestamps(vp); + fuse_vnode_undirty_cached_timestamps(vp, false); return (err); } diff --git a/sys/fs/fuse/fuse_node.c b/sys/fs/fuse/fuse_node.c index c8982c5a48a1..f74de2faa702 100644 --- a/sys/fs/fuse/fuse_node.c +++ b/sys/fs/fuse/fuse_node.c @@ -474,11 +474,13 @@ fuse_vnode_size(struct vnode *vp, off_t *filesize, struct ucred *cred, } void -fuse_vnode_undirty_cached_timestamps(struct vnode *vp) +fuse_vnode_undirty_cached_timestamps(struct vnode *vp, bool atime) { struct fuse_vnode_data *fvdat = VTOFUD(vp); fvdat->flag &= ~(FN_MTIMECHANGE | FN_CTIMECHANGE); + if (atime) + fvdat->flag &= ~FN_ATIMECHANGE; } /* Update a fuse file's cached timestamps */ @@ -486,7 +488,8 @@ void fuse_vnode_update(struct vnode *vp, int flags) { struct fuse_vnode_data *fvdat = VTOFUD(vp); - struct fuse_data *data = fuse_get_mpdata(vnode_mount(vp)); + struct mount *mp = vnode_mount(vp); + struct fuse_data *data = fuse_get_mpdata(mp); struct timespec ts; vfs_timestamp(&ts); @@ -494,6 +497,11 @@ fuse_vnode_update(struct vnode *vp, int flags) if (data->time_gran > 1) ts.tv_nsec = rounddown(ts.tv_nsec, data->time_gran); + if (mp->mnt_flag & MNT_NOATIME) + flags &= ~FN_ATIMECHANGE; + + if (flags & FN_ATIMECHANGE) + fvdat->cached_attrs.va_atime = ts; if (flags & FN_MTIMECHANGE) fvdat->cached_attrs.va_mtime = ts; if (flags & FN_CTIMECHANGE) diff --git a/sys/fs/fuse/fuse_node.h b/sys/fs/fuse/fuse_node.h index 8f69ef9a08f3..3a33c57001dc 100644 --- a/sys/fs/fuse/fuse_node.h +++ b/sys/fs/fuse/fuse_node.h @@ -91,6 +91,7 @@ */ #define FN_MTIMECHANGE 0x00000800 #define FN_CTIMECHANGE 0x00001000 +#define FN_ATIMECHANGE 0x00002000 struct fuse_vnode_data { /** self **/ @@ -201,7 +202,7 @@ int fuse_vnode_savesize(struct vnode *vp, struct ucred *cred, pid_t pid); int fuse_vnode_setsize(struct vnode *vp, off_t newsize, bool from_server); -void fuse_vnode_undirty_cached_timestamps(struct vnode *vp); +void fuse_vnode_undirty_cached_timestamps(struct vnode *vp, bool atime); void fuse_vnode_update(struct vnode *vp, int flags); diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c index 5630eeeb9e65..d0f4cae302c1 100644 --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -613,6 +613,7 @@ fuse_vnop_close(struct vop_close_args *ap) int fflag = ap->a_fflag; struct thread *td = ap->a_td; pid_t pid = td->td_proc->p_pid; + struct fuse_vnode_data *fvdat = VTOFUD(vp); int err = 0; if (fuse_isdeadfs(vp)) @@ -623,8 +624,15 @@ fuse_vnop_close(struct vop_close_args *ap) return 0; err = fuse_flush(vp, cred, pid, fflag); + if (err == 0 && (fvdat->flag & FN_ATIMECHANGE)) { + struct vattr vap; + + VATTR_NULL(&vap); + vap.va_atime = fvdat->cached_attrs.va_atime; + err = fuse_internal_setattr(vp, &vap, td, NULL); + } /* TODO: close the file handle, if we're sure it's no longer used */ - if ((VTOFUD(vp)->flag & FN_SIZECHANGE) != 0) { + if ((fvdat->flag & FN_SIZECHANGE) != 0) { fuse_vnode_savesize(vp, cred, td->td_proc->p_pid); } return err; diff --git a/tests/sys/fs/fusefs/cache.cc b/tests/sys/fs/fusefs/cache.cc index ac62147f15a8..4df262cecd0f 100644 --- a/tests/sys/fs/fusefs/cache.cc +++ b/tests/sys/fs/fusefs/cache.cc @@ -75,6 +75,7 @@ virtual void SetUp() { default: FAIL() << "Unknown cache mode"; } + m_noatime = true; // To prevent SETATTR for atime on close FuseTest::SetUp(); if (IsSkipped()) diff --git a/tests/sys/fs/fusefs/io.cc b/tests/sys/fs/fusefs/io.cc index 65ba1ea19bc3..1502bd263f51 100644 --- a/tests/sys/fs/fusefs/io.cc +++ b/tests/sys/fs/fusefs/io.cc @@ -114,6 +114,7 @@ void SetUp() default: FAIL() << "Unknown cache mode"; } + m_noatime = true; // To prevent SETATTR for atime on close FuseTest::SetUp(); if (IsSkipped()) diff --git a/tests/sys/fs/fusefs/mockfs.cc b/tests/sys/fs/fusefs/mockfs.cc index 99f7ccc61273..8a2d1f910867 100644 --- a/tests/sys/fs/fusefs/mockfs.cc +++ b/tests/sys/fs/fusefs/mockfs.cc @@ -392,7 +392,7 @@ void MockFS::debug_response(const mockfs_buf_out &out) { MockFS::MockFS(int max_readahead, bool allow_other, bool default_permissions, bool push_symlinks_in, bool ro, enum poll_method pm, uint32_t flags, uint32_t kernel_minor_version, uint32_t max_write, bool async, - bool noclusterr, unsigned time_gran, bool nointr) + bool noclusterr, unsigned time_gran, bool nointr, bool noatime) { struct sigaction sa; struct iovec *iov = NULL; @@ -467,6 +467,10 @@ MockFS::MockFS(int max_readahead, bool allow_other, bool default_permissions, build_iovec(&iov, &iovlen, "async", __DECONST(void*, &trueval), sizeof(bool)); } + if (noatime) { + build_iovec(&iov, &iovlen, "noatime", + __DECONST(void*, &trueval), sizeof(bool)); + } if (noclusterr) { build_iovec(&iov, &iovlen, "noclusterr", __DECONST(void*, &trueval), sizeof(bool)); diff --git a/tests/sys/fs/fusefs/mockfs.hh b/tests/sys/fs/fusefs/mockfs.hh index 600a4b4292c0..dd6d259ca5af 100644 --- a/tests/sys/fs/fusefs/mockfs.hh +++ b/tests/sys/fs/fusefs/mockfs.hh @@ -356,7 +356,8 @@ class MockFS { bool default_permissions, bool push_symlinks_in, bool ro, enum poll_method pm, uint32_t flags, uint32_t kernel_minor_version, uint32_t max_write, bool async, - bool no_clusterr, unsigned time_gran, bool nointr); + bool no_clusterr, unsigned time_gran, bool nointr, + bool noatime); virtual ~MockFS(); diff --git a/tests/sys/fs/fusefs/read.cc b/tests/sys/fs/fusefs/read.cc index 3cba564affcf..0888fbc913e8 100644 --- a/tests/sys/fs/fusefs/read.cc +++ b/tests/sys/fs/fusefs/read.cc @@ -105,6 +105,13 @@ class ReadAhead: public Read, } }; +class ReadNoatime: public Read { + virtual void SetUp() { + m_noatime = true; + Read::SetUp(); + } +}; + class ReadSigbus: public Read { public: @@ -132,6 +139,14 @@ handle_sigbus(int signo __unused, siginfo_t *info, void *uap __unused) { jmp_buf ReadSigbus::s_jmpbuf; void *ReadSigbus::s_si_addr; +class TimeGran: public Read, public WithParamInterface { +public: +virtual void SetUp() { + m_time_gran = 1 << GetParam(); + Read::SetUp(); +} +}; + /* AIO reads need to set the header's pid field correctly */ /* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236379 */ TEST_F(AioRead, aio_read) @@ -323,6 +338,172 @@ TEST_F(AsyncRead, async_read) leak(fd); } +/* The kernel should update the cached atime attribute during a read */ +TEST_F(Read, atime) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + const char *CONTENTS = "abcdefgh"; + struct stat sb1, sb2; + uint64_t ino = 42; + int fd; + ssize_t bufsize = strlen(CONTENTS); + uint8_t buf[bufsize]; + + expect_lookup(RELPATH, ino, bufsize); + expect_open(ino, 0, 1); + expect_read(ino, 0, bufsize, bufsize, CONTENTS); + + fd = open(FULLPATH, O_RDONLY); + ASSERT_LE(0, fd) << strerror(errno); + ASSERT_EQ(0, fstat(fd, &sb1)); + + /* Ensure atime will be different than it was during lookup */ + nap(); + + ASSERT_EQ(bufsize, read(fd, buf, bufsize)) << strerror(errno); + ASSERT_EQ(0, fstat(fd, &sb2)); + + /* The kernel should automatically update atime during read */ + EXPECT_TRUE(timespeccmp(&sb1.st_atim, &sb2.st_atim, <)); + EXPECT_TRUE(timespeccmp(&sb1.st_ctim, &sb2.st_ctim, ==)); + EXPECT_TRUE(timespeccmp(&sb1.st_mtim, &sb2.st_mtim, ==)); + + leak(fd); +} + +/* The kernel should update the cached atime attribute during a cached read */ +TEST_F(Read, atime_cached) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + const char *CONTENTS = "abcdefgh"; + struct stat sb1, sb2; + uint64_t ino = 42; + int fd; + ssize_t bufsize = strlen(CONTENTS); + uint8_t buf[bufsize]; + + expect_lookup(RELPATH, ino, bufsize); + expect_open(ino, 0, 1); + expect_read(ino, 0, bufsize, bufsize, CONTENTS); + + fd = open(FULLPATH, O_RDONLY); + ASSERT_LE(0, fd) << strerror(errno); + + ASSERT_EQ(bufsize, pread(fd, buf, bufsize, 0)) << strerror(errno); + ASSERT_EQ(0, fstat(fd, &sb1)); + + /* Ensure atime will be different than it was during the first read */ + nap(); + + ASSERT_EQ(bufsize, pread(fd, buf, bufsize, 0)) << strerror(errno); + ASSERT_EQ(0, fstat(fd, &sb2)); + + /* The kernel should automatically update atime during read */ + EXPECT_TRUE(timespeccmp(&sb1.st_atim, &sb2.st_atim, <)); + EXPECT_TRUE(timespeccmp(&sb1.st_ctim, &sb2.st_ctim, ==)); + EXPECT_TRUE(timespeccmp(&sb1.st_mtim, &sb2.st_mtim, ==)); + + leak(fd); +} + +/* dirty atime values should be flushed during close */ +TEST_F(Read, atime_during_close) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + const char *CONTENTS = "abcdefgh"; + struct stat sb; + uint64_t ino = 42; + const mode_t newmode = 0755; + int fd; + ssize_t bufsize = strlen(CONTENTS); + uint8_t buf[bufsize]; + + expect_lookup(RELPATH, ino, bufsize); + expect_open(ino, 0, 1); + expect_read(ino, 0, bufsize, bufsize, CONTENTS); + EXPECT_CALL(*m_mock, process( + ResultOf([&](auto in) { + uint32_t valid = FATTR_ATIME; + return (in.header.opcode == FUSE_SETATTR && + in.header.nodeid == ino && + in.body.setattr.valid == valid && + (time_t)in.body.setattr.atime == + sb.st_atim.tv_sec && + (long)in.body.setattr.atimensec == + sb.st_atim.tv_nsec); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, attr); + out.body.attr.attr.ino = ino; + out.body.attr.attr.mode = S_IFREG | newmode; + }))); + expect_flush(ino, 1, ReturnErrno(0)); + expect_release(ino, FuseTest::FH); + + fd = open(FULLPATH, O_RDONLY); + ASSERT_LE(0, fd) << strerror(errno); + + /* Ensure atime will be different than during lookup */ + nap(); + + ASSERT_EQ(bufsize, read(fd, buf, bufsize)) << strerror(errno); + ASSERT_EQ(0, fstat(fd, &sb)); + + close(fd); +} + +/* A cached atime should be flushed during FUSE_SETATTR */ +TEST_F(Read, atime_during_setattr) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + const char *CONTENTS = "abcdefgh"; + struct stat sb; + uint64_t ino = 42; + const mode_t newmode = 0755; + int fd; + ssize_t bufsize = strlen(CONTENTS); + uint8_t buf[bufsize]; + + expect_lookup(RELPATH, ino, bufsize); + expect_open(ino, 0, 1); + expect_read(ino, 0, bufsize, bufsize, CONTENTS); + EXPECT_CALL(*m_mock, process( + ResultOf([&](auto in) { + uint32_t valid = FATTR_MODE | FATTR_ATIME; + return (in.header.opcode == FUSE_SETATTR && + in.header.nodeid == ino && + in.body.setattr.valid == valid && + (time_t)in.body.setattr.atime == + sb.st_atim.tv_sec && + (long)in.body.setattr.atimensec == + sb.st_atim.tv_nsec); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, attr); + out.body.attr.attr.ino = ino; + out.body.attr.attr.mode = S_IFREG | newmode; + }))); + + fd = open(FULLPATH, O_RDONLY); + ASSERT_LE(0, fd) << strerror(errno); + + /* Ensure atime will be different than during lookup */ + nap(); + + ASSERT_EQ(bufsize, read(fd, buf, bufsize)) << strerror(errno); + ASSERT_EQ(0, fstat(fd, &sb)); + ASSERT_EQ(0, fchmod(fd, newmode)) << strerror(errno); + + leak(fd); +} + +/* The kernel should flush dirty atime values during close */ /* 0-length reads shouldn't cause any confusion */ TEST_F(Read, direct_io_read_nothing) { @@ -613,6 +794,80 @@ TEST_F(Read, mmap) leak(fd); } +/* + * The kernel should not update the cached atime attribute during a read, if + * MNT_NOATIME is used. + */ +TEST_F(ReadNoatime, atime) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + const char *CONTENTS = "abcdefgh"; + struct stat sb1, sb2; + uint64_t ino = 42; + int fd; + ssize_t bufsize = strlen(CONTENTS); + uint8_t buf[bufsize]; + + expect_lookup(RELPATH, ino, bufsize); + expect_open(ino, 0, 1); + expect_read(ino, 0, bufsize, bufsize, CONTENTS); + + fd = open(FULLPATH, O_RDONLY); + ASSERT_LE(0, fd) << strerror(errno); + ASSERT_EQ(0, fstat(fd, &sb1)); + + nap(); + + ASSERT_EQ(bufsize, read(fd, buf, bufsize)) << strerror(errno); + ASSERT_EQ(0, fstat(fd, &sb2)); + + /* The kernel should not update atime during read */ + EXPECT_TRUE(timespeccmp(&sb1.st_atim, &sb2.st_atim, ==)); + EXPECT_TRUE(timespeccmp(&sb1.st_ctim, &sb2.st_ctim, ==)); + EXPECT_TRUE(timespeccmp(&sb1.st_mtim, &sb2.st_mtim, ==)); + + leak(fd); +} + +/* + * The kernel should not update the cached atime attribute during a cached + * read, if MNT_NOATIME is used. + */ +TEST_F(ReadNoatime, atime_cached) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + const char *CONTENTS = "abcdefgh"; + struct stat sb1, sb2; + uint64_t ino = 42; + int fd; + ssize_t bufsize = strlen(CONTENTS); + uint8_t buf[bufsize]; + + expect_lookup(RELPATH, ino, bufsize); + expect_open(ino, 0, 1); + expect_read(ino, 0, bufsize, bufsize, CONTENTS); + + fd = open(FULLPATH, O_RDONLY); + ASSERT_LE(0, fd) << strerror(errno); + + ASSERT_EQ(bufsize, pread(fd, buf, bufsize, 0)) << strerror(errno); + ASSERT_EQ(0, fstat(fd, &sb1)); + + nap(); + + ASSERT_EQ(bufsize, pread(fd, buf, bufsize, 0)) << strerror(errno); + ASSERT_EQ(0, fstat(fd, &sb2)); + + /* The kernel should automatically update atime during read */ + EXPECT_TRUE(timespeccmp(&sb1.st_atim, &sb2.st_atim, ==)); + EXPECT_TRUE(timespeccmp(&sb1.st_ctim, &sb2.st_ctim, ==)); + EXPECT_TRUE(timespeccmp(&sb1.st_mtim, &sb2.st_mtim, ==)); + + leak(fd); +} + /* Read of an mmap()ed file fails */ TEST_F(ReadSigbus, mmap_eio) { @@ -1068,3 +1323,44 @@ INSTANTIATE_TEST_CASE_P(RA, ReadAhead, tuple(true, 0), tuple(true, 1), tuple(true, 2))); + +/* fuse_init_out.time_gran controls the granularity of timestamps */ +TEST_P(TimeGran, atime_during_setattr) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + const char *CONTENTS = "abcdefgh"; + ssize_t bufsize = strlen(CONTENTS); + uint8_t buf[bufsize]; + uint64_t ino = 42; + const mode_t newmode = 0755; + int fd; + + expect_lookup(RELPATH, ino, bufsize); + expect_open(ino, 0, 1); + expect_read(ino, 0, bufsize, bufsize, CONTENTS); + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + uint32_t valid = FATTR_MODE | FATTR_ATIME; + return (in.header.opcode == FUSE_SETATTR && + in.header.nodeid == ino && + in.body.setattr.valid == valid && + in.body.setattr.atimensec % m_time_gran == 0); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, attr); + out.body.attr.attr.ino = ino; + out.body.attr.attr.mode = S_IFREG | newmode; + }))); + + fd = open(FULLPATH, O_RDWR); + ASSERT_LE(0, fd) << strerror(errno); + + ASSERT_EQ(bufsize, read(fd, buf, bufsize)) << strerror(errno); + ASSERT_EQ(0, fchmod(fd, newmode)) << strerror(errno); + + leak(fd); +} + +INSTANTIATE_TEST_CASE_P(TG, TimeGran, Range(0u, 10u)); diff --git a/tests/sys/fs/fusefs/utils.cc b/tests/sys/fs/fusefs/utils.cc index 4b59f6e26e12..16dfc9c52939 100644 --- a/tests/sys/fs/fusefs/utils.cc +++ b/tests/sys/fs/fusefs/utils.cc @@ -161,7 +161,7 @@ void FuseTest::SetUp() { m_default_permissions, m_push_symlinks_in, m_ro, m_pm, m_init_flags, m_kernel_minor_version, m_maxwrite, m_async, m_noclusterr, m_time_gran, - m_nointr); + m_nointr, m_noatime); /* * FUSE_ACCESS is called almost universally. Expecting it in * each test case would be super-annoying. Instead, set a diff --git a/tests/sys/fs/fusefs/utils.hh b/tests/sys/fs/fusefs/utils.hh index 5d5c5290a60d..a6f1d63ada6b 100644 --- a/tests/sys/fs/fusefs/utils.hh +++ b/tests/sys/fs/fusefs/utils.hh @@ -64,6 +64,7 @@ class FuseTest : public ::testing::Test { bool m_default_permissions; uint32_t m_kernel_minor_version; enum poll_method m_pm; + bool m_noatime; bool m_push_symlinks_in; bool m_ro; bool m_async; @@ -85,6 +86,7 @@ class FuseTest : public ::testing::Test { m_default_permissions(false), m_kernel_minor_version(FUSE_KERNEL_MINOR_VERSION), m_pm(BLOCKING), + m_noatime(false), m_push_symlinks_in(false), m_ro(false), m_async(false),