fusefs: don't force direct io for files opened O_WRONLY

Previously fusefs would treat any file opened O_WRONLY as though the
FOPEN_DIRECT_IO flag were set, in an attempt to avoid issuing reads as part
of a RMW write operation on a cached part of the file.  However, the FUSE
protocol explicitly allows reads of write-only files for precisely that
reason.

Sponsored by:	The FreeBSD Foundation
This commit is contained in:
asomers 2019-03-30 00:57:07 +00:00
parent b2f9195e90
commit 99552bb6c6
4 changed files with 42 additions and 21 deletions

View File

@ -144,16 +144,7 @@ fuse_filehandle_open(struct vnode *vp, fufh_type_t fufh_type,
fuse_filehandle_init(vp, fufh_type, fufhp, foo->fh);
/*
* For WRONLY opens, force DIRECT_IO. This is necessary
* since writing a partial block through the buffer cache
* will result in a read of the block and that read won't
* be allowed by the WRONLY open.
*/
if (fufh_type == FUFH_WRONLY)
fuse_vnode_open(vp, foo->open_flags | FOPEN_DIRECT_IO, td);
else
fuse_vnode_open(vp, foo->open_flags, td);
fuse_vnode_open(vp, foo->open_flags, td);
out:
fdisp_destroy(&fdi);

View File

@ -648,6 +648,15 @@ fuse_io_strategy(struct vnode *vp, struct buf *bp)
error = fuse_filehandle_getrw(vp,
(bp->b_iocmd == BIO_READ) ? FUFH_RDONLY : FUFH_WRONLY, &fufh);
if (bp->b_iocmd == BIO_READ && error == EBADF) {
/*
* This may be a read-modify-write operation on a cached file
* opened O_WRONLY. The FUSE protocol allows this.
*
* TODO: eliminate this hacky check once the FUFH table is gone
*/
error = fuse_filehandle_get(vp, FUFH_WRONLY, &fufh);
}
if (error) {
printf("FUSE: strategy: filehandles are closed\n");
bp->b_ioflags |= BIO_ERROR;

View File

@ -1208,7 +1208,6 @@ fuse_vnop_open(struct vop_open_args *ap)
int mode = ap->a_mode;
struct thread *td = ap->a_td;
struct ucred *cred = ap->a_cred;
int32_t fuse_open_flags = 0;
fufh_type_t fufh_type;
struct fuse_vnode_data *fvdat;
@ -1228,16 +1227,8 @@ fuse_vnop_open(struct vop_open_args *ap)
fufh_type = fuse_filehandle_xlate_from_fflags(mode);
}
/*
* For WRONLY opens, force DIRECT_IO. This is necessary since writing
* a partial block through the buffer cache will result in a read of
* the block and that read won't be allowed by the WRONLY open.
*/
if (fufh_type == FUFH_WRONLY || (fvdat->flag & FN_DIRECTIO) != 0)
fuse_open_flags = FOPEN_DIRECT_IO;
if (fuse_filehandle_validrw(vp, fufh_type) != FUFH_INVALID) {
fuse_vnode_open(vp, fuse_open_flags, td);
fuse_vnode_open(vp, 0, td);
return 0;
}

View File

@ -391,7 +391,7 @@ TEST_F(Write, DISABLED_mmap)
free(zeros);
}
TEST_F(Write, pwrite)
TEST_F(WriteThrough, pwrite)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
const char RELPATH[] = "some_file.txt";
@ -521,6 +521,36 @@ TEST_F(WriteBack, close)
close(fd);
}
/*
* In writeback mode, writes to an O_WRONLY file could trigger reads from the
* server. The FUSE protocol explicitly allows that.
*/
TEST_F(WriteBack, rmw)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
const char RELPATH[] = "some_file.txt";
const char *CONTENTS = "abcdefgh";
const char *INITIAL = "XXXXXXXXXX";
uint64_t ino = 42;
uint64_t offset = 1;
off_t fsize = 10;
int fd;
ssize_t bufsize = strlen(CONTENTS);
expect_lookup(RELPATH, ino, 0);
expect_open(ino, 0, 1);
expect_getattr(ino, fsize);
expect_read(ino, 0, fsize, fsize, INITIAL);
expect_write(ino, offset, bufsize, bufsize, 0, CONTENTS);
fd = open(FULLPATH, O_WRONLY);
EXPECT_LE(0, fd) << strerror(errno);
ASSERT_EQ(bufsize, pwrite(fd, CONTENTS, bufsize, offset))
<< strerror(errno);
/* Deliberately leak fd. close(2) will be tested in release.cc */
}
/*
* Without direct_io, writes should be committed to cache
*/