Date: Tue, 03 Sep 2019 14:06:00 -0000 From: Alan Somers <asomers@FreeBSD.org> To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r345722 - in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs Message-ID: <201903300057.x2U0v7qp088358@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: asomers Date: Sat Mar 30 00:57:07 2019 New Revision: 345722 URL: https://svnweb.freebsd.org/changeset/base/345722 Log: 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 Modified: projects/fuse2/sys/fs/fuse/fuse_file.c projects/fuse2/sys/fs/fuse/fuse_io.c projects/fuse2/sys/fs/fuse/fuse_vnops.c projects/fuse2/tests/sys/fs/fusefs/write.cc Modified: projects/fuse2/sys/fs/fuse/fuse_file.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_file.c Sat Mar 30 00:54:01 2019 (r345721) +++ projects/fuse2/sys/fs/fuse/fuse_file.c Sat Mar 30 00:57:07 2019 (r345722) @@ -144,16 +144,7 @@ fuse_filehandle_open(struct vnode *vp, fufh_type_t fuf 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); Modified: projects/fuse2/sys/fs/fuse/fuse_io.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_io.c Sat Mar 30 00:54:01 2019 (r345721) +++ projects/fuse2/sys/fs/fuse/fuse_io.c Sat Mar 30 00:57:07 2019 (r345722) @@ -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; Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_vnops.c Sat Mar 30 00:54:01 2019 (r345721) +++ projects/fuse2/sys/fs/fuse/fuse_vnops.c Sat Mar 30 00:57:07 2019 (r345722) @@ -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; } Modified: projects/fuse2/tests/sys/fs/fusefs/write.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/write.cc Sat Mar 30 00:54:01 2019 (r345721) +++ projects/fuse2/tests/sys/fs/fusefs/write.cc Sat Mar 30 00:57:07 2019 (r345722) @@ -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"; @@ -519,6 +519,36 @@ TEST_F(WriteBack, close) ASSERT_EQ(bufsize, write(fd, CONTENTS, bufsize)) << strerror(errno); 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 */ } /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201903300057.x2U0v7qp088358>