svn commit: r366121 - in head: sys/fs/fuse tests/sys/fs/fusefs

[Alan Somers]

Author: asomers
Date: Thu Sep 24 16:27:53 2020
New Revision: 366121
URL: https://svnweb.freebsd.org/changeset/base/366121

Log:
  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
  Reported by:	trapexit at spawn.link
  Reviewed by:	cem
  MFC after:	3 days
  Differential Revision:	https://reviews.freebsd.org/D26485

Modified:
  head/sys/fs/fuse/fuse_io.c
  head/tests/sys/fs/fusefs/write.cc

Modified: head/sys/fs/fuse/fuse_io.c
==============================================================================
--- head/sys/fs/fuse/fuse_io.c	Thu Sep 24 16:21:30 2020	(r366120)
+++ head/sys/fs/fuse/fuse_io.c	Thu Sep 24 16:27:53 2020	(r366121)
@@ -291,6 +291,7 @@ fuse_io_dispatch(struct vnode *vp, struct uio *uio, in
 		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, in
 
 			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");

Modified: head/tests/sys/fs/fusefs/write.cc
==============================================================================
--- head/tests/sys/fs/fusefs/write.cc	Thu Sep 24 16:21:30 2020	(r366120)
+++ head/tests/sys/fs/fusefs/write.cc	Thu Sep 24 16:27:53 2020	(r366121)
@@ -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
  */