d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fusefs: fix mmap'd writes in direct_io mode

Browse Source 
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@spawn.link
Reviewed by:	cem
MFC after:	3 days
Differential Revision:	https://reviews.freebsd.org/D26485
This commit is contained in:
asomers 2020-09-24 16:27:53 +00:00
parent 2c79e4bbcd
commit d0def21d9a
2 changed files with 78 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
sys/fs/fuse/fuse_io.c
View File

@ -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");

70
tests/sys/fs/fusefs/write.cc
View File

@ -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
*/