diff options
author | Alan Somers <asomers@FreeBSD.org> | 2020-09-28 00:23:59 +0000 |
---|---|---|
committer | Alan Somers <asomers@FreeBSD.org> | 2020-09-28 00:23:59 +0000 |
commit | df309babcf29db1895ec5fe6bc7f5289231a3b31 (patch) | |
tree | e27f063fbf8ba99158fa139a46f358c4c84fa76e | |
parent | 7d38c4af1773bf0278414e4cf60d540e0a280eb6 (diff) | |
download | src-test2-df309babcf29db1895ec5fe6bc7f5289231a3b31.tar.gz src-test2-df309babcf29db1895ec5fe6bc7f5289231a3b31.zip |
MF stable/12 r366190:
fusefs: fix mmap'd writes in direct_io mode
If a FUSE server returns FOPEN_DIRECT_IO in response to FUSE_OPEN, that
instructs the kernel to bypass the page cache for that file. This feature
is also known by libfuse's name: "direct_io".
However, when accessing a file via mmap, there is no possible way to bypass
the cache completely. This change fixes a deadlock that would happen when
an mmap'd write tried to invalidate a portion of the cache, wrongly assuming
that a write couldn't possibly come from cache if direct_io were set.
Arguably, we could instead disable mmap for files with FOPEN_DIRECT_IO set.
But allowing it is less likely to cause user complaints, and is more in
keeping with the spirit of open(2), where O_DIRECT instructs the kernel to
"reduce", not "eliminate" cache effects.
PR: 247276
Approved by: re (gjb)
Reported by: trapexit@spawn.link
Reviewed by: cem
Differential Revision: https://reviews.freebsd.org/D26485
Notes
Notes:
svn path=/releng/12.2/; revision=366211
-rw-r--r-- | sys/fs/fuse/fuse_io.c | 16 | ||||
-rw-r--r-- | tests/sys/fs/fusefs/write.cc | 70 |
2 files changed, 78 insertions, 8 deletions
diff --git a/sys/fs/fuse/fuse_io.c b/sys/fs/fuse/fuse_io.c index 734053f40653..ea599e9aab34 100644 --- a/sys/fs/fuse/fuse_io.c +++ b/sys/fs/fuse/fuse_io.c @@ -291,6 +291,7 @@ fuse_io_dispatch(struct vnode *vp, struct uio *uio, int ioflag, fuse_vnode_update(vp, FN_MTIMECHANGE | FN_CTIMECHANGE); if (directio) { off_t start, end, filesize; + bool pages = (ioflag & IO_VMIO) != 0; SDT_PROBE2(fusefs, , io, trace, 1, "direct write of vnode"); @@ -301,15 +302,14 @@ fuse_io_dispatch(struct vnode *vp, struct uio *uio, int ioflag, start = uio->uio_offset; end = start + uio->uio_resid; - KASSERT((ioflag & (IO_VMIO | IO_DIRECT)) != - (IO_VMIO | IO_DIRECT), - ("IO_DIRECT used for a cache flush?")); - /* Invalidate the write cache when writing directly */ - err = fuse_inval_buf_range(vp, filesize, start, end); - if (err) - return (err); + if (!pages) { + err = fuse_inval_buf_range(vp, filesize, start, + end); + if (err) + return (err); + } err = fuse_write_directbackend(vp, uio, cred, fufh, - filesize, ioflag, false); + filesize, ioflag, pages); } else { SDT_PROBE2(fusefs, , io, trace, 1, "buffered write of vnode"); diff --git a/tests/sys/fs/fusefs/write.cc b/tests/sys/fs/fusefs/write.cc index 506fefaa071b..f1771e84e999 100644 --- a/tests/sys/fs/fusefs/write.cc +++ b/tests/sys/fs/fusefs/write.cc @@ -923,6 +923,76 @@ TEST_F(WriteBack, o_direct) leak(fd); } +TEST_F(WriteBack, direct_io) +{ + 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); + uint8_t readbuf[bufsize]; + + expect_lookup(RELPATH, ino, 0); + expect_open(ino, FOPEN_DIRECT_IO, 1); + FuseTest::expect_write(ino, 0, bufsize, bufsize, 0, FUSE_WRITE_CACHE, + CONTENTS); + expect_read(ino, 0, bufsize, bufsize, CONTENTS); + + fd = open(FULLPATH, O_RDWR); + EXPECT_LE(0, fd) << strerror(errno); + + ASSERT_EQ(bufsize, write(fd, CONTENTS, bufsize)) << strerror(errno); + /* A subsequent read must query the daemon because cache is empty */ + ASSERT_EQ(0, lseek(fd, 0, SEEK_SET)) << strerror(errno); + ASSERT_EQ(0, fcntl(fd, F_SETFL, 0)) << strerror(errno); + ASSERT_EQ(bufsize, read(fd, readbuf, bufsize)) << strerror(errno); + leak(fd); +} + +/* + * mmap should still be possible even if the server used direct_io. Mmap will + * still use the cache, though. + * + * Regression test for bug 247276 + * https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=247276 + */ +TEST_F(WriteBack, mmap_direct_io) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + const char *CONTENTS = "abcdefgh"; + uint64_t ino = 42; + int fd; + size_t len; + ssize_t bufsize = strlen(CONTENTS); + void *p, *zeros; + + len = getpagesize(); + zeros = calloc(1, len); + ASSERT_NE(nullptr, zeros); + + expect_lookup(RELPATH, ino, len); + expect_open(ino, FOPEN_DIRECT_IO, 1); + expect_read(ino, 0, len, len, zeros); + expect_flush(ino, 1, ReturnErrno(0)); + FuseTest::expect_write(ino, 0, len, len, FUSE_WRITE_CACHE, 0, zeros); + expect_release(ino, ReturnErrno(0)); + + fd = open(FULLPATH, O_RDWR); + EXPECT_LE(0, fd) << strerror(errno); + + p = mmap(NULL, len, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); + ASSERT_NE(MAP_FAILED, p) << strerror(errno); + + memmove((uint8_t*)p, CONTENTS, bufsize); + + ASSERT_EQ(0, munmap(p, len)) << strerror(errno); + close(fd); // Write mmap'd data on close + + free(zeros); +} + /* * When mounted with -o async, the writeback cache mode should delay writes */ |