diff --git a/sys/fs/fuse/fuse_internal.c b/sys/fs/fuse/fuse_internal.c --- a/sys/fs/fuse/fuse_internal.c +++ b/sys/fs/fuse/fuse_internal.c @@ -925,6 +925,7 @@ 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; @@ -949,6 +950,10 @@ 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; @@ -1208,6 +1213,10 @@ 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; @@ -1256,7 +1265,7 @@ } 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 --- a/sys/fs/fuse/fuse_io.c +++ b/sys/fs/fuse/fuse_io.c @@ -236,6 +236,7 @@ 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 @@ 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.h b/sys/fs/fuse/fuse_node.h --- a/sys/fs/fuse/fuse_node.h +++ b/sys/fs/fuse/fuse_node.h @@ -92,6 +92,7 @@ */ #define FN_MTIMECHANGE 0x00000800 #define FN_CTIMECHANGE 0x00001000 +#define FN_ATIMECHANGE 0x00002000 struct fuse_vnode_data { /** self **/ @@ -203,7 +204,7 @@ 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_node.c b/sys/fs/fuse/fuse_node.c --- a/sys/fs/fuse/fuse_node.c +++ b/sys/fs/fuse/fuse_node.c @@ -475,11 +475,13 @@ } 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 */ @@ -487,7 +489,8 @@ 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); @@ -495,6 +498,11 @@ 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_vnops.c b/sys/fs/fuse/fuse_vnops.c --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -613,6 +613,7 @@ 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 @@ 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 --- a/tests/sys/fs/fusefs/cache.cc +++ b/tests/sys/fs/fusefs/cache.cc @@ -75,6 +75,7 @@ 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 --- a/tests/sys/fs/fusefs/io.cc +++ b/tests/sys/fs/fusefs/io.cc @@ -114,6 +114,7 @@ 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.hh b/tests/sys/fs/fusefs/mockfs.hh --- a/tests/sys/fs/fusefs/mockfs.hh +++ b/tests/sys/fs/fusefs/mockfs.hh @@ -356,7 +356,8 @@ 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/mockfs.cc b/tests/sys/fs/fusefs/mockfs.cc --- a/tests/sys/fs/fusefs/mockfs.cc +++ b/tests/sys/fs/fusefs/mockfs.cc @@ -392,7 +392,7 @@ 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 @@ 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/read.cc b/tests/sys/fs/fusefs/read.cc --- a/tests/sys/fs/fusefs/read.cc +++ b/tests/sys/fs/fusefs/read.cc @@ -105,6 +105,13 @@ } }; +class ReadNoatime: public Read { + virtual void SetUp() { + m_noatime = true; + Read::SetUp(); + } +}; + class ReadSigbus: public Read { public: @@ -132,6 +139,14 @@ 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 @@ 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 && + 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 && + 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 @@ 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 @@ 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.hh b/tests/sys/fs/fusefs/utils.hh --- a/tests/sys/fs/fusefs/utils.hh +++ b/tests/sys/fs/fusefs/utils.hh @@ -64,6 +64,7 @@ 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 @@ 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), diff --git a/tests/sys/fs/fusefs/utils.cc b/tests/sys/fs/fusefs/utils.cc --- a/tests/sys/fs/fusefs/utils.cc +++ b/tests/sys/fs/fusefs/utils.cc @@ -161,7 +161,7 @@ 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